tests(refactor): Adjust `mail_changedetector` + change detection helpers (#2997)

* tests(refactor): `mail_changedetector.bats` - Leverage DRY methods

`supervisorctl tail` is not the most reliably way to get logs for the latest change detection and has been known to be fragile in the past.

We can instead read the full log for the service directly with `tac` and `sed` to extract all log content since the last change detection.

Common asserts have also been extracted out into separate methods.

* tests(chore): Remove sleep and redundant change event

Container 1 is still blocked at this point from an existing lock and change event.

Make the lock stale immediately and no extra sleep is required when paired with the helper method to wait until the event is processed (which should remove the stale lock).

* tests(refactor): Add more DRY methods

- Simplify the test case so it's easier to grok.
- 2nd test case (blocking) extracts out initial setup into a separate method and merges the later service restart logic which is redundant.
- Additional comments for improved context of what is going on / expected.

* tests(chore): Revise the change detection helper method

- Add explicit counting arg to change detection support.
- Extract revised logic into it's own generic helper method.
- Utilize this for a separate method that monitors for a change event having started, but not waiting for completion.

This allows dropping the 40 sec of remaining `sleep` in `mail_changedetector` test. It was also required due to potentially missing the timing of a change event completing concurrently in a 2nd container that needed to be waited on and then checked.

* tests(chore): Migrate to current test conventions

- Switch to common container setup helpers
- Update container name and change usage to variables instead.
- Adopt the new convention of prefix variable for test cases (revised test case descriptions).

* tests(chore): Remove legacy change detection

This has since been replaced with the new helper watches the `changedetector` service logs directly instead of only detecting a change has occurred via checksum comparison.

No tests use this method anymore, it was originally for `tests.bats`. Thus the tests in `test_helper.bats` are being dropped too. The new helper has test coverage in `changedetector` tests.

* chore: Lock removal should not incur `sleep 5` afterwards

- A new lock should be created by this script after removal. The sleep doesn't help avoid a race condition with lock file creation after removal.
- Reduces test time as a bonus.
- Added some additional comments to test.

* tests(chore): `tls_letsencrypt.bats` leverage improved change detection

- No need to wait on the change detection service anymore during container startup.
- No need to count change events processed either as waiting a fixed duration is no longer relied on.
- This makes the reload count method redundant, dropped.

* tests(chore): Convert `setup-cli.bats` to new test conventions

This test file was already adapted to the original common setup helpers.

- `TEST_NAME` replaced with `CONTAINER_NAME`.
- Prefix var added, test case descriptions drop explicit prefix.
- No other changes.

* tests(chore): Extract out helpers related to change-detection

- New helper file for sharing these helpers to tests.
- Includes the helpful log method from changedetector tests.
- No longer need to maintain duplicate copies of these methods during the test migration. All tests that use them are now importing the separate helper file.
- `tls_letsencrypt.bats` has switched to using the log helper.
- Generic log count helper is removed from `test_helper/common.bash` as any test that needs it in future can adapt to `helper/common.bash`.

* tests(refactor): `tls_letsencrypt.bats` remove `_get_service_logs()`

This helper does not seem useful as moving away from `supervisorctl tail` and no other tests had a need for it.

* tests(chore): Remove common setup methods from `test_helper/common.bash`

No other tests depend on this. Future tests will adopt the revised versions from `helper/setup.bash`.

Additionally updates `helper/setup.bash` comments that are no longer applicable to `TEST_TMP_CONFIG` and `CONTAINER_NAME`.

* chore: Use `|| true` to simplify setting `EXPECTED_COUNT` correctly

target/scripts/helpers/lock.sh

@@ -12,15 +12,16 @@ function _create_lock
   LOCK_FILE="/tmp/docker-mailserver/${SCRIPT_NAME}.lock"
   while [[ -e "${LOCK_FILE}" ]]
   do
-    _log 'warn' "Lock file '${LOCK_FILE}' exists - another execution of '${SCRIPT_NAME}' is happening - trying again shortly"
     # Handle stale lock files left behind on crashes
     # or premature/non-graceful exits of containers while they're making changes
     if [[ -n "$(find "${LOCK_FILE}" -mmin +1 2>/dev/null)" ]]
     then
       _log 'warn' 'Lock file older than 1 minute - removing stale lock file'
       rm -f "${LOCK_FILE}"
+    else
+      _log 'warn' "Lock file '${LOCK_FILE}' exists - another execution of '${SCRIPT_NAME}' is happening - trying again shortly"
+      sleep 5
     fi
-    sleep 5
   done
   trap _remove_lock EXIT
 

test/helper/change-detection.bash

@@ -0,0 +1,29 @@
+#!/bin/bash
+
+load "${REPOSITORY_ROOT}/test/helper/common"
+
+function wait_until_change_detection_event_begins() {
+  local MATCH_CONTENT='Change detected'
+  local MATCH_IN_LOG='/var/log/supervisor/changedetector.log'
+
+  _wait_until_expected_count_is_matched "${@}"
+}
+
+# NOTE: Change events can start and finish all within < 1 sec,
+# Reliably track the completion of a change event by counting events:
+function wait_until_change_detection_event_completes() {
+  local MATCH_CONTENT='Completed handling of detected change'
+  local MATCH_IN_LOG='/var/log/supervisor/changedetector.log'
+  
+  _wait_until_expected_count_is_matched "${@}"
+}
+
+function _get_logs_since_last_change_detection() {
+  local CONTAINER_NAME=${1}
+  local MATCH_IN_FILE='/var/log/supervisor/changedetector.log'
+  local MATCH_STRING='Change detected'
+
+  # Read file in reverse, collect lines until match with sed is found,
+  # then stop and return these lines back in original order (flipped again through tac):
+  docker exec "${CONTAINER_NAME}" bash -c "tac ${MATCH_IN_FILE} | sed '/${MATCH_STRING}/q' | tac"
+}

test/helper/common.bash

@@ -180,40 +180,40 @@ function wait_for_service() {
     container_has_service_running "${CONTAINER_NAME}" "${SERVICE_NAME}"
 }
 
-function wait_for_changes_to_be_detected_in_container() {
-  local CONTAINER_NAME="${1}"
-  local TIMEOUT=${TEST_TIMEOUT_IN_SECONDS}
-
-  # shellcheck disable=SC2016
-  repeat_in_container_until_success_or_timeout "${TIMEOUT}" "${CONTAINER_NAME}" bash -c 'source /usr/local/bin/helpers/index.sh; _obtain_hostname_and_domainname; cmp --silent -- <(_monitored_files_checksums) "${CHKSUM_FILE}" >/dev/null'
-}
-
 # NOTE: Relies on ENV `LOG_LEVEL=debug` or higher
-function wait_until_change_detection_event_completes() {
+function _wait_until_expected_count_is_matched() {
-  local CONTAINER_NAME="${1}"
+  function __get_count() {
+    # NOTE: `|| true` required due to `set -e` usage:
+    # https://github.com/docker-mailserver/docker-mailserver/pull/2997#discussion_r1070583876
+    docker exec "${CONTAINER_NAME}" grep --count "${MATCH_CONTENT}" "${MATCH_IN_LOG}" || true
+  }
+
+  # WARNING: Keep in mind it is a '>=' comparison.
+  # If you provide an explict count to match, ensure it is not too low to cause a false-positive.
+  function __has_expected_count() {
+    [[ $(__get_count) -ge "${EXPECTED_COUNT}" ]]
+  }
+
+  local CONTAINER_NAME=${1}
+  local EXPECTED_COUNT=${2}
+
   # Ensure early failure if arg is missing:
-  assert_not_equal "${CONTAINER_NAME}" ""
+  assert_not_equal "${CONTAINER_NAME}" ''
 
   # Ensure the container is configured with the required `LOG_LEVEL` ENV:
   assert_regex \
     $(docker exec "${CONTAINER_NAME}" env | grep '^LOG_LEVEL=') \
     '=(debug|trace)$'
 
-  # NOTE: Change events can start and finish all within < 1 sec,
+  # Default behaviour is to wait until one new match is found (eg: incremented),
-  # Reliably track the completion of a change event by comparing the before/after count:
+  # unless explicitly set (useful for waiting on a min count to be reached):
-  function __change_event_count() {
+  if [[ -z $EXPECTED_COUNT ]]
-    docker exec "${CONTAINER_NAME}" grep --count "${CHANGE_EVENT_END}" /var/log/supervisor/changedetector.log
+  then
-  }
+    # +1 of starting count:
+    EXPECTED_COUNT=$(( $(__get_count) + 1 ))
+  fi
 
-  function __is_changedetector_finished() {
+  repeat_until_success_or_timeout 20 __has_expected_count
-    [[ $(__change_event_count) -gt "${NUM_CHANGE_EVENTS_BEFORE}" ]]
-  }
-
-  # Count by completions of this debug log line from `check-for-changes.sh`:
-  local CHANGE_EVENT_END='Completed handling of detected change'
-  local NUM_CHANGE_EVENTS_BEFORE=$(__change_event_count)
-
-  repeat_until_success_or_timeout 60 __is_changedetector_finished
 }
 
 # An account added to `postfix-accounts.cf` must wait for the `changedetector` service

test/helper/setup.bash

@@ -64,15 +64,14 @@ function wait_for_finished_setup_in_container() {
 #
 # For example, if you need an immutable config volume that can't be affected by other tests
 # in the file, then use `local TEST_TMP_CONFIG=$(duplicate_config_for_container . "${UNIQUE_ID_HERE}")`
+#
+# REQUIRED: `CONTAINER_NAME` must be set before this method is called.
+# It only affects the `TEST_TMP_CONFIG` directory created,
+# but will be used in `common_container_create()` and implicitly in other helper methods.
 function init_with_defaults() {
   __initialize_variables
 
   export TEST_TMP_CONFIG
-
-  # In `setup_file()` the default name to use for the currently tested docker container
-  # is `${CONTAINER_NAME}` global defined here. It derives the name from the test filename:
-  # `basename` to ignore absolute dir path and file extension, only extract filename.
-  # In `setup_file()` creates a single copy of the test config folder to use for an entire test file:
   TEST_TMP_CONFIG=$(duplicate_config_for_container . "${CONTAINER_NAME}")
 
   # Common complimentary test files, read-only safe to share across containers:

test/test_helper/common.bash

@@ -173,42 +173,6 @@ function wait_for_service() {
     container_has_service_running "${CONTAINER_NAME}" "${SERVICE_NAME}"
 }
 
-function wait_for_changes_to_be_detected_in_container() {
-  local CONTAINER_NAME="${1}"
-  local TIMEOUT=${TEST_TIMEOUT_IN_SECONDS}
-
-  # shellcheck disable=SC2016
-  repeat_in_container_until_success_or_timeout "${TIMEOUT}" "${CONTAINER_NAME}" bash -c 'source /usr/local/bin/helpers/index.sh; _obtain_hostname_and_domainname; cmp --silent -- <(_monitored_files_checksums) "${CHKSUM_FILE}" >/dev/null'
-}
-
-# NOTE: Relies on ENV `LOG_LEVEL=debug` or higher
-function wait_until_change_detection_event_completes() {
-  local CONTAINER_NAME="${1}"
-  # Ensure early failure if arg is missing:
-  assert_not_equal "${CONTAINER_NAME}" ""
-
-  # Ensure the container is configured with the required `LOG_LEVEL` ENV:
-  assert_regex \
-    $(docker exec "${CONTAINER_NAME}" env | grep '^LOG_LEVEL=') \
-    '=(debug|trace)$'
-
-  # NOTE: Change events can start and finish all within < 1 sec,
-  # Reliably track the completion of a change event by comparing the before/after count:
-  function __change_event_count() {
-    docker exec "${CONTAINER_NAME}" grep --count "${CHANGE_EVENT_END}" /var/log/supervisor/changedetector.log
-  }
-
-  function __is_changedetector_finished() {
-    [[ $(__change_event_count) -gt "${NUM_CHANGE_EVENTS_BEFORE}" ]]
-  }
-
-  # Count by completions of this debug log line from `check-for-changes.sh`:
-  local CHANGE_EVENT_END='Completed handling of detected change'
-  local NUM_CHANGE_EVENTS_BEFORE=$(__change_event_count)
-
-  repeat_until_success_or_timeout 60 __is_changedetector_finished
-}
-
 # An account added to `postfix-accounts.cf` must wait for the `changedetector` service
 # to process the update before Dovecot creates the mail account and associated storage dir:
 function wait_until_account_maildir_exists() {
@@ -241,72 +205,3 @@ function wait_for_empty_mail_queue_in_container() {
   # shellcheck disable=SC2016
   repeat_in_container_until_success_or_timeout "${TIMEOUT}" "${CONTAINER_NAME}" bash -c '[[ $(mailq) == *"Mail queue is empty"* ]]'
 }
-
-# Common defaults appropriate for most tests, override vars in each test when necessary.
-# For all tests override in `setup_file()` via an `export` var.
-# For individual test override the var via `local` var instead.
-#
-# For example, if you need an immutable config volume that can't be affected by other tests
-# in the file, then use `local TEST_TMP_CONFIG=$(duplicate_config_for_container . "${UNIQUE_ID_HERE}")`
-function init_with_defaults() {
-  export TEST_NAME TEST_TMP_CONFIG
-
-  # In `setup_file()` the default name to use for the currently tested docker container
-  # is `${TEST_NAME}` global defined here. It derives the name from the test filename:
-  # `basename` to ignore absolute dir path and file extension, only extract filename.
-  TEST_NAME=$(basename "${BATS_TEST_FILENAME}" '.bats')
-  # In `setup_file()` creates a single copy of the test config folder to use for an entire test file:
-  TEST_TMP_CONFIG=$(duplicate_config_for_container . "${TEST_NAME}")
-
-  # Common complimentary test files, read-only safe to share across containers:
-  export TEST_FILES_CONTAINER_PATH='/tmp/docker-mailserver-test'
-  export TEST_FILES_VOLUME="${PWD}/test/test-files:${TEST_FILES_CONTAINER_PATH}:ro"
-
-  # The config volume cannot be read-only as some data needs to be written at container startup
-  # - two sed failures (unknown lines)
-  # - dovecot-quotas.cf (setup-stack.sh:_setup_dovecot_quotas)
-  # - postfix-aliases.cf (setup-stack.sh:_setup_postfix_aliases)
-  # TODO: Check how many tests need write access. Consider using `docker create` + `docker cp` for easier cleanup.
-  export TEST_CONFIG_VOLUME="${TEST_TMP_CONFIG}:/tmp/docker-mailserver"
-
-  # The common default FQDN assigned to the container `--hostname` option:
-  export TEST_FQDN='mail.my-domain.com'
-
-  # Default Root CA cert used in TLS tests with `openssl` commands:
-  export TEST_CA_CERT="${TEST_FILES_CONTAINER_PATH}/ssl/example.test/with_ca/ecdsa/ca-cert.ecdsa.pem"
-}
-
-# Using `create` and `start` instead of only `run` allows to modify
-# the container prior to starting it. Otherwise use this combined method.
-# NOTE: Forwards all args to the create method at present.
-function common_container_setup() {
-  common_container_create "$@"
-  common_container_start
-}
-
-# Common docker setup is centralized here.
-#
-# `X_EXTRA_ARGS` - Optional: Pass an array by it's variable name as a string, it will
-# be used as a reference for appending extra config into the `docker create` below:
-#
-# NOTE: Using array reference for a single input parameter, as this method is still
-# under development while adapting tests to it and requirements it must serve (eg: support base config matrix in CI)
-function common_container_create() {
-  [[ -n ${1} ]] && local -n X_EXTRA_ARGS=${1}
-
-  run docker create --name "${TEST_NAME}" \
-    --hostname "${TEST_FQDN}" \
-    --tty \
-    --volume "${TEST_FILES_VOLUME}" \
-    --volume "${TEST_CONFIG_VOLUME}" \
-    "${X_EXTRA_ARGS[@]}" \
-    "${NAME}"
-  assert_success
-}
-
-function common_container_start() {
-  run docker start "${TEST_NAME}"
-  assert_success
-
-  wait_for_finished_setup_in_container "${TEST_NAME}"
-}

test/tests/parallel/set2/tls/tls_letsencrypt.bats

@@ -1,5 +1,6 @@
-load "${REPOSITORY_ROOT}/test/helper/setup"
 load "${REPOSITORY_ROOT}/test/helper/common"
+load "${REPOSITORY_ROOT}/test/helper/change-detection"
+load "${REPOSITORY_ROOT}/test/helper/setup"
 load "${REPOSITORY_ROOT}/test/helper/tls"
 
 BATS_TEST_NAME_PREFIX='[Security] (TLS) (SSL_TYPE=letsencrypt) '
@@ -103,18 +104,13 @@ function _initial_setup() {
     )
     common_container_setup 'CUSTOM_SETUP_ARGUMENTS'
     wait_for_service "${CONTAINER_NAME}" 'changedetector'
-
-    # Wait until the changedetector service startup delay is over:
-    repeat_until_success_or_timeout 20 sh -c "$(_get_service_logs 'changedetector') | grep 'Changedetector is ready'"
   }
 
   # Test `acme.json` extraction works at container startup:
   # It should have already extracted `mail.example.test` from the original mounted `acme.json`.
   function _acme_ecdsa() {
-    _should_have_succeeded_at_extraction 'mail.example.test'
+    # SSL_DOMAIN value should not be present in current `acme.json`:
-
+    _should_fail_to_extract_for_wildcard_env
-    # SSL_DOMAIN set as ENV, but startup should not have match in `acme.json`:
-    _should_have_failed_at_extraction '*.example.test' 'mailserver'
     _should_have_valid_config 'mail.example.test' 'key.pem' 'fullchain.pem'
 
     local ECDSA_KEY_PATH="${LOCAL_BASE_PATH}/key.ecdsa.pem"
@@ -127,7 +123,6 @@ function _initial_setup() {
   # It should replace the cert files in the existing `letsencrypt/live/mail.example.test/` folder.
   function _acme_rsa() {
     _should_extract_on_changes 'mail.example.test' "${LOCAL_BASE_PATH}/rsa.acme.json"
-    _should_have_service_reload_count '1'
 
     local RSA_KEY_PATH="${LOCAL_BASE_PATH}/key.rsa.pem"
     local RSA_CERT_PATH="${LOCAL_BASE_PATH}/cert.rsa.pem"
@@ -139,7 +134,6 @@ function _initial_setup() {
   # Wildcard `*.example.test` should extract to `example.test/` in `letsencrypt/live/`:
   function _acme_wildcard() {
     _should_extract_on_changes 'example.test' "${LOCAL_BASE_PATH}/wildcard/rsa.acme.json"
-    _should_have_service_reload_count '2'
 
     # As the FQDN has changed since startup, the Postfix + Dovecot configs should be updated:
     _should_have_valid_config 'example.test' 'key.pem' 'fullchain.pem'
@@ -194,21 +188,17 @@ function _has_matching_line() {
 # Traefik `acme.json` specific
 #
 
-# It should log success of extraction for the expected domain and restart Postfix.
+function _should_fail_to_extract_for_wildcard_env() {
-function _should_have_succeeded_at_extraction() {
+  # Set as value for ENV `SSL_DOMAIN`, but during startup it should fail to find a match in the current `acme.json`:
-  local EXPECTED_DOMAIN=${1}
+  local DOMAIN_WILDCARD='*.example.test'
-  local SERVICE=${2}
+  # The expected domain to be found and extracted instead (value from container `--hostname`):
+  local DOMAIN_MAIL='mail.example.test'
 
-  run $(_get_service_logs "${SERVICE}")
+  # /var/log/mail/mail.log is not equivalent to stdout content,
-  assert_output --partial "_extract_certs_from_acme | Certificate successfully extracted for '${EXPECTED_DOMAIN}'"
+  # Relevant log content only available via docker logs:
-}
+  run docker logs "${CONTAINER_NAME}"
-
+  assert_output --partial "_extract_certs_from_acme | Unable to find key and/or cert for '${DOMAIN_WILDCARD}' in '/etc/letsencrypt/acme.json'"
-function _should_have_failed_at_extraction() {
+  assert_output --partial "_extract_certs_from_acme | Certificate successfully extracted for '${DOMAIN_MAIL}'"
-  local EXPECTED_DOMAIN=${1}
-  local SERVICE=${2}
-
-  run $(_get_service_logs "${SERVICE}")
-  assert_output --partial "_extract_certs_from_acme | Unable to find key and/or cert for '${EXPECTED_DOMAIN}' in '/etc/letsencrypt/acme.json'"
 }
 
 # Replace the mounted `acme.json` and wait to see if changes were detected.
@@ -217,25 +207,12 @@ function _should_extract_on_changes() {
   local ACME_JSON=${2}
 
   cp "${ACME_JSON}" "${TEST_TMP_CONFIG}/letsencrypt/acme.json"
-  # Change detection takes a little over 5 seconds to complete (restart services)
+  wait_until_change_detection_event_completes "${CONTAINER_NAME}"
-  sleep 10
 
   # Expected log lines from the changedetector service:
-  run $(_get_service_logs 'changedetector')
+  run _get_logs_since_last_change_detection "${CONTAINER_NAME}"
-  assert_output --partial 'Change detected'
   assert_output --partial "'/etc/letsencrypt/acme.json' has changed - extracting certificates"
   assert_output --partial "_extract_certs_from_acme | Certificate successfully extracted for '${EXPECTED_DOMAIN}'"
-  assert_output --partial 'Reloading services due to detected changes'
-  assert_output --partial 'Completed handling of detected change'
-}
-
-# Ensure change detection is not mistakenly validating against previous change events:
-function _should_have_service_reload_count() {
-  local NUM_RELOADS=${1}
-
-  # Count how many times processes (like Postfix and Dovecot) have been reloaded by the `changedetector` service:
-  _run_in_container grep --count 'Completed handling of detected change' '/var/log/supervisor/changedetector.log'
-  assert_output "${NUM_RELOADS}"
 }
 
 # Extracted cert files from `acme.json` have content matching the expected reference files:
@@ -278,18 +255,3 @@ function _should_be_equal_in_content() {
   assert_output "$(cat "${LOCAL_PATH}")"
   assert_success
 }
-
-function _get_service_logs() {
-  local SERVICE=${1:-'mailserver'}
-
-  local CMD_LOGS=(docker exec "${CONTAINER_NAME}" "supervisorctl tail -2200 ${SERVICE}")
-
-  # As the `mailserver` service logs are not stored in a file but output to stdout/stderr,
-  # The `supervisorctl tail` command won't work; we must instead query via `docker logs`:
-  if [[ ${SERVICE} == 'mailserver' ]]
-  then
-    CMD_LOGS=(docker logs "${CONTAINER_NAME}")
-  fi
-
-  echo "${CMD_LOGS[@]}"
-}

test/tests/parallel/set3/smtp-delivery.bats

@@ -1,4 +1,5 @@
 load "${REPOSITORY_ROOT}/test/helper/common"
+load "${REPOSITORY_ROOT}/test/helper/change-detection"
 load "${REPOSITORY_ROOT}/test/helper/setup"
 
 BATS_TEST_NAME_PREFIX='[SMTP] (delivery) '

test/tests/serial/mail_changedetector.bats

@@ -1,87 +1,139 @@
-load "${REPOSITORY_ROOT}/test/test_helper/common"
+load "${REPOSITORY_ROOT}/test/helper/common"
+load "${REPOSITORY_ROOT}/test/helper/change-detection"
+load "${REPOSITORY_ROOT}/test/helper/setup"
 
-# Note if tests fail asserting against `supervisorctl tail changedetector` output,
+BATS_TEST_NAME_PREFIX='[Change Detection] '
-# use `supervisorctl tail -<num bytes> changedetector` instead to increase log output.
+
-# Default `<num bytes>` appears to be around 1500.
+CONTAINER1_NAME='dms-test_changedetector_one'
+CONTAINER2_NAME='dms-test_changedetector_two'
 
 function setup_file() {
-  local PRIVATE_CONFIG
+  export CONTAINER_NAME
-  PRIVATE_CONFIG=$(duplicate_config_for_container . mail_changedetector_one)
 
-  docker run -d --name mail_changedetector_one \
+  local CUSTOM_SETUP_ARGUMENTS=(
-    -v "${PRIVATE_CONFIG}":/tmp/docker-mailserver \
+    --env LOG_LEVEL=trace
-    -v "$(pwd)/test/test-files":/tmp/docker-mailserver-test:ro \
+  )
-    -e LOG_LEVEL=trace \
-    -h mail.my-domain.com -t "${NAME}"
 
-  docker run -d --name mail_changedetector_two \
+  CONTAINER_NAME=${CONTAINER1_NAME}
-    -v "${PRIVATE_CONFIG}":/tmp/docker-mailserver \
+  init_with_defaults
-    -v "$(pwd)/test/test-files":/tmp/docker-mailserver-test:ro \
+  common_container_setup 'CUSTOM_SETUP_ARGUMENTS'
-    -e LOG_LEVEL=trace \
-    -h mail.my-domain.com -t "${NAME}"
 
-  wait_for_finished_setup_in_container mail_changedetector_one
+  CONTAINER_NAME=${CONTAINER2_NAME}
-  wait_for_finished_setup_in_container mail_changedetector_two
+  # NOTE: No `init_with_defaults` used here,
+  # Intentionally sharing previous containers config instead.
+  common_container_setup 'CUSTOM_SETUP_ARGUMENTS'
+
+  # Set default implicit container fallback for helpers:
+  CONTAINER_NAME=${CONTAINER1_NAME}
 }
 
 function teardown_file() {
-  docker rm -f mail_changedetector_one
+  docker rm -f "${CONTAINER1_NAME}" "${CONTAINER2_NAME}"
-  docker rm -f mail_changedetector_two
 }
 
-@test "checking changedetector: servers are ready" {
+@test "changedetector service is ready" {
-  wait_for_service mail_changedetector_one changedetector
+  wait_for_service "${CONTAINER1_NAME}" changedetector
-  wait_for_service mail_changedetector_two changedetector
+  wait_for_service "${CONTAINER2_NAME}" changedetector
 }
 
-@test "checking changedetector: can detect changes & between two containers using same config" {
+# NOTE: Non-deterministic behaviour - One container will perform change detection before the other.
-  echo "" >> "$(private_config_path mail_changedetector_one)/postfix-accounts.cf"
+# Depending on the timing of the other container checking for a lock file, the lock may no longer be
-  sleep 25
+# present, avoiding the 5 second delay. The first container to create a lock is not deterministic either.
+# NOTE: Change detection at this point typically occurs at 2 or 4 seconds since the service was up,
+# thus expect 2-8 seconds to complete.
+@test "should detect and process changes (in both containers with shared config)" {
+  _create_change_event
 
-  run docker exec mail_changedetector_one /bin/bash -c 'supervisorctl tail -3000 changedetector'
+  _should_perform_standard_change_event "${CONTAINER1_NAME}"
-  assert_output --partial 'Change detected'
+  _should_perform_standard_change_event "${CONTAINER2_NAME}"
-  assert_output --partial 'Reloading services due to detected changes'
+}
-  assert_output --partial 'Removed lock'
-  assert_output --partial 'Completed handling of detected change'
 
-  run docker exec mail_changedetector_two /bin/bash -c 'supervisorctl tail -3000 changedetector'
+# Both containers should acknowledge the foreign lock file added,
-  assert_output --partial 'Change detected'
+# blocking an attempt to process the pending change event detected.
+@test "should find existing lock file and block processing changes (without removing lock)" {
+  _prepare_blocking_lock_test
+
+  # Wait until the 2nd change event attempts to process:
+  _should_block_change_event_from_processing "${CONTAINER1_NAME}" 2
+  # NOTE: Although the service is restarted, a change detection should still occur (previous checksum still exists):
+  _should_block_change_event_from_processing "${CONTAINER2_NAME}" 1
+}
+
+@test "should remove lock file when stale" {
+  # Avoid a race condition (to remove the lock file) by removing the 2nd container:
+  docker rm -f "${CONTAINER2_NAME}"
+  # Make the previously created lock file become stale:
+  docker exec "${CONTAINER1_NAME}" touch -d '60 seconds ago' /tmp/docker-mailserver/check-for-changes.sh.lock
+
+  # A 2nd change event should complete (or may already have if quick enough?):
+  wait_until_change_detection_event_completes "${CONTAINER1_NAME}" 2
+
+  # Should have removed the stale lock file, then handle the change event:
+  run _get_logs_since_last_change_detection "${CONTAINER1_NAME}"
+  assert_output --partial 'Lock file older than 1 minute - removing stale lock file'
+  _assert_has_standard_change_event_logs
+}
+
+function _should_perform_standard_change_event() {
+  local CONTAINER_NAME=${1}
+
+  # Wait for change detection event to start and complete processing:
+  # NOTE: An explicit count is provided as the 2nd container may have already completed processing.
+  wait_until_change_detection_event_completes "${CONTAINER_NAME}" 1
+
+  # Container should have created it's own lock file,
+  # and later removed it when finished processing:
+  run _get_logs_since_last_change_detection "${CONTAINER_NAME}"
+  _assert_has_standard_change_event_logs
+}
+
+function _should_block_change_event_from_processing() {
+  local CONTAINER_NAME=${1}
+  local EXPECTED_COUNT=${2}
+
+  # Once the next change event has started, the processing blocked log ('another execution') should be present:
+  wait_until_change_detection_event_begins "${CONTAINER_NAME}" "${EXPECTED_COUNT}"
+
+  run _get_logs_since_last_change_detection "${CONTAINER_NAME}"
+  _assert_foreign_lock_exists
+  # This additionally verifies that the change event processing is incomplete (blocked):
+  _assert_no_lock_actions_performed
+}
+
+function _assert_has_standard_change_event_logs() {
+  assert_output --partial "Creating lock '/tmp/docker-mailserver/check-for-changes.sh.lock'"
   assert_output --partial 'Reloading services due to detected changes'
   assert_output --partial 'Removed lock'
   assert_output --partial 'Completed handling of detected change'
 }
 
-@test "checking changedetector: lock file found, blocks, and doesn't get prematurely removed" {
+function _assert_foreign_lock_exists() {
-  run docker exec mail_changedetector_two /bin/bash -c "supervisorctl stop changedetector"
+  assert_output --partial "Lock file '/tmp/docker-mailserver/check-for-changes.sh.lock' exists"
-  docker exec mail_changedetector_one /bin/bash -c "touch /tmp/docker-mailserver/check-for-changes.sh.lock"
+  assert_output --partial "- another execution of 'check-for-changes.sh' is happening - trying again shortly"
-  echo "" >> "$(private_config_path mail_changedetector_one)/postfix-accounts.cf"
-  run docker exec mail_changedetector_two /bin/bash -c "supervisorctl start changedetector"
-  sleep 15
-
-  run docker exec mail_changedetector_one /bin/bash -c "supervisorctl tail changedetector"
-  assert_output --partial "another execution of 'check-for-changes.sh' is happening"
-  run docker exec mail_changedetector_two /bin/bash -c "supervisorctl tail changedetector"
-  assert_output --partial "another execution of 'check-for-changes.sh' is happening"
-
-  # Ensure starting a new check-for-changes.sh instance (restarting here) doesn't delete the lock
-  docker exec mail_changedetector_two /bin/bash -c "rm -f /var/log/supervisor/changedetector.log"
-  run docker exec mail_changedetector_two /bin/bash -c "supervisorctl restart changedetector"
-  sleep 5
-  run docker exec mail_changedetector_two /bin/bash -c "supervisorctl tail changedetector"
-  refute_output --partial "another execution of 'check-for-changes.sh' is happening"
-  refute_output --partial "Removed lock"
 }
 
-@test "checking changedetector: lock stale and cleaned up" {
+function _assert_no_lock_actions_performed() {
-  docker rm -f mail_changedetector_two
+  refute_output --partial 'Lock file older than 1 minute - removing stale lock file'
-  docker exec mail_changedetector_one /bin/bash -c "touch /tmp/docker-mailserver/check-for-changes.sh.lock"
+  refute_output --partial "Creating lock '/tmp/docker-mailserver/check-for-changes.sh.lock'"
-  echo "" >> "$(private_config_path mail_changedetector_one)/postfix-accounts.cf"
+  refute_output --partial 'Removed lock'
-  sleep 15
+}
-
+
-  run docker exec mail_changedetector_one /bin/bash -c "supervisorctl tail changedetector"
+function _prepare_blocking_lock_test {
-  assert_output --partial "another execution of 'check-for-changes.sh' is happening"
+  # Temporarily disable the Container2 changedetector service:
-  sleep 65
+  docker exec "${CONTAINER2_NAME}" bash -c 'supervisorctl stop changedetector'
-
+  docker exec "${CONTAINER2_NAME}" bash -c 'rm -f /var/log/supervisor/changedetector.log'
-  run docker exec mail_changedetector_one /bin/bash -c "supervisorctl tail -3000 changedetector"
+
-  assert_output --partial "removing stale lock file"
+  # Create a foreign lock file to prevent change processing (in both containers):
+  docker exec "${CONTAINER1_NAME}" bash -c 'touch /tmp/docker-mailserver/check-for-changes.sh.lock'
+  # Create a new change to detect (that the foreign lock should prevent from processing):
+  _create_change_event
+
+  # Restore Container2 changedetector service:
+  # NOTE: The last known checksum is retained in Container2,
+  #       It will be compared to and start a change event.
+  docker exec "${CONTAINER2_NAME}" bash -c 'supervisorctl start changedetector'
+}
+
+function _create_change_event() {
+  echo '' >>"${TEST_TMP_CONFIG}/postfix-accounts.cf"
 }

test/tests/serial/setup-cli.bats

@@ -1,7 +1,9 @@
-load "${REPOSITORY_ROOT}/test/test_helper/common"
+load "${REPOSITORY_ROOT}/test/helper/common"
+load "${REPOSITORY_ROOT}/test/helper/change-detection"
+load "${REPOSITORY_ROOT}/test/helper/setup"
 
-# Globals referenced from `test_helper/common`:
+BATS_TEST_NAME_PREFIX='[setup.sh] '
-# TEST_NAME (should match the filename, minus the bats extension)
+CONTAINER_NAME='dms-test_setup-cli'
 
 # This is a bare minimal container setup.
 # All test-cases run sequentially against the same container instance,
@@ -21,29 +23,29 @@ function setup_file() {
 }
 
 function teardown_file() {
-  docker rm -f "${TEST_NAME}"
+  docker rm -f "${CONTAINER_NAME}"
 }
 
-@test "checking setup.sh: show usage when no arguments provided" {
+@test "show usage when no arguments provided" {
   run ./setup.sh
   assert_success
   assert_output --partial "This is the main administration script that you use for all your interactions with"
 }
 
-@test "checking setup.sh: exit with error when wrong arguments provided" {
+@test "exit with error when wrong arguments provided" {
   run ./setup.sh lol troll
   assert_failure
   assert_line --index 0 --partial "The command 'lol troll' is invalid."
 }
 
 # Create a new account for subsequent tests to depend upon
-@test "checking setup.sh: setup.sh email add and login" {
+@test "email add (with login)" {
   local MAIL_ACCOUNT='user@example.com'
   local MAIL_PASS='test_password'
   local DATABASE_ACCOUNTS="${TEST_TMP_CONFIG}/postfix-accounts.cf"
 
   # Create an account
-  run ./setup.sh -c "${TEST_NAME}" email add "${MAIL_ACCOUNT}" "${MAIL_PASS}"
+  run ./setup.sh -c "${CONTAINER_NAME}" email add "${MAIL_ACCOUNT}" "${MAIL_PASS}"
   assert_success
 
   # Verify account was added to `postfix-accounts.cf`:
@@ -53,25 +55,25 @@ function teardown_file() {
 
   # Ensure you wait until `changedetector` is finished.
   # Mail account and storage directory should now be valid
-  wait_until_change_detection_event_completes "${TEST_NAME}"
+  wait_until_change_detection_event_completes "${CONTAINER_NAME}"
 
   # Verify mail storage directory exists (polls if storage is slow, eg remote mount):
-  wait_until_account_maildir_exists "${TEST_NAME}" "${MAIL_ACCOUNT}"
+  wait_until_account_maildir_exists "${CONTAINER_NAME}" "${MAIL_ACCOUNT}"
 
   # Verify account authentication is successful (account added to Dovecot UserDB+PassDB):
-  wait_for_service "${TEST_NAME}" dovecot
+  wait_for_service "${CONTAINER_NAME}" dovecot
   local RESPONSE
-  RESPONSE=$(docker exec "${TEST_NAME}" doveadm auth test "${MAIL_ACCOUNT}" "${MAIL_PASS}" | grep 'passdb')
+  RESPONSE=$(docker exec "${CONTAINER_NAME}" doveadm auth test "${MAIL_ACCOUNT}" "${MAIL_PASS}" | grep 'passdb')
   assert_equal "${RESPONSE}" "passdb: ${MAIL_ACCOUNT} auth succeeded"
 }
 
-@test "checking setup.sh: setup.sh email list" {
+@test "email list" {
-  run ./setup.sh -c "${TEST_NAME}" email list
+  run ./setup.sh -c "${CONTAINER_NAME}" email list
   assert_success
 }
 
 # Update an existing account
-@test "checking setup.sh: setup.sh email update" {
+@test "email update" {
   local MAIL_ACCOUNT='user@example.com'
   local MAIL_PASS='test_password'
   local DATABASE_ACCOUNTS="${TEST_TMP_CONFIG}/postfix-accounts.cf"
@@ -83,12 +85,12 @@ function teardown_file() {
 
   # Update the password should be successful:
   local NEW_PASS='new_password'
-  run ./setup.sh -c "${TEST_NAME}" email update "${MAIL_ACCOUNT}" "${NEW_PASS}"
+  run ./setup.sh -c "${CONTAINER_NAME}" email update "${MAIL_ACCOUNT}" "${NEW_PASS}"
   refute_output --partial 'Password must not be empty'
   assert_success
 
   # NOTE: this was put in place for the next test `setup.sh email del` to properly work.
-  wait_until_change_detection_event_completes "${TEST_NAME}"
+  wait_until_change_detection_event_completes "${CONTAINER_NAME}"
 
   # `postfix-accounts.cf` should have an updated password hash stored:
   local NEW_PASS_HASH
@@ -97,7 +99,7 @@ function teardown_file() {
   assert_not_equal "${NEW_PASS_HASH}" "${MAIL_PASS_HASH}"
 
   # Verify Dovecot derives NEW_PASS_HASH from NEW_PASS:
-  run docker exec "${TEST_NAME}" doveadm pw -t "${NEW_PASS_HASH}" -p "${NEW_PASS}"
+  run docker exec "${CONTAINER_NAME}" doveadm pw -t "${NEW_PASS_HASH}" -p "${NEW_PASS}"
   refute_output 'Fatal: reverse password verification check failed: Password mismatch'
   assert_output "${NEW_PASS_HASH} (verified)"
 }
@@ -107,19 +109,19 @@ function teardown_file() {
 # has no support to mount a volume to `/var/mail` (only via `-c` to use a running container),
 # thus the `-y` option to delete the account maildir has no effect nor informs the user.
 # https://github.com/docker-mailserver/docker-mailserver/issues/949
-@test "checking setup.sh: setup.sh email del" {
+@test "email del" {
   local MAIL_ACCOUNT='user@example.com'
   local MAIL_PASS='test_password'
 
   # Account deletion is successful:
-  run ./setup.sh -c "${TEST_NAME}" email del -y "${MAIL_ACCOUNT}"
+  run ./setup.sh -c "${CONTAINER_NAME}" email del -y "${MAIL_ACCOUNT}"
   assert_success
 
   # NOTE: Sometimes the directory still exists, possibly from change detection
   # of the previous test (`email udpate`) triggering. Therefore, the function
   # `wait_until_change_detection_event_completes was added to the
   # `setup.sh email update` test.
-  repeat_in_container_until_success_or_timeout 60 "${TEST_NAME}" bash -c '[[ ! -d /var/mail/example.com/user ]]'
+  repeat_in_container_until_success_or_timeout 60 "${CONTAINER_NAME}" bash -c '[[ ! -d /var/mail/example.com/user ]]'
 
   # Account is not present in `postfix-accounts.cf`:
   run grep "${MAIL_ACCOUNT}" "${TEST_TMP_CONFIG}/postfix-accounts.cf"
@@ -130,83 +132,83 @@ function teardown_file() {
   # which will rebuild Dovecots accounts from scratch.
 }
 
-@test "checking setup.sh: setup.sh email restrict" {
+@test "email restrict" {
-  run ./setup.sh -c "${TEST_NAME}" email restrict
+  run ./setup.sh -c "${CONTAINER_NAME}" email restrict
   assert_failure
-  run ./setup.sh -c "${TEST_NAME}" email restrict add
+  run ./setup.sh -c "${CONTAINER_NAME}" email restrict add
   assert_failure
-  ./setup.sh -c "${TEST_NAME}" email restrict add send lorem@impsum.org
+  ./setup.sh -c "${CONTAINER_NAME}" email restrict add send lorem@impsum.org
-  run ./setup.sh -c "${TEST_NAME}" email restrict list send
+  run ./setup.sh -c "${CONTAINER_NAME}" email restrict list send
   assert_output --regexp "^lorem@impsum.org.*REJECT"
 
-  run ./setup.sh -c "${TEST_NAME}" email restrict del send lorem@impsum.org
+  run ./setup.sh -c "${CONTAINER_NAME}" email restrict del send lorem@impsum.org
   assert_success
-  run ./setup.sh -c "${TEST_NAME}" email restrict list send
+  run ./setup.sh -c "${CONTAINER_NAME}" email restrict list send
   assert_output --partial "Everyone is allowed"
 
-  ./setup.sh -c "${TEST_NAME}" email restrict add receive rec_lorem@impsum.org
+  ./setup.sh -c "${CONTAINER_NAME}" email restrict add receive rec_lorem@impsum.org
-  run ./setup.sh -c "${TEST_NAME}" email restrict list receive
+  run ./setup.sh -c "${CONTAINER_NAME}" email restrict list receive
   assert_output --regexp "^rec_lorem@impsum.org.*REJECT"
-  run ./setup.sh -c "${TEST_NAME}" email restrict del receive rec_lorem@impsum.org
+  run ./setup.sh -c "${CONTAINER_NAME}" email restrict del receive rec_lorem@impsum.org
   assert_success
 }
 
 # alias
-@test "checking setup.sh: setup.sh alias list" {
+@test "alias list" {
-  run ./setup.sh -c "${TEST_NAME}" alias list
+  run ./setup.sh -c "${CONTAINER_NAME}" alias list
   assert_success
   assert_output --partial "alias1@localhost.localdomain user1@localhost.localdomain"
   assert_output --partial "@localdomain2.com user1@localhost.localdomain"
 }
 
-@test "checking setup.sh: setup.sh alias add" {
+@test "alias add" {
-  ./setup.sh -c "${TEST_NAME}" alias add alias@example.com target1@forward.com
+  ./setup.sh -c "${CONTAINER_NAME}" alias add alias@example.com target1@forward.com
-  ./setup.sh -c "${TEST_NAME}" alias add alias@example.com target2@forward.com
+  ./setup.sh -c "${CONTAINER_NAME}" alias add alias@example.com target2@forward.com
-  ./setup.sh -c "${TEST_NAME}" alias add alias2@example.org target3@forward.com
+  ./setup.sh -c "${CONTAINER_NAME}" alias add alias2@example.org target3@forward.com
   sleep 5
   run grep "alias@example.com target1@forward.com,target2@forward.com" "${TEST_TMP_CONFIG}/postfix-virtual.cf"
   assert_success
 }
 
-@test "checking setup.sh: setup.sh alias del" {
+@test "alias del" {
-  ./setup.sh -c "${TEST_NAME}" alias del alias@example.com target1@forward.com
+  ./setup.sh -c "${CONTAINER_NAME}" alias del alias@example.com target1@forward.com
   run grep "target1@forward.com" "${TEST_TMP_CONFIG}/postfix-virtual.cf"
   assert_failure
 
   run grep "target2@forward.com" "${TEST_TMP_CONFIG}/postfix-virtual.cf"
   assert_output "alias@example.com target2@forward.com"
 
-  ./setup.sh -c "${TEST_NAME}" alias del alias@example.org target2@forward.com
+  ./setup.sh -c "${CONTAINER_NAME}" alias del alias@example.org target2@forward.com
   run grep "alias@example.org" "${TEST_TMP_CONFIG}/postfix-virtual.cf"
   assert_failure
 
   run grep "alias2@example.org" "${TEST_TMP_CONFIG}/postfix-virtual.cf"
   assert_success
 
-  ./setup.sh -c "${TEST_NAME}" alias del alias2@example.org target3@forward.com
+  ./setup.sh -c "${CONTAINER_NAME}" alias del alias2@example.org target3@forward.com
   run grep "alias2@example.org" "${TEST_TMP_CONFIG}/postfix-virtual.cf"
   assert_failure
 }
 
 # quota
-@test "checking setup.sh: setup.sh setquota" {
+@test "setquota" {
-  ./setup.sh -c "${TEST_NAME}" email add quota_user@example.com test_password
+  ./setup.sh -c "${CONTAINER_NAME}" email add quota_user@example.com test_password
-  ./setup.sh -c "${TEST_NAME}" email add quota_user2@example.com test_password
+  ./setup.sh -c "${CONTAINER_NAME}" email add quota_user2@example.com test_password
 
-  run ./setup.sh -c "${TEST_NAME}" quota set quota_user@example.com 12M
+  run ./setup.sh -c "${CONTAINER_NAME}" quota set quota_user@example.com 12M
   assert_success
-  run ./setup.sh -c "${TEST_NAME}" quota set 51M quota_user@example.com
+  run ./setup.sh -c "${CONTAINER_NAME}" quota set 51M quota_user@example.com
   assert_failure
-  run ./setup.sh -c "${TEST_NAME}" quota set unknown@domain.com 150M
+  run ./setup.sh -c "${CONTAINER_NAME}" quota set unknown@domain.com 150M
   assert_failure
 
-  run ./setup.sh -c "${TEST_NAME}" quota set quota_user2 51M
+  run ./setup.sh -c "${CONTAINER_NAME}" quota set quota_user2 51M
   assert_failure
 
   run /bin/sh -c "cat ${TEST_TMP_CONFIG}/dovecot-quotas.cf | grep -E '^quota_user@example.com\:12M\$' | wc -l | grep 1"
   assert_success
 
-  run ./setup.sh -c "${TEST_NAME}" quota set quota_user@example.com 26M
+  run ./setup.sh -c "${CONTAINER_NAME}" quota set quota_user@example.com 26M
   assert_success
   run /bin/sh -c "cat ${TEST_TMP_CONFIG}/dovecot-quotas.cf | grep -E '^quota_user@example.com\:26M\$' | wc -l | grep 1"
   assert_success
@@ -216,47 +218,47 @@ function teardown_file() {
 }
 
 # `quota_user@example.com` created in previous `setquota` test
-@test "checking setup.sh: setup.sh delquota" {
+@test "delquota" {
-  run ./setup.sh -c "${TEST_NAME}" quota set quota_user@example.com 12M
+  run ./setup.sh -c "${CONTAINER_NAME}" quota set quota_user@example.com 12M
   assert_success
   run /bin/sh -c "cat ${TEST_TMP_CONFIG}/dovecot-quotas.cf | grep -E '^quota_user@example.com\:12M\$' | wc -l | grep 1"
   assert_success
 
-  run ./setup.sh -c "${TEST_NAME}" quota del unknown@domain.com
+  run ./setup.sh -c "${CONTAINER_NAME}" quota del unknown@domain.com
   assert_failure
   run /bin/sh -c "cat ${TEST_TMP_CONFIG}/dovecot-quotas.cf | grep -E '^quota_user@example.com\:12M\$' | wc -l | grep 1"
   assert_success
 
-  run ./setup.sh -c "${TEST_NAME}" quota del quota_user@example.com
+  run ./setup.sh -c "${CONTAINER_NAME}" quota del quota_user@example.com
   assert_success
   run grep "quota_user@example.com" "${TEST_TMP_CONFIG}/dovecot-quotas.cf"
   assert_failure
 }
 
-@test "checking setup.sh: setup.sh config dkim help correctly displayed" {
+@test "config dkim (help correctly displayed)" {
-  run ./setup.sh -c "${TEST_NAME}" config dkim help
+  run ./setup.sh -c "${CONTAINER_NAME}" config dkim help
   assert_success
   assert_line --index 3 --partial "    open-dkim - configure DomainKeys Identified Mail (DKIM)"
 }
 
 # debug
 
-@test "checking setup.sh: setup.sh debug fetchmail" {
+@test "debug fetchmail" {
-  run ./setup.sh -c "${TEST_NAME}" debug fetchmail
+  run ./setup.sh -c "${CONTAINER_NAME}" debug fetchmail
   assert_failure
   assert_output --partial "fetchmail: normal termination, status 11"
 }
 
-@test "checking setup.sh: setup.sh debug login ls" {
+@test "debug login ls" {
-  run ./setup.sh -c "${TEST_NAME}" debug login ls
+  run ./setup.sh -c "${CONTAINER_NAME}" debug login ls
   assert_success
 }
 
-@test "checking setup.sh: setup.sh relay add-domain" {
+@test "relay add-domain" {
-  ./setup.sh -c "${TEST_NAME}" relay add-domain example1.org smtp.relay1.com 2525
+  ./setup.sh -c "${CONTAINER_NAME}" relay add-domain example1.org smtp.relay1.com 2525
-  ./setup.sh -c "${TEST_NAME}" relay add-domain example2.org smtp.relay2.com
+  ./setup.sh -c "${CONTAINER_NAME}" relay add-domain example2.org smtp.relay2.com
-  ./setup.sh -c "${TEST_NAME}" relay add-domain example3.org smtp.relay3.com 2525
+  ./setup.sh -c "${CONTAINER_NAME}" relay add-domain example3.org smtp.relay3.com 2525
-  ./setup.sh -c "${TEST_NAME}" relay add-domain example3.org smtp.relay.com 587
+  ./setup.sh -c "${CONTAINER_NAME}" relay add-domain example3.org smtp.relay.com 587
 
   # check adding
   run /bin/sh -c "cat ${TEST_TMP_CONFIG}/postfix-relaymap.cf | grep -e '^@example1.org\s\+\[smtp.relay1.com\]:2525' | wc -l | grep 1"
@@ -269,10 +271,10 @@ function teardown_file() {
   assert_success
 }
 
-@test "checking setup.sh: setup.sh relay add-auth" {
+@test "relay add-auth" {
-  ./setup.sh -c "${TEST_NAME}" relay add-auth example.org smtp_user smtp_pass
+  ./setup.sh -c "${CONTAINER_NAME}" relay add-auth example.org smtp_user smtp_pass
-  ./setup.sh -c "${TEST_NAME}" relay add-auth example2.org smtp_user2 smtp_pass2
+  ./setup.sh -c "${CONTAINER_NAME}" relay add-auth example2.org smtp_user2 smtp_pass2
-  ./setup.sh -c "${TEST_NAME}" relay add-auth example2.org smtp_user2 smtp_pass_new
+  ./setup.sh -c "${CONTAINER_NAME}" relay add-auth example2.org smtp_user2 smtp_pass_new
 
   # test adding
   run /bin/sh -c "cat ${TEST_TMP_CONFIG}/postfix-sasl-password.cf | grep -e '^@example.org\s\+smtp_user:smtp_pass' | wc -l | grep 1"
@@ -282,8 +284,8 @@ function teardown_file() {
   assert_success
 }
 
-@test "checking setup.sh: setup.sh relay exclude-domain" {
+@test "relay exclude-domain" {
-  ./setup.sh -c "${TEST_NAME}" relay exclude-domain example.org
+  ./setup.sh -c "${CONTAINER_NAME}" relay exclude-domain example.org
 
   run /bin/sh -c "cat ${TEST_TMP_CONFIG}/postfix-relaymap.cf | grep -e '^@example.org\s*$' | wc -l | grep 1"
   assert_success

test/tests/serial/test_helper.bats

@@ -158,57 +158,6 @@ BATS_TEST_NAME_PREFIX='test helper functions:'
   assert_failure
 }
 
-@test "wait_for_changes_to_be_detected_in_container fails when timeout is reached" {
-  local PRIVATE_CONFIG
-  PRIVATE_CONFIG=$(duplicate_config_for_container .)
-
-  # variable not local to make visible to teardown
-  CONTAINER_NAME=$(docker run -d --rm \
-    -v "${PRIVATE_CONFIG}":/tmp/docker-mailserver \
-    -h mail.my-domain.com \
-    -t "${NAME}")
-
-  teardown() { docker rm -f "${CONTAINER_NAME}"; }
-
-  # wait for the initial checksum file to be created
-  # shellcheck disable=SC2016
-  repeat_in_container_until_success_or_timeout 60 "${CONTAINER_NAME}" bash -c 'source /usr/local/bin/helpers/index.sh; test -e "${CHKSUM_FILE}"'
-
-  # there should be no changes in the beginning
-  TEST_TIMEOUT_IN_SECONDS=0 wait_for_changes_to_be_detected_in_container "${CONTAINER_NAME}"
-
-  # trigger some change
-  docker exec "${CONTAINER_NAME}" /bin/sh -c "addmailuser auser3@mail.my-domain.com mypassword"
-
-  # that should be picked up as not yet detected
-  ! TEST_TIMEOUT_IN_SECONDS=0 wait_for_changes_to_be_detected_in_container "${CONTAINER_NAME}"
-}
-
-@test "wait_for_changes_to_be_detected_in_container succeeds within timeout" {
-  local PRIVATE_CONFIG
-  PRIVATE_CONFIG=$(duplicate_config_for_container .)
-
-  # variable not local to make visible to teardown
-  CONTAINER_NAME=$(docker run -d --rm \
-    -v "${PRIVATE_CONFIG}":/tmp/docker-mailserver \
-    -h mail.my-domain.com \
-    -t "${NAME}")
-
-  teardown() { docker rm -f "${CONTAINER_NAME}"; }
-
-  # wait for the initial checksum file to be created
-  # shellcheck disable=SC2016
-  repeat_in_container_until_success_or_timeout 60 "${CONTAINER_NAME}" bash -c 'source /usr/local/bin/helpers/index.sh; test -e "${CHKSUM_FILE}"'
-
-  # trigger some change
-  docker exec "${CONTAINER_NAME}" /bin/sh -c "addmailuser auser3@mail.my-domain.com mypassword"
-
-  # that should eventually be detected
-  SECONDS=0
-  wait_for_changes_to_be_detected_in_container "${CONTAINER_NAME}"
-  [[ ${SECONDS} -gt 0 ]]
-}
-
 # TODO investigate why this test fails
 @test "wait_for_empty_mail_queue_in_container fails when timeout reached" {
   skip 'disabled as it fails randomly: https://github.com/docker-mailserver/docker-mailserver/pull/2177'

test/tests/serial/tests.bats

@@ -1,4 +1,5 @@
 load "${REPOSITORY_ROOT}/test/helper/common"
+load "${REPOSITORY_ROOT}/test/helper/change-detection"
 load "${REPOSITORY_ROOT}/test/helper/setup"
 
 CONTAINER_NAME='mail'