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

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2327 bytes --]

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

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]