[PATCH v2 0/5] t: new helper test_line_count_cmd

[PATCH v2 0/5] t: new helper test_line_count_cmd

[Đoàn Trần Công Danh]

This is a series to clear false positive when applying Junio's suggestion to
to a series written by Ævar [1].

Change since v1:
* Documentation for test_line_count_cmd has been written in more detail
  with examples
* The outfile and errfile will be created only if --out and/or --err was
  specified for better "-v"
* outfile and errfile will be created in $TRASH_DIRECTORY/.git/trash
  iff $TRASH_DIRECTORY/.git is a directory, otherwise $TRASH_DIRECTORY,
  avoid "git rev-parse --git-dir" because we may want to test it, too.
* Use test_when_finished to clean those files instead of manual "rm -f",
  also for better "-v"
* Merge multiple instance of "$@" run into one, for better auditing
* t0041 is also converted to use new helper
* With the change to location of outfile and errfile,
  output of "git ls-files -o" has been restored.
* Fix double space before "&&" in the end of test command.

1: https://lore.kernel.org/git/87r1j42ffz.fsf@evledraar.gmail.com/

Đoàn Trần Công Danh (5):
  test-lib-functions: introduce test_line_count_cmd
  t6402: use find(1) builtin to filter instead of grep
  t0041: use test_line_count_cmd to check std{out,err}
  t6400: use test_line_count_cmd to count # of lines in stdout
  t6402: use test_line_count_cmd to count # of lines in stdout

 t/t0041-usage.sh        |  53 ++++++----------
 t/t6400-merge-df.sh     |  16 ++---
 t/t6402-merge-rename.sh | 132 +++++++++++++++++++---------------------
 t/test-lib-functions.sh | 117 +++++++++++++++++++++++++++++++++++
 4 files changed, 204 insertions(+), 114 deletions(-)

Range-diff against v1:
1:  bdce5e51ff < -:  ---------- test-lib-functions: introduce test_line_count_cmd
-:  ---------- > 1:  a823312b19 test-lib-functions: introduce test_line_count_cmd
2:  09a440ed25 = 2:  6e8f2d4289 t6402: use find(1) builtin to filter instead of grep
-:  ---------- > 3:  33daa5ee2f t0041: use test_line_count_cmd to check std{out,err}
3:  98a335a442 ! 4:  729ebb8f50 t6400: use test_line_count_cmd to count # of lines in stdout
    @@ t/t6400-merge-df.sh: test_expect_success 'modify/delete + directory/file conflic
     -	test 5 -eq $(git ls-files -s | wc -l) &&
     -	test 4 -eq $(git ls-files -u | wc -l) &&
     +	test_line_count_cmd --out = 5 git ls-files -s &&
    -+	test_line_count_cmd --out = 4 git ls-files -u  &&
    ++	test_line_count_cmd --out = 4 git ls-files -u &&
      	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
      	then
     -		test 0 -eq $(git ls-files -o | wc -l)
    -+		test_line_count_cmd --out = 2 git ls-files -o
    ++		test_line_count_cmd --out = 0 git ls-files -o
      	else
     -		test 1 -eq $(git ls-files -o | wc -l)
    -+		test_line_count_cmd --out = 3 git ls-files -o
    ++		test_line_count_cmd --out = 1 git ls-files -o
      	fi &&
      
      	test_path_is_file letters/file &&
    @@ t/t6400-merge-df.sh: test_expect_success 'modify/delete + directory/file conflic
      
     -	test 5 -eq $(git ls-files -s | wc -l) &&
     -	test 4 -eq $(git ls-files -u | wc -l) &&
    -+	test_line_count_cmd --out = 5 git ls-files -s  &&
    -+	test_line_count_cmd --out = 4 git ls-files -u  &&
    ++	test_line_count_cmd --out = 5 git ls-files -s &&
    ++	test_line_count_cmd --out = 4 git ls-files -u &&
      	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
      	then
     -		test 0 -eq $(git ls-files -o | wc -l)
    -+		test_line_count_cmd --out = 2 git ls-files -o
    ++		test_line_count_cmd --out = 0 git ls-files -o
      	else
     -		test 1 -eq $(git ls-files -o | wc -l)
    -+		test_line_count_cmd --out = 3 git ls-files -o
    ++		test_line_count_cmd --out = 1 git ls-files -o
      	fi &&
      
      	test_path_is_file letters/file &&
4:  69e4a0b6d7 ! 5:  1b450e4148 t6402: use test_line_count_cmd to count # of lines in stdout
    @@ t/t6402-merge-rename.sh: test_expect_success 'Rename+D/F conflict; renamed file
      
     -	test 3 -eq "$(git ls-files -u | wc -l)" &&
     -	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
    -+	test_line_count_cmd --out = 3 git ls-files -u  &&
    -+	test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way  &&
    ++	test_line_count_cmd --out = 3 git ls-files -u &&
    ++	test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way &&
      
      	test_must_fail git diff --quiet &&
      	test_must_fail git diff --cached --quiet &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'Same as previous, but merged other
      
     -	test 3 -eq "$(git ls-files -u | wc -l)" &&
     -	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
    -+	test_line_count_cmd --out = 3 git ls-files -u  &&
    -+	test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way  &&
    ++	test_line_count_cmd --out = 3 git ls-files -u &&
    ++	test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way &&
      
      	test_must_fail git diff --quiet &&
      	test_must_fail git diff --cached --quiet &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'Rename+D/F conflict; renamed file
      
     -	test 3 -eq "$(git ls-files -u | wc -l)" &&
     -	test 3 -eq "$(git ls-files -u dir | wc -l)" &&
    -+	test_line_count_cmd --out = 3 git ls-files -u  &&
    -+	test_line_count_cmd --out = 3 git ls-files -u dir  &&
    ++	test_line_count_cmd --out = 3 git ls-files -u &&
    ++	test_line_count_cmd --out = 3 git ls-files -u dir &&
      
      	test_must_fail git diff --quiet &&
      	test_must_fail git diff --cached --quiet &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'Rename+D/F conflict; renamed file
      	test_must_fail git merge --strategy=recursive dir-in-way &&
      
     -	test 5 -eq "$(git ls-files -u | wc -l)" &&
    -+	test_line_count_cmd --out = 5 git ls-files -u  &&
    ++	test_line_count_cmd --out = 5 git ls-files -u &&
      	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
      	then
     -		test 3 -eq "$(git ls-files -u dir~HEAD | wc -l)"
    @@ t/t6402-merge-rename.sh: test_expect_success 'Rename+D/F conflict; renamed file
     +		test_line_count_cmd --out = 3 grep -v file-in-the-way out
      	fi &&
     -	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
    -+	test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way  &&
    ++	test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way &&
      
      	test_must_fail git diff --quiet &&
      	test_must_fail git diff --cached --quiet &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'Same as previous, but merged other
      	test_must_fail git merge --strategy=recursive renamed-file-has-conflicts &&
      
     -	test 5 -eq "$(git ls-files -u | wc -l)" &&
    -+	test_line_count_cmd --out = 5 git ls-files -u  &&
    ++	test_line_count_cmd --out = 5 git ls-files -u &&
      	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
      	then
     -		test 3 -eq "$(git ls-files -u dir~renamed-file-has-conflicts | wc -l)"
    @@ t/t6402-merge-rename.sh: test_expect_success 'Same as previous, but merged other
     +		test_line_count_cmd --out = 3 grep -v file-in-the-way out
      	fi &&
     -	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
    -+	test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way  &&
    ++	test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way &&
      
      	test_must_fail git diff --quiet &&
      	test_must_fail git diff --cached --quiet &&
    @@ t/t6402-merge-rename.sh: then
     -		test 4 -eq "$(git ls-files -u | wc -l)" &&
     -		test 2 -eq "$(git ls-files -u one | wc -l)" &&
     -		test 2 -eq "$(git ls-files -u two | wc -l)" &&
    -+		test_line_count_cmd --out = 4 git ls-files -u  &&
    -+		test_line_count_cmd --out = 2 git ls-files -u one  &&
    -+		test_line_count_cmd --out = 2 git ls-files -u two  &&
    ++		test_line_count_cmd --out = 4 git ls-files -u &&
    ++		test_line_count_cmd --out = 2 git ls-files -u one &&
    ++		test_line_count_cmd --out = 2 git ls-files -u two &&
      
      		test_must_fail git diff --quiet &&
      
    @@ t/t6402-merge-rename.sh: else
     -		test 2 -eq "$(git ls-files -u | wc -l)" &&
     -		test 1 -eq "$(git ls-files -u one | wc -l)" &&
     -		test 1 -eq "$(git ls-files -u two | wc -l)" &&
    -+		test_line_count_cmd --out = 2 git ls-files -u  &&
    -+		test_line_count_cmd --out = 1 git ls-files -u one  &&
    -+		test_line_count_cmd --out = 1 git ls-files -u two  &&
    ++		test_line_count_cmd --out = 2 git ls-files -u &&
    ++		test_line_count_cmd --out = 1 git ls-files -u one &&
    ++		test_line_count_cmd --out = 1 git ls-files -u two &&
      
      		test_must_fail git diff --quiet &&
      
    @@ t/t6402-merge-rename.sh: test_expect_success 'pair rename to parent of other (D/
     -		test 4 -eq "$(git ls-files -u | wc -l)" &&
     -		test 2 -eq "$(git ls-files -u one | wc -l)" &&
     -		test 2 -eq "$(git ls-files -u two | wc -l)"
    -+		test_line_count_cmd --out = 4 git ls-files -u  &&
    -+		test_line_count_cmd --out = 2 git ls-files -u one  &&
    ++		test_line_count_cmd --out = 4 git ls-files -u &&
    ++		test_line_count_cmd --out = 2 git ls-files -u one &&
     +		test_line_count_cmd --out = 2 git ls-files -u two
      	else
     -		test 2 -eq "$(git ls-files -u | wc -l)" &&
     -		test 1 -eq "$(git ls-files -u one | wc -l)" &&
     -		test 1 -eq "$(git ls-files -u two | wc -l)"
    -+		test_line_count_cmd --out = 2 git ls-files -u  &&
    -+		test_line_count_cmd --out = 1 git ls-files -u one  &&
    ++		test_line_count_cmd --out = 2 git ls-files -u &&
    ++		test_line_count_cmd --out = 1 git ls-files -u one &&
     +		test_line_count_cmd --out = 1 git ls-files -u two
      	fi &&
      
    @@ t/t6402-merge-rename.sh: test_expect_success 'check handling of differently rena
     -		test 1 -eq "$(git ls-files -u two~second-rename | wc -l)" &&
     -		test 1 -eq "$(git ls-files -u original | wc -l)" &&
     -		test 0 -eq "$(git ls-files -o | wc -l)"
    -+		test_line_count_cmd --out = 5 git ls-files -s  &&
    -+		test_line_count_cmd --out = 3 git ls-files -u  &&
    -+		test_line_count_cmd --out = 1 git ls-files -u one~HEAD  &&
    -+		test_line_count_cmd --out = 1 git ls-files -u two~second-rename  &&
    -+		test_line_count_cmd --out = 1 git ls-files -u original  &&
    -+		test_line_count_cmd --out = 2 git ls-files -o
    ++		test_line_count_cmd --out = 5 git ls-files -s &&
    ++		test_line_count_cmd --out = 3 git ls-files -u &&
    ++		test_line_count_cmd --out = 1 git ls-files -u one~HEAD &&
    ++		test_line_count_cmd --out = 1 git ls-files -u two~second-rename &&
    ++		test_line_count_cmd --out = 1 git ls-files -u original &&
    ++		test_line_count_cmd --out = 0 git ls-files -o
      	else
     -		test 5 -eq "$(git ls-files -s | wc -l)" &&
     -		test 3 -eq "$(git ls-files -u | wc -l)" &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'check handling of differently rena
     -		test 1 -eq "$(git ls-files -u two | wc -l)" &&
     -		test 1 -eq "$(git ls-files -u original | wc -l)" &&
     -		test 2 -eq "$(git ls-files -o | wc -l)"
    -+		test_line_count_cmd --out = 5 git ls-files -s  &&
    -+		test_line_count_cmd --out = 3 git ls-files -u  &&
    -+		test_line_count_cmd --out = 1 git ls-files -u one  &&
    -+		test_line_count_cmd --out = 1 git ls-files -u two  &&
    -+		test_line_count_cmd --out = 1 git ls-files -u original  &&
    -+		test_line_count_cmd --out = 4 git ls-files -o
    ++		test_line_count_cmd --out = 5 git ls-files -s &&
    ++		test_line_count_cmd --out = 3 git ls-files -u &&
    ++		test_line_count_cmd --out = 1 git ls-files -u one &&
    ++		test_line_count_cmd --out = 1 git ls-files -u two &&
    ++		test_line_count_cmd --out = 1 git ls-files -u original &&
    ++		test_line_count_cmd --out = 2 git ls-files -o
      	fi &&
      
      	test_path_is_file one/file &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'check handling of differently rena
     -	test 1 -eq "$(git ls-files -u two | wc -l)" &&
     -	test 1 -eq "$(git ls-files -u original | wc -l)" &&
     -	test 0 -eq "$(git ls-files -o | wc -l)" &&
    -+	test_line_count_cmd --out = 3 git ls-files -u  &&
    -+	test_line_count_cmd --out = 1 git ls-files -u one  &&
    -+	test_line_count_cmd --out = 1 git ls-files -u two  &&
    -+	test_line_count_cmd --out = 1 git ls-files -u original  &&
    -+	test_line_count_cmd --out = 2 git ls-files -o  &&
    ++	test_line_count_cmd --out = 3 git ls-files -u &&
    ++	test_line_count_cmd --out = 1 git ls-files -u one &&
    ++	test_line_count_cmd --out = 1 git ls-files -u two &&
    ++	test_line_count_cmd --out = 1 git ls-files -u original &&
    ++	test_line_count_cmd --out = 0 git ls-files -o &&
      
      	test_path_is_file one &&
      	test_path_is_file two &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'setup merge of rename + small chan
      
     -	test 1 -eq $(git ls-files -s | wc -l) &&
     -	test 0 -eq $(git ls-files -o | wc -l) &&
    -+	test_line_count_cmd --out = 1 git ls-files -s  &&
    -+	test_line_count_cmd --out = 2 git ls-files -o  &&
    ++	test_line_count_cmd --out = 1 git ls-files -s &&
    ++	test_line_count_cmd --out = 0 git ls-files -o &&
      	test $(git rev-parse HEAD:renamed_file) = $(git rev-parse HEAD~1:file)
      '
      
-- 
2.32.0.278.gd42b80f139

[PATCH v2 1/5] test-lib-functions: introduce test_line_count_cmd

[Đoàn Trần Công Danh]

In Git project, we have multiple occasions that requires checking number
of lines of text in stdout and/or stderr of a command. One of such
example is t6400, which 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 check for line count, thus loosing the exit status.

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

This helper will create 2 temporary files in process, thus it may affect
output of some checks.

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

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index f0448daa74..b3281409de 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -845,6 +845,123 @@ test_line_count () {
 	fi
 }
 
