Subject: [RFC] test-lib: detect common misuse of test_expect_failure

From: Junio C Hamano <[email-masked]>
Date: Fri, 14 Oct 2016 15:38:41 -0700

It is a very easy mistake to make to say test_expect_failure when
making sure a step in the test fails, which must be spelled
"test_must_fail".  By introducing a toggle $test_in_progress that is
turned on at the beginning of test_start_() and off at the end of
test_finish_() helper, we can detect this fairly easily.

Strictly speaking, writing "test_expect_success" inside another
test_expect_success (or inside test_expect_failure for that matter)
can be detected with the same mechanism if we really wanted to, but
that is a lot less likely confusion, so let's not bother.

Signed-off-by: Junio C Hamano <[email-masked]>
---

 * It is somewhat embarrassing to admit that I had to stare at the
   offending code for more than 5 minutes to notice what went wrong
   to come up with <[email-masked]>;
   if we had something like this, it would have helped.

 t/test-lib-functions.sh | 4 ++++
 t/test-lib.sh           | 3 +++
 2 files changed, 7 insertions(+)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index fdaeb3a96b..fc8c10a061 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -381,6 +381,10 @@ test_verify_prereq () {
 }
 
 test_expect_failure () {
+	if test "$test_in_progress" = 1
+	then
+		error "bug in the test script: did you mean test_must_fail instead of test_expect_failure?"
+	fi
 	test_start_
 	test "$#" = 3 && { test_prereq=$1; shift; } || test_prereq=
 	test "$#" = 2 ||
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 11562bde10..4c360216e5 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -344,6 +344,7 @@ test_count=0
 test_fixed=0
 test_broken=0
 test_success=0
+test_in_progress=0
 
 test_external_has_tap=0
 
@@ -625,6 +626,7 @@ test_run_ () {
 }
 
 test_start_ () {
+	test_in_progress=1
 	test_count=$(($test_count+1))
 	maybe_setup_verbose
 	maybe_setup_valgrind
@@ -634,6 +636,7 @@ test_finish_ () {
 	echo >&3 ""
 	maybe_teardown_valgrind
 	maybe_teardown_verbose
+	test_in_progress=0
 }
 
 test_skip () {