From: Junio C Hamano <gitster@pobox.com>
To: PRASHANT S BISHT <prashantjee2025@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v3] t4200-rerere: use test_path helpers
Date: Mon, 16 Mar 2026 09:50:59 -0700 [thread overview]
Message-ID: <xmqqqzpjpvkc.fsf@gitster.g> (raw)
In-Reply-To: <20260316070812.84760-1-prashantjee2025@gmail.com> (PRASHANT S. BISHT's message of "Mon, 16 Mar 2026 12:38:12 +0530")
PRASHANT S BISHT <prashantjee2025@gmail.com> writes:
> Convert assertion-style test -[df] checks in t4200-rerere.sh to test_path_* helpers.
>
> Instances were found with:
>
> git grep 'test -[def]' t/t4200-rerere.sh
People seem to add the above to their test-path helper patches, but
unless the coverage of the work is fairly thorough and you want to
say "all the similar issues should be found with this command and I
addressed all of them", I do not see much point saying how you found
one of them and addressed it.
You could have used "git grep -e <pattern> -- t/\*.sh", or you could
have been working to fix something in t9200 and noticed these while
you were doing something else to the file.
I do not see it as too huge a deal and it is probably not a cause to
send in another iteration once it is already written, though.
>
> ---
But the lack of sign-off is reason enough to require an update.
For an "introduction to the project" patch, the change itself is not
as important or interesting than demonstrating that you studied
CodingGuidelines and SubmittingPatches well.
> t/t4200-rerere.sh | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
> index 204325f4d5..1717f407c8 100755
> --- a/t/t4200-rerere.sh
> +++ b/t/t4200-rerere.sh
> @@ -72,7 +72,7 @@ test_expect_success 'nothing recorded without rerere' '
> rm -rf .git/rr-cache &&
> git config rerere.enabled false &&
> test_must_fail git merge first &&
> - ! test -d .git/rr-cache
> + test_path_is_missing .git/rr-cache
> '
>
> test_expect_success 'activate rerere, old style (conflicting merge)' '
> @@ -84,8 +84,8 @@ test_expect_success 'activate rerere, old style (conflicting merge)' '
> sha1=$(sed "s/ .*//" .git/MERGE_RR) &&
> rr=.git/rr-cache/$sha1 &&
> grep "^=======\$" $rr/preimage &&
> - ! test -f $rr/postimage &&
> - ! test -f $rr/thisimage
> + test_path_is_missing $rr/postimage &&
> + test_path_is_missing $rr/thisimage
> '
>
> test_expect_success 'rerere.enabled works, too' '
> @@ -110,8 +110,8 @@ test_expect_success 'set up rr-cache' '
>
> test_expect_success 'rr-cache looks sane' '
> # no postimage or thisimage yet
> - ! test -f $rr/postimage &&
> - ! test -f $rr/thisimage &&
> + test_path_is_missing $rr/postimage &&
> + test_path_is_missing $rr/thisimage &&
>
> # preimage has right number of lines
> cnt=$(sed -ne "/^<<<<<<</,/^>>>>>>>/p" $rr/preimage | wc -l) &&
> @@ -167,7 +167,7 @@ test_expect_success 'first postimage wins' '
> git show first:a1 | sed "s/To die: t/To die! T/" >expect &&
>
> git commit -q -a -m "prefer first over second" &&
> - test -f $rr/postimage &&
> + test_path_is_file $rr/postimage &&
>
> oldmtimepost=$(test-tool chmtime --get -60 $rr/postimage) &&
>
> @@ -190,14 +190,14 @@ test_expect_success 'rerere clear' '
> mv $rr/postimage .git/post-saved &&
> echo "$sha1 a1" | tr "\012" "\000" >.git/MERGE_RR &&
> git rerere clear &&
> - ! test -d $rr
> + test_path_is_missing $rr
> '
>
> test_expect_success 'leftover directory' '
> git reset --hard &&
> mkdir -p $rr &&
> test_must_fail git merge first &&
> - test -f $rr/preimage
> + test_path_is_file $rr/preimage
> '
>
> test_expect_success 'missing preimage' '
> @@ -205,7 +205,7 @@ test_expect_success 'missing preimage' '
> mkdir -p $rr &&
> cp .git/post-saved $rr/postimage &&
> test_must_fail git merge first &&
> - test -f $rr/preimage
> + test_path_is_file $rr/preimage
> '
>
> test_expect_success 'set up for garbage collection tests' '
> @@ -230,16 +230,16 @@ test_expect_success 'set up for garbage collection tests' '
>
> test_expect_success 'gc preserves young or recently used records' '
> git rerere gc &&
> - test -f $rr/preimage &&
> - test -f $rr2/preimage
> + test_path_is_file $rr/preimage &&
> + test_path_is_file $rr2/preimage
> '
>
> test_expect_success 'old records rest in peace' '
> test-tool chmtime =$just_over_60_days_ago $rr/postimage &&
> test-tool chmtime =$just_over_15_days_ago $rr2/preimage &&
> git rerere gc &&
> - ! test -f $rr/preimage &&
> - ! test -f $rr2/preimage
> + test_path_is_missing $rr/preimage &&
> + test_path_is_missing $rr2/preimage
> '
>
> rerere_gc_custom_expiry_test () {
prev parent reply other threads:[~2026-03-16 16:51 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-16 7:08 [PATCH v3] t4200-rerere: use test_path helpers PRASHANT S BISHT
2026-03-16 16:50 ` Junio C Hamano [this message]
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=xmqqqzpjpvkc.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=prashantjee2025@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