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

From: Derrick Stolee <derrickstolee@github.com>
Date: Mar 22, 2022, 14:43 UTC
Message-ID: <a9478c1b-4441-0c79-332d-a7c5592dbc70@github.com>
In-Reply-To: <xmqqy2144hwa.fsf@gitster.g>

On 3/20/22 2:07 PM, Junio C Hamano wrote:

Show 22 quoted lines

> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
>> .... I had a look
>> at some of the conversions with your test_todo --want/--expect/--reset
>> and found the result really hard to follow. Junio's suggestions chimed
>> with some things I've been thinking about so I had a go at
>> implementing it and doing some sample conversions (see below). Marking
>> the individual commands that should fail is a big step forward and the
>> failing commands are checked to make sure they don't segfault etc.
> 
> ;-)
> 
> Another small detail in my suggestion that will make a huge
> difference in the end is not to introduce test_expect_todo as a
> separate top-level construct, and instead teach test_expect_success
> to pay attention to the use of test_todo "unfortunately this does
> not work yet" mark in it.  It allows us to use test_todo in a shared
> test helper function and call them in test_expect_success, and when
> the step the test helper has trouble with gets fixed, the "unmark"
> step will be an isolated change.
> 
> Your sample change seems to already have it, which is good.

After reading this thread, I agree with many of the ideas that were generated in response to this topic.

The thing I'm hoping to see from a final version is that a top-level helper like test_expect_todo will expect at least one test_todo helper to be executed inside of the test (perhaps communicated by setting a special GIT_TEST_* environment variable?) and if any of the test_todo lines change from fail to pass, then that is communicated as a _failure_ from CI's perspective. Let us discover if we have accidentally "fixed" any of these test cases and update the tests accordingly.

I can predict writing a test case with multiple test_todo lines that need to be updated to drop the test_todo helpers one-by-one as a change is being introduced.

Thanks, -Stolee