On 7/19/2022 5:05 PM, Ævar Arnfjörð Bjarmason wrote: > Mark those remaining tests that pass when run under SANITIZE=leak with > TEST_PASSES_SANITIZE_LEAK=true, these were either omitted in > f346fcb62a0 (Merge branch 'ab/mark-leak-free-tests-even-more', > 2021-12-15) and 5a4f8381b68 (Merge branch 'ab/mark-leak-free-tests', > 2021-10-25), or have had their memory leaks fixed since then. > > With this change there's now a a 1=1 mapping between those tests that nit: here's another use of "1=1" which I read as "one equals one" and not "one-to-one", so please expand into the words. > we have opted-in via "TEST_PASSES_SANITIZE_LEAK=true", and those that > pass with the new "check" mode: > > GIT_TEST_PASSING_SANITIZE_LEAK=check GIT_TEST_SANITIZE_LEAK_LOG=true make test SANITIZE=leak Maybe split this line? GIT_TEST_PASSING_SANITIZE_LEAK=check \ GIT_TEST_SANITIZE_LEAK_LOG=true \ make test SANITIZE=leak > Note that the "GIT_TEST_SANITIZE_LEAK_LOG=true" is needed due to the > edge cases noted in a preceding commit, i.e. in some cases we'd pass > the test itself, but still have outstanding leaks due to ignored exit > codes. > > The "GIT_TEST_SANITIZE_LEAK_LOG=true" corrects for that, we're only > marking those tests as passing that really don't have any leaks, > whether that was reflected in their exit code or not. This paragraph repeats the previous one, but with different words. Consider removing it. > t/t0027-auto-crlf.sh | 1 + > t/t0032-reftable-unittest.sh | 1 + > t/t0033-safe-directory.sh | 1 + > t/t0050-filesystem.sh | 1 + > t/t0095-bloom.sh | 2 ++ > t/t1405-main-ref-store.sh | 1 + > t/t1407-worktree-ref-store.sh | 1 + > t/t1418-reflog-exists.sh | 1 + > t/t1701-racy-split-index.sh | 1 + > t/t2006-checkout-index-basic.sh | 1 + > t/t2023-checkout-m.sh | 1 + > t/t2205-add-worktree-config.sh | 1 + > t/t3012-ls-files-dedup.sh | 1 + > t/t4017-diff-retval.sh | 1 + > t/t4051-diff-function-context.sh | 1 + > t/t4057-diff-combined-paths.sh | 1 + > t/t4114-apply-typechange.sh | 1 + > t/t4301-merge-tree-write-tree.sh | 1 + > t/t5315-pack-objects-compression.sh | 1 + > t/t5351-unpack-large-objects.sh | 1 + > t/t5402-post-merge-hook.sh | 1 + > t/t5503-tagfollow.sh | 1 + > t/t6404-recursive-merge.sh | 1 + > t/t6405-merge-symlinks.sh | 1 + > t/t6408-merge-up-to-date.sh | 1 + > t/t6411-merge-filemode.sh | 1 + > t/t6413-merge-crlf.sh | 1 + > t/t6415-merge-dir-to-symlink.sh | 1 + > t/t6425-merge-rename-delete.sh | 1 + > t/t6431-merge-criscross.sh | 1 + > t/t7060-wtstatus.sh | 1 + > t/t7062-wtstatus-ignorecase.sh | 1 + > t/t7110-reset-merge.sh | 1 + > t/t7111-reset-table.sh | 1 + > t/t7609-mergetool--lib.sh | 1 + > t/t9100-git-svn-basic.sh | 1 - > t/t9901-git-web--browse.sh | 1 + That's a lot of tests that we can mark as having no leaks. Nice. I imagine that after this series cooks for a while we can consider running this check in CI. That is, unless it's prohibitively expensive to run under SANITIZE=leak. Thanks, -Stolee
