[RFC] test-lib: detect common misuse of test_expect_failure
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 14, 2016, 22:38 UTC
- Message-ID
- <xmqqk2day2ry.fsf@gitster.mtv.corp.google.com>
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 <gitster@pobox.com> ---
* 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 <xmqqr37iy5bw.fsf@gitster.mtv.corp.google.com>; 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 () {