From 306592fcad61c1d450e82efa99af24899d8657ef Mon Sep 17 00:00:00 2001 From: Brennan Kinney <5098581+polarathene@users.noreply.github.com> Date: Tue, 3 Jan 2023 19:00:13 +1300 Subject: [PATCH] tests: Adjusted files not directly related to tests `tls.bash` helper was adapted to the new helper scripts location. The `setup.bash` helper saw a bugfix (expanding the array properly) and updates the container default config to configure for IPv4 explicitly. The IPv4 default was added after recent Docker pushes and I saw weird IPv6 related errors in the logs.. now we're sure IPv4 is the default during tests. Added functionality to check if a process is running: - This change adds a helper function to check whether a program is running inside a container or not. - This added the need for a function like `_run_in_container` but allowing for providing an explicit container name. - Future PRs can use this helper function now to check whether a process is running or not. This was done for the tests of Fail2Ban, but can be used for other tests in the future as well. --- chore: Restructured BATS flags in `Makefile` The `Makefile` has seen a bit of a restructuring when it comes to flags: 1. The `MAKEFLAGS` variables is used by `make`, and allows for adding additional flags that can be used within in recursive calls (via `$(MAKE)`) too, thus DRY approach. 2. The flags for calling BATS were adjusted. `--no-parallelize-within-files` has been added as well to ensure tests _inside_ a single file are run sequentially. `dms-test` prefix matching changed to expect a `_` suffix as a delimiter. --- docs: Add a note regarding output from running tests in parallel --- Makefile | 19 ++++++++++--------- docs/content/contributing/general.md | 14 ++++++++++++-- test/helper/common.bash | 19 +++++++++++++++++++ test/helper/setup.bash | 7 +++++-- test/helper/tls.bash | 15 ++++----------- 5 files changed, 50 insertions(+), 24 deletions(-) diff --git a/Makefile b/Makefile index 1577ab7c..2367954e 100644 --- a/Makefile +++ b/Makefile @@ -1,11 +1,15 @@ SHELL := /bin/bash .SHELLFLAGS += -e -u -o pipefail -PARALLEL_JOBS ?= 2 export REPOSITORY_ROOT := $(CURDIR) export IMAGE_NAME ?= mailserver-testing:ci export NAME ?= $(IMAGE_NAME) +MAKEFLAGS += --no-print-directory +BATS_FLAGS ?= --timing +BATS_PARALLEL_JOBS ?= 2 +BATS_FLAGS_PARALLEL ?= $(BATS_FLAGS) --no-parallelize-within-files --jobs $(BATS_PARALLEL_JOBS) + .PHONY: ALWAYS_RUN # ----------------------------------------------- @@ -33,7 +37,7 @@ backup: clean: # remove test containers and restore test/config directory -@ [[ -d testconfig.bak ]] && { sudo rm -rf test/config ; mv testconfig.bak test/config ; } || : - -@ for CONTAINER in $$(docker ps -a --filter name='^dms-test-.*|^mail_.*|^hadolint$$|^eclint$$|^shellcheck$$' | sed 1d | cut -f 1-1 -d ' '); do docker rm -f $${CONTAINER}; done + -@ for CONTAINER in $$(docker ps -a --filter name='^dms-test_.*|^mail_.*|^hadolint$$|^eclint$$|^shellcheck$$' | sed 1d | cut -f 1-1 -d ' '); do docker rm -f $${CONTAINER}; done -@ while read -r LINE; do [[ $${LINE} =~ test/.+ ]] && sudo rm -rf $${LINE}; done < .gitignore # ----------------------------------------------- @@ -43,19 +47,16 @@ clean: tests: ALWAYS_RUN # See https://github.com/docker-mailserver/docker-mailserver/pull/2857#issuecomment-1312724303 # on why `generate-accounts` is run before each set (TODO/FIXME) - @ $(MAKE) generate-accounts tests/serial - @ $(MAKE) generate-accounts tests/parallel/set1 - @ $(MAKE) generate-accounts tests/parallel/set2 - @ $(MAKE) generate-accounts tests/parallel/set3 + @ for DIR in tests/{serial,parallel/set{1,2,3}} ; do $(MAKE) generate-accounts "$${DIR}" ; done tests/serial: ALWAYS_RUN - @ shopt -s globstar ; ./test/bats/bin/bats --timing --jobs 1 test/$@/**.bats + @ shopt -s globstar ; ./test/bats/bin/bats $(BATS_FLAGS) test/$@/*.bats tests/parallel/set%: ALWAYS_RUN - @ shopt -s globstar ; ./test/bats/bin/bats --timing --jobs $(PARALLEL_JOBS) test/$@/**.bats + @ shopt -s globstar ; ./test/bats/bin/bats $(BATS_FLAGS_PARALLEL) test/$@/**/*.bats test/%: ALWAYS_RUN - @ shopt -s globstar nullglob ; ./test/bats/bin/bats --timing test/tests/**/{$*,}.bats + @ shopt -s globstar nullglob ; ./test/bats/bin/bats $(BATS_FLAGS) test/tests/**/{$*,}.bats # ----------------------------------------------- # --- Lints ------------------------------------- diff --git a/docs/content/contributing/general.md b/docs/content/contributing/general.md index 7270fcb8..d6bdcb94 100644 --- a/docs/content/contributing/general.md +++ b/docs/content/contributing/general.md @@ -24,11 +24,21 @@ To run the test suite, you will need to We do not support running linting, tests, etc on macOS at this time. Please use a linux VM. +??? tip "Setting the Degree of Parallelization for Tests" + + If your machine is capable, you can increase the amount of tests that are run simultaneously by prepending the `make clean all` command with `BATS_PARALLEL_JOBS=X` (i.e. `BATS_PARALLEL_JOBS=X make clean all`). This wil speed up the test procedure. You can also run all tests in serial by setting `BATS_PARALLEL_JOBS=1` this way. + + The default value of `BATS_PARALLEL_JOBS` is 2. Increasing it to `3` requires 6 threads and 6GB of main memory; increasing it to `4` requires 8 threads and at least 8GB of main memory. + +!!! warning "Test Output when Running in Parallel" + + When running test in parallel, BATS will run more than one test at any given time. This can result in output not being exactly what you'd expect, i.e. there could be _more_ or _less_ than you'd think. Those writing tests need to take care of this. Always test with `make clean generate-accounts tests/parallel/setX`. + ??? tip "Running a Specific Test" - To run a specific test, use `make build generate-accounts test/`, where `` is the file name of the test (_for more precision use a relative path: `test/test/`_) excluding the `.bats` suffix. + To run a specific test, use `make build generate-accounts test/`, where `` is the file name of the test (_for more precision use a relative path: `test/test/`_) **excluding** the `.bats` suffix. - To run only the tests in `template.bats`, use `make test/template` (or `make test/parallel/set2/template`). + Example: To run only the tests in `template.bats`, use `make test/template` (or `make test/parallel/set2/template`). [Install Docker]: https://docs.docker.com/get-docker/ diff --git a/test/helper/common.bash b/test/helper/common.bash index 6fc553f2..1f38b132 100644 --- a/test/helper/common.bash +++ b/test/helper/common.bash @@ -9,16 +9,35 @@ __load_bats_helper # ------------------------------------------------------------------- +# like _run_in_container_explicit but infers ${1} by using the ENV CONTAINER_NAME function _run_in_container() { run docker exec "${CONTAINER_NAME}" "${@}" } +# @param ${1} container name [REQUIRED] +# @param ... command to execute +function _run_in_container_explicit() { + local CONTAINER_NAME=${1:?Container name must be given when using explicit} + shift 1 + run docker exec "${CONTAINER_NAME}" "${@}" +} + function _default_teardown() { docker rm -f "${CONTAINER_NAME}" } # ------------------------------------------------------------------- +# @param ${1} program name [REQUIRED] +# @param ${2} container name [IF UNSET: ${CONTAINER_NAME}] +function _check_if_process_is_running() { + local PROGRAM_NAME=${1:?Program name must be provided explicitly} + local CONTAINER_NAME=${2:-${CONTAINER_NAME}} + _run_in_container_explicit "${CONTAINER_NAME}" pgrep "${PROGRAM_NAME}" +} + +# ------------------------------------------------------------------- + # @param ${1} timeout # @param --fatal-test additional test whose failure aborts immediately # @param ... test to run diff --git a/test/helper/setup.bash b/test/helper/setup.bash index 522cfc84..4bab5925 100644 --- a/test/helper/setup.bash +++ b/test/helper/setup.bash @@ -17,14 +17,15 @@ function __initialize_variables() { 'CONTAINER_NAME' ) - for VARIABLE in "${REQUIRED_VARIABLES_FOR_TESTS}" + for VARIABLE in "${REQUIRED_VARIABLES_FOR_TESTS[@]}" do __check_if_set "${VARIABLE}" done + export SETUP_FILE_MARKER TEST_TIMEOUT_IN_SECONDS NUMBER_OF_LOG_LINES + SETUP_FILE_MARKER="${BATS_TMPDIR:?}/$(basename "${BATS_TEST_FILENAME:?}").setup_file" TEST_TIMEOUT_IN_SECONDS=${TEST_TIMEOUT_IN_SECONDS:-120} NUMBER_OF_LOG_LINES=${NUMBER_OF_LOG_LINES:-10} - SETUP_FILE_MARKER="${BATS_TMPDIR:?}/$(basename "${BATS_TEST_FILENAME:?}").setup_file" } # ------------------------------------------------------------------- @@ -118,6 +119,8 @@ function common_container_create() { --env ENABLE_UPDATE_CHECK=0 \ --env ENABLE_SPAMASSASSIN=0 \ --env ENABLE_FAIL2BAN=0 \ + --env POSTFIX_INET_PROTOCOLS=ipv4 \ + --env DOVECOT_INET_PROTOCOLS=ipv4 \ --env LOG_LEVEL=debug \ "${X_EXTRA_ARGS[@]}" \ "${IMAGE_NAME}" diff --git a/test/helper/tls.bash b/test/helper/tls.bash index 4987a66f..bb107a4f 100644 --- a/test/helper/tls.bash +++ b/test/helper/tls.bash @@ -1,21 +1,17 @@ #!/bin/bash -load "${REPOSITORY_ROOT}/test/test_helper/common" +load "${REPOSITORY_ROOT}/test/helper/common" -# Helper methods for testing TLS. # `_should_*` methods are useful for common high-level functionality. - # ? --------------------------------------------- Negotiate TLS - # For certs actually provisioned from LetsEncrypt the Root CA cert should not need to be provided, # as it would already be available by default in `/etc/ssl/certs`, requiring only the cert chain (fullchain.pem). function _should_succesfully_negotiate_tls() { local FQDN=${1} - local CONTAINER_NAME=${2:-${TEST_NAME}} # shellcheck disable=SC2031 - local CA_CERT=${3:-${TEST_CA_CERT}} + local CA_CERT=${2:-${TEST_CA_CERT}} # Postfix and Dovecot are ready: wait_for_smtp_port_in_container_to_respond "${CONTAINER_NAME}" @@ -36,9 +32,8 @@ function _should_succesfully_negotiate_tls() { function _negotiate_tls() { local FQDN=${1} local PORT=${2} - local CONTAINER_NAME=${3:-${TEST_NAME}} # shellcheck disable=SC2031 - local CA_CERT=${4:-${TEST_CA_CERT}} + local CA_CERT=${3:-${TEST_CA_CERT}} local CMD_OPENSSL_VERIFY CMD_OPENSSL_VERIFY=$(_generate_openssl_cmd "${PORT}") @@ -83,7 +78,6 @@ function _generate_openssl_cmd() { echo "${CMD_OPENSSL} ${EXTRA_ARGS} 2>/dev/null" } - # ? --------------------------------------------- Verify FQDN function _get_fqdn_match_query() { @@ -115,9 +109,8 @@ function escape_fqdn() { function _get_fqdns_for_cert() { local FQDN=${1} local PORT=${2:-'25'} - local CONTAINER_NAME=${3:-${TEST_NAME}} # shellcheck disable=SC2031 - local CA_CERT=${4:-${TEST_CA_CERT}} + local CA_CERT=${3:-${TEST_CA_CERT}} # `-servername` is for SNI, where the port may be for a service that serves multiple certs, # and needs a specific FQDN to return the correct cert. Such as a reverse-proxy.