diff mbox series

[1/5] test-lib: use individual lsan dir for --stress runs

Message ID 20241230042401.GA113400@coredump.intra.peff.net (mailing list archive)
State Accepted
Commit d601aee6056a0afc6df7f77e15cdc155ff402dee
Headers show
Series fixing thread races in linux-leaks CI | expand

Commit Message

Jeff King Dec. 30, 2024, 4:24 a.m. UTC
When storing output in test-results/, we usually give each numbered run
in a --stress set its own output file. But we don't do that for storing
LSan logs, so something like:

  ./t0003-attributes.sh --stress

will have many scripts simultaneously creating, writing to, and deleting
the test-results/t0003-attributes.leak directory. This can cause logs
from one run to be attributed to another, spurious failures when
creation and deletion race, and so on.

This has always been broken, but nobody noticed because it's rare to do
a --stress run with LSan (since the point is for the code to run quickly
many times in order to hit races). But if you're trying to find a race
in the leak sanitizing code, it makes sense to use these together.

We can fix it by using $TEST_RESULTS_BASE, which already incorporates
the stress job suffix.

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

Comments

Jeff King Dec. 30, 2024, 4:34 a.m. UTC | #1
On Sun, Dec 29, 2024 at 11:24:01PM -0500, Jeff King wrote:

> -- 
> bar

BTW, in case anybody is curious about this, it's because I still had:

  GIT_VERSION = bar

in my config.mak from testing Patrick's GIT-VERSION-GEN series. :)

Naturally "foo" was taken from testing:

  make GIT_VERSION=foo

-Peff
Junio C Hamano Dec. 30, 2024, 5:08 p.m. UTC | #2
Jeff King <peff@peff.net> writes:

> When storing output in test-results/, we usually give each numbered run
> in a --stress set its own output file. But we don't do that for storing
> LSan logs, so something like:
>
>   ./t0003-attributes.sh --stress
>
> will have many scripts simultaneously creating, writing to, and deleting
> the test-results/t0003-attributes.leak directory. This can cause logs
> from one run to be attributed to another, spurious failures when
> creation and deletion race, and so on.
>
> This has always been broken, but nobody noticed because it's rare to do
> a --stress run with LSan (since the point is for the code to run quickly
> many times in order to hit races). But if you're trying to find a race
> in the leak sanitizing code, it makes sense to use these together.

Makes sense.

> We can fix it by using $TEST_RESULTS_BASE, which already incorporates
> the stress job suffix.

Thanks.  Queued.
diff mbox series

Patch

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1a67adb207..96f2dfb69d 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -331,7 +331,7 @@  TEST_RESULTS_BASE="$TEST_RESULTS_DIR/$TEST_NAME$TEST_STRESS_JOB_SFX"
 TEST_RESULTS_SAN_FILE_PFX=trace
 TEST_RESULTS_SAN_DIR_SFX=leak
 TEST_RESULTS_SAN_FILE=
-TEST_RESULTS_SAN_DIR="$TEST_RESULTS_DIR/$TEST_NAME.$TEST_RESULTS_SAN_DIR_SFX"
+TEST_RESULTS_SAN_DIR="$TEST_RESULTS_BASE.$TEST_RESULTS_SAN_DIR_SFX"
 TRASH_DIRECTORY="trash directory.$TEST_NAME$TEST_STRESS_JOB_SFX"
 test -n "$root" && TRASH_DIRECTORY="$root/$TRASH_DIRECTORY"
 case "$TRASH_DIRECTORY" in