From: Jeff King <peff@peff•net>
To: Junio C Hamano <gitster@pobox•com>
Cc: Patrick Steinhardt <ps@pks•im>, git@vger•kernel.org
Subject: [PATCH 4/6] test-lib: simplify leak-log checking
Date: Wed, 1 Jan 2025 15:17:21 -0500 [thread overview]
Message-ID: <20250101201721.GD3305462@coredump.intra.peff.net> (raw)
In-Reply-To: <20250101201226.GA3304465@coredump.intra.peff.net>
We have a function to count the number of leaks found (actually, it is
the number of processes which produced a log file). Once upon a time we
cared about seeing if this number increased between runs. But we
simplified that away in 95c679ad86 (test-lib: stop showing old leak
logs, 2024-09-24), and now we only care if it returns any results or
not.
In preparation for refactoring it further, let's drop the counting
function entirely, and roll it into the "is it empty" check. The outcome
should be the same, but we'll be free to return a boolean "did we find
anything" without worrying about somebody adding a new call to the
counting function.
Signed-off-by: Jeff King <peff@peff•net>
---
We need the extra "!" to invert the exit code of the final grep, which
made my head spin a little at first. Maybe it would be less confusing if
this was reporting non-empty results, as "check_test_results_has_leaks"
or something? We'd have to update the callers, but there are only 2 of
them. I dunno.
t/test-lib.sh | 21 ++++++++-------------
1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index dd2ba6e6cc..23eb26bfbe 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -340,17 +340,6 @@ case "$TRASH_DIRECTORY" in
*) TRASH_DIRECTORY="$TEST_OUTPUT_DIRECTORY/$TRASH_DIRECTORY" ;;
esac
-# Utility functions using $TEST_RESULTS_* variables
-nr_san_dir_leaks_ () {
- # stderr piped to /dev/null because the directory may have
- # been "rmdir"'d already.
- find "$TEST_RESULTS_SAN_DIR" \
- -type f \
- -name "$TEST_RESULTS_SAN_FILE_PFX.*" 2>/dev/null |
- xargs grep -lv "Unable to get registers from thread" |
- wc -l
-}
-
# If --stress was passed, run this test repeatedly in several parallel loops.
if test "$GIT_TEST_STRESS_STARTED" = "done"
then
@@ -1181,8 +1170,14 @@ test_atexit_handler () {
}
check_test_results_san_file_empty_ () {
- test -z "$TEST_RESULTS_SAN_FILE" ||
- test "$(nr_san_dir_leaks_)" = 0
+ test -z "$TEST_RESULTS_SAN_FILE" && return 0
+
+ # stderr piped to /dev/null because the directory may have
+ # been "rmdir"'d already.
+ ! find "$TEST_RESULTS_SAN_DIR" \
+ -type f \
+ -name "$TEST_RESULTS_SAN_FILE_PFX.*" 2>/dev/null |
+ xargs grep -qv "Unable to get registers from thread"
}
check_test_results_san_file_ () {
--
2.48.0.rc1.363.g2bf91ec010
next prev parent reply other threads:[~2025-01-01 20:17 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-30 17:33 What's cooking in git.git (Dec 2024, #11; Mon, 30) Junio C Hamano
2024-12-31 17:27 ` René Scharfe
2025-01-03 7:39 ` Patrick Steinhardt
2025-01-01 19:14 ` a less-invasive racy-leak fix, was " Jeff King
2025-01-01 20:12 ` [PATCH 0/6] a less-invasive racy-leak fix Jeff King
2025-01-01 20:12 ` [PATCH 1/6] test-lib: use individual lsan dir for --stress runs Jeff King
2025-01-01 20:12 ` [PATCH 2/6] Revert "index-pack: spawn threads atomically" Jeff King
2025-01-01 20:14 ` [PATCH 3/6] test-lib: rely on logs to detect leaks Jeff King
2025-01-03 12:05 ` Patrick Steinhardt
2025-01-03 20:10 ` Jeff King
2025-01-01 20:17 ` Jeff King [this message]
2025-01-03 12:05 ` [PATCH 4/6] test-lib: simplify leak-log checking Patrick Steinhardt
2025-01-03 20:24 ` Jeff King
2025-01-06 7:56 ` Patrick Steinhardt
2025-01-07 7:01 ` Jeff King
2025-01-01 20:18 ` [PATCH 5/6] test-lib: check leak logs for presence of DEDUP_TOKEN Jeff King
2025-01-01 20:21 ` [PATCH 6/6] test-lib: ignore leaks in the sanitizer's thread code Jeff King
2025-01-03 12:05 ` Patrick Steinhardt
2025-01-03 20:26 ` Jeff King
2025-01-06 7:56 ` Patrick Steinhardt
2025-01-07 7:04 ` [PATCH 0/3] lsan test-lib readability Jeff King
2025-01-07 7:05 ` [PATCH 1/3] test-lib: invert return value of check_test_results_san_file_empty Jeff King
2025-01-07 7:07 ` [PATCH 2/3] test-lib: simplify lsan results check Jeff King
2025-01-07 7:37 ` Patrick Steinhardt
2025-01-09 7:57 ` Jeff King
2025-01-09 10:00 ` Patrick Steinhardt
2025-01-07 16:23 ` Junio C Hamano
2025-01-09 7:59 ` Jeff King
2025-01-07 7:08 ` [PATCH 3/3] test-lib: add a few comments to LSan log checking Jeff King
2025-01-07 7:37 ` Patrick Steinhardt
2025-01-02 0:25 ` a less-invasive racy-leak fix, was Re: What's cooking in git.git (Dec 2024, #11; Mon, 30) Junio C Hamano
2025-01-02 2:32 ` Jeff King
2025-01-02 2:41 ` Chris Torek
2025-01-02 14:42 ` Junio C Hamano
2025-01-02 19:06 ` Jeff King
2025-01-02 19:33 ` Junio C Hamano
2025-01-02 3:24 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250101201721.GD3305462@coredump.intra.peff.net \
--to=peff@peff$(echo .)net \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=ps@pks$(echo .)im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox