Add value testing to the assign mutator #1113
Conversation
|
note that this change depends on #1101 |
Codecov Report
@@ Coverage Diff @@
## master #1113 +/- ##
==========================================
+ Coverage 47.84% 48.63% +0.79%
==========================================
Files 62 63 +1
Lines 4264 4369 +105
==========================================
+ Hits 2040 2125 +85
- Misses 1966 1989 +23
+ Partials 258 255 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
maxsmythe
force-pushed
the
valuetest
branch
2 times, most recently
from
1686141
to
ae1a67e
Compare
| // IfNotIn Only mutate if the current value is NOT in the supplied list | ||
| IfNotIn []string `json:"ifNotIn,omitempty"` | ||
|
|
||
| // once https://github.com/kubernetes-sigs/controller-tools/pull/528 |
There was a problem hiding this comment.
It seems to have been merged - should this PR be updated or will that be a followup?
There was a problem hiding this comment.
follow-up. It's been merged, but I don't know if a version of the tool with that feature included has been released.
Signed-off-by: Max Smythe <smythe@google.com>
Signed-off-by: Max Smythe <smythe@google.com>
Signed-off-by: Max Smythe <smythe@google.com>
Signed-off-by: Max Smythe <smythe@google.com>
Signed-off-by: Max Smythe <smythe@google.com>
| @@ -25,6 +25,9 @@ func mutate(mutator types.Mutator, tester *path.Tester, obj *unstructured.Unstru | |||
| type mutatorState struct { | |||
| mutator types.Mutator | |||
| tester *path.Tester | |||
| // valueTest takes the input value and whether that value already existed. | |||
| // It returns true if the value should be mutated | |||
| valueTest func(interface{}, bool) bool | |||
There was a problem hiding this comment.
Just curious - was this made pluggable to facilitate testing?
There was a problem hiding this comment.
For the AssignMetadata mutator, which currently has no value test b/c it already requires target values to be undefined.
* Add locking, deepcopy path tester memoization Signed-off-by: Max Smythe <smythe@google.com> * Add path parsing error context Signed-off-by: Max Smythe <smythe@google.com> * Tester should also have a DeepCopy method Signed-off-by: Max Smythe <smythe@google.com> * Eager initialize path tester, guard against nil Signed-off-by: Max Smythe <smythe@google.com> * Add value test to assign mutator Signed-off-by: Max Smythe <smythe@google.com>
What this PR does / why we need it:
This PR adds value testing to the assign mutator, per the mutation design.
Note that the schema of the value tests has been changed slightly, I now assume a format like:
Which issue(s) this PR fixes (optional, using
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when the PR gets merged):Fixes #1080
Special notes for your reviewer: