threads / patch / 39094

patchtest-lib-functions.sh: fix the second argument to some helper functions

Subject: [PATCH] test-lib-functions.sh: fix the second argument to some helper functions

## tl;dr

4 messages between Apr 16, 2015 and Apr 20, 2015. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Elia Pinto· Apr 16, 2015, 14:12 UTC · lore

The second argument to test_path_is_file and test_path_is_dir must be $2 and not $*, which instead would repeat the file name in the error message.

Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
---
 t/test-lib-functions.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Show changes to t/test-lib-functions.sh +2 −2

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 0698ce7..8f8858a 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -478,7 +478,7 @@ test_external_without_stderr () {
 test_path_is_file () {
 	if ! test -f "$1"
 	then
-		echo "File $1 doesn't exist. $*"
+		echo "File $1 doesn't exist. $2"
 		false
 	fi
 }
@@ -486,7 +486,7 @@ test_path_is_file () {
 test_path_is_dir () {
 	if ! test -d "$1"
 	then
-		echo "Directory $1 doesn't exist. $*"
+		echo "Directory $1 doesn't exist. $2"
 		false
 	fi
 }

-- 
2.1.0

Matthieu Moy· Apr 16, 2015, 14:38 UTC · re: Elia Pinto · lore

Re: [PATCH] test-lib-functions.sh: fix the second argument to some helper functions

Elia Pinto <gitter.spiros@gmail.com> writes:

Show 20 quoted lines

> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -478,7 +478,7 @@ test_external_without_stderr () {
>  test_path_is_file () {
>  	if ! test -f "$1"
>  	then
> -		echo "File $1 doesn't exist. $*"
> +		echo "File $1 doesn't exist. $2"
>  		false
>  	fi
>  }
> @@ -486,7 +486,7 @@ test_path_is_file () {
>  test_path_is_dir () {
>  	if ! test -d "$1"
>  	then
> -		echo "Directory $1 doesn't exist. $*"
> +		echo "Directory $1 doesn't exist. $2"
>  		false
>  	fi
>  }

Sounds straightforwardly correct to me.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Junio C Hamano· Apr 16, 2015, 20:31 UTC · re: Matthieu Moy · lore

Re: [PATCH] test-lib-functions.sh: fix the second argument to some helper functions

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

Show 24 quoted lines

> Elia Pinto <gitter.spiros@gmail.com> writes:
>
>> --- a/t/test-lib-functions.sh
>> +++ b/t/test-lib-functions.sh
>> @@ -478,7 +478,7 @@ test_external_without_stderr () {
>>  test_path_is_file () {
>>  	if ! test -f "$1"
>>  	then
>> -		echo "File $1 doesn't exist. $*"
>> +		echo "File $1 doesn't exist. $2"
>>  		false
>>  	fi
>>  }
>> @@ -486,7 +486,7 @@ test_path_is_file () {
>>  test_path_is_dir () {
>>  	if ! test -d "$1"
>>  	then
>> -		echo "Directory $1 doesn't exist. $*"
>> +		echo "Directory $1 doesn't exist. $2"
>>  		false
>>  	fi
>>  }
>
> Sounds straightforwardly correct to me.

Thanks. This however makes me wonder why you were nominated for reviewing this patch, though...

Matthieu Moy· Apr 20, 2015, 11:03 UTC · re: Junio C Hamano · lore

Re: [PATCH] test-lib-functions.sh: fix the second argument to some helper functions

Junio C Hamano <gitster@pobox.com> writes:

Show 29 quoted lines

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> Elia Pinto <gitter.spiros@gmail.com> writes:
>>
>>> --- a/t/test-lib-functions.sh
>>> +++ b/t/test-lib-functions.sh
>>> @@ -478,7 +478,7 @@ test_external_without_stderr () {
>>>  test_path_is_file () {
>>>  	if ! test -f "$1"
>>>  	then
>>> -		echo "File $1 doesn't exist. $*"
>>> +		echo "File $1 doesn't exist. $2"
>>>  		false
>>>  	fi
>>>  }
>>> @@ -486,7 +486,7 @@ test_path_is_file () {
>>>  test_path_is_dir () {
>>>  	if ! test -d "$1"
>>>  	then
>>> -		echo "Directory $1 doesn't exist. $*"
>>> +		echo "Directory $1 doesn't exist. $2"
>>>  		false
>>>  	fi
>>>  }
>>
>> Sounds straightforwardly correct to me.
>
> Thanks.  This however makes me wonder why you were nominated for
> reviewing this patch, though...

It seems I'm the one who introduced the bug indeed, in 2caf20c52b7f6.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/