diff mbox series

[2/3] test-lib: show leak-sanitizer logs on --immediate failure

Message ID 20240924213636.GB1142403@coredump.intra.peff.net (mailing list archive)
State Accepted
Commit 5fabf6e5ad28dab9c2c5944a9d4d4c5ab7720885
Headers show
Series LSan quality of life improvements | expand

Commit Message

Jeff King Sept. 24, 2024, 9:36 p.m. UTC
When we've compiled with SANITIZE=leak, at the end of the test script
we'll dump any collected logs to stdout. These logs have two uses:

  1. Leaks don't always cause a test snippet to fail (e.g., if they
     happen in a sub-process that we expect to return non-zero).
     Checking the logs catches these cases that we'd otherwise miss
     entirely.

  2. LSan will dump the leak info to stderr, but that is sometimes
     hidden (e.g., because it's redirected by the test, or because it's
     in a sub-process whose stderr goes elsewhere). Dumping the logs is
     the easiest way for the developer to see them.

One downside is that the set of logs for an entire script may be very
long, especially when you're trying to fix existing test scripts. You
can run with --immediate to stop at the first failing test, which means
we'll have accrued fewer logs. But we don't show the logs in that case!

Let's start doing so. This can only help case (2), of course (since it
depends on test failure). And it's somewhat weakened by the fact that
any cases of (1) will pollute the logs. But we can improve things
further in the next patch.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/test-lib.sh | 1 +
 1 file changed, 1 insertion(+)

Comments

Patrick Steinhardt Sept. 26, 2024, 2:19 p.m. UTC | #1
On Tue, Sep 24, 2024 at 05:36:36PM -0400, Jeff King wrote:
> When we've compiled with SANITIZE=leak, at the end of the test script
> we'll dump any collected logs to stdout. These logs have two uses:
> 
>   1. Leaks don't always cause a test snippet to fail (e.g., if they
>      happen in a sub-process that we expect to return non-zero).
>      Checking the logs catches these cases that we'd otherwise miss
>      entirely.
> 
>   2. LSan will dump the leak info to stderr, but that is sometimes
>      hidden (e.g., because it's redirected by the test, or because it's
>      in a sub-process whose stderr goes elsewhere). Dumping the logs is
>      the easiest way for the developer to see them.
> 
> One downside is that the set of logs for an entire script may be very
> long, especially when you're trying to fix existing test scripts. You
> can run with --immediate to stop at the first failing test, which means
> we'll have accrued fewer logs. But we don't show the logs in that case!
> 
> Let's start doing so. This can only help case (2), of course (since it
> depends on test failure). And it's somewhat weakened by the fact that
> any cases of (1) will pollute the logs. But we can improve things
> further in the next patch.

Yes, please!

Patrick
diff mbox series

Patch

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 7d4471fbc5..d624ee186c 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -847,6 +847,7 @@  test_failure_ () {
 			GIT_EXIT_OK=t
 			exit 0
 		fi
+		check_test_results_san_file_ "$test_failure"
 		_error_exit
 	fi
 	finalize_test_case_output failure "$failure_label" "$@"