Enhance testing for `document` package. #1180

j0n3lson · 2020-03-15T05:54:45Z

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

In support of #1016 this PR enhances and increase code coverage within document package.

Fixes

When a category (kind) has multiple note entries, ensure each entry is printed on a separate line.
Remove leading unwanted characters from note markdowns. The default go template
prepends a dash ("-") before each note.

Deprecation:

Removes TestNoteCollection and TestRenderMarkdownTeamplateGoldenFile. Both of these methods have been replaced with TestDocument_RenderMarkdownTemplate which tests the same functionality.
Removes sortKinds() and associated test case. This method was deprecated in
PR#1148 when sorting was moved to CreateDocument()

Enhancements:

Add a test case for CreateDocument().
When testing template rendering, use CreateDocument() the same way a package user would. This makes it easier to test from the user's point of view.

Which issue(s) this PR fixes:

NONE

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

k8s-ci-robot · 2020-03-15T05:54:53Z

Hi @j0n3lson. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

j0n3lson · 2020-03-15T05:54:58Z

/assign @saschagrunert

j0n3lson · 2020-03-15T06:48:14Z

/hold I found a bug in the rendering. I will update the PR once fixed.

saschagrunert

/lgtm

saschagrunert · 2020-03-15T08:24:30Z

/ok-to-test
/approve

justaugustus · 2020-03-16T00:48:27Z

/lgtm cancel
(since @j0n3lson mentioned there was a bug)

justaugustus · 2020-03-16T00:49:07Z

/kind cleanup
/priority important-soon

j0n3lson · 2020-03-16T03:00:01Z

/hold cancel

Ready to go. The bug was for entries not printing on their new line. I introduced the back a few commits back while trying to fix the "no-new-line-at-end-of-document-problem" which annoyingly doesn't seem possible in golang templates. I settled for strings.TrimSuffix() on the rendered template.

pkg/notes/document/document.go

saschagrunert

/lgtm

pkg/notes/document/template.go

droslean · 2020-03-18T08:46:58Z

/retest

droslean · 2020-03-18T09:05:11Z

/test pull-release-test

saschagrunert · 2020-03-18T09:09:13Z

@j0n3lson might give it a rebase to pull in the latest test-fixes

j0n3lson · 2020-04-09T15:56:14Z

Hey Sasha, I fixed it in my local side. I've not had a chance to look again in a bit. I think I'll have time on Friday to rebase and push. I was only blocked on making the test pass.

saschagrunert · 2020-04-14T14:47:43Z

Hey Sasha, I fixed it in my local side. I've not had a chance to look again in a bit. I think I'll have time on Friday to rebase and push. I was only blocked on making the test pass.

Awesome, thank you for the update and keeping us posted.

j0n3lson · 2020-05-02T18:13:56Z

Heya @saschagrunert just crawled out from my rock. Mind taking a look now?

Fixes: * When a category (kind) has multiple note entries, ensure each entry is printed on a separate line. * Remove leading unwanted characters from note markdowns. The default go template prepends a dash ("-") before each note. Deprecation: * Removes `TestNoteCollection()` and `TestRenderMarkdownTeamplateGoldenFile()`. Both of these methods have been replaced with `TestDocument_RenderMarkdownTemplate` which tests the same functionality. * Removes `sortKinds()` and associated test case. This method was deprecated in PR#1148 when sorting was moved to `CreateDocument()` Enhancements: * Add a test case for `CreateDocument()`. * When testing template rendering, use `CreateDocument()` the same way a package user would. This makes it easier to test from the user's point of view.

saschagrunert

/lgtm

cpanato

/lgtm

justaugustus · 2020-05-04T09:30:32Z

/lgtm
/approve

Thanks!

k8s-ci-robot · 2020-05-04T09:30:49Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cpanato, j0n3lson, justaugustus, saschagrunert

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [justaugustus]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-priority labels Mar 15, 2020

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Mar 15, 2020

k8s-ci-robot assigned saschagrunert Mar 15, 2020

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 15, 2020

k8s-ci-robot requested review from cpanato and jeefy March 15, 2020 05:55

k8s-ci-robot added area/release-eng Issues or PRs related to the Release Engineering subproject sig/release Categorizes an issue or PR as relevant to SIG Release. labels Mar 15, 2020

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 15, 2020

saschagrunert approved these changes Mar 15, 2020

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 15, 2020

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 15, 2020

k8s-ci-robot assigned justaugustus Mar 16, 2020

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 16, 2020

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 16, 2020

j0n3lson requested a review from saschagrunert March 16, 2020 03:00

saschagrunert reviewed Mar 16, 2020

View reviewed changes

pkg/notes/document/document.go Outdated Show resolved Hide resolved

j0n3lson requested a review from saschagrunert March 17, 2020 03:19

saschagrunert approved these changes Mar 17, 2020

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 17, 2020

saschagrunert reviewed Mar 17, 2020

View reviewed changes

pkg/notes/document/template.go Show resolved Hide resolved

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 18, 2020

droslean mentioned this pull request Mar 18, 2020

[Flaky Test] release-test (pull-release-test) kubernetes/kubernetes#89197

Closed

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 21, 2020

jeefy mentioned this pull request Mar 26, 2020

Set ReleaseVersion for generated notes #1217

Merged

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 2, 2020

j0n3lson requested a review from saschagrunert May 2, 2020 18:14

saschagrunert approved these changes May 4, 2020

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 4, 2020

cpanato approved these changes May 4, 2020

View reviewed changes

k8s-ci-robot assigned cpanato May 4, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 4, 2020

k8s-ci-robot merged commit 5ad08a0 into kubernetes:master May 4, 2020

k8s-ci-robot added this to the v1.19 milestone May 4, 2020

Enhance testing for document package. #1180

Enhance testing for document package. #1180

Uh oh!

Conversation

j0n3lson commented Mar 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

What this PR does / why we need it:

Fixes

Deprecation:

Enhancements:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Uh oh!

k8s-ci-robot commented Mar 15, 2020

Uh oh!

j0n3lson commented Mar 15, 2020

Uh oh!

j0n3lson commented Mar 15, 2020

Uh oh!

saschagrunert left a comment

Choose a reason for hiding this comment

Uh oh!

saschagrunert commented Mar 15, 2020

Uh oh!

justaugustus commented Mar 16, 2020

Uh oh!

justaugustus commented Mar 16, 2020

Uh oh!

j0n3lson commented Mar 16, 2020

Uh oh!

Uh oh!

saschagrunert left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

droslean commented Mar 18, 2020

Uh oh!

droslean commented Mar 18, 2020

Uh oh!

saschagrunert commented Mar 18, 2020

Uh oh!

j0n3lson commented Apr 9, 2020 via email

Uh oh!

saschagrunert commented Apr 14, 2020

Uh oh!

j0n3lson commented May 2, 2020

Uh oh!

saschagrunert left a comment

Choose a reason for hiding this comment

Uh oh!

cpanato left a comment

Choose a reason for hiding this comment

Uh oh!

justaugustus commented May 4, 2020

Uh oh!

k8s-ci-robot commented May 4, 2020

Uh oh!

Uh oh!

Enhance testing for `document` package. #1180

Enhance testing for `document` package. #1180

j0n3lson commented Mar 15, 2020 •

edited

Loading