Apply suggestions from code review

CHANGELOG.md

@@ -8,8 +8,8 @@ All notable changes to this project will be documented in this file. The format
 
 ### Updates
 
-- **Tests**
+- **Tests:**
-  - further improved helpers for sending e-mails with specific `Message-ID` headers (required when filtering the mail og when multiple e-mails have been sent); associated filtering functions were adjusted or added accordingly
+  - Refactored helper methods for sending e-mails with specific `Message-ID` headers and the helpers for retrieving + filtering logs, which together help isolate logs relevant to specific mail when multiple mails have been processed within a single test.
 
 ## [v13.3.0](https://github.com/docker-mailserver/docker-mailserver/releases/tag/v13.3.0)
 

test/helper/log_and_filtering.bash

@@ -31,11 +31,11 @@ function _filter_service_log() {
   _run_in_container grep "${@}" "${STRING}" "${FILE}"
 }
 
-# Use this function to print the complete mail log, but use it only where necessary.
+# Prints the entirety of the primary mail log.
-# In most cases, you will rather want to filter the log with
+# Avoid using this method when you could filter more specific log lines with:
 #
 # 1. _filter_service_log
-# 2. _service_log_should_[not_]contain_string
+# 2. _service_log_should[_not]_contain_string
 function _show_complete_mail_log() {
   _run_in_container cat /var/log/mail/mail.log
 }
@@ -45,12 +45,6 @@ function _show_complete_mail_log() {
 # @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() {
   _filter_service_log "${@}"
   assert_success
@@ -61,12 +55,6 @@ function _service_log_should_contain_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 _service_log_should_not_contain_string() {
   _filter_service_log "${@}"
   assert_failure

test/helper/sending.bash

@@ -123,7 +123,7 @@ function _send_email_with_msgid() {
   local MSG_ID=$(__construct_msgid "${1:?The local-part for MSG_ID was not provided}")
   shift 1
 
-  _send_email "${@}" --header "Message-Id: ${MSG_ID}"
+  _send_email "${@}" --header "Message-ID: ${MSG_ID}"
 }
 
 # Send a spam e-mail by utilizing GTUBE.

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

@@ -58,8 +58,8 @@ function teardown_file() {
   # mail is sent to the DMS server container (CONTAINER1_NAME) via `--server` parameter:
   CONTAINER_NAME=${CONTAINER2_NAME} _send_email --expect-rejection --server "${CONTAINER1_IP}" --port 25 --data 'postscreen.txt'
   # TODO: Use _send_email_with_msgid when proper resolution of domain names is possible:
-  # CONTAINER_NAME=${CONTAINER2_NAME} _send_email_with_msgid 'postscreen' --server "${CONTAINER1_IP}" --data 'postscreen.txt'
+  # CONTAINER_NAME=${CONTAINER2_NAME} _send_email_with_msgid 'msgid-postscreen' --server "${CONTAINER1_IP}" --data 'postscreen.txt'
-  # _print_mail_log_for_msgid 'postscreen'
+  # _print_mail_log_for_msgid 'msgid-postscreen'
   # assert_output --partial "stored mail into mailbox 'INBOX'"
 
   _service_log_should_contain_string 'mail' 'PASS NEW'

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

@@ -124,9 +124,9 @@ function teardown_file() { _default_teardown ; }
   _print_mail_log_of_queue_id_from_msgid 'dms-test-email-spam'
   assert_output --partial 'milter-reject'
   assert_output --partial '5.7.1 Gtube pattern'
-  refute_output --partial "stored mail into mailbox 'INBOX'"
 
   _print_mail_log_for_msgid 'dms-test-email-spam'
+  refute_output --partial "stored mail into mailbox 'INBOX'"
   assert_failure
 
   _count_files_in_directory_in_container /var/mail/localhost.localdomain/user1/new/ 1
@@ -287,7 +287,7 @@ function teardown_file() { _default_teardown ; }
   _nc_wrapper 'nc/rspamd_imap_move_to_inbox.txt' '0.0.0.0 143'
   sleep 1 # wait for the transaction to finish
 
-  _service_log_should_contain_string 'mail' 'imapsieve: Matched static mailbox rule \[2\]'
+  _service_log_should_contain_string 'mail' 'imapsieve: Matched static mailbox rule [2]'
 
   _show_complete_mail_log
   for LINE in "${LEARN_HAM_LINES[@]}"; do

test/tests/serial/mail_pop3.bats

@@ -36,7 +36,7 @@ function teardown_file() { _default_teardown ; }
 # TODO: Remove in favor of a common helper method, as described in vmail-id.bats equivalent test-case
 @test 'Mail log is error free' {
   _service_log_should_not_contain_string 'mail' 'non-null host address bits in'
-  _service_log_should_not_contain_string 'mail' ': error:'
+  _service_log_should_not_contain_string 'mail' ': Error:'
 }
 
 @test '(Manage Sieve) disabled per default' {

test/tests/serial/tests.bats

@@ -182,13 +182,14 @@ function teardown_file() { _default_teardown ; }
   assert_success
 }
 
+# TODO: Remove in favor of a common helper method, as described in vmail-id.bats equivalent test-case
 @test "system: Mail log is error free" {
   _service_log_should_not_contain_string 'mail' 'non-null host address bits in'
   _service_log_should_not_contain_string 'mail' 'mail system configuration error'
-  _service_log_should_not_contain_string 'mail' ': error:'
+  _service_log_should_not_contain_string 'mail' ': Error:'
   _service_log_should_not_contain_string 'mail' 'is not writable'
-  _service_log_should_not_contain_string 'mail' 'permission denied'
+  _service_log_should_not_contain_string 'mail' 'Permission denied'
-  _service_log_should_not_contain_string 'mail' '\(!\)connect'
+  _service_log_should_not_contain_string 'mail' '(!)connect'
   _service_log_should_not_contain_string 'mail' 'using backwards-compatible default setting'
   _service_log_should_not_contain_string 'mail' 'connect to 127.0.0.1:10023: Connection refused'
 }

test/tests/serial/vmail-id.bats

@@ -48,14 +48,14 @@ function teardown_file() { _default_teardown ; }
   _service_log_should_not_contain_string 'mail' 'mail system configuration error'
 
   # Unknown error source: https://github.com/docker-mailserver/docker-mailserver/pull/85
-  _service_log_should_not_contain_string 'mail' ': error:'
+  _service_log_should_not_contain_string 'mail' ': Error:'
 
   # Unknown error source: https://github.com/docker-mailserver/docker-mailserver/pull/320
   _service_log_should_not_contain_string 'mail' 'not writable'
-  _service_log_should_not_contain_string 'mail' 'permission denied'
+  _service_log_should_not_contain_string 'mail' 'Permission denied'
 
   # Amavis: https://forum.howtoforge.com/threads/postfix-smtp-error-caused-by-clamav-cant-connect-to-a-unix-socket-var-run-clamav-clamd-ctl.81002/
-  _service_log_should_not_contain_string 'mail' '\(!\)connect'
+  _service_log_should_not_contain_string 'mail' '(!)connect'
 
   # Postfix: https://github.com/docker-mailserver/docker-mailserver/pull/2597
   # Log line match example: https://github.com/docker-mailserver/docker-mailserver/pull/2598#issuecomment-1141176633