Re: [PATCH v3] t4200-rerere: use test_path helpers
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 16, 2026, 16:50 UTC
- Message-ID
- <xmqqqzpjpvkc.fsf@gitster.g>
- In-Reply-To
- <20260316070812.84760-1-prashantjee2025@gmail.com>
PRASHANT S BISHT <prashantjee2025@gmail.com> writes:
Show 5 quoted lines
> 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.
Show 94 quoted lines
> 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 () {