Git development
 help / color / mirror / Atom feed
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
>  '
>  

  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