{"thread":{"id":"65177","subject":"[GSoC PATCH] t9200: use helpers to replace test -f <path> and test -d <path>","startedAt":"2026-03-09T15:10:05Z","lastAt":"2026-03-12T17:37:40Z","messageCount":16,"participants":["Pablo Sabater","Junio C Hamano","Pablo"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"538285","messageId":"20260309150935.578465-1-pabloosabaterr@gmail.com","threadId":"65177","inReplyTo":null,"subject":"[GSoC PATCH] t9200: use helpers to replace test -f <path> and test -d <path>","fromName":"Pablo Sabater","fromEmail":"pabloosabaterr@gmail.com","sentAt":"2026-03-09T15:09:35Z","receivedAt":"2026-03-09T15:10:05Z","isPatch":true,"sender":{"key":"pabloosabaterr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/63864637?v=4"},"body":"Replaced 'test -f' and 'test -d' with 'test_path_is_file' and 'test_path_is_dir'\n\nI've used 'git grep \"test -f\" t/t9*.sh' to find a file without the fix done as specified on the microproject information\nI've done '9*' because the ones I've found first had already fix patches.\nI've taken as example another patch sent 't4131' from Junio C Hamano https://lore.kernel.org/git/xmqq1rpodn25.fsf@gitster.c.googlers.com/#r\n\nSigned-off-by: Pablo Sabater <pabloosabaterr@gmail.com>\n---\n t/t9200-git-cvsexportcommit.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh\nindex a44eabf0d8..4507e8e6db 100755\n--- a/t/t9200-git-cvsexportcommit.sh\n+++ b/t/t9200-git-cvsexportcommit.sh\n@@ -31,7 +31,7 @@ export CVSROOT CVSWORK GIT_DIR\n rm -rf \"$CVSROOT\" \"$CVSWORK\"\n \n cvs init &&\n-test -d \"$CVSROOT\" &&\n+test_path_is_dir \"$CVSROOT\" &&\n cvs -Q co -d \"$CVSWORK\" . &&\n echo >empty &&\n git add empty &&\n@@ -303,7 +303,7 @@ test_expect_success 're-commit a removed filename which remains in CVS attic' '\n \tgit commit -m \"Added attic_gremlin\" &&\n \tgit cvsexportcommit -w \"$CVSWORK\" -c HEAD &&\n \t(cd \"$CVSWORK\" && cvs -Q update -d) &&\n-\ttest -f \"$CVSWORK/attic_gremlin\"\n+\ttest_path_is_file \"$CVSWORK/attic_gremlin\"\n '\n \n # the state of the CVS sandbox may be indeterminate for ' space'\n-- \n2.43.0\n\n"},{"id":"538289","messageId":"xmqqo6kx58si.fsf@gitster.g","threadId":"65177","inReplyTo":"20260309150935.578465-1-pabloosabaterr@gmail.com","subject":"Re: [GSoC PATCH] t9200: use helpers to replace test -f <path> and test -d <path>","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-09T15:20:29Z","receivedAt":"2026-03-09T15:20:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pablo Sabater <pabloosabaterr@gmail.com> writes:\n\n> Replaced 'test -f' and 'test -d' with 'test_path_is_file' and 'test_path_is_dir'\n>\n> I've used 'git grep \"test -f\" t/t9*.sh' to find a file without the fix done as specified on the microproject information\n> I've done '9*' because the ones I've found first had already fix patches.\n> 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\n\nAfter studying Documentation/{SubmittingPatches,CodingGuidelines},\nuse the list archive to find what instructions GSoC participant\ncandidates have received regarding the proposed log messages in\ntheir microproject submissions.\n\nThanks.\n"},{"id":"538293","messageId":"CAN5EUNQzbr50JZ4DPpyWRLjx0Wgki1rFHm=OPEiD2LjeQ52ytg@mail.gmail.com","threadId":"65177","inReplyTo":"xmqqo6kx58si.fsf@gitster.g","subject":"Re: [GSoC PATCH] t9200: use helpers to replace test -f <path> and test -d <path>","fromName":"Pablo","fromEmail":"pabloosabaterr@gmail.com","sentAt":"2026-03-09T15:39:22Z","receivedAt":"2026-03-09T15:39:38Z","isPatch":true,"sender":{"key":"pabloosabaterr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/63864637?v=4"},"body":"> After studying Documentation/{SubmittingPatches,CodingGuidelines},\n> use the list archive to find what instructions GSoC participant\n> candidates have received regarding the proposed log messages in\n> their microproject submissions.\n\nThanks for the feedback, I'll work on that right now, once done I'll send a v2.\n\nPablo\n"},{"id":"538297","messageId":"20260309162832.605969-1-pabloosabaterr@gmail.com","threadId":"65177","inReplyTo":"20260309150935.578465-1-pabloosabaterr@gmail.com","subject":"[GSoC PATCH v2] t9200: replace test -f/-d with modern path helpers","fromName":"Pablo Sabater","fromEmail":"pabloosabaterr@gmail.com","sentAt":"2026-03-09T16:28:32Z","receivedAt":"2026-03-09T16:30:07Z","isPatch":true,"sender":{"key":"pabloosabaterr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/63864637?v=4"},"body":"Replace old style 'test -f' and 'test -d' with modern helpers\n'test_path_is_file' and 'test_path_is_dir' respectively.\n\nThe instances were found with:\n\n\tgit grep \"test -[efd]\" t/\n\nSigned-off-by: Pablo Sabater <pabloosabaterr@gmail.com>\n---\n t/t9200-git-cvsexportcommit.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh\nindex a44eabf0d8..4507e8e6db 100755\n--- a/t/t9200-git-cvsexportcommit.sh\n+++ b/t/t9200-git-cvsexportcommit.sh\n@@ -31,7 +31,7 @@ export CVSROOT CVSWORK GIT_DIR\n rm -rf \"$CVSROOT\" \"$CVSWORK\"\n \n cvs init &&\n-test -d \"$CVSROOT\" &&\n+test_path_is_dir \"$CVSROOT\" &&\n cvs -Q co -d \"$CVSWORK\" . &&\n echo >empty &&\n git add empty &&\n@@ -303,7 +303,7 @@ test_expect_success 're-commit a removed filename which remains in CVS attic' '\n \tgit commit -m \"Added attic_gremlin\" &&\n \tgit cvsexportcommit -w \"$CVSWORK\" -c HEAD &&\n \t(cd \"$CVSWORK\" && cvs -Q update -d) &&\n-\ttest -f \"$CVSWORK/attic_gremlin\"\n+\ttest_path_is_file \"$CVSWORK/attic_gremlin\"\n '\n \n # the state of the CVS sandbox may be indeterminate for ' space'\n-- \n2.43.0\n\n"},{"id":"538318","messageId":"xmqq8qc04sxh.fsf@gitster.g","threadId":"65177","inReplyTo":"20260309162832.605969-1-pabloosabaterr@gmail.com","subject":"Re: [GSoC PATCH v2] t9200: replace test -f/-d with modern path helpers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-09T21:03:06Z","receivedAt":"2026-03-09T21:03:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pablo Sabater <pabloosabaterr@gmail.com> writes:\n\n> Replace old style 'test -f' and 'test -d' with modern helpers\n> 'test_path_is_file' and 'test_path_is_dir' respectively.\n\nOK.  Being \"modern\" does not automatically mean \"better\", and it\nwould be helpful to say why we do this change for those relatively\nunexperienced who will read \"git log\" later and find this commit.\nPerhaps\n\n    Replace ... with ..., because it makes debugging a failing test\n    easier by loudly reporting what expectation was not met.\n\nor something.  The patch text looks good.\n\nThanks.\n\n> The instances were found with:\n>\n> \tgit grep \"test -[efd]\" t/\n>\n> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>\n> ---\n>  t/t9200-git-cvsexportcommit.sh | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n\nOK.  \n\n> diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh\n> index a44eabf0d8..4507e8e6db 100755\n> --- a/t/t9200-git-cvsexportcommit.sh\n> +++ b/t/t9200-git-cvsexportcommit.sh\n> @@ -31,7 +31,7 @@ export CVSROOT CVSWORK GIT_DIR\n>  rm -rf \"$CVSROOT\" \"$CVSWORK\"\n>  \n>  cvs init &&\n> -test -d \"$CVSROOT\" &&\n> +test_path_is_dir \"$CVSROOT\" &&\n>  cvs -Q co -d \"$CVSWORK\" . &&\n>  echo >empty &&\n>  git add empty &&\n> @@ -303,7 +303,7 @@ test_expect_success 're-commit a removed filename which remains in CVS attic' '\n>  \tgit commit -m \"Added attic_gremlin\" &&\n>  \tgit cvsexportcommit -w \"$CVSWORK\" -c HEAD &&\n>  \t(cd \"$CVSWORK\" && cvs -Q update -d) &&\n> -\ttest -f \"$CVSWORK/attic_gremlin\"\n> +\ttest_path_is_file \"$CVSWORK/attic_gremlin\"\n>  '\n>  \n>  # the state of the CVS sandbox may be indeterminate for ' space'\n"},{"id":"538330","messageId":"CAN5EUNQZsrtAUzQ5GgyFg5vJ-aMvAVLAQCYK6aahOSUoPa0dOQ@mail.gmail.com","threadId":"65177","inReplyTo":"xmqq8qc04sxh.fsf@gitster.g","subject":"Re: [GSoC PATCH v2] t9200: replace test -f/-d with modern path helpers","fromName":"Pablo","fromEmail":"pabloosabaterr@gmail.com","sentAt":"2026-03-09T22:54:48Z","receivedAt":"2026-03-09T22:55:03Z","isPatch":true,"sender":{"key":"pabloosabaterr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/63864637?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> OK.  Being \"modern\" does not automatically mean \"better\", and it\n> would be helpful to say why we do this change for those relatively\n> unexperienced who will read \"git log\" later and find this commit.\n> Perhaps\n\nOkay, thanks. I'll do that on the v3\n\nPablo\n"},{"id":"538331","messageId":"20260309230134.758107-1-pabloosabaterr@gmail.com","threadId":"65177","inReplyTo":"20260309150935.578465-1-pabloosabaterr@gmail.com","subject":"[GSoC PATCH v3] t9200: replace test -f/-d with modern path helpers","fromName":"Pablo Sabater","fromEmail":"pabloosabaterr@gmail.com","sentAt":"2026-03-09T23:01:34Z","receivedAt":"2026-03-09T23:01:42Z","isPatch":true,"sender":{"key":"pabloosabaterr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/63864637?v=4"},"body":"Replace old style 'test -f' and 'test -d' with helpers\n'test_path_is_file' and 'test_path_is_dir' respectively,\nwhich make debugging a failing test easier by loudly\nreporting what expectation was not met.\n\nThe instances were found with:\n\n\tgit grep \"test -[efd]\" t/\n\nSigned-off-by: Pablo Sabater <pabloosabaterr@gmail.com>\n---\n t/t9200-git-cvsexportcommit.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh\nindex a44eabf0d8..4507e8e6db 100755\n--- a/t/t9200-git-cvsexportcommit.sh\n+++ b/t/t9200-git-cvsexportcommit.sh\n@@ -31,7 +31,7 @@ export CVSROOT CVSWORK GIT_DIR\n rm -rf \"$CVSROOT\" \"$CVSWORK\"\n \n cvs init &&\n-test -d \"$CVSROOT\" &&\n+test_path_is_dir \"$CVSROOT\" &&\n cvs -Q co -d \"$CVSWORK\" . &&\n echo >empty &&\n git add empty &&\n@@ -303,7 +303,7 @@ test_expect_success 're-commit a removed filename which remains in CVS attic' '\n \tgit commit -m \"Added attic_gremlin\" &&\n \tgit cvsexportcommit -w \"$CVSWORK\" -c HEAD &&\n \t(cd \"$CVSWORK\" && cvs -Q update -d) &&\n-\ttest -f \"$CVSWORK/attic_gremlin\"\n+\ttest_path_is_file \"$CVSWORK/attic_gremlin\"\n '\n \n # the state of the CVS sandbox may be indeterminate for ' space'\n-- \n2.43.0\n\n"},{"id":"538334","messageId":"xmqqldg01t5o.fsf@gitster.g","threadId":"65177","inReplyTo":"20260309230134.758107-1-pabloosabaterr@gmail.com","subject":"Re: [GSoC PATCH v3] t9200: replace test -f/-d with modern path helpers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-09T23:26:27Z","receivedAt":"2026-03-09T23:26:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pablo Sabater <pabloosabaterr@gmail.com> writes:\n\n> Replace old style 'test -f' and 'test -d' with helpers\n> 'test_path_is_file' and 'test_path_is_dir' respectively,\n> which make debugging a failing test easier by loudly\n> reporting what expectation was not met.\n>\n> The instances were found with:\n>\n> \tgit grep \"test -[efd]\" t/\n>\n> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>\n> ---\n>  t/t9200-git-cvsexportcommit.sh | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n\nLooking good.  Will queue.  Thanks.\n\n\n> diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh\n> index a44eabf0d8..4507e8e6db 100755\n> --- a/t/t9200-git-cvsexportcommit.sh\n> +++ b/t/t9200-git-cvsexportcommit.sh\n> @@ -31,7 +31,7 @@ export CVSROOT CVSWORK GIT_DIR\n>  rm -rf \"$CVSROOT\" \"$CVSWORK\"\n>  \n>  cvs init &&\n> -test -d \"$CVSROOT\" &&\n> +test_path_is_dir \"$CVSROOT\" &&\n>  cvs -Q co -d \"$CVSWORK\" . &&\n>  echo >empty &&\n>  git add empty &&\n> @@ -303,7 +303,7 @@ test_expect_success 're-commit a removed filename which remains in CVS attic' '\n>  \tgit commit -m \"Added attic_gremlin\" &&\n>  \tgit cvsexportcommit -w \"$CVSWORK\" -c HEAD &&\n>  \t(cd \"$CVSWORK\" && cvs -Q update -d) &&\n> -\ttest -f \"$CVSWORK/attic_gremlin\"\n> +\ttest_path_is_file \"$CVSWORK/attic_gremlin\"\n>  '\n>  \n>  # the state of the CVS sandbox may be indeterminate for ' space'\n"},{"id":"538581","messageId":"CAN5EUNTNu5NYgeJ0OQSS25Ld_kJqtEEPGcXrg_EcyRdCkr5ORw@mail.gmail.com","threadId":"65177","inReplyTo":"20260309230134.758107-1-pabloosabaterr@gmail.com","subject":"Re: [GSoC PATCH v3] t9200: replace test -f/-d with modern path helpers","fromName":"Pablo","fromEmail":"pabloosabaterr@gmail.com","sentAt":"2026-03-11T10:59:31Z","receivedAt":"2026-03-11T10:59:48Z","isPatch":true,"sender":{"key":"pabloosabaterr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/63864637?v=4"},"body":"Adding project mentors to CC.\n\nThanks.\n\nEl mar, 10 mar 2026 a las 0:01, Pablo Sabater\n(<pabloosabaterr@gmail.com>) escribió:\n>\n> Replace old style 'test -f' and 'test -d' with helpers\n> 'test_path_is_file' and 'test_path_is_dir' respectively,\n> which make debugging a failing test easier by loudly\n> reporting what expectation was not met.\n>\n> The instances were found with:\n>\n>         git grep \"test -[efd]\" t/\n>\n> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>\n> ---\n>  t/t9200-git-cvsexportcommit.sh | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh\n> index a44eabf0d8..4507e8e6db 100755\n> --- a/t/t9200-git-cvsexportcommit.sh\n> +++ b/t/t9200-git-cvsexportcommit.sh\n> @@ -31,7 +31,7 @@ export CVSROOT CVSWORK GIT_DIR\n>  rm -rf \"$CVSROOT\" \"$CVSWORK\"\n>\n>  cvs init &&\n> -test -d \"$CVSROOT\" &&\n> +test_path_is_dir \"$CVSROOT\" &&\n>  cvs -Q co -d \"$CVSWORK\" . &&\n>  echo >empty &&\n>  git add empty &&\n> @@ -303,7 +303,7 @@ test_expect_success 're-commit a removed filename which remains in CVS attic' '\n>         git commit -m \"Added attic_gremlin\" &&\n>         git cvsexportcommit -w \"$CVSWORK\" -c HEAD &&\n>         (cd \"$CVSWORK\" && cvs -Q update -d) &&\n> -       test -f \"$CVSWORK/attic_gremlin\"\n> +       test_path_is_file \"$CVSWORK/attic_gremlin\"\n>  '\n>\n>  # the state of the CVS sandbox may be indeterminate for ' space'\n> --\n> 2.43.0\n>\n"},{"id":"538644","messageId":"xmqqwlzip82b.fsf@gitster.g","threadId":"65177","inReplyTo":"20260309230134.758107-1-pabloosabaterr@gmail.com","subject":"Re: [GSoC PATCH v3] t9200: replace test -f/-d with modern path helpers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-11T17:52:44Z","receivedAt":"2026-03-11T17:52:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pablo Sabater <pabloosabaterr@gmail.com> writes:\n\n> Replace old style 'test -f' and 'test -d' with helpers\n> 'test_path_is_file' and 'test_path_is_dir' respectively,\n> which make debugging a failing test easier by loudly\n> reporting what expectation was not met.\n\nWell explained.\n\n> The instances were found with:\n>\n> \tgit grep \"test -[efd]\" t/\n\nPeople seem to add the above to their test-path helper patches, but\nunless the coverage of the work is fairly thorough and you want to\nsay \"all the similar issues should be found with this command and I\naddressed all of them\", I do not see much point saying how you found\none of them and addressed it.\n\nYou could have used \"git grep -e <pattern> -- t/\\*.sh\", or you could\nhave been working to fix something in t9200 and noticed these while\nyou were doing something else to the file.\n\nI do not see it as too huge a deal and it is probably not a cause to\nsend in another iteration once it is already written, though.\n\n> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>\n> ---\n>  t/t9200-git-cvsexportcommit.sh | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh\n> index a44eabf0d8..4507e8e6db 100755\n> --- a/t/t9200-git-cvsexportcommit.sh\n> +++ b/t/t9200-git-cvsexportcommit.sh\n> @@ -31,7 +31,7 @@ export CVSROOT CVSWORK GIT_DIR\n>  rm -rf \"$CVSROOT\" \"$CVSWORK\"\n>  \n>  cvs init &&\n> -test -d \"$CVSROOT\" &&\n> +test_path_is_dir \"$CVSROOT\" &&\n>  cvs -Q co -d \"$CVSWORK\" . &&\n>  echo >empty &&\n>  git add empty &&\n\nOur test-path helpers should work even outside test_expect_*\nfunctions, so this is not wrong per-se, but it somehow looks a bit\nunusual.  A related clean-up would be to wrap the CVS initialization\npart inside another \"do we even have a working CVS installation to\nmake it worth our time testing 'git cvsexportcommit' command?\"\ncheck, i.e.,\n\n\tif ! cvs init || ! test -d \"$CVSROOT\" || ! cvs -Q co -d \"$CVSWORK\" .\n        then\n\t\tskip_all=\"cvs repository set-up fails\"\n\t\ttest_done\n\tfi\n\nand then move the git initialization part to its own test, e.g.,\n\n\ttest_expect_success 'git setup' '\n\t\techo >empty &&\n\t\tgit add empty &&\n\t\tgit commit -q -a -m Initial\n\t'\n\n> @@ -303,7 +303,7 @@ test_expect_success 're-commit a removed filename which remains in CVS attic' '\n>  \tgit commit -m \"Added attic_gremlin\" &&\n>  \tgit cvsexportcommit -w \"$CVSWORK\" -c HEAD &&\n>  \t(cd \"$CVSWORK\" && cvs -Q update -d) &&\n> -\ttest -f \"$CVSWORK/attic_gremlin\"\n> +\ttest_path_is_file \"$CVSWORK/attic_gremlin\"\n>  '\n\nOK.\n\n>  \n>  # the state of the CVS sandbox may be indeterminate for ' space'\n"},{"id":"538659","messageId":"CAN5EUNRZQP6ATE87AeZiJx-OTnNn_4NxhW4zyH6AspGUfnV7TA@mail.gmail.com","threadId":"65177","inReplyTo":"xmqqwlzip82b.fsf@gitster.g","subject":"Re: [GSoC PATCH v3] t9200: replace test -f/-d with modern path helpers","fromName":"Pablo","fromEmail":"pabloosabaterr@gmail.com","sentAt":"2026-03-11T19:06:26Z","receivedAt":"2026-03-11T19:06:38Z","isPatch":true,"sender":{"key":"pabloosabaterr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/63864637?v=4"},"body":"> Our test-path helpers should work even outside test_expect_*\n> functions, so this is not wrong per-se, but it somehow looks a bit\n> unusual.  A related clean-up would be to wrap the CVS initialization\n> part inside another \"do we even have a working CVS installation to\n> make it worth our time testing 'git cvsexportcommit' command?\"\n\n\nThanks for the feedback, I can send a separate patch to wrap the CVS\nin a skip_all git move the git setup\n"},{"id":"538670","messageId":"xmqqbjgunofq.fsf@gitster.g","threadId":"65177","inReplyTo":"CAN5EUNRZQP6ATE87AeZiJx-OTnNn_4NxhW4zyH6AspGUfnV7TA@mail.gmail.com","subject":"Re: [GSoC PATCH v3] t9200: replace test -f/-d with modern path helpers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-11T19:42:01Z","receivedAt":"2026-03-11T19:42:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pablo <pabloosabaterr@gmail.com> writes:\n\n>> Our test-path helpers should work even outside test_expect_*\n>> functions, so this is not wrong per-se, but it somehow looks a bit\n>> unusual.  A related clean-up would be to wrap the CVS initialization\n>> part inside another \"do we even have a working CVS installation to\n>> make it worth our time testing 'git cvsexportcommit' command?\"\n>\n>\n> Thanks for the feedback, I can send a separate patch to wrap the CVS\n> in a skip_all git move the git setup\n\nYeah, but if we are going to do so eventually, it would be pointless\nto use the path helper in that \"set up CVS environment and make sure\nwe got a sensible directory structure\" check, no?  Upon failure, we \nwill hit test_done that loudly says that their CVS installation is\nnot working as we expect.\n"},{"id":"538675","messageId":"CAN5EUNSmZmdnDzpAKAh8fZRex3--tnKaWZZSQ+o5WATc6sLy_Q@mail.gmail.com","threadId":"65177","inReplyTo":"xmqqbjgunofq.fsf@gitster.g","subject":"Re: [GSoC PATCH v3] t9200: replace test -f/-d with modern path helpers","fromName":"Pablo","fromEmail":"pabloosabaterr@gmail.com","sentAt":"2026-03-11T19:49:54Z","receivedAt":"2026-03-11T19:50:07Z","isPatch":true,"sender":{"key":"pabloosabaterr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/63864637?v=4"},"body":"> Yeah, but if we are going to do so eventually, it would be pointless\n> to use the path helper in that \"set up CVS environment and make sure\n> we got a sensible directory structure\" check, no?  Upon failure, we\n> will hit test_done that loudly says that their CVS installation is\n> not working as we expect.\n\nYeah, the new patch will change it back to test -d because it ends up\nin a if condition instead of an assertion.\nWould you prefer to drop that hunk from my v3 or should I send a v4 ?\n"},{"id":"538677","messageId":"xmqqtsumm7kf.fsf@gitster.g","threadId":"65177","inReplyTo":"CAN5EUNSmZmdnDzpAKAh8fZRex3--tnKaWZZSQ+o5WATc6sLy_Q@mail.gmail.com","subject":"Re: [GSoC PATCH v3] t9200: replace test -f/-d with modern path helpers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-11T20:31:44Z","receivedAt":"2026-03-11T20:31:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pablo <pabloosabaterr@gmail.com> writes:\n\n>> Yeah, but if we are going to do so eventually, it would be pointless\n>> to use the path helper in that \"set up CVS environment and make sure\n>> we got a sensible directory structure\" check, no?  Upon failure, we\n>> will hit test_done that loudly says that their CVS installation is\n>> not working as we expect.\n>\n> Yeah, the new patch will change it back to test -d because it ends up\n> in a if condition instead of an assertion.\n> Would you prefer to drop that hunk from my v3 or should I send a v4 ?\n\nYup, let me mark the \"cvs setup failure\" one ready for 'next'.  The\nother hunk that updates \"test -[efd]\" can become a separate patch.\n\nThanks.\n"},{"id":"538779","messageId":"20260312173305.15112-1-pabloosabaterr@gmail.com","threadId":"65177","inReplyTo":"20260309150935.578465-1-pabloosabaterr@gmail.com","subject":"[GSoC PATCH v4] t9200: replace test -f with modern path helper","fromName":"Pablo Sabater","fromEmail":"pabloosabaterr@gmail.com","sentAt":"2026-03-12T17:33:05Z","receivedAt":"2026-03-12T17:33:14Z","isPatch":true,"sender":{"key":"pabloosabaterr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/63864637?v=4"},"body":"Replace old style 'test -f' with helper\n'test_path_is_file', which make debugging\na failing test easier by loudly reporting\nwhat expectation was not met.\n\nSigned-off-by: Pablo Sabater <pabloosabaterr@gmail.com>\n---\nChanges from v3:\nThe first hunk was dropped from this patch, and sent as a separate patch.\nhttps://lore.kernel.org/git/20260311194002.190195-1-pabloosabaterr@gmail.com/\n\n t/t9200-git-cvsexportcommit.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh\nindex a44eabf0d8..15a91931a2 100755\n--- a/t/t9200-git-cvsexportcommit.sh\n+++ b/t/t9200-git-cvsexportcommit.sh\n@@ -303,7 +303,7 @@ test_expect_success 're-commit a removed filename which remains in CVS attic' '\n \tgit commit -m \"Added attic_gremlin\" &&\n \tgit cvsexportcommit -w \"$CVSWORK\" -c HEAD &&\n \t(cd \"$CVSWORK\" && cvs -Q update -d) &&\n-\ttest -f \"$CVSWORK/attic_gremlin\"\n+\ttest_path_is_file \"$CVSWORK/attic_gremlin\"\n '\n \n # the state of the CVS sandbox may be indeterminate for ' space'\n-- \n2.43.0\n\n"},{"id":"538781","messageId":"xmqqbjgteyot.fsf@gitster.g","threadId":"65177","inReplyTo":"20260312173305.15112-1-pabloosabaterr@gmail.com","subject":"Re: [GSoC PATCH v4] t9200: replace test -f with modern path helper","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-12T17:37:38Z","receivedAt":"2026-03-12T17:37:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pablo Sabater <pabloosabaterr@gmail.com> writes:\n\n> Replace old style 'test -f' with helper\n> 'test_path_is_file', which make debugging\n> a failing test easier by loudly reporting\n> what expectation was not met.\n>\n> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>\n> ---\n\nWill queue.  Looks good.  Thanks.\n\n> Changes from v3:\n> The first hunk was dropped from this patch, and sent as a separate patch.\n> https://lore.kernel.org/git/20260311194002.190195-1-pabloosabaterr@gmail.com/\n>\n>  t/t9200-git-cvsexportcommit.sh | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh\n> index a44eabf0d8..15a91931a2 100755\n> --- a/t/t9200-git-cvsexportcommit.sh\n> +++ b/t/t9200-git-cvsexportcommit.sh\n> @@ -303,7 +303,7 @@ test_expect_success 're-commit a removed filename which remains in CVS attic' '\n>  \tgit commit -m \"Added attic_gremlin\" &&\n>  \tgit cvsexportcommit -w \"$CVSWORK\" -c HEAD &&\n>  \t(cd \"$CVSWORK\" && cvs -Q update -d) &&\n> -\ttest -f \"$CVSWORK/attic_gremlin\"\n> +\ttest_path_is_file \"$CVSWORK/attic_gremlin\"\n>  '\n>  \n>  # the state of the CVS sandbox may be indeterminate for ' space'\n"}]}