From: Ramsay Jones <ramsay@ramsayjones.plus.com>
To: Lars Schneider <larsxschneider@gmail.com>
Cc: Jeff King <peff@peff.net>, Junio C Hamano <gitster@pobox.com>,
GIT Mailing-list <git@vger.kernel.org>
Subject: [PATCH] test-lib-functions: fix logic error in test_must_fail
Date: Tue, 24 Nov 2015 15:59:24 +0000 [thread overview]
Message-ID: <5654895C.2000909@ramsayjones.plus.com> (raw)
After commit 710eb805 ("implement test_might_fail using a refactored
test_must_fail", 19-11-2015) several tests now unexpectedly pass:
$ tail -21 ptest-out
[17:25:53]
All tests successful.
Test Summary Report
-------------------
t1060-object-corruption.sh (Wstat: 0 Tests: 13 Failed: 0)
TODO passed: 13
t3510-cherry-pick-sequence.sh (Wstat: 0 Tests: 39 Failed: 0)
TODO passed: 31
t5707-clone-detached.sh (Wstat: 0 Tests: 13 Failed: 0)
TODO passed: 4
t6042-merge-rename-corner-cases.sh (Wstat: 0 Tests: 26 Failed: 0)
TODO passed: 8
Files=727, Tests=13139, 407 wallclock secs ( 3.66 usr 0.54 sys + 85.10 cusr 258.06 csys = 347.36 CPU)
Result: PASS
make clean-except-prove-cache
make[2]: Entering directory `/home/ramsay/git/t'
rm -f -r 'trash directory'.* 'test-results'
rm -f -r valgrind/bin
make[2]: Leaving directory `/home/ramsay/git/t'
make[1]: Leaving directory `/home/ramsay/git/t'
$
Fix the logic bug in the refactored test_must_fail. Also, use the
integer comparison operator -eq when comparing the exit code, rather
than the string = operator.
Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
Hi Lars,
This patch is on top of the pu branch, but it relates to the two
patches on your 'ls/test-must-fail-sigpipe' branch. Could you
please squash the relevant parts of this patch into your patches.
So, in addition to the logic error in commit 710eb805, I modified
the test introduced by commit 653a48e56 ("add "ok=sigpipe" to
test_must_fail and use it to fix flaky tests", 19-11-2015).
Since I cannot test this second change (t5516 and t5504 don't
fail for me), I don't know if this change is correct - please
test and confirm. (No, it's not clear to me exactly what this
commit is supposed to do! :-D ).
[I didn't have time to go look what value would be returned by
a case statement where there is no 'default' limb - I suspect
that it is undefined behaviour. Even if it is defined, do all
shells behave properly? In any event, it is much simpler to
compare the strings directly!]
I have to say, I'm not keen on either of these commits, but Jeff
and Junio seem OK with it, so ... (the tests being flaky implies
that the git client is flaky - we should fix that).
Thanks.
ATB,
Ramsay Jones
t/test-lib-functions.sh | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 1fdc58c..9061742 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -593,24 +593,22 @@ test_must_fail () {
esac
"$@"
exit_code=$?
- if ! case ",$_test_ok," in *,success,*) false;; esac &&
- test $exit_code = 0
+ if test $exit_code -eq 0 && test x$_test_ok != xsuccess
then
echo >&2 "test_must_fail: command succeeded: $*"
- return 0
- elif ! case ",$_test_ok," in *,sigpipe,*) false;; esac &&
- test $exit_code = 141
+ return 1
+ elif test $exit_code -eq 141 && test x$_test_ok = xsigpipe
then
return 0
elif test $exit_code -gt 129 && test $exit_code -le 192
then
echo >&2 "test_must_fail: died by signal: $*"
return 1
- elif test $exit_code = 127
+ elif test $exit_code -eq 127
then
echo >&2 "test_must_fail: command not found: $*"
return 1
- elif test $exit_code = 126
+ elif test $exit_code -eq 126
then
echo >&2 "test_must_fail: valgrind error: $*"
return 1
--
2.6.0
next reply other threads:[~2015-11-24 16:00 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-24 15:59 Ramsay Jones [this message]
2015-11-24 21:08 ` [PATCH] test-lib-functions: fix logic error in test_must_fail Jeff King
2015-11-25 0:05 ` Ramsay Jones
2015-11-25 0:10 ` Jeff King
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=5654895C.2000909@ramsayjones.plus.com \
--to=ramsay@ramsayjones.plus.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=larsxschneider@gmail.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).