Re: [PATCH 2/2] test-lib: fix GIT_TEST_SANITIZE_LEAK_LOG
From: Junio C Hamano <hidden>
Date: 2023-09-15 17:50:00
Rubén Justo [off-list ref] writes:
quoted
And the problem is in (3). You switch it to trigger only if we have no failures (fixing the inversion). But should we have the same a/b split for this case? I.e.: 3a. if we saw no test failures, invert to cause a failure 3b. we saw other failures; do not invert, but _do_ mention that the log found extra leaks In 3b we are explaining to the user what happened. Though maybe it is not super important, because I think we'd have dumped the log contents anyway?I think so too. At that point we've already dumped the contents of the $TEST_RESULTS_SAN_FILE file. ... However, if you or anyone else thinks it adds value, I have no objection to re-roll with it.
I do not know offhand if we need the code update to implement what Peff called "maybe it is not super important", but if we decide not to, at least it would help future developers to document the fact that we were aware of the issue when the code was developed, and why we decided not to address it (in other words, describe why we decided it is not super important). Thanks both for polishing the series and making it better.