+# test_line_count_cmd checks exit status, and the number of lines in
+# captured stdout and/or stderr of a command.
+#
+# Usage:
+#
+# test_line_count_cmd [--[out|err] <binop> <value>]... [--] [!] cmd [args...]
+#
+# Options:
+# --out <binop> <value>:
+# --err <binop> <value>:
+#	Run sh's "test <# of lines> <binop> <value>" on # of lines in stdout
+#	(for --out) or stderr (for --err)
+# !:
+#	Instead of expecting "cmd [args...]" succeed, expect its failure.
+#	Note, if command under testing is "git", test_must_fail should be used
+#	instead of "!".
+#
+# Example:
+#	test_line_count_cmd --out -ge 10 --err = 0 git tag --no-contains v1.0.0
+#	test_line_count_cmd --out -le 10 ! grep some-text a-file
+#	test_line_count_cmd --out = 0 test_must_fail git rev-parse --verify abcd1234
+#
+# NOTE:
+# * if "--out" is specified, a temporary file named test_line_count_cmd_.out
+#   will be created.
+# * if "--err" is specified, a temporary file named test_line_count_cmd_.err
+#   will be created.
+# Those temporary files will be created under $TRASH_DIRECTORY/.git/trash
+# if $TRASH_DIRECTORY/.git directory existed.
+# Otherwise, they will be created in $TRASH_DIRECTORY.
+# Those temporary files will be cleant by test_when_finished
+test_line_count_cmd () {
+	{
+		local outop outval outfile
+		local errop errval errfile
+		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 &&
+		while test $# != 0
+		do
+			case "$1" in
+			--out)
+				outop="$2" &&
+				outval="$3" &&
+				outfile="$trashdir/test_line_count_cmd_.out" &&
+				shift 3
+				;;
+			--err)
+				errop="$2" &&
+				errval="$3" &&
+				errfile="$trashdir/test_line_count_cmd_.err" &&
+				shift 3
+				;;
+			--)
+				shift &&
+				break
+				;;
+			-*)
+				BUG "test_line_count_cmd: unknown options: '$1'"
+				;;
+			*)
+				break
+				;;
+			esac
+		done &&
+		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"
+		elif test -z "$outop$errop"
+		then
+			BUG "test_line_count_cmd: check which stream?"
+		else
+			if test -n "$outfile"
+			then
+				test_when_finished "rm -f '$outfile'" &&
+				exec 8>"$outfile"
+			fi &&
+			if test -n "$errfile"
+			then
+				test_when_finished "rm -f '$errfile'" &&
+				exec 9>"$errfile"
+			fi &&
+			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 &&
+		if test -n "$outop"
+		then
+			test_line_count "$outop" "$outval" "$outfile" >&4
+		fi &&
+		if test -n "$errop"
+		then
+			test_line_count "$errop" "$errval" "$errfile" >&4
+		fi
+	} 9>&2 2>&4
+}
+
 test_file_size () {
 	test "$#" -ne 1 && BUG "1 param"
 	test-tool path-utils file-size "$1"
-- 
2.32.0.278.gd42b80f139

[PATCH v2 2/5] t6402: use find(1) builtin to filter instead of grep

[Đoàn Trần Công Danh]

find(1) has a builtin (-prune) to filter its output, save a bit of time
for invoking grep(1).

In addition, in a later change, we will try to use test_line_count_cmd
to count number of lines in stdout and/or stderr of a command, due to
limitation of current implementation, it can handle pipe.

Let's replace grep(1)'s usage with find(1) builtin filter.

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 t/t6402-merge-rename.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh
index 425dad97d5..5d76cd6414 100755
--- a/t/t6402-merge-rename.sh
+++ b/t/t6402-merge-rename.sh
@@ -546,7 +546,7 @@ then
 
 		test_must_fail git diff --quiet &&
 
-		test 3 -eq $(find . | grep -v .git | wc -l) &&
+		test 3 -eq $(find . -name .git -prune -o -print | wc -l) &&
 
 		test_path_is_file one &&
 		test_path_is_file two &&
@@ -565,7 +565,7 @@ else
 
 		test_must_fail git diff --quiet &&
 
-		test 4 -eq $(find . | grep -v .git | wc -l) &&
+		test 4 -eq $(find . -name .git -prune -o -print | wc -l) &&
 
 		test_path_is_dir one &&
 		test_path_is_file one~rename-two &&
@@ -593,7 +593,7 @@ test_expect_success 'pair rename to parent of other (D/F conflicts) w/ clean sta
 
 	test_must_fail git diff --quiet &&
 
-	test 3 -eq $(find . | grep -v .git | wc -l) &&
+	test 3 -eq $(find . -name .git -prune -o -print | wc -l) &&
 
 	test_path_is_file one &&
 	test_path_is_file two &&
-- 
2.32.0.278.gd42b80f139

[PATCH v2 3/5] t0041: use test_line_count_cmd to check std{out,err}

[Đoàn Trần Công Danh]

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 t/t0041-usage.sh | 53 ++++++++++++++++--------------------------------
 1 file changed, 17 insertions(+), 36 deletions(-)

diff --git a/t/t0041-usage.sh b/t/t0041-usage.sh
index c4fc34eb18..24e24f698c 100755
--- a/t/t0041-usage.sh
+++ b/t/t0041-usage.sh
@@ -12,98 +12,79 @@ test_expect_success 'setup ' '
 '
 
 test_expect_success 'tag --contains <existent_tag>' '
-	git tag --contains "v1.0" >actual 2>actual.err &&
-	grep "v1.0" actual &&
-	test_line_count = 0 actual.err
+	test_line_count_cmd --err = 0 git tag --contains v1.0 >actual &&
+	grep "v1.0" actual
 '
 
 test_expect_success 'tag --contains <inexistent_tag>' '
-	test_must_fail git tag --contains "notag" >actual 2>actual.err &&
-	test_line_count = 0 actual &&
+	test_line_count_cmd --out = 0 test_must_fail git tag --contains notag 2>actual.err &&
 	test_i18ngrep "error" actual.err &&
 	test_i18ngrep ! "usage" actual.err
 '
 
 test_expect_success 'tag --no-contains <existent_tag>' '
-	git tag --no-contains "v1.0" >actual 2>actual.err  &&
-	test_line_count = 0 actual &&
-	test_line_count = 0 actual.err
+	test_line_count_cmd --out = 0 --err = 0 git tag --no-contains v1.0
 '
 
 test_expect_success 'tag --no-contains <inexistent_tag>' '
-	test_must_fail git tag --no-contains "notag" >actual 2>actual.err &&
-	test_line_count = 0 actual &&
+	test_line_count_cmd --out = 0 test_must_fail git tag --no-contains notag 2>actual.err &&
 	test_i18ngrep "error" actual.err &&
 	test_i18ngrep ! "usage" actual.err
 '
 
 test_expect_success 'tag usage error' '
-	test_must_fail git tag --noopt >actual 2>actual.err &&
-	test_line_count = 0 actual &&
+	test_line_count_cmd --out = 0 test_must_fail git tag --noopt 2>actual.err &&
 	test_i18ngrep "usage" actual.err
 '
 
 test_expect_success 'branch --contains <existent_commit>' '
-	git branch --contains "main" >actual 2>actual.err &&
-	test_i18ngrep "main" actual &&
-	test_line_count = 0 actual.err
+	test_line_count_cmd --err = 0 git branch --contains main >actual &&
+	test_i18ngrep "main" actual
 '
 
 test_expect_success 'branch --contains <inexistent_commit>' '
-	test_must_fail git branch --no-contains "nocommit" >actual 2>actual.err &&
-	test_line_count = 0 actual &&
+	test_line_count_cmd --out = 0 test_must_fail git branch --no-contains "nocommit" 2>actual.err &&
 	test_i18ngrep "error" actual.err &&
 	test_i18ngrep ! "usage" actual.err
 '
 
 test_expect_success 'branch --no-contains <existent_commit>' '
-	git branch --no-contains "main" >actual 2>actual.err &&
-	test_line_count = 0 actual &&
-	test_line_count = 0 actual.err
+	test_line_count_cmd --out = 0 --err = 0 git branch --no-contains main
 '
 
 test_expect_success 'branch --no-contains <inexistent_commit>' '
-	test_must_fail git branch --no-contains "nocommit" >actual 2>actual.err &&
-	test_line_count = 0 actual &&
+	test_line_count_cmd --out = 0 test_must_fail git branch --no-contains "nocommit" 2>actual.err &&
 	test_i18ngrep "error" actual.err &&
 	test_i18ngrep ! "usage" actual.err
 '
 
 test_expect_success 'branch usage error' '
-	test_must_fail git branch --noopt >actual 2>actual.err &&
-	test_line_count = 0 actual &&
+	test_line_count_cmd --out = 0 test_must_fail git branch --noopt 2>actual.err &&
 	test_i18ngrep "usage" actual.err
 '
 
 test_expect_success 'for-each-ref --contains <existent_object>' '
-	git for-each-ref --contains "main" >actual 2>actual.err &&
-	test_line_count = 2 actual &&
-	test_line_count = 0 actual.err
+	test_line_count_cmd --out = 2 --err = 0 git for-each-ref --contains main
 '
 
 test_expect_success 'for-each-ref --contains <inexistent_object>' '
-	test_must_fail git for-each-ref --no-contains "noobject" >actual 2>actual.err &&
-	test_line_count = 0 actual &&
+	test_line_count_cmd --out = 0 test_must_fail git for-each-ref --no-contains "noobject" 2>actual.err &&
 	test_i18ngrep "error" actual.err &&
 	test_i18ngrep ! "usage" actual.err
 '
 
 test_expect_success 'for-each-ref --no-contains <existent_object>' '
-	git for-each-ref --no-contains "main" >actual 2>actual.err &&
-	test_line_count = 0 actual &&
-	test_line_count = 0 actual.err
+	test_line_count_cmd --out = 0 --err = 0 git for-each-ref --no-contains "main"
 '
 
 test_expect_success 'for-each-ref --no-contains <inexistent_object>' '
-	test_must_fail git for-each-ref --no-contains "noobject" >actual 2>actual.err &&
-	test_line_count = 0 actual &&
+	test_line_count_cmd --out = 0 test_must_fail git for-each-ref --no-contains "noobject" 2>actual.err &&
 	test_i18ngrep "error" actual.err &&
 	test_i18ngrep ! "usage" actual.err
 '
 
 test_expect_success 'for-each-ref usage error' '
-	test_must_fail git for-each-ref --noopt >actual 2>actual.err &&
-	test_line_count = 0 actual &&
+	test_line_count_cmd --out = 0 test_must_fail git for-each-ref --noopt 2>actual.err &&
 	test_i18ngrep "usage" actual.err
 '
 
-- 
2.32.0.278.gd42b80f139

[PATCH v2 4/5] t6400: use test_line_count_cmd to count # of lines in stdout

[Đoàn Trần Công Danh]

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 t/t6400-merge-df.sh | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/t/t6400-merge-df.sh b/t/t6400-merge-df.sh
index 38700d29b5..8cf3603a24 100755
--- a/t/t6400-merge-df.sh
+++ b/t/t6400-merge-df.sh
@@ -82,13 +82,13 @@ test_expect_success 'modify/delete + directory/file conflict' '
 	git checkout delete^0 &&
 	test_must_fail git merge modify &&
 
-	test 5 -eq $(git ls-files -s | wc -l) &&
-	test 4 -eq $(git ls-files -u | wc -l) &&
+	test_line_count_cmd --out = 5 git ls-files -s &&
+	test_line_count_cmd --out = 4 git ls-files -u &&
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 0 -eq $(git ls-files -o | wc -l)
+		test_line_count_cmd --out = 0 git ls-files -o
 	else
-		test 1 -eq $(git ls-files -o | wc -l)
+		test_line_count_cmd --out = 1 git ls-files -o
 	fi &&
 
 	test_path_is_file letters/file &&
@@ -103,13 +103,13 @@ test_expect_success 'modify/delete + directory/file conflict; other way' '
 
 	test_must_fail git merge delete &&
 
-	test 5 -eq $(git ls-files -s | wc -l) &&
-	test 4 -eq $(git ls-files -u | wc -l) &&
+	test_line_count_cmd --out = 5 git ls-files -s &&
+	test_line_count_cmd --out = 4 git ls-files -u &&
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 0 -eq $(git ls-files -o | wc -l)
+		test_line_count_cmd --out = 0 git ls-files -o
 	else
-		test 1 -eq $(git ls-files -o | wc -l)
+		test_line_count_cmd --out = 1 git ls-files -o
 	fi &&
 
 	test_path_is_file letters/file &&
-- 
2.32.0.278.gd42b80f139

[PATCH v2 5/5] t6402: use test_line_count_cmd to count # of lines in stdout

[Đoàn Trần Công Danh]

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 t/t6402-merge-rename.sh | 126 +++++++++++++++++++---------------------
 1 file changed, 59 insertions(+), 67 deletions(-)

diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh
index 5d76cd6414..ca84588eb5 100755
--- a/t/t6402-merge-rename.sh
+++ b/t/t6402-merge-rename.sh
@@ -105,10 +105,8 @@ test_expect_success 'pull renaming branch into unrenaming one' \
 	git show-branch &&
 	test_expect_code 1 git pull . white &&
 	git ls-files -s &&
-	git ls-files -u B >b.stages &&
-	test_line_count = 3 b.stages &&
-	git ls-files -s N >n.stages &&
-	test_line_count = 1 n.stages &&
+	test_line_count_cmd --out = 3 git ls-files -u B &&
+	test_line_count_cmd --out = 1 git ls-files -s N &&
 	sed -ne "/^g/{
 	p
 	q
@@ -122,10 +120,8 @@ test_expect_success 'pull renaming branch into another renaming one' \
 	git reset --hard &&
 	git checkout red &&
 	test_expect_code 1 git pull . white &&
-	git ls-files -u B >b.stages &&
-	test_line_count = 3 b.stages &&
-	git ls-files -s N >n.stages &&
-	test_line_count = 1 n.stages &&
+	test_line_count_cmd --out = 3 git ls-files -u B &&
+	test_line_count_cmd --out = 1 git ls-files -s N &&
 	sed -ne "/^g/{
 	p
 	q
@@ -138,10 +134,8 @@ test_expect_success 'pull unrenaming branch into renaming one' \
 	git reset --hard &&
 	git show-branch &&
 	test_expect_code 1 git pull . main &&
-	git ls-files -u B >b.stages &&
-	test_line_count = 3 b.stages &&
-	git ls-files -s N >n.stages &&
-	test_line_count = 1 n.stages &&
+	test_line_count_cmd --out = 3 git ls-files -u B &&
+	test_line_count_cmd --out = 1 git ls-files -s N &&
 	sed -ne "/^g/{
 	p
 	q
@@ -154,14 +148,10 @@ test_expect_success 'pull conflicting renames' \
 	git reset --hard &&
 	git show-branch &&
 	test_expect_code 1 git pull . blue &&
-	git ls-files -u A >a.stages &&
-	test_line_count = 1 a.stages &&
-	git ls-files -u B >b.stages &&
-	test_line_count = 1 b.stages &&
-	git ls-files -u C >c.stages &&
-	test_line_count = 1 c.stages &&
-	git ls-files -s N >n.stages &&
-	test_line_count = 1 n.stages &&
+	test_line_count_cmd --out = 1 git ls-files -u A &&
+	test_line_count_cmd --out = 1 git ls-files -u B &&
+	test_line_count_cmd --out = 1 git ls-files -u C &&
+	test_line_count_cmd --out = 1 git ls-files -s N &&
 	sed -ne "/^g/{
 	p
 	q
@@ -330,8 +320,8 @@ test_expect_success 'Rename+D/F conflict; renamed file merges but dir in way' '
 		test_i18ngrep "Adding as dir~HEAD instead" output
 	fi &&
 
-	test 3 -eq "$(git ls-files -u | wc -l)" &&
-	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
+	test_line_count_cmd --out = 3 git ls-files -u &&
+	test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way &&
 
 	test_must_fail git diff --quiet &&
 	test_must_fail git diff --cached --quiet &&
@@ -357,8 +347,8 @@ test_expect_success 'Same as previous, but merged other way' '
 		test_i18ngrep "Adding as dir~renamed-file-has-no-conflicts instead" output
 	fi &&
 
-	test 3 -eq "$(git ls-files -u | wc -l)" &&
-	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
+	test_line_count_cmd --out = 3 git ls-files -u &&
+	test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way &&
 
 	test_must_fail git diff --quiet &&
 	test_must_fail git diff --cached --quiet &&
@@ -374,8 +364,8 @@ test_expect_success 'Rename+D/F conflict; renamed file cannot merge, dir not in
 	git checkout -q renamed-file-has-conflicts^0 &&
 	test_must_fail git merge --strategy=recursive dir-not-in-way &&
 
-	test 3 -eq "$(git ls-files -u | wc -l)" &&
-	test 3 -eq "$(git ls-files -u dir | wc -l)" &&
+	test_line_count_cmd --out = 3 git ls-files -u &&
+	test_line_count_cmd --out = 3 git ls-files -u dir &&
 
 	test_must_fail git diff --quiet &&
 	test_must_fail git diff --cached --quiet &&
@@ -409,14 +399,15 @@ test_expect_success 'Rename+D/F conflict; renamed file cannot merge and dir in t
 	git checkout -q renamed-file-has-conflicts^0 &&
 	test_must_fail git merge --strategy=recursive dir-in-way &&
 
-	test 5 -eq "$(git ls-files -u | wc -l)" &&
+	test_line_count_cmd --out = 5 git ls-files -u &&
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 3 -eq "$(git ls-files -u dir~HEAD | wc -l)"
+		test_line_count_cmd --out = 3 git ls-files -u dir~HEAD
 	else
-		test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)"
+		git ls-files -u dir >out &&
+		test_line_count_cmd --out = 3 grep -v file-in-the-way out
 	fi &&
-	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
+	test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way &&
 
 	test_must_fail git diff --quiet &&
 	test_must_fail git diff --cached --quiet &&
@@ -432,14 +423,15 @@ test_expect_success 'Same as previous, but merged other way' '
 	git checkout -q dir-in-way^0 &&
 	test_must_fail git merge --strategy=recursive renamed-file-has-conflicts &&
 
-	test 5 -eq "$(git ls-files -u | wc -l)" &&
+	test_line_count_cmd --out = 5 git ls-files -u &&
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 3 -eq "$(git ls-files -u dir~renamed-file-has-conflicts | wc -l)"
+		test_line_count_cmd --out = 3 git ls-files -u dir~renamed-file-has-conflicts
 	else
-		test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)"
+		git ls-files -u dir >out &&
+		test_line_count_cmd --out = 3 grep -v file-in-the-way out
 	fi &&
-	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
+	test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way &&
 
 	test_must_fail git diff --quiet &&
 	test_must_fail git diff --cached --quiet &&
@@ -496,9 +488,9 @@ test_expect_success 'both rename source and destination involved in D/F conflict
 
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 2 -eq "$(git ls-files -u | wc -l)"
+		test_line_count_cmd --out = 2 git ls-files -u
 	else
-		test 1 -eq "$(git ls-files -u | wc -l)"
+		test_line_count_cmd --out = 1 git ls-files -u
 	fi &&
 
 	test_must_fail git diff --quiet &&
@@ -540,9 +532,9 @@ then
 		mkdir one &&
 		test_must_fail git merge --strategy=recursive rename-two &&
 
-		test 4 -eq "$(git ls-files -u | wc -l)" &&
-		test 2 -eq "$(git ls-files -u one | wc -l)" &&
-		test 2 -eq "$(git ls-files -u two | wc -l)" &&
+		test_line_count_cmd --out = 4 git ls-files -u &&
+		test_line_count_cmd --out = 2 git ls-files -u one &&
+		test_line_count_cmd --out = 2 git ls-files -u two &&
 
 		test_must_fail git diff --quiet &&
 
@@ -559,9 +551,9 @@ else
 		mkdir one &&
 		test_must_fail git merge --strategy=recursive rename-two &&
 
-		test 2 -eq "$(git ls-files -u | wc -l)" &&
-		test 1 -eq "$(git ls-files -u one | wc -l)" &&
-		test 1 -eq "$(git ls-files -u two | wc -l)" &&
+		test_line_count_cmd --out = 2 git ls-files -u &&
+		test_line_count_cmd --out = 1 git ls-files -u one &&
+		test_line_count_cmd --out = 1 git ls-files -u two &&
 
 		test_must_fail git diff --quiet &&
 
@@ -582,13 +574,13 @@ test_expect_success 'pair rename to parent of other (D/F conflicts) w/ clean sta
 
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 4 -eq "$(git ls-files -u | wc -l)" &&
-		test 2 -eq "$(git ls-files -u one | wc -l)" &&
-		test 2 -eq "$(git ls-files -u two | wc -l)"
+		test_line_count_cmd --out = 4 git ls-files -u &&
+		test_line_count_cmd --out = 2 git ls-files -u one &&
+		test_line_count_cmd --out = 2 git ls-files -u two
 	else
-		test 2 -eq "$(git ls-files -u | wc -l)" &&
-		test 1 -eq "$(git ls-files -u one | wc -l)" &&
-		test 1 -eq "$(git ls-files -u two | wc -l)"
+		test_line_count_cmd --out = 2 git ls-files -u &&
+		test_line_count_cmd --out = 1 git ls-files -u one &&
+		test_line_count_cmd --out = 1 git ls-files -u two
 	fi &&
 
 	test_must_fail git diff --quiet &&
@@ -631,19 +623,19 @@ test_expect_success 'check handling of differently renamed file with D/F conflic
 
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 5 -eq "$(git ls-files -s | wc -l)" &&
-		test 3 -eq "$(git ls-files -u | wc -l)" &&
-		test 1 -eq "$(git ls-files -u one~HEAD | wc -l)" &&
-		test 1 -eq "$(git ls-files -u two~second-rename | wc -l)" &&
-		test 1 -eq "$(git ls-files -u original | wc -l)" &&
-		test 0 -eq "$(git ls-files -o | wc -l)"
+		test_line_count_cmd --out = 5 git ls-files -s &&
+		test_line_count_cmd --out = 3 git ls-files -u &&
+		test_line_count_cmd --out = 1 git ls-files -u one~HEAD &&
+		test_line_count_cmd --out = 1 git ls-files -u two~second-rename &&
+		test_line_count_cmd --out = 1 git ls-files -u original &&
+		test_line_count_cmd --out = 0 git ls-files -o
 	else
-		test 5 -eq "$(git ls-files -s | wc -l)" &&
-		test 3 -eq "$(git ls-files -u | wc -l)" &&
-		test 1 -eq "$(git ls-files -u one | wc -l)" &&
-		test 1 -eq "$(git ls-files -u two | wc -l)" &&
-		test 1 -eq "$(git ls-files -u original | wc -l)" &&
-		test 2 -eq "$(git ls-files -o | wc -l)"
+		test_line_count_cmd --out = 5 git ls-files -s &&
+		test_line_count_cmd --out = 3 git ls-files -u &&
+		test_line_count_cmd --out = 1 git ls-files -u one &&
+		test_line_count_cmd --out = 1 git ls-files -u two &&
+		test_line_count_cmd --out = 1 git ls-files -u original &&
+		test_line_count_cmd --out = 2 git ls-files -o
 	fi &&
 
 	test_path_is_file one/file &&
@@ -679,11 +671,11 @@ test_expect_success 'check handling of differently renamed file with D/F conflic
 	git checkout -q first-rename-redo^0 &&
 	test_must_fail git merge --strategy=recursive second-rename-redo &&
 
-	test 3 -eq "$(git ls-files -u | wc -l)" &&
-	test 1 -eq "$(git ls-files -u one | wc -l)" &&
-	test 1 -eq "$(git ls-files -u two | wc -l)" &&
-	test 1 -eq "$(git ls-files -u original | wc -l)" &&
-	test 0 -eq "$(git ls-files -o | wc -l)" &&
+	test_line_count_cmd --out = 3 git ls-files -u &&
+	test_line_count_cmd --out = 1 git ls-files -u one &&
+	test_line_count_cmd --out = 1 git ls-files -u two &&
+	test_line_count_cmd --out = 1 git ls-files -u original &&
+	test_line_count_cmd --out = 0 git ls-files -o &&
 
 	test_path_is_file one &&
 	test_path_is_file two &&
