diff mbox series

[v3,1/4] test-lib-functions: introduce test_line_count_cmd

Message ID 20210619013035.26313-2-congdanhqx@gmail.com (mailing list archive)
State New
Headers show
Series [v3,1/4] test-lib-functions: introduce test_line_count_cmd | expand

Commit Message

Đoàn Trần Công Danh June 19, 2021, 1:30 a.m. UTC
In the Git project, we have multiple instances that requires
checking number of lines of text in the stdout of a command.
One of such examples is t6400, that checks number of files
in various states.

Some of those commands are Git command, and we would like to check
their exit status.  In some of those checks, we pipe the stdout of
those commands to "wc -l" to count the number lines, thus losing
the exit status.

Introduce a helper function to check for the number of lines in stdout
from those commands.

This helper will create a temporary file in the process, thus it may
affect the output of some checks.

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 t/test-lib-functions.sh | 80 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

Comments

Andrei Rybak June 21, 2021, 9:08 a.m. UTC | #1
On 19/06/2021 03:30, Đoàn Trần Công Danh wrote:
> In the Git project, we have multiple instances that requires

s/requires/require/

> checking number of lines of text in the stdout of a command.
> One of such examples is t6400, that checks number of files
> in various states.
> 
> Some of those commands are Git command, and we would like to check
> their exit status.  In some of those checks, we pipe the stdout of
> those commands to "wc -l" to count the number lines, thus losing
> the exit status.
> 
> Introduce a helper function to check for the number of lines in stdout
> from those commands.
> 
> This helper will create a temporary file in the process, thus it may
> affect the output of some checks.
> 
> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> ---
>   t/test-lib-functions.sh | 80 +++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 80 insertions(+)
> 
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index f0448daa74..e055a4f808 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -845,6 +845,86 @@ test_line_count () {
>   	fi
>   }
>   
> +# test_line_count_cmd checks the exit status, and the number of lines in
> +# the captured stdout of a command.
> +#
> +# SYNOPSIS:
> +#
> +#	test_line_count_cmd <binop> <value> [!] cmd [args...]
> +#
> +# Expect succeed exit status when running
> +#
> +#	cmd [args...]
> +#
> +# then, run sh's "test <# of lines in stdout> <binop> <value>"
> +#
> +# OPTIONS:
> +# !:
> +#	Instead of expecting "cmd [args...]" succeed, expect its failure.
> +#	Note, if the command under testing is "git",
> +#	test_must_fail should be used instead of "!".
> +#
> +# EXAMPLE:
> +#	test_line_count_cmd -ge 10 git tag --no-contains v1.0.0
> +#	test_line_count_cmd -le 10 ! grep some-text a-file
> +#	test_line_count_cmd = 0 test_must_fail git rev-parse --verify abcd1234
> +#
> +# NOTE:
> +# * a temporary file named test_line_count_cmd_.out will be created under
> +# $TRASH_DIRECTORY/.git/trash iff $TRASH_DIRECTORY/.git/ exists.
> +# Otherwise, created in $TRASH_DIRECTORY. This temporary file will be
> +# cleaned by test_when_finished
> +test_line_count_cmd () {
> +	{
> +		local outop outval outfile
> +		local expect_failure actual_failure
> +		local trashdir="$TRASH_DIRECTORY"
> +
> +		if test -d "$TRASH_DIRECTORY/.git"
> +		then
> +			trashdir="$TRASH_DIRECTORY/.git/trash" &&
> +			mkdir -p "$trashdir"
> +		fi &&
> +		if test $# -lt 3
> +		then
> +			BUG "missing <binary-ops> and <value>"

Nit: s/binary-ops/binop/ for consistency with documentation comment
above.  Also, technically the invocation of test_line_count_cmd could be
missing any of its required three parameters, including "cmd".  How
about something similar to the call to BUG in test_i18ngrep:

	BUG "too few parameters to test_line_count_cmd"

?

> +		fi &&
> +		outop="$1" &&
> +		outval="$2" &&
> +		shift 2 &&
> +		outfile="$trashdir/test_line_count_cmd_.out" &&
> +		if test "x$1" = "x!"
> +		then
> +			shift &&
> +			expect_failure=yes
> +		fi &&
> +		if test $# = 0
> +		then
> +			BUG "test_line_count_cmd: no command to be run"
> +		else
> +			test_when_finished "rm -f '$outfile'" &&
> +			exec 8>"$outfile"
> +			# We need to redirect stderr to &9,
> +			# and redirect this function's 9>&2
> +			# in order to not messed with -x
> +			if ! "$@" >&8 2>&9
> +			then
> +				actual_failure=yes
> +			fi
> +		fi 8>&1 &&
> +		case "$expect_failure,$actual_failure" in
> +		yes,)
> +			echo >&4 "error: '$@' succeed!" &&

It seems that function error() could be used here and below instead of
"echo >&4".

s/succeed/succeeded/ --- it might be worth borrowing wording from
test_must_fail().  Something like:

	error "test_line_count_cmd: command succeeded: '$@'"

> +			return 1
> +			;;
> +		,yes)
> +			echo >&4 "error: '$@' run into failure!" &&
> +			return 1
> +		esac &&

