unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH 0/9] test: cleanup and refactoring
@ 2017-02-26 13:42 Jani Nikula
  2017-02-26 13:42 ` [PATCH 1/9] test: remove unused regexp convenience variables Jani Nikula
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Jani Nikula @ 2017-02-26 13:42 UTC (permalink / raw)
  To: notmuch

The first half is cleanups, throwing out unused stuff.

The second half requires test_begin_subtest before *all* subtests.

BR,
Jani.


Jani Nikula (9):
  test: remove unused regexp convenience variables
  test: remove unused filter functions
  test: remove unused test_external and test_external_without_stderr
  test: remove unused and no-op --long-tests parameter
  test: only accept short and long options, not silly in-betweens
  test: ensure test_begin_subtest has been called before test_expect_*
  test: drop the implicit prereq check mechanism from test_expect_*
  test: require test_begin_subtest before test_expect_success
  test: require test_begin_subtest before test_expect_code

 test/README                    |  17 ++--
 test/T000-basic.sh             |  44 ++++-------
 test/T010-help-test.sh         |  27 ++++---
 test/T020-compact.sh           |  10 ++-
 test/T050-new.sh               |   4 +-
 test/T070-insert.sh            |  34 ++++----
 test/T150-tagging.sh           |  13 ++-
 test/T160-json.sh              |   8 +-
 test/T190-multipart.sh         |  20 ++---
 test/T240-dump-restore.sh      |  33 +++++---
 test/T310-emacs.sh             |   9 +--
 test/T340-maildir-sync.sh      |   6 +-
 test/T350-crypto.sh            |   9 ++-
 test/T355-smime.sh             |   6 +-
 test/T380-atomicity.sh         |   3 +-
 test/T400-hooks.sh             |  18 +++--
 test/T530-upgrade.sh           |   5 +-
 test/T560-lib-error.sh         |   3 +-
 test/T570-revision-tracking.sh |  32 ++++----
 test/T600-named-queries.sh     |  12 +--
 test/test-lib.sh               | 176 ++++++++++-------------------------------
 test/test-verbose              |   6 +-
 22 files changed, 217 insertions(+), 278 deletions(-)

-- 
2.11.0

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

* [PATCH 1/9] test: remove unused regexp convenience variables
  2017-02-26 13:42 [PATCH 0/9] test: cleanup and refactoring Jani Nikula
@ 2017-02-26 13:42 ` Jani Nikula
  2017-02-26 13:42 ` [PATCH 2/9] test: remove unused filter functions Jani Nikula
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jani Nikula @ 2017-02-26 13:42 UTC (permalink / raw)
  To: notmuch

They've been unused since their introduction in commit 0083854b1204
("Copy test framework from Git").
---
 test/test-lib.sh | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index d8e159437ca9..b136fb949ec1 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -93,15 +93,6 @@ unset GREP_OPTIONS
 # For emacsclient
 unset ALTERNATE_EDITOR
 
-# Convenience
-#
-# A regexp to match 5 and 40 hexdigits
-_x05='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
-_x40="$_x05$_x05$_x05$_x05$_x05$_x05$_x05$_x05"
-
-_x04='[0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
-_x32="$_x04$_x04$_x04$_x04$_x04$_x04$_x04$_x04"
-
 # Each test should start with something like this, after copyright notices:
 #
 # test_description='Description of this test...
-- 
2.11.0

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

* [PATCH 2/9] test: remove unused filter functions
  2017-02-26 13:42 [PATCH 0/9] test: cleanup and refactoring Jani Nikula
  2017-02-26 13:42 ` [PATCH 1/9] test: remove unused regexp convenience variables Jani Nikula
@ 2017-02-26 13:42 ` Jani Nikula
  2017-02-26 13:42 ` [PATCH 3/9] test: remove unused test_external and test_external_without_stderr Jani Nikula
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jani Nikula @ 2017-02-26 13:42 UTC (permalink / raw)
  To: notmuch

They've been unused since their introduction in commit 0083854b1204
("Copy test framework from Git").
---
 test/test-lib.sh | 27 ---------------------------
 1 file changed, 27 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index b136fb949ec1..2ccb62eec122 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -274,33 +274,6 @@ TEST_TMPDIR=$(mktemp -d "${TMPDIR:-/tmp}/notmuch-test-$$.XXXXXX")
 trap 'trap_exit' EXIT
 trap 'trap_signal' HUP INT TERM
 
-test_decode_color () {
-	sed	-e 's/.\[1m/<WHITE>/g' \
-		-e 's/.\[31m/<RED>/g' \
-		-e 's/.\[32m/<GREEN>/g' \
-		-e 's/.\[33m/<YELLOW>/g' \
-		-e 's/.\[34m/<BLUE>/g' \
-		-e 's/.\[35m/<MAGENTA>/g' \
-		-e 's/.\[36m/<CYAN>/g' \
-		-e 's/.\[m/<RESET>/g'
-}
-
-q_to_nul () {
-	perl -pe 'y/Q/\000/'
-}
-
-q_to_cr () {
-	tr Q '\015'
-}
-
-append_cr () {
-	sed -e 's/$/Q/' | tr Q '\015'
-}
-
-remove_cr () {
-	tr '\015' Q | sed -e 's/Q$//'
-}
-
 # Generate a new message in the mail directory, with a unique message
 # ID and subject. The message is not added to the index.
 #
-- 
2.11.0

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

* [PATCH 3/9] test: remove unused test_external and test_external_without_stderr
  2017-02-26 13:42 [PATCH 0/9] test: cleanup and refactoring Jani Nikula
  2017-02-26 13:42 ` [PATCH 1/9] test: remove unused regexp convenience variables Jani Nikula
  2017-02-26 13:42 ` [PATCH 2/9] test: remove unused filter functions Jani Nikula
@ 2017-02-26 13:42 ` Jani Nikula
  2017-02-26 13:42 ` [PATCH 4/9] test: remove unused and no-op --long-tests parameter Jani Nikula
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jani Nikula @ 2017-02-26 13:42 UTC (permalink / raw)
  To: notmuch

They've been unused since their introduction in commit 0083854b1204
("Copy test framework from Git"), only causing maintenance burden.
---
 test/test-lib.sh | 59 --------------------------------------------------------
 1 file changed, 59 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 2ccb62eec122..218a06527f88 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -963,65 +963,6 @@ test_expect_code () {
 	fi
 }
 
-# test_external runs external test scripts that provide continuous
-# test output about their progress, and succeeds/fails on
-# zero/non-zero exit code.  It outputs the test output on stdout even
-# in non-verbose mode, and announces the external script with "* run
-# <n>: ..." before running it.  When providing relative paths, keep in
-# mind that all scripts run in "trash directory".
-# Usage: test_external description command arguments...
-# Example: test_external 'Perl API' perl ../path/to/test.pl
-test_external () {
-	test "$#" = 4 && { prereq=$1; shift; } || prereq=
-	test "$#" = 3 ||
-	error >&6 "bug in the test script: not 3 or 4 parameters to test_external"
-	test_subtest_name="$1"
-	shift
-	test_reset_state_
-	if ! test_skip "$test_subtest_name" "$@"
-	then
-		# Announce the script to reduce confusion about the
-		# test output that follows.
-		say_color "" " run $test_count: $descr ($*)"
-		# Run command; redirect its stderr to &4 as in
-		# test_run_, but keep its stdout on our stdout even in
-		# non-verbose mode.
-		"$@" 2>&4
-		if [ "$?" = 0 ]
-		then
-			test_ok_
-		else
-			test_failure_ "$@"
-		fi
-	fi
-}
-
-# Like test_external, but in addition tests that the command generated
-# no output on stderr.
-test_external_without_stderr () {
-	# The temporary file has no (and must have no) security
-	# implications.
-	tmp="$TMPDIR"; if [ -z "$tmp" ]; then tmp=/tmp; fi
-	stderr="$tmp/git-external-stderr.$$.tmp"
-	test_external "$@" 4> "$stderr"
-	[ -f "$stderr" ] || error "Internal error: $stderr disappeared."
-	test_subtest_name="no stderr: $1"
-	shift
-	if [ ! -s "$stderr" ]; then
-		rm "$stderr"
-		test_ok_
-	else
-		if [ "$verbose" = t ]; then
-			output=`echo; echo Stderr is:; cat "$stderr"`
-		else
-			output=
-		fi
-		# rm first in case test_failure exits.
-		rm "$stderr"
-		test_failure_ "$@" "$output"
-	fi
-}
-
 # This is not among top-level (test_expect_success)
 # but is a prefix that can be used in the test script, like:
 #
