Thread (1 message) 1 message, 1 author, 2023-09-15

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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help