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

From: Patrick Steinhardt <ps@pks.im>
Date: Mar 4, 2024, 09:59 UTC
Message-ID: <ZeWbdvFmhUYN9ekE@tanuki>
In-Reply-To: <20240304095436.56399-2-shejialuo@gmail.com>

On Mon, Mar 04, 2024 at 05:54:36PM +0800, shejialuo wrote:

> test -(e|d) does not provide a nice error message when we hit test
> failures, so use test_path_exists, test_path_is_dir instead.
> 
> Signed-off-by: shejialuo <shejialuo@gmail.com>

This version looks good to me, thanks!

One suggestion for potential future contributions by you: it's always helpful to create a "range-diff" of what has changed between the previous version of your patch series and the next one. Like this, reviewers can immediately see what the difference is between the two versions, which helps them to get the review done faster.

Assuming you use git-format-patch(1) you can generate such a range diff with the `--range-diff=` parameter.

Patrick

Show 52 quoted lines

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