From: Tomi Ollila <tomi.ollila@iki.fi>
To: notmuch@notmuchmail.org
Subject: [PATCH 1/2] test/test-lib.sh: use $test_subtest_name in all tests
Date: Mon, 21 Jan 2013 05:01:45 +0200
Message-Id: <1358737306-21744-1-git-send-email-tomi.ollila@iki.fi>
In-Reply-To: <id:1358717493-11231-1-git-send-email-tomi.ollila@iki.fi>
References: <id:1358717493-11231-1-git-send-email-tomi.ollila@iki.fi>
Cc: tomi.ollila@iki.fi
Precedence: list

Set the variable '$test_subtest_name' in all functions which starts
a new test and use that variable in all functions that output
test results.

Additionally output the latest '$test_subtest_name' in case of
abnormal exit, to avoid confusion.
---

This obsoletes id:1358717806-11376-1-git-send-email-tomi.ollila@iki.fi
which had fd:s in function 'die' wrong order in 'exec [1]>&5' line.
I did plenty of (ad hoc) hand-testing for this but failed to notice that
messages weren't always as verbose as those should have been. The wip
patch set mentioned below has it right but this was hand-rewritten as
this is somewhat different here...

The main reason to do this change is to get latest '$test_subtest_name'
printed in case of abnormal exit. I cherry-picked this change from a
larger work-in-progress patch set that adds 'set -e -o pipefail' support...

I am pretty sure I got all the cases covered. If not, we'll notice
it later when some test fail in a way I could not anticipate.
Anyway, tests success & fail as they used to be.

 test/test-lib.sh | 52 +++++++++++++++++++++++++++++-----------------------
 1 file changed, 29 insertions(+), 23 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 6ce3b31..0098bfd 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -197,7 +197,11 @@ die () {
 	then
 		exit $code
 	else
-		echo >&5 "FATAL: Unexpected exit with code $code"
+		exec >&5
+		say_color error '%-6s' FATAL
+		echo " $test_subtest_name"
+		echo
+		echo "Unexpected exit while executing $0. Exit code $code."
 		exit 1
 	fi
 }
@@ -494,12 +498,12 @@ test_expect_equal ()
 	if ! test_skip "$test_subtest_name"
 	then
 		if [ "$output" = "$expected" ]; then
-			test_ok_ "$test_subtest_name"
+			test_ok_
 		else
 			testname=$this_test.$test_count
 			echo "$expected" > $testname.expected
 			echo "$output" > $testname.output
-			test_failure_ "$test_subtest_name" "$(diff -u $testname.expected $testname.output)"
+			test_failure_ "$(diff -u $testname.expected $testname.output)"
 		fi
     fi
 }
@@ -520,12 +524,12 @@ test_expect_equal_file ()
 	if ! test_skip "$test_subtest_name"
 	then
 		if diff -q "$file1" "$file2" >/dev/null ; then
-			test_ok_ "$test_subtest_name"
+			test_ok_
 		else
 			testname=$this_test.$test_count
 			cp "$file1" "$testname.$basename1"
 			cp "$file2" "$testname.$basename2"
-			test_failure_ "$test_subtest_name" "$(diff -u "$testname.$basename1" "$testname.$basename2")"
+			test_failure_ "$(diff -u "$testname.$basename1" "$testname.$basename2")"
 		fi
     fi
 }
