Subject: Re: [PATCH v3 1/1] [PATCH] t9117: prefer test_path_* helper functions

From: Patrick Steinhardt <[email-masked]>
Date: Mon, 4 Mar 2024 10:24:03 +0100

On Fri, Mar 01, 2024 at 09:03:34PM +0800, shejialuo wrote:
> test -(e|f) does not provide a nice error message when we hit test
> failures, so use test_path_exists, test_path_is_dir instead.

Nit: you mention `test -e` and `test -f`, but then talk about
`test_path_exists` (correct) and `test_path_is_dir` (wrong). You
probably meant to write `test -(e|d)`.

Other than that all the conversions look correct to me. Thanks!

Patrick

> 
> Signed-off-by: shejialuo <[email-masked]>
> ---
>  t/t9117-git-svn-init-clone.sh | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/t/t9117-git-svn-init-clone.sh b/t/t9117-git-svn-init-clone.sh
> index 62de819a44..3b038c338f 100755
> --- a/t/t9117-git-svn-init-clone.sh
> +++ b/t/t9117-git-svn-init-clone.sh
> @@ -17,32 +17,32 @@ test_expect_success 'setup svnrepo' '
>  test_expect_success 'basic clone' '
>  	test ! -d trunk &&
>  	git svn clone "$svnrepo"/project/trunk &&
> -	test -d trunk/.git/svn &&
> -	test -e trunk/foo &&
> +	test_path_is_dir trunk/.git/svn &&
> +	test_path_exists trunk/foo &&
>  	rm -rf trunk
>  	'
>  
>  test_expect_success 'clone to target directory' '
>  	test ! -d target &&
>  	git svn clone "$svnrepo"/project/trunk target &&
> -	test -d target/.git/svn &&
> -	test -e target/foo &&
> +	test_path_is_dir target/.git/svn &&
> +	test_path_exists target/foo &&
>  	rm -rf target
>  	'
>  
>  test_expect_success 'clone with --stdlayout' '
>  	test ! -d project &&
>  	git svn clone -s "$svnrepo"/project &&
> -	test -d project/.git/svn &&
> -	test -e project/foo &&
> +	test_path_is_dir project/.git/svn &&
> +	test_path_exists project/foo &&
>  	rm -rf project
>  	'
>  
>  test_expect_success 'clone to target directory with --stdlayout' '
>  	test ! -d target &&
>  	git svn clone -s "$svnrepo"/project target &&
> -	test -d target/.git/svn &&
> -	test -e target/foo &&
> +	test_path_is_dir target/.git/svn &&
> +	test_path_exists target/foo &&
>  	rm -rf target
>  	'
>  
> -- 
> 2.44.0
> 
>