ci: fix ShellCheck linting for BATS tests (#3347)

* updated `lint.sh` to lint BATS (again)
* fix linting errors

test/linting/lint.sh

@@ -48,21 +48,24 @@ function _hadolint
 
 function _shellcheck
 {
+  local F_SH F_BIN F_BATS
+
   # File paths for shellcheck:
-  F_SH=$(find . -type f -iname '*.sh' \
+  readarray -d '' F_SH < <(find . -type f -iname '*.sh' \
     -not -path './test/bats/*' \
     -not -path './test/test_helper/*' \
-    -not -path './.git/*'
+    -not -path './.git/*' \
+    -print0 \
   )
   # shellcheck disable=SC2248
-  F_BIN=$(find 'target/bin' -type f -not -name '*.py')
-  F_BATS=$(find 'test' -maxdepth 1 -type f -iname '*.bats')
+  readarray -d '' F_BIN < <(find 'target/bin' -type f -not -name '*.py' -print0)
+  readarray -d '' F_BATS < <(find 'test/tests/' -type f -iname '*.bats' -print0)
 
   # This command is a bit easier to grok as multi-line.
   # There is a `.shellcheckrc` file, but it's only supports half of the options below, thus kept as CLI:
   # `SCRIPTDIR` is a special value that represents the path of the script being linted,
   # all sourced scripts share the same SCRIPTDIR source-path of the original script being linted.
-  CMD_SHELLCHECK=(shellcheck
+  local CMD_SHELLCHECK=(shellcheck
     --external-sources
     --check-sourced
     --severity=style
@@ -74,7 +77,13 @@ function _shellcheck
     --exclude=SC2311
     --exclude=SC2312
     --source-path=SCRIPTDIR
-    "${F_SH} ${F_BIN} ${F_BATS}"
+  )
+
+  local BATS_EXTRA_ARGS=(
+    --exclude=SC2030
+    --exclude=SC2031
+    --exclude=SC2034
+    --exclude=SC2155
   )
 
   # The linter can reference additional source-path values declared in scripts,
@@ -87,11 +96,22 @@ function _shellcheck
   # Otherwise it only applies to the line below it. You can declare multiple source-paths, they don't override the previous.
   # `source=relative/path/to/file.sh` will check the source value in each source-path as well.
   # shellcheck disable=SC2068
-  if docker run --rm --tty \
+  local ERROR=0
+
+  docker run --rm --tty \
     --volume "${REPOSITORY_ROOT}:/ci:ro" \
     --workdir "/ci" \
     --name dms-test_shellcheck \
-    "koalaman/shellcheck-alpine:v${SHELLCHECK_VERSION}" ${CMD_SHELLCHECK[@]}
+    "koalaman/shellcheck-alpine:v${SHELLCHECK_VERSION}" "${CMD_SHELLCHECK[@]}" "${F_SH[@]}" "${F_BIN[@]}" || ERROR=1
+
+  docker run --rm --tty \
+    --volume "${REPOSITORY_ROOT}:/ci:ro" \
+    --workdir "/ci" \
+    --name dms-test_shellcheck \
+    "koalaman/shellcheck-alpine:v${SHELLCHECK_VERSION}" "${CMD_SHELLCHECK[@]}" \
+    "${BATS_EXTRA_ARGS[@]}" "${F_BATS[@]}" || ERROR=1
+
+  if [[ ${ERROR} -eq 0 ]]
   then
     _log 'info' 'ShellCheck succeeded'
   else

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

@@ -61,18 +61,22 @@ function teardown_file() {
   export CONTAINER_NAME=${CONTAINER1_NAME}
 
   local AMAVIS_DEFAULTS_FILE='/etc/amavis/conf.d/20-debian_defaults'
+  # shellcheck disable=SC2016
   _run_in_container grep '\$sa_tag_level_deflt' "${AMAVIS_DEFAULTS_FILE}"
   assert_success
   assert_output --partial '= 2.0'
 
+  # shellcheck disable=SC2016
   _run_in_container grep '\$sa_tag2_level_deflt' "${AMAVIS_DEFAULTS_FILE}"
   assert_success
   assert_output --partial '= 6.31'
 
+  # shellcheck disable=SC2016
   _run_in_container grep '\$sa_kill_level_deflt' "${AMAVIS_DEFAULTS_FILE}"
   assert_success
   assert_output --partial '= 10.0'
 
+  # shellcheck disable=SC2016
   _run_in_container grep '\$sa_spam_subject_tag' "${AMAVIS_DEFAULTS_FILE}"
   assert_success
   assert_output --partial "= '***SPAM*** ';"

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

@@ -132,5 +132,5 @@ function _should_have_log_entry() {
 
 # `lines` is a special BATS variable updated via `run`:
 function _should_output_number_of_lines() {
-  assert_equal "${#lines[@]}" $1
+  assert_equal "${#lines[@]}" "${1}"
 }

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

@@ -6,7 +6,7 @@ CONTAINER1_NAME='dms-test_postscreen_enforce'
 CONTAINER2_NAME='dms-test_postscreen_sender'
 
 function setup() {
-  CONTAINER1_IP=$(_get_container_ip ${CONTAINER1_NAME})
+  CONTAINER1_IP=$(_get_container_ip "${CONTAINER1_NAME}")
 }
 
 function setup_file() {
@@ -70,6 +70,7 @@ function _should_wait_turn_speaking_smtp() {
   local SMTP_TEMPLATE=$3
   local EXPECTED=$4
 
+  # shellcheck disable=SC2016
   local UGLY_WORKAROUND='exec 3<>/dev/tcp/'"${TARGET_CONTAINER_IP}"'/25 && \
     while IFS= read -r cmd; do \
       head -1 <&3; \

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

@@ -64,7 +64,7 @@ function teardown_file() { _default_teardown ; }
   assert_output --partial "Finished DKIM key creation"
   _run_in_container_bash "[[ -f ${SIGNING_CONF_FILE} ]]"
   assert_success
-  _exec_in_container_bash "echo "blabla" >${SIGNING_CONF_FILE}"
+  _exec_in_container_bash "echo 'blabla' >${SIGNING_CONF_FILE}"
   local INITIAL_SHA512_SUM=$(_exec_in_container sha512sum "${SIGNING_CONF_FILE}")
 
   __create_key

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

@@ -58,6 +58,7 @@ function setup_file() {
 function teardown_file() { _default_teardown ; }
 
 @test "Postfix's main.cf was adjusted" {
+  # shellcheck disable=SC2016
   _run_in_container grep -F 'smtpd_milters = $rspamd_milter' /etc/postfix/main.cf
   assert_success
   _run_in_container postconf rspamd_milter
@@ -171,6 +172,7 @@ function teardown_file() { _default_teardown ; }
   MODULE_PATH='/etc/rspamd/override.d/testmodule4.something'
   _run_in_container_bash "[[ -f ${MODULE_PATH} ]]"
   assert_success
+  # shellcheck disable=SC2016
   _run_in_container grep -F 'some very long line with "weird $charact"ers' "${MODULE_PATH}"
   assert_success
   _run_in_container grep -F 'and! ano. ther &line' "${MODULE_PATH}"

test/tests/parallel/set3/container_configuration/hostname.bats

@@ -193,6 +193,7 @@ function _should_be_configured_to_fqdn() {
   assert_success
 
   # Amavis
+  # shellcheck disable=SC2016
   _run_in_container grep '^\$myhostname' /etc/amavis/conf.d/05-node_id
   assert_output "\$myhostname = \"${EXPECTED_FQDN}\";"
   assert_success

test/tests/parallel/set3/container_configuration/process_check_restart.bats

@@ -179,12 +179,12 @@ function _should_restart_when_killed() {
 function _check_if_process_is_running() {
   local PROCESS=${1}
   local MIN_SECS_RUNNING
-  [[ -n ${2} ]] && MIN_SECS_RUNNING="--older ${2}"
+  [[ -n ${2:-} ]] && MIN_SECS_RUNNING=('--older' "${2}")
 
-  local IS_RUNNING=$(docker exec "${CONTAINER_NAME}" pgrep --list-full ${MIN_SECS_RUNNING} "${PROCESS}")
+  local IS_RUNNING=$(docker exec "${CONTAINER_NAME}" pgrep --list-full "${MIN_SECS_RUNNING[@]}" "${PROCESS}")
 
   # When no matches are found, nothing is returned. Provide something we can assert on (helpful for debugging):
-  if [[ ! ${IS_RUNNING} =~ "${PROCESS}" ]]
+  if [[ ! ${IS_RUNNING} =~ ${PROCESS} ]]
   then
     echo "'${PROCESS}' is not running"
     return 1

test/tests/parallel/set3/scripts/helper_functions.bats

@@ -4,6 +4,7 @@ BATS_TEST_NAME_PREFIX='[Scripts] (helper functions) '
 SOURCE_BASE_PATH="${REPOSITORY_ROOT:?Expected REPOSITORY_ROOT to be set}/target/scripts/helpers"
 
 @test '(network.sh) _sanitize_ipv4_to_subnet_cidr' {
+  # shellcheck source=../../../../../target/scripts/helpers/network.sh
   source "${SOURCE_BASE_PATH}/network.sh"
 
   run _sanitize_ipv4_to_subnet_cidr '255.255.255.255/0'
@@ -17,7 +18,9 @@ SOURCE_BASE_PATH="${REPOSITORY_ROOT:?Expected REPOSITORY_ROOT to be set}/target/
 }
 
 @test '(utils.sh) _env_var_expect_zero_or_one' {
+  # shellcheck source=../../../../../target/scripts/helpers/log.sh
   source "${SOURCE_BASE_PATH}/log.sh"
+  # shellcheck source=../../../../../target/scripts/helpers/utils.sh
   source "${SOURCE_BASE_PATH}/utils.sh"
 
   ZERO=0
@@ -40,7 +43,9 @@ SOURCE_BASE_PATH="${REPOSITORY_ROOT:?Expected REPOSITORY_ROOT to be set}/target/
 }
 
 @test '(utils.sh) _env_var_expect_integer' {
+  # shellcheck source=../../../../../target/scripts/helpers/log.sh
   source "${SOURCE_BASE_PATH}/log.sh"
+  # shellcheck source=../../../../../target/scripts/helpers/utils.sh
   source "${SOURCE_BASE_PATH}/utils.sh"
 
   INTEGER=1234

test/tests/serial/test_helper.bats

@@ -1,3 +1,5 @@
+# shellcheck disable=SC2314,SC2317
+
 load "${REPOSITORY_ROOT}/test/test_helper/common"
 
 BATS_TEST_NAME_PREFIX='test helper functions:'

test/tests/serial/tests.bats

@@ -164,6 +164,7 @@ function teardown_file() { _default_teardown ; }
 }
 
 @test "amavis: old virusmail is wipped by cron" {
+  # shellcheck disable=SC2016
   _exec_in_container_bash 'touch -d "`date --date=2000-01-01`" /var/lib/amavis/virusmails/should-be-deleted'
   _run_in_container_bash '/usr/local/bin/virus-wiper'
   assert_success
@@ -172,6 +173,7 @@ function teardown_file() { _default_teardown ; }
 }
 
 @test "amavis: recent virusmail is not wipped by cron" {
+  # shellcheck disable=SC2016
   _exec_in_container_bash 'touch -d "`date`"  /var/lib/amavis/virusmails/should-not-be-deleted'
   _run_in_container_bash '/usr/local/bin/virus-wiper'
   assert_success