From: Mahendra Dani <danimahendra0904@gmail.com>
Date: Tue, 04 Mar 2025 18:19:40 GMT
Subject: Re: [PATCH v4 1/1] t1403: verify that path exists and is a file
Message-ID: <CABGrwBAhjSkodUe-SGxJsZLRE2zgfqr+3W8U-Op+jG5iKBK80g@mail.gmail.com>
In-Reply-To: <xmqq5xkopsti.fsf@gitster.g>

On Tue, Mar 4, 2025 at 11:43 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Mahendra Dani <danimahendra0904@gmail.com> writes:
>
> > Verify that if the path exists then it is a file using test_path_is_file().
> >
> > Signed-off-by: Mahendra Dani <danimahendra0904@gmail.com>
> > ---
> >  t/t1403-show-ref.sh | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh
> > index 9d698b3cc3..9da3650e91 100755
> > --- a/t/t1403-show-ref.sh
> > +++ b/t/t1403-show-ref.sh
> > @@ -196,7 +196,7 @@ test_expect_success 'show-ref --verify with dangling ref' '
> >
> >       remove_object() {
> >               file=$(sha1_file "$*") &&
> > -             test -e "$file" &&
> > +             test_path_is_file "$file" &&
> >               rm -f "$file"
> >       } &&
>
> Yup, this makes perfect sense.  I would have explained it a bit
> differently, perhaps like
>
>     The original uses 'test -e' to ensure that the file exists, but
>     (1) it fails silently if the expectation is not met, and (2) we
>     expect the loose object file not just to exist but to be a file
>     (in other words, the original should have been 'test -f' in the
>     first place).
>
>     Use test_path_is_file to improve on both points.
>
> or something, but the proposed commit log message is sufficiently
> readable.

I will take more care while writing commit messages and cover letter
from now onwards.
>
> Will queue.

Thanks.

>
> Thanks.

Thanks,
Mahendra