threads / patch / 25680

patcht6022: Use -eq not = to test output of wc -l

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

## tl;dr

9 messages between Nov 8, 2010 and Nov 16, 2010. Diffs are folded; open one to read it.

replies: 8people: 6as markdown or json

Brian Gernhardt· Nov 8, 2010, 21:29 UTC · lore

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 <brian@gernhardtsoftware.com>
---
 The alternative would be to use 3 = $(wc -l) (sans quotes), but other parts
 of the test used the -eq method.
 t/t6022-merge-rename.sh |   64 +++++++++++++++++++++++-----------------------
 1 files changed, 32 insertions(+), 32 deletions(-)
Show changes to t/t6022-merge-rename.sh +32 −32
diff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh
index 422092e..66473f0 100755
--- a/t/t6022-merge-rename.sh
+++ b/t/t6022-merge-rename.sh
@@ -404,8 +404,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 2 -eq "$(git ls-files -u | wc -l)" &&
+	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
 
 	test_must_fail git diff --quiet &&
 	test_must_fail git diff --cached --quiet &&
@@ -426,8 +426,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 2 -eq "$(git ls-files -u | wc -l)" &&
+	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
 
 	test_must_fail git diff --quiet &&
 	test_must_fail git diff --cached --quiet &&
@@ -461,8 +461,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 3 -eq "$(git ls-files -u | wc -l)" &&
+	test 3 -eq "$(git ls-files -u dir | wc -l)" &&
 
 	test_must_fail git diff --quiet &&
 	test_must_fail git diff --cached --quiet &&
@@ -479,9 +479,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 5 -eq "$(git ls-files -u | wc -l)" &&
+	test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)" &&
+	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
 
 	test_must_fail git diff --quiet &&
 	test_must_fail git diff --cached --quiet &&
@@ -515,9 +515,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 5 -eq "$(git ls-files -u | wc -l)" &&
+	test 3 -eq "$(git ls-files -u dir | grep -v file-in-the-way | wc -l)" &&
+	test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
 
 	test_must_fail git diff --quiet &&
 	test_must_fail git diff --cached --quiet &&
@@ -555,7 +555,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 1 -eq "$(git ls-files -u | wc -l)" &&
 
 	test_must_fail git diff --quiet &&
 
@@ -594,13 +594,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 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_must_fail git diff --quiet &&
 
-	test 4 = $(find . | grep -v .git | wc -l) &&
+	test 4 -eq $(find . | grep -v .git | wc -l) &&
 
 	test -d one &&
 	test -f one~rename-two &&
@@ -614,13 +614,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 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_must_fail git diff --quiet &&
 
-	test 3 = $(find . | grep -v .git | wc -l) &&
+	test 3 -eq $(find . | grep -v .git | wc -l) &&
 
 	test -f one &&
 	test -f two &&
@@ -656,12 +656,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 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 -f one/file &&
 	test -f two/file &&
@@ -696,11 +696,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 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 -f one &&
 	test -f two &&

-- 
1.7.3.2.333.g23b6d8
Ævar Arnfjörð Bjarmason· Nov 8, 2010, 21:35 UTC · re: Brian Gernhardt · lore

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

>  The alternative would be to use 3 = $(wc -l) (sans quotes), but other parts
>  of the test used the -eq method.
A IMO saner alternative is to add a new test-lib.sh function for this.
E.g.:
-       test 3 = "$(git ls-files -u | wc -l)" &&
-       test_wc 3 "git ls-files -u" &&

We could use test_cmp or something internally so we'd get meaningful output when those tests fail.

But this is a good fix anyway.
Jonathan Nieder· Nov 8, 2010, 21:38 UTC · re: Ævar Arnfjörð Bjarmason · lore

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

Ævar Arnfjörð Bjarmason wrote:
Show 12 quoted lines
>>  The alternative would be to use 3 = $(wc -l) (sans quotes), but other parts
>>  of the test used the -eq method.
>
> A IMO saner alternative is to add a new test-lib.sh function for this.
> 
> E.g.:
> 
> -       test 3 = "$(git ls-files -u | wc -l)" &&
> -       test_wc 3 "git ls-files -u" &&
> 
> We could use test_cmp or something internally so we'd get meaningful
> output when those tests fail.

See http://thread.gmane.org/gmane.comp.version-control.git/157903/focus=160421 for example.

Junio C Hamano· Nov 15, 2010, 22:21 UTC · re: Brian Gernhardt · lore

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

Brian Gernhardt <brian@gernhardtsoftware.com> writes:
Show 8 quoted lines
> 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 <brian@gernhardtsoftware.com>
> ---
>  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 <gitster@pobox.com>
---
 t/README                |    4 ++-
 t/t6022-merge-rename.sh |   64 +++++++++++++++++++++++-----------------------
 t/test-lib.sh           |    5 +++-
 3 files changed, 39 insertions(+), 34 deletions(-)
Show changes to 3 files +39 −34

t/README, t/t6022-merge-rename.sh, t/test-lib.sh

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"
Bert Wesarg· Nov 16, 2010, 06:46 UTC · re: Junio C Hamano · lore

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

On Mon, Nov 15, 2010 at 23:21, Junio C Hamano <gitster@pobox.com> wrote:
Show 16 quoted lines
> 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
Johannes Sixt· Nov 16, 2010, 07:12 UTC · re: Junio C Hamano · lore

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

Am 11/15/2010 23:21, schrieb Junio C Hamano:
Show 18 quoted lines
> Brian Gernhardt <brian@gernhardtsoftware.com> 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 <brian@gernhardtsoftware.com>
>> ---
>>  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
Jonathan Nieder· Nov 16, 2010, 17:10 UTC · re: Johannes Sixt · lore

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

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 <jrnieder@gmail.com>
---
Show changes to t/test-lib.sh +10 −1
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)
Junio C Hamano· Nov 16, 2010, 19:10 UTC · re: Jonathan Nieder · lore

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

Jonathan Nieder <jrnieder@gmail.com> writes:
Show 47 quoted lines
> 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 <jrnieder@gmail.com>
> ---
> 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
You forgot to clean the temporary file here.
Also if the file is huge, do we really want to cat the whole thing?
Show 5 quoted lines
>  	fi
> +	rm -f "$line_count_tmp"
>  }
>  
>  # This is not among top-level (test_expect_success | test_expect_failure)
Jonathan Nieder· Nov 16, 2010, 19:23 UTC · re: Junio C Hamano · lore

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

Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
Show 13 quoted lines
>> +	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
>
> You forgot to clean the temporary file here.

The idea was to leave it around to help in diagnosis. But you are right, it is more useful to not break later tests:

		rm -f "$line_count_tmp"
		return 1
> Also if the file is huge, do we really want to cat the whole thing?
I think so.  (Maybe not if it is binary, though.)

← back to recent threads