{"thread":{"id":"59714","subject":"[PATCH 0/3] dropping \"verbose\" test helper function","startedAt":"2023-05-08T18:59:59Z","lastAt":"2023-05-08T22:26:35Z","messageCount":6,"participants":["Jeff King","Taylor Blau"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"476789","messageId":"20230508185953.GA2108869@coredump.intra.peff.net","threadId":"59714","inReplyTo":null,"subject":"[PATCH 0/3] dropping \"verbose\" test helper function","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-05-08T18:59:53Z","receivedAt":"2023-05-08T18:59:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"I happened across an instance of the \"verbose\" helper function in a\ntest recently. I think it's clear we prefer the \"-x\" tracing to it these\ndays, and we've been slowly removing instances. This series gets rid of\nthe last few.\n\nThe actual change (and rationale) is in patch 3. Patch 1 just fixes pipe\nproblems on lines we'll be touching patch 3, and patch 2 is a cute\noptimization enabled by patch 1. I don't think any of it should be very\ncontroversial, but it can all be split up if need be.\n\n  [1/3]: t7001: avoid git on upstream of pipe\n  [2/3]: t7001: use \"ls-files --format\" instead of \"cut\"\n  [3/3]: t: drop \"verbose\" helper function\n\n t/t0020-crlf.sh            | 38 +++++++++++++++++++-------------------\n t/t1301-shared-repo.sh     |  4 ++--\n t/t3427-rebase-subtree.sh  | 12 ++++++------\n t/t4022-diff-rewrite.sh    |  2 +-\n t/t4062-diff-pickaxe.sh    |  2 +-\n t/t5304-prune.sh           | 16 ++++++++--------\n t/t6006-rev-list-format.sh |  2 +-\n t/t6501-freshen-objects.sh |  2 +-\n t/t7001-mv.sh              | 37 +++++++++++++++++++++----------------\n t/t7300-clean.sh           |  4 ++--\n t/t9902-completion.sh      | 30 +++++++++++++++---------------\n t/test-lib-functions.sh    |  9 ---------\n 12 files changed, 77 insertions(+), 81 deletions(-)\n\n-Peff\n"},{"id":"476790","messageId":"20230508190112.GA2366490@coredump.intra.peff.net","threadId":"59714","inReplyTo":"20230508185953.GA2108869@coredump.intra.peff.net","subject":"[PATCH 1/3] t7001: avoid git on upstream of pipe","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-05-08T19:01:12Z","receivedAt":"2023-05-08T19:01:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We generally avoid git on the left-hand side of a pipe, because it loses\nthe exit code of the command (and thus we'd miss things like segfaults\nor unexpected failures). In the cases in t7001, we wouldn't expect\nfailures (they are just inspecting the repository state, and are not the\nmain point of the test), but it doesn't hurt to be careful.\n\nIn all but one case here we're piping \"ls-files --stage\" to cut off the\npathname (since we compare entries before and after moving). Let's pull\nthat into a helper function to avoid repeating the slightly awkward\nreplacement.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t7001-mv.sh | 38 ++++++++++++++++++++++----------------\n 1 file changed, 22 insertions(+), 16 deletions(-)\n\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex d72cef8826..ea70419928 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -4,6 +4,11 @@ test_description='git mv in subdirs'\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-diff-data.sh\n \n+index_at_path () {\n+\tentry=$(git ls-files --stage \"$@\") &&\n+\techo \"$entry\" | cut -f 1\n+}\n+\n test_expect_success 'mv -f refreshes updated index entry' '\n \techo test >bar &&\n \tgit add bar &&\n@@ -187,7 +192,8 @@ test_expect_success \"Michael Cassar's test case\" '\n \tgit mv papers/unsorted/Thesis.pdf papers/all-papers/moo-blah.pdf &&\n \n \tT=$(git write-tree) &&\n-\tgit ls-tree -r $T | verbose grep partA/outline.txt\n+\tgit ls-tree -r $T >out &&\n+\tverbose grep partA/outline.txt out\n '\n \n rm -fr papers partA path?\n@@ -260,12 +266,12 @@ test_expect_success 'git mv should not change sha1 of moved cache entry' '\n \tgit init &&\n \techo 1 >dirty &&\n \tgit add dirty &&\n-\tentry=\"$(git ls-files --stage dirty | cut -f 1)\" &&\n+\tentry=\"$(index_at_path dirty)\" &&\n \tgit mv dirty dirty2 &&\n-\ttest \"$entry\" = \"$(git ls-files --stage dirty2 | cut -f 1)\" &&\n+\ttest \"$entry\" = \"$(index_at_path dirty2)\" &&\n \techo 2 >dirty2 &&\n \tgit mv dirty2 dirty &&\n-\ttest \"$entry\" = \"$(git ls-files --stage dirty | cut -f 1)\"\n+\ttest \"$entry\" = \"$(index_at_path dirty)\"\n '\n \n rm -f dirty dirty2\n@@ -342,7 +348,7 @@ test_expect_success 'git mv cannot move a submodule in a file' '\n '\n \n test_expect_success 'git mv moves a submodule with a .git directory and no .gitmodules' '\n-\tentry=\"$(git ls-files --stage sub | cut -f 1)\" &&\n+\tentry=\"$(index_at_path sub)\" &&\n \tgit rm .gitmodules &&\n \t(\n \t\tcd sub &&\n@@ -353,7 +359,7 @@ test_expect_success 'git mv moves a submodule with a .git directory and no .gitm\n \tmkdir mod &&\n \tgit mv sub mod/sub &&\n \ttest_path_is_missing sub &&\n-\ttest \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" &&\n+\ttest \"$entry\" = \"$(index_at_path mod/sub)\" &&\n \tgit -C mod/sub status &&\n \tgit update-index --refresh &&\n \tgit diff-files --quiet\n@@ -363,7 +369,7 @@ test_expect_success 'git mv moves a submodule with a .git directory and .gitmodu\n \trm -rf mod &&\n \tgit reset --hard &&\n \tgit submodule update &&\n-\tentry=\"$(git ls-files --stage sub | cut -f 1)\" &&\n+\tentry=\"$(index_at_path sub)\" &&\n \t(\n \t\tcd sub &&\n \t\trm -f .git &&\n@@ -373,7 +379,7 @@ test_expect_success 'git mv moves a submodule with a .git directory and .gitmodu\n \tmkdir mod &&\n \tgit mv sub mod/sub &&\n \ttest_path_is_missing sub &&\n-\ttest \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" &&\n+\ttest \"$entry\" = \"$(index_at_path mod/sub)\" &&\n \tgit -C mod/sub status &&\n \techo mod/sub >expected &&\n \tgit config -f .gitmodules submodule.sub.path >actual &&\n@@ -386,11 +392,11 @@ test_expect_success 'git mv moves a submodule with gitfile' '\n \trm -rf mod &&\n \tgit reset --hard &&\n \tgit submodule update &&\n-\tentry=\"$(git ls-files --stage sub | cut -f 1)\" &&\n+\tentry=\"$(index_at_path sub)\" &&\n \tmkdir mod &&\n \tgit -C mod mv ../sub/ . &&\n \ttest_path_is_missing sub &&\n-\ttest \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" &&\n+\ttest \"$entry\" = \"$(index_at_path mod/sub)\" &&\n \tgit -C mod/sub status &&\n \techo mod/sub >expected &&\n \tgit config -f .gitmodules submodule.sub.path >actual &&\n@@ -404,12 +410,12 @@ test_expect_success 'mv does not complain when no .gitmodules file is found' '\n \tgit reset --hard &&\n \tgit submodule update &&\n \tgit rm .gitmodules &&\n-\tentry=\"$(git ls-files --stage sub | cut -f 1)\" &&\n+\tentry=\"$(index_at_path sub)\" &&\n \tmkdir mod &&\n \tgit mv sub mod/sub 2>actual.err &&\n \ttest_must_be_empty actual.err &&\n \ttest_path_is_missing sub &&\n-\ttest \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" &&\n+\ttest \"$entry\" = \"$(index_at_path mod/sub)\" &&\n \tgit -C mod/sub status &&\n \tgit update-index --refresh &&\n \tgit diff-files --quiet\n@@ -420,7 +426,7 @@ test_expect_success 'mv will error out on a modified .gitmodules file unless sta\n \tgit reset --hard &&\n \tgit submodule update &&\n \tgit config -f .gitmodules foo.bar true &&\n-\tentry=\"$(git ls-files --stage sub | cut -f 1)\" &&\n+\tentry=\"$(index_at_path sub)\" &&\n \tmkdir mod &&\n \ttest_must_fail git mv sub mod/sub 2>actual.err &&\n \ttest_file_not_empty actual.err &&\n@@ -430,7 +436,7 @@ test_expect_success 'mv will error out on a modified .gitmodules file unless sta\n \tgit mv sub mod/sub 2>actual.err &&\n \ttest_must_be_empty actual.err &&\n \ttest_path_is_missing sub &&\n-\ttest \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" &&\n+\ttest \"$entry\" = \"$(index_at_path mod/sub)\" &&\n \tgit -C mod/sub status &&\n \tgit update-index --refresh &&\n \tgit diff-files --quiet\n@@ -442,13 +448,13 @@ test_expect_success 'mv issues a warning when section is not found in .gitmodule\n \tgit submodule update &&\n \tgit config -f .gitmodules --remove-section submodule.sub &&\n \tgit add .gitmodules &&\n-\tentry=\"$(git ls-files --stage sub | cut -f 1)\" &&\n+\tentry=\"$(index_at_path sub)\" &&\n \techo \"warning: Could not find section in .gitmodules where path=sub\" >expect.err &&\n \tmkdir mod &&\n \tgit mv sub mod/sub 2>actual.err &&\n \ttest_cmp expect.err actual.err &&\n \ttest_path_is_missing sub &&\n-\ttest \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" &&\n+\ttest \"$entry\" = \"$(index_at_path mod/sub)\" &&\n \tgit -C mod/sub status &&\n \tgit update-index --refresh &&\n \tgit diff-files --quiet\n-- \n2.40.1.802.gdef2a8734a\n\n"},{"id":"476791","messageId":"20230508190146.GB2366490@coredump.intra.peff.net","threadId":"59714","inReplyTo":"20230508185953.GA2108869@coredump.intra.peff.net","subject":"[PATCH 2/3] t7001: use \"ls-files --format\" instead of \"cut\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-05-08T19:01:46Z","receivedAt":"2023-05-08T19:01:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Since ls-files recently learned a \"--format\" option, we can use that\nrather than asking for all of \"--stage\" and then pulling out the bits we\nwant with \"cut\". That's simpler and avoids two extra processes (one for\ncut, and one for the subshell to hold the intermediate result).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t7001-mv.sh | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex ea70419928..2e6a3c0a54 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -5,8 +5,7 @@ test_description='git mv in subdirs'\n . \"$TEST_DIRECTORY\"/lib-diff-data.sh\n \n index_at_path () {\n-\tentry=$(git ls-files --stage \"$@\") &&\n-\techo \"$entry\" | cut -f 1\n+\tgit ls-files --format='%(objectmode) %(objectname) %(stage)' \"$@\"\n }\n \n test_expect_success 'mv -f refreshes updated index entry' '\n-- \n2.40.1.802.gdef2a8734a\n\n"},{"id":"476792","messageId":"20230508190457.GC2366490@coredump.intra.peff.net","threadId":"59714","inReplyTo":"20230508185953.GA2108869@coredump.intra.peff.net","subject":"[PATCH 3/3] t: drop \"verbose\" helper function","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-05-08T19:04:57Z","receivedAt":"2023-05-08T19:05:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We have a small helper function called \"verbose\", with the idea that you\ncan write:\n\n  verbose foo\n\nto get a message to stderr when the \"foo\" command fails, even if it does\nnot produce any output itself. This goes back to 8ad1652418 (t5304: use\nhelper to report failure of \"test foo = bar\", 2014-10-10). It does work,\nbut overall it has not been a big success for two reasons:\n\n  1. Test writers have to remember to put it there (and the resulting\n     test code is longer as a result).\n\n  2. It doesn't handle the opposite case (we expect \"foo\" to fail, but\n     it succeeds), leading to inconsistencies in tests (which you can\n     see in many hunks of this patch, e.g. ones involving \"has_cr\").\n\nMost importantly, we added a136f6d8ff (test-lib.sh: support -x option\nfor shell-tracing, 2014-10-10) at the same time, and it does roughly the\nsame thing. The output is not quite as succinct as \"verbose\", and you\nhave to watch out for stray shell-traces ending up in stderr. But it\nsolves both of the problems above, and has clearly become the preferred\ntool.\n\nLet's consider the \"verbose\" function a failed experiment and remove the\nlast few callers (which are all many years old, and have been dwindling\nas we remove them from scripts we touch for other reasons). It will be\none less thing for new test writers to see and wonder if they should be\nusing themselves.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI think this should be all calls. Most tests will fail after removing\nthe function, of course, but it's possible one could be hiding in an\nexpect_failure or something. I didn't see any after grepping for\n'[^-]verbose' in t/, which is not too long to look through.\n\n t/t0020-crlf.sh            | 38 +++++++++++++++++++-------------------\n t/t1301-shared-repo.sh     |  4 ++--\n t/t3427-rebase-subtree.sh  | 12 ++++++------\n t/t4022-diff-rewrite.sh    |  2 +-\n t/t4062-diff-pickaxe.sh    |  2 +-\n t/t5304-prune.sh           | 16 ++++++++--------\n t/t6006-rev-list-format.sh |  2 +-\n t/t6501-freshen-objects.sh |  2 +-\n t/t7001-mv.sh              |  2 +-\n t/t7300-clean.sh           |  4 ++--\n t/t9902-completion.sh      | 30 +++++++++++++++---------------\n t/test-lib-functions.sh    |  9 ---------\n 12 files changed, 57 insertions(+), 66 deletions(-)\n\ndiff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh\nindex 35cc8c3b39..81946e87cc 100755\n--- a/t/t0020-crlf.sh\n+++ b/t/t0020-crlf.sh\n@@ -125,7 +125,7 @@ test_expect_success 'update with autocrlf=input' '\n \tmunge_cr append dir/two &&\n \tgit update-index -- one dir/two &&\n \tdiffers=$(git diff-index --cached HEAD) &&\n-\tverbose test -z \"$differs\"\n+\ttest -z \"$differs\"\n \n '\n \n@@ -138,7 +138,7 @@ test_expect_success 'update with autocrlf=true' '\n \tmunge_cr append dir/two &&\n \tgit update-index -- one dir/two &&\n \tdiffers=$(git diff-index --cached HEAD) &&\n-\tverbose test -z \"$differs\"\n+\ttest -z \"$differs\"\n \n '\n \n@@ -153,7 +153,7 @@ test_expect_success 'checkout with autocrlf=true' '\n \ttest \"$one\" = $(git hash-object --stdin <one) &&\n \ttest \"$two\" = $(git hash-object --stdin <dir/two) &&\n \tdiffers=$(git diff-index --cached HEAD) &&\n-\tverbose test -z \"$differs\"\n+\ttest -z \"$differs\"\n '\n \n test_expect_success 'checkout with autocrlf=input' '\n@@ -167,7 +167,7 @@ test_expect_success 'checkout with autocrlf=input' '\n \ttest \"$one\" = $(git hash-object --stdin <one) &&\n \ttest \"$two\" = $(git hash-object --stdin <dir/two) &&\n \tdiffers=$(git diff-index --cached HEAD) &&\n-\tverbose test -z \"$differs\"\n+\ttest -z \"$differs\"\n '\n \n test_expect_success 'apply patch (autocrlf=input)' '\n@@ -177,7 +177,7 @@ test_expect_success 'apply patch (autocrlf=input)' '\n \tgit read-tree --reset -u HEAD &&\n \n \tgit apply patch.file &&\n-\tverbose test \"$patched\" = \"$(git hash-object --stdin <one)\"\n+\ttest \"$patched\" = \"$(git hash-object --stdin <one)\"\n '\n \n test_expect_success 'apply patch --cached (autocrlf=input)' '\n@@ -187,7 +187,7 @@ test_expect_success 'apply patch --cached (autocrlf=input)' '\n \tgit read-tree --reset -u HEAD &&\n \n \tgit apply --cached patch.file &&\n-\tverbose test \"$patched\" = $(git rev-parse :one)\n+\ttest \"$patched\" = $(git rev-parse :one)\n '\n \n test_expect_success 'apply patch --index (autocrlf=input)' '\n@@ -197,8 +197,8 @@ test_expect_success 'apply patch --index (autocrlf=input)' '\n \tgit read-tree --reset -u HEAD &&\n \n \tgit apply --index patch.file &&\n-\tverbose test \"$patched\" = $(git rev-parse :one) &&\n-\tverbose test \"$patched\" = $(git hash-object --stdin <one)\n+\ttest \"$patched\" = $(git rev-parse :one) &&\n+\ttest \"$patched\" = $(git hash-object --stdin <one)\n '\n \n test_expect_success 'apply patch (autocrlf=true)' '\n@@ -208,7 +208,7 @@ test_expect_success 'apply patch (autocrlf=true)' '\n \tgit read-tree --reset -u HEAD &&\n \n \tgit apply patch.file &&\n-\tverbose test \"$patched\" = \"$(remove_cr <one | git hash-object --stdin)\"\n+\ttest \"$patched\" = \"$(remove_cr <one | git hash-object --stdin)\"\n '\n \n test_expect_success 'apply patch --cached (autocrlf=true)' '\n@@ -218,7 +218,7 @@ test_expect_success 'apply patch --cached (autocrlf=true)' '\n \tgit read-tree --reset -u HEAD &&\n \n \tgit apply --cached patch.file &&\n-\tverbose test \"$patched\" = $(git rev-parse :one)\n+\ttest \"$patched\" = $(git rev-parse :one)\n '\n \n test_expect_success 'apply patch --index (autocrlf=true)' '\n@@ -228,8 +228,8 @@ test_expect_success 'apply patch --index (autocrlf=true)' '\n \tgit read-tree --reset -u HEAD &&\n \n \tgit apply --index patch.file &&\n-\tverbose test \"$patched\" = $(git rev-parse :one) &&\n-\tverbose test \"$patched\" = \"$(remove_cr <one | git hash-object --stdin)\"\n+\ttest \"$patched\" = $(git rev-parse :one) &&\n+\ttest \"$patched\" = \"$(remove_cr <one | git hash-object --stdin)\"\n '\n \n test_expect_success '.gitattributes says two is binary' '\n@@ -240,7 +240,7 @@ test_expect_success '.gitattributes says two is binary' '\n \tgit read-tree --reset -u HEAD &&\n \n \t! has_cr dir/two &&\n-\tverbose has_cr one &&\n+\thas_cr one &&\n \t! has_cr three\n '\n \n@@ -259,8 +259,8 @@ test_expect_success '.gitattributes says two and three are text' '\n \techo \"t* crlf\" >.gitattributes &&\n \tgit read-tree --reset -u HEAD &&\n \n-\tverbose has_cr dir/two &&\n-\tverbose has_cr three\n+\thas_cr dir/two &&\n+\thas_cr three\n '\n \n test_expect_success 'in-tree .gitattributes (1)' '\n@@ -273,7 +273,7 @@ test_expect_success 'in-tree .gitattributes (1)' '\n \tgit read-tree --reset -u HEAD &&\n \n \t! has_cr one &&\n-\tverbose has_cr three\n+\thas_cr three\n '\n \n test_expect_success 'in-tree .gitattributes (2)' '\n@@ -283,7 +283,7 @@ test_expect_success 'in-tree .gitattributes (2)' '\n \tgit checkout-index -f -q -u -a &&\n \n \t! has_cr one &&\n-\tverbose has_cr three\n+\thas_cr three\n '\n \n test_expect_success 'in-tree .gitattributes (3)' '\n@@ -294,7 +294,7 @@ test_expect_success 'in-tree .gitattributes (3)' '\n \tgit checkout-index -u one dir/two three &&\n \n \t! has_cr one &&\n-\tverbose has_cr three\n+\thas_cr three\n '\n \n test_expect_success 'in-tree .gitattributes (4)' '\n@@ -305,7 +305,7 @@ test_expect_success 'in-tree .gitattributes (4)' '\n \tgit checkout-index -u .gitattributes &&\n \n \t! has_cr one &&\n-\tverbose has_cr three\n+\thas_cr three\n '\n \n test_expect_success 'checkout with existing .gitattributes' '\ndiff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh\nindex 1b6437ec07..ae5cd3f5a0 100755\n--- a/t/t1301-shared-repo.sh\n+++ b/t/t1301-shared-repo.sh\n@@ -89,7 +89,7 @@ do\n \t\trm -f .git/info/refs &&\n \t\tgit update-server-info &&\n \t\tactual=\"$(test_modebits .git/info/refs)\" &&\n-\t\tverbose test \"x$actual\" = \"x-$y\"\n+\t\ttest \"x$actual\" = \"x-$y\"\n \n \t'\n \n@@ -99,7 +99,7 @@ do\n \t\trm -f .git/info/refs &&\n \t\tgit update-server-info &&\n \t\tactual=\"$(test_modebits .git/info/refs)\" &&\n-\t\tverbose test \"x$actual\" = \"x-$x\"\n+\t\ttest \"x$actual\" = \"x-$x\"\n \n \t'\n \ndiff --git a/t/t3427-rebase-subtree.sh b/t/t3427-rebase-subtree.sh\nindex 48b76f8232..1b3e97c875 100755\n--- a/t/t3427-rebase-subtree.sh\n+++ b/t/t3427-rebase-subtree.sh\n@@ -74,9 +74,9 @@ test_expect_success 'Rebase -Xsubtree --empty=ask --onto commit' '\n \ttest_must_fail git rebase -Xsubtree=files_subtree --empty=ask --onto files-main main &&\n \t: first pick results in no changes &&\n \tgit rebase --skip &&\n-\tverbose test \"$(commit_message HEAD~2)\" = \"topic_4\" &&\n-\tverbose test \"$(commit_message HEAD~)\" = \"files_subtree/topic_5\" &&\n-\tverbose test \"$(commit_message HEAD)\" = \"Empty commit\"\n+\ttest \"$(commit_message HEAD~2)\" = \"topic_4\" &&\n+\ttest \"$(commit_message HEAD~)\" = \"files_subtree/topic_5\" &&\n+\ttest \"$(commit_message HEAD)\" = \"Empty commit\"\n '\n \n test_expect_success 'Rebase -Xsubtree --empty=ask --rebase-merges --onto commit' '\n@@ -85,9 +85,9 @@ test_expect_success 'Rebase -Xsubtree --empty=ask --rebase-merges --onto commit'\n \ttest_must_fail git rebase -Xsubtree=files_subtree --empty=ask --rebase-merges --onto files-main --root &&\n \t: first pick results in no changes &&\n \tgit rebase --skip &&\n-\tverbose test \"$(commit_message HEAD~2)\" = \"topic_4\" &&\n-\tverbose test \"$(commit_message HEAD~)\" = \"files_subtree/topic_5\" &&\n-\tverbose test \"$(commit_message HEAD)\" = \"Empty commit\"\n+\ttest \"$(commit_message HEAD~2)\" = \"topic_4\" &&\n+\ttest \"$(commit_message HEAD~)\" = \"files_subtree/topic_5\" &&\n+\ttest \"$(commit_message HEAD)\" = \"Empty commit\"\n '\n \n test_done\ndiff --git a/t/t4022-diff-rewrite.sh b/t/t4022-diff-rewrite.sh\nindex 1c89050a97..6fed993ea0 100755\n--- a/t/t4022-diff-rewrite.sh\n+++ b/t/t4022-diff-rewrite.sh\n@@ -24,7 +24,7 @@ test_expect_success setup '\n test_expect_success 'detect rewrite' '\n \n \tactual=$(git diff-files -B --summary test) &&\n-\tverbose expr \"$actual\" : \" rewrite test ([0-9]*%)$\"\n+\texpr \"$actual\" : \" rewrite test ([0-9]*%)$\"\n \n '\n \ndiff --git a/t/t4062-diff-pickaxe.sh b/t/t4062-diff-pickaxe.sh\nindex 9aaa068ed9..a90b46b678 100755\n--- a/t/t4062-diff-pickaxe.sh\n+++ b/t/t4062-diff-pickaxe.sh\n@@ -24,7 +24,7 @@ test_expect_success '-G matches' '\n \n test_expect_success '-S --pickaxe-regex' '\n \tgit diff --name-only -S0 --pickaxe-regex HEAD^ >out &&\n-\tverbose test 4096-zeroes.txt = \"$(cat out)\"\n+\ttest 4096-zeroes.txt = \"$(cat out)\"\n '\n \n test_done\ndiff --git a/t/t5304-prune.sh b/t/t5304-prune.sh\nindex 662ae9b152..f367327441 100755\n--- a/t/t5304-prune.sh\n+++ b/t/t5304-prune.sh\n@@ -16,7 +16,7 @@ add_blob() {\n \tbefore=$(git count-objects | sed \"s/ .*//\") &&\n \tBLOB=$(echo aleph_0 | git hash-object -w --stdin) &&\n \tBLOB_FILE=.git/objects/$(echo $BLOB | sed \"s/^../&\\//\") &&\n-\tverbose test $((1 + $before)) = $(git count-objects | sed \"s/ .*//\") &&\n+\ttest $((1 + $before)) = $(git count-objects | sed \"s/ .*//\") &&\n \ttest_path_is_file $BLOB_FILE &&\n \ttest-tool chmtime =+0 $BLOB_FILE\n }\n@@ -51,23 +51,23 @@ test_expect_success 'prune stale packs' '\n test_expect_success 'prune --expire' '\n \tadd_blob &&\n \tgit prune --expire=1.hour.ago &&\n-\tverbose test $((1 + $before)) = $(git count-objects | sed \"s/ .*//\") &&\n+\ttest $((1 + $before)) = $(git count-objects | sed \"s/ .*//\") &&\n \ttest_path_is_file $BLOB_FILE &&\n \ttest-tool chmtime =-86500 $BLOB_FILE &&\n \tgit prune --expire 1.day &&\n-\tverbose test $before = $(git count-objects | sed \"s/ .*//\") &&\n+\ttest $before = $(git count-objects | sed \"s/ .*//\") &&\n \ttest_path_is_missing $BLOB_FILE\n '\n \n test_expect_success 'gc: implicit prune --expire' '\n \tadd_blob &&\n \ttest-tool chmtime =-$((2*$week-30)) $BLOB_FILE &&\n \tgit gc --no-cruft &&\n-\tverbose test $((1 + $before)) = $(git count-objects | sed \"s/ .*//\") &&\n+\ttest $((1 + $before)) = $(git count-objects | sed \"s/ .*//\") &&\n \ttest_path_is_file $BLOB_FILE &&\n \ttest-tool chmtime =-$((2*$week+1)) $BLOB_FILE &&\n \tgit gc --no-cruft &&\n-\tverbose test $before = $(git count-objects | sed \"s/ .*//\") &&\n+\ttest $before = $(git count-objects | sed \"s/ .*//\") &&\n \ttest_path_is_missing $BLOB_FILE\n '\n \n@@ -138,7 +138,7 @@ test_expect_success 'gc --no-prune' '\n \ttest-tool chmtime =-$((5001*$day)) $BLOB_FILE &&\n \tgit config gc.pruneExpire 2.days.ago &&\n \tgit gc --no-prune --no-cruft &&\n-\tverbose test 1 = $(git count-objects | sed \"s/ .*//\") &&\n+\ttest 1 = $(git count-objects | sed \"s/ .*//\") &&\n \ttest_path_is_file $BLOB_FILE\n '\n \n@@ -192,10 +192,10 @@ test_expect_success 'gc: prune old objects after local clone' '\n \tgit clone --no-hardlinks . aclone &&\n \t(\n \t\tcd aclone &&\n-\t\tverbose test 1 = $(git count-objects | sed \"s/ .*//\") &&\n+\t\ttest 1 = $(git count-objects | sed \"s/ .*//\") &&\n \t\ttest_path_is_file $BLOB_FILE &&\n \t\tgit gc --prune --no-cruft &&\n-\t\tverbose test 0 = $(git count-objects | sed \"s/ .*//\") &&\n+\t\ttest 0 = $(git count-objects | sed \"s/ .*//\") &&\n \t\ttest_path_is_missing $BLOB_FILE\n \t)\n '\ndiff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh\nindex 41d0ca00b1..573eb97a0f 100755\n--- a/t/t6006-rev-list-format.sh\n+++ b/t/t6006-rev-list-format.sh\n@@ -493,7 +493,7 @@ test_expect_success 'empty email' '\n \ttest_tick &&\n \tC=$(GIT_AUTHOR_EMAIL= git commit-tree HEAD^{tree} </dev/null) &&\n \tA=$(git show --pretty=format:%an,%ae,%ad%n -s $C) &&\n-\tverbose test \"$A\" = \"$GIT_AUTHOR_NAME,,Thu Apr 7 15:14:13 2005 -0700\"\n+\ttest \"$A\" = \"$GIT_AUTHOR_NAME,,Thu Apr 7 15:14:13 2005 -0700\"\n '\n \n test_expect_success 'del LF before empty (1)' '\ndiff --git a/t/t6501-freshen-objects.sh b/t/t6501-freshen-objects.sh\nindex dbfa8a4d4c..4521508b83 100755\n--- a/t/t6501-freshen-objects.sh\n+++ b/t/t6501-freshen-objects.sh\n@@ -152,7 +152,7 @@ test_expect_success 'do not complain about existing broken links (commit)' '\n \tEOF\n \tcommit=$(git hash-object -t commit -w broken-commit) &&\n \tgit gc --no-cruft -q 2>stderr &&\n-\tverbose git cat-file -e $commit &&\n+\tgit cat-file -e $commit &&\n \ttest_must_be_empty stderr\n '\n \ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex 2e6a3c0a54..898a920532 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -192,7 +192,7 @@ test_expect_success \"Michael Cassar's test case\" '\n \n \tT=$(git write-tree) &&\n \tgit ls-tree -r $T >out &&\n-\tverbose grep partA/outline.txt out\n+\tgrep partA/outline.txt out\n '\n \n rm -fr papers partA path?\ndiff --git a/t/t7300-clean.sh b/t/t7300-clean.sh\nindex c975eb54d2..0ef7b78457 100755\n--- a/t/t7300-clean.sh\n+++ b/t/t7300-clean.sh\n@@ -120,7 +120,7 @@ test_expect_success 'git clean with relative prefix' '\n \t\tgrep part3 |\n \t\tsed -n -e \"s|^Would remove ||p\"\n \t) &&\n-\tverbose test \"$would_clean\" = ../src/part3.c\n+\ttest \"$would_clean\" = ../src/part3.c\n '\n \n test_expect_success 'git clean with absolute path' '\n@@ -133,7 +133,7 @@ test_expect_success 'git clean with absolute path' '\n \t\tgrep part3 |\n \t\tsed -n -e \"s|^Would remove ||p\"\n \t) &&\n-\tverbose test \"$would_clean\" = ../src/part3.c\n+\ttest \"$would_clean\" = ../src/part3.c\n '\n \n test_expect_success 'git clean with out of work tree relative path' '\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex d6c0478d98..8835e16e81 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -405,40 +405,40 @@ test_expect_success '__gitdir - remote as argument' '\n \n test_expect_success '__git_dequote - plain unquoted word' '\n \t__git_dequote unquoted-word &&\n-\tverbose test unquoted-word = \"$dequoted_word\"\n+\ttest unquoted-word = \"$dequoted_word\"\n '\n \n # input:    b\\a\\c\\k\\'\\\\\\\"s\\l\\a\\s\\h\\es\n # expected: back'\\\"slashes\n test_expect_success '__git_dequote - backslash escaped' '\n \t__git_dequote \"b\\a\\c\\k\\\\'\\''\\\\\\\\\\\\\\\"s\\l\\a\\s\\h\\es\" &&\n-\tverbose test \"back'\\''\\\\\\\"slashes\" = \"$dequoted_word\"\n+\ttest \"back'\\''\\\\\\\"slashes\" = \"$dequoted_word\"\n '\n \n # input:    sin'gle\\' '\"quo'ted\n # expected: single\\ \"quoted\n test_expect_success '__git_dequote - single quoted' '\n \t__git_dequote \"'\"sin'gle\\\\\\\\' '\\\\\\\"quo'ted\"'\" &&\n-\tverbose test '\\''single\\ \"quoted'\\'' = \"$dequoted_word\"\n+\ttest '\\''single\\ \"quoted'\\'' = \"$dequoted_word\"\n '\n \n # input:    dou\"ble\\\\\" \"\\\"\\quot\"ed\n # expected: double\\ \"\\quoted\n test_expect_success '__git_dequote - double quoted' '\n \t__git_dequote '\\''dou\"ble\\\\\" \"\\\"\\quot\"ed'\\'' &&\n-\tverbose test '\\''double\\ \"\\quoted'\\'' = \"$dequoted_word\"\n+\ttest '\\''double\\ \"\\quoted'\\'' = \"$dequoted_word\"\n '\n \n # input: 'open single quote\n test_expect_success '__git_dequote - open single quote' '\n \t__git_dequote \"'\\''open single quote\" &&\n-\tverbose test \"open single quote\" = \"$dequoted_word\"\n+\ttest \"open single quote\" = \"$dequoted_word\"\n '\n \n # input: \"open double quote\n test_expect_success '__git_dequote - open double quote' '\n \t__git_dequote \"\\\"open double quote\" &&\n-\tverbose test \"open double quote\" = \"$dequoted_word\"\n+\ttest \"open double quote\" = \"$dequoted_word\"\n '\n \n \n@@ -616,7 +616,7 @@ test_expect_success '__git_is_configured_remote' '\n \ttest_when_finished \"git remote remove remote_2\" &&\n \tgit remote add remote_2 git://remote_2 &&\n \t(\n-\t\tverbose __git_is_configured_remote remote_2 &&\n+\t\t__git_is_configured_remote remote_2 &&\n \t\ttest_must_fail __git_is_configured_remote non-existent\n \t)\n '\n@@ -2596,30 +2596,30 @@ test_expect_success 'options with value' '\n test_expect_success 'sourcing the completion script clears cached commands' '\n \t(\n \t\t__git_compute_all_commands &&\n-\t\tverbose test -n \"$__git_all_commands\" &&\n+\t\ttest -n \"$__git_all_commands\" &&\n \t\t. \"$GIT_BUILD_DIR/contrib/completion/git-completion.bash\" &&\n-\t\tverbose test -z \"$__git_all_commands\"\n+\t\ttest -z \"$__git_all_commands\"\n \t)\n '\n \n test_expect_success 'sourcing the completion script clears cached merge strategies' '\n \t(\n \t\t__git_compute_merge_strategies &&\n-\t\tverbose test -n \"$__git_merge_strategies\" &&\n+\t\ttest -n \"$__git_merge_strategies\" &&\n \t\t. \"$GIT_BUILD_DIR/contrib/completion/git-completion.bash\" &&\n-\t\tverbose test -z \"$__git_merge_strategies\"\n+\t\ttest -z \"$__git_merge_strategies\"\n \t)\n '\n \n test_expect_success 'sourcing the completion script clears cached --options' '\n \t(\n \t\t__gitcomp_builtin checkout &&\n-\t\tverbose test -n \"$__gitcomp_builtin_checkout\" &&\n+\t\ttest -n \"$__gitcomp_builtin_checkout\" &&\n \t\t__gitcomp_builtin notes_edit &&\n-\t\tverbose test -n \"$__gitcomp_builtin_notes_edit\" &&\n+\t\ttest -n \"$__gitcomp_builtin_notes_edit\" &&\n \t\t. \"$GIT_BUILD_DIR/contrib/completion/git-completion.bash\" &&\n-\t\tverbose test -z \"$__gitcomp_builtin_checkout\" &&\n-\t\tverbose test -z \"$__gitcomp_builtin_notes_edit\"\n+\t\ttest -z \"$__gitcomp_builtin_checkout\" &&\n+\t\ttest -z \"$__gitcomp_builtin_notes_edit\"\n \t)\n '\n \ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 999d46fafe..6e19ebc922 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -1227,15 +1227,6 @@ test_i18ngrep () {\n \treturn 1\n }\n \n-# Call any command \"$@\" but be more verbose about its\n-# failure. This is handy for commands like \"test\" which do\n-# not output anything when they fail.\n-verbose () {\n-\t\"$@\" && return 0\n-\techo >&4 \"command failed: $(git rev-parse --sq-quote \"$@\")\"\n-\treturn 1\n-}\n-\n # Check if the file expected to be empty is indeed empty, and barfs\n # otherwise.\n \n-- \n2.40.1.802.gdef2a8734a\n"},{"id":"476837","messageId":"ZFl3BCzDwetyAkyQ@nand.local","threadId":"59714","inReplyTo":"20230508190457.GC2366490@coredump.intra.peff.net","subject":"Re: [PATCH 3/3] t: drop \"verbose\" helper function","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-05-08T22:26:12Z","receivedAt":"2023-05-08T22:26:19Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, May 08, 2023 at 03:04:57PM -0400, Jeff King wrote:\n> Let's consider the \"verbose\" function a failed experiment and remove the\n> last few callers (which are all many years old, and have been dwindling\n> as we remove them from scripts we touch for other reasons). It will be\n> one less thing for new test writers to see and wonder if they should be\n> using themselves.\n\nWell put.\n\n> I think this should be all calls. Most tests will fail after removing\n> the function, of course, but it's possible one could be hiding in an\n> expect_failure or something. I didn't see any after grepping for\n> '[^-]verbose' in t/, which is not too long to look through.\n>\n>  t/t0020-crlf.sh            | 38 +++++++++++++++++++-------------------\n>  t/t1301-shared-repo.sh     |  4 ++--\n>  t/t3427-rebase-subtree.sh  | 12 ++++++------\n>  t/t4022-diff-rewrite.sh    |  2 +-\n>  t/t4062-diff-pickaxe.sh    |  2 +-\n>  t/t5304-prune.sh           | 16 ++++++++--------\n>  t/t6006-rev-list-format.sh |  2 +-\n>  t/t6501-freshen-objects.sh |  2 +-\n>  t/t7001-mv.sh              |  2 +-\n>  t/t7300-clean.sh           |  4 ++--\n>  t/t9902-completion.sh      | 30 +++++++++++++++---------------\n>  t/test-lib-functions.sh    |  9 ---------\n>  12 files changed, 57 insertions(+), 66 deletions(-)\n\nI applied these myself and grepped around myself, and also could not\nfind any stragglers. So I'd be happy to drop the implementation of\nverbose() in this series, too, to avoid the appearance of it continuing\nto be a blessed path.\n\nThanks,\nTaylor\n"},{"id":"476838","messageId":"ZFl3FSt2a59G/X7m@nand.local","threadId":"59714","inReplyTo":"20230508185953.GA2108869@coredump.intra.peff.net","subject":"Re: [PATCH 0/3] dropping \"verbose\" test helper function","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-05-08T22:26:29Z","receivedAt":"2023-05-08T22:26:35Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, May 08, 2023 at 02:59:53PM -0400, Jeff King wrote:\n> I happened across an instance of the \"verbose\" helper function in a\n> test recently. I think it's clear we prefer the \"-x\" tracing to it these\n> days, and we've been slowly removing instances. This series gets rid of\n> the last few.\n>\n> The actual change (and rationale) is in patch 3. Patch 1 just fixes pipe\n> problems on lines we'll be touching patch 3, and patch 2 is a cute\n> optimization enabled by patch 1. I don't think any of it should be very\n> controversial, but it can all be split up if need be.\n>\n>   [1/3]: t7001: avoid git on upstream of pipe\n>   [2/3]: t7001: use \"ls-files --format\" instead of \"cut\"\n>   [3/3]: t: drop \"verbose\" helper function\n\nAll look good to me, I'd be happy to see this get picked up.\n\nThanks,\nTaylor\n"}]}