make failing test showing spurious variable conflicts #1634
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/M
|
/assign @monopole |
|
Not sure I went as far as you'd like with the functional "sugar" for the tests @monopole, but I tried to clean it up in a way that I felt would be much more readable for future maintainers. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: tkellen The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
size/L
tkellen
force-pushed
the
gh-1600-failing
branch
from
643fcd6 to
d851305
Compare
k8s-ci-robot
added
size/M
|
|
||
| func makeVarToNamepaceAndPath( | ||
| name string, | ||
| namespace string, |
There was a problem hiding this comment.
namespace isn't used, did you mean env?
There was a problem hiding this comment.
Whoops. No, I did mean to use it, though clearly it isn't strictly required I felt it helped illustrate my particular incarnation of this issue. I've updated this.
| } | ||
| } | ||
|
|
||
| func TestResolveVarConflicts(t *testing.T) { |
There was a problem hiding this comment.
this test reads like an example, which is perfect. thanks!
There was a problem hiding this comment.
My pleasure, thanks for prompting me to take a bit more care.
| // value do not produce a conflict. | ||
| err := ac0.MergeAccumulator(ac1) | ||
| if err != nil { | ||
| t.Fatalf("dupe var names w/ same concrete val should not conflict: %v", err) |
There was a problem hiding this comment.
so the current form of this test should be
if err == nil {
t.Fatalf("see bug whatever")
}
the subsequent PR to fix that bug should change this line.
we cannot check in failing tests, but we can check in tests that succeed showing behavior we want to change
There was a problem hiding this comment.
Got it. Updated this and the now-dependent PR.
Per @monopole's request in https://github.com/kubernetes-sigs/kustomize/pull/1620/files#r334282443, this PR isolates a failing test for gh-1600.