[PATCH 1/2] test/test-lib.sh: use $test_subtest_name in all tests

[PATCH 1/2] test/test-lib.sh: use $test_subtest_name in all tests

[Tomi Ollila]

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

[PATCH 2/2] test/test-lib.sh: separate signaled exit

[Tomi Ollila]

When execution of tests is interrupted by signal coming outside of the
test system itself, output just one line "interrupted by signal <num>"
message to standard output. This distinguishes the case from internal
exit and reduces noise.
---
 test/test-lib.sh | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 0098bfd..e717c52 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -190,9 +190,15 @@ test_fixed=0
 test_broken=0
 test_success=0
 
-die () {
+_die_common () {
 	code=$?
+	trap - EXIT
+	set +ex
 	rm -rf "$TEST_TMPDIR"
+}
+
+die () {
+	_die_common
 	if test -n "$GIT_EXIT_OK"
 	then
 		exit $code
@@ -206,10 +212,17 @@ die () {
 	fi
 }
 
+die_signal () {
+	_die_common
+	echo >&5 "FATAL: $0: interrupted by signal" $((code - 128))
+	exit $code
+}
+
 GIT_EXIT_OK=
 # Note: TEST_TMPDIR *NOT* exported!
 TEST_TMPDIR=$(mktemp -d "${TMPDIR:-/tmp}/notmuch-test-$$.XXXXXX")
 trap 'die' EXIT
+trap 'die_signal' HUP INT TERM
 
 test_decode_color () {
 	sed	-e 's/.\[1m/<WHITE>/g' \
-- 
1.8.0

Re: [PATCH 1/2] test/test-lib.sh: use $test_subtest_name in all tests

[Tomi Ollila]

On Mon, Jan 21 2013, Tomi Ollila <tomi.ollila@iki.fi> wrote:

> 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

How can this be so hard again, the patch that obsoleted is
id:1358717493-11231-1-git-send-email-tomi.ollila@iki.fi

Additionally, I tried to add that mail as reply-to to that
particular email but git-send-email is not smart enough to 
strip the 'id:' part from beginning of the Reply-To: field ;/.
Enter 'V' in notmuch show window to see what was written
into Reply-To: header ;/.

Tomi

Re: [PATCH 1/2] test/test-lib.sh: use $test_subtest_name in all tests

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:

> 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.

The series looks OK to me. The first patch is a bit noisy, but I guess
it results in simpler code.

d

Re: [PATCH 1/2] test/test-lib.sh: use $test_subtest_name in all tests

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:

> 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.

pushed both

d