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

set -o nounset make exit trap fail. #88

Open
choffee opened this issue Jan 22, 2015 · 5 comments
Open

set -o nounset make exit trap fail. #88

choffee opened this issue Jan 22, 2015 · 5 comments

Comments

@choffee
Copy link

choffee commented Jan 22, 2015

I am sourcing my functions in the test as part of setup so:

setup() {
. ./my_functs
}

I like to run my bash scripts with "set -o nounset" but get this error on exit.

/usr/lib/bats/bats-exec-test: line 246: BATS_TEST_SKIPPED: unbound variable

Is this a bug or am I just doing it wrong?!

@cjwelborn
Copy link

I guess if no tests have been skipped then $BATS_TEST_SKIPPED will never be set, which is exactly what set -o nounset is for. When sourcing a file (instead of running it), it affects the current environment (using cd in a sourced file actually changes the terminal directory, but running with ./ only changes the subshell). Anyway, setting -u globally in your my_functs file will trigger this and prevent you from running all of the tests, but I found that using set -u at the top of functions will allow all of the tests to run (any test with unset variables will fail).

In my_functs:

# Don't `set -u` here, it will trigger the error.

function my_code {
    # It's okay to do it here:
    set -u
    # This will cause a test failure.
    x="${nonexistent}"
    echo "pass?"
}

In the bats file:

setup() {
    . ./my_functs
}
@test "Test sourced code" {
    [ "$(my_code)" == "pass?" ]
}

..and the output:

 ✗ Test sourced code
   (in test file my_tests.bats, line 10)
     `[ "$(my_code)" == "pass?" ]' failed
   my_functs: line 5: nonexistent: unbound variable

..pay no attention to those line numbers. I had other stuff in those files so they don't match my example's line numbers.

@netj
Copy link

netj commented May 7, 2015

It seems like bats-exec-test should explicitly initialize all variables it uses, e.g., by adding a line BATS_TEST_SKIPPED= somewhere close to the beginning, to play nice with set -u.

textarcana pushed a commit to textarcana/bats that referenced this issue Jun 20, 2018
Fix for: set -o nounset make exit trap fail. · Issue sstephenson#88 · sstephenson/bats
sstephenson#88
@textarcana
Copy link

I can confirm that just adding export BATS_TEST_SKIPPED= at the top of bats-exec-test fixes the problem.

I've made a fork with the line added, here: https://github.com/textarcana/bats/blob/eeeae6f35cd4213dac88de0b3c3132f7ce7ba355/libexec/bats-exec-test#L6

KevCui added a commit to KevCui/bahn-cli that referenced this issue Jun 25, 2019
Why:
More restrict and fail-safe

See: [Best Practices for Writing Bash Scripts](http://kvz.io/blog/2013/11/21/bash-best-practices/)

How:
set -e/set -o errexit: script exit when a command fails
set -u/set -o nounset: script exit when undeclared variable is used

`set -u` causes bats test running failed, becasue BATS_TEST_SKIPPED is unbound variable.
Ignore BATS_TEST_SKIPPED in bahn.bats
See: [set -o nounset make exit trap fail.](sstephenson/bats#88)
yarikoptic pushed a commit to neurodebian/bats that referenced this issue Aug 6, 2019
yarikoptic pushed a commit to neurodebian/bats that referenced this issue Aug 6, 2019
Closes sstephenson#90. Closes sstephenson#83. Closes sstephenson#56. Closes sstephenson#55. Closes sstephenson#53. Resolves
outstanding issues from sstephenson#32.

Per all these earlier issues and PRs:

- The original bin/bats symlink was causing problems on Windows.
- The attempted fix from sstephenson#32 created problems for install.sh.
- Per sstephenson#90, changing bin/bats to a one-line script in sstephenson#88 broke some Bats
  installations that rely on symlink schemes (such as when installed via
  https://github.com/basherpm/basher).

For an idea of why I wanted to keep bin/bats as a script due to how
symlinks complicate things on Windows, see the following results I
discovered during a search for "git symlink windows":

- https://github.com/git-for-windows/git/wiki/Symbolic-Links
- https://blogs.windows.com/buildingapps/2016/12/02/symlinks-windows-10/
- libgit2/libgit2#4107
- https://stackoverflow.com/q/5917249

In the course of applying these changes, I realized libexec/bats
computed some extraneous variables, so I removed them, eliminating a few
external processes and subshells. I also cleaned up other small bits of
logic.

On top of making install.sh, bin/bats, and libexec/bats more resilient
and compact, the existing test suite (before adding the new
test/installer.bats file) sped up significantly, running 0.6s-0.7s
faster on my MacBook Pro (2.9 GHz Intel Core i5, 8GB RAM):

  Bash 3.2.57(1)-release before:
  58 tests, 0 failures

  real    0m4.924s
  user    0m3.045s
  sys     0m1.798s

  Bash 3.2.57(1)-release after:
  58 tests, 0 failures

  real    0m4.341s
  user    0m2.808s
  sys     0m1.540s

  Bash 4.4.23(1)-release before:
  58 tests, 0 failures

  real    0m5.228s
  user    0m3.046s
  sys     0m1.952s

  Bash 4.4.23(1)-release after:
  58 tests, 0 failures

  real    0m4.582s
  user    0m2.791s
  sys     0m1.643s

Also tweaks the Dockerfile to update the symlink to point to bin/bats,
not libexec/bats.
yarikoptic pushed a commit to neurodebian/bats that referenced this issue Aug 6, 2019
While running my mbland/go-script-bash tests with v1.0.0 (which I
should've tried _before_ v1.0.0), I noticed some failed due to the last
line of output missing. This happened because the `while` loop used to
replace `sed` in sstephenson#88 would break the loop when the last line didn't end
with a newline.

As it turns out, the fix was pretty easy, thanks to the hints from:

- https://stackoverflow.com/a/14547230
- https://unix.stackexchange.com/a/418067

However, there was a slight performance hit for the existing tests
when the `[[ -n "$line" ]]` conditional appeared as part of the `while`
expression. Instead, this change emits the last line in an `if`
statement after the `while` loop.

I got my mbland/go-script-bash tests to pass by updating the code to
always emit a newline, which was the right thing to do regardless.
However, it seemed still worth handling this condition in Bats itself,
to maintain behavioral parity with v0.4.0.
@ssbarnea
Copy link

ssbarnea commented Jan 9, 2020

Any chance to fix this 4 year old bug? Testing bash scripts with strict mode is key for making the reliable and this bug prevents that.

@xmik
Copy link

xmik commented Jan 11, 2020

@ssbarnea, do you know about https://github.com/bats-core/bats-core? This bug could be fixed there.

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

6 participants