{"thread":{"id":"64709","subject":"[PATCH] t1300: use test helpers instead of shell primitives","startedAt":"2026-01-02T06:21:07Z","lastAt":"2026-01-05T14:13:43Z","messageCount":11,"participants":["pushkarkumarsingh1970@gmail.com","Karthik Nayak","Pushkar Singh","Junio C Hamano","Abraham Samuel Adekunle","Samuel Abraham"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"532900","messageId":"20260102062029.18210-1-pushkarkumarsingh1970@gmail.com","threadId":"64709","inReplyTo":null,"subject":"[PATCH] t1300: use test helpers instead of shell primitives","fromName":"","fromEmail":"pushkarkumarsingh1970@gmail.com","sentAt":"2026-01-02T06:20:23Z","receivedAt":"2026-01-02T06:21:07Z","isPatch":true,"sender":{"key":"pushkarkumarsingh1970@gmail.com","avatar":"https://avatars.githubusercontent.com/u/173247767?v=4"},"body":"From: Pushkar Singh \n\nReplace plain \"test -f\" checks with \"test_path_is_file\" and symbolic\nlink checks with \"test_path_is_symlink\". The test framework helpers\nprovide clearer diagnostics and better consistency across the test\nsuite.\n\nSigned-off-by: Pushkar Singh \n---\n t/t1300-config.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t1300-config.sh b/t/t1300-config.sh\nindex 358d636379..9850fcd5b5 100755\n--- a/t/t1300-config.sh\n+++ b/t/t1300-config.sh\n@@ -1232,12 +1232,12 @@ test_expect_success SYMLINKS 'symlinked configuration' '\n \ttest_when_finished \"rm myconfig\" &&\n \tln -s notyet myconfig &&\n \tgit config --file=myconfig test.frotz nitfol &&\n-\ttest -h myconfig &&\n-\ttest -f notyet &&\n+\ttest_path_is_symlink myconfig &&\n+\ttest_path_is_file notyet &&\n \ttest \"z$(git config --file=notyet test.frotz)\" = znitfol &&\n \tgit config --file=myconfig test.xyzzy rezrov &&\n-\ttest -h myconfig &&\n-\ttest -f notyet &&\n+\ttest_path_is_symlink myconfig &&\n+\ttest_path_is_file notyet &&\n \tcat >expect <<-\\EOF &&\n \tnitfol\n \trezrov\n-- \n2.43.0\n\n"},{"id":"532909","messageId":"CAOLa=ZQrKO-Aeoa_9E9Gi3rzM2AuVETDT4sUEuc0V8XQUyk4Pw@mail.gmail.com","threadId":"64709","inReplyTo":"20260102062029.18210-1-pushkarkumarsingh1970@gmail.com","subject":"Re: [PATCH] t1300: use test helpers instead of shell primitives","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-02T09:09:35Z","receivedAt":"2026-01-02T09:09:37Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"pushkarkumarsingh1970@gmail.com writes:\n\n> From: Pushkar Singh \n>\n> Replace plain \"test -f\" checks with \"test_path_is_file\" and symbolic\n\nSo 'test -f' checks for regular files\n\n> link checks with \"test_path_is_symlink\". The test framework helpers\n\nand 'test -h' check for symlinks. Would be nice to also mention the\nlatter.\n\n> provide clearer diagnostics and better consistency across the test\n> suite.\n\n> Signed-off-by: Pushkar Singh \n> ---\n> t/t1300-config.sh | 8 ++++----\n> 1 file changed, 4 insertions(+), 4 deletions(-)\n>\n> diff --git a/t/t1300-config.sh b/t/t1300-config.sh\n> index 358d636379..9850fcd5b5 100755\n> --- a/t/t1300-config.sh\n> +++ b/t/t1300-config.sh\n> @@ -1232,12 +1232,12 @@ test_expect_success SYMLINKS 'symlinked configuration' '\n> \ttest_when_finished \"rm myconfig\" &&\n\nTangent: Not your patch's responsibility, but we should also remove\n'notyet' :)\n\n> \tln -s notyet myconfig &&\n> \tgit config --file=myconfig test.frotz nitfol &&\n> -\ttest -h myconfig &&\n> -\ttest -f notyet &&\n> +\ttest_path_is_symlink myconfig &&\n> +\ttest_path_is_file notyet &&\n> \ttest \"z$(git config --file=notyet test.frotz)\" = znitfol &&\n> \tgit config --file=myconfig test.xyzzy rezrov &&\n> -\ttest -h myconfig &&\n> -\ttest -f notyet &&\n> +\ttest_path_is_symlink myconfig &&\n> +\ttest_path_is_file notyet &&\n> \tcat >expect <<-\\EOF &&\n> \tnitfol\n> \trezrov\n> --\n> 2.43.0\n\nThe patch looks good. We have two files, one being a regular file and\nanother being a symlink to that regular file and we simple need to\nensure that they exist.\n"},{"id":"532910","messageId":"CALE2CrRkKneSJL_F-mF_aTBzbGwW8c5A+qSU+8etQdbB=o2wJg@mail.gmail.com","threadId":"64709","inReplyTo":"CAOLa=ZQrKO-Aeoa_9E9Gi3rzM2AuVETDT4sUEuc0V8XQUyk4Pw@mail.gmail.com","subject":"Re: [PATCH] t1300: use test helpers instead of shell primitives","fromName":"Pushkar Singh","fromEmail":"pushkarkumarsingh1970@gmail.com","sentAt":"2026-01-02T09:39:37Z","receivedAt":"2026-01-02T09:39:50Z","isPatch":true,"sender":{"key":"pushkarkumarsingh1970@gmail.com","avatar":"https://avatars.githubusercontent.com/u/173247767?v=4"},"body":"Hi Karthik,\n\nThank you for the review!\n\nYou’re right, I should have clarified that `test -f` checks for a\nregular file and `test -h` checks for a symbolic link. I’ll update\nthe commit message accordingly and send a v2.\n\nThanks again!\nPushkar\n\nOn Fri, Jan 2, 2026 at 2:39 PM Karthik Nayak wrote:\n>\n> pushkarkumarsingh1970@gmail.com writes:\n>\n> > From: Pushkar Singh \n> >\n> > Replace plain \"test -f\" checks with \"test_path_is_file\" and symbolic\n>\n> So 'test -f' checks for regular files\n>\n> > link checks with \"test_path_is_symlink\". The test framework helpers\n>\n> and 'test -h' check for symlinks. Would be nice to also mention the\n> latter.\n>\n> > provide clearer diagnostics and better consistency across the test\n> > suite.\n>\n> > Signed-off-by: Pushkar Singh \n> > ---\n> > t/t1300-config.sh | 8 ++++----\n> > 1 file changed, 4 insertions(+), 4 deletions(-)\n> >\n> > diff --git a/t/t1300-config.sh b/t/t1300-config.sh\n> > index 358d636379..9850fcd5b5 100755\n> > --- a/t/t1300-config.sh\n> > +++ b/t/t1300-config.sh\n> > @@ -1232,12 +1232,12 @@ test_expect_success SYMLINKS 'symlinked configuration' '\n> > test_when_finished \"rm myconfig\" &&\n>\n> Tangent: Not your patch's responsibility, but we should also remove\n> 'notyet' :)\n>\n> > ln -s notyet myconfig &&\n> > git config --file=myconfig test.frotz nitfol &&\n> > - test -h myconfig &&\n> > - test -f notyet &&\n> > + test_path_is_symlink myconfig &&\n> > + test_path_is_file notyet &&\n> > test \"z$(git config --file=notyet test.frotz)\" = znitfol &&\n> > git config --file=myconfig test.xyzzy rezrov &&\n> > - test -h myconfig &&\n> > - test -f notyet &&\n> > + test_path_is_symlink myconfig &&\n> > + test_path_is_file notyet &&\n> > cat >expect <<-\\EOF &&\n> > nitfol\n> > rezrov\n> > --\n> > 2.43.0\n>\n> The patch looks good. We have two files, one being a regular file and\n> another being a symlink to that regular file and we simple need to\n> ensure that they exist.\n"},{"id":"532965","messageId":"xmqqfr8mp0cx.fsf@gitster.g","threadId":"64709","inReplyTo":"20260102062029.18210-1-pushkarkumarsingh1970@gmail.com","subject":"Re: [PATCH] t1300: use test helpers instead of shell primitives","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-04T02:39:42Z","receivedAt":"2026-01-04T02:39:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"pushkarkumarsingh1970@gmail.com writes:\n\n> From: Pushkar Singh \n>\n> Replace plain \"test -f\" checks with \"test_path_is_file\" and symbolic\n> link checks with \"test_path_is_symlink\". The test framework helpers\n> provide clearer diagnostics and better consistency across the test\n> suite.\n\nThe \"test\" is often implemented as a built-in utility in a shell,\nbut not necessarily so. Either way, it is not correct to call it\n\"shell primitive\", as unlike \"if\", \"for\", it is not.\n"},{"id":"532991","messageId":"20260104124255.15609-1-pushkarkumarsingh1970@gmail.com","threadId":"64709","inReplyTo":"20260102062029.18210-1-pushkarkumarsingh1970@gmail.com","subject":"[PATCH v3] t1300: use test helpers instead of test builtins","fromName":"Pushkar Singh","fromEmail":"pushkarkumarsingh1970@gmail.com","sentAt":"2026-01-04T12:41:53Z","receivedAt":"2026-01-04T12:43:01Z","isPatch":true,"sender":{"key":"pushkarkumarsingh1970@gmail.com","avatar":"https://avatars.githubusercontent.com/u/173247767?v=4"},"body":"This version updates the commit message to avoid calling `test` a shell\nprimitive, as suggested.\n\nSigned-off-by: Pushkar Singh \n---\n t/t1300-config.sh | 8 ++++----\n t/t2021-checkout-overwrite.sh | 4 ++--\n 2 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t1300-config.sh b/t/t1300-config.sh\nindex 358d636379..9850fcd5b5 100755\n--- a/t/t1300-config.sh\n+++ b/t/t1300-config.sh\n@@ -1232,12 +1232,12 @@ test_expect_success SYMLINKS 'symlinked configuration' '\n \ttest_when_finished \"rm myconfig\" &&\n \tln -s notyet myconfig &&\n \tgit config --file=myconfig test.frotz nitfol &&\n-\ttest -h myconfig &&\n-\ttest -f notyet &&\n+\ttest_path_is_symlink myconfig &&\n+\ttest_path_is_file notyet &&\n \ttest \"z$(git config --file=notyet test.frotz)\" = znitfol &&\n \tgit config --file=myconfig test.xyzzy rezrov &&\n-\ttest -h myconfig &&\n-\ttest -f notyet &&\n+\ttest_path_is_symlink myconfig &&\n+\ttest_path_is_file notyet &&\n \tcat >expect <<-\\EOF &&\n \tnitfol\n \trezrov\ndiff --git a/t/t2021-checkout-overwrite.sh b/t/t2021-checkout-overwrite.sh\nindex a5c03d5d4a..38c41ae373 100755\n--- a/t/t2021-checkout-overwrite.sh\n+++ b/t/t2021-checkout-overwrite.sh\n@@ -27,7 +27,7 @@ test_expect_success 'checkout commit with dir must not remove untracked a/b' '\n \tgit rm --cached a/b &&\n \tgit commit -m \"un-track the file\" &&\n \ttest_must_fail git checkout start &&\n-\ttest -f a/b\n+\ttest_path_is_file a/b\n '\n \n test_expect_success 'create a commit where dir a/b changed to symlink' '\n@@ -49,7 +49,7 @@ test_expect_success 'checkout commit with dir must not remove untracked a/b' '\n \n test_expect_success SYMLINKS 'the symlink remained' '\n \n-\ttest -h a/b\n+\ttest_path_is_symlink a/b\n '\n \n test_expect_success 'cleanup after previous symlink tests' '\n-- \n2.43.0\n\n"},{"id":"532993","messageId":"aVqIjHIi6aKvFYXg@Adekunles-MacBook-Air.local","threadId":"64709","inReplyTo":"20260104124255.15609-1-pushkarkumarsingh1970@gmail.com","subject":"[PATCH v3] t1300: use test helpers instead of test builtins","fromName":"Abraham Samuel Adekunle","fromEmail":"abrahamadekunle50@gmail.com","sentAt":"2026-01-04T15:34:36Z","receivedAt":"2026-01-04T15:34:27Z","isPatch":true,"sender":{"key":"abrahamadekunle50@gmail.com","avatar":"https://avatars.githubusercontent.com/u/110066922?v=4"},"body":">This version updates the commit message to avoid calling `test` a shell\n>primitive, as suggested.\n\n>Signed-off-by: Pushkar Singh \n>---\n\nHello Pushkar,\n\nI think the right approach to send an updated version after modifying your commit\nmessage is to modify your commit message to INCLUDE the recommendation, not change\nthe commit message to the recommendation alone.\nThen under these three dashes after the 'Signed-off-by:', (---), which is here,\nwhere I am currently replying to you, you state what you changed in the new version\ncompared to the previous version.\n\ne.g\n\nChanges in v3:\n- Modified commit message to ...\n- Modified subject to use builtin instead of primitive\n\n\nThanks\nAbraham.\n"},{"id":"532998","messageId":"CALE2CrQoXZjkpd+Lswd=qME9nkmUGu3HK8XacZ+Xz6Y2w+=t3A@mail.gmail.com","threadId":"64709","inReplyTo":"aVqIjHIi6aKvFYXg@Adekunles-MacBook-Air.local","subject":"Re: [PATCH v3] t1300: use test helpers instead of test builtins","fromName":"Pushkar Singh","fromEmail":"pushkarkumarsingh1970@gmail.com","sentAt":"2026-01-04T19:40:05Z","receivedAt":"2026-01-04T19:40:18Z","isPatch":true,"sender":{"key":"pushkarkumarsingh1970@gmail.com","avatar":"https://avatars.githubusercontent.com/u/173247767?v=4"},"body":"Hi Abraham,\n\nThanks for pointing that out.\n\nUnderstood. I should keep the commit message itself focused on the change,\nand describe what was updated between versions under the `---` section.\n\nI will send a v4 with the commit message adjusted accordingly and include a\n\"Changes in v4\" note below the separator.\n\nThanks for the clarification.\nPushkar\n\nOn Sun, Jan 4, 2026 at 9:04 PM Abraham Samuel Adekunle\n wrote:\n>\n> >This version updates the commit message to avoid calling `test` a shell\n> >primitive, as suggested.\n>\n> >Signed-off-by: Pushkar Singh \n> >---\n>\n> Hello Pushkar,\n>\n> I think the right approach to send an updated version after modifying your commit\n> message is to modify your commit message to INCLUDE the recommendation, not change\n> the commit message to the recommendation alone.\n> Then under these three dashes after the 'Signed-off-by:', (---), which is here,\n> where I am currently replying to you, you state what you changed in the new version\n> compared to the previous version.\n>\n> e.g\n>\n> Changes in v3:\n> - Modified commit message to ...\n> - Modified subject to use builtin instead of primitive\n>\n>\n> Thanks\n> Abraham.\n"},{"id":"533000","messageId":"20260104194812.15134-1-pushkarkumarsingh1970@gmail.com","threadId":"64709","inReplyTo":"20260104124255.15609-1-pushkarkumarsingh1970@gmail.com","subject":"[PATCH v4] t1300: use test helpers instead of test builtins","fromName":"Pushkar Singh","fromEmail":"pushkarkumarsingh1970@gmail.com","sentAt":"2026-01-04T19:47:59Z","receivedAt":"2026-01-04T19:48:18Z","isPatch":true,"sender":{"key":"pushkarkumarsingh1970@gmail.com","avatar":"https://avatars.githubusercontent.com/u/173247767?v=4"},"body":"Replace test -f and test -h checks with test_path_is_file and\ntest_path_is_symlink. Using the test framework helpers provides clearer\ndiagnostics and keeps tests consistent across the suite.\n\nSigned-off-by: Pushkar Singh \n---\nChanges in v4:\n- Update commit message to avoid calling `test` a shell primitive\n- No code changes\n\n t/t1300-config.sh | 8 ++++----\n t/t2021-checkout-overwrite.sh | 4 ++--\n 2 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t1300-config.sh b/t/t1300-config.sh\nindex 358d636379..9850fcd5b5 100755\n--- a/t/t1300-config.sh\n+++ b/t/t1300-config.sh\n@@ -1232,12 +1232,12 @@ test_expect_success SYMLINKS 'symlinked configuration' '\n \ttest_when_finished \"rm myconfig\" &&\n \tln -s notyet myconfig &&\n \tgit config --file=myconfig test.frotz nitfol &&\n-\ttest -h myconfig &&\n-\ttest -f notyet &&\n+\ttest_path_is_symlink myconfig &&\n+\ttest_path_is_file notyet &&\n \ttest \"z$(git config --file=notyet test.frotz)\" = znitfol &&\n \tgit config --file=myconfig test.xyzzy rezrov &&\n-\ttest -h myconfig &&\n-\ttest -f notyet &&\n+\ttest_path_is_symlink myconfig &&\n+\ttest_path_is_file notyet &&\n \tcat >expect <<-\\EOF &&\n \tnitfol\n \trezrov\ndiff --git a/t/t2021-checkout-overwrite.sh b/t/t2021-checkout-overwrite.sh\nindex a5c03d5d4a..38c41ae373 100755\n--- a/t/t2021-checkout-overwrite.sh\n+++ b/t/t2021-checkout-overwrite.sh\n@@ -27,7 +27,7 @@ test_expect_success 'checkout commit with dir must not remove untracked a/b' '\n \tgit rm --cached a/b &&\n \tgit commit -m \"un-track the file\" &&\n \ttest_must_fail git checkout start &&\n-\ttest -f a/b\n+\ttest_path_is_file a/b\n '\n \n test_expect_success 'create a commit where dir a/b changed to symlink' '\n@@ -49,7 +49,7 @@ test_expect_success 'checkout commit with dir must not remove untracked a/b' '\n \n test_expect_success SYMLINKS 'the symlink remained' '\n \n-\ttest -h a/b\n+\ttest_path_is_symlink a/b\n '\n \n test_expect_success 'cleanup after previous symlink tests' '\n-- \n2.43.0\n\n"},{"id":"533023","messageId":"CAOLa=ZQOryB1nf86BUkmL8JB88S+C4=z120uoExO7t9wmMP_Ow@mail.gmail.com","threadId":"64709","inReplyTo":"CALE2CrQoXZjkpd+Lswd=qME9nkmUGu3HK8XacZ+Xz6Y2w+=t3A@mail.gmail.com","subject":"Re: [PATCH v3] t1300: use test helpers instead of test builtins","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-05T10:55:07Z","receivedAt":"2026-01-05T10:55:09Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Pushkar Singh writes:\n\n> Hi Abraham,\n>\n> Thanks for pointing that out.\n>\n> Understood. I should keep the commit message itself focused on the change,\n> and describe what was updated between versions under the `---` section.\n>\n> I will send a v4 with the commit message adjusted accordingly and include a\n> \"Changes in v4\" note below the separator.\n>\n> Thanks for the clarification.\n> Pushkar\n>\n\nI also find using b4 [1] to be very beneficial to handle this. Where b4\nprovides patch versioning and you can simply worry about your commits :)\n\n[1]: https://b4.docs.kernel.org/en/latest/\n\n> On Sun, Jan 4, 2026 at 9:04 PM Abraham Samuel Adekunle\n> wrote:\n>>\n>> >This version updates the commit message to avoid calling `test` a shell\n>> >primitive, as suggested.\n>>\n>> >Signed-off-by: Pushkar Singh \n>> >---\n>>\n>> Hello Pushkar,\n>>\n>> I think the right approach to send an updated version after modifying your commit\n>> message is to modify your commit message to INCLUDE the recommendation, not change\n>> the commit message to the recommendation alone.\n>> Then under these three dashes after the 'Signed-off-by:', (---), which is here,\n>> where I am currently replying to you, you state what you changed in the new version\n>> compared to the previous version.\n>>\n>> e.g\n>>\n>> Changes in v3:\n>> - Modified commit message to ...\n>> - Modified subject to use builtin instead of primitive\n>>\n>>\n>> Thanks\n>> Abraham.\n"},{"id":"533024","messageId":"CAOLa=ZR-LrLcL7F+dS_gBO-jVoiiGEXMKd4pbg6wS+EPbdZy6w@mail.gmail.com","threadId":"64709","inReplyTo":"20260104194812.15134-1-pushkarkumarsingh1970@gmail.com","subject":"Re: [PATCH v4] t1300: use test helpers instead of test builtins","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-05T10:55:47Z","receivedAt":"2026-01-05T10:55:49Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Pushkar Singh writes:\n\n> Replace test -f and test -h checks with test_path_is_file and\n> test_path_is_symlink. Using the test framework helpers provides clearer\n> diagnostics and keeps tests consistent across the suite.\n>\n> Signed-off-by: Pushkar Singh \n\nThis version looks good to me. Thanks.\n\n[snip]\n"},{"id":"533046","messageId":"CADYq+fYW0oY5yRLNeqveZqrT9-p_t=ndTZveEwGRpQ-JfYEH7g@mail.gmail.com","threadId":"64709","inReplyTo":"CAOLa=ZQOryB1nf86BUkmL8JB88S+C4=z120uoExO7t9wmMP_Ow@mail.gmail.com","subject":"Re: [PATCH v3] t1300: use test helpers instead of test builtins","fromName":"Samuel Abraham","fromEmail":"abrahamadekunle50@gmail.com","sentAt":"2026-01-05T14:13:43Z","receivedAt":"2026-01-05T14:13:43Z","isPatch":true,"sender":{"key":"abrahamadekunle50@gmail.com","avatar":"https://avatars.githubusercontent.com/u/110066922?v=4"},"body":"On Mon, Jan 5, 2026 at 11:55 AM Karthik Nayak wrote:\n>\n> Pushkar Singh writes:\n>\n> > Hi Abraham,\n> >\n> > Thanks for pointing that out.\n> >\n> > Understood. I should keep the commit message itself focused on the change,\n> > and describe what was updated between versions under the `---` section.\n> >\n> > I will send a v4 with the commit message adjusted accordingly and include a\n> > \"Changes in v4\" note below the separator.\n> >\n> > Thanks for the clarification.\n> > Pushkar\n> >\n>\n> I also find using b4 [1] to be very beneficial to handle this. Where b4\n> provides patch versioning and you can simply worry about your commits :)\n>\n> [1]: https://b4.docs.kernel.org/en/latest/\n>\n\nOh thank you very much Karthik.\nI will surely look into this.\n\nAbraham.\n"}]}