Re: [GSoC PATCH v2] t9200: replace test -f/-d with modern path helpers

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 9, 2026, 21:03 UTC
Message-ID: <xmqq8qc04sxh.fsf@gitster.g>
In-Reply-To: <20260309162832.605969-1-pabloosabaterr@gmail.com>

Pablo Sabater <pabloosabaterr@gmail.com> writes:

> Replace old style 'test -f' and 'test -d' with modern helpers
> 'test_path_is_file' and 'test_path_is_dir' respectively.

OK. Being "modern" does not automatically mean "better", and it would be helpful to say why we do this change for those relatively unexperienced who will read "git log" later and find this commit. Perhaps

    Replace ... with ..., because it makes debugging a failing test
    easier by loudly reporting what expectation was not met.

or something.  The patch text looks good.

Thanks.

Show 8 quoted lines

> The instances were found with:
>
> 	git grep "test -[efd]" t/
>
> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
> ---
>  t/t9200-git-cvsexportcommit.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

OK.

Show 22 quoted lines

> diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh
> index a44eabf0d8..4507e8e6db 100755
> --- a/t/t9200-git-cvsexportcommit.sh
> +++ b/t/t9200-git-cvsexportcommit.sh
> @@ -31,7 +31,7 @@ export CVSROOT CVSWORK GIT_DIR
>  rm -rf "$CVSROOT" "$CVSWORK"
>  
>  cvs init &&
> -test -d "$CVSROOT" &&
> +test_path_is_dir "$CVSROOT" &&
>  cvs -Q co -d "$CVSWORK" . &&
>  echo >empty &&
>  git add empty &&
> @@ -303,7 +303,7 @@ test_expect_success 're-commit a removed filename which remains in CVS attic' '
>  	git commit -m "Added attic_gremlin" &&
>  	git cvsexportcommit -w "$CVSWORK" -c HEAD &&
>  	(cd "$CVSWORK" && cvs -Q update -d) &&
> -	test -f "$CVSWORK/attic_gremlin"
> +	test_path_is_file "$CVSWORK/attic_gremlin"
>  '
>  
>  # the state of the CVS sandbox may be indeterminate for ' space'