e2e: new ExitCleanly matcher #19802
e2e: new ExitCleanly matcher #19802
Conversation
openshift-ci
bot
added
release-note-none
approved
edsantiago
force-pushed
the
e2e_exit_cleanly_matcher
branch
from
233ca95 to
ffd4ae7
Compare
| NGINX_IMAGE = "quay.io/lsm5/alpine_nginx-aarch64:latest" //nolint:revive,stylecheck | ||
| BB_GLIBC = "docker.io/library/busybox:glibc" //nolint:revive,stylecheck | ||
| REGISTRY_IMAGE = "quay.io/libpod/registry:2.8.2" //nolint:revive,stylecheck | ||
| LABELS_IMAGE = "quay.io/libpod/alpine_labels:latest" //nolint:revive,stylecheck | ||
| CITEST_IMAGE = "quay.io/libpod/testimage:20221018" //nolint:revive,stylecheck |
There was a problem hiding this comment.
Two questions for further consideration. They don't need to be addressed for this PR.
-
As this variable is referenced in at least two places, is there a place it could be added to and then pulled into here?
-
Just looking at the testimage date. It's almost a year old, is it time to bump it?
There was a problem hiding this comment.
Could you rephrase question 1? I can't find a way to understand it. I'm sorry.
Bumping: absent any pressing need, bumping doesn't really gain us anything...
|
LGTM |
edsantiago
force-pushed
the
e2e_exit_cleanly_matcher
branch
from
ffd4ae7 to
b449017
Compare
|
Unsurprisingly, it got complicated. f38 root containerized is showing a ton of: Looking deeper I see two things:
This is not something I'll be able to fix. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: edsantiago, 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 |
|
Filed #19809 for the syslog bug. This is blocked until that gets resolved. |
Finally, after so many years, let's start using testimage:YYYYMMDD. Use it in place of LABELS_IMAGE, which nothing/nowhere was using. Signed-off-by: Ed Santiago <[email protected]>
edsantiago
force-pushed
the
e2e_exit_cleanly_matcher
branch
from
b449017 to
67afd46
Compare
Combined test for (exitcode == 0) && (nothing on stderr). Returns more useful diagnostic messages than the default: old: Expected N to equal 0 new: Command failed with exit status N new: Unexpected warnings seen on stderr: "...." Signed-off-by: Ed Santiago <[email protected]>
A nearly-trivial first effort to use the new ExitCleanly().
Requires using the new CITEST_IMAGE (see prior commit)
because nginx causes the tests to fail:
[FAILED] Unexpected warnings seen on stderr: \
level=warning \
msg="HEALTHCHECK is not supported for OCI image format ...
Oh, I also took the liberty of rewriting "play kube" -> "kube play".
Signed-off-by: Ed Santiago <[email protected]>
edsantiago
force-pushed
the
e2e_exit_cleanly_matcher
branch
from
67afd46 to
4082b67
Compare
| // FIXME: #19809, "failed to connect to syslog" warnings on f38 | ||
| // FIXME: so, until that is fixed, don't check stderr if containerized | ||
| if !Containerized() { |
There was a problem hiding this comment.
Given:
- I can't figure out where that warning is coming from; and
- enabling some testing is better than what we have now
...I've decided to kludge in a manual exception to skip warn-testing when running in container.
@containers/podman-maintainers please take another look
|
/lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
e2e test logs are full of warning messages that we never see. Let's start looking for them.
This PR introduces a new gomega matcher,
ExitCleanly(), with the purpose of replacingShould(Exit(0)). This matcher checks exit code and that no unexpected messages are emitted on stderr. Checking stdout is still up to the test writer.As proof of concept, use the new matcher in
play_build_test.go. This resulted in tests failing, because all thosekube playcommands have been spitting out warnings for lo all these years:To remove that warning, use our standard system-test
libpod/testimage:YYYYMMDDimage instead ofalpine_nginx. Image is now fetched and cached as part of standard test prep; I expect to use it much more in the future.FWIW I tried updating
play_kube_test.gobut discovered and filed #19801, a bug that makes that impossible.