Merge branch 'sg/test-x'
authorJunio C Hamano <gitster@pobox.com>
Wed, 14 Mar 2018 19:01:03 +0000 (12:01 -0700)
committerJunio C Hamano <gitster@pobox.com>
Wed, 14 Mar 2018 19:01:03 +0000 (12:01 -0700)
Running test scripts under -x option of the shell is often not a
useful way to debug them, because the error messages from the
commands tests try to capture and inspect are contaminated by the
tracing output by the shell. An earlier work done to make it more
pleasant to run tests under -x with recent versions of bash is
extended to cover posix shells that do not support BASH_XTRACEFD.

* sg/test-x:
travis-ci: run tests with '-x' tracing
t/README: add a note about don't saving stderr of compound commands
t1510-repo-setup: mark as untraceable with '-x'
t9903-bash-prompt: don't check the stderr of __git_ps1()
t5570-git-daemon: don't check the stderr of a subshell
t5526: use $TRASH_DIRECTORY to specify the path of GIT_TRACE log file
t5500-fetch-pack: don't check the stderr of a subshell
t3030-merge-recursive: don't check the stderr of a subshell
t1507-rev-parse-upstream: don't check the stderr of a shell function
t: add means to disable '-x' tracing for individual test scripts
t: prevent '-x' tracing from interfering with test helpers' stderr

1  2 
t/README
t/t5526-fetch-submodules.sh
t/test-lib-functions.sh
t/test-lib.sh
diff --combined t/README
index 1a1361a8063b8e247b6a92b83b6e8876953f95bd,615682263edb8d3101c2498526ce7e9c5a9356a8..24ddebfabf97be1251452fe5b6bba847597431fd
+++ b/t/README
@@@ -84,9 -84,10 +84,10 @@@ appropriately before running "make"
  
  -x::
        Turn on shell tracing (i.e., `set -x`) during the tests
-       themselves. Implies `--verbose`. Note that in non-bash shells,
-       this can cause failures in some tests which redirect and test
-       the output of shell functions. Use with caution.
+       themselves. Implies `--verbose`.
+       Ignored in test scripts that set the variable 'test_untraceable'
+       to a non-empty value, unless it's run with a Bash version
+       supporting BASH_XTRACEFD, i.e. v4.1 or later.
  
  -d::
  --debug::
@@@ -452,6 -453,22 +453,22 @@@ Don't
     causing the next test to start in an unexpected directory.  Do so
     inside a subshell if necessary.
  
+  - save and verify the standard error of compound commands, i.e. group
+    commands, subshells, and shell functions (except test helper
+    functions like 'test_must_fail') like this:
+      ( cd dir && git cmd ) 2>error &&
+      test_cmp expect error
+    When running the test with '-x' tracing, then the trace of commands
+    executed in the compound command will be included in standard error
+    as well, quite possibly throwing off the subsequent checks examining
+    the output.  Instead, save only the relevant git command's standard
+    error:
+      ( cd dir && git cmd 2>../error ) &&
+      test_cmp expect error
   - Break the TAP output
  
     The raw output from your test may be interpreted by a TAP harness. TAP
