remove redandunt check for imagestore create #1800
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1800 +/- ##
==========================================
+ Coverage 48.71% 48.73% +0.01%
==========================================
Files 81 81
Lines 8116 8113 -3
==========================================
Hits 3954 3954
+ Misses 3515 3513 -2
+ Partials 647 646 -1
Continue to review full report at Codecov.
|
| @@ -177,7 +173,7 @@ func (s *imageStore) Update(ctx context.Context, image images.Image, fieldpaths | |||
| updated = image | |||
| } | |||
|
|
|||
| if err := validateImage(&image); err != nil { | |||
| if err := validateImage(&updated); err != nil { | |||
There was a problem hiding this comment.
Could you add a test case that fails without this change?
There was a problem hiding this comment.
I am trying to think of a test case here that would meet that criteria. A test could exist which errors with invalid values in image but are not in the field path and therefore do not get set in updated, however I am not sure that is a useful test. We currently don't enforce the overall label size so can't test that the update pushes the resulting updated into an invalid state. Every other invalid input I can think of would fail with both image and updated.
There was a problem hiding this comment.
@dmcgowan The problem here is that I've scoured this code twice and seen this mistake twice. I am not sure this actually matters if we don't think a test case can catch it.
A possible test case here would be to have a field that is validated with a wrong value, but not a part of the field path. Its possible none may exist for the image type.
There was a problem hiding this comment.
@yanxuean To be clear, no need to add a test for this case, as its not possible. ;)
|
nit: typo in commit message |
|
LGTM The test doesn't seem possible or necessary. I'll give this a day for the spelling error to correct itself. |
yanxuean
force-pushed
the
image-store-redandunt
branch
from
e16b3f8
to
3b670eb
Compare
done @dmcgowan |
correct check object for imagestore update Signed-off-by: yanxuean <yan.xuean@zte.com.cn>
|
LGTM |
1 similar comment
|
LGTM |
Signed-off-by: yanxuean yan.xuean@zte.com.cn