From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Michael Montalbo via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, "D. Ben Knoble" <ben.knoble@gmail.com>,
Eric Sunshine <sunshine@sunshineco.com>,
Michael Montalbo <mmontalbo@gmail.com>
Subject: Re: [PATCH v2 5/6] t: convert grep assertions to test_grep
Date: Sat, 27 Jun 2026 09:08:52 +0200 [thread overview]
Message-ID: <aj93BE8MYatQAjoy@szeder.dev> (raw)
In-Reply-To: <3a589ef7386303075413f388e61c203c4e325d44.1781323575.git.gitgitgadget@gmail.com>
On Sat, Jun 13, 2026 at 04:06:14AM +0000, Michael Montalbo via GitGitGadget wrote:
> From: Michael Montalbo <mmontalbo@gmail.com>
>
> Replace bare grep with test_grep in test assertions across the
> suite, including sourced test helpers (lib-*.sh, *-tests.sh).
> test_grep prints the contents of the file being searched on
> failure, making debugging easier than a bare grep which fails
> silently.
>
> Only assertion-style greps are converted: grep used as a filter
> in pipelines, command substitutions, conditionals, or with
> redirected I/O is left as-is with a "# lint-ok" annotation.
> Existing '! test_grep' calls are rewritten to 'test_grep !' so
> that the diagnostic output is preserved on failure.
Thanks for taking the effort for cleaning up all those negated '!
grep' and '! test_grep' callsites and turning them into 'test_grep !'.
> The conversion was generated using a grep-assertion linter
> (greplint.pl, added in the following commit) to identify bare
> grep calls at command position. To reproduce:
>
> # Step 1: mark bare greps that should not be converted
> sed -i '/! grep "$m" \.git\/packed-refs/s/$/ # lint-ok: file may not exist (reftable)/' \
> t/t1400-update-ref.sh
> sed -i '/! grep dirty file3 &&/{/lint-ok/!s/$/ # lint-ok: file may not exist after --quit/}' \
> t/t3420-rebase-autostash.sh
I think in this case checking the file3's contents is wrong, because
at this point file3 should not exist in the first place. I've sent a
patch to fix this long ago, but apparently didn't manage to follow
through back then.
https://lore.kernel.org/git/20211010172809.1472914-1-szeder.dev@gmail.com/
> diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh
> index f0bbc476ff..f6cb3dd72e 100755
> --- a/t/t3420-rebase-autostash.sh
> +++ b/t/t3420-rebase-autostash.sh
> @@ -141,8 +141,8 @@ testrebase () {
> git checkout -b rebased-feature-branch feature-branch &&
> echo dirty >>file3 &&
> git rebase$type unrelated-onto-branch >actual 2>&1 &&
> - grep unrelated file4 &&
> - grep dirty file3 &&
> + test_grep unrelated file4 &&
> + test_grep dirty file3 &&
> git checkout feature-branch
> '
>
> @@ -165,8 +165,8 @@ testrebase () {
> echo dirty >>file3 &&
> git add file3 &&
> git rebase$type unrelated-onto-branch &&
> - grep unrelated file4 &&
> - grep dirty file3 &&
> + test_grep unrelated file4 &&
> + test_grep dirty file3 &&
> git checkout feature-branch
> '
>
> @@ -197,7 +197,7 @@ testrebase () {
> git add file2 &&
> git rebase --continue &&
> test_path_is_missing $dotest/autostash &&
> - grep dirty file3 &&
> + test_grep dirty file3 &&
> git checkout feature-branch
> '
>
> @@ -212,7 +212,7 @@ testrebase () {
> test_path_is_missing file3 &&
> git rebase --skip &&
> test_path_is_missing $dotest/autostash &&
> - grep dirty file3 &&
> + test_grep dirty file3 &&
> git checkout feature-branch
> '
>
> @@ -227,7 +227,7 @@ testrebase () {
> test_path_is_missing file3 &&
> git rebase --abort &&
> test_path_is_missing $dotest/autostash &&
> - grep dirty file3 &&
> + test_grep dirty file3 &&
> git checkout feature-branch
> '
>
> @@ -244,7 +244,7 @@ testrebase () {
> git rebase --quit &&
> test_when_finished git stash drop &&
> test_path_is_missing $dotest/autostash &&
> - ! grep dirty file3 &&
> + ! grep dirty file3 && # lint-ok: file may not exist after --quit
> git stash show -p >actual &&
> test_cmp expect actual &&
> git reset --hard &&
> @@ -260,11 +260,11 @@ testrebase () {
> git rebase$type unrelated-onto-branch >actual 2>&1 &&
> test_path_is_missing $dotest &&
> git reset --hard &&
> - grep unrelated file4 &&
> - ! grep dirty file4 &&
> + test_grep unrelated file4 &&
> + test_grep ! dirty file4 &&
> git checkout feature-branch &&
> git stash pop &&
> - grep dirty file4
> + test_grep dirty file4
> '
>
> test_expect_success "rebase$type: check output with conflicting stash" '
> @@ -286,7 +286,7 @@ test_expect_success "rebase: fast-forward rebase" '
> test_when_finished git branch -D behind-feature-branch &&
> echo dirty >>file1 &&
> git rebase feature-branch &&
> - grep dirty file1 &&
> + test_grep dirty file1 &&
> git checkout feature-branch
> '
>
> @@ -297,7 +297,7 @@ test_expect_success "rebase: noop rebase" '
> test_when_finished git branch -D same-feature-branch &&
> echo dirty >>file1 &&
> git rebase feature-branch &&
> - grep dirty file1 &&
> + test_grep dirty file1 &&
> git checkout feature-branch
> '
>
next prev parent reply other threads:[~2026-06-27 7:08 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-04 7:45 [PATCH 0/6] t: add lint-style.pl and convert grep to test_grep Michael Montalbo via GitGitGadget
2026-06-04 7:45 ` [PATCH 1/6] t/README: document test_grep helper Michael Montalbo via GitGitGadget
2026-06-04 7:45 ` [PATCH 2/6] t: extract chainlint's parser into shared module Michael Montalbo via GitGitGadget
2026-06-04 7:45 ` [PATCH 3/6] t: fix Lexer line count for $() inside double-quoted strings Michael Montalbo via GitGitGadget
2026-06-04 7:45 ` [PATCH 4/6] t: add lint-style.pl with test_grep negation rule Michael Montalbo via GitGitGadget
2026-06-04 18:34 ` D. Ben Knoble
2026-06-04 19:36 ` Michael Montalbo
2026-06-04 7:45 ` [PATCH 5/6] t: fix grep assertions missing file arguments Michael Montalbo via GitGitGadget
2026-06-04 7:45 ` [PATCH 6/6] t: lint and convert grep assertions to test_grep Michael Montalbo via GitGitGadget
2026-06-08 21:36 ` [PATCH 0/6] t: add lint-style.pl and convert grep " Junio C Hamano
2026-06-13 16:28 ` Michael Montalbo
2026-06-13 4:06 ` [PATCH v2 0/6] t: add greplint.pl " Michael Montalbo via GitGitGadget
2026-06-13 4:06 ` [PATCH v2 1/6] t/README: document test_grep helper Michael Montalbo via GitGitGadget
2026-06-13 4:06 ` [PATCH v2 2/6] t: fix grep assertions missing file arguments Michael Montalbo via GitGitGadget
2026-06-13 4:06 ` [PATCH v2 3/6] t: extract chainlint's parser into shared module Michael Montalbo via GitGitGadget
2026-06-13 4:06 ` [PATCH v2 4/6] t: fix Lexer line count for $() inside double-quoted strings Michael Montalbo via GitGitGadget
2026-06-13 4:06 ` [PATCH v2 5/6] t: convert grep assertions to test_grep Michael Montalbo via GitGitGadget
2026-06-27 7:08 ` SZEDER Gábor [this message]
2026-06-13 4:06 ` [PATCH v2 6/6] t: add greplint to detect bare grep assertions Michael Montalbo via GitGitGadget
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=aj93BE8MYatQAjoy@szeder.dev \
--to=szeder.dev@gmail.com \
--cc=ben.knoble@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=mmontalbo@gmail.com \
--cc=sunshine@sunshineco.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