Re: [PATCH] test-lib: fix GIT_TEST_SANITIZE_LEAK_LOG

["Rubén Justo" <rjusto@gmail.com>]

On Sat, Jul 06, 2024 at 02:18:50AM -0400, Jeff King wrote:
> On Wed, Jul 03, 2024 at 11:44:33PM +0200, Rubén Justo wrote:
> 
> > > Explicitly indicating that the error is being forced due to
> > > "GIT_TEST_SANITIZE_LEAK_LOG=true", for a test that doesn't fail when run
> > > normally or even when run with just
> > > "GIT_TEST_PASSING_SANITIZE_LEAK=yes", could save us some confusion.
> > > 
> > > So, I dunno.
> > > 
> > > Anyway, I agree that this can be addressed later.
> > > 
> > > Thanks.
> > 
> > Maybe what we should do is integrate "GIT_TEST_SANITIZE_LEAK_LOG" into
> > "GIT_TEST_PASSING_SANITIZE_LEAK" because I'm not sure what value we get
> > by keeping them separate (test performance?).  But that's another topic,
> > even further out of scope of this patch :-)
> 
> I don't think we want to integrate them, but I'd suggest that
> SANITIZE_LEAK_LOG should be the default/only option.
> 
> Without it, you are potentially missing leaks in programs whose failing
> exit codes do not trigger a test failure. So there is no point in
> running PASSING_SANITIZE_LEAK=check without also checking the logs. But
> it is still useful to set SANITIZE_LEAK_LOG just for normal runs to look
> for leaks.
> 
> I don't know of any reason we couldn't always check the logs (for a
> leak-checking build), and I didn't see anything in the history. I think
> it was written that way only because there is otherwise no affirmative
> action by the user to say "and btw, look for leaks" (and if we are not
> looking for leaks, there might not be any logs!).
> 
> But really, if you have done a leak-checking build, then every run of
> the tests is looking for leaks, whether you check the logs or not. So we
> should able to just check that $SANITIZE_LEAK is set.

> And then there would be one less thing for people checking for leaks
> to remember to set.

I completely agree.

Let's wait for the dust to settle after the fix in this series, and then
I'll address the change as you described.

Thanks.