Subject: Re: [PATCH] Sane use of test_expect_failure

From: Junio C Hamano <[email-masked]>
Date: Sat, 02 Feb 2008 02:06:35 -0800

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?