This series fixes issues where we ignored the exit code of "git" due to it being on the LHS of a pipe, or because we interpolated its output with $() in a "test" construct, or had missing &&- chains in helper functions etc. As noted in v1 (https://lore.kernel.org/git/cover-00.15-00000000000-20220302T171755Z-avarab@xxxxxxxxx/) this is not a general search/replacement, but fixes real issues we run into with SANITIZE=leak. Changes since v1: * Minor commit message improvement for 11/15 * Added a comment for some non-obvious prereq setup code, as suggested by Junio. Ævar Arnfjörð Bjarmason (15): tests: change some 'test $(git) = "x"' to test_cmp tests: use "test_stdout_line_count", not "test $(git [...] | wc -l)" read-tree tests: check "diff-files" exit code on failure diff tests: don't ignore "git diff" exit code diff tests: don't ignore "git diff" exit code in "read" loop apply tests: use "test_must_fail" instead of ad-hoc pattern merge tests: use "test_must_fail" instead of ad-hoc pattern rev-parse tests: don't ignore "git reflog" exit code notes tests: don't ignore "git" exit code diff tests: don't ignore "git rev-list" exit code rev-list tests: don't hide abort() in "test_expect_failure" gettext tests: don't ignore "test-tool regex" exit code apply tests: don't ignore "git ls-files" exit code, drop sub-shell checkout tests: don't ignore "git <cmd>" exit code rev-list simplify tests: don't ignore "git" exit code t/t0002-gitfile.sh | 6 +- t/t1001-read-tree-m-2way.sh | 6 +- t/t1002-read-tree-m-u-2way.sh | 6 +- t/t1503-rev-parse-verify.sh | 5 +- t/t2012-checkout-last.sh | 51 ++++++--- t/t2200-add-update.sh | 33 ++++-- t/t3302-notes-index-expensive.sh | 6 +- t/t3303-notes-subtrees.sh | 9 +- t/t3305-notes-fanout.sh | 14 +-- t/t4020-diff-external.sh | 153 ++++++++++++------------- t/t4027-diff-submodule.sh | 7 +- t/t4123-apply-shrink.sh | 18 +-- t/t4128-apply-root.sh | 36 +++--- t/t6005-rev-list-count.sh | 43 ++++--- t/t6012-rev-list-simplify.sh | 12 +- t/t6102-rev-list-unexpected-objects.sh | 13 ++- t/t6407-merge-binary.sh | 22 +--- t/t7103-reset-bare.sh | 7 +- t/t7812-grep-icase-non-ascii.sh | 16 ++- 19 files changed, 245 insertions(+), 218 deletions(-) Range-diff against v1: 1: 78b9c52551f = 1: 78b9c52551f tests: change some 'test $(git) = "x"' to test_cmp 2: e1105b029d6 = 2: e1105b029d6 tests: use "test_stdout_line_count", not "test $(git [...] | wc -l)" 3: 5f02e30d1ab = 3: 5f02e30d1ab read-tree tests: check "diff-files" exit code on failure 4: a425ced5609 = 4: a425ced5609 diff tests: don't ignore "git diff" exit code 5: b1aeac3f68e = 5: b1aeac3f68e diff tests: don't ignore "git diff" exit code in "read" loop 6: 7952ae1f3b5 = 6: 7952ae1f3b5 apply tests: use "test_must_fail" instead of ad-hoc pattern 7: 276be19e35e = 7: 276be19e35e merge tests: use "test_must_fail" instead of ad-hoc pattern 8: dca2ac3a171 = 8: dca2ac3a171 rev-parse tests: don't ignore "git reflog" exit code 9: ca9e12f2bac = 9: ca9e12f2bac notes tests: don't ignore "git" exit code 10: 946397033d4 = 10: 946397033d4 diff tests: don't ignore "git rev-list" exit code 11: 26c838f0560 ! 11: 52397b3575a rev-list tests: don't hide abort() in "test_expect_failure" @@ Commit message "test_expect_success", but would allow us say "test_todo" here (and "success" would emit a "not ok [...] # TODO", not "ok [...]". - In lieu of those larger changes let's more narrowly fix the problem at + So even though using "test_expect_success" here comes with its own + problems[2], let's use it as a narrow change to fix the problem at hand here and stop conflating the current "success" with actual SANITIZE=leak failures. 1. https://lore.kernel.org/git/87tuhmk19c.fsf@xxxxxxxxxxxxxxxxxxx/ + 2. https://lore.kernel.org/git/xmqq4k9kj15p.fsf@gitster.g/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 12: f3cc5bc7eb9 ! 12: 552dcac705d gettext tests: don't ignore "test-tool regex" exit code @@ t/t7812-grep-icase-non-ascii.sh: test_expect_success GETTEXT_LOCALE 'setup' ' -test-tool regex "HALLÓ" "Halló" ICASE && -test_set_prereq REGEX_LOCALE +test_expect_success GETTEXT_LOCALE 'setup REGEX_LOCALE prerequisite' ' ++ # This "test-tool" invocation is identical... + if test-tool regex "HALLÓ" "Halló" ICASE + then + test_set_prereq REGEX_LOCALE + else ++ ++ # ... to this one, but this way "test_must_fail" will ++ # tell a segfault or abort() from the regexec() test ++ # itself + test_must_fail test-tool regex "HALLÓ" "Halló" ICASE + fi +' 13: 834809b1b8a = 13: dbe8d168401 apply tests: don't ignore "git ls-files" exit code, drop sub-shell 14: 34cada14fec = 14: 22b81d7f93a checkout tests: don't ignore "git <cmd>" exit code 15: 4ee216711cf = 15: 16889ed154f rev-list simplify tests: don't ignore "git" exit code -- 2.35.1.1242.gfeba0eae32b
