-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
CI: system tests: enable parallel tests #23989
Changes from all commits
d571ca6
bcffa9c
6b621d9
6502e30
bf61317
5fc3de5
b3da5be
7fcf94d
8402b65
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,22 @@ load helpers | |
args="some sort of argument list" | ||
run_podman subcmd $args | ||
assert "$output" == "what we expect" "output from 'podman subcmd $args'" | ||
|
||
# safename() provides a lower-case string with both the BATS test number | ||
# and a pseudorandom element. Its use is strongly encouraged for container | ||
# names, volumes, networks, images, everything, because the test number | ||
# makes it possible to find leaked elements. | ||
cname="c-$(safename)" | ||
|
||
# Run "top" in a detached container with a safe name | ||
run_podman run -d --name $cname --this-option --that $IMAGE top | ||
|
||
# A number ("17") as the first run_podman arg means "expect this exit code". | ||
# By default, run_podman expects success, and will barf on nonzero status. | ||
run_podman 17 run --this-option --that $IMAGE exit 17 | ||
|
||
# Give a hoot, don't pollute | ||
run_podman rm -f -t0 $cname | ||
} | ||
|
||
# vim: filetype=sh | ||
|
@@ -110,5 +126,31 @@ size | -\\\?[0-9]\\\+ | |
done < <(parse_table "$tests") | ||
} | ||
|
||
# Whenever possible, please add the ci:parallel tag to new tests, | ||
# not only for speed but for stress testing. | ||
# | ||
# This is an example of what NOT to do when enabling parallel tests. | ||
# | ||
# bats test_tags=ci:parallel | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should likely mention the possibility of a file_tag for the whole file? i.e. do not set it if the file already has it set and a author must check if the whole file is parallel Also examples of what not to do are bad if they do not show what to do instead. This should be in the style of do not do etc... |
||
@test "this test is completely broken in parallel" { | ||
# Never use "--name HARDCODED". Define 'cname=c-$(safename)' instead. | ||
# Not only does that guarantee uniqueness, it also gives the test number | ||
# in the name, so if there's a leak (at end of tests) it will be possible | ||
# to identify the culprit. | ||
run_podman --name mycontainer $IMAGE top | ||
|
||
# Same thing for build and -t | ||
run_podman build -t myimage ... | ||
|
||
# Never assume exact output from podman ps! Many other containers may be running. | ||
run_podman ps | ||
assert "$output" = "mycontainer" | ||
|
||
# Never use "-l". It is meaningless when other processes are running. | ||
run_podman container inspect -l | ||
|
||
# Never 'rm -a'!!! OMG like seriously just don't. | ||
run_podman rm -f -a | ||
} | ||
|
||
# vim: filetype=sh |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -428,12 +428,14 @@ EOF | |
|
||
# A quadlet container depends on a named quadlet volume | ||
@test "quadlet - named volume dependency" { | ||
local volume_name="v-$(safename)" | ||
|
||
# Save the unit name to use as the volume for the container | ||
local quadlet_vol_unit=dep_$(safename).volume | ||
local quadlet_vol_file=$PODMAN_TMPDIR/${quadlet_vol_unit} | ||
cat > $quadlet_vol_file <<EOF | ||
[Volume] | ||
VolumeName=foo | ||
VolumeName=$volume_name | ||
EOF | ||
|
||
# Have quadlet create the systemd unit file for the volume unit | ||
|
@@ -442,7 +444,6 @@ EOF | |
|
||
# Save the volume service name since the variable will be overwritten | ||
local vol_service=$QUADLET_SERVICE_NAME | ||
local volume_name="foo" | ||
|
||
local quadlet_file=$PODMAN_TMPDIR/user_$(safename).container | ||
cat > $quadlet_file <<EOF | ||
|
@@ -975,6 +976,7 @@ EOF | |
|
||
service_setup $QUADLET_SERVICE_NAME | ||
|
||
# FIXME: log.91: Starting, not Started | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have no idea what this is about? |
||
# Ensure we have output. Output is synced via sd-notify (socat in Exec) | ||
run journalctl "--since=$STARTED_TIME" --unit="$QUADLET_SERVICE_NAME" | ||
is "$output" '.*Started.*\.service.*' | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,11 +5,10 @@ debug failures. | |
Quick Start | ||
=========== | ||
|
||
Look at [030-run.bats](030-run.bats) for a simple but packed example. | ||
Look at [000-TEMPLATE](000-TEMPLATE) for a simple starting point. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are no docs here in the Readme about parallel tests requirements and I assume many will not look into the template file. I think the parallel testing should have its own section here |
||
This introduces the basic set of helper functions: | ||
|
||
* `setup` (implicit) - resets container storage so there's | ||
one and only one (standard) image, and no running containers. | ||
* `setup` (implicit) - establishes a test environment. | ||
|
||
* `parse_table` - you can define tables of inputs and expected results, | ||
then read those in a `while` loop. This makes it easy to add new tests. | ||
|
@@ -21,7 +20,7 @@ examples of how to deal with the more typical such issues. | |
but could also be './bin/podman' or 'podman-remote'), with a timeout. | ||
Checks its exit status. | ||
|
||
* `is` - compare actual vs expected output. Emits a useful diagnostic | ||
* `assert` - compare actual vs expected output. Emits a useful diagnostic | ||
on failure. | ||
|
||
* `die` - output a properly-formatted message to stderr, and fail test | ||
|
@@ -30,7 +29,13 @@ on failure. | |
|
||
* `skip_if_remote` - like the above, but skip if testing `podman-remote` | ||
|
||
* `random_string` - returns a pseudorandom alphanumeric string | ||
* `safename` - generates a pseudorandom lower-case string suitable | ||
for use in names for containers, images, volumes, any object. String | ||
includes the BATS test number, making it possible to identify the | ||
source of leaks (failure to clean up) at the end of tests. | ||
|
||
* `random_string` - returns a pseudorandom alphanumeric string suitable | ||
for verifying I/O. | ||
|
||
Test files are of the form `NNN-name.bats` where NNN is a three-digit | ||
number. Please preserve this convention, it simplifies viewing the | ||
|
@@ -59,7 +64,7 @@ commands, their output and exit codes. In a normal run you will never | |
see this, but BATS will display it on failure. The goal here is to | ||
give you everything you need to diagnose without having to rerun tests. | ||
|
||
The `is` comparison function is designed to emit useful diagnostics, | ||
The `assert` comparison function is designed to emit useful diagnostics, | ||
in particular, the actual and expected strings. Please do not use | ||
the horrible BATS standard of `[ x = y ]`; that's nearly useless | ||
for tracking down failures. | ||
|
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.
slowest test in this PR was 23:23 so this looks very close to hitting this already, maybe it was as particular bad run but we should not set it to close IMO
I guess it is fine as you keep converting more files to run parallel so we speed up over the next days/weeks?