archive mirror
 help / color / mirror / Atom feed
From: Phillip Wood <>
To: Junio C Hamano <>,
	Derrick Stolee <>
Cc: "Ævar Arnfjörð Bjarmason" <>,, "John Cai" <>,
	"Elijah Newren" <>,
	"Derrick Stolee" <>
Subject: Re: [PATCH 1/7] test-lib: add a "test_expect_todo", similar to "test_expect_failure"
Date: Thu, 24 Mar 2022 11:24:07 +0000	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <xmqqy210pbbh.fsf@gitster.g>

On 23/03/2022 22:13, Junio C Hamano wrote:
> Derrick Stolee <> writes:
>> 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
> I was hoping that we can do without test_expect_todo.
> test_expect_success can turn itself into test_expect_todo when it
> sees test_todo is invoked even once in it.  And Phillip's outline
> actually implements the idea, if I am not mistaken.

You are correct, there is no test_expect_todo in the outline I posted, 
test_todo lives within test_expect_success and works like test_must_fail.
>> 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.
> In other words, we do not want to lose the "TODO fixed" we have been
> getting out of test_expect_failure, which I agree with. 

I read Stolee's comments the other way, that we want the test harness to 
see the test fail rather passing as it does with the "TODO fixed" 
feature. In one of my early contributions I inadvertently fixed a 
submodule test and did not realize until someone pointed it out because 
the CI passes rather than fails when a test_expect_failure is fixed.

> I am not
> sure if Phillip's outline had that feature.

In my outline the test fails if any command marked by test_todo is 
successful and test_todo prints a messaging saying the command was 
unexpectedly successful. Implementing the "TODO fixed" feature gets 
tricky when there is more than one test_todo within a single 
test_expect_success - what if one is test_todo command is successful and 
the others fail?

Best Wishes


>> 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.
> Yes.

  reply	other threads:[~2022-03-24 11:24 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-18  0:33 [PATCH 0/7] test-lib-functions: a better "test_expect_failure" Ævar Arnfjörð Bjarmason
2022-03-18  0:33 ` [PATCH 1/7] test-lib: add a "test_expect_todo", similar to "test_expect_failure" Ævar Arnfjörð Bjarmason
2022-03-18 18:59   ` Junio C Hamano
2022-03-18 20:50     ` Junio C Hamano
2022-03-18 23:07       ` Ævar Arnfjörð Bjarmason
2022-03-19  7:12         ` Junio C Hamano
2022-03-19 11:11           ` Ævar Arnfjörð Bjarmason
2022-03-20 15:13             ` Phillip Wood
2022-03-20 18:07               ` Junio C Hamano
2022-03-22 14:43                 ` Derrick Stolee
2022-03-23 22:13                   ` Junio C Hamano
2022-03-24 11:24                     ` Phillip Wood [this message]
2022-03-18  0:33 ` [PATCH 2/7] test-lib-functions: add and use a "test_todo" helper Ævar Arnfjörð Bjarmason
2022-03-18  0:33 ` [PATCH 3/7] tests: allow test_* in "test_must_fail_acceptable" for "test_todo" Ævar Arnfjörð Bjarmason
2022-03-18  0:33 ` [PATCH 4/7] test-lib-functions: add and use a "todo_test_cmp" helper Ævar Arnfjörð Bjarmason
2022-03-18  0:34 ` [PATCH 5/7] test-lib-functions: add and use a "todo_test_path" helper Ævar Arnfjörð Bjarmason
2022-03-18  0:34 ` [PATCH 6/7] test-lib-functions: make test_todo support a --reset Ævar Arnfjörð Bjarmason
2022-03-18  0:34 ` [PATCH 7/7] sparse tests: convert a TODO test to use "test_expect_todo" Ævar Arnfjörð Bjarmason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).