diff mbox series

[v2,3/3] test-lib-functions: remove last two parameter count assertions

Message ID patch-3.3-8fd51861b5-20210420T122706Z-avarab@gmail.com (mailing list archive)
State Superseded
Headers show
Series test-lib-functions.sh: trickery to make -x less verbose | expand

Commit Message

Ævar Arnfjörð Bjarmason April 20, 2021, 12:29 p.m. UTC
Remove a couple of parameter count assertions where, unlike the
preceding commit's migration to 'test -$x "$@"', we'll now silently do
the "wrong" thing if given too many parameters. The benefit is less
verbose trace output, as noted in the preceding commit.

In the case of "test_file_size", the "test-tool" we're invoking is
happy to accept N parameters (it'll print out all N sizes). Let's just
use "$@" in that case anyway. There's only a few callers, and
eventually those should probably be moved to use the test-tool
directly.

That only leaves test_line_count, I suppose I could leave that one
alone, but since it's the only common function left that does this
assertion let's remove it for the brevity of the -x output and
consistency with other functions.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/test-lib-functions.sh | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

Comments

Junio C Hamano April 20, 2021, 9:15 p.m. UTC | #1
Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Remove a couple of parameter count assertions where, unlike the
> preceding commit's migration to 'test -$x "$@"', we'll now silently do
> the "wrong" thing if given too many parameters. The benefit is less
> verbose trace output, as noted in the preceding commit.
>
> In the case of "test_file_size", the "test-tool" we're invoking is
> happy to accept N parameters (it'll print out all N sizes). Let's just
> use "$@" in that case anyway. There's only a few callers, and
> eventually those should probably be moved to use the test-tool
> directly.
>
> That only leaves test_line_count, I suppose I could leave that one
> alone, but since it's the only common function left that does this
> assertion let's remove it for the brevity of the -x output and
> consistency with other functions.

Unlike the previous step that did not entirely sacrifice the "test
writers are human and they need our help in detecting their
mistakes", this step feels like "shortening '-x' output is the most
important thing in the world, and helping test writers is much less
important."

IOW, I am not convinced it is a good change.


> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  t/test-lib-functions.sh | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index f8f5bf9de1..e128b341ff 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -827,10 +827,7 @@ test_path_is_missing () {
>  # output through when the number of lines is wrong.
>  
>  test_line_count () {
> -	if test $# != 3
> -	then
> -		BUG "not 3 parameters to test_line_count"
> -	elif ! test $(wc -l <"$3") "$1" "$2"
> +	if ! test $(wc -l <"$3") "$1" "$2"
>  	then
>  		echo "test_line_count: line count for $3 !$1 $2"
>  		cat "$3"
> @@ -839,8 +836,7 @@ test_line_count () {
>  }
>  
>  test_file_size () {
> -	test "$#" -ne 1 && BUG "1 param"
> -	test-tool path-utils file-size "$1"
> +	test-tool path-utils file-size "$@"
>  }
>  
>  # Returns success if a comma separated string of keywords ($1) contains a
diff mbox series

Patch

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index f8f5bf9de1..e128b341ff 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -827,10 +827,7 @@  test_path_is_missing () {
 # output through when the number of lines is wrong.
 
 test_line_count () {
-	if test $# != 3
-	then
-		BUG "not 3 parameters to test_line_count"
-	elif ! test $(wc -l <"$3") "$1" "$2"
+	if ! test $(wc -l <"$3") "$1" "$2"
 	then
 		echo "test_line_count: line count for $3 !$1 $2"
 		cat "$3"
@@ -839,8 +836,7 @@  test_line_count () {
 }
 
 test_file_size () {
-	test "$#" -ne 1 && BUG "1 param"
-	test-tool path-utils file-size "$1"
+	test-tool path-utils file-size "$@"
 }
 
 # Returns success if a comma separated string of keywords ($1) contains a