git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Subject: [PATCH 25/26] test-lib: unconditionally enable leak checking
Date: Wed, 6 Nov 2024 16:11:34 +0100	[thread overview]
Message-ID: <ad7529ff2a2d5dd952b153ff9c9ee93d3eb4aa3c.1730901926.git.ps@pks.im> (raw)
In-Reply-To: <cover.1730901926.git.ps@pks.im>

Over the last two releases we have plugged a couple hundred of memory
leaks exposed by the Git test suite. With the preceding commits we have
finally fixed the last leak exposed by our test suite, which means that
we are now basically leak free wherever we have branch coverage.

From hereon, the Git test suite should ideally stay free of memory
leaks. Most importantly, any test suite that is being added should
automatically be subject to the leak checker, and if that test does not
pass it is a strong signal that the added code introduced new memory
leaks and should not be accepted without further changes.

Drop the infrastructure around TEST_PASSES_SANITIZE_LEAK to reflect this
new requirement. Like this, all test suites will be subject to the leak
checker by default.

This is being intentionally strict, but we still have an escape hatch:
the SANITIZE_LEAK prerequisite. There is one known case in t5601 where
the leak sanitizer itself is buggy, so adding this prereq in such cases
is acceptable. Another acceptable situation is when a newly added test
uncovers preexisting memory leaks: when fixing that memory leak would be
sufficiently complicated it is fine to annotate and document the leak
accordingly. But in any case, the burden is now on the patch author to
explain why exactly they have to add the SANITIZE_LEAK prerequisite.

The TEST_PASSES_SANITIZE_LEAK annotations will be dropped in the next
patch.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 ci/lib.sh        |  1 -
 t/README         | 21 --------------
 t/lib-git-svn.sh |  4 ---
 t/test-lib.sh    | 72 +-----------------------------------------------
 4 files changed, 1 insertion(+), 97 deletions(-)

diff --git a/ci/lib.sh b/ci/lib.sh
index 246072a0932..930f98d7228 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -384,7 +384,6 @@ linux-musl)
 	;;
 linux-leaks|linux-reftable-leaks)
 	export SANITIZE=leak
-	export GIT_TEST_PASSING_SANITIZE_LEAK=true
 	;;
 linux-asan-ubsan)
 	export SANITIZE=address,undefined
diff --git a/t/README b/t/README
index 8c0319b58e5..e84824dc002 100644
--- a/t/README
+++ b/t/README
@@ -368,27 +368,6 @@ excluded as so much relies on it, but this might change in the future.
 GIT_TEST_SPLIT_INDEX=<boolean> forces split-index mode on the whole
 test suite. Accept any boolean values that are accepted by git-config.
 
-GIT_TEST_PASSING_SANITIZE_LEAK=true skips those tests that haven't
-declared themselves as leak-free by setting
-"TEST_PASSES_SANITIZE_LEAK=true" before sourcing "test-lib.sh". This
-test mode is used by the "linux-leaks" CI target.
-
-GIT_TEST_PASSING_SANITIZE_LEAK=check checks that our
-"TEST_PASSES_SANITIZE_LEAK=true" markings are current. Rather than
-skipping those tests that haven't set "TEST_PASSES_SANITIZE_LEAK=true"
-before sourcing "test-lib.sh" this mode runs them with
-"--invert-exit-code". This is used to check that there's a one-to-one
-mapping between "TEST_PASSES_SANITIZE_LEAK=true" and those tests that
-pass under "SANITIZE=leak". This is especially useful when testing a
-series that fixes various memory leaks with "git rebase -x".
-
-GIT_TEST_PASSING_SANITIZE_LEAK=check when combined with "--immediate"
-will run to completion faster, and result in the same failing
-tests.
-
-GIT_TEST_PASSING_SANITIZE_LEAK=check-failing behaves the same as "check",
-but skips all tests which are already marked as leak-free.
-
 GIT_TEST_PROTOCOL_VERSION=<n>, when set, makes 'protocol.version'
 default to n.
 
diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
index ea28971e8ee..2fde2353fd3 100644
--- a/t/lib-git-svn.sh
+++ b/t/lib-git-svn.sh
@@ -1,7 +1,3 @@
-if test -z "$TEST_FAILS_SANITIZE_LEAK"
-then
-	TEST_PASSES_SANITIZE_LEAK=true
-fi
 . ./test-lib.sh
 
 if test -n "$NO_SVN_TESTS"
