sstephenson/bats

Support for "unofficial strict mode"?

felixSchl opened this issue · 3 comments

Hey, I am seeing a strange behaviour when using load or directly sourceing files that set the unofficial strict mode. The behaviour I am seeing is that my test stops failing:

test.bats:

load test_helper
@test "foo" {
    exit 1
}

test_helper.bash:

# !/bin/bash
set -euo pipefail
IFS=$'\n\t'

Result:

1 test, 0 failures

shell returned 1

What am I doing wrong?

@felixSchl You're not doing anything wrong. This is a bug. It's not load or source that causes the problem, however. The set -u in your test file breaks Bats. This causes the incomplete and inaccurate results.

I'll bring it up in the discussion about upcoming versions. This is a serious bug that should get high priority. Bats suffers from a lack of manpower so I have no idea when this may be fixed. Until then check out the workaround below.

TL;DR

This is a bug where a test file, i.e. arbitrary user supplied code, has an unintended effect on the execution of Bats.

Bats sources preprocessed test files in order to execute the test cases within. This means that shell options and variables set in the test file can inadvertently, or maliciously, influence the execution of Bats.

In this case, the set -u has the unintended effect of breaking bats-exec-test when it attempts to expand unset variables.

Protecting against such errors are notoriously hard, but should be high priority for future releases.

Details

set -u in a test file causes errors in bats-exec-test when it attempts to expand unset variables.

-u

Treat unset variables and parameters other than the special parameters "@" and "*" as an error when performing parameter expansion. If expansion is attempted on an unset variable or parameter, the shell prints an error message, and, if not interactive, exits with a non-zero status.

The errors can be seen in the test's .out file (/tmp/bats.<PID>.out by default).

/usr/lib/bats/bats-exec-test: line 96: BATS_CURRENT_STACK_TRACE[@]: unbound variable
/usr/lib/bats/bats-exec-test: line 248: BATS_TEST_SKIPPED: unbound variable

The failing test triggers capturing of the stack trace where the first error occours. Since set -e has been set previously, this triggers bats-exec-test to exit after executing its exit trap where the second error occours.

-e

Exit immediately if a pipeline (which may consist of a single simple command), a list, or a compound command (see SHELL GRAMMAR above), exits with a non-zero status. ...

The failing bats-exec-test process causes premature termination, and incomplete and inaccurate output and status. In fact, no further tests are ran from the current test file.

Example

fail.bats:

#!/usr/bin/env bats

set -u

@test '1: failing' {
  exit 1
}

@test '2: never executed' {
  touch 'this-will-never-appear.file'
}

pass.bats:

#!/usr/bin/env bats

set -eo pipefail
IFS=$'\n\t'

@test '3: passing' {
  true
}

Output:

$ bats fail.bats pass.bats 
  ✓ 3: passing

3 tests, 0 failures
$ echo $?
0

Workaround

One solution is to isolate the effect of set -u, and similar commands, from Bats by putting the "offending" code into a separate file and running it in a separate process.

The example below puts the problematic code into fixtures/isolated/set-u.bash.

test.bats:

#!/usr/bin/env bats

load 'test_helper'
fixtures 'isolated'

@test 'set -u does not mess up Bats' {
  run "${TEST_FIXTURE_ROOT}/set-u.bash"
  echo "$output"
  [ "$status" -eq 1 ]
  [[ $output == *'UNSET_VARIABLE: unbound variable' ]] || false 
}

@test 'second test runs' {
  true
}

fixtures/isolated/set-u.bash:

#!/usr/bin/env bash

set -u

echo "$UNSET_VARIABLE"

test_helper.bash:

# Set variables for easily accessing sub-directories of `./fixtures'.
#
# Globals:
#   BATS_TEST_DIRNAME
#   TEST_FIXTURE_ROOT
#   TEST_RELATIVE_FIXTURE_ROOT
# Arguments:
#   $1 - name of sub-directory
# Returns:
#   none
fixtures() {
  TEST_FIXTURE_ROOT="${BATS_TEST_DIRNAME}/fixtures/$1"
  TEST_RELATIVE_FIXTURE_ROOT="$(bats_trim_filename "${TEST_FIXTURE_ROOT}")"
}

Output:

$ bats test.bats 
  ✓ set -u does not mess up Bats
  ✓ second test runs OK

2 tests, 0 failures
$ echo $?
0

Thank you for the detailed explanation. This could sound silly, but could bats itself not work under set -u? I know this only fixes this particular issue and not the isolation in general.

If we wanted to go down that route, then all variables would need to be checked using [[ -z "$VAR" ]]. Another gotcha is that the empty array () will also cause an unbound variable error when accessed using ${VAR[@]}, so we'd need to check the length first [ ${#VAR[@]} -eq 0 ]`...

@felixSchl That's not silly at all. The solution is likely to be along those lines. I haven't looked into it any further than that, but this seems a plausible solution.

Bats must source test files, so complete isolation will never be possible. But it can try to cope as best as Bash allows it. This and similar issues will trip up even novice users and should be addressed in coming releases.

Regardless, I think using set -u in general is a good idea. You can open a pull request with your solution if you like.