| Message ID | cover-00.10-00000000000-20211006T094705Z-avarab@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | leak tests: mark more tests as passing | expand |
On Wed, Oct 6, 2021 at 2:50 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > > This goes on top of ab/sanitize-leak-ci, in that topic I introduced a > "linux-leaks" job that runs in CI and checks that we don't have > SANITIZE=leak regressions, but it just marked one test file as passing > under that mode. > > That was out of an abundance of caution, and to not conflate any > potential failures with the mode itself. > > This series marks up a lot of tests as passing, ensuring that they > won't regress when it comes to memory leaks. I like the series. It does have the potential to annoy folks who want to add additional tests which make use of git commands outside the area they are modifying and which happen to have pre-existing leaks. But, I think in such cases, they could just remove the TEST_PASSES_SANITIZE_LEAK=true and mention it in their commit message. And when the checks fail because of a git command someone is modifying, then we get useful early signal and the author can address the problem. > > I did a "git rebase -i --exec" of each of these commits where I merged > with origin/seen, ran all tests under > GIT_TEST_PASSING_SANITIZE_LEAK=true. They all pass, so hopefully these > won't cause any disruption. I was careful to leave out any areas that > are actively being worked on (e.g. I had to eject a test in the > "sparse" area, since it would pass on master+ab/sanitize-leak-ci, but > has a regression in "seen". I didn't look into why). > > Ævar Arnfjörð Bjarmason (10): > leak tests: run various built-in tests in t00*.sh SANITIZE=leak > leak tests: run various "test-tool" tests in t00*.sh SANITIZE=leak > leak tests: mark t0000-init.sh as passing with SANITIZE=leak > leak tests: mark all ls-tree tests as passing with SANITIZE=leak > leak tests: mark all trace2 tests as passing with SANITIZE=leak > leak tests: mark all checkout-index tests as passing with > SANITIZE=leak > leak tests: mark some ls-files tests as passing with SANITIZE=leak > leak tests: mark some read-tree tests as passing with SANITIZE=leak > leak tests: mark various "generic" tests as passing with SANITIZE=leak > leak tests: mark some misc tests as passing with SANITIZE=leak > > t/t0001-init.sh | 1 + > t/t0002-gitfile.sh | 1 + > t/t0003-attributes.sh | 1 + > t/t0005-signals.sh | 2 ++ > t/t0007-git-var.sh | 2 ++ > t/t0008-ignores.sh | 1 + > t/t0010-racy-git.sh | 1 + > t/t0013-sha1dc.sh | 2 ++ > t/t0022-crlf-rename.sh | 1 + > t/t0024-crlf-archive.sh | 1 + > t/t0025-crlf-renormalize.sh | 1 + > t/t0026-eol-config.sh | 1 + > t/t0029-core-unsetenvvars.sh | 1 + > t/t0052-simple-ipc.sh | 1 + > t/t0055-beyond-symlinks.sh | 1 + > t/t0061-run-command.sh | 1 + > t/t0065-strcmp-offset.sh | 1 + > t/t0066-dir-iterator.sh | 1 + > t/t0067-parse_pathspec_file.sh | 1 + > t/t0210-trace2-normal.sh | 2 ++ > t/t0211-trace2-perf.sh | 2 ++ > t/t0212-trace2-event.sh | 2 ++ > t/t1000-read-tree-m-3way.sh | 2 ++ > t/t1003-read-tree-prefix.sh | 1 + > t/t1009-read-tree-new-index.sh | 1 + > t/t1010-mktree.sh | 1 + > t/t1012-read-tree-df.sh | 1 + > t/t1014-read-tree-confusing.sh | 2 ++ > t/t1100-commit-tree-options.sh | 1 + > t/t1430-bad-ref-name.sh | 1 + > t/t1504-ceiling-dirs.sh | 2 ++ > t/t1510-repo-setup.sh | 1 + > t/t2002-checkout-cache-u.sh | 1 + > t/t2003-checkout-cache-mkdir.sh | 1 + > t/t2004-checkout-cache-temp.sh | 1 + > t/t2005-checkout-index-symlinks.sh | 1 + > t/t2050-git-dir-relative.sh | 1 + > t/t2081-parallel-checkout-collisions.sh | 1 + > t/t2300-cd-to-toplevel.sh | 1 + > t/t3000-ls-files-others.sh | 2 ++ > t/t3002-ls-files-dashpath.sh | 2 ++ > t/t3003-ls-files-exclude.sh | 2 ++ > t/t3004-ls-files-basic.sh | 1 + > t/t3006-ls-files-long.sh | 2 ++ > t/t3008-ls-files-lazy-init-name-hash.sh | 1 + > t/t3070-wildmatch.sh | 1 + > t/t3100-ls-tree-restrict.sh | 2 ++ > t/t3101-ls-tree-dirname.sh | 2 ++ > t/t3102-ls-tree-wildcards.sh | 1 + > t/t3103-ls-tree-misc.sh | 1 + > t/t3205-branch-color.sh | 1 + > t/t3211-peel-ref.sh | 1 + > t/t3300-funny-names.sh | 1 + > t/t3601-rm-pathspec-file.sh | 1 + > t/t3902-quoted.sh | 1 + > t/t4002-diff-basic.sh | 2 ++ > t/t4016-diff-quote.sh | 1 + > t/t4019-diff-wserror.sh | 1 + > t/t4025-hunk-header.sh | 1 + > t/t4026-color.sh | 2 ++ > t/t4300-merge-tree.sh | 2 ++ > t/t5580-unc-paths.sh | 1 + > t/t5615-alternate-env.sh | 2 ++ > t/t7518-ident-corner-cases.sh | 2 ++ > t/t7812-grep-icase-non-ascii.sh | 1 + > t/t7813-grep-icase-iso.sh | 1 + > t/t7816-grep-binary-pattern.sh | 1 + > 67 files changed, 87 insertions(+) > > -- > 2.33.0.1441.gbbcdb4c3c66
Elijah Newren <newren@gmail.com> writes: > I like the series. It does have the potential to annoy folks who want > to add additional tests which make use of git commands outside the > area they are modifying and which happen to have pre-existing leaks. True on both counts. Except for the t000-*.sh thing being a separate, and s/white// everywhere, I think all of them looked sensible and almost ready to be picked up. Thanks.
On Wed, Oct 06 2021, Elijah Newren wrote: > On Wed, Oct 6, 2021 at 2:50 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: >> >> This goes on top of ab/sanitize-leak-ci, in that topic I introduced a >> "linux-leaks" job that runs in CI and checks that we don't have >> SANITIZE=leak regressions, but it just marked one test file as passing >> under that mode. >> >> That was out of an abundance of caution, and to not conflate any >> potential failures with the mode itself. >> >> This series marks up a lot of tests as passing, ensuring that they >> won't regress when it comes to memory leaks. > > I like the series. It does have the potential to annoy folks who want > to add additional tests which make use of git commands outside the > area they are modifying and which happen to have pre-existing leaks. > But, I think in such cases, they could just remove the > TEST_PASSES_SANITIZE_LEAK=true and mention it in their commit message. > And when the checks fail because of a git command someone is > modifying, then we get useful early signal and the author can address > the problem. Yes there's a definite chance for annoyance while the common leak fixes I've got planned are cooking, i.e. as we've discussed elsewhere "git log" and "git checkout" among others leak in almost any invocation you cank think of, so if you add a new test that steps on those landmines (including via test_commit!) you'll run afoul of this. But I'm hoping to have those common cases sorted out SOON, so hopefully any problems with this are short lived. Just FWIW if this does happen you probably won't and shouldn't need to remove the TEST_PASSES_SANITIZE_LEAK=true entirely, it should be enough to make the specific test use "!SANITIZE_LEAK" as a prerequisite. See 956d2e4639b (tests: add a test mode for SANITIZE=leak, run it in CI, 2021-09-23). Well, that can also get annoying if e.g. you want to add a "test_commit" to a short test that has an existing setup (you'd need to split up the "setup" phase). Anyway, will re-roll, just FYI^
This goes on top of ab/sanitize-leak-ci, in that topic I introduced a "linux-leaks" job that runs in CI and checks that we don't have SANITIZE=leak regressions, but it just marked one test file as passing under that mode. That was out of an abundance of caution, and to not conflate any potential failures with the mode itself. This series marks up a lot of tests as passing, ensuring that they won't regress when it comes to memory leaks. I did a "git rebase -i --exec" of each of these commits where I merged with origin/seen, ran all tests under GIT_TEST_PASSING_SANITIZE_LEAK=true. They all pass, so hopefully these won't cause any disruption. I was careful to leave out any areas that are actively being worked on (e.g. I had to eject a test in the "sparse" area, since it would pass on master+ab/sanitize-leak-ci, but has a regression in "seen". I didn't look into why). Ævar Arnfjörð Bjarmason (10): leak tests: run various built-in tests in t00*.sh SANITIZE=leak leak tests: run various "test-tool" tests in t00*.sh SANITIZE=leak leak tests: mark t0000-init.sh as passing with SANITIZE=leak leak tests: mark all ls-tree tests as passing with SANITIZE=leak leak tests: mark all trace2 tests as passing with SANITIZE=leak leak tests: mark all checkout-index tests as passing with SANITIZE=leak leak tests: mark some ls-files tests as passing with SANITIZE=leak leak tests: mark some read-tree tests as passing with SANITIZE=leak leak tests: mark various "generic" tests as passing with SANITIZE=leak leak tests: mark some misc tests as passing with SANITIZE=leak t/t0001-init.sh | 1 + t/t0002-gitfile.sh | 1 + t/t0003-attributes.sh | 1 + t/t0005-signals.sh | 2 ++ t/t0007-git-var.sh | 2 ++ t/t0008-ignores.sh | 1 + t/t0010-racy-git.sh | 1 + t/t0013-sha1dc.sh | 2 ++ t/t0022-crlf-rename.sh | 1 + t/t0024-crlf-archive.sh | 1 + t/t0025-crlf-renormalize.sh | 1 + t/t0026-eol-config.sh | 1 + t/t0029-core-unsetenvvars.sh | 1 + t/t0052-simple-ipc.sh | 1 + t/t0055-beyond-symlinks.sh | 1 + t/t0061-run-command.sh | 1 + t/t0065-strcmp-offset.sh | 1 + t/t0066-dir-iterator.sh | 1 + t/t0067-parse_pathspec_file.sh | 1 + t/t0210-trace2-normal.sh | 2 ++ t/t0211-trace2-perf.sh | 2 ++ t/t0212-trace2-event.sh | 2 ++ t/t1000-read-tree-m-3way.sh | 2 ++ t/t1003-read-tree-prefix.sh | 1 + t/t1009-read-tree-new-index.sh | 1 + t/t1010-mktree.sh | 1 + t/t1012-read-tree-df.sh | 1 + t/t1014-read-tree-confusing.sh | 2 ++ t/t1100-commit-tree-options.sh | 1 + t/t1430-bad-ref-name.sh | 1 + t/t1504-ceiling-dirs.sh | 2 ++ t/t1510-repo-setup.sh | 1 + t/t2002-checkout-cache-u.sh | 1 + t/t2003-checkout-cache-mkdir.sh | 1 + t/t2004-checkout-cache-temp.sh | 1 + t/t2005-checkout-index-symlinks.sh | 1 + t/t2050-git-dir-relative.sh | 1 + t/t2081-parallel-checkout-collisions.sh | 1 + t/t2300-cd-to-toplevel.sh | 1 + t/t3000-ls-files-others.sh | 2 ++ t/t3002-ls-files-dashpath.sh | 2 ++ t/t3003-ls-files-exclude.sh | 2 ++ t/t3004-ls-files-basic.sh | 1 + t/t3006-ls-files-long.sh | 2 ++ t/t3008-ls-files-lazy-init-name-hash.sh | 1 + t/t3070-wildmatch.sh | 1 + t/t3100-ls-tree-restrict.sh | 2 ++ t/t3101-ls-tree-dirname.sh | 2 ++ t/t3102-ls-tree-wildcards.sh | 1 + t/t3103-ls-tree-misc.sh | 1 + t/t3205-branch-color.sh | 1 + t/t3211-peel-ref.sh | 1 + t/t3300-funny-names.sh | 1 + t/t3601-rm-pathspec-file.sh | 1 + t/t3902-quoted.sh | 1 + t/t4002-diff-basic.sh | 2 ++ t/t4016-diff-quote.sh | 1 + t/t4019-diff-wserror.sh | 1 + t/t4025-hunk-header.sh | 1 + t/t4026-color.sh | 2 ++ t/t4300-merge-tree.sh | 2 ++ t/t5580-unc-paths.sh | 1 + t/t5615-alternate-env.sh | 2 ++ t/t7518-ident-corner-cases.sh | 2 ++ t/t7812-grep-icase-non-ascii.sh | 1 + t/t7813-grep-icase-iso.sh | 1 + t/t7816-grep-binary-pattern.sh | 1 + 67 files changed, 87 insertions(+)