Missing ";;" in the last branch of `case`.

> +		test_line_count "$outop" "$outval" "$outfile" >&4
> +	} 9>&2 2>&4
> +}
> +
>   test_file_size () {
>   	test "$#" -ne 1 && BUG "1 param"
>   	test-tool path-utils file-size "$1"
>
Andrei Rybak June 24, 2021, 7:23 p.m. UTC | #2
On 21/06/2021 11:08, Andrei Rybak wrote:
> On 19/06/2021 03:30, Đoàn Trần Công Danh wrote:
>> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
>> index f0448daa74..e055a4f808 100644
>> --- a/t/test-lib-functions.sh
>> +++ b/t/test-lib-functions.sh
>> @@ -845,6 +845,86 @@ test_line_count () {
>>       fi
>>   }
>> +# test_line_count_cmd checks the exit status, and the number of lines in
>> +# the captured stdout of a command.
>> +#
>> +# SYNOPSIS:
>> +#
>> +#    test_line_count_cmd <binop> <value> [!] cmd [args...]
>> +#
>> +# Expect succeed exit status when running
>> +#
>> +#    cmd [args...]
>> +#
>> +# then, run sh's "test <# of lines in stdout> <binop> <value>"
>> +#
>> +# OPTIONS:
>> +# !:
>> +#    Instead of expecting "cmd [args...]" succeed, expect its failure.
>> +#    Note, if the command under testing is "git",
>> +#    test_must_fail should be used instead of "!".
>> +#
>> +# EXAMPLE:
>> +#    test_line_count_cmd -ge 10 git tag --no-contains v1.0.0
>> +#    test_line_count_cmd -le 10 ! grep some-text a-file
>> +#    test_line_count_cmd = 0 test_must_fail git rev-parse --verify 
>> abcd1234
>> +#
>> +# NOTE:
>> +# * a temporary file named test_line_count_cmd_.out will be created 
>> under
>> +# $TRASH_DIRECTORY/.git/trash iff $TRASH_DIRECTORY/.git/ exists.
>> +# Otherwise, created in $TRASH_DIRECTORY. This temporary file will be
>> +# cleaned by test_when_finished
>> +test_line_count_cmd () {
>> +    {
>> +        local outop outval outfile
>> +        local expect_failure actual_failure
>> +        local trashdir="$TRASH_DIRECTORY"
>> +
>> +        if test -d "$TRASH_DIRECTORY/.git"
>> +        then
>> +            trashdir="$TRASH_DIRECTORY/.git/trash" &&
>> +            mkdir -p "$trashdir"
>> +        fi &&
>> +        if test $# -lt 3
>> +        then
>> +            BUG "missing <binary-ops> and <value>"
> 
> Nit: s/binary-ops/binop/ for consistency with documentation comment
> above.  Also, technically the invocation of test_line_count_cmd could be
> missing any of its required three parameters, including "cmd".  How
> about something similar to the call to BUG in test_i18ngrep:
> 
>      BUG "too few parameters to test_line_count_cmd"
> 
> ?
> 
>> +        fi &&
>> +        outop="$1" &&
>> +        outval="$2" &&
>> +        shift 2 &&
>> +        outfile="$trashdir/test_line_count_cmd_.out" &&
>> +        if test "x$1" = "x!"
>> +        then
>> +            shift &&
>> +            expect_failure=yes
>> +        fi &&
>> +        if test $# = 0
>> +        then
>> +            BUG "test_line_count_cmd: no command to be run"
>> +        else
>> +            test_when_finished "rm -f '$outfile'" &&
>> +            exec 8>"$outfile"
>> +            # We need to redirect stderr to &9,
>> +            # and redirect this function's 9>&2
>> +            # in order to not messed with -x
>> +            if ! "$@" >&8 2>&9
>> +            then
>> +                actual_failure=yes
>> +            fi
>> +        fi 8>&1 &&
>> +        case "$expect_failure,$actual_failure" in
>> +        yes,)
>> +            echo >&4 "error: '$@' succeed!" &&
> 
> It seems that function error() could be used here and below instead of
> "echo >&4".

After spending some time reading t/test-lib-functions.sh, now I don't
think that using error() is a good suggestion.  Closest relatives of
test_line_count_cmd -- test_line_count and test_must_be_empty -- both
just use "echo".  Other usages of error() in t/test-lib.sh and
t/test-lib-functions.sh suggest that error() is not meant to be used
for reporting test failure messages, but internal failures. For example:

	error "You haven't built things yet, have you?"

and

	error "Internal error: $stderr disappeared."

> s/succeed/succeeded/ --- it might be worth borrowing wording from
> test_must_fail().  Something like:
> 
>      error "test_line_count_cmd: command succeeded: '$@'"
diff mbox series

Patch

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index f0448daa74..e055a4f808 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -845,6 +845,86 @@  test_line_count () {
 	fi
 }
 
+# test_line_count_cmd checks the exit status, and the number of lines in
+# the captured stdout of a command.
+#
+# SYNOPSIS:
+#
+#	test_line_count_cmd <binop> <value> [!] cmd [args...]
+#
+# Expect succeed exit status when running
+#
+#	cmd [args...]
+#
+# then, run sh's "test <# of lines in stdout> <binop> <value>"
+#
+# OPTIONS:
+# !:
+#	Instead of expecting "cmd [args...]" succeed, expect its failure.
+#	Note, if the command under testing is "git",
+#	test_must_fail should be used instead of "!".
+#
+# EXAMPLE:
+#	test_line_count_cmd -ge 10 git tag --no-contains v1.0.0
+#	test_line_count_cmd -le 10 ! grep some-text a-file
+#	test_line_count_cmd = 0 test_must_fail git rev-parse --verify abcd1234
+#
+# NOTE:
+# * a temporary file named test_line_count_cmd_.out will be created under
+# $TRASH_DIRECTORY/.git/trash iff $TRASH_DIRECTORY/.git/ exists.
+# Otherwise, created in $TRASH_DIRECTORY. This temporary file will be
+# cleaned by test_when_finished
+test_line_count_cmd () {
+	{
+		local outop outval outfile
+		local expect_failure actual_failure
+		local trashdir="$TRASH_DIRECTORY"
+
+		if test -d "$TRASH_DIRECTORY/.git"
+		then
+			trashdir="$TRASH_DIRECTORY/.git/trash" &&
+			mkdir -p "$trashdir"
+		fi &&
+		if test $# -lt 3
+		then
+			BUG "missing <binary-ops> and <value>"
+		fi &&
+		outop="$1" &&
+		outval="$2" &&
+		shift 2 &&
+		outfile="$trashdir/test_line_count_cmd_.out" &&
+		if test "x$1" = "x!"
+		then
+			shift &&
+			expect_failure=yes
+		fi &&
+		if test $# = 0
+		then
+			BUG "test_line_count_cmd: no command to be run"
+		else
+			test_when_finished "rm -f '$outfile'" &&
+			exec 8>"$outfile"
+			# We need to redirect stderr to &9,
+			# and redirect this function's 9>&2
+			# in order to not messed with -x
+			if ! "$@" >&8 2>&9
+			then
+				actual_failure=yes
+			fi
+		fi 8>&1 &&
+		case "$expect_failure,$actual_failure" in
+		yes,)
+			echo >&4 "error: '$@' succeed!" &&
+			return 1
+			;;
+		,yes)
+			echo >&4 "error: '$@' run into failure!" &&
+			return 1
+		esac &&
+		test_line_count "$outop" "$outval" "$outfile" >&4
+	} 9>&2 2>&4
+}
+
 test_file_size () {
 	test "$#" -ne 1 && BUG "1 param"
 	test-tool path-utils file-size "$1"