{"thread":{"id":"55907","subject":"[PATCH 0/4] t: new helper test_line_count_cmd","startedAt":"2021-06-12T04:28:13Z","lastAt":"2021-06-24T23:22:59Z","messageCount":22,"participants":["Đoàn Trần Công Danh","Bagas Sanjaya","Eric Sunshine","Ævar Arnfjörð Bjarmason","Phillip Wood","Felipe Contreras","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"427163","messageId":"20210612042755.28342-1-congdanhqx@gmail.com","threadId":"55907","inReplyTo":null,"subject":"[PATCH 0/4] t: new helper test_line_count_cmd","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2021-06-12T04:27:51Z","receivedAt":"2021-06-12T04:28:13Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"This is a rewrite of a series I sent earlier [1].\nWhich was a response to a false positive when applying Junio's suggestion\nto a series written by Ævar [2].\n\n1: https://lore.kernel.org/git/20210421104102.3409-1-congdanhqx@gmail.com/\n2: https://lore.kernel.org/git/87r1j42ffz.fsf@evledraar.gmail.com/\n\nĐoàn Trần Công Danh (4):\n  test-lib-functions: introduce test_line_count_cmd\n  t6402: use find(1) builtin to filter instead of grep\n  t6400: use test_line_count_cmd to count # of lines in stdout\n  t6402: use test_line_count_cmd to count # of lines in stdout\n\n t/t6400-merge-df.sh     |  16 ++---\n t/t6402-merge-rename.sh | 132 +++++++++++++++++++---------------------\n t/test-lib-functions.sh |  64 +++++++++++++++++++\n 3 files changed, 134 insertions(+), 78 deletions(-)\n\n-- \n2.32.0.278.gd42b80f139\n\n"},{"id":"427164","messageId":"20210612042755.28342-4-congdanhqx@gmail.com","threadId":"55907","inReplyTo":"20210612042755.28342-1-congdanhqx@gmail.com","subject":"[PATCH 3/4] t6400: use test_line_count_cmd to count # of lines in stdout","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2021-06-12T04:27:54Z","receivedAt":"2021-06-12T04:28:19Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n---\n t/t6400-merge-df.sh | 16 ++++++++--------\n 1 file changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t6400-merge-df.sh b/t/t6400-merge-df.sh\nindex 38700d29b5..9d7b9354c5 100755\n--- a/t/t6400-merge-df.sh\n+++ b/t/t6400-merge-df.sh\n@@ -82,13 +82,13 @@ test_expect_success 'modify/delete + directory/file conflict' '\n \tgit checkout delete^0 &&\n \ttest_must_fail git merge modify &&\n \n-\ttest 5 -eq $(git ls-files -s | wc -l) &&\n-\ttest 4 -eq $(git ls-files -u | wc -l) &&\n+\ttest_line_count_cmd --out = 5 git ls-files -s &&\n+\ttest_line_count_cmd --out = 4 git ls-files -u  &&\n \tif test \"$GIT_TEST_MERGE_ALGORITHM\" = ort\n \tthen\n-\t\ttest 0 -eq $(git ls-files -o | wc -l)\n+\t\ttest_line_count_cmd --out = 2 git ls-files -o\n \telse\n-\t\ttest 1 -eq $(git ls-files -o | wc -l)\n+\t\ttest_line_count_cmd --out = 3 git ls-files -o\n \tfi &&\n \n \ttest_path_is_file letters/file &&\n@@ -103,13 +103,13 @@ test_expect_success 'modify/delete + directory/file conflict; other way' '\n \n \ttest_must_fail git merge delete &&\n \n-\ttest 5 -eq $(git ls-files -s | wc -l) &&\n-\ttest 4 -eq $(git ls-files -u | wc -l) &&\n+\ttest_line_count_cmd --out = 5 git ls-files -s  &&\n+\ttest_line_count_cmd --out = 4 git ls-files -u  &&\n \tif test \"$GIT_TEST_MERGE_ALGORITHM\" = ort\n \tthen\n-\t\ttest 0 -eq $(git ls-files -o | wc -l)\n+\t\ttest_line_count_cmd --out = 2 git ls-files -o\n \telse\n-\t\ttest 1 -eq $(git ls-files -o | wc -l)\n+\t\ttest_line_count_cmd --out = 3 git ls-files -o\n \tfi &&\n \n \ttest_path_is_file letters/file &&\n-- \n2.32.0.278.gd42b80f139\n\n"},{"id":"427165","messageId":"20210612042755.28342-3-congdanhqx@gmail.com","threadId":"55907","inReplyTo":"20210612042755.28342-1-congdanhqx@gmail.com","subject":"[PATCH 2/4] t6402: use find(1) builtin to filter instead of grep","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2021-06-12T04:27:53Z","receivedAt":"2021-06-12T04:28:22Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"find(1) has a builtin (-prune) to filter its output, save a bit of time\nfor invoking grep(1).\n\nIn addition, in a later change, we will try to use test_line_count_cmd\nto count number of lines in stdout and/or stderr of a command, due to\nlimitation of current implementation, it can handle pipe.\n\nLet's replace grep(1)'s usage with find(1) builtin filter.\n\nSigned-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n---\n t/t6402-merge-rename.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh\nindex 425dad97d5..5d76cd6414 100755\n--- a/t/t6402-merge-rename.sh\n+++ b/t/t6402-merge-rename.sh\n@@ -546,7 +546,7 @@ then\n \n \t\ttest_must_fail git diff --quiet &&\n \n-\t\ttest 3 -eq $(find . | grep -v .git | wc -l) &&\n+\t\ttest 3 -eq $(find . -name .git -prune -o -print | wc -l) &&\n \n \t\ttest_path_is_file one &&\n \t\ttest_path_is_file two &&\n@@ -565,7 +565,7 @@ else\n \n \t\ttest_must_fail git diff --quiet &&\n \n-\t\ttest 4 -eq $(find . | grep -v .git | wc -l) &&\n+\t\ttest 4 -eq $(find . -name .git -prune -o -print | wc -l) &&\n \n \t\ttest_path_is_dir one &&\n \t\ttest_path_is_file one~rename-two &&\n@@ -593,7 +593,7 @@ test_expect_success 'pair rename to parent of other (D/F conflicts) w/ clean sta\n \n \ttest_must_fail git diff --quiet &&\n \n-\ttest 3 -eq $(find . | grep -v .git | wc -l) &&\n+\ttest 3 -eq $(find . -name .git -prune -o -print | wc -l) &&\n \n \ttest_path_is_file one &&\n \ttest_path_is_file two &&\n-- \n2.32.0.278.gd42b80f139\n\n"},{"id":"427166","messageId":"20210612042755.28342-5-congdanhqx@gmail.com","threadId":"55907","inReplyTo":"20210612042755.28342-1-congdanhqx@gmail.com","subject":"[PATCH 4/4] t6402: use test_line_count_cmd to count # of lines in stdout","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2021-06-12T04:27:55Z","receivedAt":"2021-06-12T04:28:31Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n---\n t/t6402-merge-rename.sh | 126 +++++++++++++++++++---------------------\n 1 file changed, 59 insertions(+), 67 deletions(-)\n\ndiff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh\nindex 5d76cd6414..81502dba84 100755\n--- a/t/t6402-merge-rename.sh\n+++ b/t/t6402-merge-rename.sh\n@@ -105,10 +105,8 @@ test_expect_success 'pull renaming branch into unrenaming one' \\\n \tgit show-branch &&\n \ttest_expect_code 1 git pull . white &&\n \tgit ls-files -s &&\n-\tgit ls-files -u B >b.stages &&\n-\ttest_line_count = 3 b.stages &&\n-\tgit ls-files -s N >n.stages &&\n-\ttest_line_count = 1 n.stages &&\n+\ttest_line_count_cmd --out = 3 git ls-files -u B &&\n+\ttest_line_count_cmd --out = 1 git ls-files -s N &&\n \tsed -ne \"/^g/{\n \tp\n \tq\n@@ -122,10 +120,8 @@ test_expect_success 'pull renaming branch into another renaming one' \\\n \tgit reset --hard &&\n \tgit checkout red &&\n \ttest_expect_code 1 git pull . white &&\n-\tgit ls-files -u B >b.stages &&\n-\ttest_line_count = 3 b.stages &&\n-\tgit ls-files -s N >n.stages &&\n-\ttest_line_count = 1 n.stages &&\n+\ttest_line_count_cmd --out = 3 git ls-files -u B &&\n+\ttest_line_count_cmd --out = 1 git ls-files -s N &&\n \tsed -ne \"/^g/{\n \tp\n \tq\n@@ -138,10 +134,8 @@ test_expect_success 'pull unrenaming branch into renaming one' \\\n \tgit reset --hard &&\n \tgit show-branch &&\n \ttest_expect_code 1 git pull . main &&\n-\tgit ls-files -u B >b.stages &&\n-\ttest_line_count = 3 b.stages &&\n-\tgit ls-files -s N >n.stages &&\n-\ttest_line_count = 1 n.stages &&\n+\ttest_line_count_cmd --out = 3 git ls-files -u B &&\n+\ttest_line_count_cmd --out = 1 git ls-files -s N &&\n \tsed -ne \"/^g/{\n \tp\n \tq\n@@ -154,14 +148,10 @@ test_expect_success 'pull conflicting renames' \\\n \tgit reset --hard &&\n \tgit show-branch &&\n \ttest_expect_code 1 git pull . blue &&\n-\tgit ls-files -u A >a.stages &&\n-\ttest_line_count = 1 a.stages &&\n-\tgit ls-files -u B >b.stages &&\n-\ttest_line_count = 1 b.stages &&\n-\tgit ls-files -u C >c.stages &&\n-\ttest_line_count = 1 c.stages &&\n-\tgit ls-files -s N >n.stages &&\n-\ttest_line_count = 1 n.stages &&\n+\ttest_line_count_cmd --out = 1 git ls-files -u A &&\n+\ttest_line_count_cmd --out = 1 git ls-files -u B &&\n+\ttest_line_count_cmd --out = 1 git ls-files -u C &&\n+\ttest_line_count_cmd --out = 1 git ls-files -s N &&\n \tsed -ne \"/^g/{\n \tp\n \tq\n@@ -330,8 +320,8 @@ test_expect_success 'Rename+D/F conflict; renamed file merges but dir in way' '\n \t\ttest_i18ngrep \"Adding as dir~HEAD instead\" output\n \tfi &&\n \n-\ttest 3 -eq \"$(git ls-files -u | wc -l)\" &&\n-\ttest 2 -eq \"$(git ls-files -u dir/file-in-the-way | wc -l)\" &&\n+\ttest_line_count_cmd --out = 3 git ls-files -u  &&\n+\ttest_line_count_cmd --out = 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@@ -357,8 +347,8 @@ test_expect_success 'Same as previous, but merged other way' '\n \t\ttest_i18ngrep \"Adding as dir~renamed-file-has-no-conflicts instead\" output\n \tfi &&\n \n-\ttest 3 -eq \"$(git ls-files -u | wc -l)\" &&\n-\ttest 2 -eq \"$(git ls-files -u dir/file-in-the-way | wc -l)\" &&\n+\ttest_line_count_cmd --out = 3 git ls-files -u  &&\n+\ttest_line_count_cmd --out = 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@@ -374,8 +364,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 -eq \"$(git ls-files -u | wc -l)\" &&\n-\ttest 3 -eq \"$(git ls-files -u dir | wc -l)\" &&\n+\ttest_line_count_cmd --out = 3 git ls-files -u  &&\n+\ttest_line_count_cmd --out = 3 git ls-files -u dir  &&\n \n \ttest_must_fail git diff --quiet &&\n \ttest_must_fail git diff --cached --quiet &&\n@@ -409,14 +399,15 @@ 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 -eq \"$(git ls-files -u | wc -l)\" &&\n+\ttest_line_count_cmd --out = 5 git ls-files -u  &&\n \tif test \"$GIT_TEST_MERGE_ALGORITHM\" = ort\n \tthen\n-\t\ttest 3 -eq \"$(git ls-files -u dir~HEAD | wc -l)\"\n+\t\ttest_line_count_cmd --out = 3 git ls-files -u dir~HEAD\n \telse\n-\t\ttest 3 -eq \"$(git ls-files -u dir | grep -v file-in-the-way | wc -l)\"\n+\t\tgit ls-files -u dir >out &&\n+\t\ttest_line_count_cmd --out = 3 grep -v file-in-the-way out\n \tfi &&\n-\ttest 2 -eq \"$(git ls-files -u dir/file-in-the-way | wc -l)\" &&\n+\ttest_line_count_cmd --out = 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@@ -432,14 +423,15 @@ 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 -eq \"$(git ls-files -u | wc -l)\" &&\n+\ttest_line_count_cmd --out = 5 git ls-files -u  &&\n \tif test \"$GIT_TEST_MERGE_ALGORITHM\" = ort\n \tthen\n-\t\ttest 3 -eq \"$(git ls-files -u dir~renamed-file-has-conflicts | wc -l)\"\n+\t\ttest_line_count_cmd --out = 3 git ls-files -u dir~renamed-file-has-conflicts\n \telse\n-\t\ttest 3 -eq \"$(git ls-files -u dir | grep -v file-in-the-way | wc -l)\"\n+\t\tgit ls-files -u dir >out &&\n+\t\ttest_line_count_cmd --out = 3 grep -v file-in-the-way out\n \tfi &&\n-\ttest 2 -eq \"$(git ls-files -u dir/file-in-the-way | wc -l)\" &&\n+\ttest_line_count_cmd --out = 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@@ -496,9 +488,9 @@ test_expect_success 'both rename source and destination involved in D/F conflict\n \n \tif test \"$GIT_TEST_MERGE_ALGORITHM\" = ort\n \tthen\n-\t\ttest 2 -eq \"$(git ls-files -u | wc -l)\"\n+\t\ttest_line_count_cmd --out = 2 git ls-files -u\n \telse\n-\t\ttest 1 -eq \"$(git ls-files -u | wc -l)\"\n+\t\ttest_line_count_cmd --out = 1 git ls-files -u\n \tfi &&\n \n \ttest_must_fail git diff --quiet &&\n@@ -540,9 +532,9 @@ then\n \t\tmkdir one &&\n \t\ttest_must_fail git merge --strategy=recursive rename-two &&\n \n-\t\ttest 4 -eq \"$(git ls-files -u | wc -l)\" &&\n-\t\ttest 2 -eq \"$(git ls-files -u one | wc -l)\" &&\n-\t\ttest 2 -eq \"$(git ls-files -u two | wc -l)\" &&\n+\t\ttest_line_count_cmd --out = 4 git ls-files -u  &&\n+\t\ttest_line_count_cmd --out = 2 git ls-files -u one  &&\n+\t\ttest_line_count_cmd --out = 2 git ls-files -u two  &&\n \n \t\ttest_must_fail git diff --quiet &&\n \n@@ -559,9 +551,9 @@ else\n \t\tmkdir one &&\n \t\ttest_must_fail git merge --strategy=recursive rename-two &&\n \n-\t\ttest 2 -eq \"$(git ls-files -u | wc -l)\" &&\n-\t\ttest 1 -eq \"$(git ls-files -u one | wc -l)\" &&\n-\t\ttest 1 -eq \"$(git ls-files -u two | wc -l)\" &&\n+\t\ttest_line_count_cmd --out = 2 git ls-files -u  &&\n+\t\ttest_line_count_cmd --out = 1 git ls-files -u one  &&\n+\t\ttest_line_count_cmd --out = 1 git ls-files -u two  &&\n \n \t\ttest_must_fail git diff --quiet &&\n \n@@ -582,13 +574,13 @@ test_expect_success 'pair rename to parent of other (D/F conflicts) w/ clean sta\n \n \tif test \"$GIT_TEST_MERGE_ALGORITHM\" = ort\n \tthen\n-\t\ttest 4 -eq \"$(git ls-files -u | wc -l)\" &&\n-\t\ttest 2 -eq \"$(git ls-files -u one | wc -l)\" &&\n-\t\ttest 2 -eq \"$(git ls-files -u two | wc -l)\"\n+\t\ttest_line_count_cmd --out = 4 git ls-files -u  &&\n+\t\ttest_line_count_cmd --out = 2 git ls-files -u one  &&\n+\t\ttest_line_count_cmd --out = 2 git ls-files -u two\n \telse\n-\t\ttest 2 -eq \"$(git ls-files -u | wc -l)\" &&\n-\t\ttest 1 -eq \"$(git ls-files -u one | wc -l)\" &&\n-\t\ttest 1 -eq \"$(git ls-files -u two | wc -l)\"\n+\t\ttest_line_count_cmd --out = 2 git ls-files -u  &&\n+\t\ttest_line_count_cmd --out = 1 git ls-files -u one  &&\n+\t\ttest_line_count_cmd --out = 1 git ls-files -u two\n \tfi &&\n \n \ttest_must_fail git diff --quiet &&\n@@ -631,19 +623,19 @@ test_expect_success 'check handling of differently renamed file with D/F conflic\n \n \tif test \"$GIT_TEST_MERGE_ALGORITHM\" = ort\n \tthen\n-\t\ttest 5 -eq \"$(git ls-files -s | wc -l)\" &&\n-\t\ttest 3 -eq \"$(git ls-files -u | wc -l)\" &&\n-\t\ttest 1 -eq \"$(git ls-files -u one~HEAD | wc -l)\" &&\n-\t\ttest 1 -eq \"$(git ls-files -u two~second-rename | wc -l)\" &&\n-\t\ttest 1 -eq \"$(git ls-files -u original | wc -l)\" &&\n-\t\ttest 0 -eq \"$(git ls-files -o | wc -l)\"\n+\t\ttest_line_count_cmd --out = 5 git ls-files -s  &&\n+\t\ttest_line_count_cmd --out = 3 git ls-files -u  &&\n+\t\ttest_line_count_cmd --out = 1 git ls-files -u one~HEAD  &&\n+\t\ttest_line_count_cmd --out = 1 git ls-files -u two~second-rename  &&\n+\t\ttest_line_count_cmd --out = 1 git ls-files -u original  &&\n+\t\ttest_line_count_cmd --out = 2 git ls-files -o\n \telse\n-\t\ttest 5 -eq \"$(git ls-files -s | wc -l)\" &&\n-\t\ttest 3 -eq \"$(git ls-files -u | wc -l)\" &&\n-\t\ttest 1 -eq \"$(git ls-files -u one | wc -l)\" &&\n-\t\ttest 1 -eq \"$(git ls-files -u two | wc -l)\" &&\n-\t\ttest 1 -eq \"$(git ls-files -u original | wc -l)\" &&\n-\t\ttest 2 -eq \"$(git ls-files -o | wc -l)\"\n+\t\ttest_line_count_cmd --out = 5 git ls-files -s  &&\n+\t\ttest_line_count_cmd --out = 3 git ls-files -u  &&\n+\t\ttest_line_count_cmd --out = 1 git ls-files -u one  &&\n+\t\ttest_line_count_cmd --out = 1 git ls-files -u two  &&\n+\t\ttest_line_count_cmd --out = 1 git ls-files -u original  &&\n+\t\ttest_line_count_cmd --out = 4 git ls-files -o\n \tfi &&\n \n \ttest_path_is_file one/file &&\n@@ -679,11 +671,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 -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+\ttest_line_count_cmd --out = 3 git ls-files -u  &&\n+\ttest_line_count_cmd --out = 1 git ls-files -u one  &&\n+\ttest_line_count_cmd --out = 1 git ls-files -u two  &&\n+\ttest_line_count_cmd --out = 1 git ls-files -u original  &&\n+\ttest_line_count_cmd --out = 2 git ls-files -o  &&\n \n \ttest_path_is_file one &&\n \ttest_path_is_file two &&\n@@ -861,8 +853,8 @@ test_expect_success 'setup merge of rename + small change' '\n test_expect_success 'merge rename + small change' '\n \tgit merge rename_branch &&\n \n-\ttest 1 -eq $(git ls-files -s | wc -l) &&\n-\ttest 0 -eq $(git ls-files -o | wc -l) &&\n+\ttest_line_count_cmd --out = 1 git ls-files -s  &&\n+\ttest_line_count_cmd --out = 2 git ls-files -o  &&\n \ttest $(git rev-parse HEAD:renamed_file) = $(git rev-parse HEAD~1:file)\n '\n \n-- \n2.32.0.278.gd42b80f139\n\n"},{"id":"427167","messageId":"20210612042755.28342-2-congdanhqx@gmail.com","threadId":"55907","inReplyTo":"20210612042755.28342-1-congdanhqx@gmail.com","subject":"[PATCH 1/4] test-lib-functions: introduce test_line_count_cmd","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2021-06-12T04:27:52Z","receivedAt":"2021-06-12T04:29:27Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"In Git project, we have multiple occasions that requires checking number\nof lines of text in stdout and/or stderr of a command. One of such\nexample is t6400, which checks number of files in various states.\n\nSome of those commands are Git command, and we would like to check their\nexit status.  In some of those checks, we pipe the stdout of those\ncommands to \"wc -l\" to check for line count, thus loosing the exit status.\n\nIntroduce a helper function to check for number of lines in stdout and\nstderr from those commands.\n\nThis helper will create 2 temporary files in process, thus it may affect\noutput of some checks.\n\nSigned-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n---\n\nNotes:\n    Theoretically, we could avoid those temporary files by this shenanigan:\n    \n    \t! (\n    \t  test $(\n    \t\t(\n    \t\t  test $(\n    \t\t\t  ( \"$@\" || echo \"'$*' run into failure\" >&3) | wc -l\n    \t\t\t)\n    \t\t\t\"$out_ops\" \"$out_val\" ||\n    \t\t  echo \"stdout: !$outop $outval '$*'\" >&3\n    \t\t) 2>&1 | wc -l\n    \t  ) \"$errop\" \"$errval\" ||\n    \t  echo \"stderr: !$errop $errval '$*'\" >&3\n    \t) 3>&1 | grep .\n    \n    However, it looks too complicated.\n\n t/test-lib-functions.sh | 64 +++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 64 insertions(+)\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex b823c14027..85bb31ea4c 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -817,6 +817,70 @@ test_line_count () {\n \tfi\n }\n \n+# test_line_count_cmd checks the number of lines of captured stdout and/or\n+# stderr of a command.\n+#\n+# NOTE: this helper function will create 2 temporary files named:\n+# * test_line_count_cmd_.out; and\n+# * test_line_count_cmd_.err\n+#\n+# And this helper function will remove those 2 files if the check is succeed.\n+# In case of failure, those files will be preserved.\n+test_line_count_cmd () {\n+\tlocal outop outval\n+\tlocal errop errval\n+\n+\twhile test $# -ge 3\n+\tdo\n+\t\tcase \"$1\" in\n+\t\t--out)\n+\t\t\toutop=\"$2\"\n+\t\t\toutval=\"$3\"\n+\t\t\t;;\n+\t\t--err)\n+\t\t\terrop=\"$2\"\n+\t\t\terrval=\"$3\"\n+\t\t\t;;\n+\t\t*)\n+\t\t\tbreak\n+\t\t\t;;\n+\t\tesac\n+\t\tshift 3\n+\tdone &&\n+\tif test $# = 0 ||\n+\t   { test \"x$1\" = \"x!\" && test $# = 1 ; }\n+\tthen\n+\t\tBUG \"test_line_count_cmd: no command to be run\"\n+\tfi &&\n+\tif test -z \"$outop$errop\"\n+\tthen\n+\t\tBUG \"test_line_count_cmd: check which stream?\"\n+\tfi &&\n+\n+\tif test \"x$1\" = \"x!\" \n+\tthen\n+\t\tshift &&\n+\t\tif \"$@\" >test_line_count_cmd_.out 2>test_line_count_cmd_.err\n+\t\tthen\n+\t\t\techo \"error: '$@' succeed!\"\n+\t\t\treturn 1\n+\t\tfi\n+\telif ! \"$@\" >test_line_count_cmd_.out 2>test_line_count_cmd_.err\n+\tthen\n+\t\techo \"error: '$@' run into failure!\"\n+\t\treturn 1\n+\tfi &&\n+\tif test -n \"$outop\"\n+\tthen\n+\t\ttest_line_count \"$outop\" \"$outval\" test_line_count_cmd_.out\n+\tfi &&\n+\tif test -n \"$errop\"\n+\tthen\n+\t\ttest_line_count \"$errop\" \"$errval\" test_line_count_cmd_.err\n+\tfi &&\n+\trm -f test_line_count_cmd_.out test_line_count_cmd_.err\n+}\n+\n test_file_size () {\n \ttest \"$#\" -ne 1 && BUG \"1 param\"\n \ttest-tool path-utils file-size \"$1\"\n-- \n2.32.0.278.gd42b80f139\n\n"},{"id":"427168","messageId":"68b41d84-e9ac-6cf1-184b-5ce9bbfbb18e@gmail.com","threadId":"55907","inReplyTo":"20210612042755.28342-4-congdanhqx@gmail.com","subject":"Re: [PATCH 3/4] t6400: use test_line_count_cmd to count # of lines in stdout","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-06-12T04:33:49Z","receivedAt":"2021-06-12T04:34:54Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"Hi Đoàn,\n\n> -\ttest 5 -eq $(git ls-files -s | wc -l) &&\n> -\ttest 4 -eq $(git ls-files -u | wc -l) &&\n> +\ttest_line_count_cmd --out = 5 git ls-files -s &&\n> +\ttest_line_count_cmd --out = 4 git ls-files -u  &&\n\nI read lines above as \"Formerly I tested that 5/4 should be equal to \noutput of git ls-files -s/-u piped to wc -l, now I do the same with \ntest_line_count_cmd\".\n\nAm I right?\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"},{"id":"427203","messageId":"CAPig+cR9OnRHYxzVsd6aX=Q_5Fkm4dMxPR2n6kXE+r+cTdw5ug@mail.gmail.com","threadId":"55907","inReplyTo":"20210612042755.28342-2-congdanhqx@gmail.com","subject":"Re: [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-06-13T03:10:19Z","receivedAt":"2021-06-13T03:11:45Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Jun 12, 2021 at 12:28 AM Đoàn Trần Công Danh\n<congdanhqx@gmail.com> wrote:\n> In Git project, we have multiple occasions that requires checking number\n> of lines of text in stdout and/or stderr of a command. One of such\n> example is t6400, which checks number of files in various states.\n>\n> Some of those commands are Git command, and we would like to check their\n> exit status.  In some of those checks, we pipe the stdout of those\n> commands to \"wc -l\" to check for line count, thus loosing the exit status.\n>\n> Introduce a helper function to check for number of lines in stdout and\n> stderr from those commands.\n>\n> This helper will create 2 temporary files in process, thus it may affect\n> output of some checks.\n\nIf the presence of these files is a concern, I wonder if it would make\nsense to turn these into dot-files (leading dot in name) or shove them\ninto the .git/ directory? (Not necessarily an actionable comment; just\ntossing around some thoughts.)\n\n> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n> ---\n> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\n> @@ -817,6 +817,70 @@ test_line_count () {\n> +# test_line_count_cmd checks the number of lines of captured stdout and/or\n> +# stderr of a command.\n> +#\n> +# NOTE: this helper function will create 2 temporary files named:\n> +# * test_line_count_cmd_.out; and\n> +# * test_line_count_cmd_.err\n> +#\n> +# And this helper function will remove those 2 files if the check is succeed.\n> +# In case of failure, those files will be preserved.\n> +test_line_count_cmd () {\n\nIt would be good to have some usage information in the above comment\nso that people aren't forced to consult the implementation to learn\nwhat options this function takes. At minimum, it should mention\n`--out`, `--err`, and `!`, and should explain the arguments each\noption takes (even if just through examples).\n\n> +       local outop outval\n> +       local errop errval\n> +\n> +       while test $# -ge 3\n> +       do\n> +               case \"$1\" in\n> +               --out)\n> +                       outop=\"$2\"\n> +                       outval=\"$3\"\n> +                       ;;\n> +               --err)\n> +                       errop=\"$2\"\n> +                       errval=\"$3\"\n> +                       ;;\n> +               *)\n> +                       break\n> +                       ;;\n> +               esac\n> +               shift 3\n> +       done &&\n\nThis is really minor, but if test_line_count_cmd() ever learns some\nnew option and that option does not consume two arguments, then the\n`shift 3` at the end of the `case/esac` will need to be adjusted in\nsome fashion. It might make more sense, therefore, to perform the\n`shift 3` closer to where it is needed (that is, in the `--out` case\nand in the `--err` case) rather than delaying it as is done here. (Not\nnecessarily worth a re-roll.)\n\nAnother minor comment: Since you're &&-chaining everything else in the\nfunction, it wouldn't hurt to also do so for the `local` declarations\nand the assignments within each `case` arm, and to chain `esac` with\n`shift 3`. Doing so could help some future programmer who might (for\nsome reason) insert code above the `while` loop, thinking that a\nfailure in the new code will abort the function, but not realizing\nthat the &&-chain is not intact in this area of the code.\n\n> +       if test $# = 0 ||\n> +          { test \"x$1\" = \"x!\" && test $# = 1 ; }\n> +       then\n> +               BUG \"test_line_count_cmd: no command to be run\"\n> +       fi &&\n> +       if test -z \"$outop$errop\"\n> +       then\n> +               BUG \"test_line_count_cmd: check which stream?\"\n> +       fi &&\n> +\n> +       if test \"x$1\" = \"x!\"\n> +       then\n> +               shift &&\n> +               if \"$@\" >test_line_count_cmd_.out 2>test_line_count_cmd_.err\n> +               then\n> +                       echo \"error: '$@' succeed!\"\n> +                       return 1\n> +               fi\n> +       elif ! \"$@\" >test_line_count_cmd_.out 2>test_line_count_cmd_.err\n> +       then\n> +               echo \"error: '$@' run into failure!\"\n> +               return 1\n> +       fi &&\n\nDo we care that the \"!\" negated case doesn't have the same semantics\nas test_must_fail()? If we do care, then should there be a way to tell\nthe function whether we want test_must_fail() semantics or `!`\nsemantics (i.e. whether we're running a Git command or a non-Git\ncommand) or should it infer it on its own? (These are genuine\nquestions -- not necessarily requests for changes -- as I'm trying to\nreason through the implications of this implementation choice.)\n\n> +       if test -n \"$outop\"\n> +       then\n> +               test_line_count \"$outop\" \"$outval\" test_line_count_cmd_.out\n> +       fi &&\n> +       if test -n \"$errop\"\n> +       then\n> +               test_line_count \"$errop\" \"$errval\" test_line_count_cmd_.err\n> +       fi &&\n> +       rm -f test_line_count_cmd_.out test_line_count_cmd_.err\n> +}\n"},{"id":"427206","messageId":"CAPig+cRNqX3F3GmYP3X4Z-cY2aRGFac9NZ3=KymW43etNaDjEw@mail.gmail.com","threadId":"55907","inReplyTo":"20210612042755.28342-4-congdanhqx@gmail.com","subject":"Re: [PATCH 3/4] t6400: use test_line_count_cmd to count # of lines in stdout","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-06-13T03:39:51Z","receivedAt":"2021-06-13T03:40:35Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Jun 12, 2021 at 12:28 AM Đoàn Trần Công Danh\n<congdanhqx@gmail.com> wrote:\n>\n> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n> ---\n> diff --git a/t/t6400-merge-df.sh b/t/t6400-merge-df.sh\n> @@ -82,13 +82,13 @@ test_expect_success 'modify/delete + directory/file conflict' '\n> -       test 5 -eq $(git ls-files -s | wc -l) &&\n> -       test 4 -eq $(git ls-files -u | wc -l) &&\n> +       test_line_count_cmd --out = 5 git ls-files -s &&\n> +       test_line_count_cmd --out = 4 git ls-files -u  &&\n\nNit: too many spaces before the `&&` on the second line: s/\\s+/ /\n\n> @@ -103,13 +103,13 @@ test_expect_success 'modify/delete + directory/file conflict; other way' '\n> -       test 5 -eq $(git ls-files -s | wc -l) &&\n> -       test 4 -eq $(git ls-files -u | wc -l) &&\n> +       test_line_count_cmd --out = 5 git ls-files -s  &&\n> +       test_line_count_cmd --out = 4 git ls-files -u  &&\n\nNit: too many spaces before the `&&` on both lines.\n"},{"id":"427207","messageId":"CAPig+cS7Xh4L3YzStoU44=217jEBtFKWja7k=mTDhuHK3HBPVw@mail.gmail.com","threadId":"55907","inReplyTo":"20210612042755.28342-5-congdanhqx@gmail.com","subject":"Re: [PATCH 4/4] t6402: use test_line_count_cmd to count # of lines in stdout","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-06-13T03:43:08Z","receivedAt":"2021-06-13T03:43:36Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Jun 12, 2021 at 12:28 AM Đoàn Trần Công Danh\n<congdanhqx@gmail.com> wrote:\n> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n> ---\n> diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh\n> @@ -330,8 +320,8 @@ test_expect_success 'Rename+D/F conflict; renamed file merges but dir in way' '\n> -       test 3 -eq \"$(git ls-files -u | wc -l)\" &&\n> -       test 2 -eq \"$(git ls-files -u dir/file-in-the-way | wc -l)\" &&\n> +       test_line_count_cmd --out = 3 git ls-files -u  &&\n> +       test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way  &&\n\nNit: too many spaces before `&&`: s/\\s+/ /\n\n> @@ -357,8 +347,8 @@ test_expect_success 'Same as previous, but merged other way' '\n> -       test 3 -eq \"$(git ls-files -u | wc -l)\" &&\n> -       test 2 -eq \"$(git ls-files -u dir/file-in-the-way | wc -l)\" &&\n> +       test_line_count_cmd --out = 3 git ls-files -u  &&\n> +       test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way  &&\n\nDitto.\n\n> @@ -374,8 +364,8 @@ test_expect_success 'Rename+D/F conflict; renamed file cannot merge, dir not in\n> -       test 3 -eq \"$(git ls-files -u | wc -l)\" &&\n> -       test 3 -eq \"$(git ls-files -u dir | wc -l)\" &&\n> +       test_line_count_cmd --out = 3 git ls-files -u  &&\n> +       test_line_count_cmd --out = 3 git ls-files -u dir  &&\n\nDitto.\n\n> @@ -409,14 +399,15 @@ test_expect_success 'Rename+D/F conflict; renamed file cannot merge and dir in t\n> -       test 5 -eq \"$(git ls-files -u | wc -l)\" &&\n> +       test_line_count_cmd --out = 5 git ls-files -u  &&\n>         if test \"$GIT_TEST_MERGE_ALGORITHM\" = ort\n>         then\n> -               test 3 -eq \"$(git ls-files -u dir~HEAD | wc -l)\"\n> +               test_line_count_cmd --out = 3 git ls-files -u dir~HEAD\n>         else\n> -               test 3 -eq \"$(git ls-files -u dir | grep -v file-in-the-way | wc -l)\"\n> +               git ls-files -u dir >out &&\n> +               test_line_count_cmd --out = 3 grep -v file-in-the-way out\n>         fi &&\n> -       test 2 -eq \"$(git ls-files -u dir/file-in-the-way | wc -l)\" &&\n> +       test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way  &&\n\nAnd again.\n\n> @@ -432,14 +423,15 @@ test_expect_success 'Same as previous, but merged other way' '\n> -       test 5 -eq \"$(git ls-files -u | wc -l)\" &&\n> +       test_line_count_cmd --out = 5 git ls-files -u  &&\n\nHere too.\n\n> -       test 2 -eq \"$(git ls-files -u dir/file-in-the-way | wc -l)\" &&\n> +       test_line_count_cmd --out = 2 git ls-files -u dir/file-in-the-way  &&\n\nBlorp.\n\nAnd the rest of the changes suffer the same problem, so I won't\ncontinue commenting...\n"},{"id":"427221","messageId":"YMW1kH4PZWx2NWjW@danh.dev","threadId":"55907","inReplyTo":"CAPig+cR9OnRHYxzVsd6aX=Q_5Fkm4dMxPR2n6kXE+r+cTdw5ug@mail.gmail.com","subject":"Re: [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2021-06-13T07:36:48Z","receivedAt":"2021-06-13T07:37:05Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2021-06-12 23:10:19-0400, Eric Sunshine <sunshine@sunshineco.com> wrote:\n> On Sat, Jun 12, 2021 at 12:28 AM Đoàn Trần Công Danh\n> <congdanhqx@gmail.com> wrote:\n> > In Git project, we have multiple occasions that requires checking number\n> > of lines of text in stdout and/or stderr of a command. One of such\n> > example is t6400, which checks number of files in various states.\n> >\n> > Some of those commands are Git command, and we would like to check their\n> > exit status.  In some of those checks, we pipe the stdout of those\n> > commands to \"wc -l\" to check for line count, thus loosing the exit status.\n> >\n> > Introduce a helper function to check for number of lines in stdout and\n> > stderr from those commands.\n> >\n> > This helper will create 2 temporary files in process, thus it may affect\n> > output of some checks.\n> \n> If the presence of these files is a concern, I wonder if it would make\n> sense to turn these into dot-files (leading dot in name) or shove them\n> into the .git/ directory? (Not necessarily an actionable comment; just\n> tossing around some thoughts.)\n\nThe presence of those files is concern to some command likes:\n\n* git ls-files -o; or\n* ls -a\n\nShoving into \".git\" would be actionable if we are testing inside a git\nrepository. Should we testing outside, we don't have that luxury.\n\nI think we can accept that limitation (only allow this helper inside\na Git worktree to avoid surpising behavior).\n\n> \n> > Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n> > ---\n> > diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\n> > @@ -817,6 +817,70 @@ test_line_count () {\n> > +# test_line_count_cmd checks the number of lines of captured stdout and/or\n> > +# stderr of a command.\n> > +#\n> > +# NOTE: this helper function will create 2 temporary files named:\n> > +# * test_line_count_cmd_.out; and\n> > +# * test_line_count_cmd_.err\n> > +#\n> > +# And this helper function will remove those 2 files if the check is succeed.\n> > +# In case of failure, those files will be preserved.\n> > +test_line_count_cmd () {\n> \n> It would be good to have some usage information in the above comment\n> so that people aren't forced to consult the implementation to learn\n> what options this function takes. At minimum, it should mention\n> `--out`, `--err`, and `!`, and should explain the arguments each\n> option takes (even if just through examples).\n\nMake sense!\n\n> \n> > +       local outop outval\n> > +       local errop errval\n> > +\n> > +       while test $# -ge 3\n> > +       do\n> > +               case \"$1\" in\n> > +               --out)\n> > +                       outop=\"$2\"\n> > +                       outval=\"$3\"\n> > +                       ;;\n> > +               --err)\n> > +                       errop=\"$2\"\n> > +                       errval=\"$3\"\n> > +                       ;;\n> > +               *)\n> > +                       break\n> > +                       ;;\n> > +               esac\n> > +               shift 3\n> > +       done &&\n> \n> This is really minor, but if test_line_count_cmd() ever learns some\n> new option and that option does not consume two arguments, then the\n> `shift 3` at the end of the `case/esac` will need to be adjusted in\n> some fashion. It might make more sense, therefore, to perform the\n> `shift 3` closer to where it is needed (that is, in the `--out` case\n> and in the `--err` case) rather than delaying it as is done here. (Not\n> necessarily worth a re-roll.)\n\nMake sense, too. I think I'll add another leg here for \"-*\" to avoid\npotential break when adding/removing options. Not that I imagine about\nnew options, but I can't think of any command starts with \"-\"\n\n> Another minor comment: Since you're &&-chaining everything else in the\n> function, it wouldn't hurt to also do so for the `local` declarations\n> and the assignments within each `case` arm, and to chain `esac` with\n> `shift 3`. Doing so could help some future programmer who might (for\n> some reason) insert code above the `while` loop, thinking that a\n> failure in the new code will abort the function, but not realizing\n> that the &&-chain is not intact in this area of the code.\n\nWill do, too!\n\n> > +       if test $# = 0 ||\n> > +          { test \"x$1\" = \"x!\" && test $# = 1 ; }\n> > +       then\n> > +               BUG \"test_line_count_cmd: no command to be run\"\n> > +       fi &&\n> > +       if test -z \"$outop$errop\"\n> > +       then\n> > +               BUG \"test_line_count_cmd: check which stream?\"\n> > +       fi &&\n> > +\n> > +       if test \"x$1\" = \"x!\"\n> > +       then\n> > +               shift &&\n> > +               if \"$@\" >test_line_count_cmd_.out 2>test_line_count_cmd_.err\n> > +               then\n> > +                       echo \"error: '$@' succeed!\"\n> > +                       return 1\n> > +               fi\n> > +       elif ! \"$@\" >test_line_count_cmd_.out 2>test_line_count_cmd_.err\n> > +       then\n> > +               echo \"error: '$@' run into failure!\"\n> > +               return 1\n> > +       fi &&\n> \n> Do we care that the \"!\" negated case doesn't have the same semantics\n> as test_must_fail()? If we do care, then should there be a way to tell\n> the function whether we want test_must_fail() semantics or `!`\n> semantics (i.e. whether we're running a Git command or a non-Git\n> command) or should it infer it on its own? (These are genuine\n> questions -- not necessarily requests for changes -- as I'm trying to\n> reason through the implications of this implementation choice.)\n\nI think that we shouldn't care, in my mind, I would let users to chain\ntest_line_count and test_must_fail if they do care about that sematic.\nSo, we have the freedom to use test_line_count_cmd with both Git and\nnon-Git commands.  E.g:\n\n---------8<---------------\ndiff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh\nindex ad4746d899..94c8cfc9f2 100755\n--- a/t/t0040-parse-options.sh\n+++ b/t/t0040-parse-options.sh\n@@ -288,9 +288,8 @@ test_expect_success 'OPT_CALLBACK() and OPT_BIT() work' '\n '\n \n test_expect_success 'OPT_CALLBACK() and callback errors work' '\n-\ttest_must_fail test-tool parse-options --no-length >output 2>output.err &&\n-\ttest_must_be_empty output &&\n-\ttest_must_be_empty output.err\n+\ttest_line_count_cmd --out = 0 --err = 0 \\\n+\t\ttest_must_fail test-tool parse-options --no-length\n '\n \n cat >expect <<\\EOF\n------------>8--------------\n> \n> > +       if test -n \"$outop\"\n> > +       then\n> > +               test_line_count \"$outop\" \"$outval\" test_line_count_cmd_.out\n> > +       fi &&\n> > +       if test -n \"$errop\"\n> > +       then\n> > +               test_line_count \"$errop\" \"$errval\" test_line_count_cmd_.err\n> > +       fi &&\n> > +       rm -f test_line_count_cmd_.out test_line_count_cmd_.err\n> > +}\n\n-- \nDanh\n"},{"id":"427222","messageId":"YMW2H3duLOgEObBP@danh.dev","threadId":"55907","inReplyTo":"68b41d84-e9ac-6cf1-184b-5ce9bbfbb18e@gmail.com","subject":"Re: [PATCH 3/4] t6400: use test_line_count_cmd to count # of lines in stdout","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2021-06-13T07:39:11Z","receivedAt":"2021-06-13T07:39:32Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2021-06-12 11:33:49+0700, Bagas Sanjaya <bagasdotme@gmail.com> wrote:\n> Hi Đoàn,\n\nIt's better to call me Danh. Thanks. Don't worry, I was called Dan all\nthe time, too ;)\n\n> \n> > -\ttest 5 -eq $(git ls-files -s | wc -l) &&\n> > -\ttest 4 -eq $(git ls-files -u | wc -l) &&\n> > +\ttest_line_count_cmd --out = 5 git ls-files -s &&\n> > +\ttest_line_count_cmd --out = 4 git ls-files -u  &&\n> \n> I read lines above as \"Formerly I tested that 5/4 should be equal to output\n> of git ls-files -s/-u piped to wc -l, now I do the same with\n> test_line_count_cmd\".\n> \n> Am I right?\n\nYes, you read that right. This series is mostly a cleanup for\nlinting \"test .* -o\" (But that regex wouldn't be put to be used\nanytime soon).\n\n-- \nDanh\n"},{"id":"427223","messageId":"YMW21lasp0orYfjZ@danh.dev","threadId":"55907","inReplyTo":"CAPig+cRNqX3F3GmYP3X4Z-cY2aRGFac9NZ3=KymW43etNaDjEw@mail.gmail.com","subject":"Re: [PATCH 3/4] t6400: use test_line_count_cmd to count # of lines in stdout","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2021-06-13T07:42:14Z","receivedAt":"2021-06-13T07:42:20Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2021-06-12 23:39:51-0400, Eric Sunshine <sunshine@sunshineco.com> wrote:\n> On Sat, Jun 12, 2021 at 12:28 AM Đoàn Trần Công Danh\n> <congdanhqx@gmail.com> wrote:\n> >\n> > Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n> > ---\n> > diff --git a/t/t6400-merge-df.sh b/t/t6400-merge-df.sh\n> > @@ -82,13 +82,13 @@ test_expect_success 'modify/delete + directory/file conflict' '\n> > -       test 5 -eq $(git ls-files -s | wc -l) &&\n> > -       test 4 -eq $(git ls-files -u | wc -l) &&\n> > +       test_line_count_cmd --out = 5 git ls-files -s &&\n> > +       test_line_count_cmd --out = 4 git ls-files -u  &&\n> \n> Nit: too many spaces before the `&&` on the second line: s/\\s+/ /\n\nThanks, I'll update them, later.\nIn my defence, this patch was generated mechanically with:\n\n\t:%s/test \\([0-9]\\+\\) -eq \"*[$][(]\\(.*\\)|.*[)]\"*/test_line_count_cmd --out = \\1 \\2/gc\n\nI forgot to include the space after final \")\"\n\n> > @@ -103,13 +103,13 @@ test_expect_success 'modify/delete + directory/file conflict; other way' '\n> > -       test 5 -eq $(git ls-files -s | wc -l) &&\n> > -       test 4 -eq $(git ls-files -u | wc -l) &&\n> > +       test_line_count_cmd --out = 5 git ls-files -s  &&\n> > +       test_line_count_cmd --out = 4 git ls-files -u  &&\n> \n> Nit: too many spaces before the `&&` on both lines.\n\n-- \nDanh\n"},{"id":"427230","messageId":"87bl8928q9.fsf@evledraar.gmail.com","threadId":"55907","inReplyTo":"20210612042755.28342-2-congdanhqx@gmail.com","subject":"Re: [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-13T13:28:58Z","receivedAt":"2021-06-13T13:31:10Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sat, Jun 12 2021, Đoàn Trần Công Danh wrote:\n\n> +\t   { test \"x$1\" = \"x!\" && test $# = 1 ; }\n> [...]\n> +\tif test \"x$1\" = \"x!\" \n\nWe don't use this test idiom in other places, it's OK to just use \"$1\" =\n\"!\". I think we're past whatever portability concern made that popular\nin some older shell code in the wild.\n"},{"id":"427232","messageId":"878s3d286c.fsf@evledraar.gmail.com","threadId":"55907","inReplyTo":"20210612042755.28342-2-congdanhqx@gmail.com","subject":"Re: [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-13T13:36:11Z","receivedAt":"2021-06-13T13:41:52Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sat, Jun 12 2021, Đoàn Trần Công Danh wrote:\n\n> In Git project, we have multiple occasions that requires checking number\n> of lines of text in stdout and/or stderr of a command. One of such\n> example is t6400, which checks number of files in various states.\n\nThanks for following up on this.\n\n> Some of those commands are Git command, and we would like to check their\n> exit status.  In some of those checks, we pipe the stdout of those\n> commands to \"wc -l\" to check for line count, thus loosing the exit status.\n>\n> Introduce a helper function to check for number of lines in stdout and\n> stderr from those commands.\n>\n> This helper will create 2 temporary files in process, thus it may affect\n> output of some checks.\n\nI think it's fine to just blindly stick the name into a file in the CWD\nas long as it's not \"actual\" or some other such obviously likely to\nconflict name.\n\nThe convention you've picked here (I'm not sure if it's existing\nalready) of naming the temp files after the test lib function name is a\ngood one.\n\nMore generally speaking we have a bunch of helpers that have this\npotential issue/bug, in practice it's not a big deal.  A test that's\nbeing overly specific and doing a test_cmp on unbounded \"find\" output or\nwhatever is likely buggy anyway.\n\nIf it ever becomes a bigger issue we can easily set up two scratch\ndirectories during the test, one for the use of the test itself, and one\nfor the internals of the test run itself.\n\n> +# test_line_count_cmd checks the number of lines of captured stdout and/or\n> +# stderr of a command.\n> +#\n> +# NOTE: this helper function will create 2 temporary files named:\n> +# * test_line_count_cmd_.out; and\n> +# * test_line_count_cmd_.err\n> +#\n> +# And this helper function will remove those 2 files if the check is succeed.\n> +# In case of failure, those files will be preserved.\n> +test_line_count_cmd () {\n> +\tlocal outop outval\n> +\tlocal errop errval\n> +\n> +\twhile test $# -ge 3\n> +\tdo\n> +\t\tcase \"$1\" in\n> +\t\t--out)\n> +\t\t\toutop=\"$2\"\n> +\t\t\toutval=\"$3\"\n> +\t\t\t;;\n> +\t\t--err)\n> +\t\t\terrop=\"$2\"\n> +\t\t\terrval=\"$3\"\n> +\t\t\t;;\n\nIt looks like the end-state of the series leaves us with no user of the\n--err option; Maybe it's good to have it anyway for completeness, or\njust skip it? ...\n\n> +\t\t*)\n> +\t\t\tbreak\n> +\t\t\t;;\n> +\t\tesac\n> +\t\tshift 3\n> +\tdone &&\n> +\tif test $# = 0 ||\n> +\t   { test \"x$1\" = \"x!\" && test $# = 1 ; }\n> +\tthen\n> +\t\tBUG \"test_line_count_cmd: no command to be run\"\n> +\tfi &&\n> +\tif test -z \"$outop$errop\"\n> +\tthen\n> +\t\tBUG \"test_line_count_cmd: check which stream?\"\n> +\tfi &&\n> +\n> +\tif test \"x$1\" = \"x!\" \n> +\tthen\n> +\t\tshift &&\n> +\t\tif \"$@\" >test_line_count_cmd_.out 2>test_line_count_cmd_.err\n> +\t\tthen\n> +\t\t\techo \"error: '$@' succeed!\"\n> +\t\t\treturn 1\n> +\t\tfi\n> +\telif ! \"$@\" >test_line_count_cmd_.out 2>test_line_count_cmd_.err\n> +\tthen\n> +\t\techo \"error: '$@' run into failure!\"\n> +\t\treturn 1\n> +\tfi &&\n\n...I think it's better to not pipe to *.err if we haven't requested it,\nso under \"-v\" etc. we can get the stderr.\n\nIf we're unifying them I think a better pattern is to only run that \"$@\"\nonce, get $?, and then act differently on that in the \"!\" and \"\"\ncases. It requires less careful reading of the critical function path,\nespecially with the differing indentation.\n\n> +\tif test -n \"$outop\"\n> +\tthen\n> +\t\ttest_line_count \"$outop\" \"$outval\" test_line_count_cmd_.out\n> +\tfi &&\n> +\tif test -n \"$errop\"\n> +\tthen\n> +\t\ttest_line_count \"$errop\" \"$errval\" test_line_count_cmd_.err\n> +\tfi &&\n> +\trm -f test_line_count_cmd_.out test_line_count_cmd_.err\n\nLet's do that \"rm -f\" as a \"test_when_finished\" before we first pipe to\nthem. It's fine to do that in a test lib function, see e.g. test_config.\n\nWe'll get the benefit of preseving these files under -di etc.\n\n> +}\n> +\n>  test_file_size () {\n>  \ttest \"$#\" -ne 1 && BUG \"1 param\"\n>  \ttest-tool path-utils file-size \"$1\"\n\n"},{"id":"427241","messageId":"YMY0SlCNy8jr8T6c@danh.dev","threadId":"55907","inReplyTo":"87bl8928q9.fsf@evledraar.gmail.com","subject":"Re: [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2021-06-13T16:37:30Z","receivedAt":"2021-06-13T16:37:36Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2021-06-13 15:28:58+0200, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n> \n> On Sat, Jun 12 2021, Đoàn Trần Công Danh wrote:\n> \n> > +\t   { test \"x$1\" = \"x!\" && test $# = 1 ; }\n> > [...]\n> > +\tif test \"x$1\" = \"x!\" \n> \n> We don't use this test idiom in other places, it's OK to just use \"$1\" =\n> \"!\". I think we're past whatever portability concern made that popular\n> in some older shell code in the wild.\n\nWe actually use this test idiom in test_i18ngrep and test_cmp_rev.\n\nI don't know enough to see where should I cut the base line.\n\n-- \nDanh\n"},{"id":"427245","messageId":"3ab4f29e-987a-c7de-0cca-a64c3bf4be47@gmail.com","threadId":"55907","inReplyTo":"87bl8928q9.fsf@evledraar.gmail.com","subject":"Re: [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-06-13T18:18:30Z","receivedAt":"2021-06-13T18:19:40Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 13/06/2021 14:28, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Sat, Jun 12 2021, Đoàn Trần Công Danh wrote:\n> \n>> +\t   { test \"x$1\" = \"x!\" && test $# = 1 ; }\n>> [...]\n>> +\tif test \"x$1\" = \"x!\"\n> \n> We don't use this test idiom in other places, it's OK to just use \"$1\" =\n> \"!\". I think we're past whatever portability concern made that popular\n> in some older shell code in the wild.\n\nSlightly off topic but if anyone is interested in the history of this \ntest idiom and why it is no longer needed there is a good article at \nhttps://www.vidarholen.net/contents/blog/?p=1035\n\nBest Wishes\n\nPhillip\n\n\n"},{"id":"427247","messageId":"60c67bdf5b77e_f569220858@natae.notmuch","threadId":"55907","inReplyTo":"3ab4f29e-987a-c7de-0cca-a64c3bf4be47@gmail.com","subject":"Re: [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-13T21:42:55Z","receivedAt":"2021-06-13T21:42:59Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Phillip Wood wrote:\n> On 13/06/2021 14:28, Ævar Arnfjörð Bjarmason wrote:\n> > \n> > On Sat, Jun 12 2021, Đoàn Trần Công Danh wrote:\n> > \n> >> +\t   { test \"x$1\" = \"x!\" && test $# = 1 ; }\n> >> [...]\n> >> +\tif test \"x$1\" = \"x!\"\n> > \n> > We don't use this test idiom in other places, it's OK to just use \"$1\" =\n> > \"!\". I think we're past whatever portability concern made that popular\n> > in some older shell code in the wild.\n> \n> Slightly off topic but if anyone is interested in the history of this \n> test idiom and why it is no longer needed there is a good article at \n> https://www.vidarholen.net/contents/blog/?p=1035\n\nVery nice! I often wondered why people did that.\n\n-- \nFelipe Contreras"},{"id":"427254","messageId":"CAPig+cRFjBXpm_WwSGBYu75z8TabCDZodYi=wKgTamB1a3aGmg@mail.gmail.com","threadId":"55907","inReplyTo":"3ab4f29e-987a-c7de-0cca-a64c3bf4be47@gmail.com","subject":"Re: [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-06-13T23:43:34Z","receivedAt":"2021-06-13T23:44:01Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Jun 13, 2021 at 2:18 PM Phillip Wood <phillip.wood123@gmail.com> wrote:\n> On 13/06/2021 14:28, Ævar Arnfjörð Bjarmason wrote:\n> > On Sat, Jun 12 2021, Đoàn Trần Công Danh wrote:\n> >> +       { test \"x$1\" = \"x!\" && test $# = 1 ; }\n> >> [...]\n> >> +    if test \"x$1\" = \"x!\"\n> >\n> > We don't use this test idiom in other places, it's OK to just use \"$1\" =\n> > \"!\". I think we're past whatever portability concern made that popular\n> > in some older shell code in the wild.\n>\n> Slightly off topic but if anyone is interested in the history of this\n> test idiom and why it is no longer needed there is a good article at\n> https://www.vidarholen.net/contents/blog/?p=1035\n\nThanks for the link to the article; it was an interesting read.\nHowever, the article does seem to say that such idioms and care may\nstill be warranted. In particular, the epilog gives an example which\nis still relevant on macOS today. (Indeed, I just tried it and it does\nerror out as the article states.) Even discounting macOS, it also\ntalks about such bugs existing as late as 2015, which isn't long ago\nby any stretch. (And, as someone whose primary -- indeed only --\ndevelopment machine is ten years old, some of the other bugs it\nmentions -- which existed as recently as ten years ago -- don't seem\nall that long ago either.)\n\nAt any rate, for those of us who are old-timers, the `\"x$foo\"` idiom\nis habit and only costs a couple extra characters, so I for one have\nno problem with its presence in the proposed patch.\n"},{"id":"427260","messageId":"xmqqy2bdyx16.fsf@gitster.g","threadId":"55907","inReplyTo":"CAPig+cRFjBXpm_WwSGBYu75z8TabCDZodYi=wKgTamB1a3aGmg@mail.gmail.com","subject":"Re: [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-14T02:56:05Z","receivedAt":"2021-06-14T02:56:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> ... However, the article does seem to say that such idioms and care may\n> still be warranted. In particular, the epilog gives an example which\n> is still relevant on macOS today. ...\n>\n> At any rate, for those of us who are old-timers, the `\"x$foo\"` idiom\n> is habit and only costs a couple extra characters, so I for one have\n> no problem with its presence in the proposed patch.\n\nYes.  I think it is prudent to use the x-prefix idiom, especially\nfor the case cited upthread like comparing $1 with an exclamation\nmark '!'.\n\n"},{"id":"427261","messageId":"xmqqtum1ywre.fsf@gitster.g","threadId":"55907","inReplyTo":"878s3d286c.fsf@evledraar.gmail.com","subject":"Re: [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-14T03:01:57Z","receivedAt":"2021-06-14T03:02:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> More generally speaking we have a bunch of helpers that have this\n> potential issue/bug, in practice it's not a big deal.  A test that's\n> being overly specific and doing a test_cmp on unbounded \"find\" output or\n> whatever is likely buggy anyway.\n\nUnless we are testing \".gitignore\", \"ls-files -o\", or \"git status\".\nCarving out $TRASH_DIRECTORY/.git/trash/ directory to store these\nthrowaway files would be less error prone from that point of view.\n\n>> +\t\tcase \"$1\" in\n>> +\t\t--out)\n>> +\t\t\toutop=\"$2\"\n>> +\t\t\toutval=\"$3\"\n>> +\t\t\t;;\n>> +\t\t--err)\n>> +\t\t\terrop=\"$2\"\n>> +\t\t\terrval=\"$3\"\n>> +\t\t\t;;\n>\n> It looks like the end-state of the series leaves us with no user of the\n> --err option; Maybe it's good to have it anyway for completeness, or\n> just skip it? ...\n\nI'd rather not to see --out added if we have not yet any use for the\nother one.  When the time comes that we want to validate the error\nstream, we may find that we want to check _both_, and we'd have to\ndiscard dead code and replace with what we want, instead of just\nadding what we want to a working code without dead code.\n"},{"id":"427488","messageId":"YMjKACeN1+k2+iBK@danh.dev","threadId":"55907","inReplyTo":"878s3d286c.fsf@evledraar.gmail.com","subject":"Re: [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2021-06-15T15:40:48Z","receivedAt":"2021-06-15T15:40:56Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2021-06-13 15:36:11+0200, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n> \n> On Sat, Jun 12 2021, Đoàn Trần Công Danh wrote:\n> \n> > In Git project, we have multiple occasions that requires checking number\n> > of lines of text in stdout and/or stderr of a command. One of such\n> > example is t6400, which checks number of files in various states.\n> \n> Thanks for following up on this.\n> \n> > Some of those commands are Git command, and we would like to check their\n> > exit status.  In some of those checks, we pipe the stdout of those\n> > commands to \"wc -l\" to check for line count, thus loosing the exit status.\n> >\n> > Introduce a helper function to check for number of lines in stdout and\n> > stderr from those commands.\n> >\n> > This helper will create 2 temporary files in process, thus it may affect\n> > output of some checks.\n> \n> I think it's fine to just blindly stick the name into a file in the CWD\n> as long as it's not \"actual\" or some other such obviously likely to\n> conflict name.\n> \n> The convention you've picked here (I'm not sure if it's existing\n> already) of naming the temp files after the test lib function name is a\n> good one.\n> \n> More generally speaking we have a bunch of helpers that have this\n> potential issue/bug, in practice it's not a big deal.  A test that's\n> being overly specific and doing a test_cmp on unbounded \"find\" output or\n> whatever is likely buggy anyway.\n\nI'm going to write into $TRASH_DIRECTORY/.git/trash if .git is-a-dir,\notherwise to $TRASH_DIRECTORY in the next reroll.\n\n> If it ever becomes a bigger issue we can easily set up two scratch\n> directories during the test, one for the use of the test itself, and one\n> for the internals of the test run itself.\n> \n> > +# test_line_count_cmd checks the number of lines of captured stdout and/or\n> > +# stderr of a command.\n> > +#\n> > +# NOTE: this helper function will create 2 temporary files named:\n> > +# * test_line_count_cmd_.out; and\n> > +# * test_line_count_cmd_.err\n> > +#\n> > +# And this helper function will remove those 2 files if the check is succeed.\n> > +# In case of failure, those files will be preserved.\n> > +test_line_count_cmd () {\n> > +\tlocal outop outval\n> > +\tlocal errop errval\n> > +\n> > +\twhile test $# -ge 3\n> > +\tdo\n> > +\t\tcase \"$1\" in\n> > +\t\t--out)\n> > +\t\t\toutop=\"$2\"\n> > +\t\t\toutval=\"$3\"\n> > +\t\t\t;;\n> > +\t\t--err)\n> > +\t\t\terrop=\"$2\"\n> > +\t\t\terrval=\"$3\"\n> > +\t\t\t;;\n> \n> It looks like the end-state of the series leaves us with no user of the\n> --err option; Maybe it's good to have it anyway for completeness, or\n> just skip it? ...\n\nIn the re-roll that being prepared, t0041 will be converted, too.\nSo --err will have a user.\n\n> > +\t\t*)\n> > +\t\t\tbreak\n> > +\t\t\t;;\n> > +\t\tesac\n> > +\t\tshift 3\n> > +\tdone &&\n> > +\tif test $# = 0 ||\n> > +\t   { test \"x$1\" = \"x!\" && test $# = 1 ; }\n> > +\tthen\n> > +\t\tBUG \"test_line_count_cmd: no command to be run\"\n> > +\tfi &&\n> > +\tif test -z \"$outop$errop\"\n> > +\tthen\n> > +\t\tBUG \"test_line_count_cmd: check which stream?\"\n> > +\tfi &&\n> > +\n> > +\tif test \"x$1\" = \"x!\" \n> > +\tthen\n> > +\t\tshift &&\n> > +\t\tif \"$@\" >test_line_count_cmd_.out 2>test_line_count_cmd_.err\n> > +\t\tthen\n> > +\t\t\techo \"error: '$@' succeed!\"\n> > +\t\t\treturn 1\n> > +\t\tfi\n> > +\telif ! \"$@\" >test_line_count_cmd_.out 2>test_line_count_cmd_.err\n> > +\tthen\n> > +\t\techo \"error: '$@' run into failure!\"\n> > +\t\treturn 1\n> > +\tfi &&\n> \n> ...I think it's better to not pipe to *.err if we haven't requested it,\n> so under \"-v\" etc. we can get the stderr.\n\nAnd with t0041 converted, it reveals some difficulty when implementing\nthis suggestion.\n\nYes, it would be nice if we can get the stderr under -v,\nhowever, -x will make our life hard, since -x also writes into\n/dev/stderr, and below command is broken:\n\n\ttest_line_count_cmd --out = 0 test_must_fail git for-each-ref --no-contains something 2>actual.err &&\n\ttest_i18ngrep ! usage actual.err\n\n> If we're unifying them I think a better pattern is to only run that \"$@\"\n> once, get $?, and then act differently on that in the \"!\" and \"\"\n> cases. It requires less careful reading of the critical function path,\n> especially with the differing indentation.\n\nThis is definitely doable.\n\nThis is a replacement patch that still has trouble with \"-x\",\nI guess I should always exec 2>*.err\n---------8<--------\nSubject: [PATCH] test-lib-functions: introduce test_line_count_cmd\n\nIn Git project, we have multiple occasions that requires checking number\nof lines of text in stdout and/or stderr of a command. One of such\nexample is t6400, which checks number of files in various states.\n\nSome of those commands are Git command, and we would like to check their\nexit status.  In some of those checks, we pipe the stdout of those\ncommands to \"wc -l\" to check for line count, thus loosing the exit status.\n\nIntroduce a helper function to check for number of lines in stdout and\nstderr from those commands.\n\nThis helper will create 2 temporary files in process, thus it may affect\noutput of some checks.\n\nSigned-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n---\n t/test-lib-functions.sh | 116 ++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 116 insertions(+)\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex f0448daa74..2261de7caa 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -845,6 +845,122 @@ test_line_count () {\n \tfi\n }\n \n+# test_line_count_cmd checks exit status, and the number of lines in\n+# captured stdout and/or stderr of a command.\n+#\n+# Usage:\n+#\n+# test_line_count_cmd [--[out|err] <binop> <value>]... [--] [!] cmd [args...]\n+#\n+# Options:\n+# --out <binop> <value>:\n+# --err <binop> <value>:\n+#\tRun sh's \"test <# of lines> <binop> <value>\" on # of lines in stdout\n+#\t(for --out) or stderr (for --err)\n+# !:\n+#\tInstead of expecting \"cmd [args...]\" succeed, expect its failure.\n+#\tNote, if command under testing is \"git\", test_must_fail should be used\n+#\tinstead of \"!\".\n+#\n+# Example:\n+#\ttest_line_count_cmd --out -ge 10 --err = 0 git tag --no-contains v1.0.0\n+#\ttest_line_count_cmd --out -le 10 ! grep some-text a-file\n+#\ttest_line_count_cmd --out = 0 test_must_fail git rev-parse --verify abcd1234\n+#\n+# NOTE:\n+# * if \"--out\" is specified, a temporary file named test_line_count_cmd_.out\n+#   will be created.\n+# * if \"--err\" is specified, a temporary file named test_line_count_cmd_.err\n+#   will be created.\n+# Those temporary files will be created under $TRASH_DIRECTORY/.git/trash\n+# if $TRASH_DIRECTORY/.git directory existed.\n+# Otherwise, they will be created in $TRASH_DIRECTORY.\n+# Those temporary files will be cleant by test_when_finished\n+test_line_count_cmd () {\n+\tlocal outop outval outfile\n+\tlocal errop errval errfile\n+\tlocal expect_failure actual_failure\n+\tlocal trashdir=\"$TRASH_DIRECTORY\"\n+\n+\tif test -d \"$TRASH_DIRECTORY/.git\"\n+\tthen\n+\t\ttrashdir=\"$TRASH_DIRECTORY/.git/trash\" &&\n+\t\tmkdir -p \"$trashdir\"\n+\tfi &&\n+\twhile test $# != 0\n+\tdo\n+\t\tcase \"$1\" in\n+\t\t--out)\n+\t\t\toutop=\"$2\" &&\n+\t\t\toutval=\"$3\" &&\n+\t\t\toutfile=\"$trashdir/test_line_count_cmd_.out\" &&\n+\t\t\tshift 3\n+\t\t\t;;\n+\t\t--err)\n+\t\t\terrop=\"$2\" &&\n+\t\t\terrval=\"$3\" &&\n+\t\t\terrfile=\"$trashdir/test_line_count_cmd_.err\" &&\n+\t\t\tshift 3\n+\t\t\t;;\n+\t\t--)\n+\t\t\tshift &&\n+\t\t\tbreak\n+\t\t\t;;\n+\t\t-*)\n+\t\t\tBUG \"test_line_count_cmd: unknown options: '$1'\"\n+\t\t\t;;\n+\t\t*)\n+\t\t\tbreak\n+\t\t\t;;\n+\t\tesac\n+\tdone &&\n+\tif test \"x$1\" = \"x!\"\n+\tthen\n+\t\tshift &&\n+\t\texpect_failure=yes\n+\tfi &&\n+\tif test $# = 0\n+\tthen\n+\t\tBUG \"test_line_count_cmd: no command to be run\"\n+\telif test -z \"$outop$errop\"\n+\tthen\n+\t\tBUG \"test_line_count_cmd: check which stream?\"\n+\telse\n+\t\tif test -n \"$outfile\"\n+\t\tthen\n+\t\t\ttest_when_finished \"rm -f '$outfile'\" &&\n+\t\t\texec 3>\"$outfile\"\n+\t\tfi &&\n+\t\tif test -n \"$errfile\"\n+\t\tthen\n+\t\t\ttest_when_finished \"rm -f '$errfile'\" &&\n+\t\t\texec 5>\"$errfile\"\n+\t\tfi &&\n+\t\tif ! \"$@\" >&3 2>&5\n+\t\tthen\n+\t\t\tactual_failure=yes\n+\t\tfi\n+\t# Don't use &4, it's used for test_must_fail\n+\tfi 3>&1 5>&2 &&\n+\tcase \"$expect_failure,$actual_failure\" in\n+\tyes,)\n+\t\techo \"error: '$@' succeed!\"\n+\t\treturn 1\n+\t\t;;\n+\t,yes)\n+\t\techo \"error: '$@' run into failure!\"\n+\t\treturn 1\n+\tesac &&\n+\tif test -n \"$outop\"\n+\tthen\n+\t\ttest_line_count \"$outop\" \"$outval\" \"$outfile\"\n+\tfi &&\n+\tif test -n \"$errop\"\n+\tthen\n+\t\ttest_line_count \"$errop\" \"$errval\" \"$errfile\"\n+\tfi\n+}\n+\n test_file_size () {\n \ttest \"$#\" -ne 1 && BUG \"1 param\"\n \ttest-tool path-utils file-size \"$1\"\n-- \n\n\n-- \nDanh\n"},{"id":"428439","messageId":"87h7hmzw37.fsf@evledraar.gmail.com","threadId":"55907","inReplyTo":"CAPig+cRFjBXpm_WwSGBYu75z8TabCDZodYi=wKgTamB1a3aGmg@mail.gmail.com","subject":"Re: [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-24T23:19:42Z","receivedAt":"2021-06-24T23:22:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sun, Jun 13 2021, Eric Sunshine wrote:\n\n> On Sun, Jun 13, 2021 at 2:18 PM Phillip Wood <phillip.wood123@gmail.com> wrote:\n>> On 13/06/2021 14:28, Ævar Arnfjörð Bjarmason wrote:\n>> > On Sat, Jun 12 2021, Đoàn Trần Công Danh wrote:\n>> >> +       { test \"x$1\" = \"x!\" && test $# = 1 ; }\n>> >> [...]\n>> >> +    if test \"x$1\" = \"x!\"\n>> >\n>> > We don't use this test idiom in other places, it's OK to just use \"$1\" =\n>> > \"!\". I think we're past whatever portability concern made that popular\n>> > in some older shell code in the wild.\n>>\n>> Slightly off topic but if anyone is interested in the history of this\n>> test idiom and why it is no longer needed there is a good article at\n>> https://www.vidarholen.net/contents/blog/?p=1035\n>\n> Thanks for the link to the article; it was an interesting read.\n> However, the article does seem to say that such idioms and care may\n> still be warranted. In particular, the epilog gives an example which\n> is still relevant on macOS today. (Indeed, I just tried it and it does\n> error out as the article states.) Even discounting macOS, it also\n> talks about such bugs existing as late as 2015, which isn't long ago\n> by any stretch. (And, as someone whose primary -- indeed only --\n> development machine is ten years old, some of the other bugs it\n> mentions -- which existed as recently as ten years ago -- don't seem\n> all that long ago either.)\n\nIt's only for the case where the first byte is \"(\" or \")\" though, so\ne.g. the use of this to compare things like command-line options and\nother things that don't start with those characters is portable, if I'm\nunderstanding the article correctly.\n\n> At any rate, for those of us who are old-timers, the `\"x$foo\"` idiom\n> is habit and only costs a couple extra characters, so I for one have\n> no problem with its presence in the proposed patch.\n\nIndeed, but it's interesting to dig and see if there's any reason for\nsuch workarounds still.\n"}]}