Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test/system: Remove unnecessary --assumeyes, optimize the 'create' tests, etc. #1595

Conversation

debarshiray
Copy link
Member

No description provided.

The toolbox(1) binary always relies on the PATH environment variable to
find the podman(1) and skopeo(1) binaries.  There's no way to override
those with the PODMAN and SKOPEO environment variables, and they only
affect any direct use of podman(1) and skopeo(1) within the test suite.

Therefore, offering the PODMAN and SKOPEO environment variables in their
current form is needlessly confusing and misleading, and can lead to
surprises arising from different podman(1) and skopeo(1) binaries being
used in different places.  Either toolbox(1) should also honour them or
the test suite shouldn't offer them.  The former is more complicated
without any obvious need for it, so the latter was chosen.

containers#1592
It shouldn't be necessary to use the --assumeyes option when creating a
Toolbx container, if the corresponding image is already present in the
local containers/storage image store.  It's harmful to test it with the
option, even when it shouldn't be needed, because it's off by default
and most users won't enable it.

Therefore, it's better to test the most common scenario that most users
will encounter.

containers#1595
@debarshiray debarshiray force-pushed the wip/rishi/test-system-create-drop-unnecessary-assumeyes-update-run-flags branch from b46b98f to 91c5396 Compare November 14, 2024 11:43
The system tests can be very I/O intensive, because many of them copy
OCI images from the test suite's image cache directory to its local
container/storage store, create containers, and then delete everything
to run the next test with a clean slate.  This makes them slow.

In the case of these two particular tests, toolbox(1) is supposed to
validate the command line options before trying to find the image.  So,
there's no need to copy the image from the test suite's image cache
directory to its local container/storage store.

If the toolbox(1) code breaks, then either it won't throw the expected
error or it will download the image before validating the command line
options.  The first possibility will be easily detected.  The other
could have been harder to notice, but, fortunately, commit
55c0e63 added variants of these tests without the --assumeyes
option and there are other tests to ensure that images cannot be
downloaded without that option.  So, any unexpected attempts to download
the image will be caught by those variants of these tests.

Fallout from 32b147b

containers#1595
@debarshiray
Copy link
Member Author

recheck

Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/39c6850c8fd84912b062286aea7bb9e1

✔️ unit-test SUCCESS in 5m 35s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 10s
✔️ unit-test-restricted SUCCESS in 5m 38s
✔️ system-test-fedora-rawhide-commands-options SUCCESS in 1h 12m 56s
✔️ system-test-fedora-rawhide-runtime-environment-arch-fedora SUCCESS in 1h 17m 54s
✔️ system-test-fedora-rawhide-runtime-environment-ubuntu SUCCESS in 31m 46s
✔️ system-test-fedora-41-commands-options SUCCESS in 1h 09m 52s
✔️ system-test-fedora-41-runtime-environment-arch-fedora SUCCESS in 1h 18m 08s
✔️ system-test-fedora-41-runtime-environment-ubuntu SUCCESS in 30m 10s
✔️ system-test-fedora-40-commands-options SUCCESS in 1h 12m 26s
system-test-fedora-40-runtime-environment-arch-fedora NODE_FAILURE Node request 200-0007666927 failed in 0s
system-test-fedora-40-runtime-environment-ubuntu NODE_FAILURE Node request 200-0007666928 failed in 0s
system-test-fedora-39-commands-options NODE_FAILURE Node request 200-0007666929 failed in 0s
system-test-fedora-39-runtime-environment-arch-fedora NODE_FAILURE Node request 200-0007666930 failed in 0s
system-test-fedora-39-runtime-environment-ubuntu NODE_FAILURE Node request 200-0007666931 failed in 0s

@debarshiray
Copy link
Member Author

recheck

@debarshiray debarshiray changed the title test/system: Remove unnecessary --assumeyes test/system: Remove unnecessary --assumeyes, optimize the 'create' tests, etc. Nov 15, 2024
Currently, there's no way to get assert_line to use the stderr_lines
array [1].  This is worked around by assigning stderr_lines to the
'lines' array.

[1] bats-core/bats-assert#42

containers#1595
@debarshiray debarshiray merged commit d29adbf into containers:main Nov 15, 2024
2 of 3 checks passed
@debarshiray debarshiray deleted the wip/rishi/test-system-create-drop-unnecessary-assumeyes-update-run-flags branch November 15, 2024 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant