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@xxxxxxxxxxxxxxxxxxxx> --- 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html