Re: [PATCH v4 1/1] t1403: verify that path exists and is a file

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 4, 2025, 18:13 UTC
Message-ID: <xmqq5xkopsti.fsf@gitster.g>
In-Reply-To: <20250304112728.41228-2-danimahendra0904@gmail.com>

Mahendra Dani <danimahendra0904@gmail.com> writes:

Show 19 quoted lines

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

Will queue.

Thanks.