Skip to content
This repository has been archived by the owner on Apr 29, 2021. It is now read-only.

Support for "unofficial strict mode"? #171

Open
felixSchl opened this issue May 28, 2016 · 3 comments
Open

Support for "unofficial strict mode"? #171

felixSchl opened this issue May 28, 2016 · 3 comments

Comments

@felixSchl
Copy link

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?

@ztombol
Copy link

ztombol commented May 28, 2016

@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

@felixSchl
Copy link
Author

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 ]`...

@ztombol
Copy link

ztombol commented Jun 3, 2016

@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.

@ztombol ztombol mentioned this issue Dec 13, 2016
18 tasks
mbland added a commit to bats-core/bats-core that referenced this issue Oct 7, 2017
Part of #20. Addresses sstephenson/bats#171.

Bats is now capable of executing test cases that conform to the
"unoffical Bats strict mode" described by:

- http://redsymbol.net/articles/unofficial-bash-strict-mode/

While the test fixture is straightforward, it took trial and error to
realize a few things. First, `set -u` changed in Bash 4.1-alpha. Chapter
and verse from https://tiswww.case.edu/php/chet/bash/CHANGES:

  This document details the changes between this version,
  bash-4.1-alpha, and the previous version, bash-4.0-release.

  n.  Fixed the behavior of `set -u' to conform to the latest Posix
      interpretation: every expansion of an unset variable except $@ and
      $* will cause the shell to exit.

This almost certainly accounts for the fact that referencing
`"${FOO[@]}"` when `FOO=()` is defined is OK under the latest Bash
4.4.12(1)-release, but breaks under the Bash 3.2.57(1)-release that
ships with macOS. The fix was to do an array length check before each
such operation.

It probably also accounts for the fact that under Bash 3.2.57, no error
output is generated at all for certain unset variable accesses. The
workaround there is to run the test using the latest version of Bash.

The biggest surprise was realizing that no error output is visible when
the unset variable access happens inside a DEBUG trap handler. All
versions of Bash seem susceptible to this.

As a result of these debugging subtleties, the test case contains a
thorough error message to suggest how to proceed when the fixture fails.
Stratus3D added a commit to asdf-vm/asdf that referenced this issue May 11, 2019
We need to change the BATS we use to get around this issue:
sstephenson/bats#171
Stratus3D added a commit to Stratus3D/asdf that referenced this issue May 12, 2019
We need to change the BATS we use to get around this issue:
sstephenson/bats#171
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants