{"thread":{"id":"58191","subject":"[PATCH 00/10] leak test: add \"check\" test mode, mark leak-free tests","startedAt":"2022-07-19T21:05:39Z","lastAt":"2022-09-26T19:08:44Z","messageCount":57,"participants":["Ævar Arnfjörð Bjarmason","Derrick Stolee","Junio C Hamano","SZEDER Gábor","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":10},"messages":[{"id":"459461","messageId":"cover-00.10-00000000000-20220719T205710Z-avarab@gmail.com","threadId":"58191","inReplyTo":null,"subject":"[PATCH 00/10] leak test: add \"check\" test mode, mark leak-free tests","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-19T21:05:14Z","receivedAt":"2022-07-19T21:05:39Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Since release_revisions() we've had much fewer widespread leaks in the\ntest suite, so I think it's time to mark even more tests as running in\nthe linux-leaks CI job. The 9/10 here does that.\n\nBut before that we introduce a \"GIT_TEST_PASSING_SANITIZE_LEAK=check\"\nleak mode, this uses test-lib.sh --invert-exit-code feature added here\nto fail tests whose TEST_PASSES_SANITIZE_LEAK=true marking isn't\ncurrent.\n\nI've been using this series for a while to help in submitting all the\nother leak fixes I've been sending in. It's very useful to run the\n\"check\" with \"git rebase -x\", i.e. to atomically mark tests as\nleak-free as we fix leaks.\n\nIn adding \"check\" I needed to add a new\nGIT_TEST_SANITIZE_LEAK_LOG=true setting, and integrate it with\n\"check\". I.e. we have many cases where we lose exit codes from git, or\notherwise don't notice an abort, 08/10 here discusses why that is.\n\nWhen \"check\" and \"GIT_TEST_SANITIZE_LEAK_LOG=true\" are combined we'll\ntake into account leaks that we log to decide whether to pass the\ntests.\n\nWriting out the leak logs is also very useful to analyze our remaining\nleaks. I've got a local script that scrapes the logs, and emits a\nsorted list of which leaks provide the best bang-for-the-buck in terms\nof getting more tests passing. That's not being submitted here, but I\ncan follow-up and send that in if there's interest.\n\nÆvar Arnfjörð Bjarmason (10):\n test-lib.sh: use $1, not $@ in test_known_broken_{ok,failure}_\n test-lib.sh: don't set GIT_EXIT_OK before calling test_atexit_handler\n test-lib.sh: fix GIT_EXIT_OK logic errors, use BAIL_OUT\n test-lib.sh: add a --invert-exit-code switch\n t/README: reword the \"GIT_TEST_PASSING_SANITIZE_LEAK\" description\n test-lib: add a SANITIZE=leak logging mode\n test-lib.sh: add a GIT_TEST_PASSING_SANITIZE_LEAK=check mode\n test-lib: have the \"check\" mode for SANITIZE=leak consider leak logs\n leak tests: mark passing SANITIZE=leak tests as leak-free\n log tests: don't use \"exit 1\" outside a sub-shell\n\n t/README | 36 +++++-\n t/t0000-basic.sh | 72 +++++++++++\n t/t0027-auto-crlf.sh | 1 +\n t/t0032-reftable-unittest.sh | 1 +\n t/t0033-safe-directory.sh | 1 +\n t/t0050-filesystem.sh | 1 +\n t/t0095-bloom.sh | 2 +\n t/t1405-main-ref-store.sh | 1 +\n t/t1407-worktree-ref-store.sh | 1 +\n t/t1418-reflog-exists.sh | 1 +\n t/t1701-racy-split-index.sh | 1 +\n t/t2006-checkout-index-basic.sh | 1 +\n t/t2023-checkout-m.sh | 1 +\n t/t2205-add-worktree-config.sh | 1 +\n t/t3012-ls-files-dedup.sh | 1 +\n t/t4017-diff-retval.sh | 1 +\n t/t4051-diff-function-context.sh | 1 +\n t/t4057-diff-combined-paths.sh | 1 +\n t/t4114-apply-typechange.sh | 1 +\n t/t4205-log-pretty-formats.sh | 2 +-\n t/t4301-merge-tree-write-tree.sh | 1 +\n t/t5315-pack-objects-compression.sh | 1 +\n t/t5351-unpack-large-objects.sh | 1 +\n t/t5402-post-merge-hook.sh | 1 +\n t/t5503-tagfollow.sh | 1 +\n t/t6404-recursive-merge.sh | 1 +\n t/t6405-merge-symlinks.sh | 1 +\n t/t6407-merge-binary.sh | 1 -\n t/t6408-merge-up-to-date.sh | 1 +\n t/t6411-merge-filemode.sh | 1 +\n t/t6413-merge-crlf.sh | 1 +\n t/t6415-merge-dir-to-symlink.sh | 1 +\n t/t6425-merge-rename-delete.sh | 1 +\n t/t6431-merge-criscross.sh | 1 +\n t/t7060-wtstatus.sh | 1 +\n t/t7062-wtstatus-ignorecase.sh | 1 +\n t/t7110-reset-merge.sh | 1 +\n t/t7111-reset-table.sh | 1 +\n t/t7609-mergetool--lib.sh | 1 +\n t/t9100-git-svn-basic.sh | 1 -\n t/t9700-perl-git.sh | 6 +\n t/t9901-git-web--browse.sh | 1 +\n t/test-lib.sh | 187 ++++++++++++++++++++++++----\n 43 files changed, 310 insertions(+), 32 deletions(-)\n\n-- \n2.37.1.1062.g385eac7fccf\n\n"},{"id":"459462","messageId":"patch-01.10-5664c4f9a0e-20220719T205710Z-avarab@gmail.com","threadId":"58191","inReplyTo":"cover-00.10-00000000000-20220719T205710Z-avarab@gmail.com","subject":"[PATCH 01/10] test-lib.sh: use $1, not $@ in test_known_broken_{ok,failure}_","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-19T21:05:15Z","receivedAt":"2022-07-19T21:05:41Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Clarify that these two functions never take N arguments, they'll only\never receive one. They've needlessly used $@ over $1 since\n41ac414ea2b (Sane use of test_expect_failure, 2008-02-01).\n\nIn the future we might want to pass the test source to these, but now\nthat's not the case. This preparatory change helps to clarify a\nfollow-up change.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/test-lib.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 7726d1da88a..3f11ce35112 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -804,14 +804,14 @@ test_failure_ () {\n \n test_known_broken_ok_ () {\n \ttest_fixed=$(($test_fixed+1))\n-\tsay_color error \"ok $test_count - $@ # TODO known breakage vanished\"\n-\tfinalize_test_case_output fixed \"$@\"\n+\tsay_color error \"ok $test_count - $1 # TODO known breakage vanished\"\n+\tfinalize_test_case_output fixed \"$1\"\n }\n \n test_known_broken_failure_ () {\n \ttest_broken=$(($test_broken+1))\n-\tsay_color warn \"not ok $test_count - $@ # TODO known breakage\"\n-\tfinalize_test_case_output broken \"$@\"\n+\tsay_color warn \"not ok $test_count - $1 # TODO known breakage\"\n+\tfinalize_test_case_output broken \"$1\"\n }\n \n test_debug () {\n-- \n2.37.1.1062.g385eac7fccf\n\n"},{"id":"459463","messageId":"patch-02.10-c228308c121-20220719T205710Z-avarab@gmail.com","threadId":"58191","inReplyTo":"cover-00.10-00000000000-20220719T205710Z-avarab@gmail.com","subject":"[PATCH 02/10] test-lib.sh: don't set GIT_EXIT_OK before calling test_atexit_handler","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-19T21:05:16Z","receivedAt":"2022-07-19T21:05:44Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the control flow in test_done so that we'll set GIT_EXIT_OK=t\nafter we call test_atexit_handler(). This seems to have been a mistake\nin 900721e15c4 (test-lib: introduce 'test_atexit', 2019-03-13). It\ndoesn't make sense to allow our \"atexit\" handling to call \"exit\"\nwithout us emitting the errors we'll emit without GIT_EXIT_OK=t being\nset.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/test-lib.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 3f11ce35112..c8c84ef9b14 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -1169,12 +1169,12 @@ test_atexit_handler () {\n }\n \n test_done () {\n-\tGIT_EXIT_OK=t\n-\n \t# Run the atexit commands _before_ the trash directory is\n \t# removed, so the commands can access pidfiles and socket files.\n \ttest_atexit_handler\n \n+\tGIT_EXIT_OK=t\n+\n \tfinalize_test_output\n \n \tif test -z \"$HARNESS_ACTIVE\"\n-- \n2.37.1.1062.g385eac7fccf\n\n"},{"id":"459464","messageId":"patch-03.10-e57e7ca898e-20220719T205710Z-avarab@gmail.com","threadId":"58191","inReplyTo":"cover-00.10-00000000000-20220719T205710Z-avarab@gmail.com","subject":"[PATCH 03/10] test-lib.sh: fix GIT_EXIT_OK logic errors, use BAIL_OUT","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-19T21:05:17Z","receivedAt":"2022-07-19T21:05:46Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change various \"exit 1\" checks that happened after our \"die\" handler\nhad been set up to use BAIL_OUT instead. See 234383cd401 (test-lib.sh:\nuse \"Bail out!\" syntax on bad SANITIZE=leak use, 2021-10-14) for the\nbenefits of the BAIL_OUT function.\n\nThe previous use of \"error\" here was not a logic error, but the \"exit\"\nwithout \"GIT_EXIT_OK\" would emit the \"FATAL: Unexpected exit with code\n$code\" message on top of the error we wanted to emit.\n\nSince we'd also like to stop \"prove\" in its tracks here, the right\nthing to do is to emit a \"Bail out!\" message.\n\nLet's also move the \"GIT_EXIT_OK=t\" assignments to just above the\n\"exit [01]\" in \"test_done\". It's not OK if we exit in\ne.g. finalize_test_output.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/test-lib.sh | 16 ++++++----------\n 1 file changed, 6 insertions(+), 10 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex c8c84ef9b14..118720493bb 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -1173,8 +1173,6 @@ test_done () {\n \t# removed, so the commands can access pidfiles and socket files.\n \ttest_atexit_handler\n \n-\tGIT_EXIT_OK=t\n-\n \tfinalize_test_output\n \n \tif test -z \"$HARNESS_ACTIVE\"\n@@ -1246,6 +1244,7 @@ test_done () {\n \t\tfi\n \t\ttest_at_end_hook_\n \n+\t\tGIT_EXIT_OK=t\n \t\texit 0 ;;\n \n \t*)\n@@ -1255,6 +1254,7 @@ test_done () {\n \t\t\tsay \"1..$test_count\"\n \t\tfi\n \n+\t\tGIT_EXIT_OK=t\n \t\texit 1 ;;\n \n \tesac\n@@ -1387,14 +1387,12 @@ fi\n GITPERLLIB=\"$GIT_BUILD_DIR\"/perl/build/lib\n export GITPERLLIB\n test -d \"$GIT_BUILD_DIR\"/templates/blt || {\n-\terror \"You haven't built things yet, have you?\"\n+\tBAIL_OUT \"You haven't built things yet, have you?\"\n }\n \n if ! test -x \"$GIT_BUILD_DIR\"/t/helper/test-tool$X\n then\n-\techo >&2 'You need to build test-tool:'\n-\techo >&2 'Run \"make t/helper/test-tool\" in the source (toplevel) directory'\n-\texit 1\n+\tBAIL_OUT 'You need to build test-tool; Run \"make t/helper/test-tool\" in the source (toplevel) directory'\n fi\n \n # Are we running this test at all?\n@@ -1448,9 +1446,7 @@ remove_trash_directory () {\n \n # Test repository\n remove_trash_directory \"$TRASH_DIRECTORY\" || {\n-\tGIT_EXIT_OK=t\n-\techo >&5 \"FATAL: Cannot prepare test area\"\n-\texit 1\n+\tBAIL_OUT 'cannot prepare test area'\n }\n \n remove_trash=t\n@@ -1466,7 +1462,7 @@ fi\n \n # Use -P to resolve symlinks in our working directory so that the cwd\n # in subprocesses like git equals our $PWD (for pathname comparisons).\n-cd -P \"$TRASH_DIRECTORY\" || exit 1\n+cd -P \"$TRASH_DIRECTORY\" || BAIL_OUT \"cannot cd -P to \\\"$TRASH_DIRECTORY\\\"\"\n \n start_test_output \"$0\"\n \n-- \n2.37.1.1062.g385eac7fccf\n\n"},{"id":"459465","messageId":"patch-04.10-4aab7af60e3-20220719T205710Z-avarab@gmail.com","threadId":"58191","inReplyTo":"cover-00.10-00000000000-20220719T205710Z-avarab@gmail.com","subject":"[PATCH 04/10] test-lib.sh: add a --invert-exit-code switch","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-19T21:05:18Z","receivedAt":"2022-07-19T21:05:48Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add the ability to have those tests that fail return 0, and those\ntests that succeed return 1. This is useful e.g. to run \"--stress\"\ntests on tests that fail 99% of the time on some setup, i.e. to smoke\nout the flaky run which yielded success.\n\nIn a subsequent commit a new SANITIZE=leak mode will make use of this.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t0000-basic.sh | 72 ++++++++++++++++++++++++++++++++++++++++++++++++\n t/test-lib.sh | 36 +++++++++++++++++++++++-\n 2 files changed, 107 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex 17a268ccd1b..502b4bcf9ea 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -578,6 +578,78 @@ test_expect_success 'subtest: --run invalid range end' '\n \tEOF_ERR\n '\n \n+test_expect_success 'subtest: --invert-exit-code without --immediate' '\n+\trun_sub_test_lib_test_err full-pass \\\n+\t\t--invert-exit-code &&\n+\tcheck_sub_test_lib_test_err full-pass \\\n+\t\t<<-\\EOF_OUT 3<<-EOF_ERR\n+\tok 1 - passing test #1\n+\tok 2 - passing test #2\n+\tok 3 - passing test #3\n+\t# passed all 3 test(s)\n+\t1..3\n+\t# faking up non-zero exit with --invert-exit-code\n+\tEOF_OUT\n+\tEOF_ERR\n+'\n+\n+test_expect_success 'subtest: --invert-exit-code with --immediate: all passed' '\n+\trun_sub_test_lib_test_err full-pass \\\n+\t\t--invert-exit-code --immediate &&\n+\tcheck_sub_test_lib_test_err full-pass \\\n+\t\t<<-\\EOF_OUT 3<<-EOF_ERR\n+\tok 1 - passing test #1\n+\tok 2 - passing test #2\n+\tok 3 - passing test #3\n+\t# passed all 3 test(s)\n+\t1..3\n+\t# faking up non-zero exit with --invert-exit-code\n+\tEOF_OUT\n+\tEOF_ERR\n+'\n+\n+test_expect_success 'subtest: --invert-exit-code without --immediate: partial pass' '\n+\trun_sub_test_lib_test partial-pass \\\n+\t\t--invert-exit-code &&\n+\tcheck_sub_test_lib_test partial-pass <<-\\EOF\n+\tok 1 - passing test #1\n+\tnot ok 2 - # TODO induced breakage (--invert-exit-code): failing test #2\n+\t#\tfalse\n+\tok 3 - passing test #3\n+\t# failed 1 among 3 test(s)\n+\t1..3\n+\t# faked up failures as TODO & now exiting with 0 due to --invert-exit-code\n+\tEOF\n+'\n+\n+test_expect_success 'subtest: --invert-exit-code with --immediate: partial pass' '\n+\trun_sub_test_lib_test partial-pass \\\n+\t\t--invert-exit-code --immediate &&\n+\tcheck_sub_test_lib_test partial-pass \\\n+\t\t<<-\\EOF_OUT 3<<-EOF_ERR\n+\tok 1 - passing test #1\n+\tnot ok 2 - # TODO induced breakage (--invert-exit-code): failing test #2\n+\t#\tfalse\n+\t1..2\n+\t# faked up failures as TODO & now exiting with 0 due to --invert-exit-code\n+\tEOF_OUT\n+\tEOF_ERR\n+'\n+\n+test_expect_success 'subtest: --invert-exit-code --immediate: got a failure' '\n+\trun_sub_test_lib_test partial-pass \\\n+\t\t--invert-exit-code --immediate &&\n+\tcheck_sub_test_lib_test_err partial-pass \\\n+\t\t<<-\\EOF_OUT 3<<-EOF_ERR\n+\tok 1 - passing test #1\n+\tnot ok 2 - # TODO induced breakage (--invert-exit-code): failing test #2\n+\t#\tfalse\n+\t1..2\n+\t# faked up failures as TODO & now exiting with 0 due to --invert-exit-code\n+\tEOF_OUT\n+\tEOF_ERR\n+'\n+\n test_expect_success 'subtest: tests respect prerequisites' '\n \twrite_and_run_sub_test_lib_test prereqs <<-\\EOF &&\n \ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 118720493bb..7f730b0f94f 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -238,6 +238,9 @@ parse_option () {\n \t\t\t;;\n \t\tesac\n \t\t;;\n+\t--invert-exit-code)\n+\t\tinvert_exit_code=t\n+\t\t;;\n \t*)\n \t\techo \"error: unknown test option '$opt'\" >&2; exit 1 ;;\n \tesac\n@@ -788,15 +791,31 @@ test_ok_ () {\n \tfinalize_test_case_output ok \"$@\"\n }\n \n+_invert_exit_code_failure_end_blurb () {\n+\tsay_color warn \"# faked up failures as TODO & now exiting with 0 due to --invert-exit-code\"\n+}\n+\n test_failure_ () {\n \tfailure_label=$1\n \ttest_failure=$(($test_failure + 1))\n-\tsay_color error \"not ok $test_count - $1\"\n+\tlocal pfx=\"\"\n+\tif test -n \"$invert_exit_code\" # && test -n \"$HARNESS_ACTIVE\"\n+\tthen\n+\t\tpfx=\"# TODO induced breakage (--invert-exit-code):\"\n+\tfi\n+\tsay_color error \"not ok $test_count - ${pfx:+$pfx }$1\"\n \tshift\n \tprintf '%s\\n' \"$*\" | sed -e 's/^/#\t/'\n \tif test -n \"$immediate\"\n \tthen\n \t\tsay_color error \"1..$test_count\"\n+\t\tif test -n \"$invert_exit_code\"\n+\t\tthen\n+\t\t\tfinalize_test_output\n+\t\t\t_invert_exit_code_failure_end_blurb\n+\t\t\tGIT_EXIT_OK=t\n+\t\t\texit 0\n+\t\tfi\n \t\t_error_exit\n \tfi\n \tfinalize_test_case_output failure \"$failure_label\" \"$@\"\n@@ -1242,6 +1261,14 @@ test_done () {\n \t\t\t} ||\n \t\t\terror \"Tests passed but test cleanup failed; aborting\"\n \t\tfi\n+\n+\t\tif test -z \"$skip_all\" && test -n \"$invert_exit_code\"\n+\t\tthen\n+\t\t\tsay_color warn \"# faking up non-zero exit with --invert-exit-code\"\n+\t\t\tGIT_EXIT_OK=t\n+\t\t\texit 1\n+\t\tfi\n+\n \t\ttest_at_end_hook_\n \n \t\tGIT_EXIT_OK=t\n@@ -1254,6 +1281,13 @@ test_done () {\n \t\t\tsay \"1..$test_count\"\n \t\tfi\n \n+\t\tif test -n \"$invert_exit_code\"\n+\t\tthen\n+\t\t\t_invert_exit_code_failure_end_blurb\n+\t\t\tGIT_EXIT_OK=t\n+\t\t\texit 0\n+\t\tfi\n+\n \t\tGIT_EXIT_OK=t\n \t\texit 1 ;;\n \n-- \n2.37.1.1062.g385eac7fccf\n\n"},{"id":"459466","messageId":"patch-07.10-0961df2ab6c-20220719T205710Z-avarab@gmail.com","threadId":"58191","inReplyTo":"cover-00.10-00000000000-20220719T205710Z-avarab@gmail.com","subject":"[PATCH 07/10] test-lib.sh: add a GIT_TEST_PASSING_SANITIZE_LEAK=check mode","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-19T21:05:21Z","receivedAt":"2022-07-19T21:05:52Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a new \"GIT_TEST_PASSING_SANITIZE_LEAK=check\" mode to the\ntest-lib.sh.\n\nAs noted in the updated \"t/README\" this compliments the existing\n\"GIT_TEST_PASSING_SANITIZE_LEAK=true\" mode added in\n956d2e4639b (tests: add a test mode for SANITIZE=leak, run it in CI,\n2021-09-23).\n\nIt does so by adding the ability to check that there's a 1=1\ncorrespondence between those tests that are marked as passing with\nSANITIZE=leak, and those tests that are leak-free. I.e. a test that\npasses with SANITIZE=leak but isn't marked as such with\nTEST_PASSES_SANITIZE_LEAK=true will error out.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/README | 10 ++++++++++\n t/t9700-perl-git.sh | 6 ++++++\n t/test-lib.sh | 34 ++++++++++++++++++++++++++--------\n 3 files changed, 42 insertions(+), 8 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex c2f996656d3..7b7082386ae 100644\n--- a/t/README\n+++ b/t/README\n@@ -371,6 +371,16 @@ declared themselves as leak-free by setting\n \"TEST_PASSES_SANITIZE_LEAK=true\" before sourcing \"test-lib.sh\". This\n test mode is used by the \"linux-leaks\" CI target.\n \n+GIT_TEST_PASSING_SANITIZE_LEAK=check checks that our\n+\"TEST_PASSES_SANITIZE_LEAK=true\" markings are current. The \"check\" is\n+particularly useful with \"--immediate\", but otherwise acts the same\n+for tests that have \"TEST_PASSES_SANITIZE_LEAK=true\" set. For those\n+that don't have it set it runs them, and considers them passing\n+without errors a failure (by providing \"--invert-exit-code\"). Thus the\n+\"check\" mode can be used e.g. with \"git rebase --exec\" to ensure that\n+there's a 1=1 mapping between \"TEST_PASSES_SANITIZE_LEAK=true\" and\n+those tests that pass under \"SANITIZE=leak\".\n+\n GIT_TEST_SANITIZE_LEAK_LOG=true will log memory leaks to\n \"test-results/$TEST_NAME.leak/trace.*\" files. Useful in combination\n with \"GIT_TEST_PASSING_SANITIZE_LEAK\" to check if we're falsely\ndiff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh\nindex 102c133112c..faba941117d 100755\n--- a/t/t9700-perl-git.sh\n+++ b/t/t9700-perl-git.sh\n@@ -11,6 +11,12 @@ if ! test_have_prereq PERL; then\n \ttest_done\n fi\n \n+if test \"$GIT_TEST_PASSING_SANITIZE_LEAK\" = \"check\" && test_have_prereq SANITIZE_LEAK\n+then\n+\tskip_all='SANITIZE=leak and GIT_TEST_PASSING_SANITIZE_LEAK=check do not combine with test_external'\n+\ttest_done\n+fi\n+\n perl -MTest::More -e 0 2>/dev/null || {\n \tskip_all=\"Perl Test::More unavailable, skipping test\"\n \ttest_done\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 73600c4d887..747bf6c50e5 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -1447,17 +1447,34 @@ fi\n # skip non-whitelisted tests when compiled with SANITIZE=leak\n if test -n \"$SANITIZE_LEAK\"\n then\n-\tif test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false\n+\t# Normalize with test_bool_env\n+\tpasses_sanitize_leak=\n+\n+\t# We need to see TEST_PASSES_SANITIZE_LEAK in \"git\n+\t# env--helper\" (via test_bool_env)\n+\texport TEST_PASSES_SANITIZE_LEAK\n+\tif test_bool_env TEST_PASSES_SANITIZE_LEAK false\n+\tthen\n+\t\tpasses_sanitize_leak=t\n+\tfi\n+\n+\tif test \"$GIT_TEST_PASSING_SANITIZE_LEAK\" = \"check\"\n \tthen\n-\t\t# We need to see it in \"git env--helper\" (via\n-\t\t# test_bool_env)\n-\t\texport TEST_PASSES_SANITIZE_LEAK\n+\t\tif test -n \"$invert_exit_code\"\n+\t\tthen\n+\t\t\tBAIL_OUT \"cannot use --invert-exit-code under GIT_TEST_PASSING_SANITIZE_LEAK=check\"\n+\t\tfi\n \n-\t\tif ! test_bool_env TEST_PASSES_SANITIZE_LEAK false\n+\t\tif test -z \"$passes_sanitize_leak\"\n \t\tthen\n-\t\t\tskip_all=\"skipping $this_test under GIT_TEST_PASSING_SANITIZE_LEAK=true\"\n-\t\t\ttest_done\n+\t\t\tsay \"in GIT_TEST_PASSING_SANITIZE_LEAK=check mode, setting --invert-exit-code for TEST_PASSES_SANITIZE_LEAK != true\"\n+\t\t\tinvert_exit_code=t\n \t\tfi\n+\telif test -z \"$passes_sanitize_leak\" &&\n+\t test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false\n+\tthen\n+\t\tskip_all=\"skipping $this_test under GIT_TEST_PASSING_SANITIZE_LEAK=true\"\n+\t\ttest_done\n \tfi\n \n \tif test_bool_env GIT_TEST_SANITIZE_LEAK_LOG false\n@@ -1476,7 +1493,8 @@ then\n \t\tprepend_var LSAN_OPTIONS : log_path=\\\"$TEST_RESULTS_SAN_FILE\\\"\n \t\texport LSAN_OPTIONS\n \tfi\n-elif test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false\n+elif test \"$GIT_TEST_PASSING_SANITIZE_LEAK\" = \"check\" ||\n+ test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false\n then\n \tBAIL_OUT \"GIT_TEST_PASSING_SANITIZE_LEAK=true has no effect except when compiled with SANITIZE=leak\"\n fi\n-- \n2.37.1.1062.g385eac7fccf\n\n"},{"id":"459467","messageId":"patch-06.10-ad1395f45af-20220719T205710Z-avarab@gmail.com","threadId":"58191","inReplyTo":"cover-00.10-00000000000-20220719T205710Z-avarab@gmail.com","subject":"[PATCH 06/10] test-lib: add a SANITIZE=leak logging mode","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-19T21:05:20Z","receivedAt":"2022-07-19T21:05:55Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add the ability to run the test suite under a new\n\"GIT_TEST_SANITIZE_LEAK_LOG=true\" mode, when true we'll log the leaks\nwe find an a new \"test-results/.leak\" directory.\n\nThat new path is consistent with the existing\n\"test-results/.\" results, except that those are all\nfiles, not directories.\n\nWe also set \"log_exe_name=1\" to include the name of the executable in\nthe filename. This gives us files like \"trace.git.\" instead of\nthe default of \"trace.\". I.e. we'll be able to distinguish \"git\"\nleaks from \"test-tool\", \"git-daemon\" etc.\n\nWe then set \"dedup_token_length\" to non-zero (\"0\" is the default) to\nsuccinctly log a token we can de-duplicate these stacktraces on. The\nstring is simply a one-line stack-trace with only function names up to\nN frames, which we limit at \"9999\" as a shorthand for\n\"infinite\" (there appears to be no way to say \"no limit\").\n\nWith these combined we can now easily get e.g. the top 10 leaks in the\ntest suite grouped by full stacktrace:\n\n grep -o -P -h '(?<=DEDUP_TOKEN: ).*' test-results/*.leak/trace.git.* | sort | uniq -c | sort -nr | head -n 10\n\nOr add \"grep -E -o '[^-]+'\" to that to group by functions instead of\nstack traces:\n\n grep -o -P -h '(?<=DEDUP_TOKEN: ).*' test-results/*.leak/trace.git.* | grep -E -o '[^-]+' | sort | uniq -c | sort -nr | head -n 20\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/README | 6 ++++++\n t/test-lib.sh | 21 +++++++++++++++++++++\n 2 files changed, 27 insertions(+)\n\ndiff --git a/t/README b/t/README\nindex 7f409392537..c2f996656d3 100644\n--- a/t/README\n+++ b/t/README\n@@ -371,6 +371,12 @@ declared themselves as leak-free by setting\n \"TEST_PASSES_SANITIZE_LEAK=true\" before sourcing \"test-lib.sh\". This\n test mode is used by the \"linux-leaks\" CI target.\n \n+GIT_TEST_SANITIZE_LEAK_LOG=true will log memory leaks to\n+\"test-results/$TEST_NAME.leak/trace.*\" files. Useful in combination\n+with \"GIT_TEST_PASSING_SANITIZE_LEAK\" to check if we're falsely\n+reporting a test as \"passing\" with SANITIZE=leak due to ignored exit\n+codes.\n+\n GIT_TEST_PROTOCOL_VERSION=, when set, makes 'protocol.version'\n default to n.\n \ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 7f730b0f94f..73600c4d887 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -305,6 +305,10 @@ TEST_NUMBER=\"${TEST_NAME%%-*}\"\n TEST_NUMBER=\"${TEST_NUMBER#t}\"\n TEST_RESULTS_DIR=\"$TEST_OUTPUT_DIRECTORY/test-results\"\n TEST_RESULTS_BASE=\"$TEST_RESULTS_DIR/$TEST_NAME$TEST_STRESS_JOB_SFX\"\n+TEST_RESULTS_SAN_FILE_PFX=trace\n+TEST_RESULTS_SAN_DIR_SFX=leak\n+TEST_RESULTS_SAN_FILE=\n+TEST_RESULTS_SAN_DIR=\"$TEST_RESULTS_DIR/$TEST_NAME.$TEST_RESULTS_SAN_DIR_SFX\"\n TRASH_DIRECTORY=\"trash directory.$TEST_NAME$TEST_STRESS_JOB_SFX\"\n test -n \"$root\" && TRASH_DIRECTORY=\"$root/$TRASH_DIRECTORY\"\n case \"$TRASH_DIRECTORY\" in\n@@ -1455,6 +1459,23 @@ then\n \t\t\ttest_done\n \t\tfi\n \tfi\n+\n+\tif test_bool_env GIT_TEST_SANITIZE_LEAK_LOG false\n+\tthen\n+\t\tif ! mkdir -p \"$TEST_RESULTS_SAN_DIR\"\n+\t\tthen\n+\t\t\tBAIL_OUT \"cannot create $TEST_RESULTS_SAN_DIR\"\n+\t\tfi &&\n+\t\tTEST_RESULTS_SAN_FILE=\"$TEST_RESULTS_SAN_DIR/$TEST_RESULTS_SAN_FILE_PFX\"\n+\n+\t\t# Don't litter *.leak dirs if there was nothing to report\n+\t\ttest_atexit \"rmdir \\\"$TEST_RESULTS_SAN_DIR\\\" 2>/dev/null || :\"\n+\n+\t\tprepend_var LSAN_OPTIONS : dedup_token_length=9999\n+\t\tprepend_var LSAN_OPTIONS : log_exe_name=1\n+\t\tprepend_var LSAN_OPTIONS : log_path=\\\"$TEST_RESULTS_SAN_FILE\\\"\n+\t\texport LSAN_OPTIONS\n+\tfi\n elif test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false\n then\n \tBAIL_OUT \"GIT_TEST_PASSING_SANITIZE_LEAK=true has no effect except when compiled with SANITIZE=leak\"\n-- \n2.37.1.1062.g385eac7fccf\n\n"},{"id":"459468","messageId":"patch-05.10-6f474a0f83d-20220719T205710Z-avarab@gmail.com","threadId":"58191","inReplyTo":"cover-00.10-00000000000-20220719T205710Z-avarab@gmail.com","subject":"[PATCH 05/10] t/README: reword the \"GIT_TEST_PASSING_SANITIZE_LEAK\" description","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-19T21:05:19Z","receivedAt":"2022-07-19T21:05:57Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Reword the documentation added in 956d2e4639b (tests: add a test mode\nfor SANITIZE=leak, run it in CI, 2021-09-23) for brevity, and to avoid\na merge conflict with another topic, per [1].\n\n1. https://lore.kernel.org/git/220719.86lespun6l.gmgdl@evledraar.gmail.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/README | 10 ++++------\n 1 file changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 4f9981cf5e3..7f409392537 100644\n--- a/t/README\n+++ b/t/README\n@@ -366,12 +366,10 @@ excluded as so much relies on it, but this might change in the future.\n GIT_TEST_SPLIT_INDEX= forces split-index mode on the whole\n test suite. Accept any boolean values that are accepted by git-config.\n \n-GIT_TEST_PASSING_SANITIZE_LEAK= when compiled with\n-SANITIZE=leak will run only those tests that have whitelisted\n-themselves as passing with no memory leaks. Tests can be whitelisted\n-by setting \"TEST_PASSES_SANITIZE_LEAK=true\" before sourcing\n-\"test-lib.sh\" itself at the top of the test script. This test mode is\n-used by the \"linux-leaks\" CI target.\n+GIT_TEST_PASSING_SANITIZE_LEAK=true skips those tests that haven't\n+declared themselves as leak-free by setting\n+\"TEST_PASSES_SANITIZE_LEAK=true\" before sourcing \"test-lib.sh\". This\n+test mode is used by the \"linux-leaks\" CI target.\n \n GIT_TEST_PROTOCOL_VERSION=, when set, makes 'protocol.version'\n default to n.\n-- \n2.37.1.1062.g385eac7fccf\n\n"},{"id":"459469","messageId":"patch-09.10-b75b93822e1-20220719T205710Z-avarab@gmail.com","threadId":"58191","inReplyTo":"cover-00.10-00000000000-20220719T205710Z-avarab@gmail.com","subject":"[PATCH 09/10] leak tests: mark passing SANITIZE=leak tests as leak-free","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-19T21:05:23Z","receivedAt":"2022-07-19T21:06:08Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Mark those remaining tests that pass when run under SANITIZE=leak with\nTEST_PASSES_SANITIZE_LEAK=true, these were either omitted in\nf346fcb62a0 (Merge branch 'ab/mark-leak-free-tests-even-more',\n2021-12-15) and 5a4f8381b68 (Merge branch 'ab/mark-leak-free-tests',\n2021-10-25), or have had their memory leaks fixed since then.\n\nWith this change there's now a a 1=1 mapping between those tests that\nwe have opted-in via \"TEST_PASSES_SANITIZE_LEAK=true\", and those that\npass with the new \"check\" mode:\n\n GIT_TEST_PASSING_SANITIZE_LEAK=check GIT_TEST_SANITIZE_LEAK_LOG=true make test SANITIZE=leak\n\nNote that the \"GIT_TEST_SANITIZE_LEAK_LOG=true\" is needed due to the\nedge cases noted in a preceding commit, i.e. in some cases we'd pass\nthe test itself, but still have outstanding leaks due to ignored exit\ncodes.\n\nThe \"GIT_TEST_SANITIZE_LEAK_LOG=true\" corrects for that, we're only\nmarking those tests as passing that really don't have any leaks,\nwhether that was reflected in their exit code or not.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t0027-auto-crlf.sh | 1 +\n t/t0032-reftable-unittest.sh | 1 +\n t/t0033-safe-directory.sh | 1 +\n t/t0050-filesystem.sh | 1 +\n t/t0095-bloom.sh | 2 ++\n t/t1405-main-ref-store.sh | 1 +\n t/t1407-worktree-ref-store.sh | 1 +\n t/t1418-reflog-exists.sh | 1 +\n t/t1701-racy-split-index.sh | 1 +\n t/t2006-checkout-index-basic.sh | 1 +\n t/t2023-checkout-m.sh | 1 +\n t/t2205-add-worktree-config.sh | 1 +\n t/t3012-ls-files-dedup.sh | 1 +\n t/t4017-diff-retval.sh | 1 +\n t/t4051-diff-function-context.sh | 1 +\n t/t4057-diff-combined-paths.sh | 1 +\n t/t4114-apply-typechange.sh | 1 +\n t/t4301-merge-tree-write-tree.sh | 1 +\n t/t5315-pack-objects-compression.sh | 1 +\n t/t5351-unpack-large-objects.sh | 1 +\n t/t5402-post-merge-hook.sh | 1 +\n t/t5503-tagfollow.sh | 1 +\n t/t6404-recursive-merge.sh | 1 +\n t/t6405-merge-symlinks.sh | 1 +\n t/t6408-merge-up-to-date.sh | 1 +\n t/t6411-merge-filemode.sh | 1 +\n t/t6413-merge-crlf.sh | 1 +\n t/t6415-merge-dir-to-symlink.sh | 1 +\n t/t6425-merge-rename-delete.sh | 1 +\n t/t6431-merge-criscross.sh | 1 +\n t/t7060-wtstatus.sh | 1 +\n t/t7062-wtstatus-ignorecase.sh | 1 +\n t/t7110-reset-merge.sh | 1 +\n t/t7111-reset-table.sh | 1 +\n t/t7609-mergetool--lib.sh | 1 +\n t/t9100-git-svn-basic.sh | 1 -\n t/t9901-git-web--browse.sh | 1 +\n 37 files changed, 37 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh\nindex 7f80f463930..a22e0e1382c 100755\n--- a/t/t0027-auto-crlf.sh\n+++ b/t/t0027-auto-crlf.sh\n@@ -2,6 +2,7 @@\n \n test_description='CRLF conversion all combinations'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n compare_files () {\ndiff --git a/t/t0032-reftable-unittest.sh b/t/t0032-reftable-unittest.sh\nindex 0ed14971a58..471cb37ac28 100755\n--- a/t/t0032-reftable-unittest.sh\n+++ b/t/t0032-reftable-unittest.sh\n@@ -5,6 +5,7 @@\n \n test_description='reftable unittests'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'unittests' '\ndiff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh\nindex 3908597d42d..7db3344092d 100755\n--- a/t/t0033-safe-directory.sh\n+++ b/t/t0033-safe-directory.sh\n@@ -2,6 +2,7 @@\n \n test_description='verify safe.directory checks'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n GIT_TEST_ASSUME_DIFFERENT_OWNER=1\ndiff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh\nindex 5c9dc90d0b0..325eb1c3cd0 100755\n--- a/t/t0050-filesystem.sh\n+++ b/t/t0050-filesystem.sh\n@@ -5,6 +5,7 @@ test_description='Various filesystem issues'\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 auml=$(printf '\\303\\244')\ndiff --git a/t/t0095-bloom.sh b/t/t0095-bloom.sh\nindex daeb4a5e3e7..b567383eb83 100755\n--- a/t/t0095-bloom.sh\n+++ b/t/t0095-bloom.sh\n@@ -1,6 +1,8 @@\n #!/bin/sh\n \n test_description='Testing the various Bloom filter computations in bloom.c'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'compute unseeded murmur3 hash for empty string' '\ndiff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh\nindex 51f82916281..e4627cf1b61 100755\n--- a/t/t1405-main-ref-store.sh\n+++ b/t/t1405-main-ref-store.sh\n@@ -5,6 +5,7 @@ test_description='test main ref store api'\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 RUN=\"test-tool ref-store main\"\ndiff --git a/t/t1407-worktree-ref-store.sh b/t/t1407-worktree-ref-store.sh\nindex ad8006c8139..05b1881c591 100755\n--- a/t/t1407-worktree-ref-store.sh\n+++ b/t/t1407-worktree-ref-store.sh\n@@ -5,6 +5,7 @@ test_description='test worktree ref store api'\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 RWT=\"test-tool ref-store worktree:wt\"\ndiff --git a/t/t1418-reflog-exists.sh b/t/t1418-reflog-exists.sh\nindex d51ecd5e925..2268bca3c11 100755\n--- a/t/t1418-reflog-exists.sh\n+++ b/t/t1418-reflog-exists.sh\n@@ -4,6 +4,7 @@ test_description='Test reflog display routines'\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 test_expect_success 'setup' '\ndiff --git a/t/t1701-racy-split-index.sh b/t/t1701-racy-split-index.sh\nindex 5dc221ef382..d8fa489998a 100755\n--- a/t/t1701-racy-split-index.sh\n+++ b/t/t1701-racy-split-index.sh\n@@ -5,6 +5,7 @@\n \n test_description='racy split index'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\ndiff --git a/t/t2006-checkout-index-basic.sh b/t/t2006-checkout-index-basic.sh\nindex 7705e3a3170..5d119871d41 100755\n--- a/t/t2006-checkout-index-basic.sh\n+++ b/t/t2006-checkout-index-basic.sh\n@@ -3,6 +3,7 @@\n test_description='basic checkout-index tests\n '\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'checkout-index --gobbledegook' '\ndiff --git a/t/t2023-checkout-m.sh b/t/t2023-checkout-m.sh\nindex 7b327b75449..81e772fb4eb 100755\n--- a/t/t2023-checkout-m.sh\n+++ b/t/t2023-checkout-m.sh\n@@ -7,6 +7,7 @@ Ensures that checkout -m on a resolved file restores the conflicted file'\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 test_expect_success setup '\ndiff --git a/t/t2205-add-worktree-config.sh b/t/t2205-add-worktree-config.sh\nindex 43d950de640..98265ba1b49 100755\n--- a/t/t2205-add-worktree-config.sh\n+++ b/t/t2205-add-worktree-config.sh\n@@ -17,6 +17,7 @@ outside the repository. Two instances for which this can occur are tested:\n \t repository can be added to the index.\n \t'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success '1a: setup--config worktree' '\ndiff --git a/t/t3012-ls-files-dedup.sh b/t/t3012-ls-files-dedup.sh\nindex 2682b1f43a6..190e2f6eed7 100755\n--- a/t/t3012-ls-files-dedup.sh\n+++ b/t/t3012-ls-files-dedup.sh\n@@ -2,6 +2,7 @@\n \n test_description='git ls-files --deduplicate test'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup' '\ndiff --git a/t/t4017-diff-retval.sh b/t/t4017-diff-retval.sh\nindex ed461f481e2..5bc28ad9f04 100755\n--- a/t/t4017-diff-retval.sh\n+++ b/t/t4017-diff-retval.sh\n@@ -5,6 +5,7 @@ test_description='Return value of diffs'\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 test_expect_success 'setup' '\ndiff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh\nindex 4838a1df8b4..725278ad19c 100755\n--- a/t/t4051-diff-function-context.sh\n+++ b/t/t4051-diff-function-context.sh\n@@ -2,6 +2,7 @@\n \n test_description='diff function context'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n dir=\"$TEST_DIRECTORY/t4051\"\ndiff --git a/t/t4057-diff-combined-paths.sh b/t/t4057-diff-combined-paths.sh\nindex 04b8a1542a8..9a7505cbb8b 100755\n--- a/t/t4057-diff-combined-paths.sh\n+++ b/t/t4057-diff-combined-paths.sh\n@@ -5,6 +5,7 @@ test_description='combined diff show only paths that are different to all parent\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 # verify that diffc.expect matches output of\ndiff --git a/t/t4114-apply-typechange.sh b/t/t4114-apply-typechange.sh\nindex da3e64f8110..8ff36407667 100755\n--- a/t/t4114-apply-typechange.sh\n+++ b/t/t4114-apply-typechange.sh\n@@ -7,6 +7,7 @@ test_description='git apply should not get confused with type changes.\n \n '\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'setup repository and commits' '\ndiff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh\nindex f091259a55e..a243e3c5176 100755\n--- a/t/t4301-merge-tree-write-tree.sh\n+++ b/t/t4301-merge-tree-write-tree.sh\n@@ -2,6 +2,7 @@\n \n test_description='git merge-tree --write-tree'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # This test is ort-specific\ndiff --git a/t/t5315-pack-objects-compression.sh b/t/t5315-pack-objects-compression.sh\nindex 8bacd96275b..c80ea9e8b71 100755\n--- a/t/t5315-pack-objects-compression.sh\n+++ b/t/t5315-pack-objects-compression.sh\n@@ -2,6 +2,7 @@\n \n test_description='pack-object compression configuration'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success setup '\ndiff --git a/t/t5351-unpack-large-objects.sh b/t/t5351-unpack-large-objects.sh\nindex 8ce8aa3b147..b1703350be3 100755\n--- a/t/t5351-unpack-large-objects.sh\n+++ b/t/t5351-unpack-large-objects.sh\n@@ -5,6 +5,7 @@\n \n test_description='git unpack-objects with large objects'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n prepare_dest () {\ndiff --git a/t/t5402-post-merge-hook.sh b/t/t5402-post-merge-hook.sh\nindex 915af2de95e..46ebdfbeeba 100755\n--- a/t/t5402-post-merge-hook.sh\n+++ b/t/t5402-post-merge-hook.sh\n@@ -7,6 +7,7 @@ test_description='Test the post-merge hook.'\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 test_expect_success setup '\ndiff --git a/t/t5503-tagfollow.sh b/t/t5503-tagfollow.sh\nindex 195fc64dd44..5ebbaa48968 100755\n--- a/t/t5503-tagfollow.sh\n+++ b/t/t5503-tagfollow.sh\n@@ -5,6 +5,7 @@ test_description='test automatic tag following'\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 # End state of the repository:\ndiff --git a/t/t6404-recursive-merge.sh b/t/t6404-recursive-merge.sh\nindex b8735c6db4d..36215518b6e 100755\n--- a/t/t6404-recursive-merge.sh\n+++ b/t/t6404-recursive-merge.sh\n@@ -4,6 +4,7 @@ test_description='Test merge without common ancestors'\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 # This scenario is based on a real-world repository of Shawn Pearce.\ndiff --git a/t/t6405-merge-symlinks.sh b/t/t6405-merge-symlinks.sh\nindex 7435fce71e0..29e2b25ce5d 100755\n--- a/t/t6405-merge-symlinks.sh\n+++ b/t/t6405-merge-symlinks.sh\n@@ -11,6 +11,7 @@ if core.symlinks is false.'\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 test_expect_success 'setup' '\ndiff --git a/t/t6408-merge-up-to-date.sh b/t/t6408-merge-up-to-date.sh\nindex 7763c1ba980..8a1ba6d23a7 100755\n--- a/t/t6408-merge-up-to-date.sh\n+++ b/t/t6408-merge-up-to-date.sh\n@@ -2,6 +2,7 @@\n \n test_description='merge fast-forward and up to date'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success setup '\ndiff --git a/t/t6411-merge-filemode.sh b/t/t6411-merge-filemode.sh\nindex 6ae2489286c..b6182723aae 100755\n--- a/t/t6411-merge-filemode.sh\n+++ b/t/t6411-merge-filemode.sh\n@@ -4,6 +4,7 @@ test_description='merge: handle file mode'\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 test_expect_success 'set up mode change in one branch' '\ndiff --git a/t/t6413-merge-crlf.sh b/t/t6413-merge-crlf.sh\nindex affea255fe9..b4f4a313f48 100755\n--- a/t/t6413-merge-crlf.sh\n+++ b/t/t6413-merge-crlf.sh\n@@ -11,6 +11,7 @@ test_description='merge conflict in crlf repo\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 test_expect_success setup '\ndiff --git a/t/t6415-merge-dir-to-symlink.sh b/t/t6415-merge-dir-to-symlink.sh\nindex 2655e295f5a..ae00492c768 100755\n--- a/t/t6415-merge-dir-to-symlink.sh\n+++ b/t/t6415-merge-dir-to-symlink.sh\n@@ -4,6 +4,7 @@ test_description='merging when a directory was replaced with a symlink'\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 test_expect_success 'create a commit where dir a/b changed to symlink' '\ndiff --git a/t/t6425-merge-rename-delete.sh b/t/t6425-merge-rename-delete.sh\nindex 459b431a60d..93cd2869b12 100755\n--- a/t/t6425-merge-rename-delete.sh\n+++ b/t/t6425-merge-rename-delete.sh\n@@ -4,6 +4,7 @@ test_description='Merge-recursive rename/delete conflict message'\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 test_expect_success 'rename/delete' '\ndiff --git a/t/t6431-merge-criscross.sh b/t/t6431-merge-criscross.sh\nindex 3824756a02e..3fe14cd73e8 100755\n--- a/t/t6431-merge-criscross.sh\n+++ b/t/t6431-merge-criscross.sh\n@@ -2,6 +2,7 @@\n \n test_description='merge-recursive backend test'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n # A <- create some files\ndiff --git a/t/t7060-wtstatus.sh b/t/t7060-wtstatus.sh\nindex 0f4344c55e6..aaeb4a53344 100755\n--- a/t/t7060-wtstatus.sh\n+++ b/t/t7060-wtstatus.sh\n@@ -5,6 +5,7 @@ test_description='basic work tree status reporting'\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 test_expect_success setup '\ndiff --git a/t/t7062-wtstatus-ignorecase.sh b/t/t7062-wtstatus-ignorecase.sh\nindex 73709dbeee2..caf372a3d42 100755\n--- a/t/t7062-wtstatus-ignorecase.sh\n+++ b/t/t7062-wtstatus-ignorecase.sh\n@@ -2,6 +2,7 @@\n \n test_description='git-status with core.ignorecase=true'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'status with hash collisions' '\ndiff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh\nindex 3d62e10b53f..eb881be95b6 100755\n--- a/t/t7110-reset-merge.sh\n+++ b/t/t7110-reset-merge.sh\n@@ -5,6 +5,7 @@\n \n test_description='Tests for \"git reset\" with \"--merge\" and \"--keep\" options'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success setup '\ndiff --git a/t/t7111-reset-table.sh b/t/t7111-reset-table.sh\nindex ce421ad5ac4..78f25c1c7ea 100755\n--- a/t/t7111-reset-table.sh\n+++ b/t/t7111-reset-table.sh\n@@ -5,6 +5,7 @@\n \n test_description='Tests to check that \"reset\" options follow a known table'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n \ndiff --git a/t/t7609-mergetool--lib.sh b/t/t7609-mergetool--lib.sh\nindex 330d6d603d7..8b1c3bd39f2 100755\n--- a/t/t7609-mergetool--lib.sh\n+++ b/t/t7609-mergetool--lib.sh\n@@ -4,6 +4,7 @@ test_description='git mergetool\n \n Testing basic merge tools options'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'mergetool --tool=vimdiff creates the expected layout' '\ndiff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh\nindex 7c5b847f584..fea41b3c360 100755\n--- a/t/t9100-git-svn-basic.sh\n+++ b/t/t9100-git-svn-basic.sh\n@@ -8,7 +8,6 @@ test_description='git svn basic tests'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n-TEST_FAILS_SANITIZE_LEAK=true\n . ./lib-git-svn.sh\n \n prepare_utf8_locale\ndiff --git a/t/t9901-git-web--browse.sh b/t/t9901-git-web--browse.sh\nindex de7152f8271..19f56e5680f 100755\n--- a/t/t9901-git-web--browse.sh\n+++ b/t/t9901-git-web--browse.sh\n@@ -5,6 +5,7 @@ test_description='git web--browse basic tests\n \n This test checks that git web--browse can handle various valid URLs.'\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_web_browse () {\n-- \n2.37.1.1062.g385eac7fccf\n\n"},{"id":"459470","messageId":"patch-08.10-afbb7e19195-20220719T205710Z-avarab@gmail.com","threadId":"58191","inReplyTo":"cover-00.10-00000000000-20220719T205710Z-avarab@gmail.com","subject":"[PATCH 08/10] test-lib: have the \"check\" mode for SANITIZE=leak consider leak logs","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-19T21:05:22Z","receivedAt":"2022-07-19T21:06:09Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"As noted in previous on-list discussions[1] we have various tests that\nwill falsely report being leak-free because we're missing the relevant\nexit code from LSAN as summarized below.\n\nWe should fix those issues, but in the meantime and as an additional\nsanity check we can and should consider our own ASAN logs before\nreporting that a test is leak-free.\n\nBefore this compiling with SANITIZE=leak and running:\n\n ./t4058-diff-duplicates.sh\n\nWill exit successfully, now we'll get an error and an informative\nmessage on:\n\n GIT_TEST_SANITIZE_LEAK_LOG=true ./t4058-diff-duplicates.sh\n\nAnd even more useful, we'll now either error or exit successfully on\nthis command, depending on whether or not the test has labeled itself\nleak-free with TEST_PASSES_SANITIZE_LEAK=true or not.\n\n GIT_TEST_SANITIZE_LEAK_LOG=true GIT_TEST_PASSING_SANITIZE_LEAK=check ./t4058-diff-duplicates.sh\n\nWhy do we miss these leaks in the first place? As initially noted in\n[1] (and discussed downthread) the reasons are:\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 Notably a command that e.g. might invoke \"git pack-objects\" might\n itself exit with status 128 if that \"pack-objects\" segfaults or\n abort()'s. If the test invoking the parent command(s) is using\n \"test_must_fail\" we'll consider it an expected \"ok\" failure.\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, or simply ignoring the exit codes(s)\n entirely, e.g. these invocations in git-merge-one-file.sh leak, but\n aren't reflected in the \"git merge\" exit code:\n\n\tsrc1=$(git unpack-file $2)\n\tsrc2=$(git unpack-file $3)\n\n That case would be easily \"fixed\" by adding a line like this after\n each assignment:\n\n\ttest $? -ne 0 && exit $?\n\n But we'd then in e.g. \"t6407-merge-binary.sh\" run into\n write_tree_trivial() in \"builtin/merge.c\" calling die() instead of\n ferrying up the relevant exit code.\n\nLet's remove \"TEST_PASSES_SANITIZE_LEAK=true\" from the one test we\nwere falsely marking as leak-free, marked as such in my\n9081a421a6d (checkout: fix \"branch info\" memory leaks,\n2021-11-16). I'd previously removed other bad\n\"TEST_PASSES_SANITIZE_LEAK=true\" opt-ins in the series merged in\nea05fd5fbf7 (Merge branch 'ab/keep-git-exit-codes-in-tests',\n2022-03-16).\n\n1. https://lore.kernel.org/git/cover-00.15-00000000000-20220302T171755Z-avarab@gmail.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/README | 10 ++++++\n t/t6407-merge-binary.sh | 1 -\n t/test-lib.sh | 72 +++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 82 insertions(+), 1 deletion(-)\n\ndiff --git a/t/README b/t/README\nindex 7b7082386ae..fa16822cde9 100644\n--- a/t/README\n+++ b/t/README\n@@ -381,12 +381,22 @@ without errors a failure (by providing \"--invert-exit-code\"). Thus the\n there's a 1=1 mapping between \"TEST_PASSES_SANITIZE_LEAK=true\" and\n those tests that pass under \"SANITIZE=leak\".\n \n+The \"check\" mode is especially useful if combined with\n+GIT_TEST_SANITIZE_LEAK_LOG=true.\n+\n GIT_TEST_SANITIZE_LEAK_LOG=true will log memory leaks to\n \"test-results/$TEST_NAME.leak/trace.*\" files. Useful in combination\n with \"GIT_TEST_PASSING_SANITIZE_LEAK\" to check if we're falsely\n reporting a test as \"passing\" with SANITIZE=leak due to ignored exit\n codes.\n \n+When GIT_TEST_SANITIZE_LEAK_LOG=true is set we'll look at the\n+\"test-results/$TEST_NAME.leak/trace.*\" files at the end of the test\n+run in combination with the \"TEST_PASSES_SANITIZE_LEAK\" and\n+GIT_TEST_PASSING_SANITIZE_LEAK=check setting to see if we'll fail a\n+test leaked, but which the test run itself didn't catch due to ignored\n+or missed exit codes.\n+\n GIT_TEST_PROTOCOL_VERSION=, when set, makes 'protocol.version'\n default to n.\n \ndiff --git a/t/t6407-merge-binary.sh b/t/t6407-merge-binary.sh\nindex 0753fc95f45..e8a28717cec 100755\n--- a/t/t6407-merge-binary.sh\n+++ b/t/t6407-merge-binary.sh\n@@ -5,7 +5,6 @@ test_description='ask merge-recursive to merge binary files'\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 test_expect_success setup '\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 747bf6c50e5..a670c8808b1 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -309,6 +309,7 @@ TEST_RESULTS_SAN_FILE_PFX=trace\n TEST_RESULTS_SAN_DIR_SFX=leak\n TEST_RESULTS_SAN_FILE=\n TEST_RESULTS_SAN_DIR=\"$TEST_RESULTS_DIR/$TEST_NAME.$TEST_RESULTS_SAN_DIR_SFX\"\n+TEST_RESULTS_SAN_DIR_NR_LEAKS_STARTUP=\n TRASH_DIRECTORY=\"trash directory.$TEST_NAME$TEST_STRESS_JOB_SFX\"\n test -n \"$root\" && TRASH_DIRECTORY=\"$root/$TRASH_DIRECTORY\"\n case \"$TRASH_DIRECTORY\" in\n@@ -316,6 +317,16 @@ case \"$TRASH_DIRECTORY\" in\n *) TRASH_DIRECTORY=\"$TEST_OUTPUT_DIRECTORY/$TRASH_DIRECTORY\" ;;\n esac\n \n+# Utility functions using $TEST_RESULTS_* variables\n+nr_san_dir_leaks_ () {\n+\t# stderr piped to /dev/null because the directory may have\n+\t# been \"rmdir\"'d already.\n+\tfind \"$TEST_RESULTS_SAN_DIR\" \\\n+\t\t-type f \\\n+\t\t-name \"$TEST_RESULTS_SAN_FILE_PFX.*\" 2>/dev/null |\n+\twc -l\n+}\n+\n # If --stress was passed, run this test repeatedly in several parallel loops.\n if test \"$GIT_TEST_STRESS_STARTED\" = \"done\"\n then\n@@ -1191,6 +1202,58 @@ test_atexit_handler () {\n \tteardown_malloc_check\n }\n \n+sanitize_leak_log_message_ () {\n+\tlocal new=\"$1\" &&\n+\tlocal old=\"$2\" &&\n+\tlocal file=\"$3\" &&\n+\n+\tprintf \"With SANITIZE=leak at exit we have %d leak logs, but started with %d\n+\n+This means that we have a blindspot where git is leaking but we're\n+losing the exit code somewhere, or not propagating it appropriately\n+upwards!\n+\n+See the logs at \\\"%s.*\\\"\" \\\n+\t \"$new\" \"$old\" \"$file\"\n+}\n+\n+check_test_results_san_file_ () {\n+\tif test -z \"$TEST_RESULTS_SAN_FILE\"\n+\tthen\n+\t\treturn\n+\tfi\n+\tlocal old=\"$TEST_RESULTS_SAN_DIR_NR_LEAKS_STARTUP\" &&\n+\tlocal new=\"$(nr_san_dir_leaks_)\" &&\n+\n+\tif test $new -le $old\n+\tthen\n+\t\treturn\n+\tfi\n+\tlocal out=\"$(sanitize_leak_log_message_ \"$new\" \"$old\" \"$TEST_RESULTS_SAN_FILE\")\" &&\n+\tsay_color error \"$out\" &&\n+\n+\tif test -n \"$passes_sanitize_leak\" && test \"$test_failure\" = 0\n+\tthen\n+\t\tsay \"As TEST_PASSES_SANITIZE_LEAK=true and our logs show we're leaking, exit non-zero!\"\n+\t\tinvert_exit_code=t\n+\telif test -n \"$passes_sanitize_leak\"\n+\tthen\n+\t\tsay \"As TEST_PASSES_SANITIZE_LEAK=true and our logs show we're leaking, and we're failing for other reasons too...\"\n+\t\tinvert_exit_code=\n+\telif test -n \"$sanitize_leak_check\" && test \"$test_failure\" = 0\n+\tthen\n+\t\tsay \"As TEST_PASSES_SANITIZE_LEAK=true isn't set the above leak is 'ok' with GIT_TEST_PASSING_SANITIZE_LEAK=check\"\n+\t\tinvert_exit_code=\n+\telif test -n \"$sanitize_leak_check\"\n+\tthen\n+\t\tsay \"As TEST_PASSES_SANITIZE_LEAK=true isn't set the above leak is 'ok' with GIT_TEST_PASSING_SANITIZE_LEAK=check\"\n+\t\tinvert_exit_code=t\n+\telse\n+\t\tsay \"With GIT_TEST_SANITIZE_LEAK_LOG=true our logs revealed a memory leak, exit non-zero!\"\n+\t\tinvert_exit_code=t\n+\tfi\n+}\n+\n test_done () {\n \t# Run the atexit commands _before_ the trash directory is\n \t# removed, so the commands can access pidfiles and socket files.\n@@ -1266,6 +1329,8 @@ test_done () {\n \t\t\terror \"Tests passed but test cleanup failed; aborting\"\n \t\tfi\n \n+\t\tcheck_test_results_san_file_ \"$test_failure\"\n+\n \t\tif test -z \"$skip_all\" && test -n \"$invert_exit_code\"\n \t\tthen\n \t\t\tsay_color warn \"# faking up non-zero exit with --invert-exit-code\"\n@@ -1279,6 +1344,8 @@ test_done () {\n \t\texit 0 ;;\n \n \t*)\n+\t\tcheck_test_results_san_file_ \"$test_failure\"\n+\n \t\tif test $test_external_has_tap -eq 0\n \t\tthen\n \t\t\tsay_color error \"# failed $test_failure among $msg\"\n@@ -1460,6 +1527,7 @@ then\n \n \tif test \"$GIT_TEST_PASSING_SANITIZE_LEAK\" = \"check\"\n \tthen\n+\t\tsanitize_leak_check=t\n \t\tif test -n \"$invert_exit_code\"\n \t\tthen\n \t\t\tBAIL_OUT \"cannot use --invert-exit-code under GIT_TEST_PASSING_SANITIZE_LEAK=check\"\n@@ -1485,6 +1553,10 @@ then\n \t\tfi &&\n \t\tTEST_RESULTS_SAN_FILE=\"$TEST_RESULTS_SAN_DIR/$TEST_RESULTS_SAN_FILE_PFX\"\n \n+\t\t# In case \"test-results\" is left over from a previous\n+\t\t# run: Only report if new leaks show up.\n+\t\tTEST_RESULTS_SAN_DIR_NR_LEAKS_STARTUP=$(nr_san_dir_leaks_)\n+\n \t\t# Don't litter *.leak dirs if there was nothing to report\n \t\ttest_atexit \"rmdir \\\"$TEST_RESULTS_SAN_DIR\\\" 2>/dev/null || :\"\n \n-- \n2.37.1.1062.g385eac7fccf\n\n"},{"id":"459471","messageId":"patch-10.10-9cedf0cb0e2-20220719T205710Z-avarab@gmail.com","threadId":"58191","inReplyTo":"cover-00.10-00000000000-20220719T205710Z-avarab@gmail.com","subject":"[PATCH 10/10] log tests: don't use \"exit 1\" outside a sub-shell","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-19T21:05:24Z","receivedAt":"2022-07-19T21:06:11Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change an \"exit 1\" added in ac52d9410e5 (t4205: cover `git log\n--reflog -z` blindspot, 2019-11-19) to use \"return 1\" instead, which\ncuriously was done in an adjacent test case added in the same commit.\n\nUsing \"exit 1\" outside a sub-shell will cause the test framework\nitself to exit on failure, which isn't what we want to do here.\n\nThis issue was spotted with the new\n\"GIT_TEST_PASSING_SANITIZE_LEAK=check\" mode, i.e. that \"git show\"\ncommand leaks memory, and we'd thus \"exit 1\". Another implementation\nof \"GIT_TEST_PASSING_SANITIZE_LEAK=check\" or \"--invert-exit-code\"\nmight have intercepted the \"exit 1\", and thus hidden the underlying\nissue here, but we correctly distinguish the two.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t4205-log-pretty-formats.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex e448ef2928a..0404491d6ee 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -156,7 +156,7 @@ test_expect_success 'NUL termination with --reflog --pretty=oneline' '\n \tfor r in $revs\n \tdo\n \t\tgit show -s --pretty=oneline \"$r\" >raw &&\n-\t\tcat raw | lf_to_nul || exit 1\n+\t\tcat raw | lf_to_nul || return 1\n \tdone >expect &&\n \t# the trailing NUL is already produced so we do not need to\n \t# output another one\n-- \n2.37.1.1062.g385eac7fccf\n\n"},{"id":"459490","messageId":"bb1c7669-4d17-66e7-1b6f-2ac504516587@github.com","threadId":"58191","inReplyTo":"patch-05.10-6f474a0f83d-20220719T205710Z-avarab@gmail.com","subject":"Re: [PATCH 05/10] t/README: reword the \"GIT_TEST_PASSING_SANITIZE_LEAK\" description","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-07-20T01:38:06Z","receivedAt":"2022-07-20T01:42:21Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 7/19/2022 5:05 PM, Ævar Arnfjörð Bjarmason wrote:\n> Reword the documentation added in 956d2e4639b (tests: add a test mode\n> for SANITIZE=leak, run it in CI, 2021-09-23) for brevity, and to avoid\n> a merge conflict with another topic, per [1].\n> \n> 1. https://lore.kernel.org/git/220719.86lespun6l.gmgdl@evledraar.gmail.com/\n\n> +GIT_TEST_PASSING_SANITIZE_LEAK=true skips those tests that haven't\n> +declared themselves as leak-free by setting\n> +\"TEST_PASSES_SANITIZE_LEAK=true\" before sourcing \"test-lib.sh\". This\n> +test mode is used by the \"linux-leaks\" CI target.\n\nThanks. This paragraph reads very well.\n\n-Stolee\n"},{"id":"459493","messageId":"7e31432c-4edb-ca95-bcc2-1aa1f20a246c@github.com","threadId":"58191","inReplyTo":"patch-07.10-0961df2ab6c-20220719T205710Z-avarab@gmail.com","subject":"Re: [PATCH 07/10] test-lib.sh: add a GIT_TEST_PASSING_SANITIZE_LEAK=check mode","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-07-20T01:43:10Z","receivedAt":"2022-07-20T01:43:41Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 7/19/2022 5:05 PM, Ævar Arnfjörð Bjarmason wrote:\n\n> +GIT_TEST_PASSING_SANITIZE_LEAK=check checks that our\n> +\"TEST_PASSES_SANITIZE_LEAK=true\" markings are current. The \"check\" is\n> +particularly useful with \"--immediate\", but otherwise acts the same\n> +for tests that have \"TEST_PASSES_SANITIZE_LEAK=true\" set. For those\n> +that don't have it set it runs them, and considers them passing\n\nnit: \"...that don't have it set it runs them\" the two uses of \"it\" are\nconfusing here. Perhaps...\n\n In this mode, run the tests without TEST_PASSES_SANITIZE_LEAK=true\n and consider the test passing without errors as a failure...\n\n> +without errors a failure (by providing \"--invert-exit-code\"). Thus the\n> +\"check\" mode can be used e.g. with \"git rebase --exec\" to ensure that\n> +there's a 1=1 mapping between \"TEST_PASSES_SANITIZE_LEAK=true\" and\n> +those tests that pass under \"SANITIZE=leak\".\n\nnit: \"1:1\" is a better shortening of \"one-to-one\", but I'd use the\nlatter anyway.\n\nThanks,\n-Stolee\n"},{"id":"459494","messageId":"93e76ab0-925d-3596-ae86-b5fbd8349555@github.com","threadId":"58191","inReplyTo":"patch-08.10-afbb7e19195-20220719T205710Z-avarab@gmail.com","subject":"Re: [PATCH 08/10] test-lib: have the \"check\" mode for SANITIZE=leak consider leak logs","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-07-20T01:47:16Z","receivedAt":"2022-07-20T01:47:25Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 7/19/2022 5:05 PM, Ævar Arnfjörð Bjarmason wrote:\n\n> \n> +The \"check\" mode is especially useful if combined with\n> +GIT_TEST_SANITIZE_LEAK_LOG=true.\n> +\n> GIT_TEST_SANITIZE_LEAK_LOG=true will log memory leaks to\n> \"test-results/$TEST_NAME.leak/trace.*\" files. Useful in combination\n> with \"GIT_TEST_PASSING_SANITIZE_LEAK\" to check if we're falsely\n> reporting a test as \"passing\" with SANITIZE=leak due to ignored exit\n> codes.\n> \n> +When GIT_TEST_SANITIZE_LEAK_LOG=true is set we'll look at the\n> +\"test-results/$TEST_NAME.leak/trace.*\" files at the end of the test\n> +run in combination with the \"TEST_PASSES_SANITIZE_LEAK\" and\n> +GIT_TEST_PASSING_SANITIZE_LEAK=check setting to see if we'll fail a\n> +test leaked, but which the test run itself didn't catch due to ignored\n> +or missed exit codes.\n\nIs this paragraph about when GIT_TEST_SANITIZE_LEAK_LOG=true _and_\nGIT_TEST_PASSING_SANITIZE_LEAK=check are set? Or, when ...LEAK_LOG=true\nwe set GIT_TEST_PASSING_SANITIZE_LEAK=check at the end? If it's the\nfirst, then I think the paragraph should lead with that.\n\nThis paragraph is also a single sentence. I lose myself in it\nhalfway through. Please break it up to add clarity.\n\nThanks,\n-Stolee\n"},{"id":"459495","messageId":"fa01e4af-3744-e719-6916-4d7776da3d9a@github.com","threadId":"58191","inReplyTo":"patch-09.10-b75b93822e1-20220719T205710Z-avarab@gmail.com","subject":"Re: [PATCH 09/10] leak tests: mark passing SANITIZE=leak tests as leak-free","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-07-20T01:50:57Z","receivedAt":"2022-07-20T01:51:06Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 7/19/2022 5:05 PM, Ævar Arnfjörð Bjarmason wrote:\n> Mark those remaining tests that pass when run under SANITIZE=leak with\n> TEST_PASSES_SANITIZE_LEAK=true, these were either omitted in\n> f346fcb62a0 (Merge branch 'ab/mark-leak-free-tests-even-more',\n> 2021-12-15) and 5a4f8381b68 (Merge branch 'ab/mark-leak-free-tests',\n> 2021-10-25), or have had their memory leaks fixed since then.\n> \n> With this change there's now a a 1=1 mapping between those tests that\n\nnit: here's another use of \"1=1\" which I read as \"one equals one\" and\nnot \"one-to-one\", so please expand into the words.\n\n> we have opted-in via \"TEST_PASSES_SANITIZE_LEAK=true\", and those that\n> pass with the new \"check\" mode:\n> \n> GIT_TEST_PASSING_SANITIZE_LEAK=check GIT_TEST_SANITIZE_LEAK_LOG=true make test SANITIZE=leak\n\nMaybe split this line?\n\n\tGIT_TEST_PASSING_SANITIZE_LEAK=check \\\n\t\tGIT_TEST_SANITIZE_LEAK_LOG=true \\\n\t\tmake test SANITIZE=leak\n\n> Note that the \"GIT_TEST_SANITIZE_LEAK_LOG=true\" is needed due to the\n> edge cases noted in a preceding commit, i.e. in some cases we'd pass\n> the test itself, but still have outstanding leaks due to ignored exit\n> codes.\n> \n> The \"GIT_TEST_SANITIZE_LEAK_LOG=true\" corrects for that, we're only\n> marking those tests as passing that really don't have any leaks,\n> whether that was reflected in their exit code or not.\n\nThis paragraph repeats the previous one, but with different words.\nConsider removing it.\n \n> t/t0027-auto-crlf.sh | 1 +\n> t/t0032-reftable-unittest.sh | 1 +\n> t/t0033-safe-directory.sh | 1 +\n> t/t0050-filesystem.sh | 1 +\n> t/t0095-bloom.sh | 2 ++\n> t/t1405-main-ref-store.sh | 1 +\n> t/t1407-worktree-ref-store.sh | 1 +\n> t/t1418-reflog-exists.sh | 1 +\n> t/t1701-racy-split-index.sh | 1 +\n> t/t2006-checkout-index-basic.sh | 1 +\n> t/t2023-checkout-m.sh | 1 +\n> t/t2205-add-worktree-config.sh | 1 +\n> t/t3012-ls-files-dedup.sh | 1 +\n> t/t4017-diff-retval.sh | 1 +\n> t/t4051-diff-function-context.sh | 1 +\n> t/t4057-diff-combined-paths.sh | 1 +\n> t/t4114-apply-typechange.sh | 1 +\n> t/t4301-merge-tree-write-tree.sh | 1 +\n> t/t5315-pack-objects-compression.sh | 1 +\n> t/t5351-unpack-large-objects.sh | 1 +\n> t/t5402-post-merge-hook.sh | 1 +\n> t/t5503-tagfollow.sh | 1 +\n> t/t6404-recursive-merge.sh | 1 +\n> t/t6405-merge-symlinks.sh | 1 +\n> t/t6408-merge-up-to-date.sh | 1 +\n> t/t6411-merge-filemode.sh | 1 +\n> t/t6413-merge-crlf.sh | 1 +\n> t/t6415-merge-dir-to-symlink.sh | 1 +\n> t/t6425-merge-rename-delete.sh | 1 +\n> t/t6431-merge-criscross.sh | 1 +\n> t/t7060-wtstatus.sh | 1 +\n> t/t7062-wtstatus-ignorecase.sh | 1 +\n> t/t7110-reset-merge.sh | 1 +\n> t/t7111-reset-table.sh | 1 +\n> t/t7609-mergetool--lib.sh | 1 +\n> t/t9100-git-svn-basic.sh | 1 -\n> t/t9901-git-web--browse.sh | 1 +\n\nThat's a lot of tests that we can mark as having no leaks. Nice.\n\nI imagine that after this series cooks for a while we can consider\nrunning this check in CI. That is, unless it's prohibitively\nexpensive to run under SANITIZE=leak.\n\nThanks,\n-Stolee\n"},{"id":"459520","messageId":"xmqqmtd33e1h.fsf@gitster.g","threadId":"58191","inReplyTo":"patch-10.10-9cedf0cb0e2-20220719T205710Z-avarab@gmail.com","subject":"Re: [PATCH 10/10] log tests: don't use \"exit 1\" outside a sub-shell","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-07-20T17:11:06Z","receivedAt":"2022-07-20T17:11:17Z","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 an \"exit 1\" added in ac52d9410e5 (t4205: cover `git log\n> --reflog -z` blindspot, 2019-11-19) to use \"return 1\" instead, which\n> curiously was done in an adjacent test case added in the same commit.\n>\n> Using \"exit 1\" outside a sub-shell will cause the test framework\n> itself to exit on failure, which isn't what we want to do here.\n>\n> This issue was spotted with the new\n> \"GIT_TEST_PASSING_SANITIZE_LEAK=check\" mode, i.e. that \"git show\"\n> command leaks memory, and we'd thus \"exit 1\". Another implementation\n> of \"GIT_TEST_PASSING_SANITIZE_LEAK=check\" or \"--invert-exit-code\"\n> might have intercepted the \"exit 1\", and thus hidden the underlying\n> issue here, but we correctly distinguish the two.\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> t/t4205-log-pretty-formats.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n\nLet's not bury this, which does not have to depend on the rest of\nthe patches in this series, at the end. Instead have this a\nseparate topic, on which this topic may depend on.\n\n\n>\n> diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\n> index e448ef2928a..0404491d6ee 100755\n> --- a/t/t4205-log-pretty-formats.sh\n> +++ b/t/t4205-log-pretty-formats.sh\n> @@ -156,7 +156,7 @@ test_expect_success 'NUL termination with --reflog --pretty=oneline' '\n> \tfor r in $revs\n> \tdo\n> \t\tgit show -s --pretty=oneline \"$r\" >raw &&\n> -\t\tcat raw | lf_to_nul || exit 1\n> +\t\tcat raw | lf_to_nul || return 1\n> \tdone >expect &&\n> \t# the trailing NUL is already produced so we do not need to\n> \t# output another one\n"},{"id":"459538","messageId":"patch-v2-01.14-e53cf647b44-20220720T211221Z-avarab@gmail.com","threadId":"58191","inReplyTo":"cover-v2-00.14-00000000000-20220720T211221Z-avarab@gmail.com","subject":"[PATCH v2 01/14] test-lib: use $1, not $@ in test_known_broken_{ok,failure}_","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-20T21:21:39Z","receivedAt":"2022-07-20T21:22:08Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Clarify that these two functions never take N arguments, they'll only\never receive one. They've needlessly used $@ over $1 since\n41ac414ea2b (Sane use of test_expect_failure, 2008-02-01).\n\nIn the future we might want to pass the test source to these, but now\nthat's not the case. This preparatory change helps to clarify a\nfollow-up change.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/test-lib.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 7726d1da88a..3f11ce35112 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -804,14 +804,14 @@ test_failure_ () {\n \n test_known_broken_ok_ () {\n \ttest_fixed=$(($test_fixed+1))\n-\tsay_color error \"ok $test_count - $@ # TODO known breakage vanished\"\n-\tfinalize_test_case_output fixed \"$@\"\n+\tsay_color error \"ok $test_count - $1 # TODO known breakage vanished\"\n+\tfinalize_test_case_output fixed \"$1\"\n }\n \n test_known_broken_failure_ () {\n \ttest_broken=$(($test_broken+1))\n-\tsay_color warn \"not ok $test_count - $@ # TODO known breakage\"\n-\tfinalize_test_case_output broken \"$@\"\n+\tsay_color warn \"not ok $test_count - $1 # TODO known breakage\"\n+\tfinalize_test_case_output broken \"$1\"\n }\n \n test_debug () {\n-- \n2.37.1.1064.gc96144cf387\n\n"},{"id":"459540","messageId":"cover-v2-00.14-00000000000-20220720T211221Z-avarab@gmail.com","threadId":"58191","inReplyTo":"cover-00.10-00000000000-20220719T205710Z-avarab@gmail.com","subject":"[PATCH v2 00/14] leak test: add \"check\" test mode, mark leak-free tests","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-20T21:21:38Z","receivedAt":"2022-07-20T21:22:09Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This series adds a \"check\" leak-testing mode, which allows us to\nassert that the tests we mark as leak free and those that don't leak\nare one-to-one mapped to one another.\n\nChanges since v1:\n\n * Changed \"test-lib.sh\" to \"test-lib\" in $subject, some used that\n form already, and since some of the subjects were long let's use\n that for brevity, and use it consistently.\n\n * Fix a bug in how --stress interacted with --invert-exit-code, we'll\n no longer remove the trash directory of the \"failed\" test.\n\n * Various rewording etc. fixes to t/README. I think this should\n address all of Derrick's comments, and more.\n\n I opted to split some of that up to avoid the very lengthy\n paragraphs to some extent.\n\n * v1 had a workaround to not use the \"check\" mode for t9700. There\n were issues with how that interacted with \"test_external\" still, so\n this re-roll proposes to remove \"test_external\" entirelry.\n\n I'd proposed that as part of a series that didn't make it before, I\n think the end-state is much better:\n https://lore.kernel.org/git/20210309160219.13779-2-avarab@gmail.com/\n\n * Ejected the \"log tests: don't use \"exit 1\" outside a sub-shell\" fix\n from this series. I'll submit it seperately. This series can still\n be based on \"master\".\n\n It will fail under \"check\", but e.g. t7517-per-repo-email.sh also\n occasionally fails for me (it's flaky, sometimes reporting leaks,\n sometimes not). We can just leave it for now...\n\n * Fix a memory leak in upload-pack under gcc (which is funny with\n \"die\" sometimes), which allows us to....\n\n * Turn on \"GIT_TEST_SANITIZE_LEAK_LOG=true\" in the linux-leaks CI, to\n harden our assertions. Now we really know that we don't have any\n leaks there.\n\n * Add missing &&-chaining.\n\n * Under GIT_TEST_SANITIZE_LEAK_LOG=true we'll emit the log content\n now, rather than just linking to it. I omitted it before because\n the \"test-results\" might include logs from a previous run, but now\n the message we emit is adjusted to account for that instead.\n\n * Added some missing cases to the commit message explanation of why\n we miss certain leaks without \"GIT_TEST_SANITIZE_LEAK_LOG=true\".\n\n * Clarified in t/README how these various new modes interact with one\n another.\n\nÆvar Arnfjörð Bjarmason (14):\n test-lib: use $1, not $@ in test_known_broken_{ok,failure}_\n test-lib: don't set GIT_EXIT_OK before calling test_atexit_handler\n test-lib: fix GIT_EXIT_OK logic errors, use BAIL_OUT\n test-lib: add a --invert-exit-code switch\n t/README: reword the \"GIT_TEST_PASSING_SANITIZE_LEAK\" description\n test-lib: add a SANITIZE=leak logging mode\n t/Makefile: don't remove test-results in \"clean-except-prove-cache\"\n tests: move copy/pasted PERL + Test::More checks to a lib-perl.sh\n test-lib: simplify by removing test_external\n test-lib: add a GIT_TEST_PASSING_SANITIZE_LEAK=check mode\n test-lib: have the \"check\" mode for SANITIZE=leak consider leak logs\n leak tests: mark passing SANITIZE=leak tests as leak-free\n upload-pack: fix a memory leak in create_pack_file()\n CI: use \"GIT_TEST_SANITIZE_LEAK_LOG=true\" in linux-leaks\n\n ci/lib.sh | 1 +\n .../netrc/t-git-credential-netrc.sh | 18 +-\n contrib/scalar/t/Makefile | 2 +-\n contrib/subtree/t/Makefile | 2 +-\n t/Makefile | 2 +-\n t/README | 73 ++---\n t/lib-perl.sh | 19 ++\n t/t0000-basic.sh | 72 +++++\n t/t0027-auto-crlf.sh | 1 +\n t/t0032-reftable-unittest.sh | 1 +\n t/t0033-safe-directory.sh | 1 +\n t/t0050-filesystem.sh | 1 +\n t/t0095-bloom.sh | 2 +\n t/t0202-gettext-perl.sh | 22 +-\n t/t1405-main-ref-store.sh | 1 +\n t/t1407-worktree-ref-store.sh | 1 +\n t/t1418-reflog-exists.sh | 1 +\n t/t1701-racy-split-index.sh | 1 +\n t/t2006-checkout-index-basic.sh | 1 +\n t/t2023-checkout-m.sh | 1 +\n t/t2205-add-worktree-config.sh | 1 +\n t/t3012-ls-files-dedup.sh | 1 +\n t/t4017-diff-retval.sh | 1 +\n t/t4051-diff-function-context.sh | 1 +\n t/t4057-diff-combined-paths.sh | 1 +\n t/t4114-apply-typechange.sh | 1 +\n t/t4301-merge-tree-write-tree.sh | 1 +\n t/t5315-pack-objects-compression.sh | 1 +\n t/t5351-unpack-large-objects.sh | 1 +\n t/t5402-post-merge-hook.sh | 1 +\n t/t5503-tagfollow.sh | 1 +\n t/t6404-recursive-merge.sh | 1 +\n t/t6405-merge-symlinks.sh | 1 +\n t/t6407-merge-binary.sh | 1 -\n t/t6408-merge-up-to-date.sh | 1 +\n t/t6411-merge-filemode.sh | 1 +\n t/t6413-merge-crlf.sh | 1 +\n t/t6415-merge-dir-to-symlink.sh | 1 +\n t/t6425-merge-rename-delete.sh | 1 +\n t/t6431-merge-criscross.sh | 1 +\n t/t7060-wtstatus.sh | 1 +\n t/t7062-wtstatus-ignorecase.sh | 1 +\n t/t7110-reset-merge.sh | 1 +\n t/t7111-reset-table.sh | 1 +\n t/t7609-mergetool--lib.sh | 1 +\n t/t9100-git-svn-basic.sh | 1 -\n t/t9700-perl-git.sh | 23 +-\n t/t9901-git-web--browse.sh | 1 +\n t/test-lib-functions.sh | 89 +------\n t/test-lib.sh | 249 ++++++++++++++----\n upload-pack.c | 1 +\n 51 files changed, 396 insertions(+), 216 deletions(-)\n create mode 100644 t/lib-perl.sh\n\nRange-diff against v1:\n 1: 5664c4f9a0e ! 1: e53cf647b44 test-lib.sh: use $1, not $@ in test_known_broken_{ok,failure}_\n @@ Metadata\n Author: Ævar Arnfjörð Bjarmason \n \n ## Commit message ##\n - test-lib.sh: use $1, not $@ in test_known_broken_{ok,failure}_\n + test-lib: use $1, not $@ in test_known_broken_{ok,failure}_\n \n Clarify that these two functions never take N arguments, they'll only\n ever receive one. They've needlessly used $@ over $1 since\n 2: c228308c121 ! 2: 00af775bd0d test-lib.sh: don't set GIT_EXIT_OK before calling test_atexit_handler\n @@ Metadata\n Author: Ævar Arnfjörð Bjarmason \n \n ## Commit message ##\n - test-lib.sh: don't set GIT_EXIT_OK before calling test_atexit_handler\n + test-lib: don't set GIT_EXIT_OK before calling test_atexit_handler\n \n Change the control flow in test_done so that we'll set GIT_EXIT_OK=t\n after we call test_atexit_handler(). This seems to have been a mistake\n 3: e57e7ca898e ! 3: 419bc2c6a6e test-lib.sh: fix GIT_EXIT_OK logic errors, use BAIL_OUT\n @@ Metadata\n Author: Ævar Arnfjörð Bjarmason \n \n ## Commit message ##\n - test-lib.sh: fix GIT_EXIT_OK logic errors, use BAIL_OUT\n + test-lib: fix GIT_EXIT_OK logic errors, use BAIL_OUT\n \n Change various \"exit 1\" checks that happened after our \"die\" handler\n had been set up to use BAIL_OUT instead. See 234383cd401 (test-lib.sh:\n 4: 4aab7af60e3 ! 4: 668c25f4d7e test-lib.sh: add a --invert-exit-code switch\n @@ Metadata\n Author: Ævar Arnfjörð Bjarmason \n \n ## Commit message ##\n - test-lib.sh: add a --invert-exit-code switch\n + test-lib: add a --invert-exit-code switch\n \n Add the ability to have those tests that fail return 0, and those\n tests that succeed return 1. This is useful e.g. to run \"--stress\"\n @@ t/test-lib.sh: test_ok_ () {\n \t\t_error_exit\n \tfi\n \tfinalize_test_case_output failure \"$failure_label\" \"$@\"\n +@@ t/test-lib.sh: test_done () {\n + \t\t\tesac\n + \t\tfi\n + \n +-\t\tif test -z \"$debug\" && test -n \"$remove_trash\"\n ++\t\tif test -n \"$stress\" && test -n \"$invert_exit_code\"\n ++\t\tthen\n ++\t\t\t# We're about to move our \"$TRASH_DIRECTORY\"\n ++\t\t\t# to \"$TRASH_DIRECTORY.stress-failed\" if\n ++\t\t\t# --stress is combined with\n ++\t\t\t# --invert-exit-code.\n ++\t\t\tsay \"with --stress and --invert-exit-code we're not removing '$TRASH_DIRECTORY'\"\n ++\t\telif test -z \"$debug\" && test -n \"$remove_trash\"\n + \t\tthen\n + \t\t\ttest -d \"$TRASH_DIRECTORY\" ||\n + \t\t\terror \"Tests passed but trash directory already removed before test cleanup; aborting\"\n @@ t/test-lib.sh: test_done () {\n \t\t\t} ||\n \t\t\terror \"Tests passed but test cleanup failed; aborting\"\n 5: 6f474a0f83d = 5: a26cb02db0a t/README: reword the \"GIT_TEST_PASSING_SANITIZE_LEAK\" description\n 6: ad1395f45af ! 6: f1acf762899 test-lib: add a SANITIZE=leak logging mode\n @@ Commit message\n \n grep -o -P -h '(?<=DEDUP_TOKEN: ).*' test-results/*.leak/trace.git.* | grep -E -o '[^-]+' | sort | uniq -c | sort -nr | head -n 20\n \n + This new mode requires git to be compiled with SANITIZE=leak, rather\n + than explaining that in the documentation let's make it\n + self-documenting by bailing out if the user asks for this without git\n + having been compiled with SANITIZE=leak, as we do with\n + GIT_TEST_PASSING_SANITIZE_LEAK=true.\n +\n Signed-off-by: Ævar Arnfjörð Bjarmason \n \n ## t/README ##\n @@ t/README: declared themselves as leak-free by setting\n test mode is used by the \"linux-leaks\" CI target.\n \n +GIT_TEST_SANITIZE_LEAK_LOG=true will log memory leaks to\n -+\"test-results/$TEST_NAME.leak/trace.*\" files. Useful in combination\n -+with \"GIT_TEST_PASSING_SANITIZE_LEAK\" to check if we're falsely\n -+reporting a test as \"passing\" with SANITIZE=leak due to ignored exit\n -+codes.\n ++\"test-results/$TEST_NAME.leak/trace.*\" files. The logs include a\n ++\"dedup_token\" (see +\"ASAN_OPTIONS=help=1 ./git\") and other options to\n ++make logs +machine-readable.\n +\n GIT_TEST_PROTOCOL_VERSION=, when set, makes 'protocol.version'\n default to n.\n @@ t/test-lib.sh: TEST_NUMBER=\"${TEST_NAME%%-*}\"\n TRASH_DIRECTORY=\"trash directory.$TEST_NAME$TEST_STRESS_JOB_SFX\"\n test -n \"$root\" && TRASH_DIRECTORY=\"$root/$TRASH_DIRECTORY\"\n case \"$TRASH_DIRECTORY\" in\n +@@ t/test-lib.sh: then\n + \ttest_done\n + fi\n + \n ++BAIL_OUT_ENV_NEEDS_SANITIZE_LEAK () {\n ++\tBAIL_OUT \"$1 has no effect except when compiled with SANITIZE=leak\"\n ++}\n ++\n + # skip non-whitelisted tests when compiled with SANITIZE=leak\n + if test -n \"$SANITIZE_LEAK\"\n + then\n @@ t/test-lib.sh: then\n \t\t\ttest_done\n \t\tfi\n @@ t/test-lib.sh: then\n +\tfi\n elif test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false\n then\n - \tBAIL_OUT \"GIT_TEST_PASSING_SANITIZE_LEAK=true has no effect except when compiled with SANITIZE=leak\"\n +-\tBAIL_OUT \"GIT_TEST_PASSING_SANITIZE_LEAK=true has no effect except when compiled with SANITIZE=leak\"\n ++\tBAIL_OUT_ENV_NEEDS_SANITIZE_LEAK \"GIT_TEST_PASSING_SANITIZE_LEAK=true\"\n ++elif test_bool_env GIT_TEST_SANITIZE_LEAK_LOG false\n ++then\n ++\tBAIL_OUT_ENV_NEEDS_SANITIZE_LEAK \"GIT_TEST_SANITIZE_LEAK_LOG=true\"\n + fi\n + \n + # Last-minute variable setup\n -: ----------- > 7: 0723e90df7b t/Makefile: don't remove test-results in \"clean-except-prove-cache\"\n -: ----------- > 8: 987d9d0e98c tests: move copy/pasted PERL + Test::More checks to a lib-perl.sh\n -: ----------- > 9: 20bd31615e4 test-lib: simplify by removing test_external\n 7: 0961df2ab6c ! 10: 78a47d2b348 test-lib.sh: add a GIT_TEST_PASSING_SANITIZE_LEAK=check mode\n @@ Metadata\n Author: Ævar Arnfjörð Bjarmason \n \n ## Commit message ##\n - test-lib.sh: add a GIT_TEST_PASSING_SANITIZE_LEAK=check mode\n + test-lib: add a GIT_TEST_PASSING_SANITIZE_LEAK=check mode\n \n Add a new \"GIT_TEST_PASSING_SANITIZE_LEAK=check\" mode to the\n test-lib.sh.\n @@ Commit message\n 956d2e4639b (tests: add a test mode for SANITIZE=leak, run it in CI,\n 2021-09-23).\n \n - It does so by adding the ability to check that there's a 1=1\n - correspondence between those tests that are marked as passing with\n - SANITIZE=leak, and those tests that are leak-free. I.e. a test that\n - passes with SANITIZE=leak but isn't marked as such with\n - TEST_PASSES_SANITIZE_LEAK=true will error out.\n + Rather than document this all in one (even more) dense paragraph split\n + up the discussion of how it combines with --immediate into its own\n + paragraph following the discussion of\n + \"GIT_TEST_SANITIZE_LEAK_LOG=true\".\n +\n + Before the removal of \"test_external\" in a preceding commit we would\n + have had to special-case t9700-perl-git.sh and t0202-gettext-perl.sh.\n \n Signed-off-by: Ævar Arnfjörð Bjarmason \n \n @@ t/README: declared themselves as leak-free by setting\n test mode is used by the \"linux-leaks\" CI target.\n \n +GIT_TEST_PASSING_SANITIZE_LEAK=check checks that our\n -+\"TEST_PASSES_SANITIZE_LEAK=true\" markings are current. The \"check\" is\n -+particularly useful with \"--immediate\", but otherwise acts the same\n -+for tests that have \"TEST_PASSES_SANITIZE_LEAK=true\" set. For those\n -+that don't have it set it runs them, and considers them passing\n -+without errors a failure (by providing \"--invert-exit-code\"). Thus the\n -+\"check\" mode can be used e.g. with \"git rebase --exec\" to ensure that\n -+there's a 1=1 mapping between \"TEST_PASSES_SANITIZE_LEAK=true\" and\n -+those tests that pass under \"SANITIZE=leak\".\n ++\"TEST_PASSES_SANITIZE_LEAK=true\" markings are current. Rather than\n ++skipping those tests that haven't set \"TEST_PASSES_SANITIZE_LEAK=true\"\n ++before sourcing \"test-lib.sh\" this mode runs them with\n ++\"--invert-exit-code\". This is used to check that there's a one-to-one\n ++mapping between \"TEST_PASSES_SANITIZE_LEAK=true\" and those tests that\n ++pass under \"SANITIZE=leak\". This is especially useful when testing a\n ++series that fixes various memory leaks with \"git rebase -x\".\n +\n GIT_TEST_SANITIZE_LEAK_LOG=true will log memory leaks to\n - \"test-results/$TEST_NAME.leak/trace.*\" files. Useful in combination\n - with \"GIT_TEST_PASSING_SANITIZE_LEAK\" to check if we're falsely\n -\n - ## t/t9700-perl-git.sh ##\n -@@ t/t9700-perl-git.sh: if ! test_have_prereq PERL; then\n - \ttest_done\n - fi\n + \"test-results/$TEST_NAME.leak/trace.*\" files. The logs include a\n + \"dedup_token\" (see +\"ASAN_OPTIONS=help=1 ./git\") and other options to\n + make logs +machine-readable.\n \n -+if test \"$GIT_TEST_PASSING_SANITIZE_LEAK\" = \"check\" && test_have_prereq SANITIZE_LEAK\n -+then\n -+\tskip_all='SANITIZE=leak and GIT_TEST_PASSING_SANITIZE_LEAK=check do not combine with test_external'\n -+\ttest_done\n -+fi\n ++GIT_TEST_PASSING_SANITIZE_LEAK=check when combined with \"--immediate\"\n ++will run to completion faster, and result in the same failing\n ++tests. The only practical reason to run\n ++GIT_TEST_PASSING_SANITIZE_LEAK=check without \"--immediate\" is to\n ++combine it with \"GIT_TEST_SANITIZE_LEAK_LOG=true\". If we stop at the\n ++first failing test case our leak logs won't show subsequent leaks we\n ++might have run into.\n +\n - perl -MTest::More -e 0 2>/dev/null || {\n - \tskip_all=\"Perl Test::More unavailable, skipping test\"\n - \ttest_done\n + GIT_TEST_PROTOCOL_VERSION=, when set, makes 'protocol.version'\n + default to n.\n + \n \n ## t/test-lib.sh ##\n -@@ t/test-lib.sh: fi\n +@@ t/test-lib.sh: BAIL_OUT_ENV_NEEDS_SANITIZE_LEAK () {\n # skip non-whitelisted tests when compiled with SANITIZE=leak\n if test -n \"$SANITIZE_LEAK\"\n then\n @@ t/test-lib.sh: then\n +elif test \"$GIT_TEST_PASSING_SANITIZE_LEAK\" = \"check\" ||\n + test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false\n then\n - \tBAIL_OUT \"GIT_TEST_PASSING_SANITIZE_LEAK=true has no effect except when compiled with SANITIZE=leak\"\n - fi\n + \tBAIL_OUT_ENV_NEEDS_SANITIZE_LEAK \"GIT_TEST_PASSING_SANITIZE_LEAK=true\"\n + elif test_bool_env GIT_TEST_SANITIZE_LEAK_LOG false\n 8: afbb7e19195 ! 11: 8cc6ab390db test-lib: have the \"check\" mode for SANITIZE=leak consider leak logs\n @@ Commit message\n \n Before this compiling with SANITIZE=leak and running:\n \n - ./t4058-diff-duplicates.sh\n + ./t6407-merge-binary.sh\n \n Will exit successfully, now we'll get an error and an informative\n message on:\n \n - GIT_TEST_SANITIZE_LEAK_LOG=true ./t4058-diff-duplicates.sh\n + GIT_TEST_SANITIZE_LEAK_LOG=true ./t6407-merge-binary.sh\n \n - And even more useful, we'll now either error or exit successfully on\n - this command, depending on whether or not the test has labeled itself\n - leak-free with TEST_PASSES_SANITIZE_LEAK=true or not.\n + Even better, as noted in the updated t/README we'll now error out when\n + combined with the \"check\" mode:\n \n - GIT_TEST_SANITIZE_LEAK_LOG=true GIT_TEST_PASSING_SANITIZE_LEAK=check ./t4058-diff-duplicates.sh\n + GIT_TEST_PASSING_SANITIZE_LEAK=check \\\n + GIT_TEST_SANITIZE_LEAK_LOG=true \\\n + ./t4058-diff-duplicates.sh\n \n - Why do we miss these leaks in the first place? As initially noted in\n - [1] (and discussed downthread) the reasons are:\n + Why do we miss these leaks? Because:\n \n - * Our tests will (mostly) catch segfaults and abort(), but if we\n + * We have leaks inside \"test_expect_failure\" blocks, which by design\n + will not distinguish a \"normal\" failure from an abort() or\n + segfault. See [1] for a discussion of it shortcomings.\n +\n + * We have \"git\" invocations outside of \"test_expect_success\",\n + e.g. setup code in the main body of the test, or in test helper\n + functions that don't use &&-chaining.\n +\n + * Our tests will otherwise 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 @@ Commit message\n abort()'s. If the test invoking the parent command(s) is using\n \"test_must_fail\" we'll consider it an expected \"ok\" failure.\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 + * run-command.c doesn't (but probably should) ferry up such exit\n + codes, so for e.g. \"git push\" tests where we expect a failure and an\n + underlying \"git\" command fails we won't ferry up the segfault or\n + 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, or simply ignoring the exit codes(s)\n - entirely, e.g. these invocations in git-merge-one-file.sh leak, but\n - aren't reflected in the \"git merge\" exit code:\n + git commands, they'll usually return their own exit codes on \"git\"\n + failure, rather then ferrying up segfault or abort() exit code.\n +\n + E.g. these invocations in git-merge-one-file.sh leak, but aren't\n + reflected in the \"git merge\" exit code:\n \n src1=$(git unpack-file $2)\n src2=$(git unpack-file $3)\n @@ Commit message\n write_tree_trivial() in \"builtin/merge.c\" calling die() instead of\n ferrying up the relevant exit code.\n \n - Let's remove \"TEST_PASSES_SANITIZE_LEAK=true\" from the one test we\n - were falsely marking as leak-free, marked as such in my\n + Let's remove \"TEST_PASSES_SANITIZE_LEAK=true\" from tests we\n + were falsely marking as leak-free.\n +\n + In the case of t6407-merge-binary.sh it was marked as leak-free in\n 9081a421a6d (checkout: fix \"branch info\" memory leaks,\n 2021-11-16). I'd previously removed other bad\n \"TEST_PASSES_SANITIZE_LEAK=true\" opt-ins in the series merged in\n ea05fd5fbf7 (Merge branch 'ab/keep-git-exit-codes-in-tests',\n - 2022-03-16).\n + 2022-03-16). The case of t1060-object-corruption.sh is more subtle,\n + and will be discussed in a subsequent commit.\n \n - 1. https://lore.kernel.org/git/cover-00.15-00000000000-20220302T171755Z-avarab@gmail.com/\n + 1. https://lore.kernel.org/git/cover-0.7-00000000000-20220318T002951Z-avarab@gmail.com/\n \n Signed-off-by: Ævar Arnfjörð Bjarmason \n \n ## t/README ##\n -@@ t/README: without errors a failure (by providing \"--invert-exit-code\"). Thus the\n - there's a 1=1 mapping between \"TEST_PASSES_SANITIZE_LEAK=true\" and\n - those tests that pass under \"SANITIZE=leak\".\n +@@ t/README: GIT_TEST_SANITIZE_LEAK_LOG=true will log memory leaks to\n + \"dedup_token\" (see +\"ASAN_OPTIONS=help=1 ./git\") and other options to\n + make logs +machine-readable.\n \n -+The \"check\" mode is especially useful if combined with\n -+GIT_TEST_SANITIZE_LEAK_LOG=true.\n ++With GIT_TEST_SANITIZE_LEAK_LOG=true we'll look at the leak logs\n ++before exiting and exit on failure if the logs showed that we had a\n ++memory leak, even if the test itself would have otherwise passed. This\n ++allows us to catch e.g. missing &&-chaining. This is especially useful\n ++when combined with \"GIT_TEST_PASSING_SANITIZE_LEAK\", see below.\n +\n - GIT_TEST_SANITIZE_LEAK_LOG=true will log memory leaks to\n - \"test-results/$TEST_NAME.leak/trace.*\" files. Useful in combination\n - with \"GIT_TEST_PASSING_SANITIZE_LEAK\" to check if we're falsely\n - reporting a test as \"passing\" with SANITIZE=leak due to ignored exit\n - codes.\n + GIT_TEST_PASSING_SANITIZE_LEAK=check when combined with \"--immediate\"\n + will run to completion faster, and result in the same failing\n + tests. The only practical reason to run\n +@@ t/README: combine it with \"GIT_TEST_SANITIZE_LEAK_LOG=true\". If we stop at the\n + first failing test case our leak logs won't show subsequent leaks we\n + might have run into.\n \n -+When GIT_TEST_SANITIZE_LEAK_LOG=true is set we'll look at the\n -+\"test-results/$TEST_NAME.leak/trace.*\" files at the end of the test\n -+run in combination with the \"TEST_PASSES_SANITIZE_LEAK\" and\n -+GIT_TEST_PASSING_SANITIZE_LEAK=check setting to see if we'll fail a\n -+test leaked, but which the test run itself didn't catch due to ignored\n -+or missed exit codes.\n ++GIT_TEST_PASSING_SANITIZE_LEAK=(true|check) will not catch all memory\n ++leaks unless combined with GIT_TEST_SANITIZE_LEAK_LOG=true. Some tests\n ++run \"git\" (or \"test-tool\" etc.) without properly checking the exit\n ++code, or git will invoke itself and fail to ferry the abort() exit\n ++code to the original caller. When the two modes are combined we'll\n ++look at the \"test-results/$TEST_NAME.leak/trace.*\" files at the end of\n ++the test run to see if had memory leaks which the test itself didn't\n ++catch.\n +\n GIT_TEST_PROTOCOL_VERSION=, when set, makes 'protocol.version'\n default to n.\n \n \n + ## t/t1060-object-corruption.sh ##\n +@@\n + \n + test_description='see how we handle various forms of corruption'\n + \n +-TEST_PASSES_SANITIZE_LEAK=true\n + . ./test-lib.sh\n + \n + # convert \"1234abcd\" to \".git/objects/12/34abcd\"\n +\n ## t/t6407-merge-binary.sh ##\n @@ t/t6407-merge-binary.sh: test_description='ask merge-recursive to merge binary files'\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n @@ t/test-lib.sh: test_atexit_handler () {\n +losing the exit code somewhere, or not propagating it appropriately\n +upwards!\n +\n -+See the logs at \\\"%s.*\\\"\" \\\n ++See the logs at \\\"%s.*\\\";\n ++those logs are reproduced below.\" \\\n +\t \"$new\" \"$old\" \"$file\"\n +}\n +\n @@ t/test-lib.sh: test_atexit_handler () {\n +\tif test -z \"$TEST_RESULTS_SAN_FILE\"\n +\tthen\n +\t\treturn\n -+\tfi\n ++\tfi &&\n +\tlocal old=\"$TEST_RESULTS_SAN_DIR_NR_LEAKS_STARTUP\" &&\n +\tlocal new=\"$(nr_san_dir_leaks_)\" &&\n +\n +\tif test $new -le $old\n +\tthen\n +\t\treturn\n -+\tfi\n ++\tfi &&\n +\tlocal out=\"$(sanitize_leak_log_message_ \"$new\" \"$old\" \"$TEST_RESULTS_SAN_FILE\")\" &&\n +\tsay_color error \"$out\" &&\n ++\tif test \"$old\" != 0\n ++\tthen\n ++\t\techo &&\n ++\t\tsay_color error \"The logs include output from past runs to avoid\" &&\n ++\t\tsay_color error \"that remove 'test-results' between runs.\"\n ++\tfi &&\n ++\tsay_color error \"$(cat \"$TEST_RESULTS_SAN_FILE\".*)\" &&\n +\n +\tif test -n \"$passes_sanitize_leak\" && test \"$test_failure\" = 0\n +\tthen\n -+\t\tsay \"As TEST_PASSES_SANITIZE_LEAK=true and our logs show we're leaking, exit non-zero!\"\n ++\t\tsay \"As TEST_PASSES_SANITIZE_LEAK=true and our logs show we're leaking, exit non-zero!\" &&\n +\t\tinvert_exit_code=t\n +\telif test -n \"$passes_sanitize_leak\"\n +\tthen\n -+\t\tsay \"As TEST_PASSES_SANITIZE_LEAK=true and our logs show we're leaking, and we're failing for other reasons too...\"\n ++\t\tsay \"As TEST_PASSES_SANITIZE_LEAK=true and our logs show we're leaking, and we're failing for other reasons too...\" &&\n +\t\tinvert_exit_code=\n +\telif test -n \"$sanitize_leak_check\" && test \"$test_failure\" = 0\n +\tthen\n -+\t\tsay \"As TEST_PASSES_SANITIZE_LEAK=true isn't set the above leak is 'ok' with GIT_TEST_PASSING_SANITIZE_LEAK=check\"\n ++\t\tsay \"As TEST_PASSES_SANITIZE_LEAK=true isn't set the above leak is 'ok' with GIT_TEST_PASSING_SANITIZE_LEAK=check\" &&\n +\t\tinvert_exit_code=\n +\telif test -n \"$sanitize_leak_check\"\n +\tthen\n -+\t\tsay \"As TEST_PASSES_SANITIZE_LEAK=true isn't set the above leak is 'ok' with GIT_TEST_PASSING_SANITIZE_LEAK=check\"\n ++\t\tsay \"As TEST_PASSES_SANITIZE_LEAK=true isn't set the above leak is 'ok' with GIT_TEST_PASSING_SANITIZE_LEAK=check\" &&\n +\t\tinvert_exit_code=t\n +\telse\n -+\t\tsay \"With GIT_TEST_SANITIZE_LEAK_LOG=true our logs revealed a memory leak, exit non-zero!\"\n ++\t\tsay \"With GIT_TEST_SANITIZE_LEAK_LOG=true our logs revealed a memory leak, exit non-zero!\" &&\n +\t\tinvert_exit_code=t\n +\tfi\n +}\n @@ t/test-lib.sh: test_done () {\n \t\tthen\n \t\t\tsay_color warn \"# faking up non-zero exit with --invert-exit-code\"\n @@ t/test-lib.sh: test_done () {\n - \t\texit 0 ;;\n + \t\tsay_color error \"# failed $test_failure among $msg\"\n + \t\tsay \"1..$test_count\"\n \n - \t*)\n +\t\tcheck_test_results_san_file_ \"$test_failure\"\n +\n - \t\tif test $test_external_has_tap -eq 0\n + \t\tif test -n \"$invert_exit_code\"\n \t\tthen\n - \t\t\tsay_color error \"# failed $test_failure among $msg\"\n + \t\t\t_invert_exit_code_failure_end_blurb\n @@ t/test-lib.sh: then\n \n \tif test \"$GIT_TEST_PASSING_SANITIZE_LEAK\" = \"check\"\n 9: b75b93822e1 ! 12: e3c8909207b leak tests: mark passing SANITIZE=leak tests as leak-free\n @@ Commit message\n 2021-12-15) and 5a4f8381b68 (Merge branch 'ab/mark-leak-free-tests',\n 2021-10-25), or have had their memory leaks fixed since then.\n \n - With this change there's now a a 1=1 mapping between those tests that\n - we have opted-in via \"TEST_PASSES_SANITIZE_LEAK=true\", and those that\n - pass with the new \"check\" mode:\n + With this change there's now a a one-to-one mapping between those\n + tests that we have opted-in via \"TEST_PASSES_SANITIZE_LEAK=true\", and\n + those that pass with the new \"check\" mode:\n \n - GIT_TEST_PASSING_SANITIZE_LEAK=check GIT_TEST_SANITIZE_LEAK_LOG=true make test SANITIZE=leak\n + GIT_TEST_PASSING_SANITIZE_LEAK=check \\\n + GIT_TEST_SANITIZE_LEAK_LOG=true \\\n + make test SANITIZE=leak\n \n Note that the \"GIT_TEST_SANITIZE_LEAK_LOG=true\" is needed due to the\n edge cases noted in a preceding commit, i.e. in some cases we'd pass\n @@ Commit message\n marking those tests as passing that really don't have any leaks,\n whether that was reflected in their exit code or not.\n \n + Note that the change here to \"t9100-git-svn-basic.sh\" is marking that\n + test as passing under SANITIZE=leak, we're removing a\n + \"TEST_FAILS_SANITIZE_LEAK=true\" line, not\n + \"TEST_PASSES_SANITIZE_LEAK=true\". See 7a98d9ab00d (revisions API: have\n + release_revisions() release \"cmdline\", 2022-04-13) for the\n + introduction of that t/lib-git-svn.sh-specific variable.\n +\n Signed-off-by: Ævar Arnfjörð Bjarmason \n \n ## t/t0027-auto-crlf.sh ##\n @@ t/t9100-git-svn-basic.sh: test_description='git svn basic tests'\n \n prepare_utf8_locale\n \n + ## t/t9700-perl-git.sh ##\n +@@\n + #\n + \n + test_description='perl interface (Git.pm)'\n ++\n ++TEST_PASSES_SANITIZE_LEAK=true\n + . ./test-lib.sh\n + . \"$TEST_DIRECTORY\"/lib-perl.sh\n + \n +\n ## t/t9901-git-web--browse.sh ##\n @@ t/t9901-git-web--browse.sh: test_description='git web--browse basic tests\n \n10: 9cedf0cb0e2 < -: ----------- log tests: don't use \"exit 1\" outside a sub-shell\n -: ----------- > 13: 07b6572aea9 upload-pack: fix a memory leak in create_pack_file()\n -: ----------- > 14: eaa35d1bc59 CI: use \"GIT_TEST_SANITIZE_LEAK_LOG=true\" in linux-leaks\n-- \n2.37.1.1064.gc96144cf387\n\n"},{"id":"459539","messageId":"patch-v2-02.14-00af775bd0d-20220720T211221Z-avarab@gmail.com","threadId":"58191","inReplyTo":"cover-v2-00.14-00000000000-20220720T211221Z-avarab@gmail.com","subject":"[PATCH v2 02/14] test-lib: don't set GIT_EXIT_OK before calling test_atexit_handler","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-20T21:21:40Z","receivedAt":"2022-07-20T21:22:10Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the control flow in test_done so that we'll set GIT_EXIT_OK=t\nafter we call test_atexit_handler(). This seems to have been a mistake\nin 900721e15c4 (test-lib: introduce 'test_atexit', 2019-03-13). It\ndoesn't make sense to allow our \"atexit\" handling to call \"exit\"\nwithout us emitting the errors we'll emit without GIT_EXIT_OK=t being\nset.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/test-lib.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 3f11ce35112..c8c84ef9b14 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -1169,12 +1169,12 @@ test_atexit_handler () {\n }\n \n test_done () {\n-\tGIT_EXIT_OK=t\n-\n \t# Run the atexit commands _before_ the trash directory is\n \t# removed, so the commands can access pidfiles and socket files.\n \ttest_atexit_handler\n \n+\tGIT_EXIT_OK=t\n+\n \tfinalize_test_output\n \n \tif test -z \"$HARNESS_ACTIVE\"\n-- \n2.37.1.1064.gc96144cf387\n\n"},{"id":"459541","messageId":"patch-v2-03.14-419bc2c6a6e-20220720T211221Z-avarab@gmail.com","threadId":"58191","inReplyTo":"cover-v2-00.14-00000000000-20220720T211221Z-avarab@gmail.com","subject":"[PATCH v2 03/14] test-lib: fix GIT_EXIT_OK logic errors, use BAIL_OUT","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-20T21:21:41Z","receivedAt":"2022-07-20T21:22:20Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change various \"exit 1\" checks that happened after our \"die\" handler\nhad been set up to use BAIL_OUT instead. See 234383cd401 (test-lib.sh:\nuse \"Bail out!\" syntax on bad SANITIZE=leak use, 2021-10-14) for the\nbenefits of the BAIL_OUT function.\n\nThe previous use of \"error\" here was not a logic error, but the \"exit\"\nwithout \"GIT_EXIT_OK\" would emit the \"FATAL: Unexpected exit with code\n$code\" message on top of the error we wanted to emit.\n\nSince we'd also like to stop \"prove\" in its tracks here, the right\nthing to do is to emit a \"Bail out!\" message.\n\nLet's also move the \"GIT_EXIT_OK=t\" assignments to just above the\n\"exit [01]\" in \"test_done\". It's not OK if we exit in\ne.g. finalize_test_output.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/test-lib.sh | 16 ++++++----------\n 1 file changed, 6 insertions(+), 10 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex c8c84ef9b14..118720493bb 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -1173,8 +1173,6 @@ test_done () {\n \t# removed, so the commands can access pidfiles and socket files.\n \ttest_atexit_handler\n \n-\tGIT_EXIT_OK=t\n-\n \tfinalize_test_output\n \n \tif test -z \"$HARNESS_ACTIVE\"\n@@ -1246,6 +1244,7 @@ test_done () {\n \t\tfi\n \t\ttest_at_end_hook_\n \n+\t\tGIT_EXIT_OK=t\n \t\texit 0 ;;\n \n \t*)\n@@ -1255,6 +1254,7 @@ test_done () {\n \t\t\tsay \"1..$test_count\"\n \t\tfi\n \n+\t\tGIT_EXIT_OK=t\n \t\texit 1 ;;\n \n \tesac\n@@ -1387,14 +1387,12 @@ fi\n GITPERLLIB=\"$GIT_BUILD_DIR\"/perl/build/lib\n export GITPERLLIB\n test -d \"$GIT_BUILD_DIR\"/templates/blt || {\n-\terror \"You haven't built things yet, have you?\"\n+\tBAIL_OUT \"You haven't built things yet, have you?\"\n }\n \n if ! test -x \"$GIT_BUILD_DIR\"/t/helper/test-tool$X\n then\n-\techo >&2 'You need to build test-tool:'\n-\techo >&2 'Run \"make t/helper/test-tool\" in the source (toplevel) directory'\n-\texit 1\n+\tBAIL_OUT 'You need to build test-tool; Run \"make t/helper/test-tool\" in the source (toplevel) directory'\n fi\n \n # Are we running this test at all?\n@@ -1448,9 +1446,7 @@ remove_trash_directory () {\n \n # Test repository\n remove_trash_directory \"$TRASH_DIRECTORY\" || {\n-\tGIT_EXIT_OK=t\n-\techo >&5 \"FATAL: Cannot prepare test area\"\n-\texit 1\n+\tBAIL_OUT 'cannot prepare test area'\n }\n \n remove_trash=t\n@@ -1466,7 +1462,7 @@ fi\n \n # Use -P to resolve symlinks in our working directory so that the cwd\n # in subprocesses like git equals our $PWD (for pathname comparisons).\n-cd -P \"$TRASH_DIRECTORY\" || exit 1\n+cd -P \"$TRASH_DIRECTORY\" || BAIL_OUT \"cannot cd -P to \\\"$TRASH_DIRECTORY\\\"\"\n \n start_test_output \"$0\"\n \n-- \n2.37.1.1064.gc96144cf387\n\n"},{"id":"459542","messageId":"patch-v2-04.14-668c25f4d7e-20220720T211221Z-avarab@gmail.com","threadId":"58191","inReplyTo":"cover-v2-00.14-00000000000-20220720T211221Z-avarab@gmail.com","subject":"[PATCH v2 04/14] test-lib: add a --invert-exit-code switch","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-20T21:21:42Z","receivedAt":"2022-07-20T21:22:22Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add the ability to have those tests that fail return 0, and those\ntests that succeed return 1. This is useful e.g. to run \"--stress\"\ntests on tests that fail 99% of the time on some setup, i.e. to smoke\nout the flaky run which yielded success.\n\nIn a subsequent commit a new SANITIZE=leak mode will make use of this.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t0000-basic.sh | 72 ++++++++++++++++++++++++++++++++++++++++++++++++\n t/test-lib.sh | 45 ++++++++++++++++++++++++++++--\n 2 files changed, 115 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex 17a268ccd1b..502b4bcf9ea 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -578,6 +578,78 @@ test_expect_success 'subtest: --run invalid range end' '\n \tEOF_ERR\n '\n \n+test_expect_success 'subtest: --invert-exit-code without --immediate' '\n+\trun_sub_test_lib_test_err full-pass \\\n+\t\t--invert-exit-code &&\n+\tcheck_sub_test_lib_test_err full-pass \\\n+\t\t<<-\\EOF_OUT 3<<-EOF_ERR\n+\tok 1 - passing test #1\n+\tok 2 - passing test #2\n+\tok 3 - passing test #3\n+\t# passed all 3 test(s)\n+\t1..3\n+\t# faking up non-zero exit with --invert-exit-code\n+\tEOF_OUT\n+\tEOF_ERR\n+'\n+\n+test_expect_success 'subtest: --invert-exit-code with --immediate: all passed' '\n+\trun_sub_test_lib_test_err full-pass \\\n+\t\t--invert-exit-code --immediate &&\n+\tcheck_sub_test_lib_test_err full-pass \\\n+\t\t<<-\\EOF_OUT 3<<-EOF_ERR\n+\tok 1 - passing test #1\n+\tok 2 - passing test #2\n+\tok 3 - passing test #3\n+\t# passed all 3 test(s)\n+\t1..3\n+\t# faking up non-zero exit with --invert-exit-code\n+\tEOF_OUT\n+\tEOF_ERR\n+'\n+\n+test_expect_success 'subtest: --invert-exit-code without --immediate: partial pass' '\n+\trun_sub_test_lib_test partial-pass \\\n+\t\t--invert-exit-code &&\n+\tcheck_sub_test_lib_test partial-pass <<-\\EOF\n+\tok 1 - passing test #1\n+\tnot ok 2 - # TODO induced breakage (--invert-exit-code): failing test #2\n+\t#\tfalse\n+\tok 3 - passing test #3\n+\t# failed 1 among 3 test(s)\n+\t1..3\n+\t# faked up failures as TODO & now exiting with 0 due to --invert-exit-code\n+\tEOF\n+'\n+\n+test_expect_success 'subtest: --invert-exit-code with --immediate: partial pass' '\n+\trun_sub_test_lib_test partial-pass \\\n+\t\t--invert-exit-code --immediate &&\n+\tcheck_sub_test_lib_test partial-pass \\\n+\t\t<<-\\EOF_OUT 3<<-EOF_ERR\n+\tok 1 - passing test #1\n+\tnot ok 2 - # TODO induced breakage (--invert-exit-code): failing test #2\n+\t#\tfalse\n+\t1..2\n+\t# faked up failures as TODO & now exiting with 0 due to --invert-exit-code\n+\tEOF_OUT\n+\tEOF_ERR\n+'\n+\n+test_expect_success 'subtest: --invert-exit-code --immediate: got a failure' '\n+\trun_sub_test_lib_test partial-pass \\\n+\t\t--invert-exit-code --immediate &&\n+\tcheck_sub_test_lib_test_err partial-pass \\\n+\t\t<<-\\EOF_OUT 3<<-EOF_ERR\n+\tok 1 - passing test #1\n+\tnot ok 2 - # TODO induced breakage (--invert-exit-code): failing test #2\n+\t#\tfalse\n+\t1..2\n+\t# faked up failures as TODO & now exiting with 0 due to --invert-exit-code\n+\tEOF_OUT\n+\tEOF_ERR\n+'\n+\n test_expect_success 'subtest: tests respect prerequisites' '\n \twrite_and_run_sub_test_lib_test prereqs <<-\\EOF &&\n \ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 118720493bb..31213b5f95b 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -238,6 +238,9 @@ parse_option () {\n \t\t\t;;\n \t\tesac\n \t\t;;\n+\t--invert-exit-code)\n+\t\tinvert_exit_code=t\n+\t\t;;\n \t*)\n \t\techo \"error: unknown test option '$opt'\" >&2; exit 1 ;;\n \tesac\n@@ -788,15 +791,31 @@ test_ok_ () {\n \tfinalize_test_case_output ok \"$@\"\n }\n \n+_invert_exit_code_failure_end_blurb () {\n+\tsay_color warn \"# faked up failures as TODO & now exiting with 0 due to --invert-exit-code\"\n+}\n+\n test_failure_ () {\n \tfailure_label=$1\n \ttest_failure=$(($test_failure + 1))\n-\tsay_color error \"not ok $test_count - $1\"\n+\tlocal pfx=\"\"\n+\tif test -n \"$invert_exit_code\" # && test -n \"$HARNESS_ACTIVE\"\n+\tthen\n+\t\tpfx=\"# TODO induced breakage (--invert-exit-code):\"\n+\tfi\n+\tsay_color error \"not ok $test_count - ${pfx:+$pfx }$1\"\n \tshift\n \tprintf '%s\\n' \"$*\" | sed -e 's/^/#\t/'\n \tif test -n \"$immediate\"\n \tthen\n \t\tsay_color error \"1..$test_count\"\n+\t\tif test -n \"$invert_exit_code\"\n+\t\tthen\n+\t\t\tfinalize_test_output\n+\t\t\t_invert_exit_code_failure_end_blurb\n+\t\t\tGIT_EXIT_OK=t\n+\t\t\texit 0\n+\t\tfi\n \t\t_error_exit\n \tfi\n \tfinalize_test_case_output failure \"$failure_label\" \"$@\"\n@@ -1229,7 +1248,14 @@ test_done () {\n \t\t\tesac\n \t\tfi\n \n-\t\tif test -z \"$debug\" && test -n \"$remove_trash\"\n+\t\tif test -n \"$stress\" && test -n \"$invert_exit_code\"\n+\t\tthen\n+\t\t\t# We're about to move our \"$TRASH_DIRECTORY\"\n+\t\t\t# to \"$TRASH_DIRECTORY.stress-failed\" if\n+\t\t\t# --stress is combined with\n+\t\t\t# --invert-exit-code.\n+\t\t\tsay \"with --stress and --invert-exit-code we're not removing '$TRASH_DIRECTORY'\"\n+\t\telif test -z \"$debug\" && test -n \"$remove_trash\"\n \t\tthen\n \t\t\ttest -d \"$TRASH_DIRECTORY\" ||\n \t\t\terror \"Tests passed but trash directory already removed before test cleanup; aborting\"\n@@ -1242,6 +1268,14 @@ test_done () {\n \t\t\t} ||\n \t\t\terror \"Tests passed but test cleanup failed; aborting\"\n \t\tfi\n+\n+\t\tif test -z \"$skip_all\" && test -n \"$invert_exit_code\"\n+\t\tthen\n+\t\t\tsay_color warn \"# faking up non-zero exit with --invert-exit-code\"\n+\t\t\tGIT_EXIT_OK=t\n+\t\t\texit 1\n+\t\tfi\n+\n \t\ttest_at_end_hook_\n \n \t\tGIT_EXIT_OK=t\n@@ -1254,6 +1288,13 @@ test_done () {\n \t\t\tsay \"1..$test_count\"\n \t\tfi\n \n+\t\tif test -n \"$invert_exit_code\"\n+\t\tthen\n+\t\t\t_invert_exit_code_failure_end_blurb\n+\t\t\tGIT_EXIT_OK=t\n+\t\t\texit 0\n+\t\tfi\n+\n \t\tGIT_EXIT_OK=t\n \t\texit 1 ;;\n \n-- \n2.37.1.1064.gc96144cf387\n\n"},{"id":"459543","messageId":"patch-v2-05.14-a26cb02db0a-20220720T211221Z-avarab@gmail.com","threadId":"58191","inReplyTo":"cover-v2-00.14-00000000000-20220720T211221Z-avarab@gmail.com","subject":"[PATCH v2 05/14] t/README: reword the \"GIT_TEST_PASSING_SANITIZE_LEAK\" description","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-20T21:21:43Z","receivedAt":"2022-07-20T21:22:23Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Reword the documentation added in 956d2e4639b (tests: add a test mode\nfor SANITIZE=leak, run it in CI, 2021-09-23) for brevity, and to avoid\na merge conflict with another topic, per [1].\n\n1. https://lore.kernel.org/git/220719.86lespun6l.gmgdl@evledraar.gmail.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/README | 10 ++++------\n 1 file changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 4f9981cf5e3..7f409392537 100644\n--- a/t/README\n+++ b/t/README\n@@ -366,12 +366,10 @@ excluded as so much relies on it, but this might change in the future.\n GIT_TEST_SPLIT_INDEX= forces split-index mode on the whole\n test suite. Accept any boolean values that are accepted by git-config.\n \n-GIT_TEST_PASSING_SANITIZE_LEAK= when compiled with\n-SANITIZE=leak will run only those tests that have whitelisted\n-themselves as passing with no memory leaks. Tests can be whitelisted\n-by setting \"TEST_PASSES_SANITIZE_LEAK=true\" before sourcing\n-\"test-lib.sh\" itself at the top of the test script. This test mode is\n-used by the \"linux-leaks\" CI target.\n+GIT_TEST_PASSING_SANITIZE_LEAK=true skips those tests that haven't\n+declared themselves as leak-free by setting\n+\"TEST_PASSES_SANITIZE_LEAK=true\" before sourcing \"test-lib.sh\". This\n+test mode is used by the \"linux-leaks\" CI target.\n \n GIT_TEST_PROTOCOL_VERSION=, when set, makes 'protocol.version'\n default to n.\n-- \n2.37.1.1064.gc96144cf387\n\n"},{"id":"459544","messageId":"patch-v2-06.14-f1acf762899-20220720T211221Z-avarab@gmail.com","threadId":"58191","inReplyTo":"cover-v2-00.14-00000000000-20220720T211221Z-avarab@gmail.com","subject":"[PATCH v2 06/14] test-lib: add a SANITIZE=leak logging mode","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-20T21:21:44Z","receivedAt":"2022-07-20T21:22:25Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add the ability to run the test suite under a new\n\"GIT_TEST_SANITIZE_LEAK_LOG=true\" mode, when true we'll log the leaks\nwe find an a new \"test-results/.leak\" directory.\n\nThat new path is consistent with the existing\n\"test-results/.\" results, except that those are all\nfiles, not directories.\n\nWe also set \"log_exe_name=1\" to include the name of the executable in\nthe filename. This gives us files like \"trace.git.\" instead of\nthe default of \"trace.\". I.e. we'll be able to distinguish \"git\"\nleaks from \"test-tool\", \"git-daemon\" etc.\n\nWe then set \"dedup_token_length\" to non-zero (\"0\" is the default) to\nsuccinctly log a token we can de-duplicate these stacktraces on. The\nstring is simply a one-line stack-trace with only function names up to\nN frames, which we limit at \"9999\" as a shorthand for\n\"infinite\" (there appears to be no way to say \"no limit\").\n\nWith these combined we can now easily get e.g. the top 10 leaks in the\ntest suite grouped by full stacktrace:\n\n grep -o -P -h '(?<=DEDUP_TOKEN: ).*' test-results/*.leak/trace.git.* | sort | uniq -c | sort -nr | head -n 10\n\nOr add \"grep -E -o '[^-]+'\" to that to group by functions instead of\nstack traces:\n\n grep -o -P -h '(?<=DEDUP_TOKEN: ).*' test-results/*.leak/trace.git.* | grep -E -o '[^-]+' | sort | uniq -c | sort -nr | head -n 20\n\nThis new mode requires git to be compiled with SANITIZE=leak, rather\nthan explaining that in the documentation let's make it\nself-documenting by bailing out if the user asks for this without git\nhaving been compiled with SANITIZE=leak, as we do with\nGIT_TEST_PASSING_SANITIZE_LEAK=true.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/README | 5 +++++\n t/test-lib.sh | 30 +++++++++++++++++++++++++++++-\n 2 files changed, 34 insertions(+), 1 deletion(-)\n\ndiff --git a/t/README b/t/README\nindex 7f409392537..0664aee7ed9 100644\n--- a/t/README\n+++ b/t/README\n@@ -371,6 +371,11 @@ declared themselves as leak-free by setting\n \"TEST_PASSES_SANITIZE_LEAK=true\" before sourcing \"test-lib.sh\". This\n test mode is used by the \"linux-leaks\" CI target.\n \n+GIT_TEST_SANITIZE_LEAK_LOG=true will log memory leaks to\n+\"test-results/$TEST_NAME.leak/trace.*\" files. The logs include a\n+\"dedup_token\" (see +\"ASAN_OPTIONS=help=1 ./git\") and other options to\n+make logs +machine-readable.\n+\n GIT_TEST_PROTOCOL_VERSION=, when set, makes 'protocol.version'\n default to n.\n \ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 31213b5f95b..f5150400c14 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -305,6 +305,10 @@ TEST_NUMBER=\"${TEST_NAME%%-*}\"\n TEST_NUMBER=\"${TEST_NUMBER#t}\"\n TEST_RESULTS_DIR=\"$TEST_OUTPUT_DIRECTORY/test-results\"\n TEST_RESULTS_BASE=\"$TEST_RESULTS_DIR/$TEST_NAME$TEST_STRESS_JOB_SFX\"\n+TEST_RESULTS_SAN_FILE_PFX=trace\n+TEST_RESULTS_SAN_DIR_SFX=leak\n+TEST_RESULTS_SAN_FILE=\n+TEST_RESULTS_SAN_DIR=\"$TEST_RESULTS_DIR/$TEST_NAME.$TEST_RESULTS_SAN_DIR_SFX\"\n TRASH_DIRECTORY=\"trash directory.$TEST_NAME$TEST_STRESS_JOB_SFX\"\n test -n \"$root\" && TRASH_DIRECTORY=\"$root/$TRASH_DIRECTORY\"\n case \"$TRASH_DIRECTORY\" in\n@@ -1447,6 +1451,10 @@ then\n \ttest_done\n fi\n \n+BAIL_OUT_ENV_NEEDS_SANITIZE_LEAK () {\n+\tBAIL_OUT \"$1 has no effect except when compiled with SANITIZE=leak\"\n+}\n+\n # skip non-whitelisted tests when compiled with SANITIZE=leak\n if test -n \"$SANITIZE_LEAK\"\n then\n@@ -1462,9 +1470,29 @@ then\n \t\t\ttest_done\n \t\tfi\n \tfi\n+\n+\tif test_bool_env GIT_TEST_SANITIZE_LEAK_LOG false\n+\tthen\n+\t\tif ! mkdir -p \"$TEST_RESULTS_SAN_DIR\"\n+\t\tthen\n+\t\t\tBAIL_OUT \"cannot create $TEST_RESULTS_SAN_DIR\"\n+\t\tfi &&\n+\t\tTEST_RESULTS_SAN_FILE=\"$TEST_RESULTS_SAN_DIR/$TEST_RESULTS_SAN_FILE_PFX\"\n+\n+\t\t# Don't litter *.leak dirs if there was nothing to report\n+\t\ttest_atexit \"rmdir \\\"$TEST_RESULTS_SAN_DIR\\\" 2>/dev/null || :\"\n+\n+\t\tprepend_var LSAN_OPTIONS : dedup_token_length=9999\n+\t\tprepend_var LSAN_OPTIONS : log_exe_name=1\n+\t\tprepend_var LSAN_OPTIONS : log_path=\\\"$TEST_RESULTS_SAN_FILE\\\"\n+\t\texport LSAN_OPTIONS\n+\tfi\n elif test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false\n then\n-\tBAIL_OUT \"GIT_TEST_PASSING_SANITIZE_LEAK=true has no effect except when compiled with SANITIZE=leak\"\n+\tBAIL_OUT_ENV_NEEDS_SANITIZE_LEAK \"GIT_TEST_PASSING_SANITIZE_LEAK=true\"\n+elif test_bool_env GIT_TEST_SANITIZE_LEAK_LOG false\n+then\n+\tBAIL_OUT_ENV_NEEDS_SANITIZE_LEAK \"GIT_TEST_SANITIZE_LEAK_LOG=true\"\n fi\n \n # Last-minute variable setup\n-- \n2.37.1.1064.gc96144cf387\n\n"},{"id":"459545","messageId":"patch-v2-07.14-0723e90df7b-20220720T211221Z-avarab@gmail.com","threadId":"58191","inReplyTo":"cover-v2-00.14-00000000000-20220720T211221Z-avarab@gmail.com","subject":"[PATCH v2 07/14] t/Makefile: don't remove test-results in \"clean-except-prove-cache\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-20T21:21:45Z","receivedAt":"2022-07-20T21:22:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"When \"make test\" is run with the default of \"DEFAULT_TEST_TARGET=test\"\nwe'll leave the \"test-results\" directory in-place, but don't do so for\nthe \"prove\" target.\n\nThe reason for this is that when 28d836c8158 (test: allow running the\ntests under \"prove\", 2010-10-14) allowed for running the tests under\n\"prove\" there was no point in leaving the \"test-results\" in place.\n\nThe \"prove\" target provides its own summary, so we don't need to run\n\"aggregate-results\", which is the reason we have \"test-results\" in the\nfirst place. See 2d84e9fb6d2 (Modify test-lib.sh to output stats to\nt/test-results/*, 2008-06-08).\n\nBut in a subsequent commit test-lib.sh will start emitting reports of\nmemory leaks in test-results/*, and it will be useful to analyze these\nafter the fact.\n\nThis wouldn't be a problem as failing tests will halt the removal of\nthe files (we'll never reach \"clean-except-prove-cache\" from the\n\"prove\" target), but will be subsequently as we'll want to report a\nsuccessful run, but might still have e.g. logs of known memory leaks\nin test-results/*.\n\nSo let's stop removing this, it's sufficient that \"make clean\" removes\nit, and that \"pre-clean\" (which both \"test\" and \"prove\" depend on)\nwill remove it, i.e. we'll never have a stale \"test-results\" because\nof this change.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n contrib/scalar/t/Makefile | 2 +-\n contrib/subtree/t/Makefile | 2 +-\n t/Makefile | 2 +-\n 3 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/contrib/scalar/t/Makefile b/contrib/scalar/t/Makefile\nindex 01e82e56d15..1ed174a8cf3 100644\n--- a/contrib/scalar/t/Makefile\n+++ b/contrib/scalar/t/Makefile\n@@ -42,7 +42,7 @@ $(T):\n \t@echo \"*** $@ ***\"; GIT_CONFIG=.git/config '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)\n \n clean-except-prove-cache:\n-\t$(RM) -r 'trash directory'.* '$(TEST_RESULTS_DIRECTORY_SQ)'\n+\t$(RM) -r 'trash directory'.*\n \t$(RM) -r valgrind/bin\n \n clean: clean-except-prove-cache\ndiff --git a/contrib/subtree/t/Makefile b/contrib/subtree/t/Makefile\nindex 276898eb6bd..3d278bb0edb 100644\n--- a/contrib/subtree/t/Makefile\n+++ b/contrib/subtree/t/Makefile\n@@ -47,7 +47,7 @@ pre-clean:\n \t$(RM) -r '$(TEST_RESULTS_DIRECTORY_SQ)'\n \n clean-except-prove-cache:\n-\t$(RM) -r 'trash directory'.* '$(TEST_RESULTS_DIRECTORY_SQ)'\n+\t$(RM) -r 'trash directory'.*\n \t$(RM) -r valgrind/bin\n \n clean: clean-except-prove-cache\ndiff --git a/t/Makefile b/t/Makefile\nindex 7f56e52f767..1c80c0c79a0 100644\n--- a/t/Makefile\n+++ b/t/Makefile\n@@ -62,7 +62,7 @@ pre-clean:\n \t$(RM) -r '$(TEST_RESULTS_DIRECTORY_SQ)'\n \n clean-except-prove-cache: clean-chainlint\n-\t$(RM) -r 'trash directory'.* '$(TEST_RESULTS_DIRECTORY_SQ)'\n+\t$(RM) -r 'trash directory'.*\n \t$(RM) -r valgrind/bin\n \n clean: clean-except-prove-cache\n-- \n2.37.1.1064.gc96144cf387\n\n"},{"id":"459546","messageId":"patch-v2-08.14-987d9d0e98c-20220720T211221Z-avarab@gmail.com","threadId":"58191","inReplyTo":"cover-v2-00.14-00000000000-20220720T211221Z-avarab@gmail.com","subject":"[PATCH v2 08/14] tests: move copy/pasted PERL + Test::More checks to a lib-perl.sh","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-20T21:21:46Z","receivedAt":"2022-07-20T21:22:37Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Since the original \"perl -MTest::More\" prerequisite check was added in\n[1] it's been copy/pasted in [2], [3] and [4]. As we'll be changing\nthese codepaths in a subsequent commit let's consolidate these.\n\nWhile we're at it let's move these to a lazy prereq, and make them\nconform to our usual coding style (e.g. \"\\nthen\", not \"; then\").\n\n1. e46f9c8161a (t9700: skip when Test::More is not available,\n 2008-06-29)\n2. 5e9637c6297 (i18n: add infrastructure for translating Git with\n gettext, 2011-11-18)\n3. 8d314d7afec (send-email: reduce dependencies impact on\n parse_address_line, 2015-07-07)\n4. f07eeed123b (git-credential-netrc: adapt to test framework for git,\n 2018-05-12)\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n .../netrc/t-git-credential-netrc.sh | 11 ++---------\n t/lib-perl.sh | 19 +++++++++++++++++++\n t/t0202-gettext-perl.sh | 12 ++----------\n t/t9700-perl-git.sh | 11 ++---------\n 4 files changed, 25 insertions(+), 28 deletions(-)\n create mode 100644 t/lib-perl.sh\n\ndiff --git a/contrib/credential/netrc/t-git-credential-netrc.sh b/contrib/credential/netrc/t-git-credential-netrc.sh\nindex 07227d02287..ff17a9460cd 100755\n--- a/contrib/credential/netrc/t-git-credential-netrc.sh\n+++ b/contrib/credential/netrc/t-git-credential-netrc.sh\n@@ -3,16 +3,9 @@\n \tcd ../../../t\n \ttest_description='git-credential-netrc'\n \t. ./test-lib.sh\n+\t. \"$TEST_DIRECTORY\"/lib-perl.sh\n \n-\tif ! test_have_prereq PERL; then\n-\t\tskip_all='skipping perl interface tests, perl not available'\n-\t\ttest_done\n-\tfi\n-\n-\tperl -MTest::More -e 0 2>/dev/null || {\n-\t\tskip_all=\"Perl Test::More unavailable, skipping test\"\n-\t\ttest_done\n-\t}\n+\tskip_all_if_no_Test_More\n \n \t# set up test repository\n \ndiff --git a/t/lib-perl.sh b/t/lib-perl.sh\nnew file mode 100644\nindex 00000000000..d0bf509a167\n--- /dev/null\n+++ b/t/lib-perl.sh\n@@ -0,0 +1,19 @@\n+# Copyright (c) 2022 Ævar Arnfjörð Bjarmason\n+\n+test_lazy_prereq PERL_TEST_MORE '\n+\tperl -MTest::More -e 0\n+'\n+\n+skip_all_if_no_Test_More () {\n+\tif ! test_have_prereq PERL\n+\tthen\n+\t\tskip_all='skipping perl interface tests, perl not available'\n+\t\ttest_done\n+\tfi\n+\n+\tif ! test_have_prereq PERL_TEST_MORE\n+\tthen\n+\t\tskip_all=\"Perl Test::More unavailable, skipping test\"\n+\t\ttest_done\n+\tfi\n+}\ndiff --git a/t/t0202-gettext-perl.sh b/t/t0202-gettext-perl.sh\nindex df2ea34932b..043b190626c 100755\n--- a/t/t0202-gettext-perl.sh\n+++ b/t/t0202-gettext-perl.sh\n@@ -7,16 +7,8 @@ test_description='Perl gettext interface (Git::I18N)'\n \n TEST_PASSES_SANITIZE_LEAK=true\n . ./lib-gettext.sh\n-\n-if ! test_have_prereq PERL; then\n-\tskip_all='skipping perl interface tests, perl not available'\n-\ttest_done\n-fi\n-\n-perl -MTest::More -e 0 2>/dev/null || {\n-\tskip_all=\"Perl Test::More unavailable, skipping test\"\n-\ttest_done\n-}\n+. \"$TEST_DIRECTORY\"/lib-perl.sh\n+skip_all_if_no_Test_More\n \n # The external test will outputs its own plan\n test_external_has_tap=1\ndiff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh\nindex 102c133112c..17fc43f6e57 100755\n--- a/t/t9700-perl-git.sh\n+++ b/t/t9700-perl-git.sh\n@@ -5,16 +5,9 @@\n \n test_description='perl interface (Git.pm)'\n . ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-perl.sh\n \n-if ! test_have_prereq PERL; then\n-\tskip_all='skipping perl interface tests, perl not available'\n-\ttest_done\n-fi\n-\n-perl -MTest::More -e 0 2>/dev/null || {\n-\tskip_all=\"Perl Test::More unavailable, skipping test\"\n-\ttest_done\n-}\n+skip_all_if_no_Test_More\n \n # set up test repository\n \n-- \n2.37.1.1064.gc96144cf387\n\n"},{"id":"459547","messageId":"patch-v2-10.14-78a47d2b348-20220720T211221Z-avarab@gmail.com","threadId":"58191","inReplyTo":"cover-v2-00.14-00000000000-20220720T211221Z-avarab@gmail.com","subject":"[PATCH v2 10/14] test-lib: add a GIT_TEST_PASSING_SANITIZE_LEAK=check mode","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-20T21:21:48Z","receivedAt":"2022-07-20T21:22:41Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a new \"GIT_TEST_PASSING_SANITIZE_LEAK=check\" mode to the\ntest-lib.sh.\n\nAs noted in the updated \"t/README\" this compliments the existing\n\"GIT_TEST_PASSING_SANITIZE_LEAK=true\" mode added in\n956d2e4639b (tests: add a test mode for SANITIZE=leak, run it in CI,\n2021-09-23).\n\nRather than document this all in one (even more) dense paragraph split\nup the discussion of how it combines with --immediate into its own\nparagraph following the discussion of\n\"GIT_TEST_SANITIZE_LEAK_LOG=true\".\n\nBefore the removal of \"test_external\" in a preceding commit we would\nhave had to special-case t9700-perl-git.sh and t0202-gettext-perl.sh.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/README | 17 +++++++++++++++++\n t/test-lib.sh | 34 ++++++++++++++++++++++++++--------\n 2 files changed, 43 insertions(+), 8 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 98f69ed13d2..e13063195e5 100644\n--- a/t/README\n+++ b/t/README\n@@ -371,11 +371,28 @@ declared themselves as leak-free by setting\n \"TEST_PASSES_SANITIZE_LEAK=true\" before sourcing \"test-lib.sh\". This\n test mode is used by the \"linux-leaks\" CI target.\n \n+GIT_TEST_PASSING_SANITIZE_LEAK=check checks that our\n+\"TEST_PASSES_SANITIZE_LEAK=true\" markings are current. Rather than\n+skipping those tests that haven't set \"TEST_PASSES_SANITIZE_LEAK=true\"\n+before sourcing \"test-lib.sh\" this mode runs them with\n+\"--invert-exit-code\". This is used to check that there's a one-to-one\n+mapping between \"TEST_PASSES_SANITIZE_LEAK=true\" and those tests that\n+pass under \"SANITIZE=leak\". This is especially useful when testing a\n+series that fixes various memory leaks with \"git rebase -x\".\n+\n GIT_TEST_SANITIZE_LEAK_LOG=true will log memory leaks to\n \"test-results/$TEST_NAME.leak/trace.*\" files. The logs include a\n \"dedup_token\" (see +\"ASAN_OPTIONS=help=1 ./git\") and other options to\n make logs +machine-readable.\n \n+GIT_TEST_PASSING_SANITIZE_LEAK=check when combined with \"--immediate\"\n+will run to completion faster, and result in the same failing\n+tests. The only practical reason to run\n+GIT_TEST_PASSING_SANITIZE_LEAK=check without \"--immediate\" is to\n+combine it with \"GIT_TEST_SANITIZE_LEAK_LOG=true\". If we stop at the\n+first failing test case our leak logs won't show subsequent leaks we\n+might have run into.\n+\n GIT_TEST_PROTOCOL_VERSION=, when set, makes 'protocol.version'\n default to n.\n \ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 453d980bcf7..5785eaf08dc 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -1452,17 +1452,34 @@ BAIL_OUT_ENV_NEEDS_SANITIZE_LEAK () {\n # skip non-whitelisted tests when compiled with SANITIZE=leak\n if test -n \"$SANITIZE_LEAK\"\n then\n-\tif test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false\n+\t# Normalize with test_bool_env\n+\tpasses_sanitize_leak=\n+\n+\t# We need to see TEST_PASSES_SANITIZE_LEAK in \"git\n+\t# env--helper\" (via test_bool_env)\n+\texport TEST_PASSES_SANITIZE_LEAK\n+\tif test_bool_env TEST_PASSES_SANITIZE_LEAK false\n+\tthen\n+\t\tpasses_sanitize_leak=t\n+\tfi\n+\n+\tif test \"$GIT_TEST_PASSING_SANITIZE_LEAK\" = \"check\"\n \tthen\n-\t\t# We need to see it in \"git env--helper\" (via\n-\t\t# test_bool_env)\n-\t\texport TEST_PASSES_SANITIZE_LEAK\n+\t\tif test -n \"$invert_exit_code\"\n+\t\tthen\n+\t\t\tBAIL_OUT \"cannot use --invert-exit-code under GIT_TEST_PASSING_SANITIZE_LEAK=check\"\n+\t\tfi\n \n-\t\tif ! test_bool_env TEST_PASSES_SANITIZE_LEAK false\n+\t\tif test -z \"$passes_sanitize_leak\"\n \t\tthen\n-\t\t\tskip_all=\"skipping $this_test under GIT_TEST_PASSING_SANITIZE_LEAK=true\"\n-\t\t\ttest_done\n+\t\t\tsay \"in GIT_TEST_PASSING_SANITIZE_LEAK=check mode, setting --invert-exit-code for TEST_PASSES_SANITIZE_LEAK != true\"\n+\t\t\tinvert_exit_code=t\n \t\tfi\n+\telif test -z \"$passes_sanitize_leak\" &&\n+\t test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false\n+\tthen\n+\t\tskip_all=\"skipping $this_test under GIT_TEST_PASSING_SANITIZE_LEAK=true\"\n+\t\ttest_done\n \tfi\n \n \tif test_bool_env GIT_TEST_SANITIZE_LEAK_LOG false\n@@ -1481,7 +1498,8 @@ then\n \t\tprepend_var LSAN_OPTIONS : log_path=\\\"$TEST_RESULTS_SAN_FILE\\\"\n \t\texport LSAN_OPTIONS\n \tfi\n-elif test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false\n+elif test \"$GIT_TEST_PASSING_SANITIZE_LEAK\" = \"check\" ||\n+ test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false\n then\n \tBAIL_OUT_ENV_NEEDS_SANITIZE_LEAK \"GIT_TEST_PASSING_SANITIZE_LEAK=true\"\n elif test_bool_env GIT_TEST_SANITIZE_LEAK_LOG false\n-- \n2.37.1.1064.gc96144cf387\n\n"},{"id":"459548","messageId":"patch-v2-09.14-20bd31615e4-20220720T211221Z-avarab@gmail.com","threadId":"58191","inReplyTo":"cover-v2-00.14-00000000000-20220720T211221Z-avarab@gmail.com","subject":"[PATCH v2 09/14] test-lib: simplify by removing test_external","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-07-20T21:21:47Z","receivedAt":"2022-07-20T21:22:43Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Remove the \"test_external\" function added in [1]. This arguably makes\nthe output of t9700-perl-git.sh and friends worse. But as we'll argue\nbelow the trade-off is worth it, since \"chaining\" to another TAP\nemitter in test-lib.sh is more trouble than it's worth.\n\nThe new output of t9700-perl-git.sh is now:\n\n\t$ ./t9700-perl-git.sh\n\tok 1 - set up test repository\n\tok 2 - use t9700/test.pl to test Git.pm\n\t# passed all 2 test(s)\n\t1..2\n\nWhereas before this change it would be:\n\n\t$ ./t9700-perl-git.sh\n\tok 1 - set up test repository\n\t# run 1: Perl API (perl /home/avar/g/git/t/t9700/test.pl)\n\tok 2 - use Git;\n\t[... omitting tests 3..46 from t/t9700/test.pl ...]\n\tok 47 - unquote escape sequences\n\t1..47\n\t# test_external test Perl API was ok\n\t# test_external_without_stderr test no stderr: Perl API was ok\n\nAt the time of its addition supporting \"test_external\" was easy, but\nwhen test-lib.sh itself started to emit TAP in [2] we needed to make\neverything surrounding the emission of the plan consider\n\"test_external\". I added that support in [2] so that we could run:\n\n\tprove ./t9700-perl-git.sh :: -v\n\nBut since then in [3] the door has been closed on combining\n$HARNESS_ACTIVE and -v, we'll now just die:\n\n\t$ prove ./t9700-perl-git.sh :: -v\n\tBailout called. Further testing stopped: verbose mode forbidden under TAP harness; try --verbose-log\n\tFAILED--Further testing stopped: verbose mode forbidden under TAP harness; try --verbose-log\n\nSo the only use of this has been that *if* we had failure in one of\nthese tests we could e.g. in CI see which test failed based on the\ntest number. Now we'll need to look at the full verbose logs to get\nthat same information.\n\nI think this trade-off is acceptable given the reduction in\ncomplexity, and it brings these tests in line with other similar\ntests, e.g. the reftable tests added in [4] will be condensed down to\njust one test, which invokes the C helper:\n\n\t$ ./t0032-reftable-unittest.sh\n\tok 1 - unittests\n\t# passed all 1 test(s)\n\t1..1\n\nIt would still be nice to have that \":: -v\" form work again, it\nnever *really* worked, but even though we've had edge cases test\noutput screwing up the TAP it mostly worked between d998bd4ab67 and\n[3], so we may have been overzealous in forbidding it outright.\n\nI have local patches which I'm planning to submit sooner than later\nthat get us to that goal, and in a way that isn't buggy. In the\nmeantime getting rid of this special case makes hacking on this area\nof test-lib.sh easier, as we'll do in subsequent commits.\n\nThe switch from \"perl\" to \"$PERL_PATH\" here is because \"perl\" is\ndefined as a shell function in the test suite, see a5bf824f3b4 (t:\nprevent '-x' tracing from interfering with test helpers' stderr,\n2018-02-25). On e.g. the OSX CI the \"command perl\"... will be part of\nthe emitted stderr.\n\n1. fb32c410087 (t/test-lib.sh: add test_external and\n test_external_without_stderr, 2008-06-19)\n2. d998bd4ab67 (test-lib: Make the test_external_* functions\n TAP-aware, 2010-06-24)\n3. 614fe015212 (test-lib: bail out when \"-v\" used under\n \"prove\", 2016-10-22)\n4. ef8a6c62687 (reftable: utility functions, 2021-10-07)\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n .../netrc/t-git-credential-netrc.sh | 7 +-\n t/README | 26 ------\n t/t0202-gettext-perl.sh | 10 +--\n t/t9700-perl-git.sh | 10 +--\n t/test-lib-functions.sh | 89 +------------------\n t/test-lib.sh | 40 ++++-----\n 6 files changed, 28 insertions(+), 154 deletions(-)\n\ndiff --git a/contrib/credential/netrc/t-git-credential-netrc.sh b/contrib/credential/netrc/t-git-credential-netrc.sh\nindex ff17a9460cd..bf2777308a5 100755\n--- a/contrib/credential/netrc/t-git-credential-netrc.sh\n+++ b/contrib/credential/netrc/t-git-credential-netrc.sh\n@@ -13,13 +13,10 @@\n \t\t'set up test repository' \\\n \t\t'git config --add gpg.program test.git-config-gpg'\n \n-\t# The external test will outputs its own plan\n-\ttest_external_has_tap=1\n-\n \texport PERL5LIB=\"$GITPERLLIB\"\n-\ttest_external \\\n-\t\t'git-credential-netrc' \\\n+\ttest_expect_success 'git-credential-netrc' '\n \t\tperl \"$GIT_BUILD_DIR\"/contrib/credential/netrc/test.pl\n+\t'\n \n \ttest_done\n )\ndiff --git a/t/README b/t/README\nindex 0664aee7ed9..98f69ed13d2 100644\n--- a/t/README\n+++ b/t/README\n@@ -938,32 +938,6 @@ see test-lib-functions.sh for the full list and their options.\n \t test_done\n \tfi\n \n- - test_external []