Re: [PATCH] t6022: Use -eq not = to test...

Re: [PATCH] t6022: Use -eq not = to test output of wc -l

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:50:03

Brian Gernhardt [off-list ref] writes:

When comparing numbers such as "3" to "$(wc -l)", we should check for
numerical equality using -eq instead of string equality using = because
some implementations of wc output extra whitespace.

Signed-off-by: Brian Gernhardt <redacted>
---
 The alternative would be to use 3 = $(wc -l) (sans quotes), but other parts
 of the test used the -eq method.

... which looks a tad ugly.

How about doing something like below after merging Jonathan's fb3340a
(test-lib: introduce test_line_count to measure files, 2010-10-31) instead?

-- >8 --
test_line_count: learn how to read from a pipe

And use it in t6022 to fix

    test $number = "$(pipe | wc -l)"

which should be spelled without double quotes.

Signed-off-by: Junio C Hamano <redacted>
---
 t/README                |    4 ++-
 t/t6022-merge-rename.sh |   64 +++++++++++++++++++++++-----------------------
 t/test-lib.sh           |    5 +++-
 3 files changed, 39 insertions(+), 34 deletions(-)

diff --git a/t/README b/t/README
index 1a78982..74c892d 100644
--- a/t/README
+++ b/t/README

@@ -501,8 +501,10 @@ library for your script to use.
    helpful output when the test is run with "-v" option.
 
  - test_line_count (= | -lt | -ge | ...) <length> <file>
+   test_line_count (= | -lt | -ge | ...) <length> !<cmd>
 
-   Check whether a file has the length it is expected to.
+   Check whether a file or output of a command has the length it is
+   expected to.
 
  - test_path_is_file <file> [<diagnosis>]
    test_path_is_dir <dir> [<diagnosis>]

diff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh
index 0c8eddc..c632510 100755
--- a/t/t6022-merge-rename.sh
+++ b/t/t6022-merge-rename.sh

@@ -306,8 +306,8 @@ test_expect_success 'Rename+D/F conflict; renamed file merges but dir in way' '
 	grep "Auto-merging dir" output &&
 	grep "Adding as dir~HEAD instead" output &&
 
-	test 2 = "$(git ls-files -u | wc -l)" &&
-	test 2 = "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
+	test_line_count = 2 "!git ls-files -u" &&
+	test_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 &&

@@ -328,8 +328,8 @@ test_expect_success 'Same as previous, but merged other way' '
 	grep "Auto-merging dir" output &&
 	grep "Adding as dir~renamed-file-has-no-conflicts instead" output &&
 
-	test 2 = "$(git ls-files -u | wc -l)" &&
-	test 2 = "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
+	test_line_count = 2 "!git ls-files -u" &&
+	test_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 &&

@@ -363,8 +363,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 = "$(git ls-files -u | wc -l)" &&
-	test 3 = "$(git ls-files -u dir | wc -l)" &&
+	test_line_count = 3 "!git ls-files -u" &&
+	test_line_count = 3 "!git ls-files -u dir" &&
 
 	test_must_fail git diff --quiet &&
 	test_must_fail git diff --cached --quiet &&

@@ -381,9 +381,9 @@ 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 = "$(git ls-files -u | wc -l)" &&
-	test 3 = "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)" &&
-	test 2 = "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
+	test_line_count = 5 "!git ls-files -u" &&
+	test_line_count = 3 "!git ls-files -u dir | grep -v file-in-the-way" &&
+	test_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 &&

@@ -417,9 +417,9 @@ 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 = "$(git ls-files -u | wc -l)" &&
-	test 3 = "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)" &&
-	test 2 = "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
+	test_line_count = 5 "!git ls-files -u" &&
+	test_line_count = 3 "!git ls-files -u dir | grep -v file-in-the-way" &&
+	test_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 &&

@@ -457,7 +457,7 @@ test_expect_success 'both rename source and destination involved in D/F conflict
 	git checkout -q rename-dest^0 &&
 	test_must_fail git merge --strategy=recursive source-conflict &&
 