-- 
2.11.0

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

* [PATCH 4/9] test: remove unused and no-op --long-tests parameter
  2017-02-26 13:42 [PATCH 0/9] test: cleanup and refactoring Jani Nikula
                   ` (2 preceding siblings ...)
  2017-02-26 13:42 ` [PATCH 3/9] test: remove unused test_external and test_external_without_stderr Jani Nikula
@ 2017-02-26 13:42 ` Jani Nikula
  2017-02-26 13:42 ` [PATCH 5/9] test: only accept short and long options, not silly in-betweens Jani Nikula
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jani Nikula @ 2017-02-26 13:42 UTC (permalink / raw)
  To: notmuch

It's been unused since its introduction in commit 0083854b1204 ("Copy
test framework from Git").
---
 test/test-lib.sh | 2 --
 1 file changed, 2 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 218a06527f88..44c510517f97 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -117,8 +117,6 @@ do
 		debug=t; shift ;;
 	-i|--i|--im|--imm|--imme|--immed|--immedi|--immedia|--immediat|--immediate)
 		immediate=t; shift ;;
-	-l|--l|--lo|--lon|--long|--long-|--long-t|--long-te|--long-tes|--long-test|--long-tests)
-		GIT_TEST_LONG=t; export GIT_TEST_LONG; shift ;;
 	-h|--h|--he|--hel|--help)
 		help=t; shift ;;
 	-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
-- 
2.11.0

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

* [PATCH 5/9] test: only accept short and long options, not silly in-betweens
  2017-02-26 13:42 [PATCH 0/9] test: cleanup and refactoring Jani Nikula
                   ` (3 preceding siblings ...)
  2017-02-26 13:42 ` [PATCH 4/9] test: remove unused and no-op --long-tests parameter Jani Nikula
@ 2017-02-26 13:42 ` Jani Nikula
  2017-02-26 13:42 ` [PATCH 6/9] test: ensure test_begin_subtest has been called before test_expect_* Jani Nikula
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jani Nikula @ 2017-02-26 13:42 UTC (permalink / raw)
  To: notmuch

It's not notmuch style to accept sloppy parameter names.
---
 test/test-lib.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 44c510517f97..0a486f4cde9a 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -113,15 +113,15 @@ unset ALTERNATE_EDITOR
 while test "$#" -ne 0
 do
 	case "$1" in
-	-d|--d|--de|--deb|--debu|--debug)
+	-d|--debug)
 		debug=t; shift ;;
-	-i|--i|--im|--imm|--imme|--immed|--immedi|--immedia|--immediat|--immediate)
+	-i|--immediate)
 		immediate=t; shift ;;
-	-h|--h|--he|--hel|--help)
+	-h|--help)
 		help=t; shift ;;
-	-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
+	-v|--verbose)
 		verbose=t; shift ;;
-	-q|--q|--qu|--qui|--quie|--quiet)
+	-q|--quiet)
 		quiet=t; shift ;;
 	--with-dashes)
 		with_dashes=t; shift ;;
@@ -130,7 +130,7 @@ do
 	--no-python)
 		# noop now...
 		shift ;;
-	--va|--val|--valg|--valgr|--valgri|--valgrin|--valgrind)
+	--valgrind)
 		valgrind=t; verbose=t; shift ;;
 	--tee)
 		shift ;; # was handled already
-- 
2.11.0

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

* [PATCH 6/9] test: ensure test_begin_subtest has been called before test_expect_*
  2017-02-26 13:42 [PATCH 0/9] test: cleanup and refactoring Jani Nikula
                   ` (4 preceding siblings ...)
  2017-02-26 13:42 ` [PATCH 5/9] test: only accept short and long options, not silly in-betweens Jani Nikula
@ 2017-02-26 13:42 ` Jani Nikula
  2017-02-26 13:42 ` [PATCH 7/9] test: drop the implicit prereq check mechanism from test_expect_* Jani Nikula
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jani Nikula @ 2017-02-26 13:42 UTC (permalink / raw)
  To: notmuch

