tests: new sending and filtering functions (#3786)

* move log/filter functions into own file

* add ShellCheck global directives

* use new function for tracking logs

The new function, called `_send_email_with_mid`, aligns with suggestions
from @polarethene and is heavily simplified compared to its predecessor
`_send_email_and_get_id`. New helpers will be introduced to filter logs
according to the MID constructed in this function.

* new filters for searching logs with MID

* use new filters (and sending) functions

* add new helper for asserting non-existence of log message

* use new filters in tests

* Apply suggestions from code review

- `_mid` / `MID` => `_msgid` / `MSG_ID`
- Revised documentation / tooltip comments

* Apply suggestions from code review

* fix tests

* use more distinct names for MSG_ID headers

* update `_filter_service_log` to not use `-i -E`

Moreover, I added a function to print the whole mail log. Appropriate
comments were added to this function to indicate that one should only
use this function when necessary.

* adjust helpers to new helper filter

* follow-up of previous commit

* add CHANGELOG entry

* Apply suggestions from code review

* chore: Update OAuth2 to use new log helper

* Apply suggestions from code review

Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>

* added explicit `_regexp` filters for logs

* Apply suggestions from code review

---------

Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>

test/helper/common.bash

@@ -18,6 +18,7 @@ function __load_bats_helper() {
   load "${REPOSITORY_ROOT}/test/test_helper/bats-support/load"
   load "${REPOSITORY_ROOT}/test/test_helper/bats-assert/load"
   load "${REPOSITORY_ROOT}/test/helper/sending"
+  load "${REPOSITORY_ROOT}/test/helper/log_and_filtering"
 }
 
 __load_bats_helper
@@ -228,7 +229,6 @@ function _run_until_success_or_timeout() {
 # ! -------------------------------------------------------------------
 # ? >> Functions to wait until a condition is met
 
-
 # Wait until a port is ready.
 #
 # @param ${1} = port
@@ -351,15 +351,6 @@ function _add_mail_account_then_wait_until_ready() {
   _wait_until_account_maildir_exists "${MAIL_ACCOUNT}"
 }
 
-# Assert that the number of lines output by a previous command matches the given
-# amount (${1}). `lines` is a special BATS variable updated via `run`.
-#
-# @param ${1} = number of lines that the output should have
-function _should_output_number_of_lines() {
-  # shellcheck disable=SC2154
-  assert_equal "${#lines[@]}" "${1:?Number of lines not provided}"
-}
-
 # Reloads the postfix service.
 #
 # @param ${1} = container name [OPTIONAL]
@@ -372,7 +363,6 @@ function _reload_postfix() {
   _exec_in_container postfix reload
 }
 
-
 # Get the IP of the container (${1}).
 #
 # @param ${1} = container name [OPTIONAL]
@@ -413,62 +403,6 @@ function _should_have_content_in_directory() {
   assert_success
 }
 
-# Filters a service's logs (under `/var/log/supervisor/<SERVICE>.log`) given
-# a specific string.
-#
-# @param ${1} = service name
-# @param ${2} = string to filter by
-# @param ${3} = container name [OPTIONAL]
-#
-# ## Attention
-#
-# The string given to this function is interpreted by `grep -E`, i.e.
-# as a regular expression. In case you use characters that are special
-# in regular expressions, you need to escape them!
-function _filter_service_log() {
-  local SERVICE=${1:?Service name must be provided}
-  local STRING=${2:?String to match must be provided}
-  local CONTAINER_NAME=$(__handle_container_name "${3:-}")
-  local FILE="/var/log/supervisor/${SERVICE}.log"
-
-  # Fallback to alternative log location:
-  [[ -f ${FILE} ]] || FILE="/var/log/mail/${SERVICE}.log"
-  _run_in_container grep -E "${STRING}" "${FILE}"
-}
-
-# Like `_filter_service_log` but asserts that the string was found.
-#
-# @param ${1} = service name
-# @param ${2} = string to filter by
-# @param ${3} = container name [OPTIONAL]
-#
-# ## Attention
-#
-# The string given to this function is interpreted by `grep -E`, i.e.
-# as a regular expression. In case you use characters that are special
-# in regular expressions, you need to escape them!
-function _service_log_should_contain_string() {
-  local SERVICE=${1:?Service name must be provided}
-  local STRING=${2:?String to match must be provided}
-  local CONTAINER_NAME=$(__handle_container_name "${3:-}")
-
-  _filter_service_log "${SERVICE}" "${STRING}"
-  assert_success
-}
-
-# Filters the mail log for lines that belong to a certain email identified
-# by its ID. You can obtain the ID of an email you want to send by using
-# `_send_email_and_get_id`.
-#
-# @param ${1} = email ID
-# @param ${2} = container name [OPTIONAL]
-function _print_mail_log_for_id() {
-  local MAIL_ID=${1:?Mail ID must be provided}
-  local CONTAINER_NAME=$(__handle_container_name "${2:-}")
-
-  _run_in_container grep -E "${MAIL_ID}" /var/log/mail.log
-}
-
 # A simple wrapper for netcat (`nc`). This is useful when sending
 # "raw" e-mails or doing IMAP-related work.
 #

test/helper/log_and_filtering.bash

@@ -0,0 +1,119 @@
+#!/bin/bash
+
+# ? ABOUT: Functions defined here aid in working with logs and filtering them.
+
+# ! ATTENTION: This file is loaded by `common.sh` - do not load it yourself!
+# ! ATTENTION: This file requires helper functions from `common.sh`!
+
+# shellcheck disable=SC2034,SC2155
+
+# Assert that the number of lines output by a previous command matches the given amount (${1}).
+# `lines` is a special BATS variable updated via `run`.
+#
+# @param ${1} = number of lines that the output should have
+function _should_output_number_of_lines() {
+  # shellcheck disable=SC2154
+  assert_equal "${#lines[@]}" "${1:?Number of lines not provided}"
+}
+
+# Filters a service's logs (under `/var/log/supervisor/<SERVICE>.log`) given a specific string.
+#
+# @param ${1} = service name
+# @param ${2} = string to filter by
+# @param ...  = options given to `grep` (which is used to filter logs)
+function _filter_service_log() {
+  local SERVICE=${1:?Service name must be provided}
+  local STRING=${2:?String to match must be provided}
+  shift 2
+
+  local FILE="/var/log/supervisor/${SERVICE}.log"
+  # Alternative log location fallback:
+  [[ -f ${FILE} ]] || FILE="/var/log/mail/${SERVICE}.log"
+  _run_in_container grep "${@}" "${STRING}" "${FILE}"
+}
+
+# Prints the entirety of the primary mail log.
+# Avoid using this method when you could filter more specific log lines with:
+#
+# 1. _filter_service_log
+# 2. _service_log_should[_not]_contain_string
+function _show_complete_mail_log() {
+  _run_in_container cat /var/log/mail/mail.log
+}
+
+# Like `_filter_service_log` but asserts that the string was found.
+#
+# @param ${1} = service name
+# @param ${2} = string to filter by
+function _service_log_should_contain_string() {
+  _filter_service_log "${1}" "${2}" --fixed-strings
+  assert_success
+}
+
+# Like `_filter_service_log` but asserts that the string was _not_ found.
+#
+# @param ${1} = service name
+# @param ${2} = string to filter by
+function _service_log_should_not_contain_string() {
+  _filter_service_log "${1}" "${2}" --fixed-strings
+  assert_failure
+}
+
+# Like `_filter_service_log` but asserts that the string was found.
+# Uses regular expressions under the hood for pattern matching.
+#
+# @param ${1} = service name
+# @param ${2} = regular expression to filter by
+function _service_log_should_contain_string_regexp() {
+  _filter_service_log "${1}" "${2}" --extended-regexp
+  assert_success
+}
+
+# Like `_filter_service_log` but asserts that the string was _not_ found.
+# Uses regular expressions under the hood for pattern matching.
+#
+# @param ${1} = service name
+# @param ${2} = regular expression to filter by
+function _service_log_should_not_contain_string_regexp() {
+  _filter_service_log "${1}" "${2}" --extended-regexp
+  assert_failure
+}
+
+# Filters the mail log by the given MSG_ID (Message-ID) parameter,
+# printing log lines which include the associated Postfix Queue ID.
+#
+# @param ${1} = The local-part of a Message-ID header value (`<local-part@domain-part>`)
+function _print_mail_log_of_queue_id_from_msgid() {
+  # A unique ID Postfix generates for tracking queued mail as it's processed.
+  # The length can vary (as per the postfix docs). Hence, we use a range to safely capture it.
+  # https://github.com/docker-mailserver/docker-mailserver/pull/3747#discussion_r1446679671
+  local QUEUE_ID_REGEX='[A-Z0-9]{9,12}'
+
+  local MSG_ID=$(__construct_msgid "${1:?The local-part for MSG_ID was not provided}")
+  shift 1
+
+  _wait_for_empty_mail_queue_in_container
+
+  QUEUE_ID=$(_exec_in_container tac /var/log/mail.log                    \
+    | grep -E "postfix/cleanup.*: ${QUEUE_ID_REGEX}:.*message-id=${MSG_ID}" \
+    | grep -E --only-matching --max-count 1 "${QUEUE_ID_REGEX}" || :)
+
+  # We perform plausibility checks on the IDs.
+  assert_not_equal "${QUEUE_ID}" ''
+  run echo "${QUEUE_ID}"
+  assert_line --regexp "^${QUEUE_ID_REGEX}$"
+
+  # Postfix specific logs:
+  _filter_service_log 'mail' "${QUEUE_ID}"
+}
+
+# A convenience method that filters for Dovecot specific logs with a `msgid` field that matches the MSG_ID input.
+#
+# @param ${1} = The local-part of a Message-ID header value (`<local-part@domain-part>`)
+function _print_mail_log_for_msgid() {
+  local MSG_ID=$(__construct_msgid "${1:?The local-part for MSG_ID was not provided}")
+  shift 1
+
+  # Dovecot specific logs:
+  _filter_service_log 'mail' "msgid=${MSG_ID}"
+}

test/helper/sending.bash

@@ -1,11 +1,13 @@
 #!/bin/bash
 
-# shellcheck disable=SC2034,SC2155
-
 # ? ABOUT: Functions defined here help with sending emails in tests.
 
 # ! ATTENTION: This file is loaded by `common.sh` - do not load it yourself!
 # ! ATTENTION: This file requires helper functions from `common.sh`!
+# ! ATTENTION: Functions prefixed with `__` are intended for internal use within
+# !            this file (or other helpers) only, not in tests.
+
+# shellcheck disable=SC2034,SC2155
 
 # Sends an e-mail from the container named by the environment variable `CONTAINER_NAME`
 # to the same or another container.
@@ -100,74 +102,35 @@ function _send_email() {
   return "${RETURN_VALUE}"
 }
 
-# Like `_send_email` with two major differences:
+# Construct the value for a 'Message-ID' header.
+# For tests we use only the local-part to identify mail activity in logs. The rest of the value is fixed.
 #
-# 1. this function waits for the mail to be processed; there is no asynchronicity
-#    because filtering the logs in a synchronous way is easier and safer;
-# 2. this function takes the name of a variable and inserts IDs one can later
-#    filter by to check logs.
+# A Message-ID header value should be in the form of: `<local-part@domain-part>`
+# https://en.wikipedia.org/wiki/Message-ID
+# https://datatracker.ietf.org/doc/html/rfc5322#section-3.6.4
 #
-# No. 2 is especially useful in case you send more than one email in a single
-# test file and need to assert certain log entries for each mail individually.
+# @param ${1} = The local-part of a Message-ID header value (`<local-part@domain-part>`)
+function __construct_msgid() {
+  local MSG_ID_LOCALPART=${1:?The local-part for MSG_ID was not provided}
+  echo "<${MSG_ID_LOCALPART}@dms-tests>"
+}
+
+# Like `_send_email` but adds a "Message-ID: ${1}@dms-tests>" header,
+# which allows for filtering logs later.
 #
-# The first argument has to be the name of the variable that the e-mail ID is stored in.
-# The second argument **can** be the flag `--expect-rejection`.
-#
-# - If this flag is supplied, the function does not check whether the whole mail delivery
-#   transaction was successful. Additionally the queue ID will be retrieved differently.
-# - CAUTION: It must still be possible to `grep` for the Message-ID that Postfix
-#   generated in the mail log; otherwise this function fails.
-#
-# The rest of the arguments are the same as `_send_email`.
-#
-# ## Attention
-#
-# This function assumes `CONTAINER_NAME` to be properly set (to the container
-# name the command should be executed in)!
-#
-# ## Safety
-#
-# This functions assumes **no concurrent sending of emails to the same container**!
-# If two clients send simultaneously, there is no guarantee the correct ID is
-# chosen. Sending more than one mail at any given point in time with this function
-# is UNDEFINED BEHAVIOR!
-function _send_email_and_get_id() {
-  # Export the variable denoted by ${1} so everyone has access
-  export "${1:?Mail ID must be set for _send_email_and_get_id}"
-  # Get a "reference" to the content of the variable denoted by ${1} so we can manipulate the content
-  local -n ID_ENV_VAR_REF=${1:?}
-  # Prepare the message ID header here because we will shift away ${1} later
-  local MID="<${1}@dms-tests>"
-  # Get rid of ${1} so only the arguments for swaks remain
+# @param ${1} = The local-part of a Message-ID header value (`<local-part@domain-part>`)
+function _send_email_with_msgid() {
+  local MSG_ID=$(__construct_msgid "${1:?The local-part for MSG_ID was not provided}")
   shift 1
 
-  # The unique ID Postfix (and other services) use may be different in length
-  # on different systems. Hence, we use a range to safely capture it.
-  local QUEUE_ID_REGEX='[A-Z0-9]{9,12}'
-
-  _wait_for_empty_mail_queue_in_container
-  _send_email "${@}" --header "Message-Id: ${MID}"
-  _wait_for_empty_mail_queue_in_container
-
-  # We store Postfix's queue ID first
-  ID_ENV_VAR_REF=$(_exec_in_container tac /var/log/mail.log                    \
-    | grep -E "postfix/cleanup.*: ${QUEUE_ID_REGEX}:.*message-id=${MID}" \
-    | grep -E --only-matching --max-count 1 "${QUEUE_ID_REGEX}" || :)
-  # But we also requre potential Dovecot sieve output, which requires the mesage ID,
-  # so we need to provide the message ID too.
-  ID_ENV_VAR_REF+="|${MID}"
-
-  # Last but not least, we perform plausibility checks on the IDs.
-  assert_not_equal "${ID_ENV_VAR_REF}" ''
-  run echo "${ID_ENV_VAR_REF}"
-  assert_line --regexp "^${QUEUE_ID_REGEX}\|${MID}$"
+  _send_email "${@}" --header "Message-ID: ${MSG_ID}"
 }
 
 # Send a spam e-mail by utilizing GTUBE.
 #
-# Extra arguments given to this function will be supplied by `_send_email_and_get_id` directly.
+# Extra arguments given to this function will be supplied by `_send_email_with_msgid` directly.
 function _send_spam() {
-  _send_email_and_get_id MAIL_ID_SPAM "${@}" \
+  _send_email_with_msgid 'dms-test-email-spam' "${@}" \
     --from 'spam@external.tld' \
     --body 'XJS*C4JDBQADN1.NSBN3*2IDNEN*GTUBE-STANDARD-ANTI-UBE-TEST-EMAIL*C.34X'
 }