Re: [GSoC][PATCH] t/t5000-tar-tree: add helper function

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 2, 2023, 22:36 UTC
Message-ID: <xmqqa61vsniy.fsf@gitster.g>
In-Reply-To: <20230202202557.19297-1-kostya.farber@gmail.com>

Kostya Farber <kostya.farber@gmail.com> writes:

> Add the helper function test_file_path_exists to the
> interpret pax header test. This change makes it clearer
> as to what the test is trying to check, in this case whether
> a file path exists.

Really?

The code with "test -e" is already clear that it is checking if the path $data exists. This change does not make it any clearer. What it helps is that it gives a message upon failure, when the test is run with the "-v" option.

Show 13 quoted lines

> diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
> index d473048138..19d5bd0c04 100755
> --- a/t/t5000-tar-tree.sh
> +++ b/t/t5000-tar-tree.sh
> @@ -73,7 +73,7 @@ check_tar() {
>  			for header in *.paxheader
>  			do
>  				data=${header%.paxheader}.data &&
> -				if test -h $data || test -e $data
> +				if test -h $data || test_file_path_exists $data
>  				then
>  					path=$(get_pax_header $header path) &&
>  					if test -n "$path"

Nothing seems to be adding a new helper whose name is test_file_path_exists; the patch expects such a helper already exists and uses it in place for existing "test -e".

Perhaps you meant to say "use test_path_exists" not "add helper" on the title, and use that function in the patch instead?

Thanks.