unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH 1/2] test/test-lib.sh: use $test_subtest_name in all tests
       [not found] <id:1358717493-11231-1-git-send-email-tomi.ollila@iki.fi>
@ 2013-01-21  3:01 ` Tomi Ollila
  2013-01-21  3:01   ` [PATCH 2/2] test/test-lib.sh: separate signaled exit Tomi Ollila
                     ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Tomi Ollila @ 2013-01-21  3:01 UTC (permalink / raw)
  To: notmuch; +Cc: 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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] test/test-lib.sh: separate signaled exit
  2013-01-21  3:01 ` [PATCH 1/2] test/test-lib.sh: use $test_subtest_name in all tests Tomi Ollila
@ 2013-01-21  3:01   ` Tomi Ollila
  2013-01-21  3:07   ` [PATCH 1/2] test/test-lib.sh: use $test_subtest_name in all tests Tomi Ollila
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Tomi Ollila @ 2013-01-21  3:01 UTC (permalink / raw)
  To: notmuch; +Cc: 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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] test/test-lib.sh: use $test_subtest_name in all tests
  2013-01-21  3:01 ` [PATCH 1/2] test/test-lib.sh: use $test_subtest_name in all tests Tomi Ollila
  2013-01-21  3:01   ` [PATCH 2/2] test/test-lib.sh: separate signaled exit Tomi Ollila
@ 2013-01-21  3:07   ` Tomi Ollila
  2013-02-16 14:11   ` David Bremner
  2013-02-19  1:12   ` David Bremner
  3 siblings, 0 replies; 5+ messages in thread
From: Tomi Ollila @ 2013-01-21  3:07 UTC (permalink / raw)
  To: notmuch

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] test/test-lib.sh: use $test_subtest_name in all tests
  2013-01-21  3:01 ` [PATCH 1/2] test/test-lib.sh: use $test_subtest_name in all tests Tomi Ollila
  2013-01-21  3:01   ` [PATCH 2/2] test/test-lib.sh: separate signaled exit Tomi Ollila
  2013-01-21  3:07   ` [PATCH 1/2] test/test-lib.sh: use $test_subtest_name in all tests Tomi Ollila
@ 2013-02-16 14:11   ` David Bremner
  2013-02-19  1:12   ` David Bremner
  3 siblings, 0 replies; 5+ messages in thread
From: David Bremner @ 2013-02-16 14:11 UTC (permalink / raw)
  To: Tomi Ollila, notmuch; +Cc: tomi.ollila

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] test/test-lib.sh: use $test_subtest_name in all tests
  2013-01-21  3:01 ` [PATCH 1/2] test/test-lib.sh: use $test_subtest_name in all tests Tomi Ollila
                     ` (2 preceding siblings ...)
  2013-02-16 14:11   ` David Bremner
@ 2013-02-19  1:12   ` David Bremner
  3 siblings, 0 replies; 5+ messages in thread
From: David Bremner @ 2013-02-19  1:12 UTC (permalink / raw)
  To: Tomi Ollila, notmuch; +Cc: tomi.ollila

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-02-19  1:12 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <id:1358717493-11231-1-git-send-email-tomi.ollila@iki.fi>
2013-01-21  3:01 ` [PATCH 1/2] test/test-lib.sh: use $test_subtest_name in all tests Tomi Ollila
2013-01-21  3:01   ` [PATCH 2/2] test/test-lib.sh: separate signaled exit Tomi Ollila
2013-01-21  3:07   ` [PATCH 1/2] test/test-lib.sh: use $test_subtest_name in all tests Tomi Ollila
2013-02-16 14:11   ` David Bremner
2013-02-19  1:12   ` David Bremner

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).