Add unit tests for kustomize#1828
Conversation
Signed-off-by: David Gageot <david@gageot.net>
dgageot
requested review from
balopat,
nkubala,
priyawadhwa and
r2d4
as code owners
Codecov Report
@@ Coverage Diff @@
## master #1828 +/- ##
==========================================
+ Coverage 45.42% 46.17% +0.74%
==========================================
Files 143 143
Lines 6683 6685 +2
==========================================
+ Hits 3036 3087 +51
+ Misses 3341 3287 -54
- Partials 306 311 +5
Continue to review full report at Codecov.
|
nkubala
left a comment
nkubala
left a comment
There was a problem hiding this comment.
one small nit but otherwise 👍
| command: testutil.NewFakeCmd(t). | ||
| WithRunOut("kubectl version --client -ojson", kubectlVersion). | ||
| WithRunOut("kustomize build "+tmpDir.Root(), deploymentWebYAML). | ||
| WithRun("kubectl --context kubecontext --namespace testNamespace apply --force -f -"), |
There was a problem hiding this comment.
not a huge deal, but can you use the variables testKubeContext and testNamespace here instead of the strings?
WithRun("kubectl --context " + testKubeContext + " -- namespace " + testNamespace + " apply --force -f -")or
WithRun(fmt.Sprintf("kubectl --context %s --namespace %s apply --force -f -", testKubeContext, testNamespace))There was a problem hiding this comment.
I don't know about that. Those strings are basically assertions and I'd rather hard code the expected result than compute it. I know it's debatable...
There was a problem hiding this comment.
ah ok I misunderstood, I thought you were actually generating the command to run here. it does feel weird to me that this test will break if we rename a variable somewhere...but I see where you're coming from. I don't feel strongly enough about it to push for you to change it :)
4 participants
Signed-off-by: David Gageot david@gageot.net