This is the expectation, increase robustness of the test suite by
requiring it.
---
 test/test-lib.sh | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 0a486f4cde9a..056483c47c0a 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -566,6 +566,9 @@ test_begin_subtest ()
 test_expect_equal ()
 {
 	exec 1>&6 2>&7		# Restore stdout and stderr
+	if [ -z "$inside_subtest" ]; then
+		error "bug in the test script: test_expect_equal without test_begin_subtest"
+	fi
 	inside_subtest=
 	test "$#" = 3 && { prereq=$1; shift; } || prereq=
 	test "$#" = 2 ||
@@ -590,6 +593,9 @@ test_expect_equal ()
 test_expect_equal_file ()
 {
 	exec 1>&6 2>&7		# Restore stdout and stderr
+	if [ -z "$inside_subtest" ]; then
+		error "bug in the test script: test_expect_equal_file without test_begin_subtest"
+	fi
 	inside_subtest=
 	test "$#" = 3 && { prereq=$1; shift; } || prereq=
 	test "$#" = 2 ||
@@ -637,6 +643,9 @@ test_emacs_expect_t () {
 	test "$#" = 2 && { prereq=$1; shift; } || prereq=
 	test "$#" = 1 ||
 	error "bug in the test script: not 1 or 2 parameters to test_emacs_expect_t"
+	if [ -z "$inside_subtest" ]; then
+		error "bug in the test script: test_emacs_expect_t without test_begin_subtest"
+	fi
 
 	# Run the test.
 	if ! test_skip "$test_subtest_name"
-- 
2.11.0

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

* [PATCH 7/9] test: drop the implicit prereq check mechanism from test_expect_*
  2017-02-26 13:42 [PATCH 0/9] test: cleanup and refactoring Jani Nikula
                   ` (5 preceding siblings ...)
  2017-02-26 13:42 ` [PATCH 6/9] test: ensure test_begin_subtest has been called before test_expect_* Jani Nikula
@ 2017-02-26 13:42 ` Jani Nikula
  2017-02-26 13:43 ` [PATCH 8/9] test: require test_begin_subtest before test_expect_success Jani Nikula
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Jani Nikula @ 2017-02-26 13:42 UTC (permalink / raw)
  To: notmuch

The only place where we use the implicit prereq check is T000-basic.sh
where we check that it works. It's an added complication that we don't
use. Remove it.

The test_have_prereq function can still be used for the same effect in
subtests that use test_begin_subtest. For now, this will make it
impossible to have prereqs in one-line subtests that don't require
test_begin_subtest. This will be fixed in follow-up work.
---
 test/T000-basic.sh |  2 +-
 test/test-lib.sh   | 26 ++++++--------------------
 2 files changed, 7 insertions(+), 21 deletions(-)

diff --git a/test/T000-basic.sh b/test/T000-basic.sh
index 0a8d6cdf40fc..78e183361e7d 100755
--- a/test/T000-basic.sh
+++ b/test/T000-basic.sh
@@ -23,7 +23,7 @@ test_expect_success 'success is reported like this' '
 '
 test_set_prereq HAVEIT
 haveit=no
-test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '
+test_expect_success 'test runs if prerequisite is satisfied' '
     test_have_prereq HAVEIT &&
     haveit=yes
 '
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 056483c47c0a..3a69c399f6eb 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -570,9 +570,8 @@ test_expect_equal ()
 		error "bug in the test script: test_expect_equal without test_begin_subtest"
 	fi
 	inside_subtest=
-	test "$#" = 3 && { prereq=$1; shift; } || prereq=
 	test "$#" = 2 ||
-	error "bug in the test script: not 2 or 3 parameters to test_expect_equal"
+	error "bug in the test script: not 2 parameters to test_expect_equal"
 
 	output="$1"
 	expected="$2"
@@ -597,9 +596,8 @@ test_expect_equal_file ()
 		error "bug in the test script: test_expect_equal_file without test_begin_subtest"
 	fi
 	inside_subtest=
-	test "$#" = 3 && { prereq=$1; shift; } || prereq=
 	test "$#" = 2 ||
-	error "bug in the test script: not 2 or 3 parameters to test_expect_equal"
+	error "bug in the test script: not 2 parameters to test_expect_equal_file"
 
 	file1="$1"
 	file2="$2"
@@ -640,9 +638,8 @@ test_sort_json () {
 }
 
 test_emacs_expect_t () {
-	test "$#" = 2 && { prereq=$1; shift; } || prereq=
 	test "$#" = 1 ||
-	error "bug in the test script: not 1 or 2 parameters to test_emacs_expect_t"
+	error "bug in the test script: not 1 parameter to test_emacs_expect_t"
 	if [ -z "$inside_subtest" ]; then
 		error "bug in the test script: test_emacs_expect_t without test_begin_subtest"
 	fi
@@ -757,12 +754,8 @@ notmuch_config_sanitize ()
 # End of notmuch helper functions
 
 # Use test_set_prereq to tell that a particular prerequisite is available.
-# The prerequisite can later be checked for in two ways:
 #
-# - Explicitly using test_have_prereq.
-#
-# - Implicitly by specifying the prerequisite tag in the calls to
-#   test_expect_{success,failure,code}.
+# The prerequisite can later be checked for by using test_have_prereq.
 #
 # The single parameter is the prerequisite tag (a simple word, in all
 # capital letters by convention).
@@ -891,11 +884,6 @@ test_skip () {
 			break
 		esac
 	done
-	if test -z "$to_skip" && test -n "$prereq" &&
-	   ! test_have_prereq "$prereq"
-	then
-		to_skip=t
-	fi
 	case "$to_skip" in
 	t)
 		test_report_skip_ "$@"
@@ -929,9 +917,8 @@ test_subtest_known_broken () {
 }
 
 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"
+	error "bug in the test script: not 2 parameters to test_expect_success"
 	test_subtest_name="$1"
 	test_reset_state_
 	if ! test_skip "$@"
@@ -950,9 +937,8 @@ test_expect_success () {
 }
 
 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"
+	error "bug in the test script: not 3 parameters to test_expect_code"
 	test_subtest_name="$2"
 	test_reset_state_
 	if ! test_skip "$@"
-- 
2.11.0

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

* [PATCH 8/9] test: require test_begin_subtest before test_expect_success
  2017-02-26 13:42 [PATCH 0/9] test: cleanup and refactoring Jani Nikula
                   ` (6 preceding siblings ...)
  2017-02-26 13:42 ` [PATCH 7/9] test: drop the implicit prereq check mechanism from test_expect_* Jani Nikula
@ 2017-02-26 13:43 ` Jani Nikula
  2017-02-26 13:43 ` [PATCH 9/9] test: require test_begin_subtest before test_expect_code Jani Nikula
  2017-03-10 11:45 ` [PATCH 0/9] test: cleanup and refactoring David Bremner
  9 siblings, 0 replies; 11+ messages in thread
From: Jani Nikula @ 2017-02-26 13:43 UTC (permalink / raw)
  To: notmuch

Unify the subtests by requiring test_begin_subtest before
test_expect_success. (Similar change for test_expect_code will
follow.)

This increases clarity in the test scripts by having a separate line
for the start of the subtest with the heading, and makes it possible
to simplify the test infrastructure by making all subtests similar.
---
 test/README                    | 14 +++++++-------
 test/T000-basic.sh             | 33 +++++++++++++--------------------
 test/T010-help-test.sh         | 27 ++++++++++++++++++---------
 test/T020-compact.sh           |  7 ++++---
 test/T190-multipart.sh         | 20 ++++++++------------
 test/T240-dump-restore.sh      | 33 ++++++++++++++++++++-------------
 test/T310-emacs.sh             |  9 ++++-----
 test/T340-maildir-sync.sh      |  6 ++++--
 test/T350-crypto.sh            |  9 ++++++---
 test/T355-smime.sh             |  6 ++++--
 test/T380-atomicity.sh         |  3 ++-
 test/T400-hooks.sh             |  4 ++--
 test/T530-upgrade.sh           |  5 +++--
 test/T560-lib-error.sh         |  3 ++-
 test/T570-revision-tracking.sh | 16 ++++++++--------
 test/T600-named-queries.sh     |  8 ++++----
 test/test-lib.sh               | 18 +++++++++++-------
 test/test-verbose              |  6 ++++--
 18 files changed, 124 insertions(+), 103 deletions(-)

diff --git a/test/README b/test/README
index 104a120ea28b..7acdb4b81f4d 100644
--- a/test/README
+++ b/test/README
@@ -189,17 +189,17 @@ Test harness library
 There are a handful helper functions defined in the test harness
 library for your script to use.
 
- test_expect_success <message> <script>
-
-   This takes two strings as parameter, and evaluates the
-   <script>.  If it yields success, test is considered
-   successful.  <message> should state what it is testing.
-
  test_begin_subtest <message>
 
-   Set the test description message for a subsequent test_expect_equal
+   Set the test description message for a subsequent test_expect_*
    invocation (see below).
 
+ test_expect_success <script>
+
+   This takes a string as parameter, and evaluates the
+   <script>.  If it yields success, test is considered
+   successful.
+
  test_subtest_known_broken
 
    Mark the current test as broken.  Such tests are expected to fail.
diff --git a/test/T000-basic.sh b/test/T000-basic.sh
index 78e183361e7d..003ab95f5681 100755
--- a/test/T000-basic.sh
+++ b/test/T000-basic.sh
@@ -18,20 +18,17 @@ fi
 
 ################################################################
 # Test harness
-test_expect_success 'success is reported like this' '
-    :
-'
+test_begin_subtest 'success is reported like this'
+test_expect_success ':'
+
+test_begin_subtest 'test runs if prerequisite is satisfied'
 test_set_prereq HAVEIT
 haveit=no
-test_expect_success 'test runs if prerequisite is satisfied' '
-    test_have_prereq HAVEIT &&
-    haveit=yes
-'
+test_expect_success 'test_have_prereq HAVEIT && haveit=yes'
 
+test_begin_subtest 'tests clean up after themselves'
 clean=no
-test_expect_success 'tests clean up after themselves' '
-    test_when_finished clean=yes
-'
+test_expect_success 'test_when_finished clean=yes'
 
 cleaner=no
 test_expect_code 1 'tests clean up even after a failure' '
@@ -72,19 +69,15 @@ test_expect_equal "$output" "$expected"
 ################################################################
 # Test mail store prepared in test-lib.sh
 
-test_expect_success \
-    'test that mail store was created' \
-    'test -d "${MAIL_DIR}"'
-
+test_begin_subtest 'test that mail store was created'
+test_expect_success 'test -d "${MAIL_DIR}"'
 
+test_begin_subtest 'mail store should be empty'
 find "${MAIL_DIR}" -type f -print >should-be-empty
-test_expect_success \
-    'mail store should be empty' \
-    'cmp -s /dev/null should-be-empty'
+test_expect_success 'cmp -s /dev/null should-be-empty'
 
-test_expect_success \
-    'NOTMUCH_CONFIG is set and points to an existing file' \
-    'test -f "${NOTMUCH_CONFIG}"'
+test_begin_subtest 'NOTMUCH_CONFIG is set and points to an existing file'
+test_expect_success 'test -f "${NOTMUCH_CONFIG}"'
 
 test_begin_subtest 'PATH is set to build directory'
 test_expect_equal \
diff --git a/test/T010-help-test.sh b/test/T010-help-test.sh
index c17323763112..0c833de26217 100755
--- a/test/T010-help-test.sh
+++ b/test/T010-help-test.sh
@@ -3,18 +3,27 @@
 test_description="online help"
 . ./test-lib.sh || exit 1
 
-test_expect_success 'notmuch --help' 'notmuch --help'
-test_expect_success 'notmuch help' 'notmuch help'
-test_expect_success 'notmuch --version' 'notmuch --version'
+test_begin_subtest 'notmuch --help'
+test_expect_success 'notmuch --help'
+
+test_begin_subtest 'notmuch help'
+test_expect_success 'notmuch help'
+
+test_begin_subtest 'notmuch --version'
+test_expect_success 'notmuch --version'
 
 if [ $NOTMUCH_HAVE_MAN -eq 1 ]; then
-    test_expect_success 'notmuch --help tag' 'notmuch --help tag'
-    test_expect_success 'notmuch help tag' 'notmuch help tag'
+    test_begin_subtest 'notmuch --help tag'
+    test_expect_success 'notmuch --help tag'
+
+    test_begin_subtest 'notmuch help tag'
+    test_expect_success 'notmuch help tag'
 else
-    test_expect_success 'notmuch --help tag (man pages not available)' \
-	'test_must_fail notmuch --help tag >/dev/null'
-    test_expect_success 'notmuch help tag (man pages not available)' \
-	'test_must_fail notmuch help tag >/dev/null'
+    test_begin_subtest 'notmuch --help tag (man pages not available)'
+    test_expect_success 'test_must_fail notmuch --help tag >/dev/null'
+
+    test_begin_subtest 'notmuch help tag (man pages not available)'
+    test_expect_success 'test_must_fail notmuch help tag >/dev/null'
 fi
 
 test_done
diff --git a/test/T020-compact.sh b/test/T020-compact.sh
index 8b4dbbc48189..1fecd071e77e 100755
--- a/test/T020-compact.sh
+++ b/test/T020-compact.sh
@@ -21,7 +21,8 @@ Compaction failed: Unsupported operation"
     test_done
 fi
 
-test_expect_success "Running compact" "notmuch compact --backup=${TEST_DIRECTORY}/xapian.old"
+test_begin_subtest "Running compact"
+test_expect_success "notmuch compact --backup=${TEST_DIRECTORY}/xapian.old"
 
 test_begin_subtest "Compact preserves database"
 output=$(notmuch search \* | notmuch_search_sanitize)
@@ -30,8 +31,8 @@ thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (inbox tag1 unread)
 thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 tag2 unread)
 thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Three (inbox tag3 unread)"
 
-test_expect_success 'Restoring Backup' \
-    'rm -Rf ${MAIL_DIR}/.notmuch/xapian &&
+test_begin_subtest "Restoring Backup"
+test_expect_success 'rm -Rf ${MAIL_DIR}/.notmuch/xapian &&
      mv ${TEST_DIRECTORY}/xapian.old ${MAIL_DIR}/.notmuch/xapian'
 
 test_begin_subtest "Checking restored backup"
diff --git a/test/T190-multipart.sh b/test/T190-multipart.sh
index 18eb7b839ac0..e92fedb37b5c 100755
--- a/test/T190-multipart.sh
+++ b/test/T190-multipart.sh
@@ -338,9 +338,8 @@ Non-text part: application/pgp-signature
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
-test_expect_success \
-    "--format=text --part=8, no part, expect error" \
-    "notmuch show --format=text --part=8 'id:87liy5ap00.fsf@yoom.home.cworth.org'"
+test_begin_subtest "--format=text --part=8, no part, expect error"
+test_expect_success "notmuch show --format=text --part=8 'id:87liy5ap00.fsf@yoom.home.cworth.org'"
 
 test_begin_subtest "--format=json --part=0, full message"
 notmuch show --format=json --part=0 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
@@ -440,9 +439,8 @@ cat <<EOF >EXPECTED
 EOF
 test_expect_equal_json "$(cat OUTPUT)" "$(cat EXPECTED)"
 
-test_expect_success \
-    "--format=json --part=10, no part, expect error" \
-    "notmuch show --format=json --part=10 'id:87liy5ap00.fsf@yoom.home.cworth.org'"
+test_begin_subtest "--format=json --part=10, no part, expect error"
+test_expect_success "notmuch show --format=json --part=10 'id:87liy5ap00.fsf@yoom.home.cworth.org'"
 
 test_begin_subtest "--format=raw"
 notmuch show --format=raw 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
@@ -576,9 +574,8 @@ W6cAmQE4dcYrx/LPLtYLZm1jsGauE5hE
 EOF
 test_expect_equal_file OUTPUT EXPECTED
 
-test_expect_success \
-    "--format=raw --part=10, no part, expect error" \
-    "notmuch show --format=raw --part=8 'id:87liy5ap00.fsf@yoom.home.cworth.org'"
+test_begin_subtest "--format=raw --part=10, no part, expect error"
+test_expect_success "notmuch show --format=raw --part=8 'id:87liy5ap00.fsf@yoom.home.cworth.org'"
 
 test_begin_subtest "--format=mbox"
 notmuch show --format=mbox 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
@@ -588,9 +585,8 @@ cat "${MAIL_DIR}"/multipart >>EXPECTED
 echo >>EXPECTED
 test_expect_equal_file OUTPUT EXPECTED
 
-test_expect_success \
-    "--format=mbox --part=1, incompatible, expect error" \
-    "! notmuch show --format=mbox --part=1 'id:87liy5ap00.fsf@yoom.home.cworth.org'"
+test_begin_subtest "--format=mbox --part=1, incompatible, expect error"
+test_expect_success "! notmuch show --format=mbox --part=1 'id:87liy5ap00.fsf@yoom.home.cworth.org'"
 
 test_begin_subtest "'notmuch reply' to a multipart message"
 notmuch reply 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
diff --git a/test/T240-dump-restore.sh b/test/T240-dump-restore.sh
index faa10364acf5..206c3ae14560 100755
--- a/test/T240-dump-restore.sh
+++ b/test/T240-dump-restore.sh
@@ -4,52 +4,58 @@ test_description="\"notmuch dump\" and \"notmuch restore\""
 
 add_email_corpus
 
-test_expect_success 'Dumping all tags' \
-  'generate_message &&
-  notmuch new &&
-  notmuch dump > dump.expected'
+test_begin_subtest "Dumping all tags"
+test_expect_success 'generate_message && notmuch new && notmuch dump > dump.expected'
 
 # The use of from:cworth is rather arbitrary: it matches some of the
 # email corpus' messages, but not all of them.
 
-test_expect_success 'Dumping all tags II' \
+test_begin_subtest "Dumping all tags II"
+test_expect_success \
   'notmuch tag +ABC +DEF -- from:cworth &&
   notmuch dump > dump-ABC_DEF.expected &&
   ! cmp dump.expected dump-ABC_DEF.expected'
 
-test_expect_success 'Clearing all tags' \
+test_begin_subtest "Clearing all tags"
+test_expect_success \
   'sed -e "s/(\([^(]*\))$/()/" < dump.expected > clear.expected &&
   notmuch restore --input=clear.expected &&
   notmuch dump > clear.actual &&
   test_cmp clear.expected clear.actual'
 
-test_expect_success 'Accumulate original tags' \
+test_begin_subtest "Clearing all tags"
+test_expect_success \
   'notmuch tag +ABC +DEF -- from:cworth &&
   notmuch restore --accumulate < dump.expected &&
   notmuch dump > dump.actual &&
   test_cmp dump-ABC_DEF.expected dump.actual'
 
-test_expect_success 'Restoring original tags' \
+test_begin_subtest "Restoring original tags"
+test_expect_success \
   'notmuch restore --input=dump.expected &&
   notmuch dump > dump.actual &&
   test_cmp dump.expected dump.actual'
 
-test_expect_success 'Restore with nothing to do' \
+test_begin_subtest "Restore with nothing to do"
+test_expect_success \
   'notmuch restore < dump.expected &&
   notmuch dump > dump.actual &&
   test_cmp dump.expected dump.actual'
 
-test_expect_success 'Accumulate with existing tags' \
+test_begin_subtest "Accumulate with existing tags"
+test_expect_success \
   'notmuch restore --accumulate --input=dump.expected &&
   notmuch dump > dump.actual &&
   test_cmp dump.expected dump.actual'
 
-test_expect_success 'Accumulate with no tags' \
+test_begin_subtest "Accumulate with no tags"
+test_expect_success \
   'notmuch restore --accumulate < clear.expected &&
   notmuch dump > dump.actual &&
   test_cmp dump.expected dump.actual'
 
-test_expect_success 'Accumulate with new tags' \
+test_begin_subtest "Accumulate with new tags"
+test_expect_success \
   'notmuch restore --input=dump.expected &&
   notmuch restore --accumulate --input=dump-ABC_DEF.expected &&
   notmuch dump >  OUTPUT.$test_count &&
@@ -57,7 +63,8 @@ test_expect_success 'Accumulate with new tags' \
   test_cmp dump-ABC_DEF.expected OUTPUT.$test_count'
 
 # notmuch restore currently only considers the first argument.
-test_expect_success 'Invalid restore invocation' \
+test_begin_subtest "Invalid restore invocation"
+test_expect_success \
   'test_must_fail notmuch restore --input=dump.expected another_one'
 
 test_begin_subtest "dump --output=outfile"
diff --git a/test/T310-emacs.sh b/test/T310-emacs.sh
index 01385ae81f40..5c6bb8938838 100755
--- a/test/T310-emacs.sh
+++ b/test/T310-emacs.sh
@@ -8,8 +8,8 @@ EXPECTED=$TEST_DIRECTORY/emacs.expected-output
 add_email_corpus
 
 # syntax errors in test-lib.el cause mysterious failures
-test_expect_success 'Syntax of emacs test library' \
-    "${TEST_EMACS} -Q --batch --load $TEST_DIRECTORY/test-lib.el"
+test_begin_subtest "Syntax of emacs test library"
+test_expect_success "${TEST_EMACS} -Q --batch --load $TEST_DIRECTORY/test-lib.el"
 
 test_begin_subtest "Basic notmuch-hello view in emacs"
 test_emacs '(notmuch-hello)
@@ -905,9 +905,8 @@ test_emacs "(let ((mm-text-html-renderer
 # Different Emacs versions and renderers give very different results,
 # so just check that something reasonable showed up.  We first cat the
 # output so the test framework will print it if the test fails.
-test_expect_success "Rendering HTML mail with images" \
-    'cat OUTPUT && grep -q smiley OUTPUT'
-
+test_begin_subtest "Rendering HTML mail with images"
+test_expect_success 'cat OUTPUT && grep -q smiley OUTPUT'
 
 test_begin_subtest "Search handles subprocess error exit codes"
 cat > notmuch_fail <<EOF
diff --git a/test/T340-maildir-sync.sh b/test/T340-maildir-sync.sh
index b474bf46e4be..6d9566354bb3 100755
--- a/test/T340-maildir-sync.sh
+++ b/test/T340-maildir-sync.sh
@@ -52,7 +52,8 @@ test_expect_equal_json "$output" '[[[{"id": "XXXXX",
 "content": "This is just a test message (#3)\n"}]},
 []]]]'
 
-test_expect_success 'notmuch reply works with renamed file (without notmuch new)' 'notmuch reply id:${gen_msg_id}'
+test_begin_subtest "notmuch reply works with renamed file (without notmuch new)"
+test_expect_success 'notmuch reply id:${gen_msg_id}'
 
 test_begin_subtest "notmuch new detects no file rename after tag->flag synchronization"
 output=$(NOTMUCH_NEW)
@@ -123,9 +124,10 @@ output+=$(notmuch search subject:"Message to lose maildir info" | notmuch_search
 test_expect_equal "$output" "No new mail. Detected 1 file rename.
 thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Message to lose maildir info (inbox)"
 
+test_begin_subtest "Can remove unread tag from message in non-maildir directory"
 add_message [subject]='"Non-maildir message"' [dir]=notmaildir [filename]='non-maildir-message'
 expected=$(notmuch search --output=files subject:"Non-maildir message")
-test_expect_success "Can remove unread tag from message in non-maildir directory" 'notmuch tag -unread subject:"Non-maildir message"'
+test_expect_success 'notmuch tag -unread subject:"Non-maildir message"'
 
 test_begin_subtest "Message in non-maildir directory does not get renamed"
 output=$(notmuch search --output=files subject:"Non-maildir message")
diff --git a/test/T350-crypto.sh b/test/T350-crypto.sh
index 14b215a3c0a6..f5e4c211d8fb 100755
--- a/test/T350-crypto.sh
+++ b/test/T350-crypto.sh
@@ -28,7 +28,8 @@ add_gnupg_home
 # Change this if we ship a new test key
 FINGERPRINT="5AEAB11F5E33DCE875DDB75B6D92612D94E46381"
 
-test_expect_success 'emacs delivery of signed message' \
+test_begin_subtest "emacs delivery of signed message"
+test_expect_success \
 'emacs_fcc_message \
     "test signed message 001" \
     "This is a test signed message." \
@@ -134,11 +135,12 @@ test_expect_equal_json \
     "$expected"
 mv "${GNUPGHOME}"{.bak,}
 
+test_begin_subtest "emacs delivery of encrypted message with attachment"
 # create a test encrypted message with attachment
 cat <<EOF >TESTATTACHMENT
 This is a test file.
 EOF
-test_expect_success 'emacs delivery of encrypted message with attachment' \
+test_expect_success \
 'emacs_fcc_message \
     "test encrypted message 001" \
     "This is a test encrypted message.\n" \
@@ -263,7 +265,8 @@ test_expect_equal_json \
     "$expected"
 mv "${GNUPGHOME}"{.bak,}
 
-test_expect_success 'emacs delivery of encrypted + signed message' \
+test_begin_subtest "emacs delivery of encrypted + signed message"
+test_expect_success \
 'emacs_fcc_message \
     "test encrypted message 002" \
     "This is another test encrypted message.\n" \
diff --git a/test/T355-smime.sh b/test/T355-smime.sh
index a6db4a18276f..a79211bdc8da 100755
--- a/test/T355-smime.sh
+++ b/test/T355-smime.sh
@@ -23,14 +23,16 @@ FINGERPRINT=$(openssl x509 -fingerprint -in test_suite.pem -noout | sed -e 's/^.
 
 add_gpgsm_home
 
-test_expect_success 'emacs delivery of S/MIME signed message' \
+test_begin_subtest "emacs delivery of S/MIME signed message"
+test_expect_success \
      'emacs_fcc_message \
      "test signed message 001" \
      "This is a test signed message." \
      "(mml-secure-message-sign \"smime\")"'
 
+test_begin_subtest "emacs delivery of S/MIME encrypted + signed message"
 # Hard code the MML to avoid several interactive questions
-test_expect_success 'emacs delivery of S/MIME encrypted + signed message' \
+test_expect_success \
 'emacs_fcc_message \
     "test encrypted message 001" \
     "<#secure method=smime mode=signencrypt keyfile=\\\"test_suite.pem\\\" certfile=\\\"test_suite.pem\\\">\nThis is a test encrypted message.\n"'
diff --git a/test/T380-atomicity.sh b/test/T380-atomicity.sh
index c6a9fb980bf0..a46a2df2a1fe 100755
--- a/test/T380-atomicity.sh
+++ b/test/T380-atomicity.sh
@@ -95,6 +95,7 @@ fi
 test_begin_subtest '"notmuch new" is idempotent under arbitrary aborts'
 test_expect_equal_file searchall expectall
 
-test_expect_success "detected $outcount>10 abort points" "test $outcount -gt 10"
+test_begin_subtest "detected $outcount>10 abort points"
+test_expect_success "test $outcount -gt 10"
 
 test_done
diff --git a/test/T400-hooks.sh b/test/T400-hooks.sh
index ed1191319fec..ae571ba282d9 100755
--- a/test/T400-hooks.sh
+++ b/test/T400-hooks.sh
@@ -91,11 +91,11 @@ test_expect_equal_file expected output
 # depends on the previous subtest leaving broken hook behind
 test_expect_code 1 "post-new non-zero exit status (notmuch status)" "notmuch new"
 
+test_begin_subtest "post-insert hook does not affect insert status"
 rm_hooks
 generate_message
 create_failing_hook "post-insert"
-test_expect_success "post-insert hook does not affect insert status" \
-    "notmuch insert < \"$gen_msg_filename\" > /dev/null"
+test_expect_success "notmuch insert < \"$gen_msg_filename\" > /dev/null"
 
 # test_begin_subtest "hook without executable permissions"
 rm_hooks
diff --git a/test/T530-upgrade.sh b/test/T530-upgrade.sh
index a3a7d39c5251..f0fd1511d056 100755
--- a/test/T530-upgrade.sh
+++ b/test/T530-upgrade.sh
@@ -10,8 +10,8 @@ if [ ! -e ${TEST_DIRECTORY}/test-databases/${dbtarball} ]; then
     test_subtest_missing_external_prereq_["${dbtarball} - fetch with 'make download-test-databases'"]=t
 fi
 
+test_begin_subtest "database checksum"
 test_expect_success \
-    'database checksum' \
     '( cd $TEST_DIRECTORY/test-databases &&
        sha256sum --quiet --check --status ${dbtarball}.sha256 )'
 
@@ -25,7 +25,8 @@ test_begin_subtest "path: search does not work with old database version"
 output=$(notmuch search path:foo)
 test_expect_equal "$output" ""
 
-test_expect_success 'pre upgrade dump' 'notmuch dump | sort > pre-upgrade-dump'
+test_begin_subtest "pre upgrade dump"
+test_expect_success 'notmuch dump | sort > pre-upgrade-dump'
 
 test_begin_subtest "database upgrade from format version 1"
 output=$(notmuch new | sed -e 's/^Backing up tags to .*$/Backing up tags to FILENAME/')
diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh
index 087c6bd74707..e775216ee29b 100755
--- a/test/T560-lib-error.sh
+++ b/test/T560-lib-error.sh
@@ -5,7 +5,8 @@ test_description="error reporting for library"
 
 add_email_corpus
 
-test_expect_success "building database" "NOTMUCH_NEW"
+test_begin_subtest "building database"
+test_expect_success "NOTMUCH_NEW"
 
 test_begin_subtest "Open null pointer"
 test_C <<'EOF'
diff --git a/test/T570-revision-tracking.sh b/test/T570-revision-tracking.sh
index 0936011ae6a1..a63e691cd3d4 100755
--- a/test/T570-revision-tracking.sh
+++ b/test/T570-revision-tracking.sh
@@ -49,11 +49,11 @@ test_expect_equal 1 ${result}
 
 notmuch count --lastmod '*' | cut -f2 > UUID
 
-test_expect_success 'search succeeds with correct uuid' \
-		    "notmuch search --uuid=$(cat UUID) '*'"
+test_begin_subtest "search succeeds with correct uuid"
+test_expect_success "notmuch search --uuid=$(cat UUID) '*'"
 
-test_expect_success 'uuid works as global option ' \
-		    "notmuch --uuid=$(cat UUID) search '*'"
+test_begin_subtest "uuid works as global option"
+test_expect_success "notmuch --uuid=$(cat UUID) search '*'"
 
 test_expect_code 1 'uuid works as global option II' \
 		    "notmuch --uuid=this-is-no-uuid search '*'"
@@ -61,14 +61,14 @@ test_expect_code 1 'uuid works as global option II' \
 test_expect_code 1 'search fails with incorrect uuid' \
 		 "notmuch search --uuid=this-is-no-uuid '*'"
 
-test_expect_success 'show succeeds with correct uuid' \
-		    "notmuch show --uuid=$(cat UUID) '*'"
+test_begin_subtest "show succeeds with correct uuid"
+test_expect_success "notmuch show --uuid=$(cat UUID) '*'"
 
 test_expect_code 1 'show fails with incorrect uuid' \
 		 "notmuch show --uuid=this-is-no-uuid '*'"
 
-test_expect_success 'tag succeeds with correct uuid' \
-		    "notmuch tag --uuid=$(cat UUID) +test '*'"
+test_begin_subtest "tag succeeds with correct uuid"
+test_expect_success "notmuch tag --uuid=$(cat UUID) +test '*'"
 
 test_expect_code 1 'tag fails with incorrect uuid' \
 		 "notmuch tag --uuid=this-is-no-uuid '*' +test2"
diff --git a/test/T600-named-queries.sh b/test/T600-named-queries.sh
index f0ae24f1a5fb..450da9b0c25a 100755
--- a/test/T600-named-queries.sh
+++ b/test/T600-named-queries.sh
@@ -9,12 +9,12 @@ test_expect_code 1 "error adding named query before initializing DB" \
 
 add_email_corpus
 
-test_expect_success "adding named query" \
-		    "notmuch config set query.test \"$QUERYSTR\""
+test_begin_subtest "adding named query"
+test_expect_success "notmuch config set query.test \"$QUERYSTR\""
 
+test_begin_subtest "adding nested named query"
 QUERYSTR2="query:test and subject:Maildir"
-test_expect_success "adding nested named query" \
-		    "notmuch config set query.test2 \"$QUERYSTR2\""
+test_expect_success "notmuch config set query.test2 \"$QUERYSTR2\""
 
 test_begin_subtest "retrieve named query"
 output=$(notmuch config get query.test)
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 3a69c399f6eb..a3a4cdcd8a8b 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -917,13 +917,17 @@ test_subtest_known_broken () {
 }
 
 test_expect_success () {
-	test "$#" = 2 ||
-	error "bug in the test script: not 2 parameters to test_expect_success"
-	test_subtest_name="$1"
-	test_reset_state_
-	if ! test_skip "$@"
+	exec 1>&6 2>&7		# Restore stdout and stderr
+	if [ -z "$inside_subtest" ]; then
+		error "bug in the test script: test_expect_success without test_begin_subtest"
+	fi
+	inside_subtest=
+	test "$#" = 1 ||
+	error "bug in the test script: not 1 parameters to test_expect_success"
+
+	if ! test_skip "$test_subtest_name"
 	then
-		test_run_ "$2"
+		test_run_ "$1"
 		run_ret="$?"
 		# test_run_ may update missing external prerequisites
 		test_check_missing_external_prereqs_ "$@" ||
@@ -931,7 +935,7 @@ test_expect_success () {
 		then
 			test_ok_
 		else
-			test_failure_ "$2"
+			test_failure_ "$1"
 		fi
 	fi
 }
diff --git a/test/test-verbose b/test/test-verbose
index 1723ce665caa..158be28a365e 100755
--- a/test/test-verbose
+++ b/test/test-verbose
@@ -4,12 +4,14 @@ test_description='the verbosity options of the test framework itself.'
 
 . ./test-lib.sh || exit 1
 
-test_expect_success 'print something in test_expect_success and pass' '
+test_begin_subtest 'print something in test_expect_success and pass'
+test_expect_success '
   echo "hello stdout" &&
   echo "hello stderr" >&2 &&
   true
 '
-test_expect_success 'print something in test_expect_success and fail' '
+test_begin_subtest 'print something in test_expect_success and fail'
+test_expect_success '
   echo "hello stdout" &&
   echo "hello stderr" >&2 &&
   false
-- 
2.11.0

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

* [PATCH 9/9] test: require test_begin_subtest before test_expect_code
  2017-02-26 13:42 [PATCH 0/9] test: cleanup and refactoring Jani Nikula
                   ` (7 preceding siblings ...)
  2017-02-26 13:43 ` [PATCH 8/9] test: require test_begin_subtest before test_expect_success Jani Nikula
@ 2017-02-26 13:43 ` Jani Nikula
  2017-03-10 11:45 ` [PATCH 0/9] test: cleanup and refactoring David Bremner
  9 siblings, 0 replies; 11+ messages in thread
From: Jani Nikula @ 2017-02-26 13:43 UTC (permalink / raw)
  To: notmuch

Unify the subtests by requiring test_begin_subtest before
test_expect_code. (Similar change for test_expect_success has already
been done.)

This increases clarity in the test scripts by having a separate line
for the start of the subtest with the heading, and makes it possible
to simplify the test infrastructure by making all subtests similar.
---
 test/README                    |  5 +++++
 test/T000-basic.sh             | 11 ++++-------
 test/T020-compact.sh           |  3 ++-
 test/T050-new.sh               |  4 ++--
 test/T070-insert.sh            | 34 ++++++++++++++++++++--------------
 test/T150-tagging.sh           | 13 +++++++++----
 test/T160-json.sh              |  8 ++++----
 test/T400-hooks.sh             | 14 ++++++++------
 test/T570-revision-tracking.sh | 16 ++++++++--------
 test/T600-named-queries.sh     |  4 ++--
 test/test-lib.sh               | 18 +++++++++++-------
 11 files changed, 75 insertions(+), 55 deletions(-)

diff --git a/test/README b/test/README
index 7acdb4b81f4d..dcd05237a062 100644
--- a/test/README
+++ b/test/README
@@ -200,6 +200,11 @@ library for your script to use.
    <script>.  If it yields success, test is considered
    successful.
 
+ test_expect_code <code> <script>
+
+   This takes two strings as parameter, and evaluates the <script>.
+   If it yields <code> exit status, test is considered successful.
+
  test_subtest_known_broken
 
    Mark the current test as broken.  Such tests are expected to fail.
diff --git a/test/T000-basic.sh b/test/T000-basic.sh
index 003ab95f5681..36a7ca4c5bdf 100755
--- a/test/T000-basic.sh
+++ b/test/T000-basic.sh
@@ -30,11 +30,9 @@ test_begin_subtest 'tests clean up after themselves'
 clean=no
 test_expect_success 'test_when_finished clean=yes'
 
+test_begin_subtest 'tests clean up even after a failure'
 cleaner=no
-test_expect_code 1 'tests clean up even after a failure' '
-    test_when_finished cleaner=yes &&
-    (exit 1)
-'
+test_expect_code 1 'test_when_finished cleaner=yes && (exit 1)'
 
 if test $clean$cleaner != yesyes
 then
@@ -42,9 +40,8 @@ then
 	exit 1
 fi
 
-test_expect_code 2 'failure to clean up causes the test to fail' '
-    test_when_finished "(exit 2)"
-'
+test_begin_subtest 'failure to clean up causes the test to fail'
+test_expect_code 2 'test_when_finished "(exit 2)"'
 
 EXPECTED=$TEST_DIRECTORY/test.expected-output
 suppress_diff_date() {
diff --git a/test/T020-compact.sh b/test/T020-compact.sh
index 1fecd071e77e..a3d7380e81c8 100755
--- a/test/T020-compact.sh
+++ b/test/T020-compact.sh
@@ -16,7 +16,8 @@ if [ $NOTMUCH_HAVE_XAPIAN_COMPACT -eq 0 ]; then
     test_expect_equal "$output" "notmuch was compiled against a xapian version lacking compaction support.
 Compaction failed: Unsupported operation"
 
-    test_expect_code 1 "Compact unsupported: status code" "notmuch compact"
+    test_begin_subtest "Compact unsupported: status code"
+    test_expect_code 1 "notmuch compact"
 
     test_done
 fi
diff --git a/test/T050-new.sh b/test/T050-new.sh
index 9115de820770..ffa303efacec 100755
--- a/test/T050-new.sh
+++ b/test/T050-new.sh
@@ -286,8 +286,8 @@ notmuch config set new.tags "-foo;bar"
 output=$(NOTMUCH_NEW --debug 2>&1)
 test_expect_equal "$output" "Error: tag '-foo' in new.tags: tag starting with '-' forbidden"
 
-test_expect_code 1 "Invalid tags set exit code" \
-    "NOTMUCH_NEW --debug 2>&1"
+test_begin_subtest "Invalid tags set exit code"
+test_expect_code 1 "NOTMUCH_NEW --debug 2>&1"
 
 notmuch config set new.tags $OLDCONFIG
 
diff --git a/test/T070-insert.sh b/test/T070-insert.sh
index 9120debabf8c..48f212ee0928 100755
--- a/test/T070-insert.sh
+++ b/test/T070-insert.sh
@@ -20,13 +20,13 @@ gen_insert_msg() {
 	"[body]=\"insert-message\""
 }
 
-test_expect_code 1 "Insert zero-length file" \
-    "notmuch insert < /dev/null"
+test_begin_subtest "Insert zero-length file"
+test_expect_code 1 "notmuch insert < /dev/null"
 
 # This test is a proxy for other errors that may occur while trying to
 # add a message to the notmuch database, e.g. database locked.
-test_expect_code 1 "Insert non-message" \
-    "echo bad_message | notmuch insert"
+test_begin_subtest "Insert non-message"
+test_expect_code 1 "echo bad_message | notmuch insert"
 
 test_begin_subtest "Database empty so far"
 test_expect_equal "0" "`notmuch count --output=messages '*'`"
@@ -138,9 +138,9 @@ notmuch insert --folder=Drafts +draft -unread < "$gen_msg_filename"
 output=$(notmuch search --output=messages path:Drafts/cur tag:draft NOT tag:unread)
 test_expect_equal "$output" "id:$gen_msg_id"
 
+test_begin_subtest "Insert message into non-existent folder"
 gen_insert_msg
-test_expect_code 1 "Insert message into non-existent folder" \
-    "notmuch insert --folder=nonesuch < $gen_msg_filename"
+test_expect_code 1 "notmuch insert --folder=nonesuch < $gen_msg_filename"
 
 test_begin_subtest "Insert message, create folder"
 gen_insert_msg
@@ -162,9 +162,9 @@ notmuch insert --folder=F/G/H/I/J --create-folder +folder < "$gen_msg_filename"
 output=$(notmuch count path:F/G/H/I/J/new tag:folder)
 test_expect_equal "$output" "2"
 
+test_begin_subtest "Insert message, create invalid subfolder"
 gen_insert_msg
-test_expect_code 1 "Insert message, create invalid subfolder" \
-    "notmuch insert --folder=../G --create-folder $gen_msg_filename"
+test_expect_code 1 "notmuch insert --folder=../G --create-folder $gen_msg_filename"
 
 OLDCONFIG=$(notmuch config get new.tags)
 
@@ -180,8 +180,8 @@ gen_insert_msg
 output=$(notmuch insert $gen_msg_filename 2>&1)
 test_expect_equal "$output" "Error: tag '-foo' in new.tags: tag starting with '-' forbidden"
 
-test_expect_code 1 "Invalid tags set exit code" \
-    "notmuch insert $gen_msg_filename 2>&1"
+test_begin_subtest "Invalid tags set exit code"
+test_expect_code 1 "notmuch insert $gen_msg_filename 2>&1"
 
 notmuch config set new.tags $OLDCONFIG
 
@@ -205,22 +205,28 @@ done
 gen_insert_msg
 
 for code in  FILE_NOT_EMAIL READ_ONLY_DATABASE UPGRADE_REQUIRED PATH_ERROR; do
-    test_expect_code 1 "EXIT_FAILURE when add_message returns $code" \
+    test_begin_subtest "EXIT_FAILURE when add_message returns $code"
+    test_expect_code 1 \
          "${TEST_GDB} --batch-silent --return-child-result \
 	     -ex 'set args insert < $gen_msg_filename' \
 	     -x index-file-$code.gdb notmuch"
-    test_expect_code 0 "success exit with --keep when add_message returns $code" \
+
+    test_begin_subtest "success exit with --keep when add_message returns $code"
+    test_expect_code 0 \
          "${TEST_GDB} --batch-silent --return-child-result \
 	     -ex 'set args insert --keep < $gen_msg_filename' \
 	     -x index-file-$code.gdb notmuch"
 done
 
 for code in OUT_OF_MEMORY XAPIAN_EXCEPTION ; do
-    test_expect_code 75 "EX_TEMPFAIL when add_message returns $code" \
+    test_begin_subtest "EX_TEMPFAIL when add_message returns $code"
+    test_expect_code 75 \
          "${TEST_GDB} --batch-silent --return-child-result \
 	     -ex 'set args insert < $gen_msg_filename' \
 	     -x index-file-$code.gdb notmuch"
-    test_expect_code 0 "success exit with --keep when add_message returns $code" \
+
+    test_begin_subtest "success exit with --keep when add_message returns $code"
+    test_expect_code 0 \
          "${TEST_GDB} --batch-silent --return-child-result \
 	     -ex 'set args insert --keep < $gen_msg_filename' \
 	     -x index-file-$code.gdb notmuch"
diff --git a/test/T150-tagging.sh b/test/T150-tagging.sh
index 61d1311608a7..0d0a3b874526 100755
--- a/test/T150-tagging.sh
+++ b/test/T150-tagging.sh
@@ -19,8 +19,11 @@ test_expect_equal "$output" "\
 thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (inbox tag3 unread)
 thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag3 unread)"
 
-test_expect_code 1 "No tag operations" 'notmuch tag One'
-test_expect_code 1 "No query" 'notmuch tag +tag2'
+test_begin_subtest "No tag operations"
+test_expect_code 1 'notmuch tag One'
+
+test_begin_subtest "No query"
+test_expect_code 1 'notmuch tag +tag2'
 
 test_begin_subtest "Redundant tagging"
 notmuch tag +tag1 -tag3 One
@@ -282,9 +285,11 @@ notmuch dump --format=batch-tag| \
 
 test_expect_equal_file EXPECTED OUTPUT
 
-test_expect_code 1 "Empty tag names" 'notmuch tag + One'
+test_begin_subtest "Empty tag names"
+test_expect_code 1 'notmuch tag + One'
 
-test_expect_code 1 "Tag name beginning with -" 'notmuch tag +- One'
+test_begin_subtest "Tag name beginning with -"
+test_expect_code 1 'notmuch tag +- One'
 
 test_begin_subtest "Xapian exception: read only files"
 chmod u-w  ${MAIL_DIR}/.notmuch/xapian/*.${db_ending}
diff --git a/test/T160-json.sh b/test/T160-json.sh
index 85b9b41f314c..48d30019e44a 100755
--- a/test/T160-json.sh
+++ b/test/T160-json.sh
@@ -64,11 +64,11 @@ test_expect_equal_json "$output" "[{\"thread\": \"XXX\",
  \"tags\": [\"inbox\",
  \"unread\"]}]"
 
-test_expect_code 20 "Format version: too low" \
-    "notmuch search --format-version=0 \\*"
+test_begin_subtest "Format version: too low"
+test_expect_code 20 "notmuch search --format-version=0 \\*"
 
-test_expect_code 21 "Format version: too high" \
-    "notmuch search --format-version=999 \\*"
+test_begin_subtest "Format version: too high"
+test_expect_code 21 "notmuch search --format-version=999 \\*"
 
 test_begin_subtest "Show message: multiple filenames"
 add_message "[id]=message-id@example.com [filename]=copy1"
diff --git a/test/T400-hooks.sh b/test/T400-hooks.sh
index ae571ba282d9..7917a82f0154 100755
--- a/test/T400-hooks.sh
+++ b/test/T400-hooks.sh
@@ -70,7 +70,8 @@ output=`notmuch new 2>&1`
 test_expect_equal "$output" "Error: pre-new hook failed with status 13"
 
 # depends on the previous subtest leaving broken hook behind
-test_expect_code 1 "pre-new non-zero exit status (notmuch status)" "notmuch new"
+test_begin_subtest "pre-new non-zero exit status (notmuch status)"
+test_expect_code 1 "notmuch new"
 
 # depends on the previous subtests leaving 1 new message behind
 test_begin_subtest "pre-new non-zero exit status aborts new"
@@ -89,7 +90,8 @@ echo "Error: post-new hook failed with status 13" >> expected
 test_expect_equal_file expected output
 
 # depends on the previous subtest leaving broken hook behind
-test_expect_code 1 "post-new non-zero exit status (notmuch status)" "notmuch new"
+test_begin_subtest "post-new non-zero exit status (notmuch status)"
+test_expect_code 1 "notmuch new"
 
 test_begin_subtest "post-insert hook does not affect insert status"
 rm_hooks
@@ -97,7 +99,7 @@ generate_message
 create_failing_hook "post-insert"
 test_expect_success "notmuch insert < \"$gen_msg_filename\" > /dev/null"
 
-# test_begin_subtest "hook without executable permissions"
+test_begin_subtest "hook without executable permissions"
 rm_hooks
 mkdir -p ${HOOK_DIR}
 cat <<EOF >"${HOOK_DIR}/pre-new"
@@ -105,15 +107,15 @@ cat <<EOF >"${HOOK_DIR}/pre-new"
 echo foo
 EOF
 output=`notmuch new 2>&1`
-test_expect_code 1 "hook without executable permissions" "notmuch new"
+test_expect_code 1 "notmuch new"
 
-# test_begin_subtest "hook execution failure"
+test_begin_subtest "hook execution failure"
 rm_hooks
 mkdir -p ${HOOK_DIR}
 cat <<EOF >"${HOOK_DIR}/pre-new"
 no hashbang, execl fails
 EOF
 chmod +x "${HOOK_DIR}/pre-new"
-test_expect_code 1 "hook execution failure" "notmuch new"
+test_expect_code 1 "notmuch new"
 
 test_done
diff --git a/test/T570-revision-tracking.sh b/test/T570-revision-tracking.sh
index a63e691cd3d4..76ad227944a0 100755
--- a/test/T570-revision-tracking.sh
+++ b/test/T570-revision-tracking.sh
@@ -55,23 +55,23 @@ test_expect_success "notmuch search --uuid=$(cat UUID) '*'"
 test_begin_subtest "uuid works as global option"
 test_expect_success "notmuch --uuid=$(cat UUID) search '*'"
 
-test_expect_code 1 'uuid works as global option II' \
-		    "notmuch --uuid=this-is-no-uuid search '*'"
+test_begin_subtest "uuid works as global option II"
+test_expect_code 1 "notmuch --uuid=this-is-no-uuid search '*'"
 
-test_expect_code 1 'search fails with incorrect uuid' \
-		 "notmuch search --uuid=this-is-no-uuid '*'"
+test_begin_subtest "search fails with incorrect uuid"
+test_expect_code 1 "notmuch search --uuid=this-is-no-uuid '*'"
 
 test_begin_subtest "show succeeds with correct uuid"
 test_expect_success "notmuch show --uuid=$(cat UUID) '*'"
 
-test_expect_code 1 'show fails with incorrect uuid' \
-		 "notmuch show --uuid=this-is-no-uuid '*'"
+test_begin_subtest "show fails with incorrect uuid"
+test_expect_code 1 "notmuch show --uuid=this-is-no-uuid '*'"
 
 test_begin_subtest "tag succeeds with correct uuid"
 test_expect_success "notmuch tag --uuid=$(cat UUID) +test '*'"
 
-test_expect_code 1 'tag fails with incorrect uuid' \
-		 "notmuch tag --uuid=this-is-no-uuid '*' +test2"
+test_begin_subtest "tag fails with incorrect uuid"
+test_expect_code 1 "notmuch tag --uuid=this-is-no-uuid '*' +test2"
 
 test_begin_subtest 'lastmod:0.. matches everything'
 total=$(notmuch count '*')
diff --git a/test/T600-named-queries.sh b/test/T600-named-queries.sh
index 450da9b0c25a..495b7699a0a7 100755
--- a/test/T600-named-queries.sh
+++ b/test/T600-named-queries.sh
@@ -4,8 +4,8 @@ test_description='named queries'
 
 QUERYSTR="date:2009-11-18..2009-11-18 and tag:unread"
 
-test_expect_code 1 "error adding named query before initializing DB" \
-		 "notmuch config set query.test \"$QUERYSTR\""
+test_begin_subtest "error adding named query before initializing DB"
+test_expect_code 1 "notmuch config set query.test \"$QUERYSTR\""
 
 add_email_corpus
 
diff --git a/test/test-lib.sh b/test/test-lib.sh
index a3a4cdcd8a8b..865afd5ecc33 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -941,13 +941,17 @@ test_expect_success () {
 }
 
 test_expect_code () {
-	test "$#" = 3 ||
-	error "bug in the test script: not 3 parameters to test_expect_code"
-	test_subtest_name="$2"
-	test_reset_state_
-	if ! test_skip "$@"
+	exec 1>&6 2>&7		# Restore stdout and stderr
+	if [ -z "$inside_subtest" ]; then
+		error "bug in the test script: test_expect_code without test_begin_subtest"
+	fi
+	inside_subtest=
+	test "$#" = 2 ||
+	error "bug in the test script: not 2 parameters to test_expect_code"
+
+	if ! test_skip "$test_subtest_name"
 	then
-		test_run_ "$3"
+		test_run_ "$2"
 		run_ret="$?"
 		# test_run_ may update missing external prerequisites,
 		test_check_missing_external_prereqs_ "$@" ||
@@ -955,7 +959,7 @@ test_expect_code () {
 		then
 			test_ok_
 		else
-			test_failure_ "exit code $eval_ret, expected $1" "$3"
+			test_failure_ "exit code $eval_ret, expected $1" "$2"
 		fi
 	fi
 }
-- 
2.11.0

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

* Re: [PATCH 0/9] test: cleanup and refactoring
  2017-02-26 13:42 [PATCH 0/9] test: cleanup and refactoring Jani Nikula
                   ` (8 preceding siblings ...)
  2017-02-26 13:43 ` [PATCH 9/9] test: require test_begin_subtest before test_expect_code Jani Nikula
@ 2017-03-10 11:45 ` David Bremner
  9 siblings, 0 replies; 11+ messages in thread
From: David Bremner @ 2017-03-10 11:45 UTC (permalink / raw)
  To: Jani Nikula, notmuch

Jani Nikula <jani@nikula.org> writes:

> The first half is cleanups, throwing out unused stuff.
>
> The second half requires test_begin_subtest before *all* subtests.
>

series pushed to master

d

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

end of thread, other threads:[~2017-03-10 11:45 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-26 13:42 [PATCH 0/9] test: cleanup and refactoring Jani Nikula
2017-02-26 13:42 ` [PATCH 1/9] test: remove unused regexp convenience variables Jani Nikula
2017-02-26 13:42 ` [PATCH 2/9] test: remove unused filter functions Jani Nikula
2017-02-26 13:42 ` [PATCH 3/9] test: remove unused test_external and test_external_without_stderr Jani Nikula
2017-02-26 13:42 ` [PATCH 4/9] test: remove unused and no-op --long-tests parameter Jani Nikula
2017-02-26 13:42 ` [PATCH 5/9] test: only accept short and long options, not silly in-betweens Jani Nikula
2017-02-26 13:42 ` [PATCH 6/9] test: ensure test_begin_subtest has been called before test_expect_* Jani Nikula
2017-02-26 13:42 ` [PATCH 7/9] test: drop the implicit prereq check mechanism from test_expect_* Jani Nikula
2017-02-26 13:43 ` [PATCH 8/9] test: require test_begin_subtest before test_expect_success Jani Nikula
2017-02-26 13:43 ` [PATCH 9/9] test: require test_begin_subtest before test_expect_code Jani Nikula
2017-03-10 11:45 ` [PATCH 0/9] test: cleanup and refactoring 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).