From 1b98de486232b6e36872d4fb9c50f3b41d913892 Mon Sep 17 00:00:00 2001 From: Brennan Kinney <5098581+polarathene@users.noreply.github.com> Date: Sun, 21 Jan 2024 13:33:12 +1300 Subject: [PATCH] Apply suggestions from code review --- CHANGELOG.md | 4 ++-- test/helper/log_and_filtering.bash | 18 +++--------------- test/helper/sending.bash | 2 +- .../parallel/set1/spam_virus/postscreen.bats | 4 ++-- .../parallel/set1/spam_virus/rspamd_full.bats | 4 ++-- test/tests/serial/mail_pop3.bats | 2 +- test/tests/serial/tests.bats | 7 ++++--- test/tests/serial/vmail-id.bats | 6 +++--- 8 files changed, 18 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 56dea2dc..6bcd47d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,8 +8,8 @@ All notable changes to this project will be documented in this file. The format ### Updates -- **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 +- **Tests:** + - 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) diff --git a/test/helper/log_and_filtering.bash b/test/helper/log_and_filtering.bash index de5534b0..e4ac7390 100644 --- a/test/helper/log_and_filtering.bash +++ b/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. -# In most cases, you will rather want to filter the log with +# 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 +# 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 diff --git a/test/helper/sending.bash b/test/helper/sending.bash index 962c5fe4..1c5d844a 100644 --- a/test/helper/sending.bash +++ b/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. diff --git a/test/tests/parallel/set1/spam_virus/postscreen.bats b/test/tests/parallel/set1/spam_virus/postscreen.bats index d1c308a7..3f93fe9d 100644 --- a/test/tests/parallel/set1/spam_virus/postscreen.bats +++ b/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' - # _print_mail_log_for_msgid 'postscreen' + # CONTAINER_NAME=${CONTAINER2_NAME} _send_email_with_msgid 'msgid-postscreen' --server "${CONTAINER1_IP}" --data 'postscreen.txt' + # _print_mail_log_for_msgid 'msgid-postscreen' # assert_output --partial "stored mail into mailbox 'INBOX'" _service_log_should_contain_string 'mail' 'PASS NEW' diff --git a/test/tests/parallel/set1/spam_virus/rspamd_full.bats b/test/tests/parallel/set1/spam_virus/rspamd_full.bats index 3dfb30f6..e039eb56 100644 --- a/test/tests/parallel/set1/spam_virus/rspamd_full.bats +++ b/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 diff --git a/test/tests/serial/mail_pop3.bats b/test/tests/serial/mail_pop3.bats index 888aaa50..95ae14aa 100644 --- a/test/tests/serial/mail_pop3.bats +++ b/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' { diff --git a/test/tests/serial/tests.bats b/test/tests/serial/tests.bats index 5fb88556..dcc1bb42 100644 --- a/test/tests/serial/tests.bats +++ b/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' '\(!\)connect' + _service_log_should_not_contain_string 'mail' 'Permission denied' + _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' } diff --git a/test/tests/serial/vmail-id.bats b/test/tests/serial/vmail-id.bats index aba2163b..a0fb8537 100644 --- a/test/tests/serial/vmail-id.bats +++ b/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