test-suite: Log everything to a file in non-verbose mode

Using 'return' from a test snippet is now a no-go because it would skip the
assignment of the resulting exit code and it would also skip set +x.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>

t/.gitignore

@@ -1,2 +1,3 @@
 /trash directory*
 /test-results
+/test-log

t/Makefile

@@ -21,7 +21,7 @@ $(T):
 	@echo "*** $@ ***"; GIT_CONFIG=.git/config '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)
 
 pre-clean:
-	$(RM) -r test-results
+	$(RM) -r test-results test-log
 
 clean:
 	$(RM) -r 'trash directory'.* test-results

t/t0040-parse-options.sh

@@ -39,6 +39,7 @@ Standard options
 EOF
 
 test_expect_success 'test help' '
+	set +x &&
 	test_must_fail test-parse-options -h > output 2> output.err &&
 	test ! -s output &&
 	test_cmp expect.err output.err
@@ -158,6 +159,7 @@ error: did you mean \`--boolean\` (with two dashes ?)
 EOF
 
 test_expect_success 'detect possible typos' '
+	set +x &&
 	test_must_fail test-parse-options -boolean > output 2> output.err &&
 	test ! -s output &&
 	test_cmp typo.err output.err
@@ -223,6 +225,7 @@ Callback: "not set", 1
 EOF
 
 test_expect_success 'OPT_CALLBACK() and callback errors work' '
+	set +x &&
 	test_must_fail test-parse-options --no-length > output 2> output.err &&
 	test_cmp expect output &&
 	test_cmp expect.err output.err

t/t1503-rev-parse-verify.sh

@@ -71,6 +71,7 @@ test_expect_success 'fails with any bad rev or many good revs' '
 '
 
 test_expect_success 'fails silently when using -q' '
+	set +x &&
 	test_must_fail git rev-parse --verify --quiet 2>error &&
 	test -z "$(cat error)" &&
 	test_must_fail git rev-parse -q --verify foo 2>error &&

t/t5505-remote.sh

@@ -247,6 +247,7 @@ EOF
 
 test_expect_success 'set-head --auto fails w/multiple HEADs' '
 	(cd test &&
+	 set +x &&
 	 test_must_fail git remote set-head --auto two >output 2>&1 &&
 	test_cmp expect output)
 '

t/t6022-merge-rename.sh

@@ -95,30 +95,15 @@ git checkout master'
 test_expect_success 'pull renaming branch into unrenaming one' \
 '
 	git show-branch
-	git pull . white && {
-		echo "BAD: should have conflicted"
-		return 1
-	}
-	git ls-files -s
-	test "$(git ls-files -u B | wc -l)" -eq 3 || {
-		echo "BAD: should have left stages for B"
-		return 1
-	}
-	test "$(git ls-files -s N | wc -l)" -eq 1 || {
-		echo "BAD: should have merged N"
-		return 1
-	}
+	test_must_fail git pull . white &&
+	git ls-files -s &&
+	test "$(git ls-files -u B | wc -l)" -eq 3 &&
+	test "$(git ls-files -s N | wc -l)" -eq 1 &&
 	sed -ne "/^g/{
 	p
 	q
-	}" B | grep master || {
-		echo "BAD: should have listed our change first"
-		return 1
-	}
-	test "$(git diff white N | wc -l)" -eq 0 || {
-		echo "BAD: should have taken colored branch"
-		return 1
-	}
+	}" B | grep master &&
+	test "$(git diff white N | wc -l)" -eq 0
 '
 
 test_expect_success 'pull renaming branch into another renaming one' \
@@ -126,95 +111,44 @@ test_expect_success 'pull renaming branch into another renaming one' \
 	rm -f B
 	git reset --hard
 	git checkout red
-	git pull . white && {
-		echo "BAD: should have conflicted"
-		return 1
-	}
-	test "$(git ls-files -u B | wc -l)" -eq 3 || {
-		echo "BAD: should have left stages"
-		return 1
-	}
-	test "$(git ls-files -s N | wc -l)" -eq 1 || {
-		echo "BAD: should have merged N"
-		return 1
-	}
+	test_must_fail git pull . white &&
+	test "$(git ls-files -u B | wc -l)" -eq 3 &&
+	test "$(git ls-files -s N | wc -l)" -eq 1 &&
 	sed -ne "/^g/{
 	p
 	q
-	}" B | grep red || {
-		echo "BAD: should have listed our change first"
-		return 1
-	}
-	test "$(git diff white N | wc -l)" -eq 0 || {
-		echo "BAD: should have taken colored branch"
-		return 1
-	}
+	}" B | grep red &&
+	test "$(git diff white N | wc -l)" -eq 0
 '
 
 test_expect_success 'pull unrenaming branch into renaming one' \
 '
 	git reset --hard
 	git show-branch
-	git pull . master && {
-		echo "BAD: should have conflicted"
-		return 1
-	}
-	test "$(git ls-files -u B | wc -l)" -eq 3 || {
-		echo "BAD: should have left stages"
-		return 1
-	}
-	test "$(git ls-files -s N | wc -l)" -eq 1 || {
-		echo "BAD: should have merged N"
-		return 1
-	}
+	test_must_fail git pull . master &&
+	test "$(git ls-files -u B | wc -l)" -eq 3 &&
+	test "$(git ls-files -s N | wc -l)" -eq 1 &&
 	sed -ne "/^g/{
 	p
 	q
-	}" B | grep red || {
-		echo "BAD: should have listed our change first"
-		return 1
-	}
-	test "$(git diff white N | wc -l)" -eq 0 || {
-		echo "BAD: should have taken colored branch"
-		return 1
-	}
+	}" B | grep red &&
+	test "$(git diff white N | wc -l)" -eq 0
 '
 
 test_expect_success 'pull conflicting renames' \
 '
 	git reset --hard
 	git show-branch
-	git pull . blue && {
-		echo "BAD: should have conflicted"
-		return 1
-	}
-	test "$(git ls-files -u A | wc -l)" -eq 1 || {
-		echo "BAD: should have left a stage"
-		return 1
-	}
-	test "$(git ls-files -u B | wc -l)" -eq 1 || {
-		echo "BAD: should have left a stage"
-		return 1
-	}
-	test "$(git ls-files -u C | wc -l)" -eq 1 || {
-		echo "BAD: should have left a stage"
-		return 1
-	}
-	test "$(git ls-files -s N | wc -l)" -eq 1 || {
-		echo "BAD: should have merged N"
-		return 1
-	}
+	test_must_fail git pull . blue &&
+	test "$(git ls-files -u A | wc -l)" -eq 1 &&
+	test "$(git ls-files -u B | wc -l)" -eq 1 &&
+	test "$(git ls-files -u C | wc -l)" -eq 1 &&
+	test "$(git ls-files -s N | wc -l)" -eq 1 &&
 	sed -ne "/^g/{
 	p
 	q
-	}" B | grep red || {
-		echo "BAD: should have listed our change first"
-		return 1
-	}
-	test "$(git diff white N | wc -l)" -eq 0 || {
-		echo "BAD: should have taken colored branch"
-		return 1
-	}
+	}" B | grep red &&
+	test "$(git diff white N | wc -l)" -eq 0
 '
 
 test_expect_success 'interference with untracked working tree file' '
@@ -222,14 +156,8 @@ test_expect_success 'interference with untracked working tree file' '
 	git reset --hard
 	git show-branch
 	echo >A this file should not matter
-	git pull . white && {
-		echo "BAD: should have conflicted"
-		return 1
-	}
-	test -f A || {
-		echo "BAD: should have left A intact"
-		return 1
-	}
+	test_must_fail git pull . white &&
+	test -f A
 '
 
 test_expect_success 'interference with untracked working tree file' '
@@ -239,14 +167,8 @@ test_expect_success 'interference with untracked working tree file' '
 	git show-branch
 	rm -f A
 	echo >A this file should not matter
-	git pull . red && {
-		echo "BAD: should have conflicted"
-		return 1
-	}
-	test -f A || {
-		echo "BAD: should have left A intact"
-		return 1
-	}
+	test_must_fail git pull . red &&
+	test -f A
 '
 
 test_expect_success 'interference with untracked working tree file' '
@@ -256,14 +178,8 @@ test_expect_success 'interference with untracked working tree file' '
 	git checkout -f master
 	git tag -f anchor
 	git show-branch
-	git pull . yellow || {
-		echo "BAD: should have cleanly merged"
-		return 1
-	}
-	test -f M && {
-		echo "BAD: should have removed M"
-		return 1
-	}
+	git pull . yellow
+	test ! -f M &&
 	git reset --hard anchor
 '
 
@@ -276,14 +192,8 @@ test_expect_success 'updated working tree file should prevent the merge' '
 	git show-branch
 	echo >>M one line addition
 	cat M >M.saved
-	git pull . yellow && {
-		echo "BAD: should have complained"
-		return 1
-	}
-	diff M M.saved || {
-		echo "BAD: should have left M intact"
-		return 1
-	}
+	test_must_fail git pull . yellow &&
+	diff M M.saved &&
 	rm -f M.saved
 '
 
@@ -297,14 +207,8 @@ test_expect_success 'updated working tree file should prevent the merge' '
 	echo >>M one line addition
 	cat M >M.saved
 	git update-index M
-	git pull . yellow && {
-		echo "BAD: should have complained"
-		return 1
-	}
-	diff M M.saved || {
-		echo "BAD: should have left M intact"
-		return 1
-	}
+	test_must_fail git pull . yellow &&
+	diff M M.saved &&
 	rm -f M.saved
 '
 
@@ -316,18 +220,9 @@ test_expect_success 'interference with untracked working tree file' '
 	git tag -f anchor
 	git show-branch
 	echo >M this file should not matter
-	git pull . master || {
-		echo "BAD: should have cleanly merged"
-		return 1
-	}
-	test -f M || {
-		echo "BAD: should have left M intact"
-		return 1
-	}
-	git ls-files -s | grep M && {
-		echo "BAD: M must be untracked in the result"
-		return 1
-	}
+	git pull . master &&
+	test -f M &&
+	! (git ls-files -s | grep M) &&
 	git reset --hard anchor
 '
 

t/t6034-merge-rename-nocruft.sh

@@ -73,33 +73,12 @@ test_expect_success 'merge white into red (A->B,M->N)' \
 '
 	git checkout -b red-white red &&
 	git merge white &&
-	git write-tree >/dev/null || {
-		echo "BAD: merge did not complete"
-		return 1
-	}
-
-	test -f B || {
-		echo "BAD: B does not exist in working directory"
-		return 1
-	}
-	test -f N || {
-		echo "BAD: N does not exist in working directory"
-		return 1
-	}
-	test -f R || {
-		echo "BAD: R does not exist in working directory"
-		return 1
-	}
-
-	test -f A && {
-		echo "BAD: A still exists in working directory"
-		return 1
-	}
-	test -f M && {
-		echo "BAD: M still exists in working directory"
-		return 1
-	}
-	return 0
+	git write-tree >/dev/null &&
+	test -f B &&
+	test -f N &&
+	test -f R &&
+	! test -f A &&
+	! test -f M
 '
 
 # This test broke in 8371234ecaaf6e14fe3f2082a855eff1bbd79ae9
@@ -108,32 +87,12 @@ test_expect_success 'merge blue into white (A->B, mod A, A untracked)' \
 	git checkout -b white-blue white &&
 	echo dirty >A &&
 	git merge blue &&
-	git write-tree >/dev/null || {
-		echo "BAD: merge did not complete"
-		return 1
-	}
-
-	test -f A || {
-		echo "BAD: A does not exist in working directory"
-		return 1
-	}
-	test `cat A` = dirty || {
-		echo "BAD: A content is wrong"
-		return 1
-	}
-	test -f B || {
-		echo "BAD: B does not exist in working directory"
-		return 1
-	}
-	test -f N || {
-		echo "BAD: N does not exist in working directory"
-		return 1
-	}
-	test -f M && {
-		echo "BAD: M still exists in working directory"
-		return 1
-	}
-	return 0
+	git write-tree >/dev/null &&
+	test -f A &&
+	test `cat A` = dirty &&
+	test -f B &&
+	test -f N &&
+	! test -f M
 '
 
 test_done

t/t9300-fast-import.sh

@@ -399,20 +399,8 @@ from refs/heads/branch
 INPUT_END
 test_expect_success \
     'F: non-fast-forward update skips' \
-    'if git fast-import <input
-	 then
-		echo BAD gfi did not fail
-		return 1
-	 else
-		if test $old_branch = `git rev-parse --verify branch^0`
-		then
-			: branch unaffected and failure returned
-			return 0
-		else
-			echo BAD gfi changed branch $old_branch
-			return 1
-		fi
-	 fi
+	'test_must_fail git fast-import <input &&
+	 test $old_branch = `git rev-parse --verify branch^0`
 	'
 test_expect_success \
 	'F: verify pack' \

t/test-lib.sh

@@ -60,6 +60,7 @@ export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
 export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
 export EDITOR VISUAL
 GIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}
+TEST_DIRECTORY=$(pwd)
 
 # Protect ourselves from common misconfiguration to export
 # CDPATH into the environment
@@ -167,9 +168,15 @@ fi
 exec 5>&1
 if test "$verbose" = "t"
 then
+	set_x= set_nox=
 	exec 4>&2 3>&1
 else
-	exec 4>/dev/null 3>/dev/null
+	test_log_dir="$TEST_DIRECTORY/test-log"
+	mkdir -p "$test_log_dir"
+	test_log_path="$test_log_dir/${0%.sh}-$$"
+	set_x='set -x;'
+	set_nox='set +x'
+	exec 3>"$test_log_path" 4>&3
 fi
 
 test_failure=0
@@ -303,8 +310,9 @@ test_debug () {
 }
 
 test_run_ () {
-	eval >&3 2>&4 "$1"
-	eval_ret="$?"
+	eval >&3 2>&4 $set_x "$1" '
+		eval_ret=$?
+		$set_nox'
 	return 0
 }
 
@@ -540,7 +548,6 @@ test_done () {
 
 # Test the binaries we have just built.  The tests are kept in
 # t/ subdirectory and are run in 'trash directory' subdirectory.
-TEST_DIRECTORY=$(pwd)
 if test -z "$valgrind"
 then
 	if test -z "$GIT_TEST_INSTALLED"