From: Junio C Hamano <gitster@pobox•com>
To: git@vger•kernel.org
Subject: [RFC] test-lib: detect common misuse of test_expect_failure
Date: Fri, 14 Oct 2016 15:38:41 -0700 [thread overview]
Message-ID: <xmqqk2day2ry.fsf@gitster.mtv.corp.google.com> (raw)
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 () {
next reply other threads:[~2016-10-14 22:50 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-14 22:38 Junio C Hamano [this message]
2016-10-14 23:57 ` [RFC] test-lib: detect common misuse of test_expect_failure Jeff King
2016-10-17 17:36 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqk2day2ry.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox