Re: [PATCH 00/10] leak tests: mark more tests as passing

On Wed, Oct 06 2021, Elijah Newren wrote:

On Wed, Oct 6, 2021 at 2:50 AM Ævar Arnfjörð Bjarmason [off-list ref] wrote:

quoted

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^