-
Notifications
You must be signed in to change notification settings - Fork 220
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
Rework the system tests #375
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Build failed.
|
HarryMichal
force-pushed
the
fix-tests
branch
from
February 10, 2020 16:14
45cf013
to
ff90452
Compare
Build failed.
|
HarryMichal
force-pushed
the
fix-tests
branch
2 times, most recently
from
February 10, 2020 16:26
afd1bb7
to
45e049f
Compare
Build succeeded.
|
This change adds a pre-run task to pull the fedora-toolbox images from the registry to reduce the number of false positives caused by 'podman pull' failing to download them during the actual test. Each section needs a separate playbook because they use different versions of Fedora, and hence different default images. containers#375
The tests introduced by commit b5cdc57 have proven to be rather unstable due to mistakes in their design. The tests were quite chaotically structured, and because of that images were deleted and pulled too often, causing several false positives [1, 2]. This changes the structure of the tests in a major way. The tests (resp. commands) are now run in a manner that better simulates the way Toolbox is actually used. From a clean state, through creating containers, using them and in the end deleting them. This should reduce the strain on the bandwidth and possibly even speed up the tests themselves. [1] containers#372 [2] containers#374 containers#375
debarshiray
reviewed
Feb 18, 2020
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job with the tests! Some relatively superficial comments:
debarshiray
force-pushed
the
fix-tests
branch
from
February 18, 2020 13:13
45e049f
to
1e22327
Compare
Build succeeded.
|
Thank you for fixing the mentioned issues!! |
debarshiray
added a commit
to debarshiray/toolbox
that referenced
this pull request
Feb 28, 2024
The system tests download several images when setting up the test suite, and cache them for later use by the tests [1]. This saves time and avoids hitting rate limits imposed by OCI registries by not downloading the same images repeatedly for several tests, but at the cost of increased use of storage space to cache the images. The images are cached under BATS_TMPDIR. It defaults to the TMPDIR environment variable, and if that's not set then to /tmp [2]. Normally, TMPDIR isn't set, and the images end up getting cached under /tmp. Now, /tmp is typically on tmpfs backed by RAM or swap, which means that it should be used for smaller size-bounded files only, and /var/tmp should be used for everything else [3]. The images are big enough that a collection of them can't be described as smaller and size-bounded, and it led to: 1..306 # test suite: Set up # test suite: Tear down not ok 1 setup_suite # (from function `setup_suite' in test file ./setup_suite.bash, line 55) # `_pull_and_cache_distro_image fedora "$((system_version-1))" || false' failed # Failed to cache image registry.fedoraproject.org/fedora-toolbox:40 to /tmp/bats-run-IPz4Cn/image-cache/fedora-toolbox-40 # time="2024-02-19T11:41:43Z" level=fatal msg="copying system image from manifest list: writing blob: write /tmp/bats-run-IPz4Cn/image-cache/fedora-toolbox-40/dir-put-blob607392514: no space left on device" # bats warning: Executed 1 instead of expected 306 tests So, change the default location of the BATS_TMPDIR environment variable to /var/tmp by setting TMPDIR. [1] Commit 50683c9 containers@50683c9d9a78adc9 containers#375 [2] https://bats-core.readthedocs.io/en/stable/writing-tests.html [3] https://systemd.io/TEMPORARY_DIRECTORIES/
debarshiray
added a commit
to debarshiray/toolbox
that referenced
this pull request
Feb 28, 2024
The system tests download several images when setting up the test suite, and cache them for later use by the tests [1]. This saves time and avoids hitting rate limits imposed by OCI registries by not downloading the same images repeatedly for several tests, but at the cost of increased use of storage space to cache the images. The images are cached under BATS_TMPDIR. It defaults to the TMPDIR environment variable, and if that's not set then to /tmp [2]. Normally, TMPDIR isn't set, and the images end up getting cached under /tmp. Now, /tmp is typically on tmpfs backed by RAM or swap, which means that it should be used for smaller size-bounded files only, and /var/tmp should be used for everything else [3]. The images are big enough that a collection of them can't be described as smaller and size-bounded, and it led to: 1..306 # test suite: Set up # test suite: Tear down not ok 1 setup_suite # (from function `setup_suite' in test file ./setup_suite.bash, line 55) # `_pull_and_cache_distro_image fedora "$((system_version-1))" || false' failed # Failed to cache image registry.fedoraproject.org/fedora-toolbox:40 to /tmp/bats-run-IPz4Cn/image-cache/fedora-toolbox-40 # time="2024-02-19T11:41:43Z" level=fatal msg="copying system image from manifest list: writing blob: write /tmp/bats-run-IPz4Cn/image-cache/fedora-toolbox-40/dir-put-blob607392514: no space left on device" # bats warning: Executed 1 instead of expected 306 tests So, change the default location of the BATS_TMPDIR environment variable to /var/tmp by setting TMPDIR. [1] Commit 50683c9 containers@50683c9d9a78adc9 containers#375 [2] https://bats-core.readthedocs.io/en/stable/writing-tests.html [3] https://systemd.io/TEMPORARY_DIRECTORIES/ containers#1462
debarshiray
added a commit
to debarshiray/toolbox
that referenced
this pull request
Feb 29, 2024
The system tests download several images when setting up the test suite, and cache them for later use by the tests [1]. This saves time and avoids hitting rate limits imposed by OCI registries by not downloading the same images repeatedly for several tests, but at the cost of increased use of storage space to cache the images. The images are cached under BATS_TMPDIR. It defaults to the TMPDIR environment variable, and if that's not set then to /tmp [2]. Normally, TMPDIR isn't set, and the images end up getting cached under /tmp. Now, /tmp is typically on tmpfs backed by RAM or swap, which means that it should be used for smaller size-bounded files only, and /var/tmp should be used for everything else [3]. The images are big enough that a collection of them can't be described as smaller and size-bounded, and it led to: 1..306 # test suite: Set up # test suite: Tear down not ok 1 setup_suite # (from function `setup_suite' in test file ./setup_suite.bash, line 55) # `_pull_and_cache_distro_image fedora "$((system_version-1))" || false' failed # Failed to cache image registry.fedoraproject.org/fedora-toolbox:40 to /tmp/bats-run-IPz4Cn/image-cache/fedora-toolbox-40 # time="2024-02-19T11:41:43Z" level=fatal msg="copying system image from manifest list: writing blob: write /tmp/bats-run-IPz4Cn/image-cache/fedora-toolbox-40/dir-put-blob607392514: no space left on device" # bats warning: Executed 1 instead of expected 306 tests So, change the default location of the BATS_TMPDIR environment variable to /var/tmp by setting TMPDIR. [1] Commit 50683c9 containers@50683c9d9a78adc9 containers#375 [2] https://bats-core.readthedocs.io/en/stable/writing-tests.html [3] https://systemd.io/TEMPORARY_DIRECTORIES/ containers#1462
debarshiray
added a commit
to debarshiray/toolbox
that referenced
this pull request
Feb 29, 2024
The system tests download several images when setting up the test suite, and cache them for later use by the tests [1]. This saves time and avoids hitting rate limits imposed by OCI registries by not downloading the same images repeatedly for several tests, but at the cost of increased use of storage space to cache the images. The images are cached under BATS_TMPDIR. It defaults to the TMPDIR environment variable, and if that's not set then to /tmp [2]. Normally, TMPDIR isn't set, and the images end up getting cached under /tmp. Now, /tmp is typically on tmpfs backed by RAM or swap, which means that it should be used for smaller size-bounded files only, and /var/tmp should be used for everything else [3]. The images are big enough that a collection of them can't be described as smaller and size-bounded, and it led to: 1..306 # test suite: Set up # test suite: Tear down not ok 1 setup_suite # (from function `setup_suite' in test file ./setup_suite.bash, line 55) # `_pull_and_cache_distro_image fedora "$((system_version-1))" || false' failed # Failed to cache image registry.fedoraproject.org/fedora-toolbox:40 to /tmp/bats-run-IPz4Cn/image-cache/fedora-toolbox-40 # time="2024-02-19T11:41:43Z" level=fatal msg="copying system image from manifest list: writing blob: write /tmp/bats-run-IPz4Cn/image-cache/fedora-toolbox-40/dir-put-blob607392514: no space left on device" # bats warning: Executed 1 instead of expected 306 tests So, change the default location of the BATS_TMPDIR environment variable to /var/tmp by setting TMPDIR. [1] Commit 50683c9 containers@50683c9d9a78adc9 containers#375 [2] https://bats-core.readthedocs.io/en/stable/writing-tests.html [3] https://systemd.io/TEMPORARY_DIRECTORIES/ containers#1462
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The tests introduced by #250 have proven to be rather unstable due to
mistakes in their design. The tests very quite chaotically structured.
Because of that images were deleted and pulled too often which caused
several false positives (#374, #372).
This changes the strucutre of the tests in a major way. The tests (resp.
commands) are now ran in a manner to kinda simulate the way Toolbox is
used. From clean state, through creating containers, using them and in
the end deleting them. This should reduce the strain on the bandwidth
and possibly even speed up the tests themselves.
More information in the README.md in the directory with the tests.