{"thread":{"id":"57499","subject":"[PATCH 07/15] merge tests: use \"test_must_fail\" instead of ad-hoc pattern","startedAt":"2022-03-02T17:27:53Z","lastAt":"2022-03-07T12:50:11Z","messageCount":48,"participants":["Ævar Arnfjörð Bjarmason","Junio C Hamano","Derrick Stolee","Shubham Mishra","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":15},"messages":[{"id":"450129","messageId":"patch-07.15-276be19e35e-20220302T171755Z-avarab@gmail.com","threadId":"57499","inReplyTo":"cover-00.15-00000000000-20220302T171755Z-avarab@gmail.com","subject":"[PATCH 07/15] merge tests: use \"test_must_fail\" instead of ad-hoc pattern","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:27:16Z","receivedAt":"2022-03-02T17:27:53Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"As in the preceding commit change a similar fragile test pattern\nintroduced in b798671fa93 (merge-recursive: do not rudely die on\nbinary merge, 2007-08-14) to use a \"test_must_fail\" instead.\n\nBefore this we wouldn't distinguish normal \"git merge\" failures from\nsegfaults or abort(). Unlike the preceding commit we didn't end up\nhiding any SANITIZE=leak failures in this case, but let's\ncorrespondingly change these anyway.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t6407-merge-binary.sh | 22 ++++++----------------\n 1 file changed, 6 insertions(+), 16 deletions(-)\n\ndiff --git a/t/t6407-merge-binary.sh b/t/t6407-merge-binary.sh\nindex 8e6241f92e6..0753fc95f45 100755\n--- a/t/t6407-merge-binary.sh\n+++ b/t/t6407-merge-binary.sh\n@@ -43,14 +43,9 @@ test_expect_success resolve '\n \trm -f a* m* &&\n \tgit reset --hard anchor &&\n \n-\tif git merge -s resolve main\n-\tthen\n-\t\techo Oops, should not have succeeded\n-\t\tfalse\n-\telse\n-\t\tgit ls-files -s >current &&\n-\t\ttest_cmp expect current\n-\tfi\n+\ttest_must_fail git merge -s resolve main &&\n+\tgit ls-files -s >current &&\n+\ttest_cmp expect current\n '\n \n test_expect_success recursive '\n@@ -58,14 +53,9 @@ test_expect_success recursive '\n \trm -f a* m* &&\n \tgit reset --hard anchor &&\n \n-\tif git merge -s recursive main\n-\tthen\n-\t\techo Oops, should not have succeeded\n-\t\tfalse\n-\telse\n-\t\tgit ls-files -s >current &&\n-\t\ttest_cmp expect current\n-\tfi\n+\ttest_must_fail git merge -s recursive main &&\n+\tgit ls-files -s >current &&\n+\ttest_cmp expect current\n '\n \n test_done\n-- \n2.35.1.1226.g8b497615d32\n\n"},{"id":"450130","messageId":"cover-00.15-00000000000-20220302T171755Z-avarab@gmail.com","threadId":"57499","inReplyTo":null,"subject":"[PATCH 00/15] tests: don't ignore \"git\" exit codes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:27:09Z","receivedAt":"2022-03-02T17:27:54Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This series fixes issues where we ignored the exit code of \"git\" due\nto it being on the LHS of a pipe, or because we interpolated its\noutput with $() in a \"test\" construct, or had missing &&- chains in\nhelper functions etc.\n\nThis series is not made by string-replacing things in our test suite,\nif it was it would be much larger. These are all tests I've seen real\nhide real failures under SANITIZE=leak, either on current \"master\", or\nin combination with various local leak fixes I've got unsubmitted.\n\nIn cases where I was starting to fix a pattern in a file I'd fix the\nrest of the file if it was easy, but otherwise these are all cases\nwhere I ran SANITIZE=leak, had a test pass, but having ASAN_OPTIONS\nlog to a file revealed that we had memory leaks within that test.\n\nAs an aside we still have various potential issues with hidden\nsegfaults etc. in the test suite after this that are tricked to solve,\nbecause:\n\n * Our tests will (mostly) catch segfaults and abort(), but if we\n invoke a command that invokes another command it needs to ferry the\n exit code up to us.\n\n * run-command.c notably does not do that, so for e.g. \"git push\"\n tests where we expect a failure and an underlying \"git\" command\n fails we won't ferry up the segfault or abort exit code.\n\n * We have gitweb.perl and some other perl code ignoring return values\n from close(), i.e. ignoring exit codes from \"git rev-parse\" et al.\n\n * We have in-tree shellscripts like \"git-merge-one-file.sh\" invoking\n git commands, and if they fail returning \"1\", not ferrying up the\n segfault or abort() exit code.\n\nÆvar Arnfjörð Bjarmason (15):\n tests: change some 'test $(git) = \"x\"' to test_cmp\n tests: use \"test_stdout_line_count\", not \"test $(git [...] | wc -l)\"\n read-tree tests: check \"diff-files\" exit code on failure\n diff tests: don't ignore \"git diff\" exit code\n diff tests: don't ignore \"git diff\" exit code in \"read\" loop\n apply tests: use \"test_must_fail\" instead of ad-hoc pattern\n merge tests: use \"test_must_fail\" instead of ad-hoc pattern\n rev-parse tests: don't ignore \"git reflog\" exit code\n notes tests: don't ignore \"git\" exit code\n diff tests: don't ignore \"git rev-list\" exit code\n rev-list tests: don't hide abort() in \"test_expect_failure\"\n gettext tests: don't ignore \"test-tool regex\" exit code\n apply tests: don't ignore \"git ls-files\" exit code, drop sub-shell\n checkout tests: don't ignore \"git \" exit code\n rev-list simplify tests: don't ignore \"git\" exit code\n\n t/t0002-gitfile.sh | 6 +-\n t/t1001-read-tree-m-2way.sh | 6 +-\n t/t1002-read-tree-m-u-2way.sh | 6 +-\n t/t1503-rev-parse-verify.sh | 5 +-\n t/t2012-checkout-last.sh | 51 ++++++---\n t/t2200-add-update.sh | 33 ++++--\n t/t3302-notes-index-expensive.sh | 6 +-\n t/t3303-notes-subtrees.sh | 9 +-\n t/t3305-notes-fanout.sh | 14 +--\n t/t4020-diff-external.sh | 153 ++++++++++++-------------\n t/t4027-diff-submodule.sh | 7 +-\n t/t4123-apply-shrink.sh | 18 +--\n t/t4128-apply-root.sh | 36 +++---\n t/t6005-rev-list-count.sh | 43 ++++---\n t/t6012-rev-list-simplify.sh | 12 +-\n t/t6102-rev-list-unexpected-objects.sh | 13 ++-\n t/t6407-merge-binary.sh | 22 +---\n t/t7103-reset-bare.sh | 7 +-\n t/t7812-grep-icase-non-ascii.sh | 11 +-\n 19 files changed, 240 insertions(+), 218 deletions(-)\n\n-- \n2.35.1.1226.g8b497615d32\n\n"},{"id":"450131","messageId":"patch-08.15-dca2ac3a171-20220302T171755Z-avarab@gmail.com","threadId":"57499","inReplyTo":"cover-00.15-00000000000-20220302T171755Z-avarab@gmail.com","subject":"[PATCH 08/15] rev-parse tests: don't ignore \"git reflog\" exit code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:27:17Z","receivedAt":"2022-03-02T17:27:57Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Amend a test added in 9c46c054ae4 (rev-parse: tests git rev-parse\n--verify master@{n}, for various n, 2010-08-24) so that we'll stop\nignoring the exit code of \"git reflog\" by having it on the\nleft-hand-side of a pipe.\n\nBecause of this I'd marked this test as passing under SANITIZE=leak in\nf442c94638d (leak tests: mark some rev-parse tests as passing with\nSANITIZE=leak, 2021-10-31). As all of it except this specific test\nwill now pass, let's skip it under the !SANITIZE_LEAK prerequisite.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t1503-rev-parse-verify.sh | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh\nindex 94fe413ee37..ba43168d123 100755\n--- a/t/t1503-rev-parse-verify.sh\n+++ b/t/t1503-rev-parse-verify.sh\n@@ -132,8 +132,9 @@ test_expect_success 'use --default' '\n \ttest_must_fail git rev-parse --verify --default bar\n '\n \n-test_expect_success 'main@{n} for various n' '\n-\tN=$(git reflog | wc -l) &&\n+test_expect_success !SANITIZE_LEAK 'main@{n} for various n' '\n+\tgit reflog >out &&\n+\tN=$(wc -l \n---\n t/t6005-rev-list-count.sh | 43 +++++++++++++++++++--------------------\n 1 file changed, 21 insertions(+), 22 deletions(-)\n\ndiff --git a/t/t6005-rev-list-count.sh b/t/t6005-rev-list-count.sh\nindex 86542c650e2..e960049f647 100755\n--- a/t/t6005-rev-list-count.sh\n+++ b/t/t6005-rev-list-count.sh\n@@ -2,7 +2,6 @@\n \n test_description='git rev-list --max-count and --skip test'\n \n-TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\n@@ -14,39 +13,39 @@ test_expect_success 'setup' '\n '\n \n test_expect_success 'no options' '\n- test $(git rev-list HEAD | wc -l) = 5\n+\ttest_stdout_line_count = 5 git rev-list HEAD\n '\n \n test_expect_success '--max-count' '\n- test $(git rev-list HEAD --max-count=0 | wc -l) = 0 &&\n- test $(git rev-list HEAD --max-count=3 | wc -l) = 3 &&\n- test $(git rev-list HEAD --max-count=5 | wc -l) = 5 &&\n- test $(git rev-list HEAD --max-count=10 | wc -l) = 5\n+\ttest_stdout_line_count = 0 git rev-list HEAD --max-count=0 &&\n+\ttest_stdout_line_count = 3 git rev-list HEAD --max-count=3 &&\n+\ttest_stdout_line_count = 5 git rev-list HEAD --max-count=5 &&\n+\ttest_stdout_line_count = 5 git rev-list HEAD --max-count=10\n '\n \n test_expect_success '--max-count all forms' '\n- test $(git rev-list HEAD --max-count=1 | wc -l) = 1 &&\n- test $(git rev-list HEAD -1 | wc -l) = 1 &&\n- test $(git rev-list HEAD -n1 | wc -l) = 1 &&\n- test $(git rev-list HEAD -n 1 | wc -l) = 1\n+\ttest_stdout_line_count = 1 git rev-list HEAD --max-count=1 &&\n+\ttest_stdout_line_count = 1 git rev-list HEAD -1 &&\n+\ttest_stdout_line_count = 1 git rev-list HEAD -n1 &&\n+\ttest_stdout_line_count = 1 git rev-list HEAD -n 1\n '\n \n test_expect_success '--skip' '\n- test $(git rev-list HEAD --skip=0 | wc -l) = 5 &&\n- test $(git rev-list HEAD --skip=3 | wc -l) = 2 &&\n- test $(git rev-list HEAD --skip=5 | wc -l) = 0 &&\n- test $(git rev-list HEAD --skip=10 | wc -l) = 0\n+\ttest_stdout_line_count = 5 git rev-list HEAD --skip=0 &&\n+\ttest_stdout_line_count = 2 git rev-list HEAD --skip=3 &&\n+\ttest_stdout_line_count = 0 git rev-list HEAD --skip=5 &&\n+\ttest_stdout_line_count = 0 git rev-list HEAD --skip=10\n '\n \n test_expect_success '--skip --max-count' '\n- test $(git rev-list HEAD --skip=0 --max-count=0 | wc -l) = 0 &&\n- test $(git rev-list HEAD --skip=0 --max-count=10 | wc -l) = 5 &&\n- test $(git rev-list HEAD --skip=3 --max-count=0 | wc -l) = 0 &&\n- test $(git rev-list HEAD --skip=3 --max-count=1 | wc -l) = 1 &&\n- test $(git rev-list HEAD --skip=3 --max-count=2 | wc -l) = 2 &&\n- test $(git rev-list HEAD --skip=3 --max-count=10 | wc -l) = 2 &&\n- test $(git rev-list HEAD --skip=5 --max-count=10 | wc -l) = 0 &&\n- test $(git rev-list HEAD --skip=10 --max-count=10 | wc -l) = 0\n+\ttest_stdout_line_count = 0 git rev-list HEAD --skip=0 --max-count=0 &&\n+\ttest_stdout_line_count = 5 git rev-list HEAD --skip=0 --max-count=10 &&\n+\ttest_stdout_line_count = 0 git rev-list HEAD --skip=3 --max-count=0 &&\n+\ttest_stdout_line_count = 1 git rev-list HEAD --skip=3 --max-count=1 &&\n+\ttest_stdout_line_count = 2 git rev-list HEAD --skip=3 --max-count=2 &&\n+\ttest_stdout_line_count = 2 git rev-list HEAD --skip=3 --max-count=10 &&\n+\ttest_stdout_line_count = 0 git rev-list HEAD --skip=5 --max-count=10 &&\n+\ttest_stdout_line_count = 0 git rev-list HEAD --skip=10 --max-count=10\n '\n \n test_done\n-- \n2.35.1.1226.g8b497615d32\n\n"},{"id":"450133","messageId":"patch-05.15-b1aeac3f68e-20220302T171755Z-avarab@gmail.com","threadId":"57499","inReplyTo":"cover-00.15-00000000000-20220302T171755Z-avarab@gmail.com","subject":"[PATCH 05/15] diff tests: don't ignore \"git diff\" exit code in \"read\" loop","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:27:14Z","receivedAt":"2022-03-02T17:28:01Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a test pattern that originated in f1af60bdba4 (Support 'diff=pgm'\nattribute, 2007-04-22) so that we'll stop using \"git diff\" on the\nleft-hand-side of a pipe, and thus ignoring its exit code.\n\nRather than use intermediate files let's rewrite these tests to a much\nsimpler but more exhaustive \"test_tmp\" where we'll ignore certain\nfields in the output.\n\nNote that this is not a faithful conversion of the previous\n\"read/test\" in some cases, as we were ignoring more fields there than\nwe strictly needed to. Now we'll \"test_cmp\" everything we can, and\nonly ignore the likes of paths to $TEMPDIR etc.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t4020-diff-external.sh | 104 ++++++++++++++++++++-------------------\n 1 file changed, 53 insertions(+), 51 deletions(-)\n\ndiff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh\nindex 879ee04d291..1219f8bd4c0 100755\n--- a/t/t4020-diff-external.sh\n+++ b/t/t4020-diff-external.sh\n@@ -24,16 +24,12 @@ test_expect_success setup '\n '\n \n test_expect_success 'GIT_EXTERNAL_DIFF environment' '\n-\n-\tGIT_EXTERNAL_DIFF=echo git diff | {\n-\t\tread path oldfile oldhex oldmode newfile newhex newmode &&\n-\t\ttest \"z$path\" = zfile &&\n-\t\ttest \"z$oldmode\" = z100644 &&\n-\t\ttest \"z$newhex\" = \"z$ZERO_OID\" &&\n-\t\ttest \"z$newmode\" = z100644 &&\n-\t\toh=$(git rev-parse --verify HEAD:file) &&\n-\t\ttest \"z$oh\" = \"z$oldhex\"\n-\t}\n+\tcat >expect <<-EOF &&\n+\tfile $(git rev-parse --verify HEAD:file) 100644 file $(test_oid zero) 100644\n+\tEOF\n+\tGIT_EXTERNAL_DIFF=echo git diff >out &&\n+\tcut -d\" \" -f1,3- actual &&\n+\ttest_cmp expect actual\n \n '\n \n@@ -52,15 +48,14 @@ test_expect_success 'GIT_EXTERNAL_DIFF environment and --no-ext-diff' '\n test_expect_success SYMLINKS 'typechange diff' '\n \trm -f file &&\n \tln -s elif file &&\n-\tGIT_EXTERNAL_DIFF=echo git diff | {\n-\t\tread path oldfile oldhex oldmode newfile newhex newmode &&\n-\t\ttest \"z$path\" = zfile &&\n-\t\ttest \"z$oldmode\" = z100644 &&\n-\t\ttest \"z$newhex\" = \"z$ZERO_OID\" &&\n-\t\ttest \"z$newmode\" = z120000 &&\n-\t\toh=$(git rev-parse --verify HEAD:file) &&\n-\t\ttest \"z$oh\" = \"z$oldhex\"\n-\t} &&\n+\n+\tcat >expect <<-EOF &&\n+\tfile $(git rev-parse --verify HEAD:file) 100644 $(test_oid zero) 120000\n+\tEOF\n+\tGIT_EXTERNAL_DIFF=echo git diff >out &&\n+\tcut -d\" \" -f1,3-4,6- actual &&\n+\ttest_cmp expect actual &&\n+\n \tGIT_EXTERNAL_DIFF=echo git diff --no-ext-diff >actual &&\n \tgit diff >expect &&\n \ttest_cmp expect actual\n@@ -70,15 +65,13 @@ test_expect_success 'diff.external' '\n \tgit reset --hard &&\n \techo third >file &&\n \ttest_config diff.external echo &&\n-\tgit diff | {\n-\t\tread path oldfile oldhex oldmode newfile newhex newmode &&\n-\t\ttest \"z$path\" = zfile &&\n-\t\ttest \"z$oldmode\" = z100644 &&\n-\t\ttest \"z$newhex\" = \"z$ZERO_OID\" &&\n-\t\ttest \"z$newmode\" = z100644 &&\n-\t\toh=$(git rev-parse --verify HEAD:file) &&\n-\t\ttest \"z$oh\" = \"z$oldhex\"\n-\t}\n+\n+\tcat >expect <<-EOF &&\n+\tfile $(git rev-parse --verify HEAD:file) 100644 $(test_oid zero) 100644\n+\tEOF\n+\tgit diff >out &&\n+\tcut -d\" \" -f1,3-4,6- actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success !SANITIZE_LEAK 'diff.external should apply only to diff' '\n@@ -101,16 +94,12 @@ test_expect_success 'diff attribute' '\n \n \techo >.gitattributes \"file diff=parrot\" &&\n \n-\tgit diff | {\n-\t\tread path oldfile oldhex oldmode newfile newhex newmode &&\n-\t\ttest \"z$path\" = zfile &&\n-\t\ttest \"z$oldmode\" = z100644 &&\n-\t\ttest \"z$newhex\" = \"z$ZERO_OID\" &&\n-\t\ttest \"z$newmode\" = z100644 &&\n-\t\toh=$(git rev-parse --verify HEAD:file) &&\n-\t\ttest \"z$oh\" = \"z$oldhex\"\n-\t}\n-\n+\tcat >expect <<-EOF &&\n+\tfile $(git rev-parse --verify HEAD:file) 100644 $(test_oid zero) 100644\n+\tEOF\n+\tgit diff >out &&\n+\tcut -d\" \" -f1,3-4,6- actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success !SANITIZE_LEAK 'diff attribute should apply only to diff' '\n@@ -132,16 +121,12 @@ test_expect_success 'diff attribute' '\n \n \techo >.gitattributes \"file diff=color\" &&\n \n-\tgit diff | {\n-\t\tread path oldfile oldhex oldmode newfile newhex newmode &&\n-\t\ttest \"z$path\" = zfile &&\n-\t\ttest \"z$oldmode\" = z100644 &&\n-\t\ttest \"z$newhex\" = \"z$ZERO_OID\" &&\n-\t\ttest \"z$newmode\" = z100644 &&\n-\t\toh=$(git rev-parse --verify HEAD:file) &&\n-\t\ttest \"z$oh\" = \"z$oldhex\"\n-\t}\n-\n+\tcat >expect <<-EOF &&\n+\tfile $(git rev-parse --verify HEAD:file) 100644 $(test_oid zero) 100644\n+\tEOF\n+\tgit diff >out &&\n+\tcut -d\" \" -f1,3-4,6- actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success !SANITIZE_LEAK 'diff attribute should apply only to diff' '\n@@ -159,14 +144,26 @@ test_expect_success 'diff attribute and --no-ext-diff' '\n test_expect_success 'GIT_EXTERNAL_DIFF trumps diff.external' '\n \t>.gitattributes &&\n \ttest_config diff.external \"echo ext-global\" &&\n-\tGIT_EXTERNAL_DIFF=\"echo ext-env\" git diff | grep ext-env\n+\n+\tcat >expect <<-EOF &&\n+\text-env file $(git rev-parse --verify HEAD:file) 100644 file $(test_oid zero) 100644\n+\tEOF\n+\tGIT_EXTERNAL_DIFF=\"echo ext-env\" git diff >out &&\n+\tcut -d\" \" -f1-2,4- actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'attributes trump GIT_EXTERNAL_DIFF and diff.external' '\n \ttest_config diff.foo.command \"echo ext-attribute\" &&\n \ttest_config diff.external \"echo ext-global\" &&\n \techo \"file diff=foo\" >.gitattributes &&\n-\tGIT_EXTERNAL_DIFF=\"echo ext-env\" git diff | grep ext-attribute\n+\n+\tcat >expect <<-EOF &&\n+\text-attribute file $(git rev-parse --verify HEAD:file) 100644 file $(test_oid zero) 100644\n+\tEOF\n+\tGIT_EXTERNAL_DIFF=\"echo ext-env\" git diff >out &&\n+\tcut -d\" \" -f1-2,4- actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'no diff with -diff' '\n@@ -212,7 +209,12 @@ test_expect_success 'GIT_EXTERNAL_DIFF generates pretty paths' '\n \ttouch file.ext &&\n \tgit add file.ext &&\n \techo with extension > file.ext &&\n-\tGIT_EXTERNAL_DIFF=echo git diff file.ext | grep ......_file\\.ext &&\n+\n+\tcat >expect <<-EOF &&\n+\tfile.ext file $(git rev-parse --verify HEAD:file) 100644 file.ext $(test_oid zero) 100644\n+\tEOF\n+\tGIT_EXTERNAL_DIFF=echo git diff file.ext >out &&\n+\tcut -d\" \" -f1,3- actual &&\n \tgit update-index --force-remove file.ext &&\n \trm file.ext\n '\n-- \n2.35.1.1226.g8b497615d32\n\n"},{"id":"450134","messageId":"patch-01.15-78b9c52551f-20220302T171755Z-avarab@gmail.com","threadId":"57499","inReplyTo":"cover-00.15-00000000000-20220302T171755Z-avarab@gmail.com","subject":"[PATCH 01/15] tests: change some 'test $(git) = \"x\"' to test_cmp","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:27:10Z","receivedAt":"2022-03-02T17:28:04Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change some of the patterns in the test suite where we were hiding the\nexit code from \"git\" by invoking it in a sub-shell within a \"test\"\nexpression to use temporary files and test_cmp instead.\n\nThese are not all the occurrences of this anti-pattern, but these in\nparticular hid issues where LSAN was dying, and I'd thus marked these\ntests as passing under the linux-leaks CI job in past commits with\n\"TEST_PASSES_SANITIZE_LEAK=true\". Let's deal with that by either\nremoving that marking, or skipping specific tests under\n!SANITIZE_LEAK.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t0002-gitfile.sh | 6 ++++--\n t/t2200-add-update.sh | 33 +++++++++++++++++++++------------\n t/t4128-apply-root.sh | 33 ++++++++++++++++++++-------------\n t/t7103-reset-bare.sh | 7 +++++--\n 4 files changed, 50 insertions(+), 29 deletions(-)\n\ndiff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh\nindex 76052cb5620..f6356db183b 100755\n--- a/t/t0002-gitfile.sh\n+++ b/t/t0002-gitfile.sh\n@@ -65,9 +65,11 @@ test_expect_success 'check commit-tree' '\n \ttest_path_is_file \"$REAL/objects/$(objpath $SHA)\"\n '\n \n-test_expect_success 'check rev-list' '\n+test_expect_success !SANITIZE_LEAK 'check rev-list' '\n \tgit update-ref \"HEAD\" \"$SHA\" &&\n-\ttest \"$SHA\" = \"$(git rev-list HEAD)\"\n+\tgit rev-list HEAD >actual &&\n+\techo $SHA >expected &&\n+\ttest_cmp expected actual\n '\n \n test_expect_success 'setup_git_dir twice in subdir' '\ndiff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh\nindex acd3650d3c0..0c38f8e3569 100755\n--- a/t/t2200-add-update.sh\n+++ b/t/t2200-add-update.sh\n@@ -14,7 +14,6 @@ only the updates to dir/sub.\n Also tested are \"git add -u\" without limiting, and \"git add -u\"\n without contents changes, and other conditions'\n \n-TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success setup '\n@@ -41,20 +40,28 @@ test_expect_success update '\n '\n \n test_expect_success 'update noticed a removal' '\n-\ttest \"$(git ls-files dir1/sub1)\" = \"\"\n+\tgit ls-files dir1/sub1 >out &&\n+\ttest_must_be_empty out\n '\n \n test_expect_success 'update touched correct path' '\n-\ttest \"$(git diff-files --name-status dir2/sub3)\" = \"\"\n+\tgit diff-files --name-status dir2/sub3 >out &&\n+\ttest_must_be_empty out\n '\n \n test_expect_success 'update did not touch other tracked files' '\n-\ttest \"$(git diff-files --name-status check)\" = \"M\tcheck\" &&\n-\ttest \"$(git diff-files --name-status top)\" = \"M\ttop\"\n+\techo \"M\tcheck\" >expect &&\n+\tgit diff-files --name-status check >actual &&\n+\ttest_cmp expect actual &&\n+\n+\techo \"M\ttop\" >expect &&\n+\tgit diff-files --name-status top >actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'update did not touch untracked files' '\n-\ttest \"$(git ls-files dir2/other)\" = \"\"\n+\tgit ls-files dir2/other >out &&\n+\ttest_must_be_empty out\n '\n \n test_expect_success 'cache tree has not been corrupted' '\n@@ -76,9 +83,8 @@ test_expect_success 'update from a subdirectory' '\n '\n \n test_expect_success 'change gets noticed' '\n-\n-\ttest \"$(git diff-files --name-status dir1)\" = \"\"\n-\n+\tgit diff-files --name-status dir1 >out &&\n+\ttest_must_be_empty out\n '\n \n test_expect_success 'non-qualified update in subdir updates from the root' '\n@@ -103,7 +109,8 @@ test_expect_success 'replace a file with a symlink' '\n test_expect_success 'add everything changed' '\n \n \tgit add -u &&\n-\ttest -z \"$(git diff-files)\"\n+\tgit diff-files >out &&\n+\ttest_must_be_empty out\n \n '\n \n@@ -111,7 +118,8 @@ test_expect_success 'touch and then add -u' '\n \n \ttouch check &&\n \tgit add -u &&\n-\ttest -z \"$(git diff-files)\"\n+\tgit diff-files >out &&\n+\ttest_must_be_empty out\n \n '\n \n@@ -119,7 +127,8 @@ test_expect_success 'touch and then add explicitly' '\n \n \ttouch check &&\n \tgit add check &&\n-\ttest -z \"$(git diff-files)\"\n+\tgit diff-files >out &&\n+\ttest_must_be_empty out\n \n '\n \ndiff --git a/t/t4128-apply-root.sh b/t/t4128-apply-root.sh\nindex cb3181e8b71..ba89a2f2d73 100755\n--- a/t/t4128-apply-root.sh\n+++ b/t/t4128-apply-root.sh\n@@ -2,8 +2,6 @@\n \n test_description='apply same filename'\n \n-\n-TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\n@@ -26,10 +24,11 @@ diff a/bla/blub/dir/file b/bla/blub/dir/file\n EOF\n \n test_expect_success 'apply --directory -p (1)' '\n-\n \tgit apply --directory=some/sub -p3 --index patch &&\n-\ttest Bello = $(git show :some/sub/dir/file) &&\n-\ttest Bello = $(cat some/sub/dir/file)\n+\techo Bello >expect &&\n+\tgit show :some/sub/dir/file >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_cmp expect some/sub/dir/file\n \n '\n \n@@ -37,8 +36,10 @@ test_expect_success 'apply --directory -p (2) ' '\n \n \tgit reset --hard initial &&\n \tgit apply --directory=some/sub/ -p3 --index patch &&\n-\ttest Bello = $(git show :some/sub/dir/file) &&\n-\ttest Bello = $(cat some/sub/dir/file)\n+\techo Bello >expect &&\n+\tgit show :some/sub/dir/file >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_cmp expect some/sub/dir/file\n \n '\n \n@@ -55,8 +56,10 @@ EOF\n test_expect_success 'apply --directory (new file)' '\n \tgit reset --hard initial &&\n \tgit apply --directory=some/sub/dir/ --index patch &&\n-\ttest content = $(git show :some/sub/dir/newfile) &&\n-\ttest content = $(cat some/sub/dir/newfile)\n+\techo content >expect &&\n+\tgit show :some/sub/dir/newfile >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_cmp expect some/sub/dir/newfile\n '\n \n cat > patch << EOF\n@@ -72,8 +75,10 @@ EOF\n test_expect_success 'apply --directory -p (new file)' '\n \tgit reset --hard initial &&\n \tgit apply -p2 --directory=some/sub/dir/ --index patch &&\n-\ttest content = $(git show :some/sub/dir/newfile2) &&\n-\ttest content = $(cat some/sub/dir/newfile2)\n+\techo content >expect &&\n+\tgit show :some/sub/dir/newfile2 >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_cmp expect some/sub/dir/newfile2\n '\n \n cat > patch << EOF\n@@ -107,8 +112,10 @@ EOF\n test_expect_success 'apply --directory (quoted filename)' '\n \tgit reset --hard initial &&\n \tgit apply --directory=some/sub/dir/ --index patch &&\n-\ttest content = $(git show :some/sub/dir/quotefile) &&\n-\ttest content = $(cat some/sub/dir/quotefile)\n+\techo content >expect &&\n+\tgit show :some/sub/dir/quotefile >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_cmp expect some/sub/dir/quotefile\n '\n \n test_done\ndiff --git a/t/t7103-reset-bare.sh b/t/t7103-reset-bare.sh\nindex 0de83e36199..a60153f9f32 100755\n--- a/t/t7103-reset-bare.sh\n+++ b/t/t7103-reset-bare.sh\n@@ -63,9 +63,12 @@ test_expect_success '\"mixed\" reset is not allowed in bare' '\n \ttest_must_fail git reset --mixed HEAD^\n '\n \n-test_expect_success '\"soft\" reset is allowed in bare' '\n+test_expect_success !SANITIZE_LEAK '\"soft\" reset is allowed in bare' '\n \tgit reset --soft HEAD^ &&\n-\ttest \"$(git show --pretty=format:%s | head -n 1)\" = \"one\"\n+\tgit show --pretty=format:%s >out &&\n+\techo one >expect &&\n+\thead -n 1 out >actual &&\n+\ttest_cmp expect actual\n '\n \n test_done\n-- \n2.35.1.1226.g8b497615d32\n\n"},{"id":"450135","messageId":"patch-12.15-f3cc5bc7eb9-20220302T171755Z-avarab@gmail.com","threadId":"57499","inReplyTo":"cover-00.15-00000000000-20220302T171755Z-avarab@gmail.com","subject":"[PATCH 12/15] gettext tests: don't ignore \"test-tool regex\" exit code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:27:21Z","receivedAt":"2022-03-02T17:28:06Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Amend a prerequisite check added in 5c1ebcca4d1 (grep/icase: avoid\nkwsset on literal non-ascii strings, 2016-06-25) to do invoke\n'test-tool regex' in such a way that we'll notice if it dies under\nSANITIZE=leak due to having a memory leak, as opposed to us not having\nthe \"ICASE\" support we're checking for.\n\nBecause we weren't making a distinction between the two I'd marked\nthese tests as passing under SANITIZE=leak in 03d85e21951 (leak tests:\nmark remaining leak-free tests as such, 2021-12-17).\n\nDoing this is tricky. Ideally \"test_lazy_prereq\" would materialize as\na \"real\" test that we could check the exit code of with the same\nsignal matching that \"test_must_fail\" does.\n\nHowever lazy prerequisites aren't real tests, and are instead lazily\nmaterialized in the guts of \"test_have_prereq\" when we've already\nstarted another test.\n\nWe could detect the abort() (or similar) there and pass that exit code\ndown, and fail the test that caused the prerequisites to be\nmaterialized.\n\nBut that would require extensive changes to test-lib.sh and\ntest-lib-functions.sh. Let's instead simply check if the exit code of\n\"test-tool regex\" is zero, and if so set the prerequisites. If it's\nnon-zero let's run it again with \"test_must_fail\". We'll thus make a\ndistinction between \"bad\" non-zero (segv etc) and \"good\" (exit 1 etc.).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t7812-grep-icase-non-ascii.sh | 11 ++++++++---\n 1 file changed, 8 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh\nindex ca3f24f8079..347bf4a12f3 100755\n--- a/t/t7812-grep-icase-non-ascii.sh\n+++ b/t/t7812-grep-icase-non-ascii.sh\n@@ -11,9 +11,14 @@ test_expect_success GETTEXT_LOCALE 'setup' '\n \texport LC_ALL\n '\n \n-test_have_prereq GETTEXT_LOCALE &&\n-test-tool regex \"HALLÓ\" \"Halló\" ICASE &&\n-test_set_prereq REGEX_LOCALE\n+test_expect_success GETTEXT_LOCALE 'setup REGEX_LOCALE prerequisite' '\n+\tif test-tool regex \"HALLÓ\" \"Halló\" ICASE\n+\tthen\n+\t\ttest_set_prereq REGEX_LOCALE\n+\telse\n+\t\ttest_must_fail test-tool regex \"HALLÓ\" \"Halló\" ICASE\n+\tfi\n+'\n \n test_expect_success REGEX_LOCALE 'grep literal string, no -F' '\n \tgit grep -i \"TILRAUN: Halló Heimur!\" &&\n-- \n2.35.1.1226.g8b497615d32\n\n"},{"id":"450136","messageId":"patch-13.15-834809b1b8a-20220302T171755Z-avarab@gmail.com","threadId":"57499","inReplyTo":"cover-00.15-00000000000-20220302T171755Z-avarab@gmail.com","subject":"[PATCH 13/15] apply tests: don't ignore \"git ls-files\" exit code, drop sub-shell","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:27:22Z","receivedAt":"2022-03-02T17:28:09Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix code added in 969c877506c (git apply --directory broken for new\nfiles, 2008-10-12) so that it doesn't invoke \"git ls-files\" on the\nleft-hand-side of a pipe, instead let's use an intermediate file.\n\nSince we're doing that we can also drop the sub-shell that was here to\ngroup the two.\n\nThere are a lot of these sorts of patterns in the test suite, and\nthere's no particular reason to fix this one other than in a preceding\ncommit all similar patterns except this one were fixed in\n\"t/t4128-apply-root.sh\", so let's fix this one straggler as well.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t4128-apply-root.sh | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t4128-apply-root.sh b/t/t4128-apply-root.sh\nindex ba89a2f2d73..f6db5a79dd9 100755\n--- a/t/t4128-apply-root.sh\n+++ b/t/t4128-apply-root.sh\n@@ -96,7 +96,8 @@ test_expect_success 'apply --directory (delete file)' '\n \techo content >some/sub/dir/delfile &&\n \tgit add some/sub/dir/delfile &&\n \tgit apply --directory=some/sub/dir/ --index patch &&\n-\t! (git ls-files | grep delfile)\n+\tgit ls-files >out &&\n+\t! grep delfile out\n '\n \n cat > patch << 'EOF'\n-- \n2.35.1.1226.g8b497615d32\n\n"},{"id":"450137","messageId":"patch-14.15-34cada14fec-20220302T171755Z-avarab@gmail.com","threadId":"57499","inReplyTo":"cover-00.15-00000000000-20220302T171755Z-avarab@gmail.com","subject":"[PATCH 14/15] checkout tests: don't ignore \"git \" exit code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:27:23Z","receivedAt":"2022-03-02T17:28:11Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change a fragile pattern introduced in 696acf45f96 (checkout:\nimplement \"-\" abbreviation, add docs and tests, 2009-01-17) to check\nthe exit code of both \"git symbolic-ref\" and \"git rev-parse\".\n\nWithout this change this test will become flaky e.g. under\nSANITIZE=leak if some (but not all) memory leaks revealed by these\ncommands are fixed.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t2012-checkout-last.sh | 51 +++++++++++++++++++++++++++-------------\n 1 file changed, 35 insertions(+), 16 deletions(-)\n\ndiff --git a/t/t2012-checkout-last.sh b/t/t2012-checkout-last.sh\nindex 42601d5a310..1f6c4ed0428 100755\n--- a/t/t2012-checkout-last.sh\n+++ b/t/t2012-checkout-last.sh\n@@ -21,14 +21,20 @@ test_expect_success 'first branch switch' '\n \tgit checkout other\n '\n \n+test_cmp_symbolic_HEAD_ref () {\n+\techo refs/heads/\"$1\" >expect &&\n+\tgit symbolic-ref HEAD >actual &&\n+\ttest_cmp expect actual\n+}\n+\n test_expect_success '\"checkout -\" switches back' '\n \tgit checkout - &&\n-\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/main\"\n+\ttest_cmp_symbolic_HEAD_ref main\n '\n \n test_expect_success '\"checkout -\" switches forth' '\n \tgit checkout - &&\n-\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/other\"\n+\ttest_cmp_symbolic_HEAD_ref other\n '\n \n test_expect_success 'detach HEAD' '\n@@ -37,12 +43,16 @@ test_expect_success 'detach HEAD' '\n \n test_expect_success '\"checkout -\" attaches again' '\n \tgit checkout - &&\n-\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/other\"\n+\ttest_cmp_symbolic_HEAD_ref other\n '\n \n test_expect_success '\"checkout -\" detaches again' '\n \tgit checkout - &&\n-\ttest \"z$(git rev-parse HEAD)\" = \"z$(git rev-parse other)\" &&\n+\n+\tgit rev-parse other >expect &&\n+\tgit rev-parse HEAD >actual &&\n+\ttest_cmp expect actual &&\n+\n \ttest_must_fail git symbolic-ref HEAD\n '\n \n@@ -63,31 +73,31 @@ more_switches () {\n test_expect_success 'switch to the last' '\n \tmore_switches &&\n \tgit checkout @{-1} &&\n-\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/branch2\"\n+\ttest_cmp_symbolic_HEAD_ref branch2\n '\n \n test_expect_success 'switch to second from the last' '\n \tmore_switches &&\n \tgit checkout @{-2} &&\n-\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/branch3\"\n+\ttest_cmp_symbolic_HEAD_ref branch3\n '\n \n test_expect_success 'switch to third from the last' '\n \tmore_switches &&\n \tgit checkout @{-3} &&\n-\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/branch4\"\n+\ttest_cmp_symbolic_HEAD_ref branch4\n '\n \n test_expect_success 'switch to fourth from the last' '\n \tmore_switches &&\n \tgit checkout @{-4} &&\n-\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/branch5\"\n+\ttest_cmp_symbolic_HEAD_ref branch5\n '\n \n test_expect_success 'switch to twelfth from the last' '\n \tmore_switches &&\n \tgit checkout @{-12} &&\n-\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/branch13\"\n+\ttest_cmp_symbolic_HEAD_ref branch13\n '\n \n test_expect_success 'merge base test setup' '\n@@ -98,19 +108,28 @@ test_expect_success 'merge base test setup' '\n test_expect_success 'another...main' '\n \tgit checkout another &&\n \tgit checkout another...main &&\n-\ttest \"z$(git rev-parse --verify HEAD)\" = \"z$(git rev-parse --verify main^)\"\n+\n+\tgit rev-parse --verify main^ >expect &&\n+\tgit rev-parse --verify HEAD >actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success '...main' '\n \tgit checkout another &&\n \tgit checkout ...main &&\n-\ttest \"z$(git rev-parse --verify HEAD)\" = \"z$(git rev-parse --verify main^)\"\n+\n+\tgit rev-parse --verify main^ >expect &&\n+\tgit rev-parse --verify HEAD >actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'main...' '\n \tgit checkout another &&\n \tgit checkout main... &&\n-\ttest \"z$(git rev-parse --verify HEAD)\" = \"z$(git rev-parse --verify main^)\"\n+\n+\tgit rev-parse --verify main^ >expect &&\n+\tgit rev-parse --verify HEAD >actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success '\"checkout -\" works after a rebase A' '\n@@ -118,7 +137,7 @@ test_expect_success '\"checkout -\" works after a rebase A' '\n \tgit checkout other &&\n \tgit rebase main &&\n \tgit checkout - &&\n-\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/main\"\n+\ttest_cmp_symbolic_HEAD_ref main\n '\n \n test_expect_success '\"checkout -\" works after a rebase A B' '\n@@ -127,7 +146,7 @@ test_expect_success '\"checkout -\" works after a rebase A B' '\n \tgit checkout other &&\n \tgit rebase main moodle &&\n \tgit checkout - &&\n-\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/main\"\n+\ttest_cmp_symbolic_HEAD_ref main\n '\n \n test_expect_success '\"checkout -\" works after a rebase -i A' '\n@@ -135,7 +154,7 @@ test_expect_success '\"checkout -\" works after a rebase -i A' '\n \tgit checkout other &&\n \tgit rebase -i main &&\n \tgit checkout - &&\n-\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/main\"\n+\ttest_cmp_symbolic_HEAD_ref main\n '\n \n test_expect_success '\"checkout -\" works after a rebase -i A B' '\n@@ -144,7 +163,7 @@ test_expect_success '\"checkout -\" works after a rebase -i A B' '\n \tgit checkout other &&\n \tgit rebase main foodle &&\n \tgit checkout - &&\n-\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/main\"\n+\ttest_cmp_symbolic_HEAD_ref main\n '\n \n test_done\n-- \n2.35.1.1226.g8b497615d32\n\n"},{"id":"450138","messageId":"patch-15.15-4ee216711cf-20220302T171755Z-avarab@gmail.com","threadId":"57499","inReplyTo":"cover-00.15-00000000000-20220302T171755Z-avarab@gmail.com","subject":"[PATCH 15/15] rev-list simplify tests: don't ignore \"git\" exit code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:27:24Z","receivedAt":"2022-03-02T17:28:13Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change a fragile test pattern introduced in 65347030590 (Topo-sort\nbefore --simplify-merges, 2008-08-03) to check the exit code of both\n\"git name-rev\" and \"git log\".\n\nThis test as a whole would fail under SANITIZE=leak, but we'd pass\nseveral \"failing\" tests due to hiding these exit codes before we'd\nspot git dying with abort(). Now we'll instead spot all of the\nfailures.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t6012-rev-list-simplify.sh | 12 +++++++-----\n 1 file changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t6012-rev-list-simplify.sh b/t/t6012-rev-list-simplify.sh\nindex 63fcccec32e..de1e87f1621 100755\n--- a/t/t6012-rev-list-simplify.sh\n+++ b/t/t6012-rev-list-simplify.sh\n@@ -12,7 +12,9 @@ note () {\n }\n \n unnote () {\n-\tgit name-rev --tags --annotate-stdin | sed -e \"s|$OID_REGEX (tags/\\([^)]*\\)) |\\1 |g\"\n+\ttest_when_finished \"rm -f tmp\" &&\n+\tgit name-rev --tags --annotate-stdin >tmp &&\n+\tsed -e \"s|$OID_REGEX (tags/\\([^)]*\\)) |\\1 |g\" actual &&\n+\t\tgit log --pretty=\"$FMT\" --parents $param >out &&\n+\t\tunnote >actual check expect &&\n-\tgit log --pretty=\"$FMT\" --full-history E -- lost |\n-\tunnote >actual &&\n+\tgit log --pretty=\"$FMT\" --full-history E -- lost >out &&\n+\tunnote >actual check \n---\n t/t6102-rev-list-unexpected-objects.sh | 13 +++++++++----\n 1 file changed, 9 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t6102-rev-list-unexpected-objects.sh b/t/t6102-rev-list-unexpected-objects.sh\nindex 6f0902b8638..cf0195e8263 100755\n--- a/t/t6102-rev-list-unexpected-objects.sh\n+++ b/t/t6102-rev-list-unexpected-objects.sh\n@@ -17,8 +17,13 @@ test_expect_success 'setup unexpected non-blob entry' '\n \tbroken_tree=\"$(git hash-object -w --literally -t tree broken-tree)\"\n '\n \n-test_expect_failure 'traverse unexpected non-blob entry (lone)' '\n-\ttest_must_fail git rev-list --objects $broken_tree\n+test_expect_success !SANITIZE_LEAK 'TODO (should fail!): traverse unexpected non-blob entry (lone)' '\n+\tsed \"s/Z$//\" >expect <<-EOF &&\n+\t$broken_tree Z\n+\t$tree foo\n+\tEOF\n+\tgit rev-list --objects $broken_tree >actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'traverse unexpected non-blob entry (seen)' '\n@@ -116,8 +121,8 @@ test_expect_success 'setup unexpected non-blob tag' '\n \ttag=$(git hash-object -w --literally -t tag broken-tag)\n '\n \n-test_expect_failure 'traverse unexpected non-blob tag (lone)' '\n-\ttest_must_fail git rev-list --objects $tag\n+test_expect_success !SANITIZE_LEAK 'TODO (should fail!): traverse unexpected non-blob tag (lone)' '\n+\tgit rev-list --objects $tag\n '\n \n test_expect_success 'traverse unexpected non-blob tag (seen)' '\n-- \n2.35.1.1226.g8b497615d32\n\n"},{"id":"450140","messageId":"patch-10.15-946397033d4-20220302T171755Z-avarab@gmail.com","threadId":"57499","inReplyTo":"cover-00.15-00000000000-20220302T171755Z-avarab@gmail.com","subject":"[PATCH 10/15] diff tests: don't ignore \"git rev-list\" exit code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:27:19Z","receivedAt":"2022-03-02T17:28:16Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change a fragile pattern introduced in 2b459b483cb (diff: make sure\nwork tree side is shown as 0{40} when different, 2008-03-02) to check\nthe exit code of \"git rev-list\", while we're at it let's get rid of\nthe needless sub-shell for invoking it in favor of the \"-C\" option.\n\nBecause of this I'd marked these tests as passing under SANITIZE=leak\nin 16d4bd4f14e (leak tests: mark some diff tests as passing with\nSANITIZE=leak, 2021-10-31), let's remove the\n\"TEST_PASSES_SANITIZE_LEAK=true\" annotation as they no longer do.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t4027-diff-submodule.sh | 7 ++-----\n 1 file changed, 2 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh\nindex 6cef0da982f..295da987cce 100755\n--- a/t/t4027-diff-submodule.sh\n+++ b/t/t4027-diff-submodule.sh\n@@ -2,7 +2,6 @@\n \n test_description='difference in submodules'\n \n-TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-diff.sh\n \n@@ -28,10 +27,8 @@ test_expect_success setup '\n \t\tgit commit -m \"submodule #2\"\n \t) &&\n \n-\tset x $(\n-\t\tcd sub &&\n-\t\tgit rev-list HEAD\n-\t) &&\n+\tgit -C sub rev-list HEAD >revs &&\n+\tset x $(cat revs) &&\n \techo \":160000 160000 $3 $ZERO_OID M\tsub\" >expect &&\n \tsubtip=$3 subprev=$2\n '\n-- \n2.35.1.1226.g8b497615d32\n\n"},{"id":"450141","messageId":"patch-09.15-ca9e12f2bac-20220302T171755Z-avarab@gmail.com","threadId":"57499","inReplyTo":"cover-00.15-00000000000-20220302T171755Z-avarab@gmail.com","subject":"[PATCH 09/15] notes tests: don't ignore \"git\" exit code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:27:18Z","receivedAt":"2022-03-02T17:28:17Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change a fragile test pattern that's been with us ever since these\ntests were introduced in [1], [2] and [3] to properly return the exit\ncode of the failing command on failure.\n\nBecause of this I'd marked this test as passing under SANITIZE=leak in\n[4] and [5]. We need to remove those annotations as these tests will\nno longer pass.\n\n1. 9081a421a6d (checkout: fix \"branch info\" memory leaks, 2021-11-16)\n2. 0057c0917d3 (Add selftests verifying that we can parse notes trees\n with various fanouts, 2009-10-09)\n3. 048cdd4665e (t3305: Verify that adding many notes with git-notes\n triggers increased fanout, 2010-02-13)\n4. ca089724952 (leak tests: mark some notes tests as passing with\n SANITIZE=leak, 2021-10-31)\n5. 9081a421a6d (checkout: fix \"branch info\" memory leaks, 2021-11-16)\n---\n t/t3302-notes-index-expensive.sh | 6 +++---\n t/t3303-notes-subtrees.sh | 9 ++++-----\n t/t3305-notes-fanout.sh | 14 +++++++-------\n 3 files changed, 14 insertions(+), 15 deletions(-)\n\ndiff --git a/t/t3302-notes-index-expensive.sh b/t/t3302-notes-index-expensive.sh\nindex bc9d8ee1e6a..bb5fea02a03 100755\n--- a/t/t3302-notes-index-expensive.sh\n+++ b/t/t3302-notes-index-expensive.sh\n@@ -8,7 +8,6 @@ test_description='Test commit notes index (expensive!)'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n-TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n create_repo () {\n@@ -65,7 +64,8 @@ create_repo () {\n test_notes () {\n \tcount=$1 &&\n \tgit config core.notesRef refs/notes/commits &&\n-\tgit log | grep \"^ \" >output &&\n+\tgit log >tmp &&\n+\tgrep \"^ \" tmp >output &&\n \ti=$count &&\n \twhile test $i -gt 0\n \tdo\n@@ -90,7 +90,7 @@ write_script time_notes <<\\EOF\n \t\t\tunset GIT_NOTES_REF\n \t\t\t;;\n \t\tesac\n-\t\tgit log\n+\t\tgit log || exit $?\n \t\ti=$(($i+1))\n \tdone >/dev/null\n EOF\ndiff --git a/t/t3303-notes-subtrees.sh b/t/t3303-notes-subtrees.sh\nindex 7e0a8960af8..eac193757bf 100755\n--- a/t/t3303-notes-subtrees.sh\n+++ b/t/t3303-notes-subtrees.sh\n@@ -5,7 +5,6 @@ test_description='Test commit notes organized in subtrees'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n-TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n number_of_commits=100\n@@ -79,7 +78,7 @@ test_sha1_based () {\n \t(\n \t\tstart_note_commit &&\n \t\tnr=$number_of_commits &&\n-\t\tgit rev-list refs/heads/main |\n+\t\tgit rev-list refs/heads/main >out &&\n \t\twhile read sha1; do\n \t\t\tnote_path=$(echo \"$sha1\" | sed \"$1\")\n \t\t\tcat <gfi &&\n+\tgit fast-import --quiet /dev/null |\n+\tgit ls-tree -r --name-only $notes_commit |\n \twhile read path\n \tdo\n \t\tpath_has_fanout $path $fanout || return 1\n@@ -51,8 +50,9 @@ test_expect_success 'creating many notes with git-notes' '\n \tdone\n '\n \n-test_expect_success 'many notes created correctly with git-notes' '\n-\tgit log | grep \"^ \" > output &&\n+test_expect_success !SANITIZE_LEAK 'many notes created correctly with git-notes' '\n+\tgit log >output.raw &&\n+\tgrep \"^ \" output.raw >output &&\n \ti=$num_notes &&\n \twhile test $i -gt 0\n \tdo\n@@ -91,13 +91,13 @@ test_expect_success 'stable fanout 0 is followed by stable fanout 1' '\n test_expect_success 'deleting most notes with git-notes' '\n \tremove_notes=285 &&\n \ti=0 &&\n-\tgit rev-list HEAD |\n+\tgit rev-list HEAD >revs &&\n \twhile test $i -lt $remove_notes && read sha1\n \tdo\n \t\ti=$(($i + 1)) &&\n \t\ttest_tick &&\n-\t\tgit notes remove \"$sha1\" 2>/dev/null || return 1\n-\tdone\n+\t\tgit notes remove \"$sha1\" || return 1\n+\tdone \n---\n t/t1001-read-tree-m-2way.sh | 6 +++---\n t/t1002-read-tree-m-u-2way.sh | 6 +++---\n 2 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t1001-read-tree-m-2way.sh b/t/t1001-read-tree-m-2way.sh\nindex d1115528cb9..0710b1fb1e9 100755\n--- a/t/t1001-read-tree-m-2way.sh\n+++ b/t/t1001-read-tree-m-2way.sh\n@@ -21,7 +21,6 @@ In the test, these paths are used:\n \tyomin - not in H or M\n '\n \n-TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-read-tree.sh\n \n@@ -38,11 +37,12 @@ compare_change () {\n }\n \n check_cache_at () {\n-\tclean_if_empty=$(git diff-files -- \"$1\")\n+\tgit diff-files -- \"$1\" >out &&\n+\tclean_if_empty=$(cat out) &&\n \tcase \"$clean_if_empty\" in\n \t'') echo \"$1: clean\" ;;\n \t?*) echo \"$1: dirty\" ;;\n-\tesac\n+\tesac &&\n \tcase \"$2,$clean_if_empty\" in\n \tclean,)\t\t: ;;\n \tclean,?*)\tfalse ;;\ndiff --git a/t/t1002-read-tree-m-u-2way.sh b/t/t1002-read-tree-m-u-2way.sh\nindex ca5c5510c73..46cbd5514a6 100755\n--- a/t/t1002-read-tree-m-u-2way.sh\n+++ b/t/t1002-read-tree-m-u-2way.sh\n@@ -9,7 +9,6 @@ This is identical to t1001, but uses -u to update the work tree as well.\n \n '\n \n-TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-read-tree.sh\n \n@@ -23,11 +22,12 @@ compare_change () {\n }\n \n check_cache_at () {\n-\tclean_if_empty=$(git diff-files -- \"$1\")\n+\tgit diff-files -- \"$1\" >out &&\n+\tclean_if_empty=$(cat out) &&\n \tcase \"$clean_if_empty\" in\n \t'') echo \"$1: clean\" ;;\n \t?*) echo \"$1: dirty\" ;;\n-\tesac\n+\tesac &&\n \tcase \"$2,$clean_if_empty\" in\n \tclean,)\t\t: ;;\n \tclean,?*)\tfalse ;;\n-- \n2.35.1.1226.g8b497615d32\n\n"},{"id":"450143","messageId":"patch-06.15-7952ae1f3b5-20220302T171755Z-avarab@gmail.com","threadId":"57499","inReplyTo":"cover-00.15-00000000000-20220302T171755Z-avarab@gmail.com","subject":"[PATCH 06/15] apply tests: use \"test_must_fail\" instead of ad-hoc pattern","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:27:15Z","receivedAt":"2022-03-02T17:28:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change a fragile test pattern introduced in 6b763c424e4 (git-apply: do\nnot read past the end of buffer, 2007-09-05). Before this we wouldn't\ndistinguish normal \"git apply\" failures from segfaults or abort().\n\nI'd previously marked this test as passing under SANITIZE=leak in\nf54f48fc074 (leak tests: mark some apply tests as passing with\nSANITIZE=leak, 2021-10-31). Let's remove that annotation as this test\nwill no longer pass.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t4123-apply-shrink.sh | 18 ++----------------\n 1 file changed, 2 insertions(+), 16 deletions(-)\n\ndiff --git a/t/t4123-apply-shrink.sh b/t/t4123-apply-shrink.sh\nindex dfa053ff28e..3ef84619f53 100755\n--- a/t/t4123-apply-shrink.sh\n+++ b/t/t4123-apply-shrink.sh\n@@ -2,8 +2,6 @@\n \n test_description='apply a patch that is larger than the preimage'\n \n-\n-TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n cat >F <<\\EOF\n@@ -41,20 +39,8 @@ test_expect_success setup '\n '\n \n test_expect_success 'apply should fail gracefully' '\n-\n-\tif git apply --index patch\n-\tthen\n-\t\techo Oops, should not have succeeded\n-\t\tfalse\n-\telse\n-\t\tstatus=$? &&\n-\t\techo \"Status was $status\" &&\n-\t\tif test -f .git/index.lock\n-\t\tthen\n-\t\t\techo Oops, should not have crashed\n-\t\t\tfalse\n-\t\tfi\n-\tfi\n+\ttest_must_fail git apply --index patch &&\n+\ttest_path_is_missing .git/index.lock\n '\n \n test_done\n-- \n2.35.1.1226.g8b497615d32\n\n"},{"id":"450144","messageId":"patch-04.15-a425ced5609-20220302T171755Z-avarab@gmail.com","threadId":"57499","inReplyTo":"cover-00.15-00000000000-20220302T171755Z-avarab@gmail.com","subject":"[PATCH 04/15] diff tests: don't ignore \"git diff\" exit code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-02T17:27:13Z","receivedAt":"2022-03-02T17:28:26Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a test pattern that originated in f1af60bdba4 (Support 'diff=pgm'\nattribute, 2007-04-22) so that we'll stop using \"git diff\" on the\nleft-hand-side of a pipe, and thus ignoring its exit code.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t4020-diff-external.sh | 49 ++++++++++++++++++----------------------\n 1 file changed, 22 insertions(+), 27 deletions(-)\n\ndiff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh\nindex 54bb8ef27e7..879ee04d291 100755\n--- a/t/t4020-diff-external.sh\n+++ b/t/t4020-diff-external.sh\n@@ -37,17 +37,15 @@ test_expect_success 'GIT_EXTERNAL_DIFF environment' '\n \n '\n \n-test_expect_success 'GIT_EXTERNAL_DIFF environment should apply only to diff' '\n-\n-\tGIT_EXTERNAL_DIFF=echo git log -p -1 HEAD |\n-\tgrep \"^diff --git a/file b/file\"\n+test_expect_success !SANITIZE_LEAK 'GIT_EXTERNAL_DIFF environment should apply only to diff' '\n+\tGIT_EXTERNAL_DIFF=echo git log -p -1 HEAD >out &&\n+\tgrep \"^diff --git a/file b/file\" out\n \n '\n \n test_expect_success 'GIT_EXTERNAL_DIFF environment and --no-ext-diff' '\n-\n-\tGIT_EXTERNAL_DIFF=echo git diff --no-ext-diff |\n-\tgrep \"^diff --git a/file b/file\"\n+\tGIT_EXTERNAL_DIFF=echo git diff --no-ext-diff >out &&\n+\tgrep \"^diff --git a/file b/file\" out\n \n '\n \n@@ -83,16 +81,16 @@ test_expect_success 'diff.external' '\n \t}\n '\n \n-test_expect_success 'diff.external should apply only to diff' '\n+test_expect_success !SANITIZE_LEAK 'diff.external should apply only to diff' '\n \ttest_config diff.external echo &&\n-\tgit log -p -1 HEAD |\n-\tgrep \"^diff --git a/file b/file\"\n+\tgit log -p -1 HEAD >out &&\n+\tgrep \"^diff --git a/file b/file\" out\n '\n \n test_expect_success 'diff.external and --no-ext-diff' '\n \ttest_config diff.external echo &&\n-\tgit diff --no-ext-diff |\n-\tgrep \"^diff --git a/file b/file\"\n+\tgit diff --no-ext-diff >out &&\n+\tgrep \"^diff --git a/file b/file\" out\n '\n \n test_expect_success 'diff attribute' '\n@@ -115,17 +113,15 @@ test_expect_success 'diff attribute' '\n \n '\n \n-test_expect_success 'diff attribute should apply only to diff' '\n-\n-\tgit log -p -1 HEAD |\n-\tgrep \"^diff --git a/file b/file\"\n+test_expect_success !SANITIZE_LEAK 'diff attribute should apply only to diff' '\n+\tgit log -p -1 HEAD >out &&\n+\tgrep \"^diff --git a/file b/file\" out\n \n '\n \n test_expect_success 'diff attribute and --no-ext-diff' '\n-\n-\tgit diff --no-ext-diff |\n-\tgrep \"^diff --git a/file b/file\"\n+\tgit diff --no-ext-diff >out &&\n+\tgrep \"^diff --git a/file b/file\" out\n \n '\n \n@@ -148,17 +144,15 @@ test_expect_success 'diff attribute' '\n \n '\n \n-test_expect_success 'diff attribute should apply only to diff' '\n-\n-\tgit log -p -1 HEAD |\n-\tgrep \"^diff --git a/file b/file\"\n+test_expect_success !SANITIZE_LEAK 'diff attribute should apply only to diff' '\n+\tgit log -p -1 HEAD >out &&\n+\tgrep \"^diff --git a/file b/file\" out\n \n '\n \n test_expect_success 'diff attribute and --no-ext-diff' '\n-\n-\tgit diff --no-ext-diff |\n-\tgrep \"^diff --git a/file b/file\"\n+\tgit diff --no-ext-diff >out &&\n+\tgrep \"^diff --git a/file b/file\" out\n \n '\n \n@@ -177,7 +171,8 @@ test_expect_success 'attributes trump GIT_EXTERNAL_DIFF and diff.external' '\n \n test_expect_success 'no diff with -diff' '\n \techo >.gitattributes \"file -diff\" &&\n-\tgit diff | grep Binary\n+\tgit diff >out &&\n+\tgrep Binary out\n '\n \n echo NULZbetweenZwords | perl -pe 'y/Z/\\000/' > file\n-- \n2.35.1.1226.g8b497615d32\n\n"},{"id":"450191","messageId":"xmqqk0dcgdin.fsf@gitster.g","threadId":"57499","inReplyTo":"patch-03.15-5f02e30d1ab-20220302T171755Z-avarab@gmail.com","subject":"Re: [PATCH 03/15] read-tree tests: check \"diff-files\" exit code on failure","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-03-02T23:06:56Z","receivedAt":"2022-03-02T23:46:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> check_cache_at () {\n> -\tclean_if_empty=$(git diff-files -- \"$1\")\n> +\tgit diff-files -- \"$1\" >out &&\n> +\tclean_if_empty=$(cat out) &&\n> \tcase \"$clean_if_empty\" in\n> \t'') echo \"$1: clean\" ;;\n> \t?*) echo \"$1: dirty\" ;;\n> -\tesac\n> +\tesac &&\n\nGood to see such an attention to the detail.\n\n> \tcase \"$2,$clean_if_empty\" in\n> \tclean,)\t\t: ;;\n> \tclean,?*)\tfalse ;;\n> diff --git a/t/t1002-read-tree-m-u-2way.sh b/t/t1002-read-tree-m-u-2way.sh\n> index ca5c5510c73..46cbd5514a6 100755\n> --- a/t/t1002-read-tree-m-u-2way.sh\n> +++ b/t/t1002-read-tree-m-u-2way.sh\n> @@ -9,7 +9,6 @@ This is identical to t1001, but uses -u to update the work tree as well.\n> \n> '\n> \n> -TEST_PASSES_SANITIZE_LEAK=true\n> . ./test-lib.sh\n> . \"$TEST_DIRECTORY\"/lib-read-tree.sh\n> \n> @@ -23,11 +22,12 @@ compare_change () {\n> }\n> \n> check_cache_at () {\n> -\tclean_if_empty=$(git diff-files -- \"$1\")\n> +\tgit diff-files -- \"$1\" >out &&\n> +\tclean_if_empty=$(cat out) &&\n> \tcase \"$clean_if_empty\" in\n> \t'') echo \"$1: clean\" ;;\n> \t?*) echo \"$1: dirty\" ;;\n> -\tesac\n> +\tesac &&\n> \tcase \"$2,$clean_if_empty\" in\n> \tclean,)\t\t: ;;\n> \tclean,?*)\tfalse ;;\n"},{"id":"450192","messageId":"xmqqfso0gddv.fsf@gitster.g","threadId":"57499","inReplyTo":"patch-06.15-7952ae1f3b5-20220302T171755Z-avarab@gmail.com","subject":"Re: [PATCH 06/15] apply tests: use \"test_must_fail\" instead of ad-hoc pattern","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-03-02T23:09:48Z","receivedAt":"2022-03-02T23:46:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> test_expect_success 'apply should fail gracefully' '\n> -\n> -\tif git apply --index patch\n> -\tthen\n> -\t\techo Oops, should not have succeeded\n> -\t\tfalse\n> -\telse\n> -\t\tstatus=$? &&\n> -\t\techo \"Status was $status\" &&\n> -\t\tif test -f .git/index.lock\n> -\t\tthen\n> -\t\t\techo Oops, should not have crashed\n> -\t\t\tfalse\n> -\t\tfi\n> -\tfi\n> +\ttest_must_fail git apply --index patch &&\n> +\ttest_path_is_missing .git/index.lock\n> '\n\nWow, that is very old fashioned. Thanks for cleaning up the\nleftover mess from the days before test_must_fail was prevalent.\n"},{"id":"450197","messageId":"xmqqa6e8gd24.fsf@gitster.g","threadId":"57499","inReplyTo":"patch-11.15-26c838f0560-20220302T171755Z-avarab@gmail.com","subject":"Re: [PATCH 11/15] rev-list tests: don't hide abort() in \"test_expect_failure\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-03-02T23:16:51Z","receivedAt":"2022-03-02T23:54:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Change a couple of uses of \"test_expect_failure\" to use a\n> \"test_expect_success\" to positively assert the current behavior, and\n> replace the intent of \"test_expect_failure\" with a \"TODO\" comment int\n> the description.\n>\n> As noted in [1] the \"test_expect_failure\" feature is overly eager to\n\nAnd noted in [2], it is not a good idea to abuse \"test_expect_success\"\nfor this purpose, either, though.\n\n[2] https://lore.kernel.org/git/xmqq4k9kj15p.fsf@gitster.g/\n"},{"id":"450198","messageId":"xmqq5yowgcvg.fsf@gitster.g","threadId":"57499","inReplyTo":"patch-12.15-f3cc5bc7eb9-20220302T171755Z-avarab@gmail.com","subject":"Re: [PATCH 12/15] gettext tests: don't ignore \"test-tool regex\" exit code","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-03-02T23:20:51Z","receivedAt":"2022-03-02T23:56:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> -test_have_prereq GETTEXT_LOCALE &&\n> -test-tool regex \"HALLÓ\" \"Halló\" ICASE &&\n> -test_set_prereq REGEX_LOCALE\n> +test_expect_success GETTEXT_LOCALE 'setup REGEX_LOCALE prerequisite' '\n> +\tif test-tool regex \"HALLÓ\" \"Halló\" ICASE\n> +\tthen\n> +\t\ttest_set_prereq REGEX_LOCALE\n\nThis looks sensible but\n\n> +\telse\n> +\t\ttest_must_fail test-tool regex \"HALLÓ\" \"Halló\" ICASE\n\nthis side looks puzzling. I think this way to avoid counting abort\netc as passing \"must fail\" test would be the least bad that we can\ndo.\n\nNicely done.\n\n\n\n> +\tfi\n> +'\n\n\n\n"},{"id":"450200","messageId":"xmqqy21sey6t.fsf@gitster.g","threadId":"57499","inReplyTo":"cover-00.15-00000000000-20220302T171755Z-avarab@gmail.com","subject":"Re: [PATCH 00/15] tests: don't ignore \"git\" exit codes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-03-02T23:23:22Z","receivedAt":"2022-03-03T00:01:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> This series fixes issues where we ignored the exit code of \"git\" due\n> to it being on the LHS of a pipe, or because we interpolated its\n> output with $() in a \"test\" construct, or had missing &&- chains in\n> helper functions etc.\n\nThanks. I've looked at all the steps, left some comments, and it\nwas a pleasant read overall. Writing and reviewing all these\nchanges, both of us must have too much time on our hands ;-)\n\nWill queue.\n"},{"id":"450239","messageId":"76a1ff22-3eb0-08fb-5aa9-e612ee5b522f@github.com","threadId":"57499","inReplyTo":"cover-00.15-00000000000-20220302T171755Z-avarab@gmail.com","subject":"Re: [PATCH 00/15] tests: don't ignore \"git\" exit codes","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-03-03T02:02:05Z","receivedAt":"2022-03-03T02:02:18Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 3/2/2022 12:27 PM, Ævar Arnfjörð Bjarmason wrote:\n> This series fixes issues where we ignored the exit code of \"git\" due\n> to it being on the LHS of a pipe, or because we interpolated its\n> output with $() in a \"test\" construct, or had missing &&- chains in\n> helper functions etc.\n> \n> This series is not made by string-replacing things in our test suite,\n> if it was it would be much larger. These are all tests I've seen real\n> hide real failures under SANITIZE=leak, either on current \"master\", or\n> in combination with various local leak fixes I've got unsubmitted.\n\nMy first reaction was to check that Subham was on the CC line (yes)\nbecause they have been working in this space, too. Your focus on\nexamples that break SANITIZE=leak is appreciated so there is room\nfor that project.\n\n> In cases where I was starting to fix a pattern in a file I'd fix the\n> rest of the file if it was easy, but otherwise these are all cases\n> where I ran SANITIZE=leak, had a test pass, but having ASAN_OPTIONS\n> log to a file revealed that we had memory leaks within that test.\n\nNeat trick.\n\nThe patches in this series clearly do the right transformations to\nexpose these errors at the appropriate time. The only time I got\nhung up was patch 11 where test_expect_failure was swapped for\ntest_expect_success (while also dropping a test_must_fail inside\nthe test). The double-negative confused me at first, but in the end\nthe patch works as-is.\n\n> As an aside we still have various potential issues with hidden\n> segfaults etc. in the test suite after this that are tricked to solve,\n> because:\n> \n> * Our tests will (mostly) catch segfaults and abort(), but if we\n> invoke a command that invokes another command it needs to ferry the\n> exit code up to us.\n> \n> * run-command.c notably does not do that, so for e.g. \"git push\"\n> tests where we expect a failure and an underlying \"git\" command\n> fails we won't ferry up the segfault or abort exit code.\n\nPerhaps run-command.c could auto-exit for certain well-known error\ncodes that could only happen on certain kinds of failures (segfault,\nfor example). A simple die() might be something that is expected to\nbe handled by the top-level command in some cases.\n\n> * We have gitweb.perl and some other perl code ignoring return values\n> from close(), i.e. ignoring exit codes from \"git rev-parse\" et al.\n> \n> * We have in-tree shellscripts like \"git-merge-one-file.sh\" invoking\n> git commands, and if they fail returning \"1\", not ferrying up the\n> segfault or abort() exit code.\n\nThese are more involved and harder to evaluate. Add them to the pile\nof projects for new contributors?\n\nThanks,\n-Stolee\n"},{"id":"450242","messageId":"CAC316V7jcHTVs7f_p5zMRqpvTZBPCa6X7=L_MUEy0Zx0PJZZ_A@mail.gmail.com","threadId":"57499","inReplyTo":"cover-00.15-00000000000-20220302T171755Z-avarab@gmail.com","subject":"Re: [PATCH 00/15] tests: don't ignore \"git\" exit codes","fromName":"Shubham Mishra","fromEmail":"shivam828787@gmail.com","sentAt":"2022-03-03T05:09:42Z","receivedAt":"2022-03-03T05:16:13Z","isPatch":true,"sender":{"key":"shivam828787@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42690084?v=4"},"body":"Thanks Ævar for CCing me. Your patch is insightful for me.\n\n> This series is not made by string-replacing things in our test suite,\n> if it was it would be much larger. These are all tests I've seen real\n> hide real failures under SANITIZE=leak, either on current \"master\", or\n> in combination with various local leak fixes I've got unsubmitted.\n\nCan you please tell me what \"SANITIZE=leak\" do?\n\nThanks,\nShubham\n"},{"id":"450251","messageId":"220303.86wnhb5pft.gmgdl@evledraar.gmail.com","threadId":"57499","inReplyTo":"CAC316V7jcHTVs7f_p5zMRqpvTZBPCa6X7=L_MUEy0Zx0PJZZ_A@mail.gmail.com","subject":"Re: [PATCH 00/15] tests: don't ignore \"git\" exit codes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-03T09:53:20Z","receivedAt":"2022-03-03T09:57:15Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Mar 03 2022, Shubham Mishra wrote:\n\n> Thanks Ævar for CCing me. Your patch is insightful for me.\n>\n>> This series is not made by string-replacing things in our test suite,\n>> if it was it would be much larger. These are all tests I've seen real\n>> hide real failures under SANITIZE=leak, either on current \"master\", or\n>> in combination with various local leak fixes I've got unsubmitted.\n>\n> Can you please tell me what \"SANITIZE=leak\" do?\n>\n> Thanks,\n> Shubham\n\nSANITIZE=address and SANITIZE=leak are the flags we use to turn on the\nAddressSanitizer\n(https://github.com/google/sanitizers/wiki/AddressSanitizer) and\nLeakSanitizer\n(https://github.com/google/sanitizers/wiki/AddressSanitizerLeakSanitizer).\n\nAs the latter URL shows it's a way to instrument a program to die on\nexit if unreferenced malloc'd memory wasn't free'd.\n"},{"id":"450254","messageId":"220303.86k0db5oqq.gmgdl@evledraar.gmail.com","threadId":"57499","inReplyTo":"xmqqa6e8gd24.fsf@gitster.g","subject":"Re: [PATCH 11/15] rev-list tests: don't hide abort() in \"test_expect_failure\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-03T10:10:11Z","receivedAt":"2022-03-03T10:12:20Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Mar 02 2022, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> Change a couple of uses of \"test_expect_failure\" to use a\n>> \"test_expect_success\" to positively assert the current behavior, and\n>> replace the intent of \"test_expect_failure\" with a \"TODO\" comment int\n>> the description.\n>>\n>> As noted in [1] the \"test_expect_failure\" feature is overly eager to\n>\n> And noted in [2], it is not a good idea to abuse \"test_expect_success\"\n> for this purpose, either, though.\n>\n> [2] https://lore.kernel.org/git/xmqq4k9kj15p.fsf@gitster.g/\n\nAs noted I do have a \"test_todo\" (or \"test_expect_todo\") replacement for\n\"test_expect_failure\" which I think I think will address your concern\nthere.\n\nBut do you mind if this is left like this for now? Due to the semantics\nof \"test_expect_failure\" we can't use it in conjunction with\n\"test_must_fail\" currently and not hide segfaults or abort().\n\nSo having it marked as \"ok ... # TODO\" v.s. \"not ok ... # TODO\" isn't\nideal, but certainly better than silently hiding abort() and segfaults.\n\n"},{"id":"450255","messageId":"220303.86fsnz5o9w.gmgdl@evledraar.gmail.com","threadId":"57499","inReplyTo":"76a1ff22-3eb0-08fb-5aa9-e612ee5b522f@github.com","subject":"Re: [PATCH 00/15] tests: don't ignore \"git\" exit codes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-03T10:13:02Z","receivedAt":"2022-03-03T10:22:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Mar 02 2022, Derrick Stolee wrote:\n\n> On 3/2/2022 12:27 PM, Ævar Arnfjörð Bjarmason wrote:\n>> This series fixes issues where we ignored the exit code of \"git\" due\n>> to it being on the LHS of a pipe, or because we interpolated its\n>> output with $() in a \"test\" construct, or had missing &&- chains in\n>> helper functions etc.\n>> \n>> This series is not made by string-replacing things in our test suite,\n>> if it was it would be much larger. These are all tests I've seen real\n>> hide real failures under SANITIZE=leak, either on current \"master\", or\n>> in combination with various local leak fixes I've got unsubmitted.\n>\n> My first reaction was to check that Subham was on the CC line (yes)\n> because they have been working in this space, too. Your focus on\n> examples that break SANITIZE=leak is appreciated so there is room\n> for that project.\n\nYeah, I'm certainly not meaning to steal this whole \"hide exit code\"\nmicroproject, and there's *a lot* more to chew on there :) (and I have\nno intention of doing the rest).\n\nBut hopefully this series also serves as a nice tour-de-force through\nthe test suite showing a wide variety of exit-code-hiding cases for any\nfuture work in the area.\n\nI.e. the \"git on theon LHS of a pipe\" is an overly narrow definition of\nit.\n\nBut then again I think these should also all be covered in passing by\nt/README's \"don'ts\" list.\n\n>> In cases where I was starting to fix a pattern in a file I'd fix the\n>> rest of the file if it was easy, but otherwise these are all cases\n>> where I ran SANITIZE=leak, had a test pass, but having ASAN_OPTIONS\n>> log to a file revealed that we had memory leaks within that test.\n>\n> Neat trick.\n>\n> The patches in this series clearly do the right transformations to\n> expose these errors at the appropriate time. The only time I got\n> hung up was patch 11 where test_expect_failure was swapped for\n> test_expect_success (while also dropping a test_must_fail inside\n> the test). The double-negative confused me at first, but in the end\n> the patch works as-is.\n>\n>> As an aside we still have various potential issues with hidden\n>> segfaults etc. in the test suite after this that are tricked to solve,\n>> because:\n>> \n>> * Our tests will (mostly) catch segfaults and abort(), but if we\n>> invoke a command that invokes another command it needs to ferry the\n>> exit code up to us.\n>> \n>> * run-command.c notably does not do that, so for e.g. \"git push\"\n>> tests where we expect a failure and an underlying \"git\" command\n>> fails we won't ferry up the segfault or abort exit code.\n>\n> Perhaps run-command.c could auto-exit for certain well-known error\n> codes that could only happen on certain kinds of failures (segfault,\n> for example). A simple die() might be something that is expected to\n> be handled by the top-level command in some cases.\n\nYes. I have a local WIP patch to make it do that.\n\nBasically just set a a global \"uh oh, one of our ran commands segfaulted\nor abort()-ed, here's that exit status\".\n\nThen when we do the real exit() (which we always intercept due to the\ntrace2 logging needing to do so) an non-zero exit() of e.g. status 128\nwill be changed to the appropriate exit status of that segfault or\nabort().\n\nWe'll thus ferry such exit codes upwards, clobbering whatever the\n\"desired\" exit code of e.g. \"git pull\" would have been (but that's a\nfeature in this case).\n\nOne thing I got hung up on is that it's relatively straightforward for\nthe wait_or_whine() case in run-command.c, but I didn't look into how to\ndo that with the pthread_join() we do for finish_async(), which\ne.g. happens if the multiplex'd dialog exits in this way.\n\n>> * We have gitweb.perl and some other perl code ignoring return values\n>> from close(), i.e. ignoring exit codes from \"git rev-parse\" et al.\n>> \n>> * We have in-tree shellscripts like \"git-merge-one-file.sh\" invoking\n>> git commands, and if they fail returning \"1\", not ferrying up the\n>> segfault or abort() exit code.\n>\n> These are more involved and harder to evaluate. Add them to the pile\n> of projects for new contributors?\n\nFWIW those are relatively easy, for the first:\n\n use v5.10.1;\n use autodie qw(close);\n\nAnd for the second just go through the relatively small amount of such\nremaining shell code and convert:\n\n\tif ! git ...\n\tthen\n\t\texit 1\n\nTo:\n\n\tgit ...\n\tcode=$?\n if test $code -ne 0\n\tthen\n\t\texit $code\n\nOr whatever.\n \n\n \n"},{"id":"450264","messageId":"e3090436-4479-bbc2-3b62-00473f6f530e@gmail.com","threadId":"57499","inReplyTo":"76a1ff22-3eb0-08fb-5aa9-e612ee5b522f@github.com","subject":"Re: [PATCH 00/15] tests: don't ignore \"git\" exit codes","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-03-03T14:06:14Z","receivedAt":"2022-03-03T14:06:20Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 03/03/2022 02:02, Derrick Stolee wrote:\n> On 3/2/2022 12:27 PM, Ævar Arnfjörð Bjarmason wrote:\n>> As an aside we still have various potential issues with hidden\n>> segfaults etc. in the test suite after this that are tricked to solve,\n>> because:\n>>\n>> * Our tests will (mostly) catch segfaults and abort(), but if we\n>> invoke a command that invokes another command it needs to ferry the\n>> exit code up to us.\n>>\n>> * run-command.c notably does not do that, so for e.g. \"git push\"\n\nI'm not sure what you mean by this, the return value of run_command() \nalready indicates which signal if any killed the child see for example \nrun_specified_editor() which re-raises SIGINT and SIGQUIT if the editor \nis killed by those signals.\n\n>> tests where we expect a failure and an underlying \"git\" command\n>> fails we won't ferry up the segfault or abort exit code.\n >\n> Perhaps run-command.c could auto-exit for certain well-known error\n> codes that could only happen on certain kinds of failures (segfault,\n> for example). A simple die() might be something that is expected to\n> be handled by the top-level command in some cases.\n\nI think we need to be careful that run_command() does not re-raise a \nsignal before the caller has a chance to do any cleanup. A caller to \nrun_command() can already check the return value and choose to die based \non that after doing any cleanup. If run_command() starts dying we'll end \nup adding more unsafe signal handlers to do the cleanup.\n\nBest Wishes\n\nPhillip\n\n>> * We have gitweb.perl and some other perl code ignoring return values\n>> from close(), i.e. ignoring exit codes from \"git rev-parse\" et al.\n>>\n>> * We have in-tree shellscripts like \"git-merge-one-file.sh\" invoking\n>> git commands, and if they fail returning \"1\", not ferrying up the\n>> segfault or abort() exit code.\n> \n> These are more involved and harder to evaluate. Add them to the pile\n> of projects for new contributors?\n> \n> Thanks,\n> -Stolee\n\n"},{"id":"450268","messageId":"220303.864k4f3uqe.gmgdl@evledraar.gmail.com","threadId":"57499","inReplyTo":"e3090436-4479-bbc2-3b62-00473f6f530e@gmail.com","subject":"Re: [PATCH 00/15] tests: don't ignore \"git\" exit codes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-03T15:35:16Z","receivedAt":"2022-03-03T15:45:52Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Mar 03 2022, Phillip Wood wrote:\n\n> On 03/03/2022 02:02, Derrick Stolee wrote:\n>> On 3/2/2022 12:27 PM, Ævar Arnfjörð Bjarmason wrote:\n>>> As an aside we still have various potential issues with hidden\n>>> segfaults etc. in the test suite after this that are tricked to solve,\n>>> because:\n>>>\n>>> * Our tests will (mostly) catch segfaults and abort(), but if we\n>>> invoke a command that invokes another command it needs to ferry the\n>>> exit code up to us.\n>>>\n>>> * run-command.c notably does not do that, so for e.g. \"git push\"\n>\n> I'm not sure what you mean by this, the return value of run_command()\n> already indicates which signal if any killed the child see for example \n> run_specified_editor() which re-raises SIGINT and SIGQUIT if the\n> editor is killed by those signals.\n\nYes, it returns the correct status code, but that doesn't help with\n(pseudo)code like:\n\n\tif (run_command(\"foo\")) /* exits with e.g. 123 */\n\t\tdie(\"oh no, foo failed\"); /* exits with 128 */\n\nI should have said \"code using run-command.c does not do that...\",\nsorry.\n\nI.e. if \"pack-objects\" or whatever invoked by a \"git push\" segfaults\nwe might exit with status 128, not the code that the underlying command\nfailed with, and thus lose the segfault or abort().\n\nIf you add the appropriate \"log_path\" to LSAN_OPTIONS and run the test\nsuite you can see where this fails. I'm adding a mode to test-lib.sh\nsooner than later (waiting on the outstanding ab/test-leak-diag) to make\nit trivial to report on those.\n\n>>> tests where we expect a failure and an underlying \"git\" command\n>>> fails we won't ferry up the segfault or abort exit code.\n>>\n>> Perhaps run-command.c could auto-exit for certain well-known error\n>> codes that could only happen on certain kinds of failures (segfault,\n>> for example). A simple die() might be something that is expected to\n>> be handled by the top-level command in some cases.\n>\n> I think we need to be careful that run_command() does not re-raise a\n> signal before the caller has a chance to do any cleanup. A caller to \n> run_command() can already check the return value and choose to die\n> based on that after doing any cleanup. If run_command() starts dying\n> we'll end up adding more unsafe signal handlers to do the cleanup.\n\nI think the method I described in\nhttps://lore.kernel.org/git/220303.86fsnz5o9w.gmgdl@evledraar.gmail.com/\nin the side-thread doesn't suffer from those problems.\n\nI.e. I think the solution to this is not to interrupt whatever code\ncalls run-command, we'll let it and whatever else the calling program\nwants to do run to completion.\n\nWe'll just ignore whatever exit(status) we picked at the very end and\nexit instead with the status of our a failing child process we invoked,\nif any of them returned a status that \"test_must_fail\" would count as\n\"failed but BAD!\"\n"},{"id":"450269","messageId":"220303.86zgm72g3j.gmgdl@evledraar.gmail.com","threadId":"57499","inReplyTo":"xmqq5yowgcvg.fsf@gitster.g","subject":"Re: [PATCH 12/15] gettext tests: don't ignore \"test-tool regex\" exit code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-03T15:46:22Z","receivedAt":"2022-03-03T15:47:56Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Mar 02 2022, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> -test_have_prereq GETTEXT_LOCALE &&\n>> -test-tool regex \"HALLÓ\" \"Halló\" ICASE &&\n>> -test_set_prereq REGEX_LOCALE\n>> +test_expect_success GETTEXT_LOCALE 'setup REGEX_LOCALE prerequisite' '\n>> +\tif test-tool regex \"HALLÓ\" \"Halló\" ICASE\n>> +\tthen\n>> +\t\ttest_set_prereq REGEX_LOCALE\n>\n> This looks sensible but\n>\n>> +\telse\n>> +\t\ttest_must_fail test-tool regex \"HALLÓ\" \"Halló\" ICASE\n>\n> this side looks puzzling. I think this way to avoid counting abort\n> etc as passing \"must fail\" test would be the least bad that we can\n> do.\n>\n> Nicely done.\n\nThanks. For the purposes of a re-roll I'll note this as a \"nothing to\nchange\", since the commit message explains why we're doing this, unless\nyou have comments on that explanation (the last paragraph of the commit\nmessage).\n"},{"id":"450304","messageId":"xmqqsfryah42.fsf@gitster.g","threadId":"57499","inReplyTo":"220303.86zgm72g3j.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 12/15] gettext tests: don't ignore \"test-tool regex\" exit code","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-03-03T20:58:05Z","receivedAt":"2022-03-03T20:58:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> On Wed, Mar 02 2022, Junio C Hamano wrote:\n>\n>> Ævar Arnfjörð Bjarmason writes:\n>>\n>>> -test_have_prereq GETTEXT_LOCALE &&\n>>> -test-tool regex \"HALLÓ\" \"Halló\" ICASE &&\n>>> -test_set_prereq REGEX_LOCALE\n>>> +test_expect_success GETTEXT_LOCALE 'setup REGEX_LOCALE prerequisite' '\n>>> +\tif test-tool regex \"HALLÓ\" \"Halló\" ICASE\n>>> +\tthen\n>>> +\t\ttest_set_prereq REGEX_LOCALE\n>>\n>> This looks sensible but\n>>\n>>> +\telse\n>>> +\t\ttest_must_fail test-tool regex \"HALLÓ\" \"Halló\" ICASE\n>>\n>> this side looks puzzling. I think this way to avoid counting abort\n>> etc as passing \"must fail\" test would be the least bad that we can\n>> do.\n>>\n>> Nicely done.\n>\n> Thanks. For the purposes of a re-roll I'll note this as a \"nothing to\n> change\", since the commit message explains why we're doing this, unless\n> you have comments on that explanation (the last paragraph of the commit\n> message).\n\nThe comment was mostly if it is more appropriate to explain the\npuzzling code with an in-code comment, rather than the log message.\n\nIn-code comment is for those who may find the current code strange.\nThe log message is for those who wonder why the current code came to\nbe in today's shape.\n"},{"id":"450349","messageId":"xmqqlexq4et6.fsf@gitster.g","threadId":"57499","inReplyTo":"220303.864k4f3uqe.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 00/15] tests: don't ignore \"git\" exit codes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-03-04T02:44:21Z","receivedAt":"2022-03-04T02:44:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Yes, it returns the correct status code, but that doesn't help with\n> (pseudo)code like:\n>\n> \tif (run_command(\"foo\")) /* exits with e.g. 123 */\n> \t\tdie(\"oh no, foo failed\"); /* exits with 128 */\n>\n> I should have said \"code using run-command.c does not do that...\",\n> sorry.\n\nYeah, but even if callers of run_command() can tell \"foo\"\nsegfaulted, I do not think it is sensible to exit as if we\nsegfaulted (or, we _could_ actually die by segfaulting ourselves,\nwhich is worse).\n"},{"id":"450357","messageId":"220304.86zgm6fbk3.gmgdl@evledraar.gmail.com","threadId":"57499","inReplyTo":"xmqqlexq4et6.fsf@gitster.g","subject":"Re: [PATCH 00/15] tests: don't ignore \"git\" exit codes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-04T06:57:43Z","receivedAt":"2022-03-04T06:59:15Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Mar 03 2022, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> Yes, it returns the correct status code, but that doesn't help with\n>> (pseudo)code like:\n>>\n>> \tif (run_command(\"foo\")) /* exits with e.g. 123 */\n>> \t\tdie(\"oh no, foo failed\"); /* exits with 128 */\n>>\n>> I should have said \"code using run-command.c does not do that...\",\n>> sorry.\n>\n> Yeah, but even if callers of run_command() can tell \"foo\"\n> segfaulted, I do not think it is sensible to exit as if we\n> segfaulted (or, we _could_ actually die by segfaulting ourselves,\n> which is worse).\n\nI should have made it clear that I'm thinking about this as a test-only\nmode, so hidden behind some GIT_TEST_* variable, i.e. as a means to an\nend in getting the test suite to spot the failure in the sub-process.\n\nIt's not the only way to do it, but it's the simplest and most reliable\ngiven how our tests are run.\n"},{"id":"450552","messageId":"patch-v2-01.15-78b9c52551f-20220307T124817Z-avarab@gmail.com","threadId":"57499","inReplyTo":"cover-v2-00.15-00000000000-20220307T124817Z-avarab@gmail.com","subject":"[PATCH v2 01/15] tests: change some 'test $(git) = \"x\"' to test_cmp","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-07T12:48:52Z","receivedAt":"2022-03-07T12:49:31Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change some of the patterns in the test suite where we were hiding the\nexit code from \"git\" by invoking it in a sub-shell within a \"test\"\nexpression to use temporary files and test_cmp instead.\n\nThese are not all the occurrences of this anti-pattern, but these in\nparticular hid issues where LSAN was dying, and I'd thus marked these\ntests as passing under the linux-leaks CI job in past commits with\n\"TEST_PASSES_SANITIZE_LEAK=true\". Let's deal with that by either\nremoving that marking, or skipping specific tests under\n!SANITIZE_LEAK.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t0002-gitfile.sh | 6 ++++--\n t/t2200-add-update.sh | 33 +++++++++++++++++++++------------\n t/t4128-apply-root.sh | 33 ++++++++++++++++++++-------------\n t/t7103-reset-bare.sh | 7 +++++--\n 4 files changed, 50 insertions(+), 29 deletions(-)\n\ndiff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh\nindex 76052cb5620..f6356db183b 100755\n--- a/t/t0002-gitfile.sh\n+++ b/t/t0002-gitfile.sh\n@@ -65,9 +65,11 @@ test_expect_success 'check commit-tree' '\n \ttest_path_is_file \"$REAL/objects/$(objpath $SHA)\"\n '\n \n-test_expect_success 'check rev-list' '\n+test_expect_success !SANITIZE_LEAK 'check rev-list' '\n \tgit update-ref \"HEAD\" \"$SHA\" &&\n-\ttest \"$SHA\" = \"$(git rev-list HEAD)\"\n+\tgit rev-list HEAD >actual &&\n+\techo $SHA >expected &&\n+\ttest_cmp expected actual\n '\n \n test_expect_success 'setup_git_dir twice in subdir' '\ndiff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh\nindex acd3650d3c0..0c38f8e3569 100755\n--- a/t/t2200-add-update.sh\n+++ b/t/t2200-add-update.sh\n@@ -14,7 +14,6 @@ only the updates to dir/sub.\n Also tested are \"git add -u\" without limiting, and \"git add -u\"\n without contents changes, and other conditions'\n \n-TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success setup '\n@@ -41,20 +40,28 @@ test_expect_success update '\n '\n \n test_expect_success 'update noticed a removal' '\n-\ttest \"$(git ls-files dir1/sub1)\" = \"\"\n+\tgit ls-files dir1/sub1 >out &&\n+\ttest_must_be_empty out\n '\n \n test_expect_success 'update touched correct path' '\n-\ttest \"$(git diff-files --name-status dir2/sub3)\" = \"\"\n+\tgit diff-files --name-status dir2/sub3 >out &&\n+\ttest_must_be_empty out\n '\n \n test_expect_success 'update did not touch other tracked files' '\n-\ttest \"$(git diff-files --name-status check)\" = \"M\tcheck\" &&\n-\ttest \"$(git diff-files --name-status top)\" = \"M\ttop\"\n+\techo \"M\tcheck\" >expect &&\n+\tgit diff-files --name-status check >actual &&\n+\ttest_cmp expect actual &&\n+\n+\techo \"M\ttop\" >expect &&\n+\tgit diff-files --name-status top >actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'update did not touch untracked files' '\n-\ttest \"$(git ls-files dir2/other)\" = \"\"\n+\tgit ls-files dir2/other >out &&\n+\ttest_must_be_empty out\n '\n \n test_expect_success 'cache tree has not been corrupted' '\n@@ -76,9 +83,8 @@ test_expect_success 'update from a subdirectory' '\n '\n \n test_expect_success 'change gets noticed' '\n-\n-\ttest \"$(git diff-files --name-status dir1)\" = \"\"\n-\n+\tgit diff-files --name-status dir1 >out &&\n+\ttest_must_be_empty out\n '\n \n test_expect_success 'non-qualified update in subdir updates from the root' '\n@@ -103,7 +109,8 @@ test_expect_success 'replace a file with a symlink' '\n test_expect_success 'add everything changed' '\n \n \tgit add -u &&\n-\ttest -z \"$(git diff-files)\"\n+\tgit diff-files >out &&\n+\ttest_must_be_empty out\n \n '\n \n@@ -111,7 +118,8 @@ test_expect_success 'touch and then add -u' '\n \n \ttouch check &&\n \tgit add -u &&\n-\ttest -z \"$(git diff-files)\"\n+\tgit diff-files >out &&\n+\ttest_must_be_empty out\n \n '\n \n@@ -119,7 +127,8 @@ test_expect_success 'touch and then add explicitly' '\n \n \ttouch check &&\n \tgit add check &&\n-\ttest -z \"$(git diff-files)\"\n+\tgit diff-files >out &&\n+\ttest_must_be_empty out\n \n '\n \ndiff --git a/t/t4128-apply-root.sh b/t/t4128-apply-root.sh\nindex cb3181e8b71..ba89a2f2d73 100755\n--- a/t/t4128-apply-root.sh\n+++ b/t/t4128-apply-root.sh\n@@ -2,8 +2,6 @@\n \n test_description='apply same filename'\n \n-\n-TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\n@@ -26,10 +24,11 @@ diff a/bla/blub/dir/file b/bla/blub/dir/file\n EOF\n \n test_expect_success 'apply --directory -p (1)' '\n-\n \tgit apply --directory=some/sub -p3 --index patch &&\n-\ttest Bello = $(git show :some/sub/dir/file) &&\n-\ttest Bello = $(cat some/sub/dir/file)\n+\techo Bello >expect &&\n+\tgit show :some/sub/dir/file >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_cmp expect some/sub/dir/file\n \n '\n \n@@ -37,8 +36,10 @@ test_expect_success 'apply --directory -p (2) ' '\n \n \tgit reset --hard initial &&\n \tgit apply --directory=some/sub/ -p3 --index patch &&\n-\ttest Bello = $(git show :some/sub/dir/file) &&\n-\ttest Bello = $(cat some/sub/dir/file)\n+\techo Bello >expect &&\n+\tgit show :some/sub/dir/file >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_cmp expect some/sub/dir/file\n \n '\n \n@@ -55,8 +56,10 @@ EOF\n test_expect_success 'apply --directory (new file)' '\n \tgit reset --hard initial &&\n \tgit apply --directory=some/sub/dir/ --index patch &&\n-\ttest content = $(git show :some/sub/dir/newfile) &&\n-\ttest content = $(cat some/sub/dir/newfile)\n+\techo content >expect &&\n+\tgit show :some/sub/dir/newfile >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_cmp expect some/sub/dir/newfile\n '\n \n cat > patch << EOF\n@@ -72,8 +75,10 @@ EOF\n test_expect_success 'apply --directory -p (new file)' '\n \tgit reset --hard initial &&\n \tgit apply -p2 --directory=some/sub/dir/ --index patch &&\n-\ttest content = $(git show :some/sub/dir/newfile2) &&\n-\ttest content = $(cat some/sub/dir/newfile2)\n+\techo content >expect &&\n+\tgit show :some/sub/dir/newfile2 >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_cmp expect some/sub/dir/newfile2\n '\n \n cat > patch << EOF\n@@ -107,8 +112,10 @@ EOF\n test_expect_success 'apply --directory (quoted filename)' '\n \tgit reset --hard initial &&\n \tgit apply --directory=some/sub/dir/ --index patch &&\n-\ttest content = $(git show :some/sub/dir/quotefile) &&\n-\ttest content = $(cat some/sub/dir/quotefile)\n+\techo content >expect &&\n+\tgit show :some/sub/dir/quotefile >actual &&\n+\ttest_cmp expect actual &&\n+\ttest_cmp expect some/sub/dir/quotefile\n '\n \n test_done\ndiff --git a/t/t7103-reset-bare.sh b/t/t7103-reset-bare.sh\nindex 0de83e36199..a60153f9f32 100755\n--- a/t/t7103-reset-bare.sh\n+++ b/t/t7103-reset-bare.sh\n@@ -63,9 +63,12 @@ test_expect_success '\"mixed\" reset is not allowed in bare' '\n \ttest_must_fail git reset --mixed HEAD^\n '\n \n-test_expect_success '\"soft\" reset is allowed in bare' '\n+test_expect_success !SANITIZE_LEAK '\"soft\" reset is allowed in bare' '\n \tgit reset --soft HEAD^ &&\n-\ttest \"$(git show --pretty=format:%s | head -n 1)\" = \"one\"\n+\tgit show --pretty=format:%s >out &&\n+\techo one >expect &&\n+\thead -n 1 out >actual &&\n+\ttest_cmp expect actual\n '\n \n test_done\n-- \n2.35.1.1242.gfeba0eae32b\n\n"},{"id":"450553","messageId":"cover-v2-00.15-00000000000-20220307T124817Z-avarab@gmail.com","threadId":"57499","inReplyTo":"cover-00.15-00000000000-20220302T171755Z-avarab@gmail.com","subject":"[PATCH v2 00/15] tests: don't ignore \"git\" exit codes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-07T12:48:51Z","receivedAt":"2022-03-07T12:49:33Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This series fixes issues where we ignored the exit code of \"git\" due\nto it being on the LHS of a pipe, or because we interpolated its\noutput with $() in a \"test\" construct, or had missing &&- chains in\nhelper functions etc.\n\nAs noted in v1\n(https://lore.kernel.org/git/cover-00.15-00000000000-20220302T171755Z-avarab@gmail.com/)\nthis is not a general search/replacement, but fixes real issues we run\ninto with SANITIZE=leak.\n\nChanges since v1:\n\n * Minor commit message improvement for 11/15\n * Added a comment for some non-obvious prereq setup code, as\n suggested by Junio.\n\nÆvar Arnfjörð Bjarmason (15):\n tests: change some 'test $(git) = \"x\"' to test_cmp\n tests: use \"test_stdout_line_count\", not \"test $(git [...] | wc -l)\"\n read-tree tests: check \"diff-files\" exit code on failure\n diff tests: don't ignore \"git diff\" exit code\n diff tests: don't ignore \"git diff\" exit code in \"read\" loop\n apply tests: use \"test_must_fail\" instead of ad-hoc pattern\n merge tests: use \"test_must_fail\" instead of ad-hoc pattern\n rev-parse tests: don't ignore \"git reflog\" exit code\n notes tests: don't ignore \"git\" exit code\n diff tests: don't ignore \"git rev-list\" exit code\n rev-list tests: don't hide abort() in \"test_expect_failure\"\n gettext tests: don't ignore \"test-tool regex\" exit code\n apply tests: don't ignore \"git ls-files\" exit code, drop sub-shell\n checkout tests: don't ignore \"git \" exit code\n rev-list simplify tests: don't ignore \"git\" exit code\n\n t/t0002-gitfile.sh | 6 +-\n t/t1001-read-tree-m-2way.sh | 6 +-\n t/t1002-read-tree-m-u-2way.sh | 6 +-\n t/t1503-rev-parse-verify.sh | 5 +-\n t/t2012-checkout-last.sh | 51 ++++++---\n t/t2200-add-update.sh | 33 ++++--\n t/t3302-notes-index-expensive.sh | 6 +-\n t/t3303-notes-subtrees.sh | 9 +-\n t/t3305-notes-fanout.sh | 14 +--\n t/t4020-diff-external.sh | 153 ++++++++++++-------------\n t/t4027-diff-submodule.sh | 7 +-\n t/t4123-apply-shrink.sh | 18 +--\n t/t4128-apply-root.sh | 36 +++---\n t/t6005-rev-list-count.sh | 43 ++++---\n t/t6012-rev-list-simplify.sh | 12 +-\n t/t6102-rev-list-unexpected-objects.sh | 13 ++-\n t/t6407-merge-binary.sh | 22 +---\n t/t7103-reset-bare.sh | 7 +-\n t/t7812-grep-icase-non-ascii.sh | 16 ++-\n 19 files changed, 245 insertions(+), 218 deletions(-)\n\nRange-diff against v1:\n 1: 78b9c52551f = 1: 78b9c52551f tests: change some 'test $(git) = \"x\"' to test_cmp\n 2: e1105b029d6 = 2: e1105b029d6 tests: use \"test_stdout_line_count\", not \"test $(git [...] | wc -l)\"\n 3: 5f02e30d1ab = 3: 5f02e30d1ab read-tree tests: check \"diff-files\" exit code on failure\n 4: a425ced5609 = 4: a425ced5609 diff tests: don't ignore \"git diff\" exit code\n 5: b1aeac3f68e = 5: b1aeac3f68e diff tests: don't ignore \"git diff\" exit code in \"read\" loop\n 6: 7952ae1f3b5 = 6: 7952ae1f3b5 apply tests: use \"test_must_fail\" instead of ad-hoc pattern\n 7: 276be19e35e = 7: 276be19e35e merge tests: use \"test_must_fail\" instead of ad-hoc pattern\n 8: dca2ac3a171 = 8: dca2ac3a171 rev-parse tests: don't ignore \"git reflog\" exit code\n 9: ca9e12f2bac = 9: ca9e12f2bac notes tests: don't ignore \"git\" exit code\n10: 946397033d4 = 10: 946397033d4 diff tests: don't ignore \"git rev-list\" exit code\n11: 26c838f0560 ! 11: 52397b3575a rev-list tests: don't hide abort() in \"test_expect_failure\"\n @@ Commit message\n \"test_expect_success\", but would allow us say \"test_todo\" here (and\n \"success\" would emit a \"not ok [...] # TODO\", not \"ok [...]\".\n \n - In lieu of those larger changes let's more narrowly fix the problem at\n + So even though using \"test_expect_success\" here comes with its own\n + problems[2], let's use it as a narrow change to fix the problem at\n hand here and stop conflating the current \"success\" with actual\n SANITIZE=leak failures.\n \n 1. https://lore.kernel.org/git/87tuhmk19c.fsf@evledraar.gmail.com/\n + 2. https://lore.kernel.org/git/xmqq4k9kj15p.fsf@gitster.g/\n \n Signed-off-by: Ævar Arnfjörð Bjarmason \n \n12: f3cc5bc7eb9 ! 12: 552dcac705d gettext tests: don't ignore \"test-tool regex\" exit code\n @@ t/t7812-grep-icase-non-ascii.sh: test_expect_success GETTEXT_LOCALE 'setup' '\n -test-tool regex \"HALLÓ\" \"Halló\" ICASE &&\n -test_set_prereq REGEX_LOCALE\n +test_expect_success GETTEXT_LOCALE 'setup REGEX_LOCALE prerequisite' '\n ++\t# This \"test-tool\" invocation is identical...\n +\tif test-tool regex \"HALLÓ\" \"Halló\" ICASE\n +\tthen\n +\t\ttest_set_prereq REGEX_LOCALE\n +\telse\n ++\n ++\t\t# ... to this one, but this way \"test_must_fail\" will\n ++\t\t# tell a segfault or abort() from the regexec() test\n ++\t\t# itself\n +\t\ttest_must_fail test-tool regex \"HALLÓ\" \"Halló\" ICASE\n +\tfi\n +'\n13: 834809b1b8a = 13: dbe8d168401 apply tests: don't ignore \"git ls-files\" exit code, drop sub-shell\n14: 34cada14fec = 14: 22b81d7f93a checkout tests: don't ignore \"git \" exit code\n15: 4ee216711cf = 15: 16889ed154f rev-list simplify tests: don't ignore \"git\" exit code\n-- \n2.35.1.1242.gfeba0eae32b\n\n"},{"id":"450554","messageId":"patch-v2-02.15-e1105b029d6-20220307T124817Z-avarab@gmail.com","threadId":"57499","inReplyTo":"cover-v2-00.15-00000000000-20220307T124817Z-avarab@gmail.com","subject":"[PATCH v2 02/15] tests: use \"test_stdout_line_count\", not \"test $(git [...] | wc -l)\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-07T12:48:53Z","receivedAt":"2022-03-07T12:49:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Use the test_stdout_line_count helper added in\ncdff1bb5a3d (test-lib-functions: introduce test_stdout_line_count,\n2021-07-04) so that we'll spot if git itself dies, segfaults etc in\nthese expressions.\n\nBecause we didn't distinguish these failure conditions before I'd\nmistakenly marked these tests as passing under SANITIZE=leak in\ndd9cede9136 (leak tests: mark some rev-list tests as passing with\nSANITIZE=leak, 2021-10-31).\n\nWhile we're at it let's re-indent these lines to match our usual\nstyle, as we're having to change all of them anyway.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t6005-rev-list-count.sh | 43 +++++++++++++++++++--------------------\n 1 file changed, 21 insertions(+), 22 deletions(-)\n\ndiff --git a/t/t6005-rev-list-count.sh b/t/t6005-rev-list-count.sh\nindex 86542c650e2..e960049f647 100755\n--- a/t/t6005-rev-list-count.sh\n+++ b/t/t6005-rev-list-count.sh\n@@ -2,7 +2,6 @@\n \n test_description='git rev-list --max-count and --skip test'\n \n-TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\n@@ -14,39 +13,39 @@ test_expect_success 'setup' '\n '\n \n test_expect_success 'no options' '\n- test $(git rev-list HEAD | wc -l) = 5\n+\ttest_stdout_line_count = 5 git rev-list HEAD\n '\n \n test_expect_success '--max-count' '\n- test $(git rev-list HEAD --max-count=0 | wc -l) = 0 &&\n- test $(git rev-list HEAD --max-count=3 | wc -l) = 3 &&\n- test $(git rev-list HEAD --max-count=5 | wc -l) = 5 &&\n- test $(git rev-list HEAD --max-count=10 | wc -l) = 5\n+\ttest_stdout_line_count = 0 git rev-list HEAD --max-count=0 &&\n+\ttest_stdout_line_count = 3 git rev-list HEAD --max-count=3 &&\n+\ttest_stdout_line_count = 5 git rev-list HEAD --max-count=5 &&\n+\ttest_stdout_line_count = 5 git rev-list HEAD --max-count=10\n '\n \n test_expect_success '--max-count all forms' '\n- test $(git rev-list HEAD --max-count=1 | wc -l) = 1 &&\n- test $(git rev-list HEAD -1 | wc -l) = 1 &&\n- test $(git rev-list HEAD -n1 | wc -l) = 1 &&\n- test $(git rev-list HEAD -n 1 | wc -l) = 1\n+\ttest_stdout_line_count = 1 git rev-list HEAD --max-count=1 &&\n+\ttest_stdout_line_count = 1 git rev-list HEAD -1 &&\n+\ttest_stdout_line_count = 1 git rev-list HEAD -n1 &&\n+\ttest_stdout_line_count = 1 git rev-list HEAD -n 1\n '\n \n test_expect_success '--skip' '\n- test $(git rev-list HEAD --skip=0 | wc -l) = 5 &&\n- test $(git rev-list HEAD --skip=3 | wc -l) = 2 &&\n- test $(git rev-list HEAD --skip=5 | wc -l) = 0 &&\n- test $(git rev-list HEAD --skip=10 | wc -l) = 0\n+\ttest_stdout_line_count = 5 git rev-list HEAD --skip=0 &&\n+\ttest_stdout_line_count = 2 git rev-list HEAD --skip=3 &&\n+\ttest_stdout_line_count = 0 git rev-list HEAD --skip=5 &&\n+\ttest_stdout_line_count = 0 git rev-list HEAD --skip=10\n '\n \n test_expect_success '--skip --max-count' '\n- test $(git rev-list HEAD --skip=0 --max-count=0 | wc -l) = 0 &&\n- test $(git rev-list HEAD --skip=0 --max-count=10 | wc -l) = 5 &&\n- test $(git rev-list HEAD --skip=3 --max-count=0 | wc -l) = 0 &&\n- test $(git rev-list HEAD --skip=3 --max-count=1 | wc -l) = 1 &&\n- test $(git rev-list HEAD --skip=3 --max-count=2 | wc -l) = 2 &&\n- test $(git rev-list HEAD --skip=3 --max-count=10 | wc -l) = 2 &&\n- test $(git rev-list HEAD --skip=5 --max-count=10 | wc -l) = 0 &&\n- test $(git rev-list HEAD --skip=10 --max-count=10 | wc -l) = 0\n+\ttest_stdout_line_count = 0 git rev-list HEAD --skip=0 --max-count=0 &&\n+\ttest_stdout_line_count = 5 git rev-list HEAD --skip=0 --max-count=10 &&\n+\ttest_stdout_line_count = 0 git rev-list HEAD --skip=3 --max-count=0 &&\n+\ttest_stdout_line_count = 1 git rev-list HEAD --skip=3 --max-count=1 &&\n+\ttest_stdout_line_count = 2 git rev-list HEAD --skip=3 --max-count=2 &&\n+\ttest_stdout_line_count = 2 git rev-list HEAD --skip=3 --max-count=10 &&\n+\ttest_stdout_line_count = 0 git rev-list HEAD --skip=5 --max-count=10 &&\n+\ttest_stdout_line_count = 0 git rev-list HEAD --skip=10 --max-count=10\n '\n \n test_done\n-- \n2.35.1.1242.gfeba0eae32b\n\n"},{"id":"450555","messageId":"patch-v2-03.15-5f02e30d1ab-20220307T124817Z-avarab@gmail.com","threadId":"57499","inReplyTo":"cover-v2-00.15-00000000000-20220307T124817Z-avarab@gmail.com","subject":"[PATCH v2 03/15] read-tree tests: check \"diff-files\" exit code on failure","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-07T12:48:54Z","receivedAt":"2022-03-07T12:49:36Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix an issue with the exit code of \"diff-files\" being ignored, which\nhas been ignored ever since these tests were originally added in\nc859600954d ([PATCH] read-tree: save more user hassles during\nfast-forward., 2005-06-07).\n\nSince the exit code was ignored we'd hide errors here under\nSANITIZE=leak, which resulted in me mistakenly marking these tests as\npassing under SANITIZE=leak in e5a917fcf42 (unpack-trees: don't leak\nmemory in verify_clean_subdirectory(), 2021-10-07) and\n4ea08416b8e (leak tests: mark a read-tree test as passing\nSANITIZE=leak, 2021-10-31).\n\nAs it would be non-trivial to fix these tests (the leak is in\nrevision.c) let's un-mark them as passing under SANITIZE=leak in\naddition to fixing the issue of ignoring the exit code.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t1001-read-tree-m-2way.sh | 6 +++---\n t/t1002-read-tree-m-u-2way.sh | 6 +++---\n 2 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t1001-read-tree-m-2way.sh b/t/t1001-read-tree-m-2way.sh\nindex d1115528cb9..0710b1fb1e9 100755\n--- a/t/t1001-read-tree-m-2way.sh\n+++ b/t/t1001-read-tree-m-2way.sh\n@@ -21,7 +21,6 @@ In the test, these paths are used:\n \tyomin - not in H or M\n '\n \n-TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-read-tree.sh\n \n@@ -38,11 +37,12 @@ compare_change () {\n }\n \n check_cache_at () {\n-\tclean_if_empty=$(git diff-files -- \"$1\")\n+\tgit diff-files -- \"$1\" >out &&\n+\tclean_if_empty=$(cat out) &&\n \tcase \"$clean_if_empty\" in\n \t'') echo \"$1: clean\" ;;\n \t?*) echo \"$1: dirty\" ;;\n-\tesac\n+\tesac &&\n \tcase \"$2,$clean_if_empty\" in\n \tclean,)\t\t: ;;\n \tclean,?*)\tfalse ;;\ndiff --git a/t/t1002-read-tree-m-u-2way.sh b/t/t1002-read-tree-m-u-2way.sh\nindex ca5c5510c73..46cbd5514a6 100755\n--- a/t/t1002-read-tree-m-u-2way.sh\n+++ b/t/t1002-read-tree-m-u-2way.sh\n@@ -9,7 +9,6 @@ This is identical to t1001, but uses -u to update the work tree as well.\n \n '\n \n-TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-read-tree.sh\n \n@@ -23,11 +22,12 @@ compare_change () {\n }\n \n check_cache_at () {\n-\tclean_if_empty=$(git diff-files -- \"$1\")\n+\tgit diff-files -- \"$1\" >out &&\n+\tclean_if_empty=$(cat out) &&\n \tcase \"$clean_if_empty\" in\n \t'') echo \"$1: clean\" ;;\n \t?*) echo \"$1: dirty\" ;;\n-\tesac\n+\tesac &&\n \tcase \"$2,$clean_if_empty\" in\n \tclean,)\t\t: ;;\n \tclean,?*)\tfalse ;;\n-- \n2.35.1.1242.gfeba0eae32b\n\n"},{"id":"450556","messageId":"patch-v2-04.15-a425ced5609-20220307T124817Z-avarab@gmail.com","threadId":"57499","inReplyTo":"cover-v2-00.15-00000000000-20220307T124817Z-avarab@gmail.com","subject":"[PATCH v2 04/15] diff tests: don't ignore \"git diff\" exit code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-07T12:48:55Z","receivedAt":"2022-03-07T12:49:37Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a test pattern that originated in f1af60bdba4 (Support 'diff=pgm'\nattribute, 2007-04-22) so that we'll stop using \"git diff\" on the\nleft-hand-side of a pipe, and thus ignoring its exit code.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t4020-diff-external.sh | 49 ++++++++++++++++++----------------------\n 1 file changed, 22 insertions(+), 27 deletions(-)\n\ndiff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh\nindex 54bb8ef27e7..879ee04d291 100755\n--- a/t/t4020-diff-external.sh\n+++ b/t/t4020-diff-external.sh\n@@ -37,17 +37,15 @@ test_expect_success 'GIT_EXTERNAL_DIFF environment' '\n \n '\n \n-test_expect_success 'GIT_EXTERNAL_DIFF environment should apply only to diff' '\n-\n-\tGIT_EXTERNAL_DIFF=echo git log -p -1 HEAD |\n-\tgrep \"^diff --git a/file b/file\"\n+test_expect_success !SANITIZE_LEAK 'GIT_EXTERNAL_DIFF environment should apply only to diff' '\n+\tGIT_EXTERNAL_DIFF=echo git log -p -1 HEAD >out &&\n+\tgrep \"^diff --git a/file b/file\" out\n \n '\n \n test_expect_success 'GIT_EXTERNAL_DIFF environment and --no-ext-diff' '\n-\n-\tGIT_EXTERNAL_DIFF=echo git diff --no-ext-diff |\n-\tgrep \"^diff --git a/file b/file\"\n+\tGIT_EXTERNAL_DIFF=echo git diff --no-ext-diff >out &&\n+\tgrep \"^diff --git a/file b/file\" out\n \n '\n \n@@ -83,16 +81,16 @@ test_expect_success 'diff.external' '\n \t}\n '\n \n-test_expect_success 'diff.external should apply only to diff' '\n+test_expect_success !SANITIZE_LEAK 'diff.external should apply only to diff' '\n \ttest_config diff.external echo &&\n-\tgit log -p -1 HEAD |\n-\tgrep \"^diff --git a/file b/file\"\n+\tgit log -p -1 HEAD >out &&\n+\tgrep \"^diff --git a/file b/file\" out\n '\n \n test_expect_success 'diff.external and --no-ext-diff' '\n \ttest_config diff.external echo &&\n-\tgit diff --no-ext-diff |\n-\tgrep \"^diff --git a/file b/file\"\n+\tgit diff --no-ext-diff >out &&\n+\tgrep \"^diff --git a/file b/file\" out\n '\n \n test_expect_success 'diff attribute' '\n@@ -115,17 +113,15 @@ test_expect_success 'diff attribute' '\n \n '\n \n-test_expect_success 'diff attribute should apply only to diff' '\n-\n-\tgit log -p -1 HEAD |\n-\tgrep \"^diff --git a/file b/file\"\n+test_expect_success !SANITIZE_LEAK 'diff attribute should apply only to diff' '\n+\tgit log -p -1 HEAD >out &&\n+\tgrep \"^diff --git a/file b/file\" out\n \n '\n \n test_expect_success 'diff attribute and --no-ext-diff' '\n-\n-\tgit diff --no-ext-diff |\n-\tgrep \"^diff --git a/file b/file\"\n+\tgit diff --no-ext-diff >out &&\n+\tgrep \"^diff --git a/file b/file\" out\n \n '\n \n@@ -148,17 +144,15 @@ test_expect_success 'diff attribute' '\n \n '\n \n-test_expect_success 'diff attribute should apply only to diff' '\n-\n-\tgit log -p -1 HEAD |\n-\tgrep \"^diff --git a/file b/file\"\n+test_expect_success !SANITIZE_LEAK 'diff attribute should apply only to diff' '\n+\tgit log -p -1 HEAD >out &&\n+\tgrep \"^diff --git a/file b/file\" out\n \n '\n \n test_expect_success 'diff attribute and --no-ext-diff' '\n-\n-\tgit diff --no-ext-diff |\n-\tgrep \"^diff --git a/file b/file\"\n+\tgit diff --no-ext-diff >out &&\n+\tgrep \"^diff --git a/file b/file\" out\n \n '\n \n@@ -177,7 +171,8 @@ test_expect_success 'attributes trump GIT_EXTERNAL_DIFF and diff.external' '\n \n test_expect_success 'no diff with -diff' '\n \techo >.gitattributes \"file -diff\" &&\n-\tgit diff | grep Binary\n+\tgit diff >out &&\n+\tgrep Binary out\n '\n \n echo NULZbetweenZwords | perl -pe 'y/Z/\\000/' > file\n-- \n2.35.1.1242.gfeba0eae32b\n\n"},{"id":"450557","messageId":"patch-v2-05.15-b1aeac3f68e-20220307T124817Z-avarab@gmail.com","threadId":"57499","inReplyTo":"cover-v2-00.15-00000000000-20220307T124817Z-avarab@gmail.com","subject":"[PATCH v2 05/15] diff tests: don't ignore \"git diff\" exit code in \"read\" loop","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-07T12:48:56Z","receivedAt":"2022-03-07T12:49:37Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a test pattern that originated in f1af60bdba4 (Support 'diff=pgm'\nattribute, 2007-04-22) so that we'll stop using \"git diff\" on the\nleft-hand-side of a pipe, and thus ignoring its exit code.\n\nRather than use intermediate files let's rewrite these tests to a much\nsimpler but more exhaustive \"test_tmp\" where we'll ignore certain\nfields in the output.\n\nNote that this is not a faithful conversion of the previous\n\"read/test\" in some cases, as we were ignoring more fields there than\nwe strictly needed to. Now we'll \"test_cmp\" everything we can, and\nonly ignore the likes of paths to $TEMPDIR etc.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t4020-diff-external.sh | 104 ++++++++++++++++++++-------------------\n 1 file changed, 53 insertions(+), 51 deletions(-)\n\ndiff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh\nindex 879ee04d291..1219f8bd4c0 100755\n--- a/t/t4020-diff-external.sh\n+++ b/t/t4020-diff-external.sh\n@@ -24,16 +24,12 @@ test_expect_success setup '\n '\n \n test_expect_success 'GIT_EXTERNAL_DIFF environment' '\n-\n-\tGIT_EXTERNAL_DIFF=echo git diff | {\n-\t\tread path oldfile oldhex oldmode newfile newhex newmode &&\n-\t\ttest \"z$path\" = zfile &&\n-\t\ttest \"z$oldmode\" = z100644 &&\n-\t\ttest \"z$newhex\" = \"z$ZERO_OID\" &&\n-\t\ttest \"z$newmode\" = z100644 &&\n-\t\toh=$(git rev-parse --verify HEAD:file) &&\n-\t\ttest \"z$oh\" = \"z$oldhex\"\n-\t}\n+\tcat >expect <<-EOF &&\n+\tfile $(git rev-parse --verify HEAD:file) 100644 file $(test_oid zero) 100644\n+\tEOF\n+\tGIT_EXTERNAL_DIFF=echo git diff >out &&\n+\tcut -d\" \" -f1,3- actual &&\n+\ttest_cmp expect actual\n \n '\n \n@@ -52,15 +48,14 @@ test_expect_success 'GIT_EXTERNAL_DIFF environment and --no-ext-diff' '\n test_expect_success SYMLINKS 'typechange diff' '\n \trm -f file &&\n \tln -s elif file &&\n-\tGIT_EXTERNAL_DIFF=echo git diff | {\n-\t\tread path oldfile oldhex oldmode newfile newhex newmode &&\n-\t\ttest \"z$path\" = zfile &&\n-\t\ttest \"z$oldmode\" = z100644 &&\n-\t\ttest \"z$newhex\" = \"z$ZERO_OID\" &&\n-\t\ttest \"z$newmode\" = z120000 &&\n-\t\toh=$(git rev-parse --verify HEAD:file) &&\n-\t\ttest \"z$oh\" = \"z$oldhex\"\n-\t} &&\n+\n+\tcat >expect <<-EOF &&\n+\tfile $(git rev-parse --verify HEAD:file) 100644 $(test_oid zero) 120000\n+\tEOF\n+\tGIT_EXTERNAL_DIFF=echo git diff >out &&\n+\tcut -d\" \" -f1,3-4,6- actual &&\n+\ttest_cmp expect actual &&\n+\n \tGIT_EXTERNAL_DIFF=echo git diff --no-ext-diff >actual &&\n \tgit diff >expect &&\n \ttest_cmp expect actual\n@@ -70,15 +65,13 @@ test_expect_success 'diff.external' '\n \tgit reset --hard &&\n \techo third >file &&\n \ttest_config diff.external echo &&\n-\tgit diff | {\n-\t\tread path oldfile oldhex oldmode newfile newhex newmode &&\n-\t\ttest \"z$path\" = zfile &&\n-\t\ttest \"z$oldmode\" = z100644 &&\n-\t\ttest \"z$newhex\" = \"z$ZERO_OID\" &&\n-\t\ttest \"z$newmode\" = z100644 &&\n-\t\toh=$(git rev-parse --verify HEAD:file) &&\n-\t\ttest \"z$oh\" = \"z$oldhex\"\n-\t}\n+\n+\tcat >expect <<-EOF &&\n+\tfile $(git rev-parse --verify HEAD:file) 100644 $(test_oid zero) 100644\n+\tEOF\n+\tgit diff >out &&\n+\tcut -d\" \" -f1,3-4,6- actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success !SANITIZE_LEAK 'diff.external should apply only to diff' '\n@@ -101,16 +94,12 @@ test_expect_success 'diff attribute' '\n \n \techo >.gitattributes \"file diff=parrot\" &&\n \n-\tgit diff | {\n-\t\tread path oldfile oldhex oldmode newfile newhex newmode &&\n-\t\ttest \"z$path\" = zfile &&\n-\t\ttest \"z$oldmode\" = z100644 &&\n-\t\ttest \"z$newhex\" = \"z$ZERO_OID\" &&\n-\t\ttest \"z$newmode\" = z100644 &&\n-\t\toh=$(git rev-parse --verify HEAD:file) &&\n-\t\ttest \"z$oh\" = \"z$oldhex\"\n-\t}\n-\n+\tcat >expect <<-EOF &&\n+\tfile $(git rev-parse --verify HEAD:file) 100644 $(test_oid zero) 100644\n+\tEOF\n+\tgit diff >out &&\n+\tcut -d\" \" -f1,3-4,6- actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success !SANITIZE_LEAK 'diff attribute should apply only to diff' '\n@@ -132,16 +121,12 @@ test_expect_success 'diff attribute' '\n \n \techo >.gitattributes \"file diff=color\" &&\n \n-\tgit diff | {\n-\t\tread path oldfile oldhex oldmode newfile newhex newmode &&\n-\t\ttest \"z$path\" = zfile &&\n-\t\ttest \"z$oldmode\" = z100644 &&\n-\t\ttest \"z$newhex\" = \"z$ZERO_OID\" &&\n-\t\ttest \"z$newmode\" = z100644 &&\n-\t\toh=$(git rev-parse --verify HEAD:file) &&\n-\t\ttest \"z$oh\" = \"z$oldhex\"\n-\t}\n-\n+\tcat >expect <<-EOF &&\n+\tfile $(git rev-parse --verify HEAD:file) 100644 $(test_oid zero) 100644\n+\tEOF\n+\tgit diff >out &&\n+\tcut -d\" \" -f1,3-4,6- actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success !SANITIZE_LEAK 'diff attribute should apply only to diff' '\n@@ -159,14 +144,26 @@ test_expect_success 'diff attribute and --no-ext-diff' '\n test_expect_success 'GIT_EXTERNAL_DIFF trumps diff.external' '\n \t>.gitattributes &&\n \ttest_config diff.external \"echo ext-global\" &&\n-\tGIT_EXTERNAL_DIFF=\"echo ext-env\" git diff | grep ext-env\n+\n+\tcat >expect <<-EOF &&\n+\text-env file $(git rev-parse --verify HEAD:file) 100644 file $(test_oid zero) 100644\n+\tEOF\n+\tGIT_EXTERNAL_DIFF=\"echo ext-env\" git diff >out &&\n+\tcut -d\" \" -f1-2,4- actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'attributes trump GIT_EXTERNAL_DIFF and diff.external' '\n \ttest_config diff.foo.command \"echo ext-attribute\" &&\n \ttest_config diff.external \"echo ext-global\" &&\n \techo \"file diff=foo\" >.gitattributes &&\n-\tGIT_EXTERNAL_DIFF=\"echo ext-env\" git diff | grep ext-attribute\n+\n+\tcat >expect <<-EOF &&\n+\text-attribute file $(git rev-parse --verify HEAD:file) 100644 file $(test_oid zero) 100644\n+\tEOF\n+\tGIT_EXTERNAL_DIFF=\"echo ext-env\" git diff >out &&\n+\tcut -d\" \" -f1-2,4- actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'no diff with -diff' '\n@@ -212,7 +209,12 @@ test_expect_success 'GIT_EXTERNAL_DIFF generates pretty paths' '\n \ttouch file.ext &&\n \tgit add file.ext &&\n \techo with extension > file.ext &&\n-\tGIT_EXTERNAL_DIFF=echo git diff file.ext | grep ......_file\\.ext &&\n+\n+\tcat >expect <<-EOF &&\n+\tfile.ext file $(git rev-parse --verify HEAD:file) 100644 file.ext $(test_oid zero) 100644\n+\tEOF\n+\tGIT_EXTERNAL_DIFF=echo git diff file.ext >out &&\n+\tcut -d\" \" -f1,3- actual &&\n \tgit update-index --force-remove file.ext &&\n \trm file.ext\n '\n-- \n2.35.1.1242.gfeba0eae32b\n\n"},{"id":"450558","messageId":"patch-v2-06.15-7952ae1f3b5-20220307T124817Z-avarab@gmail.com","threadId":"57499","inReplyTo":"cover-v2-00.15-00000000000-20220307T124817Z-avarab@gmail.com","subject":"[PATCH v2 06/15] apply tests: use \"test_must_fail\" instead of ad-hoc pattern","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-07T12:48:57Z","receivedAt":"2022-03-07T12:49:51Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change a fragile test pattern introduced in 6b763c424e4 (git-apply: do\nnot read past the end of buffer, 2007-09-05). Before this we wouldn't\ndistinguish normal \"git apply\" failures from segfaults or abort().\n\nI'd previously marked this test as passing under SANITIZE=leak in\nf54f48fc074 (leak tests: mark some apply tests as passing with\nSANITIZE=leak, 2021-10-31). Let's remove that annotation as this test\nwill no longer pass.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t4123-apply-shrink.sh | 18 ++----------------\n 1 file changed, 2 insertions(+), 16 deletions(-)\n\ndiff --git a/t/t4123-apply-shrink.sh b/t/t4123-apply-shrink.sh\nindex dfa053ff28e..3ef84619f53 100755\n--- a/t/t4123-apply-shrink.sh\n+++ b/t/t4123-apply-shrink.sh\n@@ -2,8 +2,6 @@\n \n test_description='apply a patch that is larger than the preimage'\n \n-\n-TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n cat >F <<\\EOF\n@@ -41,20 +39,8 @@ test_expect_success setup '\n '\n \n test_expect_success 'apply should fail gracefully' '\n-\n-\tif git apply --index patch\n-\tthen\n-\t\techo Oops, should not have succeeded\n-\t\tfalse\n-\telse\n-\t\tstatus=$? &&\n-\t\techo \"Status was $status\" &&\n-\t\tif test -f .git/index.lock\n-\t\tthen\n-\t\t\techo Oops, should not have crashed\n-\t\t\tfalse\n-\t\tfi\n-\tfi\n+\ttest_must_fail git apply --index patch &&\n+\ttest_path_is_missing .git/index.lock\n '\n \n test_done\n-- \n2.35.1.1242.gfeba0eae32b\n\n"},{"id":"450559","messageId":"patch-v2-07.15-276be19e35e-20220307T124817Z-avarab@gmail.com","threadId":"57499","inReplyTo":"cover-v2-00.15-00000000000-20220307T124817Z-avarab@gmail.com","subject":"[PATCH v2 07/15] merge tests: use \"test_must_fail\" instead of ad-hoc pattern","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-07T12:48:58Z","receivedAt":"2022-03-07T12:49:53Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"As in the preceding commit change a similar fragile test pattern\nintroduced in b798671fa93 (merge-recursive: do not rudely die on\nbinary merge, 2007-08-14) to use a \"test_must_fail\" instead.\n\nBefore this we wouldn't distinguish normal \"git merge\" failures from\nsegfaults or abort(). Unlike the preceding commit we didn't end up\nhiding any SANITIZE=leak failures in this case, but let's\ncorrespondingly change these anyway.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t6407-merge-binary.sh | 22 ++++++----------------\n 1 file changed, 6 insertions(+), 16 deletions(-)\n\ndiff --git a/t/t6407-merge-binary.sh b/t/t6407-merge-binary.sh\nindex 8e6241f92e6..0753fc95f45 100755\n--- a/t/t6407-merge-binary.sh\n+++ b/t/t6407-merge-binary.sh\n@@ -43,14 +43,9 @@ test_expect_success resolve '\n \trm -f a* m* &&\n \tgit reset --hard anchor &&\n \n-\tif git merge -s resolve main\n-\tthen\n-\t\techo Oops, should not have succeeded\n-\t\tfalse\n-\telse\n-\t\tgit ls-files -s >current &&\n-\t\ttest_cmp expect current\n-\tfi\n+\ttest_must_fail git merge -s resolve main &&\n+\tgit ls-files -s >current &&\n+\ttest_cmp expect current\n '\n \n test_expect_success recursive '\n@@ -58,14 +53,9 @@ test_expect_success recursive '\n \trm -f a* m* &&\n \tgit reset --hard anchor &&\n \n-\tif git merge -s recursive main\n-\tthen\n-\t\techo Oops, should not have succeeded\n-\t\tfalse\n-\telse\n-\t\tgit ls-files -s >current &&\n-\t\ttest_cmp expect current\n-\tfi\n+\ttest_must_fail git merge -s recursive main &&\n+\tgit ls-files -s >current &&\n+\ttest_cmp expect current\n '\n \n test_done\n-- \n2.35.1.1242.gfeba0eae32b\n\n"},{"id":"450560","messageId":"patch-v2-08.15-dca2ac3a171-20220307T124817Z-avarab@gmail.com","threadId":"57499","inReplyTo":"cover-v2-00.15-00000000000-20220307T124817Z-avarab@gmail.com","subject":"[PATCH v2 08/15] rev-parse tests: don't ignore \"git reflog\" exit code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-07T12:48:59Z","receivedAt":"2022-03-07T12:49:56Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Amend a test added in 9c46c054ae4 (rev-parse: tests git rev-parse\n--verify master@{n}, for various n, 2010-08-24) so that we'll stop\nignoring the exit code of \"git reflog\" by having it on the\nleft-hand-side of a pipe.\n\nBecause of this I'd marked this test as passing under SANITIZE=leak in\nf442c94638d (leak tests: mark some rev-parse tests as passing with\nSANITIZE=leak, 2021-10-31). As all of it except this specific test\nwill now pass, let's skip it under the !SANITIZE_LEAK prerequisite.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t1503-rev-parse-verify.sh | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh\nindex 94fe413ee37..ba43168d123 100755\n--- a/t/t1503-rev-parse-verify.sh\n+++ b/t/t1503-rev-parse-verify.sh\n@@ -132,8 +132,9 @@ test_expect_success 'use --default' '\n \ttest_must_fail git rev-parse --verify --default bar\n '\n \n-test_expect_success 'main@{n} for various n' '\n-\tN=$(git reflog | wc -l) &&\n+test_expect_success !SANITIZE_LEAK 'main@{n} for various n' '\n+\tgit reflog >out &&\n+\tN=$(wc -l output &&\n+\tgit log >tmp &&\n+\tgrep \"^ \" tmp >output &&\n \ti=$count &&\n \twhile test $i -gt 0\n \tdo\n@@ -90,7 +90,7 @@ write_script time_notes <<\\EOF\n \t\t\tunset GIT_NOTES_REF\n \t\t\t;;\n \t\tesac\n-\t\tgit log\n+\t\tgit log || exit $?\n \t\ti=$(($i+1))\n \tdone >/dev/null\n EOF\ndiff --git a/t/t3303-notes-subtrees.sh b/t/t3303-notes-subtrees.sh\nindex 7e0a8960af8..eac193757bf 100755\n--- a/t/t3303-notes-subtrees.sh\n+++ b/t/t3303-notes-subtrees.sh\n@@ -5,7 +5,6 @@ test_description='Test commit notes organized in subtrees'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n-TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n number_of_commits=100\n@@ -79,7 +78,7 @@ test_sha1_based () {\n \t(\n \t\tstart_note_commit &&\n \t\tnr=$number_of_commits &&\n-\t\tgit rev-list refs/heads/main |\n+\t\tgit rev-list refs/heads/main >out &&\n \t\twhile read sha1; do\n \t\t\tnote_path=$(echo \"$sha1\" | sed \"$1\")\n \t\t\tcat <gfi &&\n+\tgit fast-import --quiet /dev/null |\n+\tgit ls-tree -r --name-only $notes_commit |\n \twhile read path\n \tdo\n \t\tpath_has_fanout $path $fanout || return 1\n@@ -51,8 +50,9 @@ test_expect_success 'creating many notes with git-notes' '\n \tdone\n '\n \n-test_expect_success 'many notes created correctly with git-notes' '\n-\tgit log | grep \"^ \" > output &&\n+test_expect_success !SANITIZE_LEAK 'many notes created correctly with git-notes' '\n+\tgit log >output.raw &&\n+\tgrep \"^ \" output.raw >output &&\n \ti=$num_notes &&\n \twhile test $i -gt 0\n \tdo\n@@ -91,13 +91,13 @@ test_expect_success 'stable fanout 0 is followed by stable fanout 1' '\n test_expect_success 'deleting most notes with git-notes' '\n \tremove_notes=285 &&\n \ti=0 &&\n-\tgit rev-list HEAD |\n+\tgit rev-list HEAD >revs &&\n \twhile test $i -lt $remove_notes && read sha1\n \tdo\n \t\ti=$(($i + 1)) &&\n \t\ttest_tick &&\n-\t\tgit notes remove \"$sha1\" 2>/dev/null || return 1\n-\tdone\n+\t\tgit notes remove \"$sha1\" || return 1\n+\tdone \n---\n t/t4027-diff-submodule.sh | 7 ++-----\n 1 file changed, 2 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh\nindex 6cef0da982f..295da987cce 100755\n--- a/t/t4027-diff-submodule.sh\n+++ b/t/t4027-diff-submodule.sh\n@@ -2,7 +2,6 @@\n \n test_description='difference in submodules'\n \n-TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-diff.sh\n \n@@ -28,10 +27,8 @@ test_expect_success setup '\n \t\tgit commit -m \"submodule #2\"\n \t) &&\n \n-\tset x $(\n-\t\tcd sub &&\n-\t\tgit rev-list HEAD\n-\t) &&\n+\tgit -C sub rev-list HEAD >revs &&\n+\tset x $(cat revs) &&\n \techo \":160000 160000 $3 $ZERO_OID M\tsub\" >expect &&\n \tsubtip=$3 subprev=$2\n '\n-- \n2.35.1.1242.gfeba0eae32b\n\n"},{"id":"450563","messageId":"patch-v2-12.15-552dcac705d-20220307T124817Z-avarab@gmail.com","threadId":"57499","inReplyTo":"cover-v2-00.15-00000000000-20220307T124817Z-avarab@gmail.com","subject":"[PATCH v2 12/15] gettext tests: don't ignore \"test-tool regex\" exit code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-07T12:49:03Z","receivedAt":"2022-03-07T12:50:04Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Amend a prerequisite check added in 5c1ebcca4d1 (grep/icase: avoid\nkwsset on literal non-ascii strings, 2016-06-25) to do invoke\n'test-tool regex' in such a way that we'll notice if it dies under\nSANITIZE=leak due to having a memory leak, as opposed to us not having\nthe \"ICASE\" support we're checking for.\n\nBecause we weren't making a distinction between the two I'd marked\nthese tests as passing under SANITIZE=leak in 03d85e21951 (leak tests:\nmark remaining leak-free tests as such, 2021-12-17).\n\nDoing this is tricky. Ideally \"test_lazy_prereq\" would materialize as\na \"real\" test that we could check the exit code of with the same\nsignal matching that \"test_must_fail\" does.\n\nHowever lazy prerequisites aren't real tests, and are instead lazily\nmaterialized in the guts of \"test_have_prereq\" when we've already\nstarted another test.\n\nWe could detect the abort() (or similar) there and pass that exit code\ndown, and fail the test that caused the prerequisites to be\nmaterialized.\n\nBut that would require extensive changes to test-lib.sh and\ntest-lib-functions.sh. Let's instead simply check if the exit code of\n\"test-tool regex\" is zero, and if so set the prerequisites. If it's\nnon-zero let's run it again with \"test_must_fail\". We'll thus make a\ndistinction between \"bad\" non-zero (segv etc) and \"good\" (exit 1 etc.).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t7812-grep-icase-non-ascii.sh | 16 +++++++++++++---\n 1 file changed, 13 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh\nindex ca3f24f8079..9047d665a10 100755\n--- a/t/t7812-grep-icase-non-ascii.sh\n+++ b/t/t7812-grep-icase-non-ascii.sh\n@@ -11,9 +11,19 @@ test_expect_success GETTEXT_LOCALE 'setup' '\n \texport LC_ALL\n '\n \n-test_have_prereq GETTEXT_LOCALE &&\n-test-tool regex \"HALLÓ\" \"Halló\" ICASE &&\n-test_set_prereq REGEX_LOCALE\n+test_expect_success GETTEXT_LOCALE 'setup REGEX_LOCALE prerequisite' '\n+\t# This \"test-tool\" invocation is identical...\n+\tif test-tool regex \"HALLÓ\" \"Halló\" ICASE\n+\tthen\n+\t\ttest_set_prereq REGEX_LOCALE\n+\telse\n+\n+\t\t# ... to this one, but this way \"test_must_fail\" will\n+\t\t# tell a segfault or abort() from the regexec() test\n+\t\t# itself\n+\t\ttest_must_fail test-tool regex \"HALLÓ\" \"Halló\" ICASE\n+\tfi\n+'\n \n test_expect_success REGEX_LOCALE 'grep literal string, no -F' '\n \tgit grep -i \"TILRAUN: Halló Heimur!\" &&\n-- \n2.35.1.1242.gfeba0eae32b\n\n"},{"id":"450564","messageId":"patch-v2-13.15-dbe8d168401-20220307T124817Z-avarab@gmail.com","threadId":"57499","inReplyTo":"cover-v2-00.15-00000000000-20220307T124817Z-avarab@gmail.com","subject":"[PATCH v2 13/15] apply tests: don't ignore \"git ls-files\" exit code, drop sub-shell","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-07T12:49:04Z","receivedAt":"2022-03-07T12:50:05Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix code added in 969c877506c (git apply --directory broken for new\nfiles, 2008-10-12) so that it doesn't invoke \"git ls-files\" on the\nleft-hand-side of a pipe, instead let's use an intermediate file.\n\nSince we're doing that we can also drop the sub-shell that was here to\ngroup the two.\n\nThere are a lot of these sorts of patterns in the test suite, and\nthere's no particular reason to fix this one other than in a preceding\ncommit all similar patterns except this one were fixed in\n\"t/t4128-apply-root.sh\", so let's fix this one straggler as well.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t4128-apply-root.sh | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t4128-apply-root.sh b/t/t4128-apply-root.sh\nindex ba89a2f2d73..f6db5a79dd9 100755\n--- a/t/t4128-apply-root.sh\n+++ b/t/t4128-apply-root.sh\n@@ -96,7 +96,8 @@ test_expect_success 'apply --directory (delete file)' '\n \techo content >some/sub/dir/delfile &&\n \tgit add some/sub/dir/delfile &&\n \tgit apply --directory=some/sub/dir/ --index patch &&\n-\t! (git ls-files | grep delfile)\n+\tgit ls-files >out &&\n+\t! grep delfile out\n '\n \n cat > patch << 'EOF'\n-- \n2.35.1.1242.gfeba0eae32b\n\n"},{"id":"450565","messageId":"patch-v2-14.15-22b81d7f93a-20220307T124817Z-avarab@gmail.com","threadId":"57499","inReplyTo":"cover-v2-00.15-00000000000-20220307T124817Z-avarab@gmail.com","subject":"[PATCH v2 14/15] checkout tests: don't ignore \"git \" exit code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-07T12:49:05Z","receivedAt":"2022-03-07T12:50:07Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change a fragile pattern introduced in 696acf45f96 (checkout:\nimplement \"-\" abbreviation, add docs and tests, 2009-01-17) to check\nthe exit code of both \"git symbolic-ref\" and \"git rev-parse\".\n\nWithout this change this test will become flaky e.g. under\nSANITIZE=leak if some (but not all) memory leaks revealed by these\ncommands are fixed.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t2012-checkout-last.sh | 51 +++++++++++++++++++++++++++-------------\n 1 file changed, 35 insertions(+), 16 deletions(-)\n\ndiff --git a/t/t2012-checkout-last.sh b/t/t2012-checkout-last.sh\nindex 42601d5a310..1f6c4ed0428 100755\n--- a/t/t2012-checkout-last.sh\n+++ b/t/t2012-checkout-last.sh\n@@ -21,14 +21,20 @@ test_expect_success 'first branch switch' '\n \tgit checkout other\n '\n \n+test_cmp_symbolic_HEAD_ref () {\n+\techo refs/heads/\"$1\" >expect &&\n+\tgit symbolic-ref HEAD >actual &&\n+\ttest_cmp expect actual\n+}\n+\n test_expect_success '\"checkout -\" switches back' '\n \tgit checkout - &&\n-\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/main\"\n+\ttest_cmp_symbolic_HEAD_ref main\n '\n \n test_expect_success '\"checkout -\" switches forth' '\n \tgit checkout - &&\n-\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/other\"\n+\ttest_cmp_symbolic_HEAD_ref other\n '\n \n test_expect_success 'detach HEAD' '\n@@ -37,12 +43,16 @@ test_expect_success 'detach HEAD' '\n \n test_expect_success '\"checkout -\" attaches again' '\n \tgit checkout - &&\n-\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/other\"\n+\ttest_cmp_symbolic_HEAD_ref other\n '\n \n test_expect_success '\"checkout -\" detaches again' '\n \tgit checkout - &&\n-\ttest \"z$(git rev-parse HEAD)\" = \"z$(git rev-parse other)\" &&\n+\n+\tgit rev-parse other >expect &&\n+\tgit rev-parse HEAD >actual &&\n+\ttest_cmp expect actual &&\n+\n \ttest_must_fail git symbolic-ref HEAD\n '\n \n@@ -63,31 +73,31 @@ more_switches () {\n test_expect_success 'switch to the last' '\n \tmore_switches &&\n \tgit checkout @{-1} &&\n-\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/branch2\"\n+\ttest_cmp_symbolic_HEAD_ref branch2\n '\n \n test_expect_success 'switch to second from the last' '\n \tmore_switches &&\n \tgit checkout @{-2} &&\n-\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/branch3\"\n+\ttest_cmp_symbolic_HEAD_ref branch3\n '\n \n test_expect_success 'switch to third from the last' '\n \tmore_switches &&\n \tgit checkout @{-3} &&\n-\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/branch4\"\n+\ttest_cmp_symbolic_HEAD_ref branch4\n '\n \n test_expect_success 'switch to fourth from the last' '\n \tmore_switches &&\n \tgit checkout @{-4} &&\n-\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/branch5\"\n+\ttest_cmp_symbolic_HEAD_ref branch5\n '\n \n test_expect_success 'switch to twelfth from the last' '\n \tmore_switches &&\n \tgit checkout @{-12} &&\n-\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/branch13\"\n+\ttest_cmp_symbolic_HEAD_ref branch13\n '\n \n test_expect_success 'merge base test setup' '\n@@ -98,19 +108,28 @@ test_expect_success 'merge base test setup' '\n test_expect_success 'another...main' '\n \tgit checkout another &&\n \tgit checkout another...main &&\n-\ttest \"z$(git rev-parse --verify HEAD)\" = \"z$(git rev-parse --verify main^)\"\n+\n+\tgit rev-parse --verify main^ >expect &&\n+\tgit rev-parse --verify HEAD >actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success '...main' '\n \tgit checkout another &&\n \tgit checkout ...main &&\n-\ttest \"z$(git rev-parse --verify HEAD)\" = \"z$(git rev-parse --verify main^)\"\n+\n+\tgit rev-parse --verify main^ >expect &&\n+\tgit rev-parse --verify HEAD >actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'main...' '\n \tgit checkout another &&\n \tgit checkout main... &&\n-\ttest \"z$(git rev-parse --verify HEAD)\" = \"z$(git rev-parse --verify main^)\"\n+\n+\tgit rev-parse --verify main^ >expect &&\n+\tgit rev-parse --verify HEAD >actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success '\"checkout -\" works after a rebase A' '\n@@ -118,7 +137,7 @@ test_expect_success '\"checkout -\" works after a rebase A' '\n \tgit checkout other &&\n \tgit rebase main &&\n \tgit checkout - &&\n-\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/main\"\n+\ttest_cmp_symbolic_HEAD_ref main\n '\n \n test_expect_success '\"checkout -\" works after a rebase A B' '\n@@ -127,7 +146,7 @@ test_expect_success '\"checkout -\" works after a rebase A B' '\n \tgit checkout other &&\n \tgit rebase main moodle &&\n \tgit checkout - &&\n-\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/main\"\n+\ttest_cmp_symbolic_HEAD_ref main\n '\n \n test_expect_success '\"checkout -\" works after a rebase -i A' '\n@@ -135,7 +154,7 @@ test_expect_success '\"checkout -\" works after a rebase -i A' '\n \tgit checkout other &&\n \tgit rebase -i main &&\n \tgit checkout - &&\n-\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/main\"\n+\ttest_cmp_symbolic_HEAD_ref main\n '\n \n test_expect_success '\"checkout -\" works after a rebase -i A B' '\n@@ -144,7 +163,7 @@ test_expect_success '\"checkout -\" works after a rebase -i A B' '\n \tgit checkout other &&\n \tgit rebase main foodle &&\n \tgit checkout - &&\n-\ttest \"z$(git symbolic-ref HEAD)\" = \"zrefs/heads/main\"\n+\ttest_cmp_symbolic_HEAD_ref main\n '\n \n test_done\n-- \n2.35.1.1242.gfeba0eae32b\n\n"},{"id":"450566","messageId":"patch-v2-11.15-52397b3575a-20220307T124817Z-avarab@gmail.com","threadId":"57499","inReplyTo":"cover-v2-00.15-00000000000-20220307T124817Z-avarab@gmail.com","subject":"[PATCH v2 11/15] rev-list tests: don't hide abort() in \"test_expect_failure\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-07T12:49:02Z","receivedAt":"2022-03-07T12:50:08Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change a couple of uses of \"test_expect_failure\" to use a\n\"test_expect_success\" to positively assert the current behavior, and\nreplace the intent of \"test_expect_failure\" with a \"TODO\" comment int\nthe description.\n\nAs noted in [1] the \"test_expect_failure\" feature is overly eager to\naccept any failure as OK, and thus by design hides segfaults, abort()\netc. Because of that I didn't notice in dd9cede9136 (leak tests: mark\nsome rev-list tests as passing with SANITIZE=leak, 2021-10-31) that\nthis test leaks memory under SANITIZE=leak.\n\nI have some larger local changes to add a better\n\"test_expect_failure\", which would work just like\n\"test_expect_success\", but would allow us say \"test_todo\" here (and\n\"success\" would emit a \"not ok [...] # TODO\", not \"ok [...]\".\n\nSo even though using \"test_expect_success\" here comes with its own\nproblems[2], let's use it as a narrow change to fix the problem at\nhand here and stop conflating the current \"success\" with actual\nSANITIZE=leak failures.\n\n1. https://lore.kernel.org/git/87tuhmk19c.fsf@evledraar.gmail.com/\n2. https://lore.kernel.org/git/xmqq4k9kj15p.fsf@gitster.g/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t6102-rev-list-unexpected-objects.sh | 13 +++++++++----\n 1 file changed, 9 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t6102-rev-list-unexpected-objects.sh b/t/t6102-rev-list-unexpected-objects.sh\nindex 6f0902b8638..cf0195e8263 100755\n--- a/t/t6102-rev-list-unexpected-objects.sh\n+++ b/t/t6102-rev-list-unexpected-objects.sh\n@@ -17,8 +17,13 @@ test_expect_success 'setup unexpected non-blob entry' '\n \tbroken_tree=\"$(git hash-object -w --literally -t tree broken-tree)\"\n '\n \n-test_expect_failure 'traverse unexpected non-blob entry (lone)' '\n-\ttest_must_fail git rev-list --objects $broken_tree\n+test_expect_success !SANITIZE_LEAK 'TODO (should fail!): traverse unexpected non-blob entry (lone)' '\n+\tsed \"s/Z$//\" >expect <<-EOF &&\n+\t$broken_tree Z\n+\t$tree foo\n+\tEOF\n+\tgit rev-list --objects $broken_tree >actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'traverse unexpected non-blob entry (seen)' '\n@@ -116,8 +121,8 @@ test_expect_success 'setup unexpected non-blob tag' '\n \ttag=$(git hash-object -w --literally -t tag broken-tag)\n '\n \n-test_expect_failure 'traverse unexpected non-blob tag (lone)' '\n-\ttest_must_fail git rev-list --objects $tag\n+test_expect_success !SANITIZE_LEAK 'TODO (should fail!): traverse unexpected non-blob tag (lone)' '\n+\tgit rev-list --objects $tag\n '\n \n test_expect_success 'traverse unexpected non-blob tag (seen)' '\n-- \n2.35.1.1242.gfeba0eae32b\n\n"},{"id":"450567","messageId":"patch-v2-15.15-16889ed154f-20220307T124817Z-avarab@gmail.com","threadId":"57499","inReplyTo":"cover-v2-00.15-00000000000-20220307T124817Z-avarab@gmail.com","subject":"[PATCH v2 15/15] rev-list simplify tests: don't ignore \"git\" exit code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-07T12:49:06Z","receivedAt":"2022-03-07T12:50:11Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change a fragile test pattern introduced in 65347030590 (Topo-sort\nbefore --simplify-merges, 2008-08-03) to check the exit code of both\n\"git name-rev\" and \"git log\".\n\nThis test as a whole would fail under SANITIZE=leak, but we'd pass\nseveral \"failing\" tests due to hiding these exit codes before we'd\nspot git dying with abort(). Now we'll instead spot all of the\nfailures.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t6012-rev-list-simplify.sh | 12 +++++++-----\n 1 file changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t6012-rev-list-simplify.sh b/t/t6012-rev-list-simplify.sh\nindex 63fcccec32e..de1e87f1621 100755\n--- a/t/t6012-rev-list-simplify.sh\n+++ b/t/t6012-rev-list-simplify.sh\n@@ -12,7 +12,9 @@ note () {\n }\n \n unnote () {\n-\tgit name-rev --tags --annotate-stdin | sed -e \"s|$OID_REGEX (tags/\\([^)]*\\)) |\\1 |g\"\n+\ttest_when_finished \"rm -f tmp\" &&\n+\tgit name-rev --tags --annotate-stdin >tmp &&\n+\tsed -e \"s|$OID_REGEX (tags/\\([^)]*\\)) |\\1 |g\" actual &&\n+\t\tgit log --pretty=\"$FMT\" --parents $param >out &&\n+\t\tunnote >actual check expect &&\n-\tgit log --pretty=\"$FMT\" --full-history E -- lost |\n-\tunnote >actual &&\n+\tgit log --pretty=\"$FMT\" --full-history E -- lost >out &&\n+\tunnote >actual check