From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Phillip Wood via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Derrick Stolee <stolee@gmail.com>,
Phillip Wood <phillip.wood@dunelm.org.uk>
Subject: Re: [PATCH 3/3] [RFC] test_todo: allow [verbose] test as the command
Date: Thu, 06 Oct 2022 18:02:23 +0200 [thread overview]
Message-ID: <221006.86zge9q6js.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <c0d955c2d1fa234ad2e4a8aad467b991030ccf47.1665068476.git.gitgitgadget@gmail.com>
On Thu, Oct 06 2022, Phillip Wood via GitGitGadget wrote:
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> For simplicity test_todo() allows verbose to precede any valid
> command. As POSIX specifies that a return code greater than one is an
> error rather than a failed test we take care not to hide that.
>
> I'm in two minds about this patch. Generally it is better to use one
> of our test helpers such as test_cmp() rather than calling test
> directly. There are so few instances of test being used within
> test_expect_failure() (the conversions here are not exhaustive but
> there are not many more) that it would probably be better to convert
> the tests by using the appropriate helper rather than supporting
> calling test as the command to test_todo().
I think that we might want to salvage parts of this, but we really
shouldn't be spending review time on carrying forward a bad pattern that
hides segfaults.
I.e. whatever we do about "test_todo"'s interaction with "test" let's
first change things like...
> -test_expect_failure CASE_INSENSITIVE_FS 'add (with different case)' '
> +test_expect_success CASE_INSENSITIVE_FS 'add (with different case)' '
> git reset --hard initial &&
> rm camelcase &&
> echo 1 >CamelCase &&
> @@ -108,7 +108,7 @@ test_expect_failure CASE_INSENSITIVE_FS 'add (with different case)' '
> git ls-files >tmp &&
> camel=$(grep -i camelcase tmp) &&
> test $(echo "$camel" | wc -l) = 1 &&
> - test "z$(git cat-file blob :$camel)" = z1
> + test_todo test "z$(git cat-file blob :$camel)" = z1
...this to e.g.:
echo z1 >expect &&
git cat-file blob :$camel >actual &&
test_cmp expect actual
Or whatever, then let's see if migrating "verbose" is worthwhile, in the
post-image you end up with no real users of it, only your tests.
I've wanted to just remove it for a while, all its users seem to be
either bad uses like that, or we'd get much better bang for the buck out
of it by having a t/verbose-bin/ or whatever, which would just wrap
arbitrary commands like "grep" and the like (i.e. ones where we could
provide useful context).
next prev parent reply other threads:[~2022-10-06 16:07 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-06 15:01 [PATCH 0/3] [RFC] tests: add test_todo() for known failures Phillip Wood via GitGitGadget
2022-10-06 15:01 ` [PATCH 1/3] [RFC] tests: add test_todo() to mark known breakages Phillip Wood via GitGitGadget
2022-10-06 15:36 ` Ævar Arnfjörð Bjarmason
2022-10-06 16:10 ` Phillip Wood
2022-10-06 20:33 ` Ævar Arnfjörð Bjarmason
2022-12-06 22:37 ` Victoria Dye
2022-12-07 12:08 ` Ævar Arnfjörð Bjarmason
2022-12-08 15:06 ` Phillip Wood
2022-12-09 1:09 ` Junio C Hamano
2022-12-09 9:04 ` Phillip Wood
2022-10-06 15:01 ` [PATCH 2/3] [RFC] test_todo: allow [!] grep as the command Phillip Wood via GitGitGadget
2022-10-06 15:56 ` Ævar Arnfjörð Bjarmason
2022-10-06 16:42 ` Phillip Wood
2022-10-06 20:26 ` Ævar Arnfjörð Bjarmason
2022-10-06 15:01 ` [PATCH 3/3] [RFC] test_todo: allow [verbose] test " Phillip Wood via GitGitGadget
2022-10-06 16:02 ` Ævar Arnfjörð Bjarmason [this message]
2022-10-06 17:05 ` [PATCH 0/3] [RFC] tests: add test_todo() for known failures Junio C Hamano
2022-10-06 19:28 ` Ævar Arnfjörð Bjarmason
2022-10-07 13:26 ` Phillip Wood
2022-10-07 17:08 ` Junio C Hamano
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=221006.86zge9q6js.gmgdl@evledraar.gmail.com \
--to=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=phillip.wood@dunelm.org.uk \
--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).