Re: [PATCH] test-lib: fix GIT_TEST_SANITIZE_LEAK_LOG

> On Mon, Jul 01, 2024 at 01:19:34PM -0700, Junio C Hamano wrote:
> 
> > Your patch from September 2023 [*] did mention it upfront:
> > 
> >     GIT_TEST_SANITIZE_LEAK_LOG=true with a test that leaks, will
> >     make the test return zero unintentionally.
> > 
> > With that inserted in front of the proposed log message, the
> > resulting explanation looks reasonable to me.
> 
> I see that you have already added this paragraph to what is already in
> "seen". OK.
> 
> > > diff --git a/t/test-lib.sh b/t/test-lib.sh
> > > index 79d3e0e7d9..7ed6d3fc47 100644
> > > --- a/t/test-lib.sh
> > > +++ b/t/test-lib.sh
> > > @@ -1269,9 +1269,12 @@ check_test_results_san_file_ () {
> > >  	then
> > >  		say "As TEST_PASSES_SANITIZE_LEAK=true isn't set the above leak is 'ok' with GIT_TEST_PASSING_SANITIZE_LEAK=check" &&
> > >  		invert_exit_code=t
> > > -	else
> > > +	elif test "$test_failure" = 0
> > > +	then
> > >  		say "With GIT_TEST_SANITIZE_LEAK_LOG=true our logs revealed a memory leak, exit non-zero!" &&
> > >  		invert_exit_code=t
> > > +	else
> > > +		say "With GIT_TEST_SANITIZE_LEAK_LOG=true our logs revealed a memory leak..."
> > >  	fi
> > >  }
> > 
> > This is outside the scope of this patch simply because it is
> > inherited from the original, but does ", exit non-zero!"  part of
> > the message really add any value?
> 
> 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.