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

From: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
Date: Jan 2, 2026, 09:39 UTC
Message-ID: <CALE2CrRkKneSJL_F-mF_aTBzbGwW8c5A+qSU+8etQdbB=o2wJg@mail.gmail.com>
In-Reply-To: <CAOLa=ZQrKO-Aeoa_9E9Gi3rzM2AuVETDT4sUEuc0V8XQUyk4Pw@mail.gmail.com>

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:

Show 53 quoted lines

>
> 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.