From: Mahendra Dani <danimahendra0904@gmail.com>
Date: Tue, 04 Mar 2025 18:13:23 GMT
Subject: Re: [PATCH v3 1/1] t1403: verify that path exists and is a file
Message-ID: <CABGrwBBcg9DnPHtDG=OjQWJKWJy1mD26xXm7r5W80GNBgqFa-g@mail.gmail.com>
In-Reply-To: <xmqqo6ygpt5i.fsf@gitster.g>

On Tue, Mar 4, 2025 at 11:36 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Mahendra Dani <danimahendra0904@gmail.com> writes:
> >
> >> test -e does not provide a nice error message when
> >> we hit test failures, so use test_path_exists() instead
> >> and 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 | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh
> >> index 9d698b3cc3..4afde01a29 100755
> >> --- a/t/t1403-show-ref.sh
> >> +++ b/t/t1403-show-ref.sh
> >> @@ -196,7 +196,8 @@ test_expect_success 'show-ref --verify with dangling ref' '
> >>
> >>      remove_object() {
> >>              file=$(sha1_file "$*") &&
> >> -            test -e "$file" &&
> >> +            test_path_exists "$file" &&
> >> +            test_path_is_file "$file" &&
> >>              rm -f "$file"
> >>      } &&
> >
> > Makes sense.  Will queue.
>
> No, no, no.  test_is_file alone is sufficient---if the thing does
> not exist, it would not be a file anyway ;-)
>

Yes, it was pointed out by Patrick in [1], that `test_path_is_file()`
alone is sufficient. Hence I removed the `test_path_exists()` check in
patch v4 [2].

[References]
1. https://lore.kernel.org/git/Z8bd3iHrhXb4WH6A@pks.im/
2. https://lore.kernel.org/git/20250304112728.41228-2-danimahendra0904@gmail.com/