From 0497d695ca796c4d19ad5ec08788bbb1f400c58c Mon Sep 17 00:00:00 2001
From: Jani Nikula <jani@nikula.org>
Date: Sun, 26 Feb 2017 15:43:01 +0200
Subject: [PATCH] test: require test_begin_subtest before test_expect_code

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 7acdb4b8..dcd05237 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 003ab95f..36a7ca4c 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 1fecd071..a3d7380e 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 9115de82..ffa303ef 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 9120deba..48f212ee 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 61d13116..0d0a3b87 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 5a954e3b..ac51895e 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 ae571ba2..7917a82f 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 a63e691c..76ad2279 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 450da9b0..495b7699 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 b68290c3..32bd3f12 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -940,13 +940,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_ "$@" ||
@@ -954,7 +958,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.43.0