Merge branch 'jk/tap-verbose-fix'
authorJunio C Hamano <gitster@pobox.com>
Wed, 26 Oct 2016 20:14:54 +0000 (13:14 -0700)
committerJunio C Hamano <gitster@pobox.com>
Wed, 26 Oct 2016 20:14:54 +0000 (13:14 -0700)
The Travis CI configuration we ship ran the tests with --verbose
option but this risks non-TAP output that happens to be "ok" to be
misinterpreted as TAP signalling a test that passed. This resulted
in unnecessary failure. This has been corrected by introducing a
new mode to run our tests in the test harness to send the verbose
output separately to the log file.

* jk/tap-verbose-fix:
test-lib: bail out when "-v" used under "prove"
travis: use --verbose-log test option
test-lib: add --verbose-log option
test-lib: handle TEST_OUTPUT_DIRECTORY with spaces

1  2 
.travis.yml
t/README
t/test-lib.sh
diff --combined .travis.yml
index 37a1e1fb6d48935aa22a384797ce82a39340b2ed,35f0fcb183fb5ff2e735d5af9d3aec45a55131ba..9a65514d829987eb8a0274c12f094381d90e87be
@@@ -18,8 -18,6 +18,8 @@@ addons
    apt:
      packages:
      - language-pack-is
 +    - git-svn
 +    - apache2
  
  env:
    global:
@@@ -31,8 -29,7 +31,8 @@@
      - LINUX_GIT_LFS_VERSION="1.2.0"
      - DEFAULT_TEST_TARGET=prove
      - GIT_PROVE_OPTS="--timer --jobs 3 --state=failed,slow,save"
-     - GIT_TEST_OPTS="--verbose --tee"
+     - GIT_TEST_OPTS="--verbose-log"
 +    - GIT_TEST_HTTPD=true
      - GIT_TEST_CLONE_2GB=YesPlease
      # t9810 occasionally fails on Travis CI OS X
      # t9816 occasionally fails with "TAP out of sequence errors" on Travis CI OS X
@@@ -78,14 -75,12 +78,14 @@@ before_install
          FORMULA=$1
          SHA=$(brew fetch --force $FORMULA 2>&1 | grep ^SHA256: | cut -d ' ' -f 2)
          sed -E -i.bak "s/sha256 \"[0-9a-f]{64}\"/sha256 \"$SHA\"/g" \
 -          /usr/local/Library/Taps/homebrew/homebrew-binary/$FORMULA.rb
 +          "$(brew --repository homebrew/homebrew-binary)/$FORMULA.rb"
        }
        brew update --quiet
        brew tap homebrew/binary --quiet
        brew_force_set_latest_binary_hash perforce
        brew_force_set_latest_binary_hash perforce-server
 +      # Uncomment this if you want to run perf tests:
 +      # brew install gnu-time
        brew install git-lfs perforce-server perforce gettext
        brew link --force gettext
        ;;
diff --combined t/README
index 0f764c0aef7aaa842299abde05effb73a0def9f4,243332cf5e60177c5f87b89a66496ef6744b49fa..4982d1c5216c10b7d44e66eace34e40d49cb5b2f
+++ b/t/README
@@@ -153,6 -153,12 +153,12 @@@ appropriately before running "make"
        As the names depend on the tests' file names, it is safe to
        run the tests with this option in parallel.
  
+ --verbose-log::
+       Write verbose output to the same logfile as `--tee`, but do
+       _not_ write it to stdout. Unlike `--tee --verbose`, this option
+       is safe to use when stdout is being consumed by a TAP parser
+       like `prove`. Implies `--tee` and `--verbose`.
  --with-dashes::
        By default tests are run without dashed forms of
        commands (like git-commit) in the PATH (it only uses
@@@ -265,7 -271,7 +271,7 @@@ right, so this
      $ sh ./t9200-git-cvsexport-commit.sh --run='1-4 !3'
  
  will run tests 1, 2, and 4.  Items that comes later have higher
 -precendence.  It means that this:
 +precedence.  It means that this:
  
      $ sh ./t9200-git-cvsexport-commit.sh --run='!3 1-4'
  
diff --combined t/test-lib.sh
index 11562bde10be50d47f3a82b3885b5a0aab50c14a,21ebe95139dc820b03de549e938635b4a1499fa7..b859db61ac0d034b8f2c14cea4a960a1a3d9f576
@@@ -54,12 -54,22 +54,22 @@@ case "$GIT_TEST_TEE_STARTED, $* " i
  done,*)
        # do not redirect again
        ;;
- *' --tee '*|*' --va'*)
+ *' --tee '*|*' --va'*|*' --verbose-log '*)
        mkdir -p "$TEST_OUTPUT_DIRECTORY/test-results"
        BASE="$TEST_OUTPUT_DIRECTORY/test-results/$(basename "$0" .sh)"
