test, manifest: test push retry #20242
Conversation
Signed-off-by: Aditya R <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: flouthoc The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
flouthoc
force-pushed
the
manifest-retry
branch
2 times, most recently
from
3e57e66 to
e07aae8
Compare
test/e2e/manifest_test.go
Outdated
| push := podmanTest.Podman([]string{"manifest", "push", "--all", "--tls-verify=false", "--remove-signatures", "foo", "localhost:7000/bogus"}) | ||
| push.WaitWithDefaultTimeout() | ||
| Expect(push).Should(Exit(1)) | ||
| Expect(push.ErrorToString()).To(ContainSubstring("Failed, retrying in 1s ... (1/3).")) |
There was a problem hiding this comment.
I wonder if this is correct way to capture warnings before error ?
There was a problem hiding this comment.
Yes, that looks good to me. Any chance we can avoid the 3 seconds delay in the test?
There was a problem hiding this comment.
LGTM also. Strictly speaking, that doesn't actually confirm that there was a retry, so something like this would be more confidence-inspiring:
Expect(push.ErrorToString()).To(MatchRegexp("Copying blob.*Failed, retrying in 1s \\.\\.\\. \\(1/3\\).*Copying blob.*Failed, retrying in 2s"))...but this is way in the weeds. I don't see a realistic way for "Failed, retrying" to appear but the retry itself not to happen.
Thank you for remembering to add a test!
There was a problem hiding this comment.
@edsantiago thanks I'll amend, could you let me know why current warning catch worked in some tests and failed in others ?
There was a problem hiding this comment.
Oh! Because remote. You'll need to SkipIfRemote().
There was a problem hiding this comment.
This is a good moment to remind everyone of my greasemonkey Cirrus colorizer. Look how easy it is to find the common element (pink) in this results list:
There was a problem hiding this comment.
ah warnings are not relayed in remote setup. @edsantiago thanks for pointing amended the test now.
flouthoc
force-pushed
the
manifest-retry
branch
from
e07aae8 to
9b088ea
Compare
Test: containers/common#1666 Signed-off-by: Aditya R <[email protected]>
flouthoc
force-pushed
the
manifest-retry
branch
from
9b088ea to
b576b79
Compare
|
LGTM |
|
/lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
Test: containers/common#1666