Don’t compute onbuild triggers for images that are stage names #938
Conversation
Codecov Report
@@ Coverage Diff @@
## master #938 +/- ##
==========================================
+ Coverage 43.38% 43.55% +0.16%
==========================================
Files 63 63
Lines 2639 2645 +6
==========================================
+ Hits 1145 1152 +7
+ Misses 1374 1373 -1
Partials 120 120
Continue to review full report at Codecov.
|
pkg/skaffold/docker/parse_test.go
Outdated
| @@ -171,6 +176,10 @@ var ImageConfigs = map[string]*v1.ConfigFile{ | |||
| } | |||
|
|
|||
| func mockRetrieveImage(image string) (*v1.ConfigFile, error) { | |||
| if image == "base" { | |||
| panic("we should never retrieve images which are stage names") | |||
There was a problem hiding this comment.
panic is considered a bad practice in testing as it stops the test execution - can we just return an error, or pass in testing.T?
There was a problem hiding this comment.
also assertion logic should not be embedded in the mock, this makes a test hard to understand.
There was a problem hiding this comment.
suggestion: reorganize the mock with usual "capture" semantics, that records the calls as a slice of strings and make the retrieved images an explicit assertion in the tests.
Also verifies which images are actually fetched. Signed-off-by: David Gageot <[email protected]>
|
@balopat should be all good now. |
Fix #502