+       # Make this filename available to the sub-process in case it is using
+       # --verbose-log.
+       GIT_TEST_TEE_OUTPUT_FILE=$BASE.out
+       export GIT_TEST_TEE_OUTPUT_FILE
+       # Truncate before calling "tee -a" to get rid of the results
+       # from any previous runs.
+       >"$GIT_TEST_TEE_OUTPUT_FILE"
        (GIT_TEST_TEE_STARTED=done ${SHELL_PATH} "$0" "$@" 2>&1;
-        echo $? > $BASE.exit) | tee $BASE.out
-       test "$(cat $BASE.exit)" = 0
+        echo $? >"$BASE.exit") | tee -a "$GIT_TEST_TEE_OUTPUT_FILE"
+       test "$(cat "$BASE.exit")" = 0
        exit
        ;;
  esac
@@@ -89,7 -99,6 +99,7 @@@ unset VISUAL EMAIL LANGUAGE COLUMNS $("
                UNZIP
                PERF_
                CURL_VERBOSE
 +              TRACE_CURL
        ));
        my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);
        print join("\n", @vars);
@@@ -163,9 -172,6 +173,9 @@@ _x40="$_x05$_x05$_x05$_x05$_x05$_x05$_x
  # Zero SHA-1
  _z40=0000000000000000000000000000000000000000
  
 +EMPTY_TREE=4b825dc642cb6eb9a060e54bf8d69288fbee4904
 +EMPTY_BLOB=e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
 +
  # Line feed
  LF='
  '
  # when case-folding filenames
  u200c=$(printf '\342\200\214')
  
 -export _x05 _x40 _z40 LF u200c
 +export _x05 _x40 _z40 LF u200c EMPTY_TREE EMPTY_BLOB
  
  # Each test should start with something like this, after copyright notices:
  #
@@@ -246,6 -252,9 +256,9 @@@ d
                trace=t
                verbose=t
                shift ;;
+       --verbose-log)
+               verbose_log=t
+               shift ;;
        *)
                echo "error: unknown test option '$1'" >&2; exit 1 ;;
        esac
@@@ -308,6 -317,16 +321,16 @@@ say () 
        say_color info "$*"
  }
  
+ if test -n "$HARNESS_ACTIVE"
+ then
+       if test "$verbose" = t || test -n "$verbose_only"
+       then
+               printf 'Bail out! %s\n' \
+                'verbose mode forbidden under TAP harness; try --verbose-log'
+               exit 1
+       fi
+ fi
  test "${test_description}" != "" ||
  error "Test script did not set test_description."
  
  
  exec 5>&1
  exec 6<&0
- if test "$verbose" = "t"
+ if test "$verbose_log" = "t"
+ then
+       exec 3>>"$GIT_TEST_TEE_OUTPUT_FILE" 4>&3
+ elif test "$verbose" = "t"
  then
        exec 4>&2 3>&1
  else
@@@ -688,9 -710,9 +714,9 @@@ test_done () 
                test_results_dir="$TEST_OUTPUT_DIRECTORY/test-results"
                mkdir -p "$test_results_dir"
                base=${0##*/}
 -              test_results_path="$test_results_dir/${base%.sh}-$$.counts"
 +              test_results_path="$test_results_dir/${base%.sh}.counts"
  
 -              cat >>"$test_results_path" <<-EOF
 +              cat >"$test_results_path" <<-EOF
                total $test_count
                success $test_success
                fixed $test_fixed
@@@ -802,7 -824,7 +828,7 @@@ the
        # override all git executables in TEST_DIRECTORY/..
        GIT_VALGRIND=$TEST_DIRECTORY/valgrind
        mkdir -p "$GIT_VALGRIND"/bin
 -      for file in $GIT_BUILD_DIR/git* $GIT_BUILD_DIR/test-*
 +      for file in $GIT_BUILD_DIR/git* $GIT_BUILD_DIR/t/helper/test-*
        do
                make_valgrind_symlink $file
        done
@@@ -871,10 -893,10 +897,10 @@@ test -d "$GIT_BUILD_DIR"/templates/blt 
        error "You haven't built things yet, have you?"
  }
  
 -if ! test -x "$GIT_BUILD_DIR"/test-chmtime
 +if ! test -x "$GIT_BUILD_DIR"/t/helper/test-chmtime
  then
        echo >&2 'You need to build test-chmtime:'
 -      echo >&2 'Run "make test-chmtime" in the source (toplevel) directory'
 +      echo >&2 'Run "make t/helper/test-chmtime" in the source (toplevel) directory'
        exit 1
  fi
  
@@@ -1073,10 -1095,6 +1099,10 @@@ test_lazy_prereq NOT_ROOT 
        test "$uid" != 0
  '
  
 +test_lazy_prereq JGIT '
 +      type jgit
 +'
 +
  # SANITY is about "can you correctly predict what the filesystem would
  # do by only looking at the permission bits of the files and
  # directories?"  A typical example of !SANITY is running the test
@@@ -1119,12 -1137,3 +1145,12 @@@ run_with_limited_cmdline () 
  }
  
  test_lazy_prereq CMDLINE_LIMIT 'run_with_limited_cmdline true'
 +
 +build_option () {
 +      git version --build-options |
 +      sed -ne "s/^$1: //p"
 +}
 +
 +test_lazy_prereq LONG_IS_64BIT '
 +      test 8 -le "$(build_option sizeof-long)"
 +'