test: cope better with use of return for errors

In olden times, tests would quietly exit the script when they failed
at an inconvenient moment, which was a little disconcerting.
Therefore v0.99.5~24^2~4 (Trapping exit in tests, using return for
errors, 2005-08-10) switched to an idiom of using "return" instead,
wrapping evaluation of test code in a function to make that safe:

	test_run_ () {
		eval >&3 2>&4 "$1"
		eval_ret="$?"
		return 0
	}

Years later, the implementation of test_when_finished (v1.7.1.1~95,
2010-05-02) and v1.7.2-rc2~1^2~13 (test-lib: output a newline before
"ok" under a TAP harness, 2010-06-24) took advantage of test_run_ as a
place to put code shared by all test assertion functions, without
paying attention to the function's former purpose:

	test_run_ () {
		...
		eval >&3 2>&4 "$1"
		eval_ret=$?

		if should run cleanup
		then
			eval >&3 2>&4 "$test_cleanup"
		fi
		if TAP format requires a newline here
		then
			echo
		fi
		return 0
	}

That means cleanup commands and the newline to put TAP output at
column 0 are skipped when tests use "return" to fail early.  Fix it by
introducing a test_eval_ function to catch the "return", with a
comment explaining the new function's purpose for the next person who
might touch this code.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

1 files changed, 8 insertions, 2 deletions

diff --git a/t/test-lib.sh b/t/test-lib.sh
index b16a9b9..57c3d53 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -444,15 +444,21 @@ test_debug () {
 	test "$debug" = "" || eval "$1"
 }
 
+test_eval_ () {
+	# This is a separate function because some tests use
+	# "return" to end a test_expect_success block early.
+	eval >&3 2>&4 "$*"
+}
+
 test_run_ () {
 	test_cleanup=:
 	expecting_failure=$2
-	eval >&3 2>&4 "$1"
+	test_eval_ "$1"
 	eval_ret=$?
 
 	if test -z "$immediate" || test $eval_ret = 0 || test -n "$expecting_failure"
 	then
-		eval >&3 2>&4 "$test_cleanup"
+		test_eval_ "$test_cleanup"
 	fi
 	if test "$verbose" = "t" && test -n "$HARNESS_ACTIVE"; then
 		echo ""