bats icon indicating copy to clipboard operation
bats copied to clipboard

Write a warning in big fat letters about [[ somewhere in the readme

Open andsens opened this issue 10 years ago • 13 comments

Try this in bash:

user@machine:~$ set -e
user@machine:~$ [ 1 -eq 1 ]
user@machine:~$ [ 1 -eq 0 ] # shell exits
user@machine:~$ set -e
user@machine:~$ [[ 1 == 1 ]]
user@machine:~$ [[ 1 == 0 ]] # expected to exit, but doesnt
user@machine:~$ echo $?      # however...
1
user@machine:~$

I managed to convert all my unit tests from shUnit2 to bats before I noticed that something was off. I was using the double bracket notation everywhere because it's just nicer to work with. Little did I know that the set -e shell option totally ignored those double brackets. Best of all, the docs don't mention that fact explicitly. It's because the [[ is part of the conditional constructs, which of course do not trigger that shell option.

andsens avatar Mar 24 '14 22:03 andsens

Woah, nasty. Didn't know that.

How should we warn the people? We don't have tutorials on writing actual tests (this is supposed to be left as an exercise to the reader)

mislav avatar Mar 24 '14 22:03 mislav

Hm, maybe just in the first example:

@test "addition using bc" {
  result="$(echo 2+2 | bc)"
  [ "$result" -eq 4 ]
  # Do not use double brackets ( [[ ... ]] ).
  # They are treated as conditionals by bash and will not cause any test to fail, ever.
}

Or you could write it below them as a caveat emptor in cursive.

EDIT: emptor... not empty

andsens avatar Mar 24 '14 22:03 andsens

set -e behavior with bare [[ ... ]] expressions varies depending on the version of Bash, but I agree we should discourage its use. At the very least you should append || false to the end of a bare [[ ... ]] expression.

I've started a branch that prints a warning when you use a bare [[ ... ]] expression: https://github.com/sstephenson/bats/compare/double-brackets

sstephenson avatar Jun 03 '14 02:06 sstephenson

Some additional information:

Bash 4.1-alpha "fixed" this in 2009 changelog:

j. The [[ and (( commands are now subject to the setting of `set -e' and the ERR trap.

OSX seems to ship with 3.2 (released in 2007), but anything like Ubuntu will have shipped with a more recent version of Bash (4.1) since at least 10.04.

I'm guessing a lot of tests will be written on OSX and executed in CI environments with Ubuntu. Personally, I'm all for keeping my shell up to date - is it worth introducing a dependency on a more recent version of Bash, or at least a strong recommendation on 4.1?

This can be handled with a keg-only dependency in homebrew... I'm not sure it's a real problem in other package managed environments, since they'll likely already have more recent versions of Bash. The README currently suggests "install from source" which perhaps should be amended.

It's worth noting that this will only cause unexpected behaviour with multiple test statements.

A simple test like:

@test "my foo" {
   [[ 1 = 0 ]]
}

Will fail as you'd expect it to.

duggan avatar Jul 10 '14 14:07 duggan

It seems heavy handed to require bash 4.x for something that could be handled with a warning. "Hey! We detected that you are using [[ with bash < 4! You might not want to do that..."

aw-was-here avatar Jul 10 '14 15:07 aw-was-here

"Require" is probably too strong a word - I mean try to encourage having 4.1+ where possible (ie, in package managers). Incorporating the warning where it's not available makes sense.

duggan avatar Jul 10 '14 15:07 duggan

"dependency" = "requirement" in my head. Thank you for the clarification. :D

Totally anecdotal, but as I rewrite the shell scripts that Hadoop ships (and looking at our options for shell unit testing), there was some push back on even requiring bash v3...

aw-was-here avatar Jul 10 '14 15:07 aw-was-here

Well, I'm generally for writing portable shell wherever possible, but since Bash is already a requirement here, I think that ship has sailed :)

duggan avatar Jul 10 '14 15:07 duggan

Data point: I was just bitten by this. +1 for a light-weight solution like adding a comment to the first example.

abesto avatar Nov 26 '15 20:11 abesto

A comment in the first example of README.md would be helpful. IMHO that's good enough, no need to have bats detect the use of double-brackets and issue a warning.

I've also been bitten by this, used double-brackets to test the output against a regex, like so:

run some cmd
[ "$status" -eq 0 ]
[[ "$output" =~ ^some\ +(random )?\ *regex$ ]]

The test command (i.e. [) doesn't support matching against regular expressions, so I resorted to using grep with a Here String instead:

run some cmd
[ "$status" -eq 0 ]
grep -E '^some +(random )? *regex$' <<< "$output"

Hope this helps others who stumble upon this issue.

Summary:

  • use [ instead of [[ where possible;
  • use grep with a Here String grep -E '...' <<< "$output" instead of double-bracket regex matching [[ "$output" =~ ... ]];
  • append || false if you still prefer to use [[, like so: [[ "$output" =~ blah ]] || false – though I find that quite error prone.

sgeb avatar Feb 01 '16 13:02 sgeb

Hope this helps others who stumble upon this issue.

👍

sometheycallme avatar Jul 13 '16 21:07 sometheycallme

Ran into some challenges running CircleCI test attempting to check output of commands against a running container.

Seems like Circle gets confused with sdout and stderr

To work with this, I had to craft the following (my clumsy test) test in BATS, and then seek specific output. Needed to sleep 5 to avoid race condition.

HTH

@test "ansible-controller: webserver responds to curl and playbook executes remotely (outside container)" {
 # check to see if ansible webserver accepts json data (commands) and runs fixtures playbook remotely
 run docker run -d --name=webtest -p 8080:8080 --env-file helpful_files/env_vars -v /home/ubuntu/ansible-security/fixtures/etc/ansible:/opt/staging/cleanerbot_master/ansible-security ansible-controller
   ip=$(docker inspect --format '{{ .NetworkSettings.IPAddress }}' webtest)
   port=8080
   sleep 5
  # the curl command sends ansible playbook feedback of play execution to user
  # put the output of the curl command in a variable
   testoutput=$(curl -v -X POST -d '{"branch_name": "master", "git_handle": "cleanerbot", "flags": [{"flag": "-i", "argument": "inventory"}], "playbook": "ansible-security/play_test.yml"}' http://${ip}:${port}/play)
  # echo the variable into a file
   echo $testoutput > testfile
  # run grep for the variable in the file
  run grep PLAY testfile
  #  check for the output
 [[ ${output} =~ PLAY ]]
}

sometheycallme avatar Jul 14 '16 02:07 sometheycallme

Just tripped over this myself. Very painful. I found that [[ ]] work in some cases but mis-report the offending code. Minimal example, given:

@test "t1" {
    run sh -c false
    [ $status = 1 ]
}

@test "t2" {
    run sh -c false
    [[ $status == 1 ]]
}

t1 and t2 both pass. Change the condition to [ $status = 0 ] and you get [ $status = 0 ] failed, as expected. Change t2 to [[ $status == 0 ]] and BATS instead reports run sh -c false failed. Given this was my first exposure to BATS I assumed my script was the problem and lost two hours before seeing this issue. Please add this warning to the README.

jonross avatar Oct 27 '16 17:10 jonross