@@ -861,8 +853,8 @@ test_expect_success 'setup merge of rename + small change' '
 test_expect_success 'merge rename + small change' '
 	git merge rename_branch &&
 
-	test 1 -eq $(git ls-files -s | wc -l) &&
-	test 0 -eq $(git ls-files -o | wc -l) &&
+	test_line_count_cmd --out = 1 git ls-files -s &&
+	test_line_count_cmd --out = 0 git ls-files -o &&
 	test $(git rev-parse HEAD:renamed_file) = $(git rev-parse HEAD~1:file)
 '
 
-- 
2.32.0.278.gd42b80f139

Re: [PATCH v2 3/5] t0041: use test_line_count_cmd to check std{out,err}

[Junio C Hamano]

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

>  test_expect_success 'tag --contains <existent_tag>' '
> -	git tag --contains "v1.0" >actual 2>actual.err &&
> -	grep "v1.0" actual &&
> -	test_line_count = 0 actual.err
> +	test_line_count_cmd --err = 0 git tag --contains v1.0 >actual &&
> +	grep "v1.0" actual

Sorry, but I am not impressed if this is a typical/prime example of
how the new helper helps in writing our tests.

Notice that so many tests that you touched only care about 0 lines?

Instead of this new helper, I think it would be a more useful
improvement if we check the emptyness in a more direct way, i.e.

>  test_expect_success 'tag --contains <existent_tag>' '
> 	git tag --contains "v1.0" >actual 2>actual.err &&
> 	grep "v1.0" actual &&
> -	test_line_count = 0 actual.err
> +	test_must_be_empty actual.err

I think this helper may be misnamed and test_file_is_empty would sit
better with test_dir_is_empty and test_file_not_empty that already
exist, though.

By the way, my opinion would be quite different if example like this
one ...

>  test_expect_success 'tag --no-contains <existent_tag>' '
> -	git tag --no-contains "v1.0" >actual 2>actual.err  &&
> -	test_line_count = 0 actual &&
> -	test_line_count = 0 actual.err
> +	test_line_count_cmd --out = 0 --err = 0 git tag --no-contains v1.0
>  '

... were the majority, but I do not think that is the case.  Most
tests that employ the new test_line_count_cmd in this patch still
create either actual or actual.err in the working tree anyway, so I
do not see much point in adding this new helper---it is hard to
explain to new test writers when to use it.

Re: [PATCH v2 3/5] t0041: use test_line_count_cmd to check std{out,err}

[Đoàn Trần Công Danh]

On 2021-06-16 12:06:14+0900, Junio C Hamano <gitster@pobox.com> wrote:
> Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:
> 
> >  test_expect_success 'tag --contains <existent_tag>' '
> > -	git tag --contains "v1.0" >actual 2>actual.err &&
> > -	grep "v1.0" actual &&
> > -	test_line_count = 0 actual.err
> > +	test_line_count_cmd --err = 0 git tag --contains v1.0 >actual &&
> > +	grep "v1.0" actual
> 
> Sorry, but I am not impressed if this is a typical/prime example of
> how the new helper helps in writing our tests.

Yay, reading through the patch again, and I'm become less enthusiastic
with persuading --err. The only useful application of --err is t4068.
----8<---
diff --git a/t/t4068-diff-symmetric-merge-base.sh b/t/t4068-diff-symmetric-merge-base.sh
index 2d650d8f10..33e2327072 100755
--- a/t/t4068-diff-symmetric-merge-base.sh
+++ b/t/t4068-diff-symmetric-merge-base.sh
@@ -61,9 +61,7 @@ test_expect_success 'diff with one merge base' '
 # It should have one of those two, which comes out
 # to seven lines.
 test_expect_success 'diff with two merge bases' '
-	git diff br1...main >tmp 2>err &&
-	test_line_count = 7 tmp &&
-	test_line_count = 1 err
+	test_line_count_cmd --out = 7 --err = 1 git diff br1...main
 '
 
 test_expect_success 'diff with no merge bases' '
----->8----

However, going through all the trouble for a single application is not
really worth it.  I'm going to drop --err, remove --out option, too.
So, its prototype would be:

	test_line_count_cmd <op> <val> [!] cmd [args...]

> Notice that so many tests that you touched only care about 0 lines?
> 
> Instead of this new helper, I think it would be a more useful
> improvement if we check the emptyness in a more direct way, i.e.
> 
> >  test_expect_success 'tag --contains <existent_tag>' '
> > 	git tag --contains "v1.0" >actual 2>actual.err &&
> > 	grep "v1.0" actual &&
> > -	test_line_count = 0 actual.err
> > +	test_must_be_empty actual.err
> 
> I think this helper may be misnamed and test_file_is_empty would sit
> better with test_dir_is_empty and test_file_not_empty that already
> exist, though.

That would be an improvement, but it should be written in a different
series.

> 
> By the way, my opinion would be quite different if example like this
> one ...
> 
> >  test_expect_success 'tag --no-contains <existent_tag>' '
> > -	git tag --no-contains "v1.0" >actual 2>actual.err  &&
> > -	test_line_count = 0 actual &&
> > -	test_line_count = 0 actual.err
> > +	test_line_count_cmd --out = 0 --err = 0 git tag --no-contains v1.0
> >  '
> 
> ... were the majority, but I do not think that is the case.  Most
> tests that employ the new test_line_count_cmd in this patch still
> create either actual or actual.err in the working tree anyway, so I
> do not see much point in adding this new helper---it is hard to
> explain to new test writers when to use it.

I'm not sure if I get your opinion.  Did you mean you wouldn't take
whole helper? Or you meant you still wanted to see a new helper for
checking only stdout?  If it's the former, I'll send a different
series to only clean "git ls-files ... | wc -l" in t6400 and t6402,
if it's the latter, I'll rewrite without --err.

-- 
Danh

Re: [PATCH v2 3/5] t0041: use test_line_count_cmd to check std{out,err}

[Junio C Hamano]

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

>> By the way, my opinion would be quite different if example like this
>> one ...
>> 
>> >  test_expect_success 'tag --no-contains <existent_tag>' '
>> > -	git tag --no-contains "v1.0" >actual 2>actual.err  &&
>> > -	test_line_count = 0 actual &&
>> > -	test_line_count = 0 actual.err
>> > +	test_line_count_cmd --out = 0 --err = 0 git tag --no-contains v1.0
>> >  '
>> 
>> ... were the majority, but I do not think that is the case.  Most
>> tests that employ the new test_line_count_cmd in this patch still
>> create either actual or actual.err in the working tree anyway, so I
>> do not see much point in adding this new helper---it is hard to
>> explain to new test writers when to use it.
>
> I'm not sure if I get your opinion.  Did you mean you wouldn't take
> whole helper? Or you meant you still wanted to see a new helper for
> checking only stdout?  If it's the former, I'll send a different
> series to only clean "git ls-files ... | wc -l" in t6400 and t6402,
> if it's the latter, I'll rewrite without --err.

I did not see much point in adding test_line_count_cmd with --out
and/or --err options; the upside of having it was dubious after
looking at the users of it in the patch that we are discussing.

I did not consider test_line_count_cmd that only works on the
standard output stream.  From the patch under discussion, it is not
immediately obvious how much such a simplified helper would help
clean up the existing tests, so I have no opinion without seeing
at least some sample uses.

RE: [PATCH v2 1/5] test-lib-functions: introduce test_line_count_cmd

[Felipe Contreras]

Đoàn Trần Công Danh wrote:
> In Git project,

In _the_ Git project

> we have multiple occasions that requires checking number

multiple instances that require checking the number

> of lines of text in stdout and/or stderr of a command. One of such
> example is t6400,

One of such examples

> which checks number of files in various states.

that checks the number of files (see 'that' vs. 'which' [1]).

> Some of those commands are Git command,

are git commands,

> 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 check for line count,

for a line count,

> thus loosing the exit status.

losing

> Introduce a helper function to check for number of lines in stdout and

the number of lines

> stderr from those commands.
> 
> This helper will create 2 temporary files in process,

the process

> thus it may affect output of some checks.

the output

> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> ---
>  t/test-lib-functions.sh | 117 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 117 insertions(+)
> 
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index f0448daa74..b3281409de 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -845,6 +845,123 @@ test_line_count () {
>  	fi
>  }
>  
> +# test_line_count_cmd checks exit status,

the exit status

> and the number of lines in

> +# captured stdout and/or stderr of a command.

the captured stdout...

> +#
> +# Usage:
> +#
> +# test_line_count_cmd [--[out|err] <binop> <value>]... [--] [!] cmd [args...]
> +#
> +# Options:
> +# --out <binop> <value>:
> +# --err <binop> <value>:
> +#	Run sh's "test <# of lines> <binop> <value>" on # of lines in stdout
> +#	(for --out) or stderr (for --err)
> +# !:
> +#	Instead of expecting "cmd [args...]" succeed, expect its failure.
> +#	Note, if command under testing is "git",

the command under...

> test_must_fail should be used

> +#	instead of "!".
> +#
> +# Example:
> +#	test_line_count_cmd --out -ge 10 --err = 0 git tag --no-contains v1.0.0
> +#	test_line_count_cmd --out -le 10 ! grep some-text a-file
> +#	test_line_count_cmd --out = 0 test_must_fail git rev-parse --verify abcd1234
> +#
> +# NOTE:
> +# * if "--out" is specified, a temporary file named test_line_count_cmd_.out
> +#   will be created.
> +# * if "--err" is specified, a temporary file named test_line_count_cmd_.err
> +#   will be created.
> +# Those temporary files will be created under $TRASH_DIRECTORY/.git/trash
> +# if $TRASH_DIRECTORY/.git directory existed.

if $TRASH_DIRECTORY/.git/ exists

> +# Otherwise, they will be created in $TRASH_DIRECTORY.
> +# Those temporary files will be cleant by test_when_finished

will be cleaned by ...

Cheers.

[1] https://www.merriam-webster.com/words-at-play/when-to-use-that-and-which

-- 
Felipe Contreras

[PATCH v3 0/4] t: new helper test_line_count_cmd

[Đoàn Trần Công Danh]

This is a series to clear false positive when applying Junio's suggestion to
to a series written by Ævar [1].

Change in v3 since v2:
* --err was dropped entirely
* --out is not an option anymore, <binops> and <value> is the first two
  arguments that fed into test_line_count_cmd

Change in v2 since v1:
* Documentation for test_line_count_cmd has been written in more detail
  with examples
* The outfile and errfile will be created only if --out and/or --err was
  specified for better "-v"
* outfile and errfile will be created in $TRASH_DIRECTORY/.git/trash
  iff $TRASH_DIRECTORY/.git is a directory, otherwise $TRASH_DIRECTORY,
  avoid "git rev-parse --git-dir" because we may want to test it, too.
* Use test_when_finished to clean those files instead of manual "rm -f",
  also for better "-v"
* Merge multiple instance of "$@" run into one, for better auditing
* t0041 is also converted to use new helper
* With the change to location of outfile and errfile,
  output of "git ls-files -o" has been restored.
* Fix double space before "&&" in the end of test command.

1: https://lore.kernel.org/git/87r1j42ffz.fsf@evledraar.gmail.com/


Đoàn Trần Công Danh (4):
  test-lib-functions: introduce test_line_count_cmd
  t6402: use find(1) builtin to filter instead of grep
  t6400: use test_line_count_cmd to count # of lines in stdout
  t6402: use test_line_count_cmd to count # of lines in stdout

 t/t6400-merge-df.sh     |  16 ++---
 t/t6402-merge-rename.sh | 132 +++++++++++++++++++---------------------
 t/test-lib-functions.sh |  80 ++++++++++++++++++++++++
 3 files changed, 150 insertions(+), 78 deletions(-)

Range-diff against v2:
1:  a823312b19 < -:  ---------- test-lib-functions: introduce test_line_count_cmd
-:  ---------- > 1:  255ba9b067 test-lib-functions: introduce test_line_count_cmd
2:  6e8f2d4289 = 2:  38cd3f93a0 t6402: use find(1) builtin to filter instead of grep
3:  33daa5ee2f < -:  ---------- t0041: use test_line_count_cmd to check std{out,err}
4:  729ebb8f50 ! 3:  efb0905dd3 t6400: use test_line_count_cmd to count # of lines in stdout
    @@ t/t6400-merge-df.sh: test_expect_success 'modify/delete + directory/file conflic
      
     -	test 5 -eq $(git ls-files -s | wc -l) &&
     -	test 4 -eq $(git ls-files -u | wc -l) &&
    -+	test_line_count_cmd --out = 5 git ls-files -s &&
    -+	test_line_count_cmd --out = 4 git ls-files -u &&
    ++	test_line_count_cmd = 5 git ls-files -s &&
    ++	test_line_count_cmd = 4 git ls-files -u &&
      	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
      	then
     -		test 0 -eq $(git ls-files -o | wc -l)
    -+		test_line_count_cmd --out = 0 git ls-files -o
    ++		test_line_count_cmd = 0 git ls-files -o
      	else
     -		test 1 -eq $(git ls-files -o | wc -l)
    -+		test_line_count_cmd --out = 1 git ls-files -o
    ++		test_line_count_cmd = 1 git ls-files -o
      	fi &&
      
      	test_path_is_file letters/file &&
    @@ t/t6400-merge-df.sh: test_expect_success 'modify/delete + directory/file conflic
      
     -	test 5 -eq $(git ls-files -s | wc -l) &&
     -	test 4 -eq $(git ls-files -u | wc -l) &&
    -+	test_line_count_cmd --out = 5 git ls-files -s &&
    -+	test_line_count_cmd --out = 4 git ls-files -u &&
    ++	test_line_count_cmd = 5 git ls-files -s &&
    ++	test_line_count_cmd = 4 git ls-files -u &&
      	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
      	then
     -		test 0 -eq $(git ls-files -o | wc -l)
    -+		test_line_count_cmd --out = 0 git ls-files -o
    ++		test_line_count_cmd = 0 git ls-files -o
      	else
     -		test 1 -eq $(git ls-files -o | wc -l)
    -+		test_line_count_cmd --out = 1 git ls-files -o
    ++		test_line_count_cmd = 1 git ls-files -o
      	fi &&
      
      	test_path_is_file letters/file &&
