# [PATCH] t6022: Use -eq not = to test output of wc -l 9 messages from 2010-11-08 to 2010-11-16. Participants: Brian Gernhardt, Ævar Arnfjörð Bjarmason, Jonathan Nieder, Junio C Hamano, Bert Wesarg, Johannes Sixt. Thread: https://gitlist.dev/t/25680 ## Brian Gernhardt, 2010-11-08 21:29 Subject: [PATCH] t6022: Use -eq not = to test output of wc -l Message-ID: <1289251766-48316-1-git-send-email-brian@gernhardtsoftware.com> URL: https://gitlist.dev/e/1289251766-48316-1-git-send-email-brian%40gernhardtsoftware.com ``` 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 --- 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(-) 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, 2010-11-08 21:35 Subject: Re: [PATCH] t6022: Use -eq not = to test output of wc -l Message-ID: URL: https://gitlist.dev/e/AANLkTik48HPHn4Xfwn5kuS2uHenQfjTJFaXMe9iSe5vU%40mail.gmail.com In-Reply-To: <1289251766-48316-1-git-send-email-brian@gernhardtsoftware.com> ``` >  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, 2010-11-08 21:38 Subject: Re: [PATCH] t6022: Use -eq not = to test output of wc -l Message-ID: <20101108213844.GB13114@burratino> URL: https://gitlist.dev/e/20101108213844.GB13114%40burratino In-Reply-To: ``` Ævar Arnfjörð Bjarmason wrote: >>  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, 2010-11-15 22:21 Subject: Re: [PATCH] t6022: Use -eq not = to test output of wc -l Message-ID: <7vaalajkiq.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vaalajkiq.fsf%40alter.siamese.dyndns.org In-Reply-To: <1289251766-48316-1-git-send-email-brian@gernhardtsoftware.com> ``` Brian Gernhardt 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 > --- > 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 --- 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 | ...) + test_line_count (= | -lt | -ge | ...) ! - 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 [] test_path_is_dir [] 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, 2010-11-16 06:46 Subject: Re: [PATCH] t6022: Use -eq not = to test output of wc -l Message-ID: URL: https://gitlist.dev/e/AANLkTinZVw4aB3VVxrELQgdaLZyCLzpeWgMTkxwVY%2BMt%40mail.gmail.com In-Reply-To: <7vaalajkiq.fsf@alter.siamese.dyndns.org> ``` On Mon, Nov 15, 2010 at 23:21, Junio C Hamano wrote: > 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, 2010-11-16 07:12 Subject: Re: [PATCH] t6022: Use -eq not = to test output of wc -l Message-ID: <4CE22EC2.7040603@viscovery.net> URL: https://gitlist.dev/e/4CE22EC2.7040603%40viscovery.net In-Reply-To: <7vaalajkiq.fsf@alter.siamese.dyndns.org> ``` Am 11/15/2010 23:21, schrieb Junio C Hamano: > Brian Gernhardt 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 >> --- >> 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, 2010-11-16 17:10 Subject: Re: [PATCH] t6022: Use -eq not = to test output of wc -l Message-ID: <20101116171031.GB13398@burratino> URL: https://gitlist.dev/e/20101116171031.GB13398%40burratino In-Reply-To: <4CE22EC2.7040603@viscovery.net> ``` 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 --- 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, 2010-11-16 19:10 Subject: Re: [PATCH] t6022: Use -eq not = to test output of wc -l Message-ID: <7vtyjhgk3z.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vtyjhgk3z.fsf%40alter.siamese.dyndns.org In-Reply-To: <20101116171031.GB13398@burratino> ``` Jonathan Nieder writes: > 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 > --- > 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? > fi > + rm -f "$line_count_tmp" > } > > # This is not among top-level (test_expect_success | test_expect_failure) ``` ## Jonathan Nieder, 2010-11-16 19:23 Subject: Re: [PATCH] t6022: Use -eq not = to test output of wc -l Message-ID: <20101116192318.GA15828@burratino> URL: https://gitlist.dev/e/20101116192318.GA15828%40burratino In-Reply-To: <7vtyjhgk3z.fsf@alter.siamese.dyndns.org> ``` Junio C Hamano wrote: > Jonathan Nieder writes: >> + 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.) ```