[GSOC,v2] t1300: convert "test -f" with "test_path_is_file"
diff mbox series

Message ID 20200320160723.15190-1-adrianwijaya100@gmail.com
State New
Headers show
Series
  • [GSOC,v2] t1300: convert "test -f" with "test_path_is_file"
Related show

Commit Message

Adrian Wijaya March 20, 2020, 4:07 p.m. UTC
Convert "test -f" with "test_path_is_file" to give more verbose
test output on failure.

Signed-off-by: Adrian Wijaya <adrianwijaya100@gmail.com>
---
 t/t1300-config.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Junio C Hamano March 20, 2020, 6:04 p.m. UTC | #1
Adrian Wijaya <adrianwijaya100@gmail.com> writes:

> Subject: Re: [GSOC][PATCH v2] t1300: convert "test -f" with "test_path_is_file"

I thought the suggestion was either "replace with" or "convert
into", but the above looks a bit funny cross between them.  I'd
probably go with "replace".

> Convert "test -f" with "test_path_is_file" to give more verbose
> test output on failure.

Likewise, but the first part is already said in the title.  So

	Subject: t1300: replace "test -f" with "test_path_is_file"

	This way, the test will not fail silently, making it easier
	to diagnose.

or something like that, perhaps.

> Signed-off-by: Adrian Wijaya <adrianwijaya100@gmail.com>
> ---
>  t/t1300-config.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/t1300-config.sh b/t/t1300-config.sh
> index 97ebfe1f9d..d74554fc09 100755
> --- a/t/t1300-config.sh
> +++ b/t/t1300-config.sh
> @@ -1020,11 +1020,11 @@ test_expect_success SYMLINKS 'symlinked configuration' '
>  	ln -s notyet myconfig &&
>  	git config --file=myconfig test.frotz nitfol &&
>  	test -h myconfig &&
> -	test -f notyet &&
> +	test_path_is_file notyet &&
>  	test "z$(git config --file=notyet test.frotz)" = znitfol &&
>  	git config --file=myconfig test.xyzzy rezrov &&
>  	test -h myconfig &&
> -	test -f notyet &&
> +	test_path_is_file notyet &&
>  	cat >expect <<-\EOF &&
>  	nitfol
>  	rezrov
Junio C Hamano March 20, 2020, 6:08 p.m. UTC | #2
Adrian Wijaya <adrianwijaya100@gmail.com> writes:

>  	test "z$(git config --file=notyet test.frotz)" = znitfol &&

We see 130+ instances of this rather antiquated construct that
presumably avoids to compare an empty string with something else
using "test X = Y" tool.  It might have been a good idea in 2005 (I
do not remember why we started doing this), but perhaps it is time
to clean them up by listing it as another microproject idea for the
next year?

Patch
diff mbox series

diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 97ebfe1f9d..d74554fc09 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -1020,11 +1020,11 @@  test_expect_success SYMLINKS 'symlinked configuration' '
 	ln -s notyet myconfig &&
 	git config --file=myconfig test.frotz nitfol &&
 	test -h myconfig &&
-	test -f notyet &&
+	test_path_is_file notyet &&
 	test "z$(git config --file=notyet test.frotz)" = znitfol &&
 	git config --file=myconfig test.xyzzy rezrov &&
 	test -h myconfig &&
-	test -f notyet &&
+	test_path_is_file notyet &&
 	cat >expect <<-\EOF &&
 	nitfol
 	rezrov