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

[Derrick Stolee <stolee@gmail.com>]

On 10/12/2021 12:45 PM, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> 
>> On Mon, Oct 11 2021, Junio C Hamano wrote:
>>
>> [Removed "In-reply-to: <xmqq5yu3b80j.fsf@gitster.g>" with the Subject
>> change]
> 
> Please do not do the former, although it is welcome to change Subject.
> 
>> Presumably with test_expect_failure.
>>
>> I'll change it, in this case we'd end up with a test_expect_success at
>> the end, so it doesn't matter much & I don't care.
> 
> I do agree with you that compared to expect_success, which requires
> _all_ steps to succeed, so an failure in any of its steps is
> immediately noticeable, it is harder to write and keep
> expect_failure useful, because it is not like we are happy to see
> any failure in any step.  We do not expect a failure in many
> preparation and conclusion steps in the &&-chain in expect_failure
> block, and we consider it is an error if these steps fail.  We only
> want to mark only a single step to exhibit an expected but undesirable
> behaviour.
> 
> 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.

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.

This could have the side-effect of having a "fixed" test_expect_failure
show as a failed test, not a "TODO" message.

Thanks,
-Stolee