chore: integration: add test for pushing to cache repo that requires auth #233
Conversation
| @@ -1101,6 +1107,130 @@ func TestPushImage(t *testing.T) { | |||
| require.NoError(t, err) | |||
| }) | |||
|
|
|||
| t.Run("CacheAndPushAuth", func(t *testing.T) { | |||
There was a problem hiding this comment.
review: I may look into making this more of a table-driven test, as it's getting pretty unwieldy
There was a problem hiding this comment.
I was gonna suggest the same.
There was a problem hiding this comment.
I had a try and there's a bit too much difference behaviour between the various test cases to really make it worthwhile IMO compared with the cognitive overhead of DRYing this.
| @@ -249,18 +249,3 @@ func WriteFile(t *testing.T, fs billy.Filesystem, path, content string) { | |||
| err = file.Close() | |||
| require.NoError(t, err) | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
review: moved as this isn't specific to testing git stuff
There was a problem hiding this comment.
LGTM
NOTE: this shows that being unable to push the image will fail the build outright.
Do we maybe want to relax this?
The pushed image is just an optimization to speed up the subsequent runs, correct?
It feels like Principle of Least Surprise here would be to complete the primary objective (which is building), and any ancillary steps to optimize should not fail that.
| @@ -1101,6 +1107,130 @@ func TestPushImage(t *testing.T) { | |||
| require.NoError(t, err) | |||
| }) | |||
|
|
|||
| t.Run("CacheAndPushAuth", func(t *testing.T) { | |||
There was a problem hiding this comment.
I was gonna suggest the same.
| envbuilderEnv("GET_CACHED_IMAGE", "1"), | ||
| envbuilderEnv("DOCKER_CONFIG_BASE64", base64.StdEncoding.EncodeToString(regAuthJSON)), | ||
| }}) | ||
| require.NoError(t, err) |
There was a problem hiding this comment.
Is there any more specificity we could add here? The absence of an error is fine, but validating the cached image exists explicitly would be better.
As a counterpoint, I can foresee this being used in a CI scenario along with something like |
Addresses a gap in our testing for the case when the cache repo requires auth.
NOTE: this shows that being unable to push the image will fail the build outright.
Do we maybe want to relax this?