@@ -563,9 +567,9 @@ test_emacs_expect_t () {
 		result=$(cat OUTPUT)
 		if [ "$result" = t ]
 		then
-			test_ok_ "$test_subtest_name"
+			test_ok_
 		else
-			test_failure_ "$test_subtest_name" "${result}"
+			test_failure_ "${result}"
 		fi
 	else
 		# Restore state after the (non) test.
@@ -666,12 +670,12 @@ test_require_external_prereq () {
 
 test_ok_ () {
 	if test "$test_subtest_known_broken_" = "t"; then
-		test_known_broken_ok_ "$@"
+		test_known_broken_ok_
 		return
 	fi
 	test_success=$(($test_success + 1))
 	say_color pass "%-6s" "PASS"
-	echo " $@"
+	echo " $test_subtest_name"
 }
 
 test_failure_ () {
@@ -680,7 +684,7 @@ test_failure_ () {
 		return
 	fi
 	test_failure=$(($test_failure + 1))
-	test_failure_message_ "FAIL" "$@"
+	test_failure_message_ "FAIL" "$test_subtest_name" "$@"
 	test "$immediate" = "" || { GIT_EXIT_OK=t; exit 1; }
 	return 1
 }
@@ -697,13 +701,13 @@ test_known_broken_ok_ () {
 	test_reset_state_
 	test_fixed=$(($test_fixed+1))
 	say_color pass "%-6s" "FIXED"
-	echo " $@"
+	echo " $test_subtest_name"
 }
 
 test_known_broken_failure_ () {
 	test_reset_state_
 	test_broken=$(($test_broken+1))
-	test_failure_message_ "BROKEN" "$@"
+	test_failure_message_ "BROKEN" "$test_subtest_name" "$@"
 	return 1
 }
 
@@ -771,6 +775,7 @@ test_expect_success () {
 	test "$#" = 3 && { prereq=$1; shift; } || prereq=
 	test "$#" = 2 ||
 	error "bug in the test script: not 2 or 3 parameters to test-expect-success"
+	test_subtest_name="$1"
 	test_reset_state_
 	if ! test_skip "$@"
 	then
@@ -780,9 +785,9 @@ test_expect_success () {
 		test_check_missing_external_prereqs_ "$@" ||
 		if [ "$run_ret" = 0 -a "$eval_ret" = 0 ]
 		then
-			test_ok_ "$1"
+			test_ok_
 		else
-			test_failure_ "$@"
+			test_failure_ "$2"
 		fi
 	fi
 }
@@ -791,6 +796,7 @@ test_expect_code () {
 	test "$#" = 4 && { prereq=$1; shift; } || prereq=
 	test "$#" = 3 ||
 	error "bug in the test script: not 3 or 4 parameters to test-expect-code"
+	test_subtest_name="$2"
 	test_reset_state_
 	if ! test_skip "$@"
 	then
@@ -800,9 +806,9 @@ test_expect_code () {
 		test_check_missing_external_prereqs_ "$@" ||
 		if [ "$run_ret" = 0 -a "$eval_ret" = "$1" ]
 		then
-			test_ok_ "$2"
+			test_ok_
 		else
-			test_failure_ "$@"
+			test_failure_ "exit code $eval_ret, expected $1" "$3"
 		fi
 	fi
 }
@@ -819,10 +825,10 @@ test_external () {
 	test "$#" = 4 && { prereq=$1; shift; } || prereq=
 	test "$#" = 3 ||
 	error >&5 "bug in the test script: not 3 or 4 parameters to test_external"
-	descr="$1"
+	test_subtest_name="$1"
 	shift
 	test_reset_state_
-	if ! test_skip "$descr" "$@"
+	if ! test_skip "$test_subtest_name" "$@"
 	then
 		# Announce the script to reduce confusion about the
 		# test output that follows.
@@ -833,9 +839,9 @@ test_external () {
 		"$@" 2>&4
 		if [ "$?" = 0 ]
 		then
-			test_ok_ "$descr"
+			test_ok_
 		else
-			test_failure_ "$descr" "$@"
+			test_failure_ "$@"
 		fi
 	fi
 }
@@ -849,11 +855,11 @@ test_external_without_stderr () {
 	stderr="$tmp/git-external-stderr.$$.tmp"
 	test_external "$@" 4> "$stderr"
 	[ -f "$stderr" ] || error "Internal error: $stderr disappeared."
-	descr="no stderr: $1"
+	test_subtest_name="no stderr: $1"
 	shift
 	if [ ! -s "$stderr" ]; then
 		rm "$stderr"
-		test_ok_ "$descr"
+		test_ok_
 	else
 		if [ "$verbose" = t ]; then
 			output=`echo; echo Stderr is:; cat "$stderr"`
@@ -862,7 +868,7 @@ test_external_without_stderr () {
 		fi
 		# rm first in case test_failure exits.
 		rm "$stderr"
-		test_failure_ "$descr" "$@" "$output"
+		test_failure_ "$@" "$output"
 	fi
 }
 
-- 
1.8.0