From f4f0e4ef6159027983aa53f9ffb50885f87b66b8 Mon Sep 17 00:00:00 2001
From: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
Date: Mon, 20 Sep 2021 19:35:03 +1200
Subject: [PATCH] tests: Refactored bounced spam test + Introduce common
 container setup template (#2198)

* fix: Spam bounced test copy/paste typo
* tests(docs): Expand inline documentation

Should assist maintainers like myself that are not yet familiar with this functionality, saving some time :)

* Refactor bounced test + Introduce initial container template

DRY'd up the test and extracted a common init pattern for other tests to adopt in future.

The test does not need to run distinct containers at once, so a common name is fine, although the `init_with_defaults()` method could be given an arg to add a suffix: `init_with_defaults "_${BATS_TEST_NUMBER}"` which could be called in `setup()` for tests that can benefit from being run in parallel.

Often it seems the containers only need the bare minimum config such as accounts provided to actually make the container happy to perform a test, so sharing a `:ro` config mount is fine, or in future this could be better addressed.

---

The test would fail if the test cases requiring smtp access ran before postfix was ready (_only a few seconds after setup scripts announce being done_). Added the wait condition for smtp, took a while to track that failure down.
---
 target/scripts/startup/setup-stack.sh |  4 +-
 test/mail_spam_bounced.bats           | 88 ++++++++++++++-------------
 test/test_helper/common.bash          | 32 ++++++++++
 3 files changed, 80 insertions(+), 44 deletions(-)

diff --git a/target/scripts/startup/setup-stack.sh b/target/scripts/startup/setup-stack.sh
index 300ce5fb..8a1d1c04 100644
--- a/target/scripts/startup/setup-stack.sh
+++ b/target/scripts/startup/setup-stack.sh
@@ -1469,11 +1469,13 @@ function _setup_security_stack
 
     if [[ ${SPAMASSASSIN_SPAM_TO_INBOX} -eq 1 ]]
     then
-      _notify 'inf' 'Configure Spamassassin/Amavis to put SPAM inbox'
+      _notify 'inf' 'Configuring Spamassassin/Amavis to send SPAM to inbox'
 
       sed -i "s|\$final_spam_destiny.*=.*$|\$final_spam_destiny = D_PASS;|g" /etc/amavis/conf.d/49-docker-mailserver
       sed -i "s|\$final_bad_header_destiny.*=.*$|\$final_bad_header_destiny = D_PASS;|g" /etc/amavis/conf.d/49-docker-mailserver
     else
+      _notify 'inf' 'Configuring Spamassassin/Amavis to bounce SPAM'
+
       sed -i "s|\$final_spam_destiny.*=.*$|\$final_spam_destiny = D_BOUNCE;|g" /etc/amavis/conf.d/49-docker-mailserver
       sed -i "s|\$final_bad_header_destiny.*=.*$|\$final_bad_header_destiny = D_BOUNCE;|g" /etc/amavis/conf.d/49-docker-mailserver
 
diff --git a/test/mail_spam_bounced.bats b/test/mail_spam_bounced.bats
index da62e3e3..17db7dd8 100644
--- a/test/mail_spam_bounced.bats
+++ b/test/mail_spam_bounced.bats
@@ -2,73 +2,75 @@ load 'test_helper/common'
 
 # Test case
 # ---------
-# When SPAMASSASSIN_SPAM_TO_INBOX=0, spam messages must be bounced.
+# When SPAMASSASSIN_SPAM_TO_INBOX=0, spam messages must be bounced (rejected).
+# SPAMASSASSIN_SPAM_TO_INBOX=1 is covered in `mail_spam_junk_folder.bats`.
+# Original test PR: https://github.com/docker-mailserver/docker-mailserver/pull/1485
 
+# TODO: ENV setup will move to actual ENV files in future.
 
 function setup() {
-    run_setup_file_if_necessary
+  run_setup_file_if_necessary
 }
 
 function teardown() {
-    run_teardown_file_if_necessary
+  docker rm -f "${TEST_NAME}"
+  run_teardown_file_if_necessary
 }
 
 function setup_file() {
-    local PRIVATE_CONFIG
-    PRIVATE_CONFIG="$(duplicate_config_for_container . mail_spam_bounced_defined)"
-    docker run -d --name mail_spam_bounced_defined \
-              -v "${PRIVATE_CONFIG}":/tmp/docker-mailserver \
-              -v "$(pwd)/test/test-files":/tmp/docker-mailserver-test:ro \
-              -e ENABLE_SPAMASSASSIN=1 \
-              -e SPAMASSASSIN_SPAM_TO_INBOX=0 \
-              -h mail.my-domain.com -t "${NAME}"
-
-    wait_for_finished_setup_in_container mail_spam_bounced_defined
-
-    PRIVATE_CONFIG="$(duplicate_config_for_container . mail_spam_bounced_defined)"
-    docker run -d --name mail_spam_bounced_undefined \
-              -v "${PRIVATE_CONFIG}":/tmp/docker-mailserver \
-              -v "$(pwd)/test/test-files":/tmp/docker-mailserver-test:ro \
-              -e ENABLE_SPAMASSASSIN=1 \
-              -h mail.my-domain.com -t "${NAME}"
-
-    wait_for_finished_setup_in_container mail_spam_bounced_undefined
+  init_with_defaults
 }
 
-function teardown_file() {
-    docker rm -f mail_spam_bounced_defined
-    docker rm -f mail_spam_bounced_undefined
-}
+# Not used
+# function teardown_file() {
+# }
 
 @test "first" {
   skip 'this test must come first to reliably identify when to run setup_file'
 }
 
