Fix trace for optional GitRepository.Spec.Reference#1626
Fix trace for optional GitRepository.Spec.Reference#1626hiddeco merged 2 commits intofluxcd:mainfrom
Conversation
|
Thanks for your contribution! We will get this reviewed soon 💯 |
There was a problem hiding this comment.
Thanks @allenporter, first-time contributor high five ✋
Looks reasonable to me. Usually I would like to see a test, to demonstrate the fix and prevent regressions, but for this it might be fiddly to test the relevant bit since it's all embedded in the command-line code.
(I must leave it for a fluxcd/flux2 maintainer to put their stamp to this, and to merge; I've 📣ed them)
relu
left a comment
relu
left a comment
There was a problem hiding this comment.
LGTM ✔️
Thanks for your contribution, @allenporter!
|
Yeah I looked and didn't see obvious existing tests for this, but not sure I am looking for the right thing. Do you have a pointer of a "starter" test to base this on? I'm generally interested in learning more about the code base for next time. |
|
@allenporter indeed there are no tests inside cmd you can refer to, however, you can check the helm cmd tests which are likely a good reference, you'll notice files from the testdata/output directory are used to compare the string output. Another good reference is kubectl. Hope that helps. |
|
@allenporter To be clear, I didn't mean to make tests a requirement for this PR. It would make a great follow-up, though :-D I would start by factoring out helper funcs and writing unit tests for those. |
|
Yes, thanks, understood. This is ready for review/merge. |
|
@allenporter if you rebase this branch against |
Check for existence of GitRepository.Spec.Reference when displaying a trace to avoid error: ✗ template: tmpl:28:21: executing "tmpl" at <.GitRepository.Spec.Reference.Tag>: nil pointer evaluating *v1beta1.GitRepositoryRef.Tag Fixes issue fluxcd#1621 Manually tested using the use cases highlighted in the issue. Signed-off-by: Allen Porter <allen@thebends.org>
Signed-off-by: Allen Porter <allen@thebends.org>
|
Change was rebased, thanks. |
|
First-time contributor high-five @allenporter 🥇 |
Add tests for flux trace command that fake out the kubernetes client, load objects from a yaml file and create them in the client, and assert on the output of the trace command to an expected golden file. This is a follow up from the suggestions in PR fluxcd#1626 which suggested that additional testing would be helpful. This test approach is modeled after the helm command tests. This required some changes to the kubernetes client setup to make it possible to use a fake. If we agree this pattern makes sense, it can be applied to other commands.
Add tests for flux trace command that fake out the kubernetes client, load objects from a yaml file and create them in the client, and assert on the output of the trace command to an expected golden file. This is a follow up from the suggestions in PR fluxcd#1626 which suggested that additional testing would be helpful. This test approach is modeled after the helm command tests. This required some changes to the kubernetes client setup to make it possible to use a fake. If we agree this pattern makes sense, it can be applied to other commands. Signed-off-by: Allen Porter <allen@thebends.org>
Add tests for flux trace command that fake out the kubernetes client, load objects from a yaml file and create them in the client, and assert on the output of the trace command to an expected golden file. This is a follow up from the suggestions in PR fluxcd#1626 which suggested that additional testing would be helpful. This test approach is modeled after the helm command tests. This required some changes to the kubernetes client setup to make it possible to use a fake. If we agree this pattern makes sense, it can be applied to other commands. Signed-off-by: Allen Porter <allen@thebends.org>
Add tests for flux trace command that fake out the kubernetes client, load objects from a yaml file and create them in the client, and assert on the output of the trace command to an expected golden file. This is a follow up from the suggestions in PR fluxcd#1626 which suggested that additional testing would be helpful. This test approach is modeled after the helm command tests. This required some changes to the kubernetes client setup to make it possible to use a fake. If we agree this pattern makes sense, it can be applied to other commands. Signed-off-by: Allen Porter <allen@thebends.org>
5 participants
Check for existence of GitRepository.Spec.Reference when displaying a trace to
avoid error:
✗ template: tmpl:28:21: executing "tmpl" at <.GitRepository.Spec.Reference.Tag>: nil pointer evaluating *v1beta1.GitRepositoryRef.Tag
Fixes issue #1621
Manually tested using the use cases highlighted in the issue.
Signed-off-by: Allen Porter allen@thebends.org