git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Phillip Wood <phillip.wood123@gmail.com>
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	git@vger.kernel.org, "John Cai" <johncai86@gmail.com>,
	"Elijah Newren" <newren@gmail.com>,
	"Derrick Stolee" <stolee@gmail.com>
Subject: Re: [PATCH 1/7] test-lib: add a "test_expect_todo", similar to "test_expect_failure"
Date: Sun, 20 Mar 2022 11:07:33 -0700	[thread overview]
Message-ID: <xmqqy2144hwa.fsf@gitster.g> (raw)
In-Reply-To: <db558292-2783-3270-4824-43757822a389@gmail.com> (Phillip Wood's message of "Sun, 20 Mar 2022 15:13:36 +0000")

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.

> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 0f439c99d6..deb74a22f3 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -739,6 +739,7 @@ test_expect_failure () {
>  }
>    test_expect_success () {
> +	test_todo_=
>  	test_start_
>  	test "$#" = 3 && { test_prereq=$1; shift; } || test_prereq=
>  	test "$#" = 2 ||
> @@ -750,7 +751,12 @@ test_expect_success () {
>  		say >&3 "expecting success of $TEST_NUMBER.$test_count '$1': $2"
>  		if test_run_ "$2"
>  		then
> -			test_ok_ "$1"
> +			if test -n "$test_todo_"
> +			then
> +				test_known_broken_failure_ "$1"
> +			else
> +				test_ok_ "$1"
> +			fi
>  		else
>  			test_failure_ "$@"
>  		fi

  reply	other threads:[~2022-03-20 18:07 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 [this message]
2022-03-22 14:43                 ` Derrick Stolee
2022-03-23 22:13                   ` Junio C Hamano
2022-03-24 11:24                     ` Phillip Wood
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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=xmqqy2144hwa.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=johncai86@gmail.com \
    --cc=newren@gmail.com \
    --cc=phillip.wood123@gmail.com \
    --cc=stolee@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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).