Re: test-lib.sh musings: test_expect_failure considered harmful

> Derrick Stolee <stolee@gmail.com> writes:
>
>>> But even with the shortcomings of expect_failure, it still is much
>>> better than claiming that we expect a bogus outcome.
>>> 
>>> Improving the shortcomings of expect_failure would be a much better
>>> use of our time than advocating an abuse of expect_sucess, I would
>>> think.
>>
>> I agree that test_expect_failure has these drawbacks. I've recently
>> been using _expect_success to document "bad" behavior so we can verify
>> that behavior changes when that behavior is fixed. But it does have
>> the drawback of looking like we claim the result is by design.
>
> Yeah, I think I saw (and I think I used the same technique myself)
> people expect a bad output with test_expect_success with an in-code
> (not in-log) comment that explicitly says "This documents the
> current behaviour, which is wrong", and that is a very acceptable
> solution, I would think.
>
>> One possible way to correct this is to create a "test_expected_failure"
>> helper that could be placed on the step(s) of the &&-chain that are
>> expected to fail. The helper could set some variable to true if the
>> failure is hit, and false otherwise. It can also convert a failure
>> into a positive result. Then, test_expect_failure could look for that
>> variable's value (after verifying that the &&-chain returns success)
>> to show that all expected failures completed correctly.
>
> Yup, I would very much like the direction, and further imagine that
> the above approach can be extended to ...
>
>> This could have the side-effect of having a "fixed" test_expect_failure
>> show as a failed test, not a "TODO" message.
>
> ... avoid such downside.  Perhaps call that magic "we know this step
> fails currently" test_known_breakage and declare that we deprecate
> the use of test_expect_failure in new tests.  Such a test might look
> like this:
>
>     test_expect_success 'commit error message should not duplicate' '
> 	test_when_finished "chmod -R u+rwx ." &&
> 	chmod u-rwx .git/objects/ &&
> 	orig_head=$(git rev-parse HEAD) &&
> 	test_must_fail git commit --allow-empty -m "read-only" 2>rawerr &&
> 	grep "insufficient permission" rawerr >err &&
> 	test_known_breakage test_line_count = 1 err &&
> 	new_head=$(git rev-parse HEAD) &&
> 	test "$orig_head" = "$new_head"
>     '
>
> which may use your trick to turn both failure and success to OK (to
> let the remainder of the test to continue) but signal the
> surrounding test_expect_success to say either "TODO know breakage"
> or "Fixed".