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

On 21/06/2021 11:08, Andrei Rybak wrote:

On 19/06/2021 03:30, Đoàn Trần Công Danh wrote:

quoted

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"

?

quoted

+        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: '$@'"