@@@ -655,7 -672,7 +672,7 @@@ library for your script to use
                test_expect_code 1 git merge "merge msg" B master
        '
  
 - - test_must_fail <git-command>
 + - test_must_fail [<options>] <git-command>
  
     Run a git command and ensure it fails in a controlled way.  Use
     this instead of "! <git-command>".  When git-command dies due to a
     treats it as just another expected failure, which would let such a
     bug go unnoticed.
  
 - - test_might_fail <git-command>
 +   Accepts the following options:
 +
 +     ok=<signal-name>[,<...>]:
 +       Don't treat an exit caused by the given signal as error.
 +       Multiple signals can be specified as a comma separated list.
 +       Currently recognized signal names are: sigpipe, success.
 +       (Don't use 'success', use 'test_might_fail' instead.)
 +
 + - test_might_fail [<options>] <git-command>
  
     Similar to test_must_fail, but tolerate success, too.  Use this
     instead of "<git-command> || :" to catch failures due to segv.
  
 +   Accepts the same options as test_must_fail.
 +
   - test_cmp <expected> <actual>
  
     Check whether the content of the <actual> file matches the
index 74486c73b0bcc20ee69e49d5c1cc072994e4d7b0,ce44d8aa4603f500e81117eecb0766b87be999de..9cc4b569c0566da2f5c2778f404e67e20b29ed06
@@@ -85,7 -85,7 +85,7 @@@ test_expect_success "fetch --recurse-su
        add_upstream_commit &&
        (
                cd downstream &&
-               GIT_TRACE=$(pwd)/../trace.out git fetch --recurse-submodules -j2 2>../actual.err
+               GIT_TRACE="$TRASH_DIRECTORY/trace.out" git fetch --recurse-submodules -j2 2>../actual.err
        ) &&
        test_must_be_empty actual.out &&
        test_i18ncmp expect.err actual.err &&
@@@ -485,7 -485,7 +485,7 @@@ test_expect_success "don't fetch submod
        )
  '
  
 -test_expect_success "'fetch.recurseSubmodules=on-demand' works also without .gitmodule entry" '
 +test_expect_success "'fetch.recurseSubmodules=on-demand' works also without .gitmodules entry" '
        (
                cd downstream &&
                git fetch --recurse-submodules
diff --combined t/test-lib-functions.sh
index 6e342804f96fb62ff0b3cfaf667e7efce79dc6af,37eb34044ab5794d46b4363a1d04dc9445279e2a..b895366feef6027ac00bf47271b2530e8b7e7162
@@@ -610,14 -610,6 +610,14 @@@ list_contains () 
  #
  # Writing this as "! git checkout ../outerspace" is wrong, because
  # the failure could be due to a segv.  We want a controlled failure.
 +#
 +# Accepts the following options:
 +#
 +#   ok=<signal-name>[,<...>]:
 +#     Don't treat an exit caused by the given signal as error.
 +#     Multiple signals can be specified as a comma separated list.
 +#     Currently recognized signal names are: sigpipe, success.
 +#     (Don't use 'success', use 'test_might_fail' instead.)
  
  test_must_fail () {
        case "$1" in
                _test_ok=
                ;;
        esac
-       "$@"
+       "$@" 2>&7
        exit_code=$?
        if test $exit_code -eq 0 && ! list_contains "$_test_ok" success
        then
 -              echo >&2 "test_must_fail: command succeeded: $*"
 +              echo >&4 "test_must_fail: command succeeded: $*"
                return 1
        elif test_match_signal 13 $exit_code && list_contains "$_test_ok" sigpipe
        then
                return 0
        elif test $exit_code -gt 129 && test $exit_code -le 192
        then
 -              echo >&2 "test_must_fail: died by signal $(($exit_code - 128)): $*"
 +              echo >&4 "test_must_fail: died by signal $(($exit_code - 128)): $*"
                return 1
        elif test $exit_code -eq 127
        then
 -              echo >&2 "test_must_fail: command not found: $*"
 +              echo >&4 "test_must_fail: command not found: $*"
                return 1
        elif test $exit_code -eq 126
        then
 -              echo >&2 "test_must_fail: valgrind error: $*"
 +              echo >&4 "test_must_fail: valgrind error: $*"
                return 1
        fi
        return 0
- }
+ } 7>&2 2>&4
  
  # Similar to test_must_fail, but tolerates success, too.  This is
  # meant to be used in contexts like:
  #
  # Writing "git config --unset all.configuration || :" would be wrong,
  # because we want to notice if it fails due to segv.
 +#
 +# Accepts the same options as test_must_fail.
  
  test_might_fail () {
-       test_must_fail ok=success "$@"
- }
+       test_must_fail ok=success "$@" 2>&7
+ } 7>&2 2>&4
  
  # Similar to test_must_fail and test_might_fail, but check that a
  # given command exited with a given exit code. Meant to be used as:
  test_expect_code () {
        want_code=$1
        shift
-       "$@"
+       "$@" 2>&7
        exit_code=$?
        if test $exit_code = $want_code
        then
                return 0
        fi
  
 -      echo >&2 "test_expect_code: command exited with $exit_code, we wanted $want_code $*"
 +      echo >&4 "test_expect_code: command exited with $exit_code, we wanted $want_code $*"
        return 1
- }
+ } 7>&2 2>&4
  
  # test_cmp is a helper function to compare actual and expected output.
  # You can use it like:
@@@ -752,18 -742,18 +752,18 @@@ test_i18ngrep () 
                shift
                ! grep "$@" && return 0
  
 -              echo >&2 "error: '! grep $@' did find a match in:"
 +              echo >&4 "error: '! grep $@' did find a match in:"
        else
                grep "$@" && return 0
  
 -              echo >&2 "error: 'grep $@' didn't find a match in:"
 +              echo >&4 "error: 'grep $@' didn't find a match in:"
        fi
  
        if test -s "$last_arg"
        then
 -              cat >&2 "$last_arg"
 +              cat >&4 "$last_arg"
        else
 -              echo >&2 "<File '$last_arg' is empty>"
 +              echo >&4 "<File '$last_arg' is empty>"
        fi
  
        return 1
  # not output anything when they fail.
  verbose () {
        "$@" && return 0
 -      echo >&2 "command failed: $(git rev-parse --sq-quote "$@")"
 +      echo >&4 "command failed: $(git rev-parse --sq-quote "$@")"
        return 1
  }
  
  # otherwise.
  
  test_must_be_empty () {
 -      if test -s "$1"
 +      if ! test -f "$1"
 +      then
 +              echo "'$1' is missing"
 +              return 1
 +      elif test -s "$1"
        then
                echo "'$1' is not empty, it contains:"
                cat "$1"
@@@ -896,8 -882,8 +896,8 @@@ test_write_lines () 
  }
  
  perl () {
-       command "$PERL_PATH" "$@"
- }
+       command "$PERL_PATH" "$@" 2>&7
+ } 7>&2 2>&4
  
  # Is the value one of the various ways to spell a boolean true/false?
  test_normalize_bool () {
@@@ -1037,13 -1023,13 +1037,13 @@@ test_env () 
                                shift
                                ;;
                        *)
-                               "$@"
+                               "$@" 2>&7
                                exit
                                ;;
                        esac
                done
        )
- }
+ } 7>&2 2>&4
  
  # Returns true if the numeric exit code in "$2" represents the expected signal
  # in "$1". Signals should be given numerically.
@@@ -1085,9 -1071,9 +1085,9 @@@ nongit () 
                GIT_CEILING_DIRECTORIES=$(pwd) &&
                export GIT_CEILING_DIRECTORIES &&
                cd non-repo &&
-               "$@"
+               "$@" 2>&7
        )
- }
+ } 7>&2 2>&4
  
  # convert stdin to pktline representation; note that empty input becomes an
  # empty packet, not a flush packet (for that you can just print 0000 yourself).
diff --combined t/test-lib.sh
index 9535d2e0a936eb1b7a84686a9b702d3ca9fd680d,732213ef1b1934631c0d6f8bcb3fea4c50bbbd0f..7740d511d289f44bb1313308fe49d5894f64b3c2
@@@ -116,7 -116,6 +116,7 @@@ unset VISUAL EMAIL LANGUAGE COLUMNS $("
        my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);
        print join("\n", @vars);
  ')
 +unset XDG_CACHE_HOME
  unset XDG_CONFIG_HOME
  unset GITPERLLIB
  GIT_AUTHOR_EMAIL=author@example.com
                GIT_TEST_CHAIN_LINT=0
                shift ;;
        -x)
-               trace=t
+               # Some test scripts can't be reliably traced  with '-x',
+               # unless the test is run with a Bash version supporting
+               # BASH_XTRACEFD (introduced in Bash v4.1).  Check whether
+               # this test is marked as such, and ignore '-x' if it
+               # isn't executed with a suitable Bash version.
+               if test -z "$test_untraceable" || {
+                    test -n "$BASH_VERSION" && {
+                      test ${BASH_VERSINFO[0]} -gt 4 || {
+                        test ${BASH_VERSINFO[0]} -eq 4 &&
+                        test ${BASH_VERSINFO[1]} -ge 1
+                      }
+                    }
+                  }
+               then
+                       trace=t
+               else
+                       echo >&2 "warning: ignoring -x; '$0' is untraceable without BASH_XTRACEFD"
+               fi
                shift ;;
        --verbose-log)
                verbose_log=t