refactor: Parallel Tests

- `disabled_clamav_spamassassin`:
  - Just shuffling the test order around, and removing the restart test at the end which doesn't make sense.

- `postscreen`:
  - Now uses common helper for getting container IP
  - Does not appear to need the `NET_ADMIN` capability?
  - Reduced startup time for the 2nd container + additional context about it's relevance.
  - Test cases are largely the same, but refactored the `nc` alternative that properly waits it's turn. This only needs to run once. Added additional commentary and made into a generic method if needed in other tests.

- `fail2ban`:
  - Use the common container IP helper method.
  - Postscreen isn't affecting this test, it's not required to do the much slower exchange with the mail server when sending a login failure.
  - IP being passed into ENV is no longer necessary.
  - `sleep 5` in the related test cases doesn't seem necessary, can better rely on polling with timeout.
  - `sleep 10` for `setup.sh` also doesn't appear to be necessary.

- `postgrey`:
  - Reduced POSTGREY_DELAY to 3, which shaves a fair amount of wasted time while still verifying the delay works.
  - One of the checks in `main.cf` doesn't seem to need to know about the earlier spamhaus portion of the line to work, removed.
  - Better test case descriptions.
  - Improved log matching via standard method that better documents the expected triplet under test.
  - Removed a redundant whitelist file and test that didn't seem to have any relevance. Added a TODO with additional notes about a concern with these tests.
  - Reduced test time as 8 second timeouts from `-w 8` don't appear to be required, better to poll with grep instead.
  - Replaced `wc -l` commands with a new method to assert expected line count, better enabling assertions on the actual output.

- `undef_spam_subject`:
  - Split to two separate test cases, and initialize each container in their case instead of `setup_file()`, allowing for using the default `teardown()` method (and slight benefit if running in parallel).

- `permit_docker`:
  - Not a parallel test, but I realized that the repeat helper methods don't necessarily play well with `run` as the command (can cause false positive of what was successful).

test/helper/common.bash

@@ -10,6 +10,8 @@ __load_bats_helper
 # -------------------------------------------------------------------
 
 # like _run_in_container_explicit but infers ${1} by using the ENV CONTAINER_NAME
+# WARNING: Careful using this with _until_success_or_timeout methods,
+# which can be misleading in the success of `run`, not the command given to `run`.
 function _run_in_container() {
   run docker exec "${CONTAINER_NAME}" "${@}"
 }
@@ -36,6 +38,12 @@ function check_if_process_is_running() {
   docker exec "${CONTAINER_NAME}" pgrep "${PROGRAM_NAME}"
 }
 
+# @param ${1} target container name [IF UNSET: ${CONTAINER_NAME}]
+function get_container_ip() {
+  local TARGET_CONTAINER_NAME=${1:-${CONTAINER_NAME}}
+  docker inspect --format '{{ .NetworkSettings.IPAddress }}' "${TARGET_CONTAINER_NAME}"
+}
+
 # -------------------------------------------------------------------
 
 # @param ${1} timeout

test/test-files/nc_templates/postgrey_whitelist_local.txt

@@ -1,9 +0,0 @@
-request=smtpd_access_policy
-protocol_state=RCPT
-protocol_name=ESMTP
-client_address=127.0.0.1
-client_name=whitelistlocal.tld
-helo_name=whitelistlocal.tld
-sender=test@whitelistlocal.tld
-recipient=user1@localhost.localdomain
-

test/tests/parallel/set1/spam_virus/disabled_clamav_spamassassin.bats

@@ -29,22 +29,17 @@ function teardown_file() { _default_teardown ; }
   assert_failure
 }
 
-@test "${TEST_NAME_PREFIX} SA - Amavis integration should not be active" {
-  _run_in_container /bin/sh -c "grep -i 'ANTI-SPAM-SA code' /var/log/mail/mail.log | grep 'NOT loaded'"
-  assert_success
-}
-
 @test "${TEST_NAME_PREFIX} ClamAV - Amavis integration should not be active" {
   _run_in_container grep -i 'Found secondary av scanner ClamAV-clamscan' /var/log/mail/mail.log
   assert_failure
 }
 
