testcontainers-java icon indicating copy to clipboard operation
testcontainers-java copied to clipboard

Update RemoteDockerImage to not retry on fatal error, allow user to specify custom retry time limit

Open Kami opened this issue 5 years ago • 2 comments

This pull request resolves "issue" reported in #829.

It updates the code so we don't try to retry on "no basic auth credentials" error which would get returned in case Docker credentials helper is not configured (e.g. when using private registries). This error is fatal so we should not retry on it.

In addition to that, I updated the constructor so users can specify a custom retry time limit.

Kami avatar Feb 23 '21 16:02 Kami

As far as tests go - I couldn't find existing tests for the whole logic inRemoteDockerImage.resolve() (please correct me if I'm wrong).

I could split "fatal error detection logic" in a static utility function and just add a unit test for that. Ideally though, we would have a better test for the whole resolve() method, but this would require mocking a bunch of things (if there are already any existing tests with that pattern which I could use as a starting point, please point me to them).

Kami avatar Feb 23 '21 16:02 Kami

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you believe this is a mistake, please reply to this comment to keep it open. If there isn't one already, a PR to fix or at least reproduce the problem in a test case will always help us get back on track to tackle this.

stale[bot] avatar Jun 18 '21 23:06 stale[bot]