Subject: Re: [PATCH 1/7] test-lib: add a "test_expect_todo", similar to "test_expect_failure"

From: Junio C Hamano <[email-masked]>
Date: Fri, 18 Mar 2022 13:50:12 -0700

Junio C Hamano <[email-masked]> writes:

> Marking the step that demonstrates the current shortcomings with
> "MUST FAIL" is a bad taste, but let's pretend that we didn't notice
> it ;-).  Other than that, it looks like an improvement.
> ...
>> +test_expect_failure () {
>> +	_test_expect_todo test_expect_failure "$@"
>> +}
>> +
>> +test_expect_todo () {
>> +	_test_expect_todo test_expect_todo "$@"
>> +}
>
> It is a bit surprising that _test_expect_todo does not share more of
> its implementation with test_expect_success, but let's pretend we
> didn't see it.

With a bit more tweak, I think this can be made palatable:

 * introduce something that is *NOT* test_must_fail but behaves like
   so, but with a bit of magic (see below).

 * do not introduce test_expect_todo, but use an improved version of
   test_expect_success.

Let's illustrate what I mean by starting from an example that we
want to have _after_ fixing an known breakage.  Let's say we expect
that our test preparation succeeds, 'git foo' fails when given a bad
option, 'git foo' runs successfully, and the command is expected to
emit certain output.  We may assert the ideal future world like so:

	test_expect_success 'make sure foo works the way we want' '
		preparatory step &&
		test_must_fail git foo --bad-option >error &&
		grep "expected error message" error &&
		! grep "unwanted error message" error &&
		git foo >output &&
		grep expected output &&
		! grep unwanted output
	'

Let's also imagine that right now, option parsing in "git foo",
works but the main execution of the command does not work.

With test_expect_todo, you have to write something like this
to document the current breakage:

	test_expect_todo 'document breakage' '
		preparatory step &&
		test_must_fail git foo --bad-option >error &&
		grep "expected error message" error &&
		! grep "unwanted error message" error &&
		test_must_fail git foo >output &&
		! grep expected output &&
		grep unwanted output
	'

You can see that this makes one thing unclear.

Among the two test_must_fail and two !, which one(s) document the
breakage?  In other words, which one of these four negations do we
wish to lift eventually?  The answer is the latter two (we said that
handling of "--bad-option" is already working), but it is not obvious
in the above test_expect_todo test sequence.

I'd suggest we allow our test to be written this way:

	test_expect_success 'make sure foo works the way we want' '
		preparatory step &&
		test_must_fail git foo --bad-option >error &&
		grep "expected error message" error &&
		! grep "unwanted error message" error &&
	test_ki git foo >output &&
	test_ki grep expected output &&
	test_ki ! grep unwanted output
	'

and teach test_expect_success that an invocation of test_ki ("known
issue"---a better name that is NOT test_must_fail very much welcome)
means we hope this test someday passes without test_ki but not
today, i.e. what your test_expect_todo means, and we unfortunately
have to expect that the lines annotated with test_ki would "fail".

To readers, test_ki should appear as if an annotation to a single
command that highlights what we want to eventually be able to fix,
and when the issue around the line marked as test_ki is fixed, we
can signal the fact by removing it from the beginning of these lines
(that is why the last one is "test_ki !" and not "!  test_ki").

To the test framework and the shell that is running the test,
test_ki would be almost identical to test_must_fail, i.e. runs the
rest of the command, catch segv and report, but otherwise the
failure from that "rest of the command" execution is turned into
"success" that lets &&-chain to continue.  In addition, it tells
the test_expect_success running it that a success of the test piece
should be shown as TODO (expected failure).

Hmm?