# [PATCH] t1300: use test helpers instead of shell primitives 11 messages from 2026-01-02 to 2026-01-05. Participants: pushkarkumarsingh1970@gmail.com, Karthik Nayak, Pushkar Singh, Junio C Hamano, Abraham Samuel Adekunle, Samuel Abraham. Thread: https://gitlist.dev/t/64709 ## pushkarkumarsingh1970@gmail.com, 2026-01-02 06:20 Subject: [PATCH] t1300: use test helpers instead of shell primitives Message-ID: <20260102062029.18210-1-pushkarkumarsingh1970@gmail.com> URL: https://gitlist.dev/e/20260102062029.18210-1-pushkarkumarsingh1970%40gmail.com ``` From: Pushkar Singh Replace plain "test -f" checks with "test_path_is_file" and symbolic link checks with "test_path_is_symlink". The test framework helpers provide clearer diagnostics and better consistency across the test suite. Signed-off-by: Pushkar Singh --- t/t1300-config.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/t/t1300-config.sh b/t/t1300-config.sh index 358d636379..9850fcd5b5 100755 --- a/t/t1300-config.sh +++ b/t/t1300-config.sh @@ -1232,12 +1232,12 @@ test_expect_success SYMLINKS 'symlinked configuration' ' test_when_finished "rm myconfig" && ln -s notyet myconfig && git config --file=myconfig test.frotz nitfol && - test -h myconfig && - test -f notyet && + test_path_is_symlink myconfig && + test_path_is_file notyet && test "z$(git config --file=notyet test.frotz)" = znitfol && git config --file=myconfig test.xyzzy rezrov && - test -h myconfig && - test -f notyet && + test_path_is_symlink myconfig && + test_path_is_file notyet && cat >expect <<-\EOF && nitfol rezrov -- 2.43.0 ``` ## Karthik Nayak, 2026-01-02 09:09 Subject: Re: [PATCH] t1300: use test helpers instead of shell primitives Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZQrKO-Aeoa_9E9Gi3rzM2AuVETDT4sUEuc0V8XQUyk4Pw%40mail.gmail.com In-Reply-To: <20260102062029.18210-1-pushkarkumarsingh1970@gmail.com> ``` pushkarkumarsingh1970@gmail.com writes: > From: Pushkar Singh > > Replace plain "test -f" checks with "test_path_is_file" and symbolic So 'test -f' checks for regular files > link checks with "test_path_is_symlink". The test framework helpers and 'test -h' check for symlinks. Would be nice to also mention the latter. > provide clearer diagnostics and better consistency across the test > suite. > Signed-off-by: Pushkar Singh > --- > t/t1300-config.sh | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/t/t1300-config.sh b/t/t1300-config.sh > index 358d636379..9850fcd5b5 100755 > --- a/t/t1300-config.sh > +++ b/t/t1300-config.sh > @@ -1232,12 +1232,12 @@ test_expect_success SYMLINKS 'symlinked configuration' ' > test_when_finished "rm myconfig" && Tangent: Not your patch's responsibility, but we should also remove 'notyet' :) > ln -s notyet myconfig && > git config --file=myconfig test.frotz nitfol && > - test -h myconfig && > - test -f notyet && > + test_path_is_symlink myconfig && > + test_path_is_file notyet && > test "z$(git config --file=notyet test.frotz)" = znitfol && > git config --file=myconfig test.xyzzy rezrov && > - test -h myconfig && > - test -f notyet && > + test_path_is_symlink myconfig && > + test_path_is_file notyet && > cat >expect <<-\EOF && > nitfol > rezrov > -- > 2.43.0 The patch looks good. We have two files, one being a regular file and another being a symlink to that regular file and we simple need to ensure that they exist. ``` ## Pushkar Singh, 2026-01-02 09:39 Subject: Re: [PATCH] t1300: use test helpers instead of shell primitives Message-ID: URL: https://gitlist.dev/e/CALE2CrRkKneSJL_F-mF_aTBzbGwW8c5A%2BqSU%2B8etQdbB%3Do2wJg%40mail.gmail.com In-Reply-To: ``` Hi Karthik, Thank you for the review! You’re right, I should have clarified that `test -f` checks for a regular file and `test -h` checks for a symbolic link. I’ll update the commit message accordingly and send a v2. Thanks again! Pushkar On Fri, Jan 2, 2026 at 2:39 PM Karthik Nayak wrote: > > pushkarkumarsingh1970@gmail.com writes: > > > From: Pushkar Singh > > > > Replace plain "test -f" checks with "test_path_is_file" and symbolic > > So 'test -f' checks for regular files > > > link checks with "test_path_is_symlink". The test framework helpers > > and 'test -h' check for symlinks. Would be nice to also mention the > latter. > > > provide clearer diagnostics and better consistency across the test > > suite. > > > Signed-off-by: Pushkar Singh > > --- > > t/t1300-config.sh | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/t/t1300-config.sh b/t/t1300-config.sh > > index 358d636379..9850fcd5b5 100755 > > --- a/t/t1300-config.sh > > +++ b/t/t1300-config.sh > > @@ -1232,12 +1232,12 @@ test_expect_success SYMLINKS 'symlinked configuration' ' > > test_when_finished "rm myconfig" && > > Tangent: Not your patch's responsibility, but we should also remove > 'notyet' :) > > > ln -s notyet myconfig && > > git config --file=myconfig test.frotz nitfol && > > - test -h myconfig && > > - test -f notyet && > > + test_path_is_symlink myconfig && > > + test_path_is_file notyet && > > test "z$(git config --file=notyet test.frotz)" = znitfol && > > git config --file=myconfig test.xyzzy rezrov && > > - test -h myconfig && > > - test -f notyet && > > + test_path_is_symlink myconfig && > > + test_path_is_file notyet && > > cat >expect <<-\EOF && > > nitfol > > rezrov > > -- > > 2.43.0 > > The patch looks good. We have two files, one being a regular file and > another being a symlink to that regular file and we simple need to > ensure that they exist. ``` ## Junio C Hamano, 2026-01-04 02:39 Subject: Re: [PATCH] t1300: use test helpers instead of shell primitives Message-ID: URL: https://gitlist.dev/e/xmqqfr8mp0cx.fsf%40gitster.g In-Reply-To: <20260102062029.18210-1-pushkarkumarsingh1970@gmail.com> ``` pushkarkumarsingh1970@gmail.com writes: > From: Pushkar Singh > > Replace plain "test -f" checks with "test_path_is_file" and symbolic > link checks with "test_path_is_symlink". The test framework helpers > provide clearer diagnostics and better consistency across the test > suite. The "test" is often implemented as a built-in utility in a shell, but not necessarily so. Either way, it is not correct to call it "shell primitive", as unlike "if", "for", it is not. ``` ## Pushkar Singh, 2026-01-04 12:41 Subject: [PATCH v3] t1300: use test helpers instead of test builtins Message-ID: <20260104124255.15609-1-pushkarkumarsingh1970@gmail.com> URL: https://gitlist.dev/e/20260104124255.15609-1-pushkarkumarsingh1970%40gmail.com In-Reply-To: <20260102062029.18210-1-pushkarkumarsingh1970@gmail.com> ``` This version updates the commit message to avoid calling `test` a shell primitive, as suggested. Signed-off-by: Pushkar Singh --- t/t1300-config.sh | 8 ++++---- t/t2021-checkout-overwrite.sh | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/t/t1300-config.sh b/t/t1300-config.sh index 358d636379..9850fcd5b5 100755 --- a/t/t1300-config.sh +++ b/t/t1300-config.sh @@ -1232,12 +1232,12 @@ test_expect_success SYMLINKS 'symlinked configuration' ' test_when_finished "rm myconfig" && ln -s notyet myconfig && git config --file=myconfig test.frotz nitfol && - test -h myconfig && - test -f notyet && + test_path_is_symlink myconfig && + test_path_is_file notyet && test "z$(git config --file=notyet test.frotz)" = znitfol && git config --file=myconfig test.xyzzy rezrov && - test -h myconfig && - test -f notyet && + test_path_is_symlink myconfig && + test_path_is_file notyet && cat >expect <<-\EOF && nitfol rezrov diff --git a/t/t2021-checkout-overwrite.sh b/t/t2021-checkout-overwrite.sh index a5c03d5d4a..38c41ae373 100755 --- a/t/t2021-checkout-overwrite.sh +++ b/t/t2021-checkout-overwrite.sh @@ -27,7 +27,7 @@ test_expect_success 'checkout commit with dir must not remove untracked a/b' ' git rm --cached a/b && git commit -m "un-track the file" && test_must_fail git checkout start && - test -f a/b + test_path_is_file a/b ' test_expect_success 'create a commit where dir a/b changed to symlink' ' @@ -49,7 +49,7 @@ test_expect_success 'checkout commit with dir must not remove untracked a/b' ' test_expect_success SYMLINKS 'the symlink remained' ' - test -h a/b + test_path_is_symlink a/b ' test_expect_success 'cleanup after previous symlink tests' ' -- 2.43.0 ``` ## Abraham Samuel Adekunle, 2026-01-04 15:34 Subject: [PATCH v3] t1300: use test helpers instead of test builtins Message-ID: URL: https://gitlist.dev/e/aVqIjHIi6aKvFYXg%40Adekunles-MacBook-Air.local In-Reply-To: <20260104124255.15609-1-pushkarkumarsingh1970@gmail.com> ``` >This version updates the commit message to avoid calling `test` a shell >primitive, as suggested. >Signed-off-by: Pushkar Singh >--- Hello Pushkar, I think the right approach to send an updated version after modifying your commit message is to modify your commit message to INCLUDE the recommendation, not change the commit message to the recommendation alone. Then under these three dashes after the 'Signed-off-by:', (---), which is here, where I am currently replying to you, you state what you changed in the new version compared to the previous version. e.g Changes in v3: - Modified commit message to ... - Modified subject to use builtin instead of primitive Thanks Abraham. ``` ## Pushkar Singh, 2026-01-04 19:40 Subject: Re: [PATCH v3] t1300: use test helpers instead of test builtins Message-ID: URL: https://gitlist.dev/e/CALE2CrQoXZjkpd%2BLswd%3DqME9nkmUGu3HK8XacZ%2BXz6Y2w%2B%3Dt3A%40mail.gmail.com In-Reply-To: ``` Hi Abraham, Thanks for pointing that out. Understood. I should keep the commit message itself focused on the change, and describe what was updated between versions under the `---` section. I will send a v4 with the commit message adjusted accordingly and include a "Changes in v4" note below the separator. Thanks for the clarification. Pushkar On Sun, Jan 4, 2026 at 9:04 PM Abraham Samuel Adekunle wrote: > > >This version updates the commit message to avoid calling `test` a shell > >primitive, as suggested. > > >Signed-off-by: Pushkar Singh > >--- > > Hello Pushkar, > > I think the right approach to send an updated version after modifying your commit > message is to modify your commit message to INCLUDE the recommendation, not change > the commit message to the recommendation alone. > Then under these three dashes after the 'Signed-off-by:', (---), which is here, > where I am currently replying to you, you state what you changed in the new version > compared to the previous version. > > e.g > > Changes in v3: > - Modified commit message to ... > - Modified subject to use builtin instead of primitive > > > Thanks > Abraham. ``` ## Pushkar Singh, 2026-01-04 19:47 Subject: [PATCH v4] t1300: use test helpers instead of test builtins Message-ID: <20260104194812.15134-1-pushkarkumarsingh1970@gmail.com> URL: https://gitlist.dev/e/20260104194812.15134-1-pushkarkumarsingh1970%40gmail.com In-Reply-To: <20260104124255.15609-1-pushkarkumarsingh1970@gmail.com> ``` Replace test -f and test -h checks with test_path_is_file and test_path_is_symlink. Using the test framework helpers provides clearer diagnostics and keeps tests consistent across the suite. Signed-off-by: Pushkar Singh --- Changes in v4: - Update commit message to avoid calling `test` a shell primitive - No code changes t/t1300-config.sh | 8 ++++---- t/t2021-checkout-overwrite.sh | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/t/t1300-config.sh b/t/t1300-config.sh index 358d636379..9850fcd5b5 100755 --- a/t/t1300-config.sh +++ b/t/t1300-config.sh @@ -1232,12 +1232,12 @@ test_expect_success SYMLINKS 'symlinked configuration' ' test_when_finished "rm myconfig" && ln -s notyet myconfig && git config --file=myconfig test.frotz nitfol && - test -h myconfig && - test -f notyet && + test_path_is_symlink myconfig && + test_path_is_file notyet && test "z$(git config --file=notyet test.frotz)" = znitfol && git config --file=myconfig test.xyzzy rezrov && - test -h myconfig && - test -f notyet && + test_path_is_symlink myconfig && + test_path_is_file notyet && cat >expect <<-\EOF && nitfol rezrov diff --git a/t/t2021-checkout-overwrite.sh b/t/t2021-checkout-overwrite.sh index a5c03d5d4a..38c41ae373 100755 --- a/t/t2021-checkout-overwrite.sh +++ b/t/t2021-checkout-overwrite.sh @@ -27,7 +27,7 @@ test_expect_success 'checkout commit with dir must not remove untracked a/b' ' git rm --cached a/b && git commit -m "un-track the file" && test_must_fail git checkout start && - test -f a/b + test_path_is_file a/b ' test_expect_success 'create a commit where dir a/b changed to symlink' ' @@ -49,7 +49,7 @@ test_expect_success 'checkout commit with dir must not remove untracked a/b' ' test_expect_success SYMLINKS 'the symlink remained' ' - test -h a/b + test_path_is_symlink a/b ' test_expect_success 'cleanup after previous symlink tests' ' -- 2.43.0 ``` ## Karthik Nayak, 2026-01-05 10:55 Subject: Re: [PATCH v3] t1300: use test helpers instead of test builtins Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZQOryB1nf86BUkmL8JB88S%2BC4%3Dz120uoExO7t9wmMP_Ow%40mail.gmail.com In-Reply-To: ``` Pushkar Singh writes: > Hi Abraham, > > Thanks for pointing that out. > > Understood. I should keep the commit message itself focused on the change, > and describe what was updated between versions under the `---` section. > > I will send a v4 with the commit message adjusted accordingly and include a > "Changes in v4" note below the separator. > > Thanks for the clarification. > Pushkar > I also find using b4 [1] to be very beneficial to handle this. Where b4 provides patch versioning and you can simply worry about your commits :) [1]: https://b4.docs.kernel.org/en/latest/ > On Sun, Jan 4, 2026 at 9:04 PM Abraham Samuel Adekunle > wrote: >> >> >This version updates the commit message to avoid calling `test` a shell >> >primitive, as suggested. >> >> >Signed-off-by: Pushkar Singh >> >--- >> >> Hello Pushkar, >> >> I think the right approach to send an updated version after modifying your commit >> message is to modify your commit message to INCLUDE the recommendation, not change >> the commit message to the recommendation alone. >> Then under these three dashes after the 'Signed-off-by:', (---), which is here, >> where I am currently replying to you, you state what you changed in the new version >> compared to the previous version. >> >> e.g >> >> Changes in v3: >> - Modified commit message to ... >> - Modified subject to use builtin instead of primitive >> >> >> Thanks >> Abraham. ``` ## Karthik Nayak, 2026-01-05 10:55 Subject: Re: [PATCH v4] t1300: use test helpers instead of test builtins Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZR-LrLcL7F%2BdS_gBO-jVoiiGEXMKd4pbg6wS%2BEPbdZy6w%40mail.gmail.com In-Reply-To: <20260104194812.15134-1-pushkarkumarsingh1970@gmail.com> ``` Pushkar Singh writes: > Replace test -f and test -h checks with test_path_is_file and > test_path_is_symlink. Using the test framework helpers provides clearer > diagnostics and keeps tests consistent across the suite. > > Signed-off-by: Pushkar Singh This version looks good to me. Thanks. [snip] ``` ## Samuel Abraham, 2026-01-05 14:13 Subject: Re: [PATCH v3] t1300: use test helpers instead of test builtins Message-ID: URL: https://gitlist.dev/e/CADYq%2BfYW0oY5yRLNeqveZqrT9-p_t%3DndTZveEwGRpQ-JfYEH7g%40mail.gmail.com In-Reply-To: ``` On Mon, Jan 5, 2026 at 11:55 AM Karthik Nayak wrote: > > Pushkar Singh writes: > > > Hi Abraham, > > > > Thanks for pointing that out. > > > > Understood. I should keep the commit message itself focused on the change, > > and describe what was updated between versions under the `---` section. > > > > I will send a v4 with the commit message adjusted accordingly and include a > > "Changes in v4" note below the separator. > > > > Thanks for the clarification. > > Pushkar > > > > I also find using b4 [1] to be very beneficial to handle this. Where b4 > provides patch versioning and you can simply worry about your commits :) > > [1]: https://b4.docs.kernel.org/en/latest/ > Oh thank you very much Karthik. I will surely look into this. Abraham. ```