Change [[ to [ in tests where possible (#99)

* Using [[ ... ]] on older versions of Bash (e.g. 3.2 on macOS) causes
  the test to always pass.
* Include test/README.md that explains the problem and suggests
  solutions for future tests.

test/README.md

@@ -0,0 +1,22 @@
+Testing `bash-preexec`
+======================
+
+**Note on test conditions**
+
+When writing test conditions, use `[ ... ]` instead of `[[ ... ]]` since the
+former are supported by Bats on Bash versions before 4.1. In particular, macOS
+uses Bash 3.2, and `[[ ... ]]` tests always pass on macOS.
+
+In some cases, you may want to use a feature unique to `[[ ... ]]` such as
+pattern matching (`[[ $name = a* ]]`) or regular expressions (`[[ $(date) =~
+^Fri\ ...\ 13 ]]`). In those cases, use the following pattern to replace “bare”
+`[[ ... ]]`.
+
+```
+[[ ... ]] || return 1
+```
+
+References:
+* [Differences between `[` and `[[`](http://mywiki.wooledge.org/BashFAQ/031)
+* [Problems with `[[` in Bats](https://github.com/sstephenson/bats/issues/49)
+* [Using `|| return 1` instead of `|| false`](https://github.com/bats-core/bats-core/commit/e5695a673faad4d4d33446ed5c99d70dbfa6d8be)

test/bash-preexec.bats

@@ -24,28 +24,28 @@ test_preexec_echo() {
 @test "__bp_install_after_session_init should exit with 1 if we're not using bash" {
   unset BASH_VERSION
   run '__bp_install_after_session_init'
-  [[ $status == 1 ]]
-  [[ -z "$output" ]]
+  [ $status -eq 1 ]
+  [ -z "$output" ]
 }
 
 @test "__bp_install should exit if it's already installed" {
   bp_install
 
   run '__bp_install'
-  [[ $status == 1 ]]
-  [[ -z "$output" ]]
+  [ $status -eq 1 ]
+  [ -z "$output" ]
 }
 
 @test "__bp_install should remove trap logic and itself from PROMPT_COMMAND" {
   __bp_install_after_session_init
 
-  [[ "$PROMPT_COMMAND" == *"trap DEBUG"* ]]
-  [[ "$PROMPT_COMMAND" == *"__bp_install"* ]]
+  [[ "$PROMPT_COMMAND" == *"trap DEBUG"* ]] || return 1
+  [[ "$PROMPT_COMMAND" == *"__bp_install"* ]] || return 1
 
   eval "$PROMPT_COMMAND"
 
-  [[ "$PROMPT_COMMAND" != *"trap DEBUG"* ]]
-  [[ "$PROMPT_COMMAND" != *"__bp_install"* ]]
+  [[ "$PROMPT_COMMAND" != *"trap DEBUG"* ]] || return 1
+  [[ "$PROMPT_COMMAND" != *"__bp_install"* ]] || return 1
 }
 
 @test "__bp_install should preserve an existing DEBUG trap" {
@@ -54,13 +54,13 @@ test_preexec_echo() {
 
   # note setting this causes BATS to mis-report the failure line when this test fails
   trap foo DEBUG
-  [[ "$(trap -p DEBUG | cut -d' ' -f3)" == "'foo'" ]]
+  [ "$(trap -p DEBUG | cut -d' ' -f3)" == "'foo'" ]
 
   bp_install
   trap_count_snapshot=$trap_invoked_count
 
-  [[ "$(trap -p DEBUG | cut -d' ' -f3)" == "'__bp_preexec_invoke_exec" ]]
-  [[ "${preexec_functions[*]}" == *"__bp_original_debug_trap"* ]]
+  [ "$(trap -p DEBUG | cut -d' ' -f3)" == "'__bp_preexec_invoke_exec" ]
+  [[ "${preexec_functions[*]}" == *"__bp_original_debug_trap"* ]] || return 1
 
   __bp_interactive_mode # triggers the DEBUG trap
 
@@ -79,15 +79,15 @@ test_preexec_echo() {
     __bp_interactive_mode
 
     run '__bp_preexec_invoke_exec' 'true'
-    [[ $status == 0 ]]
-    [[ -z "$output" ]]
+    [ $status -eq 0 ]
+    [ -z "$output" ]
 }
 
 @test "precmd should execute a function once" {
     precmd_functions+=(test_echo)
     run '__bp_precmd_invoke_cmd'
-    [[ $status == 0 ]]
-    [[ "$output" == "test echo" ]]
+    [ $status -eq 0 ]
+    [ "$output" == "test echo" ]
 }
 
 @test "precmd should set \$? to be the previous exit code" {
@@ -105,8 +105,8 @@ test_preexec_echo() {
 
     precmd_functions+=(echo_exit_code)
     run 'set_exit_code_and_run_precmd'
-    [[ $status == 0 ]]
-    [[ "$output" == "251" ]]
+    [ $status -eq 0 ]
+    [ "$output" == "251" ]
 }
 
 @test "precmd should set \$BP_PIPESTATUS to the previous \$PIPESTATUS" {
@@ -121,8 +121,8 @@ test_preexec_echo() {
 
   precmd_functions+=(echo_pipestatus)
   run 'set_pipestatus_and_run_precmd'
-  [[ $status == 0 ]]
-  [[ "$output" == "1 0" ]]
+  [ $status -eq 0 ]
+  [ "$output" == "1 0" ]
 }
 
 @test "precmd should set \$_ to be the previous last arg" {
@@ -137,8 +137,8 @@ test_preexec_echo() {
     __bp_preexec_invoke_exec "$_"
     eval "$bats_trap" # Restore trap
     run '__bp_precmd_invoke_cmd'
-    [[ $status == 0 ]]
-    [[ "$output" == "last-arg" ]]
+    [ $status -eq 0 ]
+    [ "$output" == "last-arg" ]
 }
 
 @test "preexec should execute a function with the last command in our history" {
@@ -148,8 +148,8 @@ test_preexec_echo() {
     history -s $git_command
 
     run '__bp_preexec_invoke_exec'
-    [[ $status == 0 ]]
-    [[ "$output" == "$git_command" ]]
+    [ $status -eq 0 ]
+    [ "$output" == "$git_command" ]
 }
 
 @test "preexec should execute multiple functions in the order added to their arrays" {
@@ -160,10 +160,10 @@ test_preexec_echo() {
     __bp_interactive_mode
 
     run '__bp_preexec_invoke_exec'
-    [[ $status == 0 ]]
-    [[ "${#lines[@]}" == 2 ]]
-    [[ "${lines[0]}" == "fake command one" ]]
-    [[ "${lines[1]}" == "fake command two" ]]
+    [ $status -eq 0 ]
+    [ "${#lines[@]}" == '2' ]
+    [ "${lines[0]}" == "fake command one" ]
+    [ "${lines[1]}" == "fake command two" ]
 }
 
 @test "preecmd should execute multiple functions in the order added to their arrays" {
@@ -173,10 +173,10 @@ test_preexec_echo() {
     precmd_functions+=(fun_2)
 
     run '__bp_precmd_invoke_cmd'
-    [[ $status == 0 ]]
-    [[ "${#lines[@]}" == 2 ]]
-    [[ "${lines[0]}" == "one" ]]
-    [[ "${lines[1]}" == "two" ]]
+    [ $status -eq 0 ]
+    [ "${#lines[@]}" == '2' ]
+    [ "${lines[0]}" == "one" ]
+    [ "${lines[1]}" == "two" ]
 }
 
 @test "preexec should execute a function with IFS defined to local scope" {
@@ -186,8 +186,8 @@ test_preexec_echo() {
 
     __bp_interactive_mode
     run '__bp_preexec_invoke_exec'
-    [[ $status == 0 ]]
-    [[ "$output" == "1 2" ]]
+    [ $status -eq 0 ]
+    [ "$output" == "1 2" ]
 }
 
 @test "precmd should execute a function with IFS defined to local scope" {
@@ -195,8 +195,8 @@ test_preexec_echo() {
     name_with_underscores_2() { parts=(2_2); echo $parts; }
     precmd_functions+=(name_with_underscores_2)
     run '__bp_precmd_invoke_cmd'
-    [[ $status == 0 ]]
-    [[ "$output" == "2 2" ]]
+    [ $status -eq 0 ]
+    [ "$output" == "2 2" ]
 }
 
 @test "preexec should set \$? to be the exit code of preexec_functions" {
@@ -208,26 +208,26 @@ test_preexec_echo() {
     __bp_interactive_mode
 
     run '__bp_preexec_invoke_exec'
-    [[ $status == 1 ]]
+    [ $status -eq 1 ]
 }
 
 @test "in_prompt_command should detect if a command is part of PROMPT_COMMAND" {
 
     PROMPT_COMMAND="precmd_invoke_cmd; something;"
     run '__bp_in_prompt_command' "something"
-    [[ $status == 0 ]]
+    [ $status -eq 0 ]
 
     run '__bp_in_prompt_command' "something_else"
-    [[ $status == 1 ]]
+    [ $status -eq 1 ]
 
     # Should trim commands and arguments here.
     PROMPT_COMMAND=" precmd_invoke_cmd ; something ; some_stuff_here;"
     run '__bp_in_prompt_command' " precmd_invoke_cmd "
-    [[ $status == 0 ]]
+    [ $status -eq 0 ]
 
     PROMPT_COMMAND=" precmd_invoke_cmd ; something ; some_stuff_here;"
     run '__bp_in_prompt_command' " not_found"
-    [[ $status == 1 ]]
+    [ $status -eq 1 ]
 
 }
 
@@ -236,18 +236,18 @@ test_preexec_echo() {
     # Should remove ignorespace
     HISTCONTROL="ignorespace:ignoredups:*"
     __bp_adjust_histcontrol
-    [[ "$HISTCONTROL" == ":ignoredups:*" ]]
+    [ "$HISTCONTROL" == ":ignoredups:*" ]
 
     # Should remove ignoreboth and replace it with ignoredups
     HISTCONTROL="ignoreboth"
     __bp_adjust_histcontrol
-    [[ "$HISTCONTROL" == "ignoredups:" ]]
+    [ "$HISTCONTROL" == "ignoredups:" ]
 
     # Handle a few inputs
     HISTCONTROL="ignoreboth:ignorespace:some_thing_else"
     __bp_adjust_histcontrol
     echo "$HISTCONTROL"
-    [[ "$HISTCONTROL" == "ignoredups:::some_thing_else" ]]
+    [ "$HISTCONTROL" == "ignoredups:::some_thing_else" ]
 
 }
 
@@ -259,8 +259,8 @@ test_preexec_echo() {
     history -s $git_command
 
     run '__bp_preexec_invoke_exec'
-    [[ $status == 0 ]]
-    [[ "$output" == "$git_command" ]]
+    [ $status -eq 0 ]
+    [ "$output" == "$git_command" ]
 }
 
 @test "preexec should not strip whitespace from commands" {
@@ -269,8 +269,8 @@ test_preexec_echo() {
     history -s " this command has whitespace "
 
     run '__bp_preexec_invoke_exec'
-    [[ $status == 0 ]]
-    [[ "$output" == " this command has whitespace " ]]
+    [ $status -eq 0 ]
+    [ "$output" == " this command has whitespace " ]
 }
 
 @test "preexec should preserve multi-line strings in commands" {
@@ -279,9 +279,9 @@ test_preexec_echo() {
     history -s "this 'command contains
 a multiline string'"
     run '__bp_preexec_invoke_exec'
-    [[ $status == 0 ]]
-    [[ "$output" == "this 'command contains
-a multiline string'" ]]
+    [ $status -eq 0 ]
+    [ "$output" == "this 'command contains
+a multiline string'" ]
 }
 
 @test "preexec should work on options to 'echo' commands" {
@@ -289,6 +289,6 @@ a multiline string'" ]]
     __bp_interactive_mode
     history -s -- '-n'
     run '__bp_preexec_invoke_exec'
-    [[ $status == 0 ]]
-    [[ "$output" == '-n' ]]
+    [ $status -eq 0 ]
+    [ "$output" == '-n' ]
 }

test/include-test.bats

@@ -3,18 +3,18 @@
 @test "should not import if it's already defined" {
   __bp_imported="defined"
   source "${BATS_TEST_DIRNAME}/../bash-preexec.sh"
-  [[ -z $(type -t __bp_preexec_and_precmd_install) ]]
+  [ -z $(type -t __bp_preexec_and_precmd_install) ]
 }
 
 @test "should import if not defined" {
   unset __bp_imported
   source "${BATS_TEST_DIRNAME}/../bash-preexec.sh"
-  [[ -n $(type -t __bp_install) ]]
+  [ -n $(type -t __bp_install) ]
 }
 
 @test "bp should stop installation if HISTTIMEFORMAT is readonly" {
   readonly HISTTIMEFORMAT
   run source "${BATS_TEST_DIRNAME}/../bash-preexec.sh"
-  [[ $status != 0 ]]
-  [[ "$output" =~ "HISTTIMEFORMAT" ]]
+  [ $status -ne 0 ]
+  [[ "$output" =~ "HISTTIMEFORMAT" ]] || return 1
 }