# [GSoC PATCH] t9200: use helpers to replace test -f and test -d 16 messages from 2026-03-09 to 2026-03-12. Participants: Pablo Sabater, Junio C Hamano, Pablo. Thread: https://gitlist.dev/t/65177 ## Pablo Sabater, 2026-03-09 15:09 Subject: [GSoC PATCH] t9200: use helpers to replace test -f and test -d Message-ID: <20260309150935.578465-1-pabloosabaterr@gmail.com> URL: https://gitlist.dev/e/20260309150935.578465-1-pabloosabaterr%40gmail.com ``` Replaced 'test -f' and 'test -d' with 'test_path_is_file' and 'test_path_is_dir' I've used 'git grep "test -f" t/t9*.sh' to find a file without the fix done as specified on the microproject information I've done '9*' because the ones I've found first had already fix patches. I've taken as example another patch sent 't4131' from Junio C Hamano https://lore.kernel.org/git/xmqq1rpodn25.fsf@gitster.c.googlers.com/#r Signed-off-by: Pablo Sabater --- t/t9200-git-cvsexportcommit.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh index a44eabf0d8..4507e8e6db 100755 --- a/t/t9200-git-cvsexportcommit.sh +++ b/t/t9200-git-cvsexportcommit.sh @@ -31,7 +31,7 @@ export CVSROOT CVSWORK GIT_DIR rm -rf "$CVSROOT" "$CVSWORK" cvs init && -test -d "$CVSROOT" && +test_path_is_dir "$CVSROOT" && cvs -Q co -d "$CVSWORK" . && echo >empty && git add empty && @@ -303,7 +303,7 @@ test_expect_success 're-commit a removed filename which remains in CVS attic' ' git commit -m "Added attic_gremlin" && git cvsexportcommit -w "$CVSWORK" -c HEAD && (cd "$CVSWORK" && cvs -Q update -d) && - test -f "$CVSWORK/attic_gremlin" + test_path_is_file "$CVSWORK/attic_gremlin" ' # the state of the CVS sandbox may be indeterminate for ' space' -- 2.43.0 ``` ## Junio C Hamano, 2026-03-09 15:20 Subject: Re: [GSoC PATCH] t9200: use helpers to replace test -f and test -d Message-ID: URL: https://gitlist.dev/e/xmqqo6kx58si.fsf%40gitster.g In-Reply-To: <20260309150935.578465-1-pabloosabaterr@gmail.com> ``` Pablo Sabater writes: > Replaced 'test -f' and 'test -d' with 'test_path_is_file' and 'test_path_is_dir' > > I've used 'git grep "test -f" t/t9*.sh' to find a file without the fix done as specified on the microproject information > I've done '9*' because the ones I've found first had already fix patches. > I've taken as example another patch sent 't4131' from Junio C Hamano https://lore.kernel.org/git/xmqq1rpodn25.fsf@gitster.c.googlers.com/#r After studying Documentation/{SubmittingPatches,CodingGuidelines}, use the list archive to find what instructions GSoC participant candidates have received regarding the proposed log messages in their microproject submissions. Thanks. ``` ## Pablo, 2026-03-09 15:39 Subject: Re: [GSoC PATCH] t9200: use helpers to replace test -f and test -d Message-ID: URL: https://gitlist.dev/e/CAN5EUNQzbr50JZ4DPpyWRLjx0Wgki1rFHm%3DOPEiD2LjeQ52ytg%40mail.gmail.com In-Reply-To: ``` > After studying Documentation/{SubmittingPatches,CodingGuidelines}, > use the list archive to find what instructions GSoC participant > candidates have received regarding the proposed log messages in > their microproject submissions. Thanks for the feedback, I'll work on that right now, once done I'll send a v2. Pablo ``` ## Pablo Sabater, 2026-03-09 16:28 Subject: [GSoC PATCH v2] t9200: replace test -f/-d with modern path helpers Message-ID: <20260309162832.605969-1-pabloosabaterr@gmail.com> URL: https://gitlist.dev/e/20260309162832.605969-1-pabloosabaterr%40gmail.com In-Reply-To: <20260309150935.578465-1-pabloosabaterr@gmail.com> ``` Replace old style 'test -f' and 'test -d' with modern helpers 'test_path_is_file' and 'test_path_is_dir' respectively. The instances were found with: git grep "test -[efd]" t/ Signed-off-by: Pablo Sabater --- t/t9200-git-cvsexportcommit.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh index a44eabf0d8..4507e8e6db 100755 --- a/t/t9200-git-cvsexportcommit.sh +++ b/t/t9200-git-cvsexportcommit.sh @@ -31,7 +31,7 @@ export CVSROOT CVSWORK GIT_DIR rm -rf "$CVSROOT" "$CVSWORK" cvs init && -test -d "$CVSROOT" && +test_path_is_dir "$CVSROOT" && cvs -Q co -d "$CVSWORK" . && echo >empty && git add empty && @@ -303,7 +303,7 @@ test_expect_success 're-commit a removed filename which remains in CVS attic' ' git commit -m "Added attic_gremlin" && git cvsexportcommit -w "$CVSWORK" -c HEAD && (cd "$CVSWORK" && cvs -Q update -d) && - test -f "$CVSWORK/attic_gremlin" + test_path_is_file "$CVSWORK/attic_gremlin" ' # the state of the CVS sandbox may be indeterminate for ' space' -- 2.43.0 ``` ## Junio C Hamano, 2026-03-09 21:03 Subject: Re: [GSoC PATCH v2] t9200: replace test -f/-d with modern path helpers Message-ID: URL: https://gitlist.dev/e/xmqq8qc04sxh.fsf%40gitster.g In-Reply-To: <20260309162832.605969-1-pabloosabaterr@gmail.com> ``` Pablo Sabater writes: > Replace old style 'test -f' and 'test -d' with modern helpers > 'test_path_is_file' and 'test_path_is_dir' respectively. OK. Being "modern" does not automatically mean "better", and it would be helpful to say why we do this change for those relatively unexperienced who will read "git log" later and find this commit. Perhaps Replace ... with ..., because it makes debugging a failing test easier by loudly reporting what expectation was not met. or something. The patch text looks good. Thanks. > The instances were found with: > > git grep "test -[efd]" t/ > > Signed-off-by: Pablo Sabater > --- > t/t9200-git-cvsexportcommit.sh | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) OK. > diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh > index a44eabf0d8..4507e8e6db 100755 > --- a/t/t9200-git-cvsexportcommit.sh > +++ b/t/t9200-git-cvsexportcommit.sh > @@ -31,7 +31,7 @@ export CVSROOT CVSWORK GIT_DIR > rm -rf "$CVSROOT" "$CVSWORK" > > cvs init && > -test -d "$CVSROOT" && > +test_path_is_dir "$CVSROOT" && > cvs -Q co -d "$CVSWORK" . && > echo >empty && > git add empty && > @@ -303,7 +303,7 @@ test_expect_success 're-commit a removed filename which remains in CVS attic' ' > git commit -m "Added attic_gremlin" && > git cvsexportcommit -w "$CVSWORK" -c HEAD && > (cd "$CVSWORK" && cvs -Q update -d) && > - test -f "$CVSWORK/attic_gremlin" > + test_path_is_file "$CVSWORK/attic_gremlin" > ' > > # the state of the CVS sandbox may be indeterminate for ' space' ``` ## Pablo, 2026-03-09 22:54 Subject: Re: [GSoC PATCH v2] t9200: replace test -f/-d with modern path helpers Message-ID: URL: https://gitlist.dev/e/CAN5EUNQZsrtAUzQ5GgyFg5vJ-aMvAVLAQCYK6aahOSUoPa0dOQ%40mail.gmail.com In-Reply-To: ``` Junio C Hamano writes: > OK. Being "modern" does not automatically mean "better", and it > would be helpful to say why we do this change for those relatively > unexperienced who will read "git log" later and find this commit. > Perhaps Okay, thanks. I'll do that on the v3 Pablo ``` ## Pablo Sabater, 2026-03-09 23:01 Subject: [GSoC PATCH v3] t9200: replace test -f/-d with modern path helpers Message-ID: <20260309230134.758107-1-pabloosabaterr@gmail.com> URL: https://gitlist.dev/e/20260309230134.758107-1-pabloosabaterr%40gmail.com In-Reply-To: <20260309150935.578465-1-pabloosabaterr@gmail.com> ``` Replace old style 'test -f' and 'test -d' with helpers 'test_path_is_file' and 'test_path_is_dir' respectively, which make debugging a failing test easier by loudly reporting what expectation was not met. The instances were found with: git grep "test -[efd]" t/ Signed-off-by: Pablo Sabater --- t/t9200-git-cvsexportcommit.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh index a44eabf0d8..4507e8e6db 100755 --- a/t/t9200-git-cvsexportcommit.sh +++ b/t/t9200-git-cvsexportcommit.sh @@ -31,7 +31,7 @@ export CVSROOT CVSWORK GIT_DIR rm -rf "$CVSROOT" "$CVSWORK" cvs init && -test -d "$CVSROOT" && +test_path_is_dir "$CVSROOT" && cvs -Q co -d "$CVSWORK" . && echo >empty && git add empty && @@ -303,7 +303,7 @@ test_expect_success 're-commit a removed filename which remains in CVS attic' ' git commit -m "Added attic_gremlin" && git cvsexportcommit -w "$CVSWORK" -c HEAD && (cd "$CVSWORK" && cvs -Q update -d) && - test -f "$CVSWORK/attic_gremlin" + test_path_is_file "$CVSWORK/attic_gremlin" ' # the state of the CVS sandbox may be indeterminate for ' space' -- 2.43.0 ``` ## Junio C Hamano, 2026-03-09 23:26 Subject: Re: [GSoC PATCH v3] t9200: replace test -f/-d with modern path helpers Message-ID: URL: https://gitlist.dev/e/xmqqldg01t5o.fsf%40gitster.g In-Reply-To: <20260309230134.758107-1-pabloosabaterr@gmail.com> ``` Pablo Sabater writes: > Replace old style 'test -f' and 'test -d' with helpers > 'test_path_is_file' and 'test_path_is_dir' respectively, > which make debugging a failing test easier by loudly > reporting what expectation was not met. > > The instances were found with: > > git grep "test -[efd]" t/ > > Signed-off-by: Pablo Sabater > --- > t/t9200-git-cvsexportcommit.sh | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Looking good. Will queue. Thanks. > diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh > index a44eabf0d8..4507e8e6db 100755 > --- a/t/t9200-git-cvsexportcommit.sh > +++ b/t/t9200-git-cvsexportcommit.sh > @@ -31,7 +31,7 @@ export CVSROOT CVSWORK GIT_DIR > rm -rf "$CVSROOT" "$CVSWORK" > > cvs init && > -test -d "$CVSROOT" && > +test_path_is_dir "$CVSROOT" && > cvs -Q co -d "$CVSWORK" . && > echo >empty && > git add empty && > @@ -303,7 +303,7 @@ test_expect_success 're-commit a removed filename which remains in CVS attic' ' > git commit -m "Added attic_gremlin" && > git cvsexportcommit -w "$CVSWORK" -c HEAD && > (cd "$CVSWORK" && cvs -Q update -d) && > - test -f "$CVSWORK/attic_gremlin" > + test_path_is_file "$CVSWORK/attic_gremlin" > ' > > # the state of the CVS sandbox may be indeterminate for ' space' ``` ## Pablo, 2026-03-11 10:59 Subject: Re: [GSoC PATCH v3] t9200: replace test -f/-d with modern path helpers Message-ID: URL: https://gitlist.dev/e/CAN5EUNTNu5NYgeJ0OQSS25Ld_kJqtEEPGcXrg_EcyRdCkr5ORw%40mail.gmail.com In-Reply-To: <20260309230134.758107-1-pabloosabaterr@gmail.com> ``` Adding project mentors to CC. Thanks. El mar, 10 mar 2026 a las 0:01, Pablo Sabater () escribió: > > Replace old style 'test -f' and 'test -d' with helpers > 'test_path_is_file' and 'test_path_is_dir' respectively, > which make debugging a failing test easier by loudly > reporting what expectation was not met. > > The instances were found with: > > git grep "test -[efd]" t/ > > Signed-off-by: Pablo Sabater > --- > t/t9200-git-cvsexportcommit.sh | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh > index a44eabf0d8..4507e8e6db 100755 > --- a/t/t9200-git-cvsexportcommit.sh > +++ b/t/t9200-git-cvsexportcommit.sh > @@ -31,7 +31,7 @@ export CVSROOT CVSWORK GIT_DIR > rm -rf "$CVSROOT" "$CVSWORK" > > cvs init && > -test -d "$CVSROOT" && > +test_path_is_dir "$CVSROOT" && > cvs -Q co -d "$CVSWORK" . && > echo >empty && > git add empty && > @@ -303,7 +303,7 @@ test_expect_success 're-commit a removed filename which remains in CVS attic' ' > git commit -m "Added attic_gremlin" && > git cvsexportcommit -w "$CVSWORK" -c HEAD && > (cd "$CVSWORK" && cvs -Q update -d) && > - test -f "$CVSWORK/attic_gremlin" > + test_path_is_file "$CVSWORK/attic_gremlin" > ' > > # the state of the CVS sandbox may be indeterminate for ' space' > -- > 2.43.0 > ``` ## Junio C Hamano, 2026-03-11 17:52 Subject: Re: [GSoC PATCH v3] t9200: replace test -f/-d with modern path helpers Message-ID: URL: https://gitlist.dev/e/xmqqwlzip82b.fsf%40gitster.g In-Reply-To: <20260309230134.758107-1-pabloosabaterr@gmail.com> ``` Pablo Sabater writes: > Replace old style 'test -f' and 'test -d' with helpers > 'test_path_is_file' and 'test_path_is_dir' respectively, > which make debugging a failing test easier by loudly > reporting what expectation was not met. Well explained. > The instances were found with: > > git grep "test -[efd]" t/ 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 -- 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. > Signed-off-by: Pablo Sabater > --- > t/t9200-git-cvsexportcommit.sh | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh > index a44eabf0d8..4507e8e6db 100755 > --- a/t/t9200-git-cvsexportcommit.sh > +++ b/t/t9200-git-cvsexportcommit.sh > @@ -31,7 +31,7 @@ export CVSROOT CVSWORK GIT_DIR > rm -rf "$CVSROOT" "$CVSWORK" > > cvs init && > -test -d "$CVSROOT" && > +test_path_is_dir "$CVSROOT" && > cvs -Q co -d "$CVSWORK" . && > echo >empty && > git add empty && Our test-path helpers should work even outside test_expect_* functions, so this is not wrong per-se, but it somehow looks a bit unusual. A related clean-up would be to wrap the CVS initialization part inside another "do we even have a working CVS installation to make it worth our time testing 'git cvsexportcommit' command?" check, i.e., if ! cvs init || ! test -d "$CVSROOT" || ! cvs -Q co -d "$CVSWORK" . then skip_all="cvs repository set-up fails" test_done fi and then move the git initialization part to its own test, e.g., test_expect_success 'git setup' ' echo >empty && git add empty && git commit -q -a -m Initial ' > @@ -303,7 +303,7 @@ test_expect_success 're-commit a removed filename which remains in CVS attic' ' > git commit -m "Added attic_gremlin" && > git cvsexportcommit -w "$CVSWORK" -c HEAD && > (cd "$CVSWORK" && cvs -Q update -d) && > - test -f "$CVSWORK/attic_gremlin" > + test_path_is_file "$CVSWORK/attic_gremlin" > ' OK. > > # the state of the CVS sandbox may be indeterminate for ' space' ``` ## Pablo, 2026-03-11 19:06 Subject: Re: [GSoC PATCH v3] t9200: replace test -f/-d with modern path helpers Message-ID: URL: https://gitlist.dev/e/CAN5EUNRZQP6ATE87AeZiJx-OTnNn_4NxhW4zyH6AspGUfnV7TA%40mail.gmail.com In-Reply-To: ``` > Our test-path helpers should work even outside test_expect_* > functions, so this is not wrong per-se, but it somehow looks a bit > unusual. A related clean-up would be to wrap the CVS initialization > part inside another "do we even have a working CVS installation to > make it worth our time testing 'git cvsexportcommit' command?" Thanks for the feedback, I can send a separate patch to wrap the CVS in a skip_all git move the git setup ``` ## Junio C Hamano, 2026-03-11 19:42 Subject: Re: [GSoC PATCH v3] t9200: replace test -f/-d with modern path helpers Message-ID: URL: https://gitlist.dev/e/xmqqbjgunofq.fsf%40gitster.g In-Reply-To: ``` Pablo writes: >> Our test-path helpers should work even outside test_expect_* >> functions, so this is not wrong per-se, but it somehow looks a bit >> unusual. A related clean-up would be to wrap the CVS initialization >> part inside another "do we even have a working CVS installation to >> make it worth our time testing 'git cvsexportcommit' command?" > > > Thanks for the feedback, I can send a separate patch to wrap the CVS > in a skip_all git move the git setup Yeah, but if we are going to do so eventually, it would be pointless to use the path helper in that "set up CVS environment and make sure we got a sensible directory structure" check, no? Upon failure, we will hit test_done that loudly says that their CVS installation is not working as we expect. ``` ## Pablo, 2026-03-11 19:49 Subject: Re: [GSoC PATCH v3] t9200: replace test -f/-d with modern path helpers Message-ID: URL: https://gitlist.dev/e/CAN5EUNSmZmdnDzpAKAh8fZRex3--tnKaWZZSQ%2Bo5WATc6sLy_Q%40mail.gmail.com In-Reply-To: ``` > Yeah, but if we are going to do so eventually, it would be pointless > to use the path helper in that "set up CVS environment and make sure > we got a sensible directory structure" check, no? Upon failure, we > will hit test_done that loudly says that their CVS installation is > not working as we expect. Yeah, the new patch will change it back to test -d because it ends up in a if condition instead of an assertion. Would you prefer to drop that hunk from my v3 or should I send a v4 ? ``` ## Junio C Hamano, 2026-03-11 20:31 Subject: Re: [GSoC PATCH v3] t9200: replace test -f/-d with modern path helpers Message-ID: URL: https://gitlist.dev/e/xmqqtsumm7kf.fsf%40gitster.g In-Reply-To: ``` Pablo writes: >> Yeah, but if we are going to do so eventually, it would be pointless >> to use the path helper in that "set up CVS environment and make sure >> we got a sensible directory structure" check, no? Upon failure, we >> will hit test_done that loudly says that their CVS installation is >> not working as we expect. > > Yeah, the new patch will change it back to test -d because it ends up > in a if condition instead of an assertion. > Would you prefer to drop that hunk from my v3 or should I send a v4 ? Yup, let me mark the "cvs setup failure" one ready for 'next'. The other hunk that updates "test -[efd]" can become a separate patch. Thanks. ``` ## Pablo Sabater, 2026-03-12 17:33 Subject: [GSoC PATCH v4] t9200: replace test -f with modern path helper Message-ID: <20260312173305.15112-1-pabloosabaterr@gmail.com> URL: https://gitlist.dev/e/20260312173305.15112-1-pabloosabaterr%40gmail.com In-Reply-To: <20260309150935.578465-1-pabloosabaterr@gmail.com> ``` Replace old style 'test -f' with helper 'test_path_is_file', which make debugging a failing test easier by loudly reporting what expectation was not met. Signed-off-by: Pablo Sabater --- Changes from v3: The first hunk was dropped from this patch, and sent as a separate patch. https://lore.kernel.org/git/20260311194002.190195-1-pabloosabaterr@gmail.com/ t/t9200-git-cvsexportcommit.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh index a44eabf0d8..15a91931a2 100755 --- a/t/t9200-git-cvsexportcommit.sh +++ b/t/t9200-git-cvsexportcommit.sh @@ -303,7 +303,7 @@ test_expect_success 're-commit a removed filename which remains in CVS attic' ' git commit -m "Added attic_gremlin" && git cvsexportcommit -w "$CVSWORK" -c HEAD && (cd "$CVSWORK" && cvs -Q update -d) && - test -f "$CVSWORK/attic_gremlin" + test_path_is_file "$CVSWORK/attic_gremlin" ' # the state of the CVS sandbox may be indeterminate for ' space' -- 2.43.0 ``` ## Junio C Hamano, 2026-03-12 17:37 Subject: Re: [GSoC PATCH v4] t9200: replace test -f with modern path helper Message-ID: URL: https://gitlist.dev/e/xmqqbjgteyot.fsf%40gitster.g In-Reply-To: <20260312173305.15112-1-pabloosabaterr@gmail.com> ``` Pablo Sabater writes: > Replace old style 'test -f' with helper > 'test_path_is_file', which make debugging > a failing test easier by loudly reporting > what expectation was not met. > > Signed-off-by: Pablo Sabater > --- Will queue. Looks good. Thanks. > Changes from v3: > The first hunk was dropped from this patch, and sent as a separate patch. > https://lore.kernel.org/git/20260311194002.190195-1-pabloosabaterr@gmail.com/ > > t/t9200-git-cvsexportcommit.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh > index a44eabf0d8..15a91931a2 100755 > --- a/t/t9200-git-cvsexportcommit.sh > +++ b/t/t9200-git-cvsexportcommit.sh > @@ -303,7 +303,7 @@ test_expect_success 're-commit a removed filename which remains in CVS attic' ' > git commit -m "Added attic_gremlin" && > git cvsexportcommit -w "$CVSWORK" -c HEAD && > (cd "$CVSWORK" && cvs -Q update -d) && > - test -f "$CVSWORK/attic_gremlin" > + test_path_is_file "$CVSWORK/attic_gremlin" > ' > > # the state of the CVS sandbox may be indeterminate for ' space' ```