-@test "${TEST_NAME_PREFIX} SA should not be called" {
+@test "${TEST_NAME_PREFIX} SA - Amavis integration should not be active" {
+  _run_in_container bash -c "grep -i 'ANTI-SPAM-SA code' /var/log/mail/mail.log | grep 'NOT loaded'"
+  assert_success
+}
+
+@test "${TEST_NAME_PREFIX} SA - should not have been called" {
   _run_in_container grep -i 'connect to /var/run/clamav/clamd.ctl failed' /var/log/mail/mail.log
   assert_failure
 }
-
-@test "${TEST_NAME_PREFIX} ClamAV process should not be restarted when killed" {
-  _run_in_container /bin/bash -c "pkill -f clamd && sleep 10 && ps aux --forest | grep -v grep | grep '/usr/sbin/clamd'"
-  assert_failure
-}

test/tests/parallel/set1/spam_virus/fail2ban.bats

@@ -5,10 +5,6 @@ TEST_NAME_PREFIX='Fail2Ban:'
 CONTAINER1_NAME='dms-test_fail2ban'
 CONTAINER2_NAME='dms-test_fail2ban_fail-auth-mailer'
 
-function get_container2_ip() {
-  docker inspect --format '{{ .NetworkSettings.IPAddress }}' "${CONTAINER2_NAME}"
-}
-
 function setup_file() {
   export CONTAINER_NAME
 
@@ -17,6 +13,7 @@ function setup_file() {
     --env ENABLE_FAIL2BAN=1
     --env POSTSCREEN_ACTION=ignore
     --cap-add=NET_ADMIN
+    # NOTE: May no longer be needed with newer F2B:
     --ulimit "nofile=$(ulimit -Sn):$(ulimit -Hn)"
   )
   init_with_defaults
@@ -25,7 +22,6 @@ function setup_file() {
 
   # Create a container which will send wrong authentications and should get banned
   CONTAINER_NAME=${CONTAINER2_NAME}
-  local CUSTOM_SETUP_ARGUMENTS=(--env MAIL_FAIL2BAN_IP="$(docker inspect --format '{{ .NetworkSettings.IPAddress }}' ${CONTAINER1_NAME})")
   init_with_defaults
   common_container_setup 'CUSTOM_SETUP_ARGUMENTS'
 
@@ -76,30 +72,21 @@ function teardown_file() {
   done
 }
 
-# NOTE: This test case is fragile if other test cases were to be run concurrently
+# NOTE: This test case is fragile if other test cases were to be run concurrently.
+# - After multiple login fails and a slight delay, f2b will ban that IP.
+# - You could hard-code `sleep 5` on both cases to avoid the alternative assertions,
+#   but the polling + piping into grep approach here reliably minimizes the delay.
 @test "${TEST_NAME_PREFIX} ban ip on multiple failed login" {
-  # can't pipe the file as usual due to postscreen
+  CONTAINER1_IP=$(get_container_ip ${CONTAINER1_NAME})
-  # respecting postscreen_greet_wait time and talking in turn):
+  # Trigger a ban by failing to login twice:
-
+  _run_in_container_explicit "${CONTAINER2_NAME}" bash -c "nc ${CONTAINER1_IP} 25 < /tmp/docker-mailserver-test/auth/smtp-auth-login-wrong.txt"
-  # shellcheck disable=SC1004
+  _run_in_container_explicit "${CONTAINER2_NAME}" bash -c "nc ${CONTAINER1_IP} 25 < /tmp/docker-mailserver-test/auth/smtp-auth-login-wrong.txt"
-  for _ in {1,2}
-  do
-    docker exec "${CONTAINER2_NAME}" /bin/bash -c \
-    'exec 3<>/dev/tcp/${MAIL_FAIL2BAN_IP}/25 && \
-    while IFS= read -r cmd; do \
-      head -1 <&3; \
-      [[ ${cmd} == "EHLO"* ]] && sleep 6; \
-      echo ${cmd} >&3; \
-    done < "/tmp/docker-mailserver-test/auth/smtp-auth-login-wrong.txt"'
-  done
-
-  sleep 5
 
   # Checking that CONTAINER2_IP is banned in "${CONTAINER1_NAME}"
-  CONTAINER2_IP=$(get_container2_ip)
+  CONTAINER2_IP=$(get_container_ip ${CONTAINER2_NAME})
-  _run_in_container fail2ban-client status postfix-sasl
+  run repeat_in_container_until_success_or_timeout 10 "${CONTAINER_NAME}" bash -c "fail2ban-client status postfix-sasl | grep -F '${CONTAINER2_IP}'"
   assert_success
-  assert_output --partial "${CONTAINER2_IP}"
+  assert_output --partial 'Banned IP list:'
 
   # Checking that CONTAINER2_IP is banned by nftables
   _run_in_container bash -c 'nft list set inet f2b-table addr-set-postfix-sasl'
@@ -107,11 +94,11 @@ function teardown_file() {
   assert_output --partial "elements = { ${CONTAINER2_IP} }"
 }
 
+# NOTE: Depends on previous test case, if no IP was banned at this point, it passes regardless..
 @test "${TEST_NAME_PREFIX} unban ip works" {
-  CONTAINER2_IP=$(get_container2_ip)
+  CONTAINER2_IP=$(get_container_ip ${CONTAINER2_NAME})
   _run_in_container fail2ban-client set postfix-sasl unbanip "${CONTAINER2_IP}"
   assert_success
-  sleep 5
 
   # Checking that CONTAINER2_IP is unbanned in "${CONTAINER1_NAME}"
   _run_in_container fail2ban-client status postfix-sasl
@@ -189,8 +176,6 @@ function teardown_file() {
   _run_in_container fail2ban-client set dovecot banip 192.0.66.4
   _run_in_container fail2ban-client set dovecot banip 192.0.66.5
 
-  sleep 10
-
   # Originally: run ./setup.sh -c "${CONTAINER1_NAME}" fail2ban
   _run_in_container setup fail2ban
   assert_output --regexp '^Banned in dovecot:.*192\.0\.66\.4'

test/tests/parallel/set1/spam_virus/postgrey_enabled.bats

@@ -10,7 +10,7 @@ function setup_file() {
     --env ENABLE_POSTGREY=1
     --env PERMIT_DOCKER=container
     --env POSTGREY_AUTO_WHITELIST_CLIENTS=5
-    --env POSTGREY_DELAY=15
+    --env POSTGREY_DELAY=3
     --env POSTGREY_MAX_AGE=35
     --env POSTGREY_TEXT="Delayed by Postgrey"
   )
@@ -24,20 +24,20 @@ function setup_file() {
 
 function teardown_file() { _default_teardown ; }
 
-@test "${TEST_NAME_PREFIX} /etc/postfix/main.cf correctly edited" {
+@test "${TEST_NAME_PREFIX} should have added Postgrey to 'main.cf:check_policy_service'" {
-  _run_in_container bash -c "grep -F 'zen.spamhaus.org=127.0.0.[2..11], check_policy_service inet:127.0.0.1:10023' /etc/postfix/main.cf | wc -l"
+  _run_in_container grep -F 'check_policy_service inet:127.0.0.1:10023' /etc/postfix/main.cf
   assert_success
-  assert_output 1
+  _should_output_number_of_lines 1
 }
 
-@test "${TEST_NAME_PREFIX} /etc/default/postgrey correctly edited and has the default values" {
+@test "${TEST_NAME_PREFIX} should have configured /etc/default/postgrey with default values and ENV overrides" {
-  _run_in_container bash -c "grep '^POSTGREY_OPTS=\"--inet=127.0.0.1:10023 --delay=15 --max-age=35 --auto-whitelist-clients=5\"$' /etc/default/postgrey | wc -l"
+  _run_in_container grep -F 'POSTGREY_OPTS="--inet=127.0.0.1:10023 --delay=3 --max-age=35 --auto-whitelist-clients=5"' /etc/default/postgrey
   assert_success
-  assert_output 1
+  _should_output_number_of_lines 1
 
-  _run_in_container bash -c "grep '^POSTGREY_TEXT=\"Delayed by Postgrey\"$' /etc/default/postgrey | wc -l"
+  _run_in_container grep -F 'POSTGREY_TEXT="Delayed by Postgrey"' /etc/default/postgrey
   assert_success
-  assert_output 1
+  _should_output_number_of_lines 1
 }
 
 @test "${TEST_NAME_PREFIX} Postgrey is running" {
@@ -45,48 +45,96 @@ function teardown_file() { _default_teardown ; }
   assert_success
 }
 
-@test "${TEST_NAME_PREFIX} there should be a log entry about a new greylisted e-mail user@external.tld in /var/log/mail/mail.log" {
+@test "${TEST_NAME_PREFIX} should initially reject (greylist) mail from 'user@external.tld'" {
-  #editing the postfix config in order to ensure that postgrey handles the test e-mail. The other spam checks at smtpd_recipient_restrictions would interfere with it.
+  # Modify the postfix config in order to ensure that postgrey handles the test e-mail.
+  # The other spam checks in `main.cf:smtpd_recipient_restrictions` would interfere with testing postgrey.
   _run_in_container bash -c "sed -ie 's/permit_sasl_authenticated.*policyd-spf,$//g' /etc/postfix/main.cf"
   _run_in_container bash -c "sed -ie 's/reject_unauth_pipelining.*reject_unknown_recipient_domain,$//g' /etc/postfix/main.cf"
   _run_in_container bash -c "sed -ie 's/reject_rbl_client.*inet:127\.0\.0\.1:10023$//g' /etc/postfix/main.cf"
   _run_in_container bash -c "sed -ie 's/smtpd_recipient_restrictions =/smtpd_recipient_restrictions = check_policy_service inet:127.0.0.1:10023/g' /etc/postfix/main.cf"
   _run_in_container postfix reload
 
-  _run_in_container bash -c "nc 0.0.0.0 25 < /tmp/docker-mailserver-test/email-templates/postgrey.txt"
+  # Send test mail (it should fail to deliver):
-  sleep 5 #ensure that the information has been written into the log
+  _send_test_mail '/tmp/docker-mailserver-test/email-templates/postgrey.txt' '25'
-  _run_in_container bash -c "grep -i 'action=greylist.*user@external\.tld' /var/log/mail/mail.log | wc -l"
+
-  assert_success
+  # Confirm mail was greylisted:
-  assert_output 1
+  _should_have_log_entry \
+    'action=greylist' \
+    'reason=new' \
+    'client_address=127.0.0.1/32, sender=user@external.tld, recipient=user1@localhost.localdomain'
+
+  repeat_until_success_or_timeout 10 _run_in_container grep \
+    'Recipient address rejected: Delayed by Postgrey' \
+    /var/log/mail/mail.log
 }
 
-@test "${TEST_NAME_PREFIX} there should be a log entry about the retried and passed e-mail user@external.tld in /var/log/mail/mail.log" {
+# NOTE: This test case depends on the previous one
-  sleep 20 #wait 20 seconds so that postgrey would accept the message
+@test "${TEST_NAME_PREFIX} should accept mail from 'user@external.tld' after POSTGREY_DELAY duration" {
-  _run_in_container bash -c "nc 0.0.0.0 25 < /tmp/docker-mailserver-test/email-templates/postgrey.txt"
+  # Wait until `$POSTGREY_DELAY` seconds pass before trying again: 
-  sleep 8
+  sleep 3
+  # Retry delivering test mail (it should be trusted this time):
+  _send_test_mail '/tmp/docker-mailserver-test/email-templates/postgrey.txt' '25'
 
-  _run_in_container bash -c "grep -i 'action=pass, reason=triplet found.*user@external\.tld' /var/log/mail/mail.log | wc -l"
+  # Confirm postgrey permitted delivery (triplet is now trusted):
-  assert_success
+  _should_have_log_entry \
-  assert_output 1
+    'action=pass' \
+    'reason=triplet found' \
+    'client_address=127.0.0.1/32, sender=user@external.tld, recipient=user1@localhost.localdomain'
 }
 
-@test "${TEST_NAME_PREFIX} there should be a log entry about the whitelisted and passed e-mail user@whitelist.tld in /var/log/mail/mail.log" {
+
-  _run_in_container bash -c "nc -w 8 0.0.0.0 10023 < /tmp/docker-mailserver-test/nc_templates/postgrey_whitelist.txt"
+# NOTE: These two whitelist tests use `test-files/nc_templates/` instead of `test-files/email-templates`.
-  _run_in_container bash -c "grep -i 'action=pass, reason=client whitelist' /var/log/mail/mail.log | wc -l"
+# - This allows to bypass the SMTP protocol on port 25, and send data directly to Postgrey instead.
-  assert_success
+# - Appears to be a workaround due to `client_name=localhost` when sent from Postfix.
-  assert_output 1
+# - Could send over port 25 if whitelisting `localhost`,
+#   - However this does not help verify that the actual client HELO address is properly whitelisted?
+#   - It'd also cause the earlier greylist test to fail.
+# - TODO: Actually confirm whitelist feature works correctly as these test cases are using a workaround:
+@test "${TEST_NAME_PREFIX} should whitelist sender 'user@whitelist.tld'" {
+  _send_test_mail '/tmp/docker-mailserver-test/nc_templates/postgrey_whitelist.txt' '10023'
+
+  _should_have_log_entry \
+    'action=pass' \
+    'reason=client whitelist' \
+    'client_address=127.0.0.1/32, sender=test@whitelist.tld, recipient=user1@localhost.localdomain'
 }
 
-@test "${TEST_NAME_PREFIX} there should be a log entry about the whitelisted local and passed e-mail user@whitelistlocal.tld in /var/log/mail/mail.log" {
+@test "${TEST_NAME_PREFIX} should whitelist recipient 'user2@otherdomain.tld'" {
-  _run_in_container bash -c "nc -w 8 0.0.0.0 10023 < /tmp/docker-mailserver-test/nc_templates/postgrey_whitelist_local.txt"
+  _send_test_mail '/tmp/docker-mailserver-test/nc_templates/postgrey_whitelist_recipients.txt' '10023'
-  _run_in_container bash -c "grep -i 'action=pass, reason=client whitelist' /var/log/mail/mail.log | wc -l"
+
-  assert_success
+  _should_have_log_entry \
-  assert_output 1
+    'action=pass' \
+    'reason=recipient whitelist' \
+    'client_address=127.0.0.1/32, sender=test@nonwhitelist.tld, recipient=user2@otherdomain.tld'
 }
 
-@test "${TEST_NAME_PREFIX} there should be a log entry about the whitelisted recipient user2@otherdomain.tld in /var/log/mail/mail.log" {
+function _send_test_mail() {
-  _run_in_container bash -c "nc -w 8 0.0.0.0 10023 < /tmp/docker-mailserver-test/nc_templates/postgrey_whitelist_recipients.txt"
+  local MAIL_TEMPLATE=$1
-  _run_in_container bash -c "grep -i 'action=pass, reason=recipient whitelist' /var/log/mail/mail.log | wc -l"
+  local PORT=${2:-25}
-  assert_success
+
-  assert_output 1
+  # `-w 0` terminates the connection after sending the template, it does not wait for a response.
+  # This is required for port 10023, otherwise the connection never drops.
+  # It could increase the number of seconds to wait for port 25 to allow for asserting a response,
+  # but that would enforce the delay in tests for port 10023.
+  _run_in_container bash -c "nc -w 0 0.0.0.0 ${PORT} < ${MAIL_TEMPLATE}"
+}
+
+function _should_have_log_entry() {
+  local ACTION=$1
+  local REASON=$2
+  local TRIPLET=$3
+
+  # Allow some extra time for logs to update to avoids a false-positive failure:
+  run_until_success_or_timeout 10 docker exec "${CONTAINER_NAME}" grep \
+    "${ACTION}, ${REASON}," \
+    /var/log/mail/mail.log
+
+  # Log entry matched should be for the expected triplet:
+  assert_output --partial "${TRIPLET}"
+  _should_output_number_of_lines 1
+}
+
+# `lines` is a special BATS variable updated via `run`:
+function _should_output_number_of_lines() {
+  assert_equal "${#lines[@]}" $1
 }

test/tests/parallel/set1/spam_virus/postscreen.bats

@@ -6,49 +6,77 @@ CONTAINER1_NAME='dms-test_postscreen_enforce'
 CONTAINER2_NAME='dms-test_postscreen_sender'
 
 function setup() {
-  MAIL_POSTSCREEN_IP=$(docker inspect --format '{{ .NetworkSettings.IPAddress }}' "${CONTAINER1_NAME}")
+  CONTAINER1_IP=$(get_container_ip ${CONTAINER1_NAME})
 }
 
 function setup_file() {
-  local CONTAINER_NAME=${CONTAINER1_NAME}
+  export CONTAINER_NAME
+
+  CONTAINER_NAME=${CONTAINER1_NAME}
   local CUSTOM_SETUP_ARGUMENTS=(
     --env POSTSCREEN_ACTION=enforce
-    --cap-add=NET_ADMIN
   )
   init_with_defaults
   common_container_setup 'CUSTOM_SETUP_ARGUMENTS'
   wait_for_smtp_port_in_container "${CONTAINER_NAME}"
 
-  local CONTAINER_NAME=${CONTAINER2_NAME}
+  # A standard DMS instance to send mail from:
+  # NOTE: None of DMS is actually used for this (just bash + nc).
+  CONTAINER_NAME=${CONTAINER2_NAME}
   init_with_defaults
-  common_container_setup
+  # No need to wait for DMS to be ready for this container:
-  wait_for_smtp_port_in_container "${CONTAINER_NAME}"
+  common_container_create
+  run docker start "${CONTAINER_NAME}"
+  assert_success
+
+  # Set default implicit container fallback for helpers:
+  CONTAINER_NAME=${CONTAINER1_NAME}
 }
 
 function teardown_file() {
   docker rm -f "${CONTAINER1_NAME}" "${CONTAINER2_NAME}"
 }
 
-@test "${TEST_NAME_PREFIX} talk too fast" {
+@test "${TEST_NAME_PREFIX} should fail login when talking out of turn" {
-  run docker exec "${CONTAINER2_NAME}" /bin/sh -c "nc ${MAIL_POSTSCREEN_IP} 25 < /tmp/docker-mailserver-test/auth/smtp-auth-login.txt"
+  _run_in_container_explicit "${CONTAINER2_NAME}" bash -c "nc ${CONTAINER1_IP} 25 < /tmp/docker-mailserver-test/auth/smtp-auth-login.txt"
   assert_success
+  assert_output --partial '502 5.5.2 Error: command not recognized'
 
-  repeat_until_success_or_timeout 10 run docker exec "${CONTAINER1_NAME}" grep 'COMMAND PIPELINING' /var/log/mail/mail.log
+  # Expected postscreen log entry:
-  assert_success
+  _run_in_container cat /var/log/mail/mail.log
+  assert_output --partial 'COMMAND PIPELINING'
 }
 
-@test "${TEST_NAME_PREFIX} positive test (respecting postscreen_greet_wait time and talking in turn)" {
+@test "${TEST_NAME_PREFIX} should successfully login (respecting postscreen_greet_wait time)" {
-  for _ in {1,2}; do
+  # NOTE: Sometimes fails on first attempt (trying too soon?),
-    # shellcheck disable=SC1004
+  # Instead of a `run` + asserting partial, Using repeat + internal grep match:
-    docker exec "${CONTAINER2_NAME}" /bin/bash -c \
+  repeat_until_success_or_timeout 10 _should_wait_turn_speaking_smtp \
-    'exec 3<>/dev/tcp/'"${MAIL_POSTSCREEN_IP}"'/25 && \
+    "${CONTAINER2_NAME}" \
+    "${CONTAINER1_IP}" \
+    '/tmp/docker-mailserver-test/auth/smtp-auth-login.txt' \
+    'Authentication successful'
+
+  # Expected postscreen log entry:
+  _run_in_container cat /var/log/mail/mail.log
+  assert_output --partial 'PASS NEW'
+}
+
+# When postscreen is active, it prevents the usual method of piping a file through nc:
+# (Won't work: _run_in_container_explicit "${CLIENT_CONTAINER_NAME}" bash -c "nc ${TARGET_CONTAINER_IP} 25 < ${SMTP_TEMPLATE}")
+# The below workaround respects `postscreen_greet_wait` time (default 6 sec), talking to the mail-server in turn:
+# https://www.postfix.org/postconf.5.html#postscreen_greet_wait
+function _should_wait_turn_speaking_smtp() {
+  local CLIENT_CONTAINER_NAME=$1
+  local TARGET_CONTAINER_IP=$2
+  local SMTP_TEMPLATE=$3
+  local EXPECTED=$4
+
+  local UGLY_WORKAROUND='exec 3<>/dev/tcp/'"${TARGET_CONTAINER_IP}"'/25 && \
     while IFS= read -r cmd; do \
       head -1 <&3; \
       [[ ${cmd} == "EHLO"* ]] && sleep 6; \
       echo ${cmd} >&3; \
-    done < "/tmp/docker-mailserver-test/auth/smtp-auth-login.txt"'
+    done < '"${SMTP_TEMPLATE}"
-  done
 
-  repeat_until_success_or_timeout 10 run docker exec "${CONTAINER1_NAME}" grep 'PASS NEW ' /var/log/mail/mail.log
+  docker exec "${CLIENT_CONTAINER_NAME}" bash -c "${UGLY_WORKAROUND}" | grep "${EXPECTED}"
-  assert_success
 }

test/tests/parallel/set1/spam_virus/undef_spam_subject.bats

@@ -7,8 +7,10 @@ CONTAINER1_NAME='dms-test_spam-undef-subject_1'
 CONTAINER2_NAME='dms-test_spam-undef-subject_2'
 CONTAINER_NAME=${CONTAINER2_NAME}
 
-function setup_file() {
+function teardown() { _default_teardown ; }
-  local CONTAINER_NAME=${CONTAINER1_NAME}
+
+@test "${TEST_NAME_PREFIX} 'SA_SPAM_SUBJECT=undef' should update Amavis config" {
+  export CONTAINER_NAME=${CONTAINER1_NAME}
   local CUSTOM_SETUP_ARGUMENTS=(
     --env ENABLE_AMAVIS=1
     --env ENABLE_SPAMASSASSIN=1
@@ -17,8 +19,14 @@ function setup_file() {
   init_with_defaults
   common_container_setup 'CUSTOM_SETUP_ARGUMENTS'
 
-  # ulimit required for `ENABLE_SRS=1`
+  _run_in_container bash -c "grep '\$sa_spam_subject_tag' /etc/amavis/conf.d/20-debian_defaults | grep '= undef'"
-  local CONTAINER_NAME=${CONTAINER2_NAME}
+  assert_success
+}
+
+# TODO: Unclear why some of these ENV are relevant for the test?
+@test "${TEST_NAME_PREFIX} Docker env variables are set correctly (custom)" {
+  export CONTAINER_NAME=${CONTAINER2_NAME}
+
   local CUSTOM_SETUP_ARGUMENTS=(
     --env ENABLE_CLAMAV=1
     --env SPOOF_PROTECTION=1
@@ -33,17 +41,12 @@ function setup_file() {
     --env ENABLE_SRS=1
     --env ENABLE_MANAGESIEVE=1
     --env PERMIT_DOCKER=host
+    # NOTE: ulimit required for `ENABLE_SRS=1` until running a newer `postsrsd`
     --ulimit "nofile=$(ulimit -Sn):$(ulimit -Hn)"
   )
   init_with_defaults
   common_container_setup 'CUSTOM_SETUP_ARGUMENTS'
-}
 
-function teardown_file() {
-  docker rm -f "${CONTAINER1_NAME}" "${CONTAINER2_NAME}"
-}
-
-@test "${TEST_NAME_PREFIX} Docker env variables are set correctly (custom)" {
   _run_in_container bash -c "grep '\$sa_tag_level_deflt' /etc/amavis/conf.d/20-debian_defaults | grep '= -5.0'"
   assert_success
 
@@ -55,7 +58,4 @@ function teardown_file() {
 
   _run_in_container bash -c "grep '\$sa_spam_subject_tag' /etc/amavis/conf.d/20-debian_defaults | grep '= .SPAM: .'"
   assert_success
-
-  run docker exec "${CONTAINER1_NAME}" bash -c "grep '\$sa_spam_subject_tag' /etc/amavis/conf.d/20-debian_defaults | grep '= undef'"
-  assert_success
 }

test/tests/serial/permit_docker.bats

@@ -71,8 +71,7 @@ teardown_file() {
   # we should be able to send from the other container on the second network!
   run docker exec mail_smtponly_second_network_sender /bin/sh -c "nc mail_smtponly_second_network 25 < /tmp/docker-mailserver-test/email-templates/smtp-only.txt"
   assert_output --partial "250 2.0.0 Ok: queued as "
-  repeat_until_success_or_timeout 60 run docker exec mail_smtponly_second_network /bin/sh -c 'grep -cE "to=<user2\@external.tld>.*status\=sent" /var/log/mail/mail.log'
+  repeat_in_container_until_success_or_timeout 60 mail_smtponly_second_network /bin/sh -c 'grep -cE "to=<user2\@external.tld>.*status\=sent" /var/log/mail/mail.log'
-  [[ ${status} -ge 0 ]]
 }
 
 @test "checking PERMIT_DOCKER: none" {