[PATCH] t1300: use test helpers instead of shell primitives

[PATCH] t1300: use test helpers instead of shell primitives

[pushkarkumarsingh1970]

From: Pushkar Singh <pushkarkumarsingh1970@gmail.com>

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 <pushkarkumarsingh1970@gmail.com>
---
 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

Re: [PATCH] t1300: use test helpers instead of shell primitives

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 1595 bytes --]

pushkarkumarsingh1970@gmail.com writes:

> From: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
>
> 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 <pushkarkumarsingh1970@gmail.com>
> ---
>  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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH] t1300: use test helpers instead of shell primitives

[Pushkar Singh]

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 <karthik.188@gmail.com> wrote:
>
> pushkarkumarsingh1970@gmail.com writes:
>
> > From: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
> >
> > 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 <pushkarkumarsingh1970@gmail.com>
> > ---
> >  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.

Re: [PATCH] t1300: use test helpers instead of shell primitives

[Junio C Hamano]

pushkarkumarsingh1970@gmail.com writes:

> From: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
>
> 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.

[PATCH v3] t1300: use test helpers instead of test builtins

[Pushkar Singh]

This version updates the commit message to avoid calling `test` a shell
primitive, as suggested.

Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
---
 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

[PATCH v3] t1300: use test helpers instead of test builtins

[Abraham Samuel Adekunle]

>This version updates the commit message to avoid calling `test` a shell
>primitive, as suggested.

>Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
>---

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.

Re: [PATCH v3] t1300: use test helpers instead of test builtins

[Pushkar Singh]

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
<abrahamadekunle50@gmail.com> wrote:
>
> >This version updates the commit message to avoid calling `test` a shell
> >primitive, as suggested.
>
> >Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
> >---
>
> 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.

Re: [PATCH v3] t1300: use test helpers instead of test builtins

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 1557 bytes --]

Pushkar Singh <pushkarkumarsingh1970@gmail.com> 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
> <abrahamadekunle50@gmail.com> wrote:
>>
>> >This version updates the commit message to avoid calling `test` a shell
>> >primitive, as suggested.
>>
>> >Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
>> >---
>>
>> 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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v3] t1300: use test helpers instead of test builtins

[Samuel Abraham]

On Mon, Jan 5, 2026 at 11:55 AM Karthik Nayak <karthik.188@gmail.com> wrote:
>
> Pushkar Singh <pushkarkumarsingh1970@gmail.com> 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.

[PATCH v4] t1300: use test helpers instead of test builtins

[Pushkar Singh]

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 <pushkarkumarsingh1970@gmail.com>
---
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

Re: [PATCH v4] t1300: use test helpers instead of test builtins

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 369 bytes --]

Pushkar Singh <pushkarkumarsingh1970@gmail.com> 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 <pushkarkumarsingh1970@gmail.com>

This version looks good to me. Thanks.

[snip]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]