diff --git a/t/test-lib.sh b/t/test-lib.sh
index a278181a056..508b5fe1f55 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1227,23 +1227,7 @@ check_test_results_san_file_ () {
 	fi &&
 	say_color error "$(cat "$TEST_RESULTS_SAN_FILE".*)" &&
 
-	if test -n "$passes_sanitize_leak" && test "$test_failure" = 0
-	then
-		say "As TEST_PASSES_SANITIZE_LEAK=true and our logs show we're leaking, exit non-zero!" &&
-		invert_exit_code=t
-	elif test -n "$passes_sanitize_leak"
-	then
-		say "As TEST_PASSES_SANITIZE_LEAK=true and our logs show we're leaking, and we're failing for other reasons too..." &&
-		invert_exit_code=
-	elif test -n "$sanitize_leak_check" && test "$test_failure" = 0
-	then
-		say "As TEST_PASSES_SANITIZE_LEAK=true isn't set the above leak is 'ok' with GIT_TEST_PASSING_SANITIZE_LEAK=check" &&
-		invert_exit_code=
-	elif test -n "$sanitize_leak_check"
-	then
-		say "As TEST_PASSES_SANITIZE_LEAK=true isn't set the above leak is 'ok' with GIT_TEST_PASSING_SANITIZE_LEAK=check" &&
-		invert_exit_code=t
-	elif test "$test_failure" = 0
+	if test "$test_failure" = 0
 	then
 		say "Our logs revealed a memory leak, exit non-zero!" &&
 		invert_exit_code=t
@@ -1274,11 +1258,6 @@ test_done () {
 		EOF
 	fi
 
-	if test -z "$passes_sanitize_leak" && test_bool_env TEST_PASSES_SANITIZE_LEAK false
-	then
-		BAIL_OUT "Please, set TEST_PASSES_SANITIZE_LEAK before sourcing test-lib.sh"
-	fi
-
 	if test "$test_fixed" != 0
 	then
 		say_color error "# $test_fixed known breakage(s) vanished; please update test(s)"
@@ -1515,51 +1494,8 @@ then
 	test_done
 fi
 
-BAIL_OUT_ENV_NEEDS_SANITIZE_LEAK () {
-	BAIL_OUT "$1 has no effect except when compiled with SANITIZE=leak"
-}
-
 if test -n "$SANITIZE_LEAK"
 then
-	# Normalize with test_bool_env
-	passes_sanitize_leak=
-
-	# We need to see TEST_PASSES_SANITIZE_LEAK in "test-tool
-	# env-helper" (via test_bool_env)
-	export TEST_PASSES_SANITIZE_LEAK
-	if test_bool_env TEST_PASSES_SANITIZE_LEAK false
-	then
-		passes_sanitize_leak=t
-	fi
-
-	if test "$GIT_TEST_PASSING_SANITIZE_LEAK" = "check" ||
-	   test "$GIT_TEST_PASSING_SANITIZE_LEAK" = "check-failing"
-	then
-		if test "$GIT_TEST_PASSING_SANITIZE_LEAK" = "check-failing" &&
-		   test -n "$passes_sanitize_leak"
-		then
-			skip_all="skipping leak-free $this_test under GIT_TEST_PASSING_SANITIZE_LEAK=check-failing"
-			test_done
-		fi
-
-		sanitize_leak_check=t
-		if test -n "$invert_exit_code"
-		then
-			BAIL_OUT "cannot use --invert-exit-code under GIT_TEST_PASSING_SANITIZE_LEAK=check"
-		fi
-
-		if test -z "$passes_sanitize_leak"
-		then
-			say "in GIT_TEST_PASSING_SANITIZE_LEAK=check mode, setting --invert-exit-code for TEST_PASSES_SANITIZE_LEAK != true"
-			invert_exit_code=t
-		fi
-	elif test -z "$passes_sanitize_leak" &&
-	     test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false
-	then
-		skip_all="skipping $this_test under GIT_TEST_PASSING_SANITIZE_LEAK=true"
-		test_done
-	fi
-
 	rm -rf "$TEST_RESULTS_SAN_DIR"
 	if ! mkdir -p "$TEST_RESULTS_SAN_DIR"
 	then
@@ -1574,12 +1510,6 @@ then
 	prepend_var LSAN_OPTIONS : log_exe_name=1
 	prepend_var LSAN_OPTIONS : log_path="'$TEST_RESULTS_SAN_FILE'"
 	export LSAN_OPTIONS
-
-elif test "$GIT_TEST_PASSING_SANITIZE_LEAK" = "check" ||
-     test "$GIT_TEST_PASSING_SANITIZE_LEAK" = "check-failing" ||
-     test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false
-then
-	BAIL_OUT_ENV_NEEDS_SANITIZE_LEAK "GIT_TEST_PASSING_SANITIZE_LEAK=true"
 fi
 
 if test "${GIT_TEST_CHAIN_LINT:-1}" != 0 &&
-- 
2.47.0.229.g8f8d6eee53.dirty


  parent reply	other threads:[~2024-11-06 15:11 UTC|newest]

Thread overview: 117+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-06 15:10 [PATCH 00/26] Memory leak fixes (pt.10, final) Patrick Steinhardt
2024-11-06 15:10 ` [PATCH 01/26] builtin/blame: fix leaking blame entries with `--incremental` Patrick Steinhardt
2024-11-06 15:10 ` [PATCH 02/26] bisect: fix leaking good/bad terms when reading multipe times Patrick Steinhardt
2024-11-06 15:10 ` [PATCH 03/26] bisect: fix leaking string in `handle_bad_merge_base()` Patrick Steinhardt
2024-11-06 15:10 ` [PATCH 04/26] bisect: fix leaking `current_bad_oid` Patrick Steinhardt
2024-11-06 15:10 ` [PATCH 05/26] bisect: fix multiple leaks in `bisect_next_all()` Patrick Steinhardt
2024-11-06 15:10 ` [PATCH 06/26] bisect: fix leaking commit list items in `check_merge_base()` Patrick Steinhardt
2024-11-06 15:10 ` [PATCH 07/26] bisect: fix various cases where we leak commit list items Patrick Steinhardt
2024-11-06 15:10 ` [PATCH 08/26] line-log: fix leak when rewriting commit parents Patrick Steinhardt
2024-11-06 15:10 ` [PATCH 09/26] strvec: introduce new `strvec_splice()` function Patrick Steinhardt
2024-11-10 21:39   ` Rubén Justo
2024-11-11  9:09     ` Patrick Steinhardt
2024-11-06 15:10 ` [PATCH 10/26] git: refactor alias handling to use a `struct strvec` Patrick Steinhardt
2024-11-10 21:41   ` Rubén Justo
2024-11-06 15:10 ` [PATCH 11/26] git: refactor builtin " Patrick Steinhardt
2024-11-06 15:10 ` [PATCH 12/26] split-index: fix memory leak in `move_cache_to_base_index()` Patrick Steinhardt
2024-11-10 21:45   ` Rubén Justo
2024-11-06 15:10 ` [PATCH 13/26] builtin/sparse-checkout: fix leaking sanitized patterns Patrick Steinhardt
2024-11-06 15:11 ` [PATCH 14/26] help: refactor to not use globals for reading config Patrick Steinhardt
2024-11-06 15:11 ` [PATCH 15/26] help: fix leaking `struct cmdnames` Patrick Steinhardt
2024-11-10 21:46   ` Rubén Justo
2024-11-11  9:09     ` Patrick Steinhardt
2024-11-06 15:11 ` [PATCH 16/26] help: fix leaking return value from `help_unknown_cmd()` Patrick Steinhardt
2024-11-06 15:11 ` [PATCH 17/26] builtin/help: fix leaks in `check_git_cmd()` Patrick Steinhardt
2024-11-06 15:11 ` [PATCH 18/26] builtin/init-db: fix leaking directory paths Patrick Steinhardt
2024-11-10 21:47   ` Rubén Justo
2024-11-06 15:11 ` [PATCH 19/26] builtin/branch: fix leaking sorting options Patrick Steinhardt
2024-11-10 21:47   ` Rubén Justo
2024-11-06 15:11 ` [PATCH 20/26] t/helper: fix leaking commit graph in "read-graph" subcommand Patrick Steinhardt
2024-11-06 15:11 ` [PATCH 21/26] git-compat-util: drop `UNLEAK()` annotation Patrick Steinhardt
2024-11-10 21:47   ` Rubén Justo
2024-11-11  9:09     ` Patrick Steinhardt
2024-11-06 15:11 ` [PATCH 22/26] t5601: work around leak sanitizer issue Patrick Steinhardt
2024-11-06 15:11 ` [PATCH 23/26] t: mark some tests as leak free Patrick Steinhardt
2024-11-06 15:11 ` [PATCH 24/26] t: remove unneeded !SANITIZE_LEAK prerequisites Patrick Steinhardt
2024-11-06 15:11 ` Patrick Steinhardt [this message]
2024-11-06 15:11 ` [PATCH 26/26] t: remove TEST_PASSES_SANITIZE_LEAK annotations Patrick Steinhardt
2024-11-10 21:48 ` [PATCH 00/26] Memory leak fixes (pt.10, final) Rubén Justo
2024-11-11  9:09   ` Patrick Steinhardt
2024-11-11 10:38 ` [PATCH v2 00/27] " Patrick Steinhardt
2024-11-11 10:38   ` [PATCH v2 01/27] builtin/blame: fix leaking blame entries with `--incremental` Patrick Steinhardt
2024-11-11 10:38   ` [PATCH v2 02/27] bisect: fix leaking good/bad terms when reading multipe times Patrick Steinhardt
2024-11-11 10:38   ` [PATCH v2 03/27] bisect: fix leaking string in `handle_bad_merge_base()` Patrick Steinhardt
2024-11-11 10:38   ` [PATCH v2 04/27] bisect: fix leaking `current_bad_oid` Patrick Steinhardt
2024-11-11 10:38   ` [PATCH v2 05/27] bisect: fix multiple leaks in `bisect_next_all()` Patrick Steinhardt
2024-11-11 10:38   ` [PATCH v2 06/27] bisect: fix leaking commit list items in `check_merge_base()` Patrick Steinhardt
2024-11-11 10:38   ` [PATCH v2 07/27] bisect: fix various cases where we leak commit list items Patrick Steinhardt
2024-11-20 10:32     ` Toon Claes
2024-11-20 12:41       ` Patrick Steinhardt
2024-11-11 10:38   ` [PATCH v2 08/27] line-log: fix leak when rewriting commit parents Patrick Steinhardt
2024-11-11 10:38   ` [PATCH v2 09/27] strvec: introduce new `strvec_splice()` function Patrick Steinhardt
2024-11-20  8:37     ` Toon Claes
2024-11-20 12:41       ` Patrick Steinhardt
2024-11-20 23:13         ` Junio C Hamano
2024-11-21  8:11           ` Jeff King
2024-11-21  8:22             ` Jeff King
2024-11-21 10:23             ` Doxygen-styled comments [was: Re: [PATCH v2 09/27] strvec: introduce new `strvec_splice()` function] Toon Claes
2024-11-21 10:32               ` Jeff King
2024-11-11 10:38   ` [PATCH v2 10/27] git: refactor alias handling to use a `struct strvec` Patrick Steinhardt
2024-11-11 10:38   ` [PATCH v2 11/27] git: refactor builtin " Patrick Steinhardt
2024-11-20 10:38     ` Toon Claes
2024-11-11 10:38   ` [PATCH v2 12/27] split-index: fix memory leak in `move_cache_to_base_index()` Patrick Steinhardt
2024-11-11 10:38   ` [PATCH v2 13/27] builtin/sparse-checkout: fix leaking sanitized patterns Patrick Steinhardt
2024-11-11 10:38   ` [PATCH v2 14/27] help: refactor to not use globals for reading config Patrick Steinhardt
2024-11-11 10:38   ` [PATCH v2 15/27] help: fix leaking `struct cmdnames` Patrick Steinhardt
2024-11-11 10:38   ` [PATCH v2 16/27] help: fix leaking return value from `help_unknown_cmd()` Patrick Steinhardt
2024-11-11 10:38   ` [PATCH v2 17/27] builtin/help: fix leaks in `check_git_cmd()` Patrick Steinhardt
2024-11-11 10:38   ` [PATCH v2 18/27] builtin/init-db: fix leaking directory paths Patrick Steinhardt
2024-11-11 10:38   ` [PATCH v2 19/27] builtin/branch: fix leaking sorting options Patrick Steinhardt
2024-11-11 10:38   ` [PATCH v2 20/27] t/helper: fix leaking commit graph in "read-graph" subcommand Patrick Steinhardt
2024-11-11 10:38   ` [PATCH v2 21/27] global: drop `UNLEAK()` annotation Patrick Steinhardt
2024-11-12  8:26     ` Jeff King
2024-11-12  8:53       ` Patrick Steinhardt
2024-11-12  9:03         ` Jeff King
2024-11-11 10:38   ` [PATCH v2 22/27] git-compat-util: drop now-unused `UNLEAK()` macro Patrick Steinhardt
2024-11-11 10:38   ` [PATCH v2 23/27] t5601: work around leak sanitizer issue Patrick Steinhardt
2024-11-11 10:38   ` [PATCH v2 24/27] t: mark some tests as leak free Patrick Steinhardt
2024-11-11 10:38   ` [PATCH v2 25/27] t: remove unneeded !SANITIZE_LEAK prerequisites Patrick Steinhardt
2024-11-11 10:38   ` [PATCH v2 26/27] test-lib: unconditionally enable leak checking Patrick Steinhardt
2024-11-11 10:38   ` [PATCH v2 27/27] t: remove TEST_PASSES_SANITIZE_LEAK annotations Patrick Steinhardt
2024-11-20 10:40     ` Toon Claes
2024-11-20 12:41       ` Patrick Steinhardt
2024-11-11 23:33   ` [PATCH v2 00/27] Memory leak fixes (pt.10, final) Rubén Justo
2024-11-12  8:06     ` Rubén Justo
2024-11-20 13:39 ` [PATCH v3 " Patrick Steinhardt
2024-11-20 13:39   ` [PATCH v3 01/27] builtin/blame: fix leaking blame entries with `--incremental` Patrick Steinhardt
2024-11-20 13:39   ` [PATCH v3 02/27] bisect: fix leaking good/bad terms when reading multipe times Patrick Steinhardt
2024-11-20 13:39   ` [PATCH v3 03/27] bisect: fix leaking string in `handle_bad_merge_base()` Patrick Steinhardt
2024-11-20 13:39   ` [PATCH v3 04/27] bisect: fix leaking `current_bad_oid` Patrick Steinhardt
2024-11-20 13:39   ` [PATCH v3 05/27] bisect: fix multiple leaks in `bisect_next_all()` Patrick Steinhardt
2024-11-20 13:39   ` [PATCH v3 06/27] bisect: fix leaking commit list items in `check_merge_base()` Patrick Steinhardt
2024-11-20 13:39   ` [PATCH v3 07/27] bisect: fix various cases where we leak commit list items Patrick Steinhardt
2024-11-25 11:27     ` Jeff King
2024-11-25 12:38       ` Patrick Steinhardt
2024-11-25 13:17         ` Jeff King
2024-11-25 14:08           ` Patrick Steinhardt
2024-11-20 13:39   ` [PATCH v3 08/27] line-log: fix leak when rewriting commit parents Patrick Steinhardt
2024-11-20 13:39   ` [PATCH v3 09/27] strvec: introduce new `strvec_splice()` function Patrick Steinhardt
2024-11-20 13:39   ` [PATCH v3 10/27] git: refactor alias handling to use a `struct strvec` Patrick Steinhardt
2024-11-20 13:39   ` [PATCH v3 11/27] git: refactor builtin " Patrick Steinhardt
2024-11-20 13:39   ` [PATCH v3 12/27] split-index: fix memory leak in `move_cache_to_base_index()` Patrick Steinhardt
2024-11-20 13:39   ` [PATCH v3 13/27] builtin/sparse-checkout: fix leaking sanitized patterns Patrick Steinhardt
2024-11-20 13:39   ` [PATCH v3 14/27] help: refactor to not use globals for reading config Patrick Steinhardt
2024-11-20 13:39   ` [PATCH v3 15/27] help: fix leaking `struct cmdnames` Patrick Steinhardt
2024-11-20 13:39   ` [PATCH v3 16/27] help: fix leaking return value from `help_unknown_cmd()` Patrick Steinhardt
2024-11-20 13:39   ` [PATCH v3 17/27] builtin/help: fix leaks in `check_git_cmd()` Patrick Steinhardt
2024-11-20 13:39   ` [PATCH v3 18/27] builtin/init-db: fix leaking directory paths Patrick Steinhardt
2024-11-20 13:39   ` [PATCH v3 19/27] builtin/branch: fix leaking sorting options Patrick Steinhardt
2024-11-20 13:39   ` [PATCH v3 20/27] t/helper: fix leaking commit graph in "read-graph" subcommand Patrick Steinhardt
2024-11-20 13:39   ` [PATCH v3 21/27] global: drop `UNLEAK()` annotation Patrick Steinhardt
2024-11-20 13:39   ` [PATCH v3 22/27] git-compat-util: drop now-unused `UNLEAK()` macro Patrick Steinhardt
2024-11-20 13:39   ` [PATCH v3 23/27] t5601: work around leak sanitizer issue Patrick Steinhardt
2024-11-20 13:39   ` [PATCH v3 24/27] t: mark some tests as leak free Patrick Steinhardt
2024-11-20 13:39   ` [PATCH v3 25/27] t: remove unneeded !SANITIZE_LEAK prerequisites Patrick Steinhardt
2024-11-20 13:39   ` [PATCH v3 26/27] test-lib: unconditionally enable leak checking Patrick Steinhardt
2024-11-20 13:39   ` [PATCH v3 27/27] t: remove TEST_PASSES_SANITIZE_LEAK annotations Patrick Steinhardt
2024-11-21 10:32   ` [PATCH v3 00/27] Memory leak fixes (pt.10, final) Toon Claes

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=ad7529ff2a2d5dd952b153ff9c9ee93d3eb4aa3c.1730901926.git.ps@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).