5:  1b450e4148 ! 4:  8fc0f3ffd2 t6402: use test_line_count_cmd to count # of lines in stdout
    @@ t/t6402-merge-rename.sh: test_expect_success 'pull renaming branch into unrenami
     -	test_line_count = 3 b.stages &&
     -	git ls-files -s N >n.stages &&
     -	test_line_count = 1 n.stages &&
    -+	test_line_count_cmd --out = 3 git ls-files -u B &&
    -+	test_line_count_cmd --out = 1 git ls-files -s N &&
    ++	test_line_count_cmd = 3 git ls-files -u B &&
    ++	test_line_count_cmd = 1 git ls-files -s N &&
      	sed -ne "/^g/{
      	p
      	q
    @@ t/t6402-merge-rename.sh: test_expect_success 'pull renaming branch into another
     -	test_line_count = 3 b.stages &&
     -	git ls-files -s N >n.stages &&
     -	test_line_count = 1 n.stages &&
    -+	test_line_count_cmd --out = 3 git ls-files -u B &&
    -+	test_line_count_cmd --out = 1 git ls-files -s N &&
    ++	test_line_count_cmd = 3 git ls-files -u B &&
    ++	test_line_count_cmd = 1 git ls-files -s N &&
      	sed -ne "/^g/{
      	p
      	q
    @@ t/t6402-merge-rename.sh: test_expect_success 'pull unrenaming branch into renami
     -	test_line_count = 3 b.stages &&
     -	git ls-files -s N >n.stages &&
     -	test_line_count = 1 n.stages &&
    -+	test_line_count_cmd --out = 3 git ls-files -u B &&
    -+	test_line_count_cmd --out = 1 git ls-files -s N &&
    ++	test_line_count_cmd = 3 git ls-files -u B &&
    ++	test_line_count_cmd = 1 git ls-files -s N &&
      	sed -ne "/^g/{
      	p
      	q
    @@ t/t6402-merge-rename.sh: test_expect_success 'pull conflicting renames' \
     -	test_line_count = 1 c.stages &&
     -	git ls-files -s N >n.stages &&
     -	test_line_count = 1 n.stages &&
    -+	test_line_count_cmd --out = 1 git ls-files -u A &&
    -+	test_line_count_cmd --out = 1 git ls-files -u B &&
    -+	test_line_count_cmd --out = 1 git ls-files -u C &&
    -+	test_line_count_cmd --out = 1 git ls-files -s N &&
    ++	test_line_count_cmd = 1 git ls-files -u A &&
    ++	test_line_count_cmd = 1 git ls-files -u B &&
    ++	test_line_count_cmd = 1 git ls-files -u C &&
    ++	test_line_count_cmd = 1 git ls-files -s N &&
      	sed -ne "/^g/{
      	p
      	q
    @@ t/t6402-merge-rename.sh: test_expect_success 'Rename+D/F conflict; renamed file
      
     -	test 3 -eq "$(git ls-files -u | wc -l)" &&
     -	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
    -+	test_line_count_cmd --out = 3 git ls-files -u &&
    -+	test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way &&
    ++	test_line_count_cmd = 3 git ls-files -u &&
    ++	test_line_count_cmd = 2 git ls-files -u dir/file-in-the-way &&
      
      	test_must_fail git diff --quiet &&
      	test_must_fail git diff --cached --quiet &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'Same as previous, but merged other
      
     -	test 3 -eq "$(git ls-files -u | wc -l)" &&
     -	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
    -+	test_line_count_cmd --out = 3 git ls-files -u &&
    -+	test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way &&
    ++	test_line_count_cmd = 3 git ls-files -u &&
    ++	test_line_count_cmd = 2 git ls-files -u dir/file-in-the-way &&
      
      	test_must_fail git diff --quiet &&
      	test_must_fail git diff --cached --quiet &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'Rename+D/F conflict; renamed file
      
     -	test 3 -eq "$(git ls-files -u | wc -l)" &&
     -	test 3 -eq "$(git ls-files -u dir | wc -l)" &&
    -+	test_line_count_cmd --out = 3 git ls-files -u &&
    -+	test_line_count_cmd --out = 3 git ls-files -u dir &&
    ++	test_line_count_cmd = 3 git ls-files -u &&
    ++	test_line_count_cmd = 3 git ls-files -u dir &&
      
      	test_must_fail git diff --quiet &&
      	test_must_fail git diff --cached --quiet &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'Rename+D/F conflict; renamed file
      	test_must_fail git merge --strategy=recursive dir-in-way &&
      
     -	test 5 -eq "$(git ls-files -u | wc -l)" &&
    -+	test_line_count_cmd --out = 5 git ls-files -u &&
    ++	test_line_count_cmd = 5 git ls-files -u &&
      	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
      	then
     -		test 3 -eq "$(git ls-files -u dir~HEAD | wc -l)"
    -+		test_line_count_cmd --out = 3 git ls-files -u dir~HEAD
    ++		test_line_count_cmd = 3 git ls-files -u dir~HEAD
      	else
     -		test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)"
     +		git ls-files -u dir >out &&
    -+		test_line_count_cmd --out = 3 grep -v file-in-the-way out
    ++		test_line_count_cmd = 3 grep -v file-in-the-way out
      	fi &&
     -	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
    -+	test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way &&
    ++	test_line_count_cmd = 2 git ls-files -u dir/file-in-the-way &&
      
      	test_must_fail git diff --quiet &&
      	test_must_fail git diff --cached --quiet &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'Same as previous, but merged other
      	test_must_fail git merge --strategy=recursive renamed-file-has-conflicts &&
      
     -	test 5 -eq "$(git ls-files -u | wc -l)" &&
    -+	test_line_count_cmd --out = 5 git ls-files -u &&
    ++	test_line_count_cmd = 5 git ls-files -u &&
      	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
      	then
     -		test 3 -eq "$(git ls-files -u dir~renamed-file-has-conflicts | wc -l)"
    -+		test_line_count_cmd --out = 3 git ls-files -u dir~renamed-file-has-conflicts
    ++		test_line_count_cmd = 3 git ls-files -u dir~renamed-file-has-conflicts
      	else
     -		test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)"
     +		git ls-files -u dir >out &&
    -+		test_line_count_cmd --out = 3 grep -v file-in-the-way out
    ++		test_line_count_cmd = 3 grep -v file-in-the-way out
      	fi &&
     -	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
    -+	test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way &&
    ++	test_line_count_cmd = 2 git ls-files -u dir/file-in-the-way &&
      
      	test_must_fail git diff --quiet &&
      	test_must_fail git diff --cached --quiet &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'both rename source and destination
      	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
      	then
     -		test 2 -eq "$(git ls-files -u | wc -l)"
    -+		test_line_count_cmd --out = 2 git ls-files -u
    ++		test_line_count_cmd = 2 git ls-files -u
      	else
     -		test 1 -eq "$(git ls-files -u | wc -l)"
    -+		test_line_count_cmd --out = 1 git ls-files -u
    ++		test_line_count_cmd = 1 git ls-files -u
      	fi &&
      
      	test_must_fail git diff --quiet &&
    @@ t/t6402-merge-rename.sh: then
     -		test 4 -eq "$(git ls-files -u | wc -l)" &&
     -		test 2 -eq "$(git ls-files -u one | wc -l)" &&
     -		test 2 -eq "$(git ls-files -u two | wc -l)" &&
    -+		test_line_count_cmd --out = 4 git ls-files -u &&
    -+		test_line_count_cmd --out = 2 git ls-files -u one &&
    -+		test_line_count_cmd --out = 2 git ls-files -u two &&
    ++		test_line_count_cmd = 4 git ls-files -u &&
    ++		test_line_count_cmd = 2 git ls-files -u one &&
    ++		test_line_count_cmd = 2 git ls-files -u two &&
      
      		test_must_fail git diff --quiet &&
      
    @@ t/t6402-merge-rename.sh: else
     -		test 2 -eq "$(git ls-files -u | wc -l)" &&
     -		test 1 -eq "$(git ls-files -u one | wc -l)" &&
     -		test 1 -eq "$(git ls-files -u two | wc -l)" &&
    -+		test_line_count_cmd --out = 2 git ls-files -u &&
    -+		test_line_count_cmd --out = 1 git ls-files -u one &&
    -+		test_line_count_cmd --out = 1 git ls-files -u two &&
    ++		test_line_count_cmd = 2 git ls-files -u &&
    ++		test_line_count_cmd = 1 git ls-files -u one &&
    ++		test_line_count_cmd = 1 git ls-files -u two &&
      
      		test_must_fail git diff --quiet &&
      
    @@ t/t6402-merge-rename.sh: test_expect_success 'pair rename to parent of other (D/
     -		test 4 -eq "$(git ls-files -u | wc -l)" &&
     -		test 2 -eq "$(git ls-files -u one | wc -l)" &&
     -		test 2 -eq "$(git ls-files -u two | wc -l)"
    -+		test_line_count_cmd --out = 4 git ls-files -u &&
    -+		test_line_count_cmd --out = 2 git ls-files -u one &&
    -+		test_line_count_cmd --out = 2 git ls-files -u two
    ++		test_line_count_cmd = 4 git ls-files -u &&
    ++		test_line_count_cmd = 2 git ls-files -u one &&
    ++		test_line_count_cmd = 2 git ls-files -u two
      	else
     -		test 2 -eq "$(git ls-files -u | wc -l)" &&
     -		test 1 -eq "$(git ls-files -u one | wc -l)" &&
     -		test 1 -eq "$(git ls-files -u two | wc -l)"
    -+		test_line_count_cmd --out = 2 git ls-files -u &&
    -+		test_line_count_cmd --out = 1 git ls-files -u one &&
    -+		test_line_count_cmd --out = 1 git ls-files -u two
    ++		test_line_count_cmd = 2 git ls-files -u &&
    ++		test_line_count_cmd = 1 git ls-files -u one &&
    ++		test_line_count_cmd = 1 git ls-files -u two
      	fi &&
      
      	test_must_fail git diff --quiet &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'check handling of differently rena
     -		test 1 -eq "$(git ls-files -u two~second-rename | wc -l)" &&
     -		test 1 -eq "$(git ls-files -u original | wc -l)" &&
     -		test 0 -eq "$(git ls-files -o | wc -l)"
    -+		test_line_count_cmd --out = 5 git ls-files -s &&
    -+		test_line_count_cmd --out = 3 git ls-files -u &&
    -+		test_line_count_cmd --out = 1 git ls-files -u one~HEAD &&
    -+		test_line_count_cmd --out = 1 git ls-files -u two~second-rename &&
    -+		test_line_count_cmd --out = 1 git ls-files -u original &&
    -+		test_line_count_cmd --out = 0 git ls-files -o
    ++		test_line_count_cmd = 5 git ls-files -s &&
    ++		test_line_count_cmd = 3 git ls-files -u &&
    ++		test_line_count_cmd = 1 git ls-files -u one~HEAD &&
    ++		test_line_count_cmd = 1 git ls-files -u two~second-rename &&
    ++		test_line_count_cmd = 1 git ls-files -u original &&
    ++		test_line_count_cmd = 0 git ls-files -o
      	else
     -		test 5 -eq "$(git ls-files -s | wc -l)" &&
     -		test 3 -eq "$(git ls-files -u | wc -l)" &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'check handling of differently rena
     -		test 1 -eq "$(git ls-files -u two | wc -l)" &&
     -		test 1 -eq "$(git ls-files -u original | wc -l)" &&
     -		test 2 -eq "$(git ls-files -o | wc -l)"
    -+		test_line_count_cmd --out = 5 git ls-files -s &&
    -+		test_line_count_cmd --out = 3 git ls-files -u &&
    -+		test_line_count_cmd --out = 1 git ls-files -u one &&
    -+		test_line_count_cmd --out = 1 git ls-files -u two &&
    -+		test_line_count_cmd --out = 1 git ls-files -u original &&
    -+		test_line_count_cmd --out = 2 git ls-files -o
    ++		test_line_count_cmd = 5 git ls-files -s &&
    ++		test_line_count_cmd = 3 git ls-files -u &&
    ++		test_line_count_cmd = 1 git ls-files -u one &&
    ++		test_line_count_cmd = 1 git ls-files -u two &&
    ++		test_line_count_cmd = 1 git ls-files -u original &&
    ++		test_line_count_cmd = 2 git ls-files -o
      	fi &&
      
      	test_path_is_file one/file &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'check handling of differently rena
     -	test 1 -eq "$(git ls-files -u two | wc -l)" &&
     -	test 1 -eq "$(git ls-files -u original | wc -l)" &&
     -	test 0 -eq "$(git ls-files -o | wc -l)" &&
    -+	test_line_count_cmd --out = 3 git ls-files -u &&
    -+	test_line_count_cmd --out = 1 git ls-files -u one &&
    -+	test_line_count_cmd --out = 1 git ls-files -u two &&
    -+	test_line_count_cmd --out = 1 git ls-files -u original &&
    -+	test_line_count_cmd --out = 0 git ls-files -o &&
    ++	test_line_count_cmd = 3 git ls-files -u &&
    ++	test_line_count_cmd = 1 git ls-files -u one &&
    ++	test_line_count_cmd = 1 git ls-files -u two &&
    ++	test_line_count_cmd = 1 git ls-files -u original &&
    ++	test_line_count_cmd = 0 git ls-files -o &&
      
      	test_path_is_file one &&
      	test_path_is_file two &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'setup merge of rename + small chan
      
     -	test 1 -eq $(git ls-files -s | wc -l) &&
     -	test 0 -eq $(git ls-files -o | wc -l) &&
    -+	test_line_count_cmd --out = 1 git ls-files -s &&
    -+	test_line_count_cmd --out = 0 git ls-files -o &&
    ++	test_line_count_cmd = 1 git ls-files -s &&
    ++	test_line_count_cmd = 0 git ls-files -o &&
      	test $(git rev-parse HEAD:renamed_file) = $(git rev-parse HEAD~1:file)
      '
      
-- 
2.32.0.278.gd42b80f139

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

[Đoàn Trần Công Danh]

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(+)

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"
-- 
2.32.0.278.gd42b80f139

[PATCH v3 2/4] t6402: use find(1) builtin to filter instead of grep

[Đoàn Trần Công Danh]

find(1) has a builtin (-prune) to filter its output, save a bit of time
for invoking grep(1).

In addition, in a later change, we will try to use test_line_count_cmd
to count number of lines in stdout and/or stderr of a command, due to
limitation of current implementation, it can handle pipe.

Let's replace grep(1)'s usage with find(1) builtin filter.

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 t/t6402-merge-rename.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh
index 425dad97d5..5d76cd6414 100755
--- a/t/t6402-merge-rename.sh
+++ b/t/t6402-merge-rename.sh
@@ -546,7 +546,7 @@ then
 
 		test_must_fail git diff --quiet &&
 
-		test 3 -eq $(find . | grep -v .git | wc -l) &&
+		test 3 -eq $(find . -name .git -prune -o -print | wc -l) &&
 
 		test_path_is_file one &&
 		test_path_is_file two &&
@@ -565,7 +565,7 @@ else
 
 		test_must_fail git diff --quiet &&
 
-		test 4 -eq $(find . | grep -v .git | wc -l) &&
+		test 4 -eq $(find . -name .git -prune -o -print | wc -l) &&
 
 		test_path_is_dir one &&
 		test_path_is_file one~rename-two &&
@@ -593,7 +593,7 @@ test_expect_success 'pair rename to parent of other (D/F conflicts) w/ clean sta
 
 	test_must_fail git diff --quiet &&
 
-	test 3 -eq $(find . | grep -v .git | wc -l) &&
+	test 3 -eq $(find . -name .git -prune -o -print | wc -l) &&
 
 	test_path_is_file one &&
 	test_path_is_file two &&
-- 
2.32.0.278.gd42b80f139

[PATCH v3 3/4] t6400: use test_line_count_cmd to count # of lines in stdout

[Đoàn Trần Công Danh]

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 t/t6400-merge-df.sh | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/t/t6400-merge-df.sh b/t/t6400-merge-df.sh
index 38700d29b5..e5e9e473fe 100755
--- a/t/t6400-merge-df.sh
+++ b/t/t6400-merge-df.sh
@@ -82,13 +82,13 @@ test_expect_success 'modify/delete + directory/file conflict' '
 	git checkout delete^0 &&
 	test_must_fail git merge modify &&
 
-	test 5 -eq $(git ls-files -s | wc -l) &&
-	test 4 -eq $(git ls-files -u | wc -l) &&
+	test_line_count_cmd = 5 git ls-files -s &&
+	test_line_count_cmd = 4 git ls-files -u &&
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 0 -eq $(git ls-files -o | wc -l)
+		test_line_count_cmd = 0 git ls-files -o
 	else
-		test 1 -eq $(git ls-files -o | wc -l)
+		test_line_count_cmd = 1 git ls-files -o
 	fi &&
 
 	test_path_is_file letters/file &&
@@ -103,13 +103,13 @@ test_expect_success 'modify/delete + directory/file conflict; other way' '
 
 	test_must_fail git merge delete &&
 
-	test 5 -eq $(git ls-files -s | wc -l) &&
-	test 4 -eq $(git ls-files -u | wc -l) &&
+	test_line_count_cmd = 5 git ls-files -s &&
+	test_line_count_cmd = 4 git ls-files -u &&
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 0 -eq $(git ls-files -o | wc -l)
+		test_line_count_cmd = 0 git ls-files -o
 	else
-		test 1 -eq $(git ls-files -o | wc -l)
+		test_line_count_cmd = 1 git ls-files -o
 	fi &&
 
 	test_path_is_file letters/file &&
-- 
2.32.0.278.gd42b80f139

[PATCH v3 4/4] t6402: use test_line_count_cmd to count # of lines in stdout

[Đoàn Trần Công Danh]

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 t/t6402-merge-rename.sh | 126 +++++++++++++++++++---------------------
 1 file changed, 59 insertions(+), 67 deletions(-)

diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh
index 5d76cd6414..52bd35899e 100755
--- a/t/t6402-merge-rename.sh
+++ b/t/t6402-merge-rename.sh
@@ -105,10 +105,8 @@ test_expect_success 'pull renaming branch into unrenaming one' \
 	git show-branch &&
 	test_expect_code 1 git pull . white &&
 	git ls-files -s &&
-	git ls-files -u B >b.stages &&
-	test_line_count = 3 b.stages &&
-	git ls-files -s N >n.stages &&
-	test_line_count = 1 n.stages &&
+	test_line_count_cmd = 3 git ls-files -u B &&
+	test_line_count_cmd = 1 git ls-files -s N &&
 	sed -ne "/^g/{
 	p
 	q
@@ -122,10 +120,8 @@ test_expect_success 'pull renaming branch into another renaming one' \
 	git reset --hard &&
 	git checkout red &&
 	test_expect_code 1 git pull . white &&
-	git ls-files -u B >b.stages &&
-	test_line_count = 3 b.stages &&
-	git ls-files -s N >n.stages &&
-	test_line_count = 1 n.stages &&
+	test_line_count_cmd = 3 git ls-files -u B &&
+	test_line_count_cmd = 1 git ls-files -s N &&
 	sed -ne "/^g/{
 	p
 	q
@@ -138,10 +134,8 @@ test_expect_success 'pull unrenaming branch into renaming one' \
 	git reset --hard &&
 	git show-branch &&
 	test_expect_code 1 git pull . main &&
-	git ls-files -u B >b.stages &&
-	test_line_count = 3 b.stages &&
-	git ls-files -s N >n.stages &&
-	test_line_count = 1 n.stages &&
+	test_line_count_cmd = 3 git ls-files -u B &&
+	test_line_count_cmd = 1 git ls-files -s N &&
 	sed -ne "/^g/{
 	p
 	q
@@ -154,14 +148,10 @@ test_expect_success 'pull conflicting renames' \
 	git reset --hard &&
 	git show-branch &&
 	test_expect_code 1 git pull . blue &&
-	git ls-files -u A >a.stages &&
-	test_line_count = 1 a.stages &&
-	git ls-files -u B >b.stages &&
-	test_line_count = 1 b.stages &&
-	git ls-files -u C >c.stages &&
-	test_line_count = 1 c.stages &&
-	git ls-files -s N >n.stages &&
-	test_line_count = 1 n.stages &&
+	test_line_count_cmd = 1 git ls-files -u A &&
+	test_line_count_cmd = 1 git ls-files -u B &&
+	test_line_count_cmd = 1 git ls-files -u C &&
+	test_line_count_cmd = 1 git ls-files -s N &&
 	sed -ne "/^g/{
 	p
 	q
@@ -330,8 +320,8 @@ test_expect_success 'Rename+D/F conflict; renamed file merges but dir in way' '
 		test_i18ngrep "Adding as dir~HEAD instead" output
 	fi &&
 
-	test 3 -eq "$(git ls-files -u | wc -l)" &&
-	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
+	test_line_count_cmd = 3 git ls-files -u &&
+	test_line_count_cmd = 2 git ls-files -u dir/file-in-the-way &&
 
 	test_must_fail git diff --quiet &&
 	test_must_fail git diff --cached --quiet &&
@@ -357,8 +347,8 @@ test_expect_success 'Same as previous, but merged other way' '
 		test_i18ngrep "Adding as dir~renamed-file-has-no-conflicts instead" output
 	fi &&
 
-	test 3 -eq "$(git ls-files -u | wc -l)" &&
-	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
+	test_line_count_cmd = 3 git ls-files -u &&
+	test_line_count_cmd = 2 git ls-files -u dir/file-in-the-way &&
 
 	test_must_fail git diff --quiet &&
 	test_must_fail git diff --cached --quiet &&
@@ -374,8 +364,8 @@ test_expect_success 'Rename+D/F conflict; renamed file cannot merge, dir not in
 	git checkout -q renamed-file-has-conflicts^0 &&
 	test_must_fail git merge --strategy=recursive dir-not-in-way &&
 
-	test 3 -eq "$(git ls-files -u | wc -l)" &&
-	test 3 -eq "$(git ls-files -u dir | wc -l)" &&
+	test_line_count_cmd = 3 git ls-files -u &&
+	test_line_count_cmd = 3 git ls-files -u dir &&
 
 	test_must_fail git diff --quiet &&
 	test_must_fail git diff --cached --quiet &&
@@ -409,14 +399,15 @@ test_expect_success 'Rename+D/F conflict; renamed file cannot merge and dir in t
 	git checkout -q renamed-file-has-conflicts^0 &&
 	test_must_fail git merge --strategy=recursive dir-in-way &&
 
-	test 5 -eq "$(git ls-files -u | wc -l)" &&
+	test_line_count_cmd = 5 git ls-files -u &&
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 3 -eq "$(git ls-files -u dir~HEAD | wc -l)"
+		test_line_count_cmd = 3 git ls-files -u dir~HEAD
 	else
-		test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)"
+		git ls-files -u dir >out &&
+		test_line_count_cmd = 3 grep -v file-in-the-way out
 	fi &&
-	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
+	test_line_count_cmd = 2 git ls-files -u dir/file-in-the-way &&
 
 	test_must_fail git diff --quiet &&
 	test_must_fail git diff --cached --quiet &&
@@ -432,14 +423,15 @@ test_expect_success 'Same as previous, but merged other way' '
 	git checkout -q dir-in-way^0 &&
 	test_must_fail git merge --strategy=recursive renamed-file-has-conflicts &&
 
-	test 5 -eq "$(git ls-files -u | wc -l)" &&
+	test_line_count_cmd = 5 git ls-files -u &&
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 3 -eq "$(git ls-files -u dir~renamed-file-has-conflicts | wc -l)"
+		test_line_count_cmd = 3 git ls-files -u dir~renamed-file-has-conflicts
 	else
-		test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)"
+		git ls-files -u dir >out &&
+		test_line_count_cmd = 3 grep -v file-in-the-way out
 	fi &&
-	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
+	test_line_count_cmd = 2 git ls-files -u dir/file-in-the-way &&
 
 	test_must_fail git diff --quiet &&
 	test_must_fail git diff --cached --quiet &&
@@ -496,9 +488,9 @@ test_expect_success 'both rename source and destination involved in D/F conflict
 
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 2 -eq "$(git ls-files -u | wc -l)"
+		test_line_count_cmd = 2 git ls-files -u
 	else
-		test 1 -eq "$(git ls-files -u | wc -l)"
+		test_line_count_cmd = 1 git ls-files -u
 	fi &&
 
 	test_must_fail git diff --quiet &&
@@ -540,9 +532,9 @@ then
 		mkdir one &&
 		test_must_fail git merge --strategy=recursive rename-two &&
 
