{"thread":{"id":"25680","subject":"[PATCH] t6022: Use -eq not = to test output of wc -l","startedAt":"2010-11-08T21:29:26Z","lastAt":"2010-11-16T19:23:18Z","messageCount":9,"participants":["Brian Gernhardt","Ævar Arnfjörð Bjarmason","Jonathan Nieder","Junio C Hamano","Bert Wesarg","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"155434","messageId":"1289251766-48316-1-git-send-email-brian@gernhardtsoftware.com","threadId":"25680","inReplyTo":null,"subject":"[PATCH] t6022: Use -eq not = to test output of wc -l","fromName":"Brian Gernhardt","fromEmail":"brian@gernhardtsoftware.com","sentAt":"2010-11-08T21:29:26Z","receivedAt":"2010-11-08T21:29:26Z","isPatch":true,"sender":{"key":"brian@gernhardtsoftware.com","avatar":"https://avatars.githubusercontent.com/u/133455?v=4"},"body":"When comparing numbers such as \"3\" to \"$(wc -l)\", we should check for\nnumerical equality using -eq instead of string equality using = because\nsome implementations of wc output extra whitespace.\n\nSigned-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>\n---\n The alternative would be to use 3 = $(wc -l) (sans quotes), but other parts\n of the test used the -eq method.\n\n t/t6022-merge-rename.sh |   64 +++++++++++++++++++++++-----------------------\n 1 files changed, 32 insertions(+), 32 deletions(-)\n\ndiff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh\nindex 422092e..66473f0 100755\n--- a/t/t6022-merge-rename.sh\n+++ b/t/t6022-merge-rename.sh\n@@ -404,8 +404,8 @@ test_expect_success 'Rename+D/F conflict; renamed file merges but dir in way' '\n \tgrep \"Auto-merging dir\" output &&\n \tgrep \"Adding as dir~HEAD instead\" output &&\n \n-\ttest 2 = \"$(git ls-files -u | wc -l)\" &&\n-\ttest 2 = \"$(git ls-files -u dir/file-in-the-way | wc -l)\" &&\n+\ttest 2 -eq \"$(git ls-files -u | wc -l)\" &&\n+\ttest 2 -eq \"$(git ls-files -u dir/file-in-the-way | wc -l)\" &&\n \n \ttest_must_fail git diff --quiet &&\n \ttest_must_fail git diff --cached --quiet &&\n@@ -426,8 +426,8 @@ test_expect_success 'Same as previous, but merged other way' '\n \tgrep \"Auto-merging dir\" output &&\n \tgrep \"Adding as dir~renamed-file-has-no-conflicts instead\" output &&\n \n-\ttest 2 = \"$(git ls-files -u | wc -l)\" &&\n-\ttest 2 = \"$(git ls-files -u dir/file-in-the-way | wc -l)\" &&\n+\ttest 2 -eq \"$(git ls-files -u | wc -l)\" &&\n+\ttest 2 -eq \"$(git ls-files -u dir/file-in-the-way | wc -l)\" &&\n \n \ttest_must_fail git diff --quiet &&\n \ttest_must_fail git diff --cached --quiet &&\n@@ -461,8 +461,8 @@ test_expect_success 'Rename+D/F conflict; renamed file cannot merge, dir not in\n \tgit checkout -q renamed-file-has-conflicts^0 &&\n \ttest_must_fail git merge --strategy=recursive dir-not-in-way &&\n \n-\ttest 3 = \"$(git ls-files -u | wc -l)\" &&\n-\ttest 3 = \"$(git ls-files -u dir | wc -l)\" &&\n+\ttest 3 -eq \"$(git ls-files -u | wc -l)\" &&\n+\ttest 3 -eq \"$(git ls-files -u dir | wc -l)\" &&\n \n \ttest_must_fail git diff --quiet &&\n \ttest_must_fail git diff --cached --quiet &&\n@@ -479,9 +479,9 @@ test_expect_success 'Rename+D/F conflict; renamed file cannot merge and dir in t\n \tgit checkout -q renamed-file-has-conflicts^0 &&\n \ttest_must_fail git merge --strategy=recursive dir-in-way &&\n \n-\ttest 5 = \"$(git ls-files -u | wc -l)\" &&\n-\ttest 3 = \"$(git ls-files -u dir | grep -v file-in-the-way | wc -l)\" &&\n-\ttest 2 = \"$(git ls-files -u dir/file-in-the-way | wc -l)\" &&\n+\ttest 5 -eq \"$(git ls-files -u | wc -l)\" &&\n+\ttest 3 -eq \"$(git ls-files -u dir | grep -v file-in-the-way | wc -l)\" &&\n+\ttest 2 -eq \"$(git ls-files -u dir/file-in-the-way | wc -l)\" &&\n \n \ttest_must_fail git diff --quiet &&\n \ttest_must_fail git diff --cached --quiet &&\n@@ -515,9 +515,9 @@ test_expect_success 'Same as previous, but merged other way' '\n \tgit checkout -q dir-in-way^0 &&\n \ttest_must_fail git merge --strategy=recursive renamed-file-has-conflicts &&\n \n-\ttest 5 = \"$(git ls-files -u | wc -l)\" &&\n-\ttest 3 = \"$(git ls-files -u dir | grep -v file-in-the-way | wc -l)\" &&\n-\ttest 2 = \"$(git ls-files -u dir/file-in-the-way | wc -l)\" &&\n+\ttest 5 -eq \"$(git ls-files -u | wc -l)\" &&\n+\ttest 3 -eq \"$(git ls-files -u dir | grep -v file-in-the-way | wc -l)\" &&\n+\ttest 2 -eq \"$(git ls-files -u dir/file-in-the-way | wc -l)\" &&\n \n \ttest_must_fail git diff --quiet &&\n \ttest_must_fail git diff --cached --quiet &&\n@@ -555,7 +555,7 @@ test_expect_success 'both rename source and destination involved in D/F conflict\n \tgit checkout -q rename-dest^0 &&\n \ttest_must_fail git merge --strategy=recursive source-conflict &&\n \n-\ttest 1 = \"$(git ls-files -u | wc -l)\" &&\n+\ttest 1 -eq \"$(git ls-files -u | wc -l)\" &&\n \n \ttest_must_fail git diff --quiet &&\n \n@@ -594,13 +594,13 @@ test_expect_success 'pair rename to parent of other (D/F conflicts) w/ untracked\n \tmkdir one &&\n \ttest_must_fail git merge --strategy=recursive rename-two &&\n \n-\ttest 2 = \"$(git ls-files -u | wc -l)\" &&\n-\ttest 1 = \"$(git ls-files -u one | wc -l)\" &&\n-\ttest 1 = \"$(git ls-files -u two | wc -l)\" &&\n+\ttest 2 -eq \"$(git ls-files -u | wc -l)\" &&\n+\ttest 1 -eq \"$(git ls-files -u one | wc -l)\" &&\n+\ttest 1 -eq \"$(git ls-files -u two | wc -l)\" &&\n \n \ttest_must_fail git diff --quiet &&\n \n-\ttest 4 = $(find . | grep -v .git | wc -l) &&\n+\ttest 4 -eq $(find . | grep -v .git | wc -l) &&\n \n \ttest -d one &&\n \ttest -f one~rename-two &&\n@@ -614,13 +614,13 @@ test_expect_success 'pair rename to parent of other (D/F conflicts) w/ clean sta\n \tgit clean -fdqx &&\n \ttest_must_fail git merge --strategy=recursive rename-two &&\n \n-\ttest 2 = \"$(git ls-files -u | wc -l)\" &&\n-\ttest 1 = \"$(git ls-files -u one | wc -l)\" &&\n-\ttest 1 = \"$(git ls-files -u two | wc -l)\" &&\n+\ttest 2 -eq \"$(git ls-files -u | wc -l)\" &&\n+\ttest 1 -eq \"$(git ls-files -u one | wc -l)\" &&\n+\ttest 1 -eq \"$(git ls-files -u two | wc -l)\" &&\n \n \ttest_must_fail git diff --quiet &&\n \n-\ttest 3 = $(find . | grep -v .git | wc -l) &&\n+\ttest 3 -eq $(find . | grep -v .git | wc -l) &&\n \n \ttest -f one &&\n \ttest -f two &&\n@@ -656,12 +656,12 @@ test_expect_success 'check handling of differently renamed file with D/F conflic\n \tgit checkout -q first-rename^0 &&\n \ttest_must_fail git merge --strategy=recursive second-rename &&\n \n-\ttest 5 = \"$(git ls-files -s | wc -l)\" &&\n-\ttest 3 = \"$(git ls-files -u | wc -l)\" &&\n-\ttest 1 = \"$(git ls-files -u one | wc -l)\" &&\n-\ttest 1 = \"$(git ls-files -u two | wc -l)\" &&\n-\ttest 1 = \"$(git ls-files -u original | wc -l)\" &&\n-\ttest 2 = \"$(git ls-files -o | wc -l)\" &&\n+\ttest 5 -eq \"$(git ls-files -s | wc -l)\" &&\n+\ttest 3 -eq \"$(git ls-files -u | wc -l)\" &&\n+\ttest 1 -eq \"$(git ls-files -u one | wc -l)\" &&\n+\ttest 1 -eq \"$(git ls-files -u two | wc -l)\" &&\n+\ttest 1 -eq \"$(git ls-files -u original | wc -l)\" &&\n+\ttest 2 -eq \"$(git ls-files -o | wc -l)\" &&\n \n \ttest -f one/file &&\n \ttest -f two/file &&\n@@ -696,11 +696,11 @@ test_expect_success 'check handling of differently renamed file with D/F conflic\n \tgit checkout -q first-rename-redo^0 &&\n \ttest_must_fail git merge --strategy=recursive second-rename-redo &&\n \n-\ttest 3 = \"$(git ls-files -u | wc -l)\" &&\n-\ttest 1 = \"$(git ls-files -u one | wc -l)\" &&\n-\ttest 1 = \"$(git ls-files -u two | wc -l)\" &&\n-\ttest 1 = \"$(git ls-files -u original | wc -l)\" &&\n-\ttest 0 = \"$(git ls-files -o | wc -l)\" &&\n+\ttest 3 -eq \"$(git ls-files -u | wc -l)\" &&\n+\ttest 1 -eq \"$(git ls-files -u one | wc -l)\" &&\n+\ttest 1 -eq \"$(git ls-files -u two | wc -l)\" &&\n+\ttest 1 -eq \"$(git ls-files -u original | wc -l)\" &&\n+\ttest 0 -eq \"$(git ls-files -o | wc -l)\" &&\n \n \ttest -f one &&\n \ttest -f two &&\n-- \n1.7.3.2.333.g23b6d8\n"},{"id":"155435","messageId":"AANLkTik48HPHn4Xfwn5kuS2uHenQfjTJFaXMe9iSe5vU@mail.gmail.com","threadId":"25680","inReplyTo":"1289251766-48316-1-git-send-email-brian@gernhardtsoftware.com","subject":"Re: [PATCH] t6022: Use -eq not = to test output of wc -l","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-11-08T21:35:38Z","receivedAt":"2010-11-08T21:35:38Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":">  The alternative would be to use 3 = $(wc -l) (sans quotes), but other parts\n>  of the test used the -eq method.\n\nA IMO saner alternative is to add a new test-lib.sh function for this.\n\nE.g.:\n\n-       test 3 = \"$(git ls-files -u | wc -l)\" &&\n-       test_wc 3 \"git ls-files -u\" &&\n\nWe could use test_cmp or something internally so we'd get meaningful\noutput when those tests fail.\n\nBut this is a good fix anyway.\n"},{"id":"155436","messageId":"20101108213844.GB13114@burratino","threadId":"25680","inReplyTo":"AANLkTik48HPHn4Xfwn5kuS2uHenQfjTJFaXMe9iSe5vU@mail.gmail.com","subject":"Re: [PATCH] t6022: Use -eq not = to test output of wc -l","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-08T21:38:44Z","receivedAt":"2010-11-08T21:38:44Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n\n>>  The alternative would be to use 3 = $(wc -l) (sans quotes), but other parts\n>>  of the test used the -eq method.\n>\n> A IMO saner alternative is to add a new test-lib.sh function for this.\n> \n> E.g.:\n> \n> -       test 3 = \"$(git ls-files -u | wc -l)\" &&\n> -       test_wc 3 \"git ls-files -u\" &&\n> \n> We could use test_cmp or something internally so we'd get meaningful\n> output when those tests fail.\n\nSee http://thread.gmane.org/gmane.comp.version-control.git/157903/focus=160421\nfor example.\n"},{"id":"155953","messageId":"7vaalajkiq.fsf@alter.siamese.dyndns.org","threadId":"25680","inReplyTo":"1289251766-48316-1-git-send-email-brian@gernhardtsoftware.com","subject":"Re: [PATCH] t6022: Use -eq not = to test output of wc -l","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-11-15T22:21:17Z","receivedAt":"2010-11-15T22:21:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brian Gernhardt <brian@gernhardtsoftware.com> writes:\n\n> When comparing numbers such as \"3\" to \"$(wc -l)\", we should check for\n> numerical equality using -eq instead of string equality using = because\n> some implementations of wc output extra whitespace.\n>\n> Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>\n> ---\n>  The alternative would be to use 3 = $(wc -l) (sans quotes), but other parts\n>  of the test used the -eq method.\n\n... which looks a tad ugly.\n\nHow about doing something like below after merging Jonathan's fb3340a\n(test-lib: introduce test_line_count to measure files, 2010-10-31) instead?\n\n-- >8 --\ntest_line_count: learn how to read from a pipe\n\nAnd use it in t6022 to fix\n\n    test $number = \"$(pipe | wc -l)\"\n\nwhich should be spelled without double quotes.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/README                |    4 ++-\n t/t6022-merge-rename.sh |   64 +++++++++++++++++++++++-----------------------\n t/test-lib.sh           |    5 +++-\n 3 files changed, 39 insertions(+), 34 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 1a78982..74c892d 100644\n--- a/t/README\n+++ b/t/README\n@@ -501,8 +501,10 @@ library for your script to use.\n    helpful output when the test is run with \"-v\" option.\n \n  - test_line_count (= | -lt | -ge | ...) <length> <file>\n+   test_line_count (= | -lt | -ge | ...) <length> !<cmd>\n \n-   Check whether a file has the length it is expected to.\n+   Check whether a file or output of a command has the length it is\n+   expected to.\n \n  - test_path_is_file <file> [<diagnosis>]\n    test_path_is_dir <dir> [<diagnosis>]\ndiff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh\nindex 0c8eddc..c632510 100755\n--- a/t/t6022-merge-rename.sh\n+++ b/t/t6022-merge-rename.sh\n@@ -306,8 +306,8 @@ test_expect_success 'Rename+D/F conflict; renamed file merges but dir in way' '\n \tgrep \"Auto-merging dir\" output &&\n \tgrep \"Adding as dir~HEAD instead\" output &&\n \n-\ttest 2 = \"$(git ls-files -u | wc -l)\" &&\n-\ttest 2 = \"$(git ls-files -u dir/file-in-the-way | wc -l)\" &&\n+\ttest_line_count = 2 \"!git ls-files -u\" &&\n+\ttest_line_count = 2 \"!git ls-files -u dir/file-in-the-way\" &&\n \n \ttest_must_fail git diff --quiet &&\n \ttest_must_fail git diff --cached --quiet &&\n@@ -328,8 +328,8 @@ test_expect_success 'Same as previous, but merged other way' '\n \tgrep \"Auto-merging dir\" output &&\n \tgrep \"Adding as dir~renamed-file-has-no-conflicts instead\" output &&\n \n-\ttest 2 = \"$(git ls-files -u | wc -l)\" &&\n-\ttest 2 = \"$(git ls-files -u dir/file-in-the-way | wc -l)\" &&\n+\ttest_line_count = 2 \"!git ls-files -u\" &&\n+\ttest_line_count = 2 \"!git ls-files -u dir/file-in-the-way\" &&\n \n \ttest_must_fail git diff --quiet &&\n \ttest_must_fail git diff --cached --quiet &&\n@@ -363,8 +363,8 @@ test_expect_success 'Rename+D/F conflict; renamed file cannot merge, dir not in\n \tgit checkout -q renamed-file-has-conflicts^0 &&\n \ttest_must_fail git merge --strategy=recursive dir-not-in-way &&\n \n-\ttest 3 = \"$(git ls-files -u | wc -l)\" &&\n-\ttest 3 = \"$(git ls-files -u dir | wc -l)\" &&\n+\ttest_line_count = 3 \"!git ls-files -u\" &&\n+\ttest_line_count = 3 \"!git ls-files -u dir\" &&\n \n \ttest_must_fail git diff --quiet &&\n \ttest_must_fail git diff --cached --quiet &&\n@@ -381,9 +381,9 @@ test_expect_success 'Rename+D/F conflict; renamed file cannot merge and dir in t\n \tgit checkout -q renamed-file-has-conflicts^0 &&\n \ttest_must_fail git merge --strategy=recursive dir-in-way &&\n \n-\ttest 5 = \"$(git ls-files -u | wc -l)\" &&\n-\ttest 3 = \"$(git ls-files -u dir | grep -v file-in-the-way | wc -l)\" &&\n-\ttest 2 = \"$(git ls-files -u dir/file-in-the-way | wc -l)\" &&\n+\ttest_line_count = 5 \"!git ls-files -u\" &&\n+\ttest_line_count = 3 \"!git ls-files -u dir | grep -v file-in-the-way\" &&\n+\ttest_line_count = 2 \"!git ls-files -u dir/file-in-the-way\" &&\n \n \ttest_must_fail git diff --quiet &&\n \ttest_must_fail git diff --cached --quiet &&\n@@ -417,9 +417,9 @@ test_expect_success 'Same as previous, but merged other way' '\n \tgit checkout -q dir-in-way^0 &&\n \ttest_must_fail git merge --strategy=recursive renamed-file-has-conflicts &&\n \n-\ttest 5 = \"$(git ls-files -u | wc -l)\" &&\n-\ttest 3 = \"$(git ls-files -u dir | grep -v file-in-the-way | wc -l)\" &&\n-\ttest 2 = \"$(git ls-files -u dir/file-in-the-way | wc -l)\" &&\n+\ttest_line_count = 5 \"!git ls-files -u\" &&\n+\ttest_line_count = 3 \"!git ls-files -u dir | grep -v file-in-the-way\" &&\n+\ttest_line_count = 2 \"!git ls-files -u dir/file-in-the-way\" &&\n \n \ttest_must_fail git diff --quiet &&\n \ttest_must_fail git diff --cached --quiet &&\n@@ -457,7 +457,7 @@ test_expect_success 'both rename source and destination involved in D/F conflict\n \tgit checkout -q rename-dest^0 &&\n \ttest_must_fail git merge --strategy=recursive source-conflict &&\n \n-\ttest 1 = \"$(git ls-files -u | wc -l)\" &&\n+\ttest_line_count = 1 \"!git ls-files -u\" &&\n \n \ttest_must_fail git diff --quiet &&\n \n@@ -496,13 +496,13 @@ test_expect_success 'pair rename to parent of other (D/F conflicts) w/ untracked\n \tmkdir one &&\n \ttest_must_fail git merge --strategy=recursive rename-two &&\n \n-\ttest 2 = \"$(git ls-files -u | wc -l)\" &&\n-\ttest 1 = \"$(git ls-files -u one | wc -l)\" &&\n-\ttest 1 = \"$(git ls-files -u two | wc -l)\" &&\n+\ttest_line_count = 2 \"!git ls-files -u\" &&\n+\ttest_line_count = 1 \"!git ls-files -u one\" &&\n+\ttest_line_count = 1 \"!git ls-files -u two\" &&\n \n \ttest_must_fail git diff --quiet &&\n \n-\ttest 4 = $(find . | grep -v .git | wc -l) &&\n+\ttest_line_count = 4 \"!find . -type d -name .git -prune -o -print\" &&\n \n \ttest -d one &&\n \ttest -f one~rename-two &&\n@@ -516,13 +516,13 @@ test_expect_success 'pair rename to parent of other (D/F conflicts) w/ clean sta\n \tgit clean -fdqx &&\n \ttest_must_fail git merge --strategy=recursive rename-two &&\n \n-\ttest 2 = \"$(git ls-files -u | wc -l)\" &&\n-\ttest 1 = \"$(git ls-files -u one | wc -l)\" &&\n-\ttest 1 = \"$(git ls-files -u two | wc -l)\" &&\n+\ttest_line_count = 2 \"!git ls-files -u\" &&\n+\ttest_line_count = 1 \"!git ls-files -u one\" &&\n+\ttest_line_count = 1 \"!git ls-files -u two\" &&\n \n \ttest_must_fail git diff --quiet &&\n \n-\ttest 3 = $(find . | grep -v .git | wc -l) &&\n+\ttest_line_count = 3 \"!find . -type d -name .git -prune -o -print\" &&\n \n \ttest -f one &&\n \ttest -f two &&\n@@ -558,12 +558,12 @@ test_expect_success 'check handling of differently renamed file with D/F conflic\n \tgit checkout -q first-rename^0 &&\n \ttest_must_fail git merge --strategy=recursive second-rename &&\n \n-\ttest 5 = \"$(git ls-files -s | wc -l)\" &&\n-\ttest 3 = \"$(git ls-files -u | wc -l)\" &&\n-\ttest 1 = \"$(git ls-files -u one | wc -l)\" &&\n-\ttest 1 = \"$(git ls-files -u two | wc -l)\" &&\n-\ttest 1 = \"$(git ls-files -u original | wc -l)\" &&\n-\ttest 2 = \"$(git ls-files -o | wc -l)\" &&\n+\ttest_line_count = 5 \"!git ls-files -s\" &&\n+\ttest_line_count = 3 \"!git ls-files -u\" &&\n+\ttest_line_count = 1 \"!git ls-files -u one\" &&\n+\ttest_line_count = 1 \"!git ls-files -u two\" &&\n+\ttest_line_count = 1 \"!git ls-files -u original\" &&\n+\ttest_line_count = 2 \"!git ls-files -o\" &&\n \n \ttest -f one/file &&\n \ttest -f two/file &&\n@@ -598,11 +598,11 @@ test_expect_success 'check handling of differently renamed file with D/F conflic\n \tgit checkout -q first-rename-redo^0 &&\n \ttest_must_fail git merge --strategy=recursive second-rename-redo &&\n \n-\ttest 3 = \"$(git ls-files -u | wc -l)\" &&\n-\ttest 1 = \"$(git ls-files -u one | wc -l)\" &&\n-\ttest 1 = \"$(git ls-files -u two | wc -l)\" &&\n-\ttest 1 = \"$(git ls-files -u original | wc -l)\" &&\n-\ttest 0 = \"$(git ls-files -o | wc -l)\" &&\n+\ttest_line_count = 3 \"!git ls-files -u\" &&\n+\ttest_line_count = 1 \"!git ls-files -u one\" &&\n+\ttest_line_count = 1 \"!git ls-files -u two\" &&\n+\ttest_line_count = 1 \"!git ls-files -u original\" &&\n+\ttest_line_count = 0 \"!git ls-files -o\" &&\n \n \ttest -f one &&\n \ttest -f two &&\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex a417bdf..3088322 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -613,7 +613,10 @@ test_line_count () {\n \tif test $# != 3\n \tthen\n \t\terror \"bug in the test script: not 3 parameters to test_line_count\"\n-\telif ! test $(wc -l <\"$3\") \"$1\" \"$2\"\n+\telif\tcase \"$3\" in\n+\t\t!*)\t! test $( eval \"${3#!}\" | wc -l ) \"$1\" \"$2\" ;;\n+\t\t*)\t! test $( wc -l <\"$3\" ) \"$1\" \"$2\" ;;\n+\t\tesac\n \tthen\n \t\techo \"test_line_count: line count for $3 !$1 $2\"\n \t\tcat \"$3\"\n"},{"id":"155979","messageId":"AANLkTinZVw4aB3VVxrELQgdaLZyCLzpeWgMTkxwVY+Mt@mail.gmail.com","threadId":"25680","inReplyTo":"7vaalajkiq.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t6022: Use -eq not = to test output of wc -l","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2010-11-16T06:46:57Z","receivedAt":"2010-11-16T06:46:57Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Mon, Nov 15, 2010 at 23:21, Junio C Hamano <gitster@pobox.com> wrote:\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index a417bdf..3088322 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -613,7 +613,10 @@ test_line_count () {\n>        if test $# != 3\n>        then\n>                error \"bug in the test script: not 3 parameters to test_line_count\"\n> -       elif ! test $(wc -l <\"$3\") \"$1\" \"$2\"\n> +       elif    case \"$3\" in\n> +               !*)     ! test $( eval \"${3#!}\" | wc -l ) \"$1\" \"$2\" ;;\n> +               *)      ! test $( wc -l <\"$3\" ) \"$1\" \"$2\" ;;\n> +               esac\n>        then\n>                echo \"test_line_count: line count for $3 !$1 $2\"\n>                cat \"$3\"\n\nThis line will fail for the !* case.\n\nBert\n"},{"id":"155980","messageId":"4CE22EC2.7040603@viscovery.net","threadId":"25680","inReplyTo":"7vaalajkiq.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t6022: Use -eq not = to test output of wc -l","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2010-11-16T07:12:02Z","receivedAt":"2010-11-16T07:12:02Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 11/15/2010 23:21, schrieb Junio C Hamano:\n> Brian Gernhardt <brian@gernhardtsoftware.com> writes:\n> \n>> When comparing numbers such as \"3\" to \"$(wc -l)\", we should check for\n>> numerical equality using -eq instead of string equality using = because\n>> some implementations of wc output extra whitespace.\n>>\n>> Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>\n>> ---\n>>  The alternative would be to use 3 = $(wc -l) (sans quotes), but other parts\n>>  of the test used the -eq method.\n> \n> ... which looks a tad ugly.\n> \n> How about doing something like below after merging Jonathan's fb3340a\n> (test-lib: introduce test_line_count to measure files, 2010-10-31) instead?\n> \n> -- >8 --\n> test_line_count: learn how to read from a pipe\n\nI don't think that any of this (neither Jonathan's nor yours) has any benefit:\n\n- The result is not easier to read.\n\n- Nor are the lines of shell code shorter.\n\n- If something in the pipe requires quoting, you need an extra level of\nquotes.\n\n- It doesn't save any messages or fix-ups during review: instead of \"do\nnot quote!\" we have to say \"use test_line_count!\".\n\nJust my opinion.\n\n-- Hannes\n"},{"id":"156000","messageId":"20101116171031.GB13398@burratino","threadId":"25680","inReplyTo":"4CE22EC2.7040603@viscovery.net","subject":"Re: [PATCH] t6022: Use -eq not = to test output of wc -l","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-16T17:10:32Z","receivedAt":"2010-11-16T17:10:32Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Johannes Sixt wrote:\n\n> - It doesn't save any messages or fix-ups during review: instead of \"do\n> not quote!\" we have to say \"use test_line_count!\".\n\nI was nervous about introducing test_line_count for that reason.\n\nAnother consideration won out: not syntax but output format.  See\ncae3aa79 (t6022 (renaming merge): chain test commands with &&,\n2010-10-31).  Kind of analogous to test_cmp, which is a similar\nheadache to get used over less portable or less pleasant alternatives.\n\nIf a piped variant is needed, I would prefer it to work something\nlike this.  Usage:\n\n\t{\n\t\tcommand_producing_five_lines |\n\t\ttest_line_count = 5 -\n\t}\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 1ea0116..35a5634 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -660,15 +660,24 @@ test_path_is_missing () {\n # output through when the number of lines is wrong.\n \n test_line_count () {\n+\tline_count_tmp=\n \tif test $# != 3\n \tthen\n \t\terror \"bug in the test script: not 3 parameters to test_line_count\"\n-\telif ! test $(wc -l <\"$3\") \"$1\" \"$2\"\n+\tfi\n+\tif test \"$3\" = -\n+\tthen\n+\t\tline_count_tmp=test_line_count.output\n+\t\tcat >\"$line_count_tmp\"\n+\t\tset -- \"$1\" \"$2\" \"$line_count_tmp\"\n+\tfi\n+\tif ! test $(wc -l <\"$3\") \"$1\" \"$2\"\n \tthen\n \t\techo \"test_line_count: line count for $3 !$1 $2\"\n \t\tcat \"$3\"\n \t\treturn 1\n \tfi\n+\trm -f \"$line_count_tmp\"\n }\n \n # This is not among top-level (test_expect_success | test_expect_failure)\n"},{"id":"156012","messageId":"7vtyjhgk3z.fsf@alter.siamese.dyndns.org","threadId":"25680","inReplyTo":"20101116171031.GB13398@burratino","subject":"Re: [PATCH] t6022: Use -eq not = to test output of wc -l","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-11-16T19:10:40Z","receivedAt":"2010-11-16T19:10:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Johannes Sixt wrote:\n>\n>> - It doesn't save any messages or fix-ups during review: instead of \"do\n>> not quote!\" we have to say \"use test_line_count!\".\n>\n> I was nervous about introducing test_line_count for that reason.\n>\n> Another consideration won out: not syntax but output format.  See\n> cae3aa79 (t6022 (renaming merge): chain test commands with &&,\n> 2010-10-31).  Kind of analogous to test_cmp, which is a similar\n> headache to get used over less portable or less pleasant alternatives.\n>\n> If a piped variant is needed, I would prefer it to work something\n> like this.  Usage:\n>\n> \t{\n> \t\tcommand_producing_five_lines |\n> \t\ttest_line_count = 5 -\n> \t}\n>\n> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>\n> ---\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 1ea0116..35a5634 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -660,15 +660,24 @@ test_path_is_missing () {\n>  # output through when the number of lines is wrong.\n>  \n>  test_line_count () {\n> +\tline_count_tmp=\n>  \tif test $# != 3\n>  \tthen\n>  \t\terror \"bug in the test script: not 3 parameters to test_line_count\"\n> -\telif ! test $(wc -l <\"$3\") \"$1\" \"$2\"\n> +\tfi\n> +\tif test \"$3\" = -\n> +\tthen\n> +\t\tline_count_tmp=test_line_count.output\n> +\t\tcat >\"$line_count_tmp\"\n> +\t\tset -- \"$1\" \"$2\" \"$line_count_tmp\"\n> +\tfi\n> +\tif ! test $(wc -l <\"$3\") \"$1\" \"$2\"\n>  \tthen\n>  \t\techo \"test_line_count: line count for $3 !$1 $2\"\n>  \t\tcat \"$3\"\n>  \t\treturn 1\n\nYou forgot to clean the temporary file here.\n\nAlso if the file is huge, do we really want to cat the whole thing?\n\n>  \tfi\n> +\trm -f \"$line_count_tmp\"\n>  }\n>  \n>  # This is not among top-level (test_expect_success | test_expect_failure)\n"},{"id":"156014","messageId":"20101116192318.GA15828@burratino","threadId":"25680","inReplyTo":"7vtyjhgk3z.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t6022: Use -eq not = to test output of wc -l","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-16T19:23:18Z","receivedAt":"2010-11-16T19:23:18Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n\n>> +\tif test \"$3\" = -\n>> +\tthen\n>> +\t\tline_count_tmp=test_line_count.output\n>> +\t\tcat >\"$line_count_tmp\"\n>> +\t\tset -- \"$1\" \"$2\" \"$line_count_tmp\"\n>> +\tfi\n>> +\tif ! test $(wc -l <\"$3\") \"$1\" \"$2\"\n>>  \tthen\n>>  \t\techo \"test_line_count: line count for $3 !$1 $2\"\n>>  \t\tcat \"$3\"\n>>  \t\treturn 1\n>\n> You forgot to clean the temporary file here.\n\nThe idea was to leave it around to help in diagnosis.  But you are\nright, it is more useful to not break later tests:\n\n\t\trm -f \"$line_count_tmp\"\n\t\treturn 1\n\n> Also if the file is huge, do we really want to cat the whole thing?\n\nI think so.  (Maybe not if it is binary, though.)\n"}]}