tests(refactor): Extract mail account management tests from tests.bats (#3055)

* chore: Extract out accounts test cases from `tests.bats`

Standard test file format, the test cases have been copied over unmodified.

* chore: Revise test case descriptions

* tests(refactor): `accounts.bats`

Revised test cases:
- Some common test case logic extracted to test methods.
- Update direct user management commands to use the `setup email ...` variants.
- Improved assertions.
- Removed `sleep 2` lines as the need for that is ambiguous (may no longer be relevant?)
- Additional commentary for maintaining
- Two test cases for missing `postfix-accounts.cf` opted to just run the image without any volumes instead, as the `without-accounts/` folder  was empty anyway. 2nd test case can instead use a single `docker run` to check  the newly created`postfix-accounts.cf` content.
- `test/config/without-accounts/` remains as `open_dkim.bats` presently uses it.

* chore: Remove unnecessary account removal assert

Traced this back to the original PR where it appears to have been a typo and was probably intended as a cleanup on the `user4` account. Not necessary, removing.

* chore: Rename `accounts.bat` -> `account_management.bats`

---------

* feedback: Avoid `ls` for detecting directories

Replace `ls -d` approach from original test cases

Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com>

* feedback: Remove asserting empty output on failure

Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com>

test/helper/common.bash

@@ -353,6 +353,9 @@ function _add_mail_account_then_wait_until_ready() {
   local MAIL_PASS="${2:-password_not_relevant_to_test}"
   local CONTAINER_NAME=$(__handle_container_name "${3:-}")
 
+  # Required to detect a new account and create the maildir:
+  _wait_for_service changedetector "${CONTAINER_NAME}"
+
   _run_in_container setup email add "${MAIL_ACCOUNT}" "${MAIL_PASS}"
   assert_success
 

test/tests/parallel/set3/mta/account_management.bats

@@ -0,0 +1,161 @@
+load "${REPOSITORY_ROOT}/test/helper/common"
+load "${REPOSITORY_ROOT}/test/helper/setup"
+
+BATS_TEST_NAME_PREFIX='[Mail Accounts] '
+CONTAINER_NAME='dms-test_accounts'
+
+function setup_file() {
+  _init_with_defaults
+  _common_container_setup
+
+  # Testing account added after start-up is also working correctly:
+  _add_mail_account_then_wait_until_ready 'added@localhost.localdomain' 'mypassword'
+  # Testing can create an account with potentially problematic input:
+  _add_mail_account_then_wait_until_ready 'pass@localhost.localdomain' 'may be \a `p^a.*ssword'
+}
+
+function teardown_file() { _default_teardown ; }
+
+### Account Setup ###
+
+@test "should have created all accounts in Dovecot UserDB" {
+  _run_in_container doveadm user '*'
+  assert_success
+  assert_line --index 0 'user1@localhost.localdomain'
+  assert_line --index 1 'user2@otherdomain.tld'
+  assert_line --index 2 'user3@localhost.localdomain'
+  assert_line --index 3 'added@localhost.localdomain'
+  assert_line --index 4 'pass@localhost.localdomain'
+  assert_line --index 5 'alias1@localhost.localdomain'
+  # TODO: Probably not intentional?:
+  assert_line --index 6 '@localdomain2.com'
+  _should_output_number_of_lines 7
+
+  # Relevant log output from scripts/helpers/accounts.sh:_create_dovecot_alias_dummy_accounts():
+  # [  DEBUG  ]  Adding alias 'alias1@localhost.localdomain' for user 'user1@localhost.localdomain' to Dovecot's userdb
+  # [  DEBUG  ]  Alias 'alias2@localhost.localdomain' is non-local (or mapped to a non-existing account) and will not be added to Dovecot's userdb
+  # [  DEBUG  ]  Adding alias '@localdomain2.com' for user 'user1@localhost.localdomain' to Dovecot's userdb
+}
+
+@test "should have created maildir for 'user1@localhost.localdomain'" {
+  _run_in_container_bash '[[ -d /var/mail/localhost.localdomain/user1 ]]'
+  assert_success
+}
+
+@test "should have created maildir for 'user2@otherdomain.tld'" {
+  _run_in_container_bash '[[ -d /var/mail/otherdomain.tld/user2 ]]'
+  assert_success
+}
+
+@test "should have created maildir for 'user3@localhost.localdomain'" {
+  _run_in_container_bash '[[ -d /var/mail/localhost.localdomain/user3 ]]'
+  assert_success
+}
+
+@test "should have created maildir for 'added@localhost.localdomain'" {
+  _run_in_container_bash '[[ -d /var/mail/localhost.localdomain/added ]]'
+  assert_success
+}
+
+@test "should not accidentally parse comments in 'postfix-accounts.cf' as accounts" {
+  _run_in_container find /var/mail -maxdepth 1
+  assert_success
+  refute_output --partial 'comment'
+}
+
+### Account Management ###
+
+@test "should fail to create a user when the domain-part ('@example.com') is missing" {
+  _run_in_container setup email add user_without_domain mypassword
+  assert_failure
+  assert_output --partial 'should include the domain (eg: user@example.com)'
+}
+
+@test "should add new user 'user3@domain.tld' into 'postfix-accounts.cf'" {
+  __should_add_new_user 'user3@domain.tld'
+}
+
+# To catch mistakes from substring matching:
+@test "should add new user 'auser3@domain.tld' into 'postfix-accounts.cf'" {
+  __should_add_new_user 'auser3@domain.tld'
+}
+
+# To catch mistakes from accidental pattern `.` matching as `u`:
+@test "should add new user 'a.ser3@domain.tld' into 'postfix-accounts.cf'" {
+  __should_add_new_user 'a.ser3@domain.tld'
+}
+
+@test "should remove user3 (but not auser3) from 'postfix-accounts.cf'" {
+  # Waits until change event has created directory but not completed:
+  _wait_until_account_maildir_exists 'user3@domain.tld'
+  # Should trigger a new change event:
+  _exec_in_container setup email del -y 'user3@domain.tld'
+
+  # NOTE: This is only checking `postfix-accounts.cf`, account may still persist
+  # elsewhere momentarily such as the Dovecot UserDB until change event kicks in.
+  __check_mail_account_exists 'user3@domain.tld'
+  assert_failure
+
+  __check_mail_account_exists 'auser3@domain.tld'
+  assert_success
+  assert_output 'auser3@domain.tld'
+}
+
+@test "should update password for user4 by modifying entry in 'postfix-accounts.cf'" {
+  # This change tends to be bundled with change detection event from previous test case
+  # deleting 'user3@domain.tld', thus both changes are usually applied together.
+  # NOTE: Technically these two `setup email ...` commands are run async, there is no
+  # proper file locking applied to `postfix-accounts.cf`, potentially a race condition.
+  _add_mail_account_then_wait_until_ready 'user4@domain.tld'
+
+  local ORIGINAL_ENTRY UPDATED_ENTRY
+  ORIGINAL_ENTRY=$(_exec_in_container grep '^user4@domain\.tld' -i /tmp/docker-mailserver/postfix-accounts.cf)
+  _exec_in_container setup email update 'user4@domain.tld' mynewpassword
+  UPDATED_ENTRY=$(_exec_in_container grep '^user4@domain\.tld' -i /tmp/docker-mailserver/postfix-accounts.cf)
+
+  assert_not_equal "${ORIGINAL_ENTRY}" "${UPDATED_ENTRY}"
+}
+
+# TODO: Prone to failure sometimes from the change event in previous test case,
+# as Dovecot service can be momentarily unavailable during reload?
+@test "(ENV ENABLE_QUOTAS=0) 'setup email list' should not display quota information" {
+  _run_in_container_bash 'echo "ENABLE_QUOTAS=0" >> /etc/dms-settings && setup email list | head -n 1'
+  assert_success
+  assert_output '* user1@localhost.localdomain'
+}
+
+@test "(ENV ENABLE_QUOTAS=1) 'setup email list' should display quota information" {
+  _run_in_container_bash 'sed -i "/ENABLE_QUOTAS=0/d" /etc/dms-settings; setup email list | head -n 1'
+  assert_success
+  assert_output '* user1@localhost.localdomain ( 0 / ~ ) [0%]'
+}
+
+@test "(missing postfix-accounts.cf) 'setup email del' should not fail with an error" {
+  run docker run --rm "${IMAGE_NAME:?}" setup email del -y 'user3@domain.tld'
+  assert_success
+  assert_output ''
+}
+
+@test "(missing postfix-accounts.cf) 'setup email add' should create 'postfix-accounts.cf' and populate with new mail account" {
+  run docker run --rm "${IMAGE_NAME:?}" \
+    /bin/bash -c 'setup email add user3@domain.tld mypassword && grep -o "^user3@domain\.tld" /tmp/docker-mailserver/postfix-accounts.cf'
+  assert_success
+  assert_output 'user3@domain.tld'
+}
+
+function __should_add_new_user() {
+  local MAIL_ACCOUNT=${1}
+  _exec_in_container setup email add "${MAIL_ACCOUNT}" mypassword
+
+  __check_mail_account_exists "${MAIL_ACCOUNT}"
+  assert_success
+  assert_output "${MAIL_ACCOUNT}"
+}
+
+# Uses a double grep to avoid test case failures from accidental substring/pattern matching
+function __check_mail_account_exists() {
+  local MAIL_ACCOUNT=${1}
+
+  # Filter out any comment lines, then truncate each line at their first `|` delimiter:
+  _run_in_container_bash "sed -e '/\s*#/d' -e 's/|.*//' /tmp/docker-mailserver/postfix-accounts.cf | grep '^${MAIL_ACCOUNT}' | grep -F '${MAIL_ACCOUNT}'"
+}

test/tests/serial/tests.bats

@@ -31,7 +31,6 @@ function setup_file() {
   _common_container_setup 'CONTAINER_ARGS_ENV_CUSTOM'
 
   _add_mail_account_then_wait_until_ready 'added@localhost.localdomain' 'mypassword'
-  _add_mail_account_then_wait_until_ready 'pass@localhost.localdomain' 'may be \a `p^a.*ssword'
 
   _wait_for_service postfix
   _wait_for_smtp_port_in_container
@@ -114,44 +113,6 @@ function teardown_file() { _default_teardown ; }
   assert_success
 }
 
-#
-# accounts
-#
-
-@test "accounts: user accounts" {
-  _run_in_container doveadm user '*'
-  assert_success
-  assert_line --index 0 "user1@localhost.localdomain"
-  assert_line --index 1 "user2@otherdomain.tld"
-  assert_line --index 2 "user3@localhost.localdomain"
-  assert_line --index 3 "added@localhost.localdomain"
-}
-
-@test "accounts: user mail folder for user1" {
-  _run_in_container_bash "ls -d /var/mail/localhost.localdomain/user1"
-  assert_success
-}
-
-@test "accounts: user mail folder for user2" {
-  _run_in_container_bash "ls -d /var/mail/otherdomain.tld/user2"
-  assert_success
-}
-
-@test "accounts: user mail folder for user3" {
-  _run_in_container_bash "ls -d /var/mail/localhost.localdomain/user3"
-  assert_success
-}
-
-@test "accounts: user mail folder for added user" {
-  _run_in_container_bash "ls -d /var/mail/localhost.localdomain/added"
-  assert_success
-}
-
-@test "accounts: comments are not parsed" {
-  _run_in_container_bash "ls /var/mail | grep 'comment'"
-  assert_failure
-}
-
 #
 # postfix
 #
@@ -283,103 +244,6 @@ zip
 EOF
 }
 
-#
-# accounts
-#
-@test "accounts: user_without_domain creation should be rejected since user@domain format is required" {
-  _run_in_container_bash "addmailuser user_without_domain mypassword"
-  assert_failure
-  assert_output --partial 'should include the domain (eg: user@example.com)'
-}
-
-@test "accounts: user3 should have been added to /tmp/docker-mailserver/postfix-accounts.cf" {
-  _exec_in_container_bash "addmailuser user3@domain.tld mypassword"
-
-  _run_in_container_bash "grep '^user3@domain\.tld|' -i /tmp/docker-mailserver/postfix-accounts.cf"
-  assert_success
-  [[ -n ${output} ]]
-}
-
-@test "accounts: auser3 should have been added to /tmp/docker-mailserver/postfix-accounts.cf" {
-  _exec_in_container_bash "addmailuser auser3@domain.tld mypassword"
-
-  _run_in_container_bash "grep '^auser3@domain\.tld|' -i /tmp/docker-mailserver/postfix-accounts.cf"
-  assert_success
-  [[ -n ${output} ]]
-}
-
-@test "accounts: a.ser3 should have been added to /tmp/docker-mailserver/postfix-accounts.cf" {
-  _exec_in_container_bash "addmailuser a.ser3@domain.tld mypassword"
-
-  _run_in_container_bash "grep '^a\.ser3@domain\.tld|' -i /tmp/docker-mailserver/postfix-accounts.cf"
-  assert_success
-  [[ -n ${output} ]]
-}
-
-@test "accounts: user3 should have been removed from /tmp/docker-mailserver/postfix-accounts.cf but not auser3" {
-  _wait_until_account_maildir_exists 'user3@domain.tld'
-
-  _exec_in_container_bash "delmailuser -y user3@domain.tld"
-
-  _run_in_container_bash "grep '^user3@domain\.tld' -i /tmp/docker-mailserver/postfix-accounts.cf"
-  assert_failure
-  [[ -z ${output} ]]
-
-  _run_in_container_bash "grep '^auser3@domain\.tld' -i /tmp/docker-mailserver/postfix-accounts.cf"
-  assert_success
-  [[ -n ${output} ]]
-}
-
-@test "user updating password for user in /tmp/docker-mailserver/postfix-accounts.cf" {
-  _add_mail_account_then_wait_until_ready 'user4@domain.tld'
-
-  initialpass=$(_exec_in_container_bash "grep '^user4@domain\.tld' -i /tmp/docker-mailserver/postfix-accounts.cf")
-  sleep 2
-  _exec_in_container_bash "updatemailuser user4@domain.tld mynewpassword"
-  sleep 2
-  changepass=$(_exec_in_container_bash "grep '^user4@domain\.tld' -i /tmp/docker-mailserver/postfix-accounts.cf")
-
-  [[ ${initialpass} != "${changepass}" ]]
-
-  _run_in_container_bash "delmailuser -y auser3@domain.tld"
-  assert_success
-}
-
-@test "accounts: listmailuser (quotas disabled)" {
-  _run_in_container_bash "echo 'ENABLE_QUOTAS=0' >> /etc/dms-settings && listmailuser | head -n 1"
-  assert_success
-  assert_output '* user1@localhost.localdomain'
-}
-
-@test "accounts: listmailuser (quotas enabled)" {
-  _run_in_container_bash "sed -i '/ENABLE_QUOTAS=0/d' /etc/dms-settings; listmailuser | head -n 1"
-  assert_success
-  assert_output '* user1@localhost.localdomain ( 0 / ~ ) [0%]'
-}
-
-@test "accounts: no error is generated when deleting a user if /tmp/docker-mailserver/postfix-accounts.cf is missing" {
-  run docker run --rm \
-    -v "$(_duplicate_config_for_container without-accounts/ without-accounts-deleting-user)":/tmp/docker-mailserver/ \
-    "${IMAGE_NAME:?}" /bin/sh -c 'delmailuser -y user3@domain.tld'
-  assert_success
-  [[ -z ${output} ]]
-}
-
-@test "accounts: user3 should have been added to /tmp/docker-mailserver/postfix-accounts.cf even when that file does not exist" {
-  local PRIVATE_CONFIG
-  PRIVATE_CONFIG=$(_duplicate_config_for_container without-accounts/ without-accounts_file_does_not_exist)
-  run docker run --rm \
-    -v "${PRIVATE_CONFIG}/without-accounts/":/tmp/docker-mailserver/ \
-    "${IMAGE_NAME:?}" /bin/sh -c 'addmailuser user3@domain.tld mypassword'
-  assert_success
-  run docker run --rm \
-    -v "${PRIVATE_CONFIG}/without-accounts/":/tmp/docker-mailserver/ \
-    "${IMAGE_NAME:?}" /bin/sh -c 'grep user3@domain.tld -i /tmp/docker-mailserver/postfix-accounts.cf'
-  assert_success
-  [[ -n ${output} ]]
-}
-
-
 @test "quota: setquota user must be existing" {
   _add_mail_account_then_wait_until_ready 'quota_user@domain.tld'