-@test "checking amavis: spam message is bounced" {
-  # this warning should only be raised when no explicit value for SPAMASSASSIN_SPAM_TO_INBOX is defined
-  run sh -c "docker logs mail_spam_bounced_defined | grep 'Spam messages WILL NOT BE DELIVERED'"
+@test "checking amavis: spam message is bounced (rejected)" {
+  local TEST_ENV_FILE="${PRIVATE_CONFIG}/defined.env"
+  echo 'ENABLE_SPAMASSASSIN=1' > "${TEST_ENV_FILE}"
+  echo 'SPAMASSASSIN_SPAM_TO_INBOX=0' >> "${TEST_ENV_FILE}"
+
+  common_container_setup "${TEST_ENV_FILE}"
+
+  run _should_emit_warning
   assert_failure
 
-  # send a spam message
-  run docker exec mail_spam_bounced_defined /bin/sh -c "nc 0.0.0.0 25 < /tmp/docker-mailserver-test/email-templates/amavis-spam.txt"
-  assert_success
-
-  run repeat_until_success_or_timeout 20 sh -c "docker logs mail_spam_bounced_defined | grep 'Blocked SPAM {NoBounceInbound,Quarantined}'"
-  assert_success
+  _should_bounce_spam
 }
 
-@test "checking amavis: spam message is bounced, undefined SPAMASSASSIN_SPAM_TO_INBOX raise a warning" {
-  run sh -c "docker logs mail_spam_bounced_undefined | grep 'Spam messages WILL NOT BE DELIVERED'"
+@test "checking amavis: spam message is bounced (rejected), undefined SPAMASSASSIN_SPAM_TO_INBOX should raise a warning" {
+  # SPAMASSASSIN_SPAM_TO_INBOX=0 is the default. If no explicit ENV value is set, it should log a warning at startup.
+  local TEST_ENV_FILE="${PRIVATE_CONFIG}/undefined.env"
+  echo 'ENABLE_SPAMASSASSIN=1' > "${TEST_ENV_FILE}"
+
+  common_container_setup "${TEST_ENV_FILE}"
+
+  run _should_emit_warning
   assert_success
 
-  # send a spam message
-  run docker exec mail_spam_bounced_defined /bin/sh -c "nc 0.0.0.0 25 < /tmp/docker-mailserver-test/email-templates/amavis-spam.txt"
-  assert_success
-
-  run repeat_until_success_or_timeout 20 sh -c "docker logs mail_spam_bounced_defined | grep 'Blocked SPAM {NoBounceInbound,Quarantined}'"
-  assert_success
+  _should_bounce_spam
 }
 
 @test "last" {
   skip 'this test is only there to reliably mark the end for the teardown_file'
 }
+
+# This warning should only be raised when the env SPAMASSASSIN_SPAM_TO_INBOX has no explicit value set
+function _should_emit_warning() {
+  sh -c "docker logs ${TEST_NAME} | grep 'Spam messages WILL NOT BE DELIVERED'"
+}
+
+function _should_bounce_spam() {
+  wait_for_smtp_port_in_container_to_respond "${TEST_NAME}"
+
+  # send a spam message
+  run docker exec "${TEST_NAME}" /bin/sh -c "nc 0.0.0.0 25 < /tmp/docker-mailserver-test/email-templates/amavis-spam.txt"
+  assert_success
+
+  run repeat_until_success_or_timeout 20 sh -c "docker logs ${TEST_NAME} | grep 'Blocked SPAM {NoBounceInbound,Quarantined}'"
+  assert_success
+}
diff --git a/test/test_helper/common.bash b/test/test_helper/common.bash
index baccc553..4b0ff38d 100644
--- a/test/test_helper/common.bash
+++ b/test/test_helper/common.bash
@@ -104,6 +104,7 @@ function wait_for_amavis_port_in_container() {
     wait_for_tcp_port_in_container 10024 "${1}"
 }
 
+# TODO: Should also fail early on "docker logs ${1} | egrep '^[  FATAL  ]'"?
 # @param ${1} name of the postfix container
 function wait_for_finished_setup_in_container() {
     local STATUS=0
@@ -205,3 +206,34 @@ 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.
+# TODO: Check how many tests need write access. Consider using `docker create` + `docker cp` for easier cleanup.
+function init_with_defaults() {
+  # Ignore absolute dir path and file extension, only extract filename:
+  export TEST_NAME
+  TEST_NAME="$(basename "${BATS_TEST_FILENAME}" '.bats')"
+
+  export PRIVATE_CONFIG
+  PRIVATE_CONFIG="$(duplicate_config_for_container . "${TEST_NAME}")"
+  export TEST_FILES_VOLUME="${PWD}/test/test-files:/tmp/docker-mailserver-test:ro"
+  export TEST_CONFIG_VOLUME="${PRIVATE_CONFIG}:/tmp/docker-mailserver:ro"
+
+  export TEST_FQDN='mail.my-domain.com'
+}
+
+# Common docker run command that should satisfy most tests which only modify ENV.
+function common_container_setup() {
+  local TEST_ENV_FILE=$1
+
+  run docker run -d --name "${TEST_NAME}" \
+    --volume "${TEST_FILES_VOLUME}" \
+    --volume "${TEST_CONFIG_VOLUME}" \
+    --hostname "${TEST_FQDN}" \
+    --env-file "${TEST_ENV_FILE}" \
+    --tty \
+    "${NAME}"
+  assert_success
+
+  wait_for_finished_setup_in_container "${TEST_NAME}"
+}
\ No newline at end of file