-		test 4 -eq "$(git ls-files -u | wc -l)" &&
-		test 2 -eq "$(git ls-files -u one | wc -l)" &&
-		test 2 -eq "$(git ls-files -u two | wc -l)" &&
+		test_line_count_cmd = 4 git ls-files -u &&
+		test_line_count_cmd = 2 git ls-files -u one &&
+		test_line_count_cmd = 2 git ls-files -u two &&
 
 		test_must_fail git diff --quiet &&
 
@@ -559,9 +551,9 @@ else
 		mkdir one &&
 		test_must_fail git merge --strategy=recursive rename-two &&
 
-		test 2 -eq "$(git ls-files -u | wc -l)" &&
-		test 1 -eq "$(git ls-files -u one | wc -l)" &&
-		test 1 -eq "$(git ls-files -u two | wc -l)" &&
+		test_line_count_cmd = 2 git ls-files -u &&
+		test_line_count_cmd = 1 git ls-files -u one &&
+		test_line_count_cmd = 1 git ls-files -u two &&
 
 		test_must_fail git diff --quiet &&
 
@@ -582,13 +574,13 @@ test_expect_success 'pair rename to parent of other (D/F conflicts) w/ clean sta
 
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 4 -eq "$(git ls-files -u | wc -l)" &&
-		test 2 -eq "$(git ls-files -u one | wc -l)" &&
-		test 2 -eq "$(git ls-files -u two | wc -l)"
+		test_line_count_cmd = 4 git ls-files -u &&
+		test_line_count_cmd = 2 git ls-files -u one &&
+		test_line_count_cmd = 2 git ls-files -u two
 	else
-		test 2 -eq "$(git ls-files -u | wc -l)" &&
-		test 1 -eq "$(git ls-files -u one | wc -l)" &&
-		test 1 -eq "$(git ls-files -u two | wc -l)"
+		test_line_count_cmd = 2 git ls-files -u &&
+		test_line_count_cmd = 1 git ls-files -u one &&
+		test_line_count_cmd = 1 git ls-files -u two
 	fi &&
 
 	test_must_fail git diff --quiet &&
@@ -631,19 +623,19 @@ test_expect_success 'check handling of differently renamed file with D/F conflic
 
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 5 -eq "$(git ls-files -s | wc -l)" &&
-		test 3 -eq "$(git ls-files -u | wc -l)" &&
-		test 1 -eq "$(git ls-files -u one~HEAD | wc -l)" &&
-		test 1 -eq "$(git ls-files -u two~second-rename | wc -l)" &&
-		test 1 -eq "$(git ls-files -u original | wc -l)" &&
-		test 0 -eq "$(git ls-files -o | wc -l)"
+		test_line_count_cmd = 5 git ls-files -s &&
+		test_line_count_cmd = 3 git ls-files -u &&
+		test_line_count_cmd = 1 git ls-files -u one~HEAD &&
+		test_line_count_cmd = 1 git ls-files -u two~second-rename &&
+		test_line_count_cmd = 1 git ls-files -u original &&
+		test_line_count_cmd = 0 git ls-files -o
 	else
-		test 5 -eq "$(git ls-files -s | wc -l)" &&
-		test 3 -eq "$(git ls-files -u | wc -l)" &&
-		test 1 -eq "$(git ls-files -u one | wc -l)" &&
-		test 1 -eq "$(git ls-files -u two | wc -l)" &&
-		test 1 -eq "$(git ls-files -u original | wc -l)" &&
-		test 2 -eq "$(git ls-files -o | wc -l)"
+		test_line_count_cmd = 5 git ls-files -s &&
+		test_line_count_cmd = 3 git ls-files -u &&
+		test_line_count_cmd = 1 git ls-files -u one &&
+		test_line_count_cmd = 1 git ls-files -u two &&
+		test_line_count_cmd = 1 git ls-files -u original &&
+		test_line_count_cmd = 2 git ls-files -o
 	fi &&
 
 	test_path_is_file one/file &&
@@ -679,11 +671,11 @@ test_expect_success 'check handling of differently renamed file with D/F conflic
 	git checkout -q first-rename-redo^0 &&
 	test_must_fail git merge --strategy=recursive second-rename-redo &&
 
-	test 3 -eq "$(git ls-files -u | wc -l)" &&
-	test 1 -eq "$(git ls-files -u one | wc -l)" &&
-	test 1 -eq "$(git ls-files -u two | wc -l)" &&
-	test 1 -eq "$(git ls-files -u original | wc -l)" &&
-	test 0 -eq "$(git ls-files -o | wc -l)" &&
+	test_line_count_cmd = 3 git ls-files -u &&
+	test_line_count_cmd = 1 git ls-files -u one &&
+	test_line_count_cmd = 1 git ls-files -u two &&
+	test_line_count_cmd = 1 git ls-files -u original &&
+	test_line_count_cmd = 0 git ls-files -o &&
 
 	test_path_is_file one &&
 	test_path_is_file two &&
@@ -861,8 +853,8 @@ test_expect_success 'setup merge of rename + small change' '
 test_expect_success 'merge rename + small change' '
 	git merge rename_branch &&
 
-	test 1 -eq $(git ls-files -s | wc -l) &&
-	test 0 -eq $(git ls-files -o | wc -l) &&
+	test_line_count_cmd = 1 git ls-files -s &&
+	test_line_count_cmd = 0 git ls-files -o &&
 	test $(git rev-parse HEAD:renamed_file) = $(git rev-parse HEAD~1:file)
 '
 
-- 
2.32.0.278.gd42b80f139

Re: [PATCH v3 0/4] t: new helper test_line_count_cmd

[Eric Sunshine]

On Fri, Jun 18, 2021 at 9:31 PM Đoàn Trần Công Danh
<congdanhqx@gmail.com> wrote:
> Change in v3 since v2:
> * --err was dropped entirely
> * --out is not an option anymore, <binops> and <value> is the first two
>   arguments that fed into test_line_count_cmd

When I read the previous version of this series, I found that many of
the instances where test_line_count_cmd() were used became quite
noisy, to the point that it was difficult to see at-a-glance the
command being tested. As an experiment, on top of your patch 1, I
crafted a patch which made `--out` and `--err` optional (defaulting to
`--out`). That helped reduce the noise level a good deal, however, I
still found it too noisy. Consequently, I crafted a second patch which
renamed the function to test_output_count(), and only then did the
noise level drop sufficiently that the command being tested didn't
entirely disappear into the background.

Since you've dropped the `--out` and `--err` options entirely, I
wonder if now would be a good time to shorten the function name, as
well, in order to further reduce the noise level. Since it now only
tests stdout (and doesn't deal with stderr), a name even shorter than
what I tried for the last version might be even better. So, for
instance, the name test_out_count() might not be too bad:

    test_out_count = 0 git ls-files -o &&

Re: [PATCH v3 0/4] t: new helper test_line_count_cmd

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> Since you've dropped the `--out` and `--err` options entirely, I
> wonder if now would be a good time to shorten the function name, as
> well, in order to further reduce the noise level. Since it now only
> tests stdout (and doesn't deal with stderr), a name even shorter than
> what I tried for the last version might be even better. So, for
> instance, the name test_out_count() might not be too bad:
>
>     test_out_count = 0 git ls-files -o &&

"Test out" to me sound like trying something out and the part "out"
in the name no longer hints it is about "output"; you may have
shortened the name too much to be meaningful, I am afraid.

Is the helper used to check with anything but equality?  Otherwise
you can lose "= " to make it shorter.

Having said all that, as an external interface, I wonder

	test_line_count -e = 0 git-ls-files -o

would work better.  It usually takes <op> <num> <file>, but when
$1 is a magic "-e", we shift it out and it becomes <op> <num> <cmd>...

Re: [PATCH v3 0/4] t: new helper test_line_count_cmd

[Eric Sunshine]

On Sat, Jun 19, 2021 at 2:17 AM Junio C Hamano <gitster@pobox.com> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> > what I tried for the last version might be even better. So, for
> > instance, the name test_out_count() might not be too bad:
> >
> >     test_out_count = 0 git ls-files -o &&
>
> "Test out" to me sound like trying something out and the part "out"
> in the name no longer hints it is about "output"; you may have
> shortened the name too much to be meaningful, I am afraid.
>
> Is the helper used to check with anything but equality?  Otherwise
> you can lose "= " to make it shorter.
>
> Having said all that, as an external interface, I wonder
>
>         test_line_count -e = 0 git-ls-files -o
>
> would work better.  It usually takes <op> <num> <file>, but when
> $1 is a magic "-e", we shift it out and it becomes <op> <num> <cmd>...

Indeed, I have no problem seeing this as a new mode of
test_line_count() triggered by an option. In fact, I suggested exactly
that[1] when this idea first arose (except I named the option `-c`
rather than `-e`, but the latter is fine). However, my suggestion was
pretty much shot down[2] (and I don't entirely disagree with [2],
which is why I didn't pursue the idea in [1]).

[1]: https://lore.kernel.org/git/CAPig+cS4tkXZLPDEWgEytzEOCR7oGrXyg1CZVKVPSXuJOifLjQ@mail.gmail.com/
[2]: https://lore.kernel.org/git/xmqq5z0fxlgn.fsf@gitster.g/

Re: [PATCH v3 0/4] t: new helper test_line_count_cmd

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> Indeed, I have no problem seeing this as a new mode of
> test_line_count() triggered by an option. In fact, I suggested exactly
> that[1] when this idea first arose (except I named the option `-c`
> rather than `-e`, but the latter is fine). However, my suggestion was
> pretty much shot down[2] (and I don't entirely disagree with [2],
> which is why I didn't pursue the idea in [1]).

;-)  

Yeah, I still am skeptical that we'd gain much by hiding the
redirection to >actual behind the helper, so as I said in response
to the v2 series, I am fine without this new helper or an enhanced
test_line_count, but go with more use of test_must_be_empty etc.

Re: [PATCH v3 2/4] t6402: use find(1) builtin to filter instead of grep

[Andrei Rybak]

On 19/06/2021 03:30, Đoàn Trần Công Danh wrote:
> find(1) has a builtin (-prune) to filter its output, save a bit of time
> for invoking grep(1).
> 
> In addition, in a later change, we will try to use test_line_count_cmd
> to count number of lines in stdout and/or stderr of a command, due to

Looking at [PATCH v3 1/4] of this series, mention of "stderr" here is no
longer relevant.

> limitation of current implementation, it can handle pipe.

Seems like a typo s/can/can't/ ?

> Let's replace grep(1)'s usage with find(1) builtin filter.
> 
> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> ---
>   t/t6402-merge-rename.sh | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh
> index 425dad97d5..5d76cd6414 100755
> --- a/t/t6402-merge-rename.sh
> +++ b/t/t6402-merge-rename.sh
> @@ -546,7 +546,7 @@ then
>   
>   		test_must_fail git diff --quiet &&
>   
> -		test 3 -eq $(find . | grep -v .git | wc -l) &&
> +		test 3 -eq $(find . -name .git -prune -o -print | wc -l) &&
>   
>   		test_path_is_file one &&
>   		test_path_is_file two &&
> @@ -565,7 +565,7 @@ else
>   
>   		test_must_fail git diff --quiet &&
>   
> -		test 4 -eq $(find . | grep -v .git | wc -l) &&
> +		test 4 -eq $(find . -name .git -prune -o -print | wc -l) &&
>   
>   		test_path_is_dir one &&
>   		test_path_is_file one~rename-two &&
> @@ -593,7 +593,7 @@ test_expect_success 'pair rename to parent of other (D/F conflicts) w/ clean sta
>   
>   	test_must_fail git diff --quiet &&
>   
> -	test 3 -eq $(find . | grep -v .git | wc -l) &&
> +	test 3 -eq $(find . -name .git -prune -o -print | wc -l) &&

Because in the original `grep` wasn't invoked with `-F` it means that
".git" is a regex which would match any path which contains the word
"git" in it, because "." matches any character, including the leading
slash that `find` outputs.  Such narrowing of what we intend to filter
out is a good change.

This semantic change in filtering doesn't affect tests in t6402, as the
test directory doesn't have paths with the word "git" except for the
".git" directory.  It might be worth mentioning in the commit message.

>   
>   	test_path_is_file one &&
>   	test_path_is_file two &&
>

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

[Andrei Rybak]

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"
> 

Re: [PATCH v3 0/4] t: new helper test_line_count_cmd

[Đoàn Trần Công Danh]

On 2021-06-19 15:50:17+0900, Junio C Hamano <gitster@pobox.com> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> 
> > Indeed, I have no problem seeing this as a new mode of
> > test_line_count() triggered by an option. In fact, I suggested exactly
> > that[1] when this idea first arose (except I named the option `-c`
> > rather than `-e`, but the latter is fine). However, my suggestion was
> > pretty much shot down[2] (and I don't entirely disagree with [2],
> > which is why I didn't pursue the idea in [1]).
> 
> ;-)  
> 
> Yeah, I still am skeptical that we'd gain much by hiding the
> redirection to >actual behind the helper, so as I said in response
> to the v2 series, I am fine without this new helper or an enhanced
> test_line_count, but go with more use of test_must_be_empty etc.

I guess the overall feedback for this new helper is negative.
I think the consensus here is a local helper in t640{0,2} for counting
ls-files?

-- 
Danh

Re: [PATCH v3 2/4] t6402: use find(1) builtin to filter instead of grep

[Đoàn Trần Công Danh]

On 2021-06-21 10:17:52+0200, Andrei Rybak <rybak.a.v@gmail.com> wrote:
> On 19/06/2021 03:30, Đoàn Trần Công Danh wrote:
> > find(1) has a builtin (-prune) to filter its output, save a bit of time
> > for invoking grep(1).
> > 
> > In addition, in a later change, we will try to use test_line_count_cmd
> > to count number of lines in stdout and/or stderr of a command, due to
> 
> Looking at [PATCH v3 1/4] of this series, mention of "stderr" here is no
> longer relevant.

Yes, you're correct.

> > limitation of current implementation, it can handle pipe.
> 
> Seems like a typo s/can/can't/ ?

This is correct, too.

> > Let's replace grep(1)'s usage with find(1) builtin filter.
> > 
> > Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> > ---
> >   t/t6402-merge-rename.sh | 6 +++---
> >   1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh
> > index 425dad97d5..5d76cd6414 100755
> > --- a/t/t6402-merge-rename.sh
> > +++ b/t/t6402-merge-rename.sh
> > @@ -546,7 +546,7 @@ then
> >   		test_must_fail git diff --quiet &&
> > -		test 3 -eq $(find . | grep -v .git | wc -l) &&
> > +		test 3 -eq $(find . -name .git -prune -o -print | wc -l) &&
> >   		test_path_is_file one &&
> >   		test_path_is_file two &&
> > @@ -565,7 +565,7 @@ else
> >   		test_must_fail git diff --quiet &&
> > -		test 4 -eq $(find . | grep -v .git | wc -l) &&
> > +		test 4 -eq $(find . -name .git -prune -o -print | wc -l) &&
> >   		test_path_is_dir one &&
> >   		test_path_is_file one~rename-two &&
> > @@ -593,7 +593,7 @@ test_expect_success 'pair rename to parent of other (D/F conflicts) w/ clean sta
> >   	test_must_fail git diff --quiet &&
> > -	test 3 -eq $(find . | grep -v .git | wc -l) &&
> > +	test 3 -eq $(find . -name .git -prune -o -print | wc -l) &&
> 
> Because in the original `grep` wasn't invoked with `-F` it means that
> ".git" is a regex which would match any path which contains the word
> "git" in it, because "." matches any character, including the leading
> slash that `find` outputs.  Such narrowing of what we intend to filter
> out is a good change.

I think the original intention is using "grep -F". I'll add that
information into the commit message.

> This semantic change in filtering doesn't affect tests in t6402, as the
> test directory doesn't have paths with the word "git" except for the
> ".git" directory.  It might be worth mentioning in the commit message.

Thanks.

-- 
Danh

Re: [PATCH v3 0/4] t: new helper test_line_count_cmd

[Eric Sunshine]

On Mon, Jun 21, 2021 at 7:52 PM Đoàn Trần Công Danh
<congdanhqx@gmail.com> wrote:
> On 2021-06-19 15:50:17+0900, Junio C Hamano <gitster@pobox.com> wrote:
> > Eric Sunshine <sunshine@sunshineco.com> writes:
> > > Indeed, I have no problem seeing this as a new mode of
> > > test_line_count() triggered by an option. In fact, I suggested exactly
> > > that[1] when this idea first arose (except I named the option `-c`
> > > rather than `-e`, but the latter is fine). However, my suggestion was
> > > pretty much shot down[2] (and I don't entirely disagree with [2],
> > > which is why I didn't pursue the idea in [1]).
> >
> > Yeah, I still am skeptical that we'd gain much by hiding the
> > redirection to >actual behind the helper, so as I said in response
> > to the v2 series, I am fine without this new helper or an enhanced
> > test_line_count, but go with more use of test_must_be_empty etc.
>
> I guess the overall feedback for this new helper is negative.
> I think the consensus here is a local helper in t640{0,2} for counting
> ls-files?

A local specialized function makes sense to me.

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

[Andrei Rybak]

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

[PATCH v4 0/2] t640{0,2}: preserve ls-files exit status code

[Đoàn Trần Công Danh]

This is a series to clear false positive when applying Junio's suggestion to
to a series written by Ævar [1].

Not that we have any conclusion on that suggestion, just to clear the way out.

In v4, I dropped the test_line_count_cmd completely.
A local to t640{0,2} helper was written instead.

Hence, the changelog and range-diff for v2 and v3 is dropped.

1: https://lore.kernel.org/git/87r1j42ffz.fsf@evledraar.gmail.com/

Đoàn Trần Công Danh (2):
  t6400: preserve git ls-files exit status code
  t6402: preserve git exit status code

 t/t6400-merge-df.sh     |  30 ++++++---
 t/t6402-merge-rename.sh | 146 +++++++++++++++++++++-------------------
 2 files changed, 100 insertions(+), 76 deletions(-)

-- 
2.32.0.278.gd42b80f139

[PATCH v4 1/2] t6400: preserve git ls-files exit status code

[Đoàn Trần Công Danh]

In t6400, we're checking number of files in the index and the working
tree by piping the output of "git ls-files" to "wc -l", thus losing the
exit status code of git.

Let's write the output of "git ls-files" to a temporary file, in order
to check exit status code of "git ls-files" properly.

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 t/t6400-merge-df.sh | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/t/t6400-merge-df.sh b/t/t6400-merge-df.sh
index 38700d29b5..c2888323c1 100755
--- a/t/t6400-merge-df.sh
+++ b/t/t6400-merge-df.sh
@@ -9,6 +9,20 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
 
+check_ls_files_count() {
+	local ops val
+	if test "$#" -le 2
+	then
+		BUG "Expect 2 or more arguments"
+	fi &&
+	ops="$1" &&
+	val="$2" &&
+	shift 2 &&
+	mkdir -p .git/trash &&
+	git ls-files "$@" >.git/trash/output &&
+	test_line_count "$ops" "$val" .git/trash/output
+}
+
 test_expect_success 'prepare repository' '
 	echo Hello >init &&
 	git add init &&
@@ -82,13 +96,13 @@ test_expect_success 'modify/delete + directory/file conflict' '
 	git checkout delete^0 &&
 	test_must_fail git merge modify &&
 
-	test 5 -eq $(git ls-files -s | wc -l) &&
-	test 4 -eq $(git ls-files -u | wc -l) &&
+	check_ls_files_count = 5 -s &&
+	check_ls_files_count = 4 -u &&
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 0 -eq $(git ls-files -o | wc -l)
+		check_ls_files_count = 0 -o
 	else
-		test 1 -eq $(git ls-files -o | wc -l)
+		check_ls_files_count = 1 -o
 	fi &&
 
 	test_path_is_file letters/file &&
@@ -103,13 +117,13 @@ test_expect_success 'modify/delete + directory/file conflict; other way' '
 
 	test_must_fail git merge delete &&
 
-	test 5 -eq $(git ls-files -s | wc -l) &&
-	test 4 -eq $(git ls-files -u | wc -l) &&
+	check_ls_files_count = 5 -s &&
+	check_ls_files_count = 4 -u &&
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 0 -eq $(git ls-files -o | wc -l)
+		check_ls_files_count = 0 -o
 	else
-		test 1 -eq $(git ls-files -o | wc -l)
+		check_ls_files_count = 1 -o
 	fi &&
 
 	test_path_is_file letters/file &&
-- 
2.32.0.278.gd42b80f139

[PATCH v4 2/2] t6402: preserve git exit status code

[Đoàn Trần Công Danh]

In t6402, we're checking number of files in the index and the working
tree by piping the output of "git ls-files" to "wc -l", thus losing the
exit status code of git.

Let's write the output of "git ls-files" to a temporary file, in order
to check exit status code of "git ls-files" properly.

While we're at it, also check exit status code of an invocation of
git-rev-parse, too.

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 t/t6402-merge-rename.sh | 146 +++++++++++++++++++++-------------------
 1 file changed, 78 insertions(+), 68 deletions(-)

diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh
index 425dad97d5..fbfa8967b0 100755
--- a/t/t6402-merge-rename.sh
+++ b/t/t6402-merge-rename.sh
@@ -11,6 +11,20 @@ modify () {
 	mv "$2.x" "$2"
 }
 
+check_ls_files_count() {
+	local ops val
+	if test "$#" -le 2
+	then
+		BUG "Expect 2 or more arguments"
+	fi &&
+	ops="$1" &&
+	val="$2" &&
+	shift 2 &&
+	mkdir -p .git/trash &&
+	git ls-files "$@" >.git/trash/output &&
+	test_line_count "$ops" "$val" .git/trash/output
+}
+
 test_expect_success 'setup' '
 	cat >A <<-\EOF &&
 	a aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
@@ -105,10 +119,8 @@ test_expect_success 'pull renaming branch into unrenaming one' \
 	git show-branch &&
 	test_expect_code 1 git pull . white &&
 	git ls-files -s &&
-	git ls-files -u B >b.stages &&
-	test_line_count = 3 b.stages &&
-	git ls-files -s N >n.stages &&
-	test_line_count = 1 n.stages &&
+	check_ls_files_count = 3 -u B &&
+	check_ls_files_count = 1 -s N &&
 	sed -ne "/^g/{
 	p
 	q
@@ -122,10 +134,8 @@ test_expect_success 'pull renaming branch into another renaming one' \
 	git reset --hard &&
 	git checkout red &&
 	test_expect_code 1 git pull . white &&
-	git ls-files -u B >b.stages &&
-	test_line_count = 3 b.stages &&
-	git ls-files -s N >n.stages &&
-	test_line_count = 1 n.stages &&
+	check_ls_files_count = 3 -u B &&
+	check_ls_files_count = 1 -s N &&
 	sed -ne "/^g/{
 	p
 	q
@@ -138,10 +148,8 @@ test_expect_success 'pull unrenaming branch into renaming one' \
 	git reset --hard &&
 	git show-branch &&
 	test_expect_code 1 git pull . main &&
-	git ls-files -u B >b.stages &&
-	test_line_count = 3 b.stages &&
-	git ls-files -s N >n.stages &&
-	test_line_count = 1 n.stages &&
+	check_ls_files_count = 3 -u B &&
+	check_ls_files_count = 1 -s N &&
 	sed -ne "/^g/{
 	p
 	q
@@ -154,14 +162,10 @@ test_expect_success 'pull conflicting renames' \
 	git reset --hard &&
 	git show-branch &&
 	test_expect_code 1 git pull . blue &&
-	git ls-files -u A >a.stages &&
-	test_line_count = 1 a.stages &&
-	git ls-files -u B >b.stages &&
-	test_line_count = 1 b.stages &&
-	git ls-files -u C >c.stages &&
-	test_line_count = 1 c.stages &&
-	git ls-files -s N >n.stages &&
-	test_line_count = 1 n.stages &&
+	check_ls_files_count = 1 -u A &&
+	check_ls_files_count = 1 -u B &&
+	check_ls_files_count = 1 -u C &&
+	check_ls_files_count = 1 -s N &&
 	sed -ne "/^g/{
 	p
 	q
@@ -330,8 +334,8 @@ test_expect_success 'Rename+D/F conflict; renamed file merges but dir in way' '
 		test_i18ngrep "Adding as dir~HEAD instead" output
 	fi &&
 
-	test 3 -eq "$(git ls-files -u | wc -l)" &&
-	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
+	check_ls_files_count = 3 -u &&
+	check_ls_files_count = 2 -u dir/file-in-the-way &&
 
 	test_must_fail git diff --quiet &&
 	test_must_fail git diff --cached --quiet &&
@@ -357,8 +361,8 @@ test_expect_success 'Same as previous, but merged other way' '
 		test_i18ngrep "Adding as dir~renamed-file-has-no-conflicts instead" output
 	fi &&
 
-	test 3 -eq "$(git ls-files -u | wc -l)" &&
-	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
+	check_ls_files_count = 3 -u &&
+	check_ls_files_count = 2 -u dir/file-in-the-way &&
 
 	test_must_fail git diff --quiet &&
 	test_must_fail git diff --cached --quiet &&
@@ -374,8 +378,8 @@ test_expect_success 'Rename+D/F conflict; renamed file cannot merge, dir not in
 	git checkout -q renamed-file-has-conflicts^0 &&
 	test_must_fail git merge --strategy=recursive dir-not-in-way &&
 
-	test 3 -eq "$(git ls-files -u | wc -l)" &&
-	test 3 -eq "$(git ls-files -u dir | wc -l)" &&
+	check_ls_files_count = 3 -u &&
+	check_ls_files_count = 3 -u dir &&
 
 	test_must_fail git diff --quiet &&
 	test_must_fail git diff --cached --quiet &&
@@ -409,14 +413,16 @@ test_expect_success 'Rename+D/F conflict; renamed file cannot merge and dir in t
 	git checkout -q renamed-file-has-conflicts^0 &&
 	test_must_fail git merge --strategy=recursive dir-in-way &&
 
-	test 5 -eq "$(git ls-files -u | wc -l)" &&
+	check_ls_files_count = 5 -u &&
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 3 -eq "$(git ls-files -u dir~HEAD | wc -l)"
+		check_ls_files_count = 3 -u dir~HEAD
 	else
-		test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)"
+		git ls-files -u dir >out &&
+		test 3 -eq $(grep -v file-in-the-way out | wc -l) &&
+		rm -f out
 	fi &&
-	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
+	check_ls_files_count = 2 -u dir/file-in-the-way &&
 
 	test_must_fail git diff --quiet &&
 	test_must_fail git diff --cached --quiet &&
@@ -432,14 +438,16 @@ test_expect_success 'Same as previous, but merged other way' '
 	git checkout -q dir-in-way^0 &&
 	test_must_fail git merge --strategy=recursive renamed-file-has-conflicts &&
 
-	test 5 -eq "$(git ls-files -u | wc -l)" &&
+	check_ls_files_count = 5 -u &&
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 3 -eq "$(git ls-files -u dir~renamed-file-has-conflicts | wc -l)"
+		check_ls_files_count = 3 -u dir~renamed-file-has-conflicts
 	else
-		test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)"
+		git ls-files -u dir >out &&
+		test 3 -eq $(grep -v file-in-the-way out | wc -l) &&
+		rm -f out
 	fi &&
-	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
+	check_ls_files_count = 2 -u dir/file-in-the-way &&
 
 	test_must_fail git diff --quiet &&
 	test_must_fail git diff --cached --quiet &&
@@ -496,9 +504,9 @@ test_expect_success 'both rename source and destination involved in D/F conflict
 
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 2 -eq "$(git ls-files -u | wc -l)"
+		check_ls_files_count = 2 -u
 	else
-		test 1 -eq "$(git ls-files -u | wc -l)"
+		check_ls_files_count = 1 -u
 	fi &&
 
 	test_must_fail git diff --quiet &&
@@ -540,9 +548,9 @@ then
 		mkdir one &&
 		test_must_fail git merge --strategy=recursive rename-two &&
 
-		test 4 -eq "$(git ls-files -u | wc -l)" &&
-		test 2 -eq "$(git ls-files -u one | wc -l)" &&
-		test 2 -eq "$(git ls-files -u two | wc -l)" &&
+		check_ls_files_count = 4 -u &&
+		check_ls_files_count = 2 -u one &&
+		check_ls_files_count = 2 -u two &&
 
 		test_must_fail git diff --quiet &&
 
@@ -559,9 +567,9 @@ else
 		mkdir one &&
 		test_must_fail git merge --strategy=recursive rename-two &&
 
-		test 2 -eq "$(git ls-files -u | wc -l)" &&
-		test 1 -eq "$(git ls-files -u one | wc -l)" &&
-		test 1 -eq "$(git ls-files -u two | wc -l)" &&
+		check_ls_files_count = 2 -u &&
+		check_ls_files_count = 1 -u one &&
+		check_ls_files_count = 1 -u two &&
 
 		test_must_fail git diff --quiet &&
 
@@ -582,13 +590,13 @@ test_expect_success 'pair rename to parent of other (D/F conflicts) w/ clean sta
 
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 4 -eq "$(git ls-files -u | wc -l)" &&
-		test 2 -eq "$(git ls-files -u one | wc -l)" &&
-		test 2 -eq "$(git ls-files -u two | wc -l)"
+		check_ls_files_count = 4 -u &&
+		check_ls_files_count = 2 -u one &&
+		check_ls_files_count = 2 -u two
 	else
-		test 2 -eq "$(git ls-files -u | wc -l)" &&
-		test 1 -eq "$(git ls-files -u one | wc -l)" &&
-		test 1 -eq "$(git ls-files -u two | wc -l)"
+		check_ls_files_count = 2 -u &&
+		check_ls_files_count = 1 -u one &&
+		check_ls_files_count = 1 -u two
 	fi &&
 
 	test_must_fail git diff --quiet &&
@@ -631,19 +639,19 @@ test_expect_success 'check handling of differently renamed file with D/F conflic
 
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 5 -eq "$(git ls-files -s | wc -l)" &&
-		test 3 -eq "$(git ls-files -u | wc -l)" &&
-		test 1 -eq "$(git ls-files -u one~HEAD | wc -l)" &&
-		test 1 -eq "$(git ls-files -u two~second-rename | wc -l)" &&
-		test 1 -eq "$(git ls-files -u original | wc -l)" &&
-		test 0 -eq "$(git ls-files -o | wc -l)"
+		check_ls_files_count = 5 -s &&
+		check_ls_files_count = 3 -u &&
+		check_ls_files_count = 1 -u one~HEAD &&
+		check_ls_files_count = 1 -u two~second-rename &&
+		check_ls_files_count = 1 -u original &&
+		check_ls_files_count = 0 -o
 	else
-		test 5 -eq "$(git ls-files -s | wc -l)" &&
-		test 3 -eq "$(git ls-files -u | wc -l)" &&
-		test 1 -eq "$(git ls-files -u one | wc -l)" &&
-		test 1 -eq "$(git ls-files -u two | wc -l)" &&
-		test 1 -eq "$(git ls-files -u original | wc -l)" &&
-		test 2 -eq "$(git ls-files -o | wc -l)"
+		check_ls_files_count = 5 -s &&
+		check_ls_files_count = 3 -u &&
+		check_ls_files_count = 1 -u one &&
+		check_ls_files_count = 1 -u two &&
+		check_ls_files_count = 1 -u original &&
+		check_ls_files_count = 2 -o
 	fi &&
 
 	test_path_is_file one/file &&
@@ -679,11 +687,11 @@ test_expect_success 'check handling of differently renamed file with D/F conflic
 	git checkout -q first-rename-redo^0 &&
 	test_must_fail git merge --strategy=recursive second-rename-redo &&
 
-	test 3 -eq "$(git ls-files -u | wc -l)" &&
-	test 1 -eq "$(git ls-files -u one | wc -l)" &&
-	test 1 -eq "$(git ls-files -u two | wc -l)" &&
-	test 1 -eq "$(git ls-files -u original | wc -l)" &&
-	test 0 -eq "$(git ls-files -o | wc -l)" &&
+	check_ls_files_count = 3 -u &&
+	check_ls_files_count = 1 -u one &&
+	check_ls_files_count = 1 -u two &&
+	check_ls_files_count = 1 -u original &&
+	check_ls_files_count = 0 -o &&
 
 	test_path_is_file one &&
 	test_path_is_file two &&
@@ -861,9 +869,11 @@ test_expect_success 'setup merge of rename + small change' '
 test_expect_success 'merge rename + small change' '
 	git merge rename_branch &&
 
-	test 1 -eq $(git ls-files -s | wc -l) &&
-	test 0 -eq $(git ls-files -o | wc -l) &&
-	test $(git rev-parse HEAD:renamed_file) = $(git rev-parse HEAD~1:file)
+	check_ls_files_count = 1 -s &&
+	check_ls_files_count = 0 -o &&
+	newhash=$(git rev-parse HEAD:renamed_file) &&
+	oldhash=$(git rev-parse HEAD~1:file) &&
+	test $newhash = $oldhash
 '
 
 test_expect_success 'setup for use of extended merge markers' '
-- 
2.32.0.278.gd42b80f139

Re: [PATCH v4 1/2] t6400: preserve git ls-files exit status code

[Eric Sunshine]

On Tue, Jun 29, 2021 at 9:57 AM Đoàn Trần Công Danh
<congdanhqx@gmail.com> wrote:
> In t6400, we're checking number of files in the index and the working
> tree by piping the output of "git ls-files" to "wc -l", thus losing the
> exit status code of git.
>
> Let's write the output of "git ls-files" to a temporary file, in order
> to check exit status code of "git ls-files" properly.

Thanks, the simplicity of this version over the previous attempts is appealing.

Just a few extremely minor style nits below; don't know if any of them
are worth a re-roll.

> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> ---
> diff --git a/t/t6400-merge-df.sh b/t/t6400-merge-df.sh
> @@ -9,6 +9,20 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>
>  . ./test-lib.sh
>
> +check_ls_files_count() {

style: funcname () {

> +       local ops val
> +       if test "$#" -le 2

I also &&-chain the `local` declaration:

    local ops val &&
    if test "$#" -le 2

By making it easy to see the `&&` upfront, when new code is inserted,
there is a better chance that the &&-chain will be kept intact:

    local ops val &&
    my new code &&
    if test "$#" -le 2

> +       then
> +               BUG "Expect 2 or more arguments"
> +       fi &&

A quick grep of the tests indicates that they are consistent about
using lowercase for the first word in a BUG():

    BUG "expect 2 or more arguments"

> +       ops="$1" &&
> +       val="$2" &&
> +       shift 2 &&
> +       mkdir -p .git/trash &&
> +       git ls-files "$@" >.git/trash/output &&
> +       test_line_count "$ops" "$val" .git/trash/output
> +}

Re: [PATCH v4 0/2] t640{0,2}: preserve ls-files exit status code

[Junio C Hamano]

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

> This is a series to clear false positive when applying Junio's suggestion to
> to a series written by Ævar [1].
>
> Not that we have any conclusion on that suggestion, just to clear the way out.

I do not think the careless and loose pattern I suggested in the old
thread has much value, so any change whose purpose is to reduce
false positive from the pattern is not needed.

But if these two patches are genuine improvement for other reasons
(like "we avoid feeding output from 'git' into pipe"), they are very
much welcome.

Thanks.

Re: [PATCH v4 1/2] t6400: preserve git ls-files exit status code

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

>> +check_ls_files_count() {
>
> style: funcname () {
> ...
> I also &&-chain the `local` declaration:
>
>     local ops val &&
>     if test "$#" -le 2
> ...
> A quick grep of the tests indicates that they are consistent about
> using lowercase for the first word in a BUG():

Thanks for a pair of sharp eyes, Eric, in your review.

> -	test 5 -eq $(git ls-files -s | wc -l) &&
> -	test 4 -eq $(git ls-files -u | wc -l) &&
> +	check_ls_files_count = 5 -s &&
> +	check_ls_files_count = 4 -u &&

I have one more comment on the main part of the patch.  It is easy
to see that this conversion is correctly done in this particular
patch from the way 5/4 and -s/u are reproduced from the preimage to
the postimage, but I doubt that readers in the future, who long have
forgotten that the "-s" came from "ls-files -s", would find the new
form easy to read and understand.

Do we have the same helper duplicated across two test scripts?

I wonder if it is worth adding a single copy that forces the callers
to spell out the command name in test-lib.sh and make the above into
something like

	test_output_wc_l = 5 ls-files -s

or even

	test_output_wc_l = 5 git ls-files -s

That way, it is easier to see what command is being run (yes, I know
you have _ls_files_ in the middle of the name of the custom helper,
but the thing is that "-s" and "_ls_files_" in the middle of the
helper are so far apart that it is not immediately obvious what the
argument "-s" is about), and by not having two identical copies, we
have less risk of them drifting apart.

Hmm?

Re: [PATCH v4 1/2] t6400: preserve git ls-files exit status code

[Eric Sunshine]

On Tue, Jun 29, 2021 at 6:49 PM Junio C Hamano <gitster@pobox.com> wrote:
> I wonder if it is worth adding a single copy that forces the callers
> to spell out the command name in test-lib.sh and make the above into
> something like
>
>         test_output_wc_l = 5 ls-files -s
>
> or even
>
>         test_output_wc_l = 5 git ls-files -s
>
> That way, it is easier to see what command is being run (yes, I know
> you have _ls_files_ in the middle of the name of the custom helper,
> but the thing is that "-s" and "_ls_files_" in the middle of the
> helper are so far apart that it is not immediately obvious what the
> argument "-s" is about), and by not having two identical copies, we
> have less risk of them drifting apart.
>
> Hmm?

I may be misunderstanding your suggestion, but isn't the proposed
test_output_wc_l() function the same as what Danh had originally
implemented several re-rolls back (though he named it
test_line_count_cmd())?

Re: [PATCH v4 1/2] t6400: preserve git ls-files exit status code

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> I may be misunderstanding your suggestion, but isn't the proposed
> test_output_wc_l() function the same as what Danh had originally
> implemented several re-rolls back (though he named it
> test_line_count_cmd())?

Could be, except that I recall we saw extra noises like --out/--err
that weren't used and contaminating the current working directory,
which are gone from the latest iteration.  The simplification
compared to that iteration is quite welcome---it made the resulting
code that uses the helper easier to follow compared to the earlier
attempts.  But this round simplifies it too much and the results got
harder to follow by burying the command name in the helper and made
it less obvious that the last part of the helper's parameters are
arguments given to ls-files, I would think.

Re: [PATCH v4 1/2] t6400: preserve git ls-files exit status code

[Đoàn Trần Công Danh]

On 2021-06-29 20:36:36-0700, Junio C Hamano <gitster@pobox.com> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> 
> > I may be misunderstanding your suggestion, but isn't the proposed
> > test_output_wc_l() function the same as what Danh had originally
> > implemented several re-rolls back (though he named it
> > test_line_count_cmd())?
> 
> Could be, except that I recall we saw extra noises like --out/--err
> that weren't used and contaminating the current working directory,
> which are gone from the latest iteration.

Yes, in v{1,2}, there's the extra noises of --out and --err,
but it's gone in v3.

I guess you're thinking about the contamination of $PWD iff it's not
a git worktree. That could be simplified by BUG-ging if $PWD is not
a git worktree.

Maybe, you're thinking about the extra noises to handle the failure run
of command under check? That could be dropped, too.

Would you mind looking at v3 1/4 again to see what is your opinion
there? I don't mind re-roll a same or simplified version of v3,
with s/test_line_count_cmd/test_output_wc_l/ if you see fit.

> The simplification
> compared to that iteration is quite welcome---it made the resulting
> code that uses the helper easier to follow compared to the earlier
> attempts.  But this round simplifies it too much and the results got
> harder to follow by burying the command name in the helper and made
> it less obvious that the last part of the helper's parameters are
> arguments given to ls-files, I would think.

-- 
Danh

Re: [PATCH v4 1/2] t6400: preserve git ls-files exit status code

[Junio C Hamano]

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

>> Could be, except that I recall we saw extra noises like --out/--err
>> that weren't used and contaminating the current working directory,
>> which are gone from the latest iteration.
>
> Yes, in v{1,2}, there's the extra noises of --out and --err,
> but it's gone in v3.
>
> I guess you're thinking about the contamination of $PWD iff it's not
> a git worktree. That could be simplified by BUG-ging if $PWD is not
> a git worktree.

No.  I am not thinking about that.  I do not think it is a big loss
if the helper cannot be used in non-repository.

> Maybe, you're thinking about the extra noises to handle the failure run
> of command under check? That could be dropped, too.

No.  I am not thinking about that, either.

> Would you mind looking at v3 1/4 again to see what is your opinion
> there? I don't mind re-roll a same or simplified version of v3,
> with s/test_line_count_cmd/test_output_wc_l/ if you see fit.

Let's not go back that far.  This is taken from v4 (t/t6400) ...

	local ops val &&
	if test "$#" -le 2
	then
		BUG "Expect 2 or more arguments"
	fi &&
	ops="$1" &&
	val="$2" &&
	shift 2 &&
	mkdir -p .git/trash &&
	"$@" >.git/trash/output &&
	test_line_count "$ops" "$val" .git/trash/output

... except that it runs '"$@"' instead of 'git ls-files "$@"', so
that we could try running things other than ls-files, and that would
be mostly good enough.  We may want to be prepared for a caller who
wants to use the helper from within a subdirectory by not hardcoding
".git/trash", though.  Something along the lines of ...

	local ops val &&
+	local trashdir=$(git rev-parse --git-dir)/trash &&
	if test ...
	...
-	mkdir -p .git/trash &&
-	"$@" >".git/trash/output" &&
-	test_line_count "$ops" "$val" .git/trash/output
+	mkdir -p "$trashdir" &&
+	"$@" >"$trashdir/output" &&
+	test_line_count "$ops" "$val" "$trashdir/output"

[PATCH v5 0/3] new test-libs-function: test_stdout_line_count

[Đoàn Trần Công Danh]

This series was started to clear false positive when applying
Junio's suggestion to to a series written by Ævar [1].

Despite, we don't have much interest in that direction.
I think it's still an improvement with this series, namely,
we're able to always check Git's exit status code.

A naive: "git grep -l  'git.* | wc -l' t????-* |wc -l" reveals
38 other tests also have that pattern.
Those tests could be cleaned up later on

Change in v5:
- restore (albeit with new name and simplified) test_stdout_line_count
- trash files will be written in $(git rev-parse --git-dir)/trash

In v4, I dropped the test_line_count_cmd completely.
A local to t640{0,2} helper was written instead.

Đoàn Trần Công Danh (3):
  test-lib-functions: introduce test_stdout_line_count
  t6400: preserve git ls-files exit status code
  t6402: preserve git exit status code

 t/t6400-merge-df.sh     |  16 ++---
 t/t6402-merge-rename.sh | 132 +++++++++++++++++++---------------------
 t/test-lib-functions.sh |  26 ++++++++
 3 files changed, 98 insertions(+), 76 deletions(-)

Range-diff against v4:
1:  49104273b8 < -:  ---------- t6400: preserve git ls-files exit status code
-:  ---------- > 1:  ab542ae9aa test-lib-functions: introduce test_stdout_line_count
-:  ---------- > 2:  f7a06994cd t6400: preserve git ls-files exit status code
2:  6d3ebcb255 ! 3:  f8ed19f858 t6402: preserve git exit status code
    @@ Commit message
         t6402: preserve git exit status code
     
         In t6402, we're checking number of files in the index and the working
    -    tree by piping the output of "git ls-files" to "wc -l", thus losing the
    +    tree by piping the output of Git's command to "wc -l", thus losing the
         exit status code of git.
     
    -    Let's write the output of "git ls-files" to a temporary file, in order
    -    to check exit status code of "git ls-files" properly.
    -
    -    While we're at it, also check exit status code of an invocation of
    -    git-rev-parse, too.
    +    Let's use the new helper test_stdout_line_count in order to preserve
    +    Git's exit status code.
     
         Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
     
      ## t/t6402-merge-rename.sh ##
    -@@ t/t6402-merge-rename.sh: modify () {
    - 	mv "$2.x" "$2"
    - }
    - 
    -+check_ls_files_count() {
    -+	local ops val
    -+	if test "$#" -le 2
    -+	then
    -+		BUG "Expect 2 or more arguments"
    -+	fi &&
    -+	ops="$1" &&
    -+	val="$2" &&
    -+	shift 2 &&
    -+	mkdir -p .git/trash &&
    -+	git ls-files "$@" >.git/trash/output &&
    -+	test_line_count "$ops" "$val" .git/trash/output
    -+}
    -+
    - test_expect_success 'setup' '
    - 	cat >A <<-\EOF &&
    - 	a aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
     @@ t/t6402-merge-rename.sh: test_expect_success 'pull renaming branch into unrenaming one' \
      	git show-branch &&
      	test_expect_code 1 git pull . white &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'pull renaming branch into unrenami
     -	test_line_count = 3 b.stages &&
     -	git ls-files -s N >n.stages &&
     -	test_line_count = 1 n.stages &&
    -+	check_ls_files_count = 3 -u B &&
    -+	check_ls_files_count = 1 -s N &&
    ++	test_stdout_line_count = 3 git ls-files -u B &&
    ++	test_stdout_line_count = 1 git ls-files -s N &&
      	sed -ne "/^g/{
      	p
      	q
    @@ t/t6402-merge-rename.sh: test_expect_success 'pull renaming branch into another
     -	test_line_count = 3 b.stages &&
     -	git ls-files -s N >n.stages &&
     -	test_line_count = 1 n.stages &&
    -+	check_ls_files_count = 3 -u B &&
    -+	check_ls_files_count = 1 -s N &&
    ++	test_stdout_line_count = 3 git ls-files -u B &&
    ++	test_stdout_line_count = 1 git ls-files -s N &&
      	sed -ne "/^g/{
      	p
      	q
    @@ t/t6402-merge-rename.sh: test_expect_success 'pull unrenaming branch into renami
     -	test_line_count = 3 b.stages &&
     -	git ls-files -s N >n.stages &&
     -	test_line_count = 1 n.stages &&
    -+	check_ls_files_count = 3 -u B &&
    -+	check_ls_files_count = 1 -s N &&
    ++	test_stdout_line_count = 3 git ls-files -u B &&
    ++	test_stdout_line_count = 1 git ls-files -s N &&
      	sed -ne "/^g/{
      	p
      	q
    @@ t/t6402-merge-rename.sh: test_expect_success 'pull conflicting renames' \
     -	test_line_count = 1 c.stages &&
     -	git ls-files -s N >n.stages &&
     -	test_line_count = 1 n.stages &&
    -+	check_ls_files_count = 1 -u A &&
    -+	check_ls_files_count = 1 -u B &&
    -+	check_ls_files_count = 1 -u C &&
    -+	check_ls_files_count = 1 -s N &&
    ++	test_stdout_line_count = 1 git ls-files -u A &&
    ++	test_stdout_line_count = 1 git ls-files -u B &&
    ++	test_stdout_line_count = 1 git ls-files -u C &&
    ++	test_stdout_line_count = 1 git ls-files -s N &&
      	sed -ne "/^g/{
      	p
      	q
    @@ t/t6402-merge-rename.sh: test_expect_success 'Rename+D/F conflict; renamed file
      
     -	test 3 -eq "$(git ls-files -u | wc -l)" &&
     -	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
    -+	check_ls_files_count = 3 -u &&
    -+	check_ls_files_count = 2 -u dir/file-in-the-way &&
    ++	test_stdout_line_count = 3 git ls-files -u &&
    ++	test_stdout_line_count = 2 git ls-files -u dir/file-in-the-way &&
      
      	test_must_fail git diff --quiet &&
      	test_must_fail git diff --cached --quiet &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'Same as previous, but merged other
      
     -	test 3 -eq "$(git ls-files -u | wc -l)" &&
     -	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
    -+	check_ls_files_count = 3 -u &&
    -+	check_ls_files_count = 2 -u dir/file-in-the-way &&
    ++	test_stdout_line_count = 3 git ls-files -u &&
    ++	test_stdout_line_count = 2 git ls-files -u dir/file-in-the-way &&
      
      	test_must_fail git diff --quiet &&
      	test_must_fail git diff --cached --quiet &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'Rename+D/F conflict; renamed file
      
     -	test 3 -eq "$(git ls-files -u | wc -l)" &&
     -	test 3 -eq "$(git ls-files -u dir | wc -l)" &&
    -+	check_ls_files_count = 3 -u &&
    -+	check_ls_files_count = 3 -u dir &&
    ++	test_stdout_line_count = 3 git ls-files -u &&
    ++	test_stdout_line_count = 3 git ls-files -u dir &&
      
      	test_must_fail git diff --quiet &&
      	test_must_fail git diff --cached --quiet &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'Rename+D/F conflict; renamed file
      	test_must_fail git merge --strategy=recursive dir-in-way &&
      
     -	test 5 -eq "$(git ls-files -u | wc -l)" &&
    -+	check_ls_files_count = 5 -u &&
    ++	test_stdout_line_count = 5 git ls-files -u &&
      	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
      	then
     -		test 3 -eq "$(git ls-files -u dir~HEAD | wc -l)"
    -+		check_ls_files_count = 3 -u dir~HEAD
    ++		test_stdout_line_count = 3 git ls-files -u dir~HEAD
      	else
     -		test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)"
     +		git ls-files -u dir >out &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'Rename+D/F conflict; renamed file
     +		rm -f out
      	fi &&
     -	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
    -+	check_ls_files_count = 2 -u dir/file-in-the-way &&
    ++	test_stdout_line_count = 2 git ls-files -u dir/file-in-the-way &&
      
      	test_must_fail git diff --quiet &&
      	test_must_fail git diff --cached --quiet &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'Same as previous, but merged other
      	test_must_fail git merge --strategy=recursive renamed-file-has-conflicts &&
      
     -	test 5 -eq "$(git ls-files -u | wc -l)" &&
    -+	check_ls_files_count = 5 -u &&
    ++	test_stdout_line_count = 5 git ls-files -u &&
      	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
      	then
     -		test 3 -eq "$(git ls-files -u dir~renamed-file-has-conflicts | wc -l)"
    -+		check_ls_files_count = 3 -u dir~renamed-file-has-conflicts
    ++		test_stdout_line_count = 3 git ls-files -u dir~renamed-file-has-conflicts
      	else
     -		test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)"
     +		git ls-files -u dir >out &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'Same as previous, but merged other
     +		rm -f out
      	fi &&
     -	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
    -+	check_ls_files_count = 2 -u dir/file-in-the-way &&
    ++	test_stdout_line_count = 2 git ls-files -u dir/file-in-the-way &&
      
      	test_must_fail git diff --quiet &&
      	test_must_fail git diff --cached --quiet &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'both rename source and destination
      	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
      	then
     -		test 2 -eq "$(git ls-files -u | wc -l)"
    -+		check_ls_files_count = 2 -u
    ++		test_stdout_line_count = 2 git ls-files -u
      	else
     -		test 1 -eq "$(git ls-files -u | wc -l)"
    -+		check_ls_files_count = 1 -u
    ++		test_stdout_line_count = 1 git ls-files -u
      	fi &&
      
      	test_must_fail git diff --quiet &&
    @@ t/t6402-merge-rename.sh: then
     -		test 4 -eq "$(git ls-files -u | wc -l)" &&
     -		test 2 -eq "$(git ls-files -u one | wc -l)" &&
     -		test 2 -eq "$(git ls-files -u two | wc -l)" &&
    -+		check_ls_files_count = 4 -u &&
    -+		check_ls_files_count = 2 -u one &&
    -+		check_ls_files_count = 2 -u two &&
    ++		test_stdout_line_count = 4 git ls-files -u &&
    ++		test_stdout_line_count = 2 git ls-files -u one &&
    ++		test_stdout_line_count = 2 git ls-files -u two &&
      
      		test_must_fail git diff --quiet &&
      
    @@ t/t6402-merge-rename.sh: else
     -		test 2 -eq "$(git ls-files -u | wc -l)" &&
     -		test 1 -eq "$(git ls-files -u one | wc -l)" &&
     -		test 1 -eq "$(git ls-files -u two | wc -l)" &&
    -+		check_ls_files_count = 2 -u &&
    -+		check_ls_files_count = 1 -u one &&
    -+		check_ls_files_count = 1 -u two &&
    ++		test_stdout_line_count = 2 git ls-files -u &&
    ++		test_stdout_line_count = 1 git ls-files -u one &&
    ++		test_stdout_line_count = 1 git ls-files -u two &&
      
      		test_must_fail git diff --quiet &&
      
    @@ t/t6402-merge-rename.sh: test_expect_success 'pair rename to parent of other (D/
     -		test 4 -eq "$(git ls-files -u | wc -l)" &&
     -		test 2 -eq "$(git ls-files -u one | wc -l)" &&
     -		test 2 -eq "$(git ls-files -u two | wc -l)"
    -+		check_ls_files_count = 4 -u &&
    -+		check_ls_files_count = 2 -u one &&
    -+		check_ls_files_count = 2 -u two
    ++		test_stdout_line_count = 4 git ls-files -u &&
    ++		test_stdout_line_count = 2 git ls-files -u one &&
    ++		test_stdout_line_count = 2 git ls-files -u two
      	else
     -		test 2 -eq "$(git ls-files -u | wc -l)" &&
     -		test 1 -eq "$(git ls-files -u one | wc -l)" &&
     -		test 1 -eq "$(git ls-files -u two | wc -l)"
    -+		check_ls_files_count = 2 -u &&
    -+		check_ls_files_count = 1 -u one &&
    -+		check_ls_files_count = 1 -u two
    ++		test_stdout_line_count = 2 git ls-files -u &&
    ++		test_stdout_line_count = 1 git ls-files -u one &&
    ++		test_stdout_line_count = 1 git ls-files -u two
      	fi &&
      
      	test_must_fail git diff --quiet &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'check handling of differently rena
     -		test 1 -eq "$(git ls-files -u two~second-rename | wc -l)" &&
     -		test 1 -eq "$(git ls-files -u original | wc -l)" &&
     -		test 0 -eq "$(git ls-files -o | wc -l)"
    -+		check_ls_files_count = 5 -s &&
    -+		check_ls_files_count = 3 -u &&
    -+		check_ls_files_count = 1 -u one~HEAD &&
    -+		check_ls_files_count = 1 -u two~second-rename &&
    -+		check_ls_files_count = 1 -u original &&
    -+		check_ls_files_count = 0 -o
    ++		test_stdout_line_count = 5 git ls-files -s &&
    ++		test_stdout_line_count = 3 git ls-files -u &&
    ++		test_stdout_line_count = 1 git ls-files -u one~HEAD &&
    ++		test_stdout_line_count = 1 git ls-files -u two~second-rename &&
    ++		test_stdout_line_count = 1 git ls-files -u original &&
    ++		test_stdout_line_count = 0 git ls-files -o
      	else
     -		test 5 -eq "$(git ls-files -s | wc -l)" &&
     -		test 3 -eq "$(git ls-files -u | wc -l)" &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'check handling of differently rena
     -		test 1 -eq "$(git ls-files -u two | wc -l)" &&
     -		test 1 -eq "$(git ls-files -u original | wc -l)" &&
     -		test 2 -eq "$(git ls-files -o | wc -l)"
    -+		check_ls_files_count = 5 -s &&
    -+		check_ls_files_count = 3 -u &&
    -+		check_ls_files_count = 1 -u one &&
    -+		check_ls_files_count = 1 -u two &&
    -+		check_ls_files_count = 1 -u original &&
    -+		check_ls_files_count = 2 -o
    ++		test_stdout_line_count = 5 git ls-files -s &&
    ++		test_stdout_line_count = 3 git ls-files -u &&
    ++		test_stdout_line_count = 1 git ls-files -u one &&
    ++		test_stdout_line_count = 1 git ls-files -u two &&
    ++		test_stdout_line_count = 1 git ls-files -u original &&
    ++		test_stdout_line_count = 2 git ls-files -o
      	fi &&
      
      	test_path_is_file one/file &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'check handling of differently rena
     -	test 1 -eq "$(git ls-files -u two | wc -l)" &&
     -	test 1 -eq "$(git ls-files -u original | wc -l)" &&
     -	test 0 -eq "$(git ls-files -o | wc -l)" &&
    -+	check_ls_files_count = 3 -u &&
    -+	check_ls_files_count = 1 -u one &&
    -+	check_ls_files_count = 1 -u two &&
    -+	check_ls_files_count = 1 -u original &&
    -+	check_ls_files_count = 0 -o &&
    ++	test_stdout_line_count = 3 git ls-files -u &&
    ++	test_stdout_line_count = 1 git ls-files -u one &&
    ++	test_stdout_line_count = 1 git ls-files -u two &&
    ++	test_stdout_line_count = 1 git ls-files -u original &&
    ++	test_stdout_line_count = 0 git ls-files -o &&
      
      	test_path_is_file one &&
      	test_path_is_file two &&
    @@ t/t6402-merge-rename.sh: test_expect_success 'setup merge of rename + small chan
     -	test 1 -eq $(git ls-files -s | wc -l) &&
     -	test 0 -eq $(git ls-files -o | wc -l) &&
     -	test $(git rev-parse HEAD:renamed_file) = $(git rev-parse HEAD~1:file)
    -+	check_ls_files_count = 1 -s &&
    -+	check_ls_files_count = 0 -o &&
    ++	test_stdout_line_count = 1 git ls-files -s &&
    ++	test_stdout_line_count = 0 git ls-files -o &&
     +	newhash=$(git rev-parse HEAD:renamed_file) &&
     +	oldhash=$(git rev-parse HEAD~1:file) &&
     +	test $newhash = $oldhash
-- 
2.32.0.278.gd42b80f139

[PATCH v5 1/3] test-lib-functions: introduce test_stdout_line_count

[Đoàn Trần Công Danh]

In some tests, we're checking the number of lines in output of some
commands, including but not limited to Git's command.

We're doing the check by running those commands in the left side of
a pipe, thus losing the exit status code of those commands. Meanwhile,
we really want to check the exit status code of Git's command.

Let's write the output of those commands to a temporary file, and use
test_line_count separately in order to check exit status code of
those commands properly.

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

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index f0448daa74..f9505be8fe 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -845,6 +845,32 @@ test_line_count () {
 	fi
 }
 
+# SYNOPSIS:
+# 	test_stdout_line_count <bin-ops> <value> <cmd> [<args>...]
+#
+# test_stdout_line_count checks that the output of a command has the number
+# of lines it ought to. For example:
+# 
+# test_stdout_line_count = 3 git ls-files -u
+# test_stdout_line_count -gt 10 ls
+test_stdout_line_count () {
+	local ops val trashdir &&
+	if test "$#" -le 3
+	then
+		BUG "expect 3 or more arguments"
+	fi &&
+	ops="$1" &&
+	val="$2" &&
+	shift 2 &&
+	if ! trashdir="$(git rev-parse --git-dir)/trash"; then
+		BUG "expect to be run inside a worktree"
+	fi &&
+	mkdir -p "$trashdir" &&
+	"$@" >"$trashdir/output" &&
+	test_line_count "$ops" "$val" "$trashdir/output"
+}
+
+
 test_file_size () {
 	test "$#" -ne 1 && BUG "1 param"
 	test-tool path-utils file-size "$1"
-- 
2.32.0.278.gd42b80f139

[PATCH v5 2/3] t6400: preserve git ls-files exit status code

[Đoàn Trần Công Danh]

In t6400, we're checking number of files in the index and the working
tree by piping the output of "git ls-files" to "wc -l", thus losing the
exit status code of git.

Let's use the newly introduced test_stdout_line_count in order to check
the exit status code of Git's command.

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 t/t6400-merge-df.sh | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/t/t6400-merge-df.sh b/t/t6400-merge-df.sh
index 38700d29b5..57a67cf362 100755
--- a/t/t6400-merge-df.sh
+++ b/t/t6400-merge-df.sh
@@ -82,13 +82,13 @@ test_expect_success 'modify/delete + directory/file conflict' '
 	git checkout delete^0 &&
 	test_must_fail git merge modify &&
 
-	test 5 -eq $(git ls-files -s | wc -l) &&
-	test 4 -eq $(git ls-files -u | wc -l) &&
+	test_stdout_line_count = 5 git ls-files -s &&
+	test_stdout_line_count = 4 git ls-files -u &&
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 0 -eq $(git ls-files -o | wc -l)
+		test_stdout_line_count = 0 git ls-files -o
 	else
-		test 1 -eq $(git ls-files -o | wc -l)
+		test_stdout_line_count = 1 git ls-files -o
 	fi &&
 
 	test_path_is_file letters/file &&
@@ -103,13 +103,13 @@ test_expect_success 'modify/delete + directory/file conflict; other way' '
 
 	test_must_fail git merge delete &&
 
-	test 5 -eq $(git ls-files -s | wc -l) &&
-	test 4 -eq $(git ls-files -u | wc -l) &&
+	test_stdout_line_count = 5 git ls-files -s &&
+	test_stdout_line_count = 4 git ls-files -u &&
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 0 -eq $(git ls-files -o | wc -l)
+		test_stdout_line_count = 0 git ls-files -o
 	else
-		test 1 -eq $(git ls-files -o | wc -l)
+		test_stdout_line_count = 1 git ls-files -o
 	fi &&
 
 	test_path_is_file letters/file &&
-- 
2.32.0.278.gd42b80f139

[PATCH v5 3/3] t6402: preserve git exit status code

[Đoàn Trần Công Danh]

In t6402, we're checking number of files in the index and the working
tree by piping the output of Git's command to "wc -l", thus losing the
exit status code of git.

Let's use the new helper test_stdout_line_count in order to preserve
Git's exit status code.

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 t/t6402-merge-rename.sh | 132 +++++++++++++++++++---------------------
 1 file changed, 64 insertions(+), 68 deletions(-)

diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh
index 425dad97d5..3da2896e3b 100755
--- a/t/t6402-merge-rename.sh
+++ b/t/t6402-merge-rename.sh
@@ -105,10 +105,8 @@ test_expect_success 'pull renaming branch into unrenaming one' \
 	git show-branch &&
 	test_expect_code 1 git pull . white &&
 	git ls-files -s &&
-	git ls-files -u B >b.stages &&
-	test_line_count = 3 b.stages &&
-	git ls-files -s N >n.stages &&
-	test_line_count = 1 n.stages &&
+	test_stdout_line_count = 3 git ls-files -u B &&
+	test_stdout_line_count = 1 git ls-files -s N &&
 	sed -ne "/^g/{
 	p
 	q
@@ -122,10 +120,8 @@ test_expect_success 'pull renaming branch into another renaming one' \
 	git reset --hard &&
 	git checkout red &&
 	test_expect_code 1 git pull . white &&
-	git ls-files -u B >b.stages &&
-	test_line_count = 3 b.stages &&
-	git ls-files -s N >n.stages &&
-	test_line_count = 1 n.stages &&
+	test_stdout_line_count = 3 git ls-files -u B &&
+	test_stdout_line_count = 1 git ls-files -s N &&
 	sed -ne "/^g/{
 	p
 	q
@@ -138,10 +134,8 @@ test_expect_success 'pull unrenaming branch into renaming one' \
 	git reset --hard &&
 	git show-branch &&
 	test_expect_code 1 git pull . main &&
-	git ls-files -u B >b.stages &&
-	test_line_count = 3 b.stages &&
-	git ls-files -s N >n.stages &&
-	test_line_count = 1 n.stages &&
+	test_stdout_line_count = 3 git ls-files -u B &&
+	test_stdout_line_count = 1 git ls-files -s N &&
 	sed -ne "/^g/{
 	p
 	q
@@ -154,14 +148,10 @@ test_expect_success 'pull conflicting renames' \
 	git reset --hard &&
 	git show-branch &&
 	test_expect_code 1 git pull . blue &&
-	git ls-files -u A >a.stages &&
-	test_line_count = 1 a.stages &&
-	git ls-files -u B >b.stages &&
-	test_line_count = 1 b.stages &&
-	git ls-files -u C >c.stages &&
-	test_line_count = 1 c.stages &&
-	git ls-files -s N >n.stages &&
-	test_line_count = 1 n.stages &&
+	test_stdout_line_count = 1 git ls-files -u A &&
+	test_stdout_line_count = 1 git ls-files -u B &&
+	test_stdout_line_count = 1 git ls-files -u C &&
+	test_stdout_line_count = 1 git ls-files -s N &&
 	sed -ne "/^g/{
 	p
 	q
@@ -330,8 +320,8 @@ test_expect_success 'Rename+D/F conflict; renamed file merges but dir in way' '
 		test_i18ngrep "Adding as dir~HEAD instead" output
 	fi &&
 
-	test 3 -eq "$(git ls-files -u | wc -l)" &&
-	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
+	test_stdout_line_count = 3 git ls-files -u &&
+	test_stdout_line_count = 2 git ls-files -u dir/file-in-the-way &&
 
 	test_must_fail git diff --quiet &&
 	test_must_fail git diff --cached --quiet &&
@@ -357,8 +347,8 @@ test_expect_success 'Same as previous, but merged other way' '
 		test_i18ngrep "Adding as dir~renamed-file-has-no-conflicts instead" output
 	fi &&
 
-	test 3 -eq "$(git ls-files -u | wc -l)" &&
-	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
+	test_stdout_line_count = 3 git ls-files -u &&
+	test_stdout_line_count = 2 git ls-files -u dir/file-in-the-way &&
 
 	test_must_fail git diff --quiet &&
 	test_must_fail git diff --cached --quiet &&
@@ -374,8 +364,8 @@ test_expect_success 'Rename+D/F conflict; renamed file cannot merge, dir not in
 	git checkout -q renamed-file-has-conflicts^0 &&
 	test_must_fail git merge --strategy=recursive dir-not-in-way &&
 
-	test 3 -eq "$(git ls-files -u | wc -l)" &&
-	test 3 -eq "$(git ls-files -u dir | wc -l)" &&
+	test_stdout_line_count = 3 git ls-files -u &&
+	test_stdout_line_count = 3 git ls-files -u dir &&
 
 	test_must_fail git diff --quiet &&
 	test_must_fail git diff --cached --quiet &&
@@ -409,14 +399,16 @@ test_expect_success 'Rename+D/F conflict; renamed file cannot merge and dir in t
 	git checkout -q renamed-file-has-conflicts^0 &&
 	test_must_fail git merge --strategy=recursive dir-in-way &&
 
-	test 5 -eq "$(git ls-files -u | wc -l)" &&
+	test_stdout_line_count = 5 git ls-files -u &&
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 3 -eq "$(git ls-files -u dir~HEAD | wc -l)"
+		test_stdout_line_count = 3 git ls-files -u dir~HEAD
 	else
-		test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)"
+		git ls-files -u dir >out &&
+		test 3 -eq $(grep -v file-in-the-way out | wc -l) &&
+		rm -f out
 	fi &&
-	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
+	test_stdout_line_count = 2 git ls-files -u dir/file-in-the-way &&
 
 	test_must_fail git diff --quiet &&
 	test_must_fail git diff --cached --quiet &&
@@ -432,14 +424,16 @@ test_expect_success 'Same as previous, but merged other way' '
 	git checkout -q dir-in-way^0 &&
 	test_must_fail git merge --strategy=recursive renamed-file-has-conflicts &&
 
-	test 5 -eq "$(git ls-files -u | wc -l)" &&
+	test_stdout_line_count = 5 git ls-files -u &&
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 3 -eq "$(git ls-files -u dir~renamed-file-has-conflicts | wc -l)"
+		test_stdout_line_count = 3 git ls-files -u dir~renamed-file-has-conflicts
 	else
-		test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)"
+		git ls-files -u dir >out &&
+		test 3 -eq $(grep -v file-in-the-way out | wc -l) &&
+		rm -f out
 	fi &&
-	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
+	test_stdout_line_count = 2 git ls-files -u dir/file-in-the-way &&
 
 	test_must_fail git diff --quiet &&
 	test_must_fail git diff --cached --quiet &&
@@ -496,9 +490,9 @@ test_expect_success 'both rename source and destination involved in D/F conflict
 
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 2 -eq "$(git ls-files -u | wc -l)"
+		test_stdout_line_count = 2 git ls-files -u
 	else
-		test 1 -eq "$(git ls-files -u | wc -l)"
+		test_stdout_line_count = 1 git ls-files -u
 	fi &&
 
 	test_must_fail git diff --quiet &&
@@ -540,9 +534,9 @@ then
 		mkdir one &&
 		test_must_fail git merge --strategy=recursive rename-two &&
 
-		test 4 -eq "$(git ls-files -u | wc -l)" &&
-		test 2 -eq "$(git ls-files -u one | wc -l)" &&
-		test 2 -eq "$(git ls-files -u two | wc -l)" &&
+		test_stdout_line_count = 4 git ls-files -u &&
+		test_stdout_line_count = 2 git ls-files -u one &&
+		test_stdout_line_count = 2 git ls-files -u two &&
 
 		test_must_fail git diff --quiet &&
 
@@ -559,9 +553,9 @@ else
 		mkdir one &&
 		test_must_fail git merge --strategy=recursive rename-two &&
 
-		test 2 -eq "$(git ls-files -u | wc -l)" &&
-		test 1 -eq "$(git ls-files -u one | wc -l)" &&
-		test 1 -eq "$(git ls-files -u two | wc -l)" &&
+		test_stdout_line_count = 2 git ls-files -u &&
+		test_stdout_line_count = 1 git ls-files -u one &&
+		test_stdout_line_count = 1 git ls-files -u two &&
 
 		test_must_fail git diff --quiet &&
 
@@ -582,13 +576,13 @@ test_expect_success 'pair rename to parent of other (D/F conflicts) w/ clean sta
 
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 4 -eq "$(git ls-files -u | wc -l)" &&
-		test 2 -eq "$(git ls-files -u one | wc -l)" &&
-		test 2 -eq "$(git ls-files -u two | wc -l)"
+		test_stdout_line_count = 4 git ls-files -u &&
+		test_stdout_line_count = 2 git ls-files -u one &&
+		test_stdout_line_count = 2 git ls-files -u two
 	else
-		test 2 -eq "$(git ls-files -u | wc -l)" &&
-		test 1 -eq "$(git ls-files -u one | wc -l)" &&
-		test 1 -eq "$(git ls-files -u two | wc -l)"
+		test_stdout_line_count = 2 git ls-files -u &&
+		test_stdout_line_count = 1 git ls-files -u one &&
+		test_stdout_line_count = 1 git ls-files -u two
 	fi &&
 
 	test_must_fail git diff --quiet &&
@@ -631,19 +625,19 @@ test_expect_success 'check handling of differently renamed file with D/F conflic
 
 	if test "$GIT_TEST_MERGE_ALGORITHM" = ort
 	then
-		test 5 -eq "$(git ls-files -s | wc -l)" &&
-		test 3 -eq "$(git ls-files -u | wc -l)" &&
-		test 1 -eq "$(git ls-files -u one~HEAD | wc -l)" &&
-		test 1 -eq "$(git ls-files -u two~second-rename | wc -l)" &&
-		test 1 -eq "$(git ls-files -u original | wc -l)" &&
-		test 0 -eq "$(git ls-files -o | wc -l)"
+		test_stdout_line_count = 5 git ls-files -s &&
+		test_stdout_line_count = 3 git ls-files -u &&
+		test_stdout_line_count = 1 git ls-files -u one~HEAD &&
+		test_stdout_line_count = 1 git ls-files -u two~second-rename &&
+		test_stdout_line_count = 1 git ls-files -u original &&
+		test_stdout_line_count = 0 git ls-files -o
 	else
-		test 5 -eq "$(git ls-files -s | wc -l)" &&
-		test 3 -eq "$(git ls-files -u | wc -l)" &&
-		test 1 -eq "$(git ls-files -u one | wc -l)" &&
-		test 1 -eq "$(git ls-files -u two | wc -l)" &&
-		test 1 -eq "$(git ls-files -u original | wc -l)" &&
-		test 2 -eq "$(git ls-files -o | wc -l)"
+		test_stdout_line_count = 5 git ls-files -s &&
+		test_stdout_line_count = 3 git ls-files -u &&
+		test_stdout_line_count = 1 git ls-files -u one &&
+		test_stdout_line_count = 1 git ls-files -u two &&
+		test_stdout_line_count = 1 git ls-files -u original &&
+		test_stdout_line_count = 2 git ls-files -o
 	fi &&
 
 	test_path_is_file one/file &&
@@ -679,11 +673,11 @@ test_expect_success 'check handling of differently renamed file with D/F conflic
 	git checkout -q first-rename-redo^0 &&
 	test_must_fail git merge --strategy=recursive second-rename-redo &&
 
-	test 3 -eq "$(git ls-files -u | wc -l)" &&
-	test 1 -eq "$(git ls-files -u one | wc -l)" &&
-	test 1 -eq "$(git ls-files -u two | wc -l)" &&
-	test 1 -eq "$(git ls-files -u original | wc -l)" &&
-	test 0 -eq "$(git ls-files -o | wc -l)" &&
+	test_stdout_line_count = 3 git ls-files -u &&
+	test_stdout_line_count = 1 git ls-files -u one &&
+	test_stdout_line_count = 1 git ls-files -u two &&
+	test_stdout_line_count = 1 git ls-files -u original &&
+	test_stdout_line_count = 0 git ls-files -o &&
 
 	test_path_is_file one &&
 	test_path_is_file two &&
@@ -861,9 +855,11 @@ test_expect_success 'setup merge of rename + small change' '
 test_expect_success 'merge rename + small change' '
 	git merge rename_branch &&
 
-	test 1 -eq $(git ls-files -s | wc -l) &&
-	test 0 -eq $(git ls-files -o | wc -l) &&
-	test $(git rev-parse HEAD:renamed_file) = $(git rev-parse HEAD~1:file)
+	test_stdout_line_count = 1 git ls-files -s &&
+	test_stdout_line_count = 0 git ls-files -o &&
+	newhash=$(git rev-parse HEAD:renamed_file) &&
+	oldhash=$(git rev-parse HEAD~1:file) &&
+	test $newhash = $oldhash
 '
 
 test_expect_success 'setup for use of extended merge markers' '
-- 
2.32.0.278.gd42b80f139

Re: [PATCH v5 1/3] test-lib-functions: introduce test_stdout_line_count

[Eric Sunshine]

On Sun, Jul 4, 2021 at 1:46 AM Đoàn Trần Công Danh <congdanhqx@gmail.com> wrote:
> In some tests, we're checking the number of lines in output of some
> commands, including but not limited to Git's command.
>
> We're doing the check by running those commands in the left side of
> a pipe, thus losing the exit status code of those commands. Meanwhile,
> we really want to check the exit status code of Git's command.
>
> Let's write the output of those commands to a temporary file, and use
> test_line_count separately in order to check exit status code of
> those commands properly.
>
> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> ---
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> @@ -845,6 +845,32 @@ test_line_count () {
> +# SYNOPSIS:
> +#      test_stdout_line_count <bin-ops> <value> <cmd> [<args>...]
> +#
> +# test_stdout_line_count checks that the output of a command has the number
> +# of lines it ought to. For example:
> +#
> +# test_stdout_line_count = 3 git ls-files -u
> +# test_stdout_line_count -gt 10 ls
> +test_stdout_line_count () {
> +       local ops val trashdir &&
> +       if test "$#" -le 3
> +       then
> +               BUG "expect 3 or more arguments"
> +       fi &&
> +       ops="$1" &&
> +       val="$2" &&
> +       shift 2 &&
> +       if ! trashdir="$(git rev-parse --git-dir)/trash"; then
> +               BUG "expect to be run inside a worktree"
> +       fi &&
> +       mkdir -p "$trashdir" &&
> +       "$@" >"$trashdir/output" &&
> +       test_line_count "$ops" "$val" "$trashdir/output"
> +}
> +
> +
>  test_file_size () {

Nit: one too many blank lines after the test body.

A minor think-out-loud: I wonder if there would be value in deriving
the name of the output file from the command being run (perhaps by
using `tr` to translate oddball characters to underscore or to fold
them out). This might or might not help someone debugging a test
failure since there would be less chance of "$trashdir/output" being
repeatedly clobbered. Anyhow, it's something that could be done later
if deemed useful, not something for the present series. (I'm not
interested in seeing this series re-rolled endlessly.)

Re: [PATCH v5 1/3] test-lib-functions: introduce test_stdout_line_count

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> A minor think-out-loud: I wonder if there would be value in deriving
> the name of the output file from the command being run (perhaps by
> using `tr` to translate oddball characters to underscore or to fold
> them out). This might or might not help someone debugging a test
> failure since there would be less chance of "$trashdir/output" being
> repeatedly clobbered.

Probably not.

The iterations of output that are clobbered are all from the passing
call to test_stdout_count_lines helper we made previously.

Re: [PATCH v5 1/3] test-lib-functions: introduce test_stdout_line_count

[Đoàn Trần Công Danh]

On 2021-07-06 12:24:05-0700, Junio C Hamano <gitster@pobox.com> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> 
> > A minor think-out-loud: I wonder if there would be value in deriving
> > the name of the output file from the command being run (perhaps by
> > using `tr` to translate oddball characters to underscore or to fold
> > them out). This might or might not help someone debugging a test
> > failure since there would be less chance of "$trashdir/output" being
> > repeatedly clobbered.
> 
> Probably not.
> 
> The iterations of output that are clobbered are all from the passing
> call to test_stdout_count_lines helper we made previously.

Yay, I also think it doesn't add much value. Since we're chaining
command in a single test-case.

I think most people with try to debug with "-i", which exits
immediately.

The only place it would help is debugging test failures with GitHub
Actions, where developers could download artifarts for test failures
from GitHub Actions.

-- 
Danh