-	test 1 = "$(git ls-files -u | wc -l)" &&
+	test_line_count = 1 "!git ls-files -u" &&
 
 	test_must_fail git diff --quiet &&

@@ -496,13 +496,13 @@ test_expect_success 'pair rename to parent of other (D/F conflicts) w/ untracked
 	mkdir one &&
 	test_must_fail git merge --strategy=recursive rename-two &&
 
-	test 2 = "$(git ls-files -u | wc -l)" &&
-	test 1 = "$(git ls-files -u one | wc -l)" &&
-	test 1 = "$(git ls-files -u two | wc -l)" &&
+	test_line_count = 2 "!git ls-files -u" &&
+	test_line_count = 1 "!git ls-files -u one" &&
+	test_line_count = 1 "!git ls-files -u two" &&
 
 	test_must_fail git diff --quiet &&
 
-	test 4 = $(find . | grep -v .git | wc -l) &&
+	test_line_count = 4 "!find . -type d -name .git -prune -o -print" &&
 
 	test -d one &&
 	test -f one~rename-two &&

@@ -516,13 +516,13 @@ test_expect_success 'pair rename to parent of other (D/F conflicts) w/ clean sta
 	git clean -fdqx &&
 	test_must_fail git merge --strategy=recursive rename-two &&
 
-	test 2 = "$(git ls-files -u | wc -l)" &&
-	test 1 = "$(git ls-files -u one | wc -l)" &&
-	test 1 = "$(git ls-files -u two | wc -l)" &&
+	test_line_count = 2 "!git ls-files -u" &&
+	test_line_count = 1 "!git ls-files -u one" &&
+	test_line_count = 1 "!git ls-files -u two" &&
 
 	test_must_fail git diff --quiet &&
 
-	test 3 = $(find . | grep -v .git | wc -l) &&
+	test_line_count = 3 "!find . -type d -name .git -prune -o -print" &&
 
 	test -f one &&
 	test -f two &&

@@ -558,12 +558,12 @@ test_expect_success 'check handling of differently renamed file with D/F conflic
 	git checkout -q first-rename^0 &&
 	test_must_fail git merge --strategy=recursive second-rename &&
 
-	test 5 = "$(git ls-files -s | wc -l)" &&
-	test 3 = "$(git ls-files -u | wc -l)" &&
-	test 1 = "$(git ls-files -u one | wc -l)" &&
-	test 1 = "$(git ls-files -u two | wc -l)" &&
-	test 1 = "$(git ls-files -u original | wc -l)" &&
-	test 2 = "$(git ls-files -o | wc -l)" &&
+	test_line_count = 5 "!git ls-files -s" &&
+	test_line_count = 3 "!git ls-files -u" &&
+	test_line_count = 1 "!git ls-files -u one" &&
+	test_line_count = 1 "!git ls-files -u two" &&
+	test_line_count = 1 "!git ls-files -u original" &&
+	test_line_count = 2 "!git ls-files -o" &&
 
 	test -f one/file &&
 	test -f two/file &&

@@ -598,11 +598,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 = "$(git ls-files -u | wc -l)" &&
-	test 1 = "$(git ls-files -u one | wc -l)" &&
-	test 1 = "$(git ls-files -u two | wc -l)" &&
-	test 1 = "$(git ls-files -u original | wc -l)" &&
-	test 0 = "$(git ls-files -o | wc -l)" &&
+	test_line_count = 3 "!git ls-files -u" &&
+	test_line_count = 1 "!git ls-files -u one" &&
+	test_line_count = 1 "!git ls-files -u two" &&
+	test_line_count = 1 "!git ls-files -u original" &&
+	test_line_count = 0 "!git ls-files -o" &&
 
 	test -f one &&
 	test -f two &&

diff --git a/t/test-lib.sh b/t/test-lib.sh
index a417bdf..3088322 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh

@@ -613,7 +613,10 @@ test_line_count () {
 	if test $# != 3
 	then
 		error "bug in the test script: not 3 parameters to test_line_count"
-	elif ! test $(wc -l <"$3") "$1" "$2"
+	elif	case "$3" in
+		!*)	! test $( eval "${3#!}" | wc -l ) "$1" "$2" ;;
+		*)	! test $( wc -l <"$3" ) "$1" "$2" ;;
+		esac
 	then
 		echo "test_line_count: line count for $3 !$1 $2"
 		cat "$3"

Re: [PATCH] t6022: Use -eq not = to test output of wc -l

From: Bert Wesarg <hidden>
Date: 2016-06-15 22:50:03

On Mon, Nov 15, 2010 at 23:21, Junio C Hamano [off-list ref] wrote:

quoted hunk

diff --git a/t/test-lib.sh b/t/test-lib.sh
index a417bdf..3088322 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh

@@ -613,7 +613,10 @@ test_line_count () {

       if test $# != 3
       then
               error "bug in the test script: not 3 parameters to test_line_count"
-       elif ! test $(wc -l <"$3") "$1" "$2"
+       elif    case "$3" in
+               !*)     ! test $( eval "${3#!}" | wc -l ) "$1" "$2" ;;
+               *)      ! test $( wc -l <"$3" ) "$1" "$2" ;;
+               esac
       then
               echo "test_line_count: line count for $3 !$1 $2"
               cat "$3"

This line will fail for the !* case.

Bert

Re: [PATCH] t6022: Use -eq not = to test output of wc -l

From: Johannes Sixt <hidden>
Date: 2016-06-15 22:50:03

Am 11/15/2010 23:21, schrieb Junio C Hamano:

Brian Gernhardt [off-list ref] writes:

quoted

When comparing numbers such as "3" to "$(wc -l)", we should check for
numerical equality using -eq instead of string equality using = because
some implementations of wc output extra whitespace.

Signed-off-by: Brian Gernhardt <redacted>
---
 The alternative would be to use 3 = $(wc -l) (sans quotes), but other parts
 of the test used the -eq method.

... which looks a tad ugly.

How about doing something like below after merging Jonathan's fb3340a
(test-lib: introduce test_line_count to measure files, 2010-10-31) instead?

-- >8 --
test_line_count: learn how to read from a pipe

I don't think that any of this (neither Jonathan's nor yours) has any benefit:

- The result is not easier to read.

- Nor are the lines of shell code shorter.

- If something in the pipe requires quoting, you need an extra level of
quotes.

- It doesn't save any messages or fix-ups during review: instead of "do
not quote!" we have to say "use test_line_count!".

Just my opinion.

-- Hannes

Re: [PATCH] t6022: Use -eq not = to test output of wc -l

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:50:03

Johannes Sixt wrote:

- It doesn't save any messages or fix-ups during review: instead of "do
not quote!" we have to say "use test_line_count!".

I was nervous about introducing test_line_count for that reason.

Another consideration won out: not syntax but output format.  See
cae3aa79 (t6022 (renaming merge): chain test commands with &&,
2010-10-31).  Kind of analogous to test_cmp, which is a similar
headache to get used over less portable or less pleasant alternatives.

If a piped variant is needed, I would prefer it to work something
like this.  Usage:

	{
		command_producing_five_lines |
		test_line_count = 5 -
	}

Signed-off-by: Jonathan Nieder <redacted>
---

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1ea0116..35a5634 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh

@@ -660,15 +660,24 @@ test_path_is_missing () {
 # output through when the number of lines is wrong.
 
 test_line_count () {
+	line_count_tmp=
 	if test $# != 3
 	then
 		error "bug in the test script: not 3 parameters to test_line_count"
-	elif ! test $(wc -l <"$3") "$1" "$2"
+	fi
+	if test "$3" = -
+	then
+		line_count_tmp=test_line_count.output
+		cat >"$line_count_tmp"
+		set -- "$1" "$2" "$line_count_tmp"
+	fi
+	if ! test $(wc -l <"$3") "$1" "$2"
 	then
 		echo "test_line_count: line count for $3 !$1 $2"
 		cat "$3"
 		return 1
 	fi
+	rm -f "$line_count_tmp"
 }
 
 # This is not among top-level (test_expect_success | test_expect_failure)

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help