Re: [PATCH] Sane use of test_expect_failure
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:44:09
As I promised, a patch to revamp test_expect_failure semantics
has been applied to 'master' and pushed out.
The rule used to be that test_expect_failure is to see if the
command sequence exits with non-zero status. It was tempting to
incorrectly use it like this:
test_expect_failure 'this should fail' '
setup1 &&
setup2 &&
setup3 &&
what you expect to fail
'
but was very error prone, because the failure can come from the
earlier "setup" stages.
The new world order is that test_expect_failure is used to mark
a known breakage, so that people can run "git grep t/" to see if
there are things to work on.
We have an example in cvsserver test:
#TODO: cvsserver doesn't support update w/o -d
test_expect_failure "cvs update w/o -d doesn't create subdir (TODO)" '
...
test ! -d test
'
If git-cvsserver did not have this bug, this should succeed, but
there is a known breakage that is waiting to be fixed.
I may have missed tests that were using test_expect_failure to
mark known bug that need to be fixed and converted that to
test_expect_success to check an error exit status from the last
command in the sequence. IOW, a mistranslation of the above
might have done:
test_expect_success "cvs update w/o -d doesn't create subdir (TODO)" '
...
test -d test
'
which would be wrong. Fixing the bug would then "break" this
test.
Could people who added test_expect_failure in the past that this
patch updated, look them over to catch such a misconversion
please?