Subject
Something else that requires developers attention
Description
The current test setup_cli.bats has been failing lately (sporadically over past 6 months or so) when a test-case tries to run setup.sh but detects a container that is running from another test file (in this case dms-test_postconf-helper from postconf-helper.bats).
not ok 78 [setup.sh] show usage when no arguments provided in 144ms
# (from function `assert_success' in file test/test_helper/bats-assert/src/assert_success.bash, line 42,
# in test file test/tests/parallel/set3/scripts/setup_cli.bats, line 30)
# `assert_success' failed
#
# -- command failed --
# status : 1
# output : Error response from daemon: No such container: dms-test_postconf-helper
# --
#
|
@test "show usage when no arguments provided" { |
|
run ./setup.sh |
|
assert_success |
Cause identified
This is due to the setup.sh script checking for any existing DMS containers and choosing one for user convenience, but in doing so hits a race condition as the other test container is stopped before it can run the command:
|
INFO=$(${CRI} ps --no-trunc --format "{{.Image}};{{.Names}}" --filter \ |
|
label=org.opencontainers.image.title="docker-mailserver" | tail -1) |
|
|
|
[[ -z ${CONTAINER_NAME} ]] && CONTAINER_NAME=${INFO#*;} |
|
if [[ -n ${CONTAINER_NAME} ]]; then |
|
${CRI} exec "${USE_TTY}" "${CONTAINER_NAME}" setup "${@}" |
|
else |
|
_run_in_new_container setup "${@}" |
|
fi |
While the test sets CONTAINER_NAME ENV, the setup.sh script itself unsets that within it's own scope:
|
CONTAINER_NAME='dms-test_setup-cli' |
Only a couple test cases use setup.sh without -c
The bulk of test cases in setup-cli.bats test file do invoke the setup.sh -c <Container name> arg, but this test case and the one after it in particular intentionally do not.
setup.sh is configured to exit immediately if a command returns a non-zero exit code (including subshells due to the shopt line that follows):
|
set -euEo pipefail |
|
shopt -s inherit_errexit 2>/dev/null || true |
There's not that much going on within setup.sh inbetween acquisition of the running container name returned and running docker exec ..., but it has resulted in multiple CI failures that it's a tad annoying as a false-positive caveat.
|
INFO=$(${CRI} ps --no-trunc --format "{{.Image}};{{.Names}}" --filter \ |
|
label=org.opencontainers.image.title="docker-mailserver" | tail -1) |
|
|
|
[[ -z ${CONTAINER_NAME} ]] && CONTAINER_NAME=${INFO#*;} |
|
[[ -z ${IMAGE_NAME} ]] && IMAGE_NAME=${INFO%;*} |
|
if [[ -z ${IMAGE_NAME} ]]; then |
|
IMAGE_NAME=${NAME:-${DEFAULT_IMAGE_NAME}} |
|
fi |
|
|
|
if test -t 0; then |
|
USE_TTY="-it" |
|
else |
|
# GitHub Actions will fail (or really anything else |
|
# lacking an interactive tty) if we don't set a |
|
# value here; "-t" alone works for these cases. |
|
USE_TTY="-t" |
|
fi |
|
|
|
_handle_config_path |
|
|
|
if [[ -n ${CONTAINER_NAME} ]]; then |
|
${CRI} exec "${USE_TTY}" "${CONTAINER_NAME}" setup "${@}" |
|
else |
Actionable
We could either move this to the serial tests group instead of a parallel test or attempt to address it by having the setup.sh conditional check on the container name variable also verify that the container is still running? (might be sufficient to avoid the race condition)
There is however another docker command run inbetween that would query volumes and likewise return a non-zero exit status when failing, though I don't recall it's output ever appearing in the tests.
|
if [[ -n ${CONTAINER_NAME} ]]; then |
|
VOLUME=$(${CRI} inspect "${CONTAINER_NAME}" \ |
|
--format="{{range .Mounts}}{{ println .Source .Destination}}{{end}}" | \ |
|
grep "${DMS_CONFIG}$" 2>/dev/null || :) |
|
fi |
Easiest action is to just migrate setup-cli.bats from the parallel set to serial tests.
Subject
Something else that requires developers attention
Description
The current test
setup_cli.batshas been failing lately (sporadically over past 6 months or so) when a test-case tries to runsetup.shbut detects a container that is running from another test file (in this casedms-test_postconf-helperfrompostconf-helper.bats).docker-mailserver/test/tests/parallel/set3/scripts/setup_cli.bats
Lines 28 to 30 in e532b37
Cause identified
This is due to the
setup.shscript checking for any existing DMS containers and choosing one for user convenience, but in doing so hits a race condition as the other test container is stopped before it can run the command:docker-mailserver/setup.sh
Lines 169 to 172 in e532b37
docker-mailserver/setup.sh
Lines 189 to 193 in e532b37
While the test sets
CONTAINER_NAMEENV, thesetup.shscript itself unsets that within it's own scope:docker-mailserver/test/tests/parallel/set3/scripts/setup_cli.bats
Line 6 in e532b37
docker-mailserver/setup.sh
Line 8 in e532b37
Only a couple test cases use
setup.shwithout-cThe bulk of test cases in
setup-cli.batstest file do invoke thesetup.sh -c <Container name>arg, but this test case and the one after it in particular intentionally do not.setup.shis configured to exit immediately if a command returns a non-zero exit code (including subshells due to theshoptline that follows):docker-mailserver/setup.sh
Lines 27 to 28 in e532b37
There's not that much going on within
setup.shinbetween acquisition of the running container name returned and runningdocker exec ..., but it has resulted in multiple CI failures that it's a tad annoying as a false-positive caveat.docker-mailserver/setup.sh
Lines 169 to 191 in e532b37
Actionable
We could either move this to the serial tests group instead of a parallel test or attempt to address it by having the
setup.shconditional check on the container name variable also verify that the container is still running? (might be sufficient to avoid the race condition)There is however another
dockercommand run inbetween that would query volumes and likewise return a non-zero exit status when failing, though I don't recall it's output ever appearing in the tests.docker-mailserver/setup.sh
Lines 92 to 96 in e532b37
Easiest action is to just migrate
setup-cli.batsfrom the parallel set to serial tests.