Skip to content

Commit 1fc7ddf

Browse files
pks-tgitster
authored andcommitted
test-lib: unconditionally enable leak checking
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 0b7f0ce commit 1fc7ddf

File tree

4 files changed

+1
-97
lines changed

4 files changed

+1
-97
lines changed

ci/lib.sh

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,6 @@ linux-musl)
384384
;;
385385
linux-leaks|linux-reftable-leaks)
386386
export SANITIZE=leak
387-
export GIT_TEST_PASSING_SANITIZE_LEAK=true
388387
;;
389388
linux-asan-ubsan)
390389
export SANITIZE=address,undefined

t/README

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -368,27 +368,6 @@ excluded as so much relies on it, but this might change in the future.
368368
GIT_TEST_SPLIT_INDEX=<boolean> forces split-index mode on the whole
369369
test suite. Accept any boolean values that are accepted by git-config.
370370

371-
GIT_TEST_PASSING_SANITIZE_LEAK=true skips those tests that haven't
372-
declared themselves as leak-free by setting
373-
"TEST_PASSES_SANITIZE_LEAK=true" before sourcing "test-lib.sh". This
374-
test mode is used by the "linux-leaks" CI target.
375-
376-
GIT_TEST_PASSING_SANITIZE_LEAK=check checks that our
377-
"TEST_PASSES_SANITIZE_LEAK=true" markings are current. Rather than
378-
skipping those tests that haven't set "TEST_PASSES_SANITIZE_LEAK=true"
379-
before sourcing "test-lib.sh" this mode runs them with
380-
"--invert-exit-code". This is used to check that there's a one-to-one
381-
mapping between "TEST_PASSES_SANITIZE_LEAK=true" and those tests that
382-
pass under "SANITIZE=leak". This is especially useful when testing a
383-
series that fixes various memory leaks with "git rebase -x".
384-
385-
GIT_TEST_PASSING_SANITIZE_LEAK=check when combined with "--immediate"
386-
will run to completion faster, and result in the same failing
387-
tests.
388-
389-
GIT_TEST_PASSING_SANITIZE_LEAK=check-failing behaves the same as "check",
390-
but skips all tests which are already marked as leak-free.
391-
392371
GIT_TEST_PROTOCOL_VERSION=<n>, when set, makes 'protocol.version'
393372
default to n.
394373

t/lib-git-svn.sh

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,3 @@
1-
if test -z "$TEST_FAILS_SANITIZE_LEAK"
2-
then
3-
TEST_PASSES_SANITIZE_LEAK=true
4-
fi
51
. ./test-lib.sh
62

73
if test -n "$NO_SVN_TESTS"

t/test-lib.sh

Lines changed: 1 addition & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1227,23 +1227,7 @@ check_test_results_san_file_ () {
12271227
fi &&
12281228
say_color error "$(cat "$TEST_RESULTS_SAN_FILE".*)" &&
12291229

1230-
if test -n "$passes_sanitize_leak" && test "$test_failure" = 0
1231-
then
1232-
say "As TEST_PASSES_SANITIZE_LEAK=true and our logs show we're leaking, exit non-zero!" &&
1233-
invert_exit_code=t
1234-
elif test -n "$passes_sanitize_leak"
1235-
then
1236-
say "As TEST_PASSES_SANITIZE_LEAK=true and our logs show we're leaking, and we're failing for other reasons too..." &&
1237-
invert_exit_code=
1238-
elif test -n "$sanitize_leak_check" && test "$test_failure" = 0
1239-
then
1240-
say "As TEST_PASSES_SANITIZE_LEAK=true isn't set the above leak is 'ok' with GIT_TEST_PASSING_SANITIZE_LEAK=check" &&
1241-
invert_exit_code=
1242-
elif test -n "$sanitize_leak_check"
1243-
then
1244-
say "As TEST_PASSES_SANITIZE_LEAK=true isn't set the above leak is 'ok' with GIT_TEST_PASSING_SANITIZE_LEAK=check" &&
1245-
invert_exit_code=t
1246-
elif test "$test_failure" = 0
1230+
if test "$test_failure" = 0
12471231
then
12481232
say "Our logs revealed a memory leak, exit non-zero!" &&
12491233
invert_exit_code=t
@@ -1274,11 +1258,6 @@ test_done () {
12741258
EOF
12751259
fi
12761260

1277-
if test -z "$passes_sanitize_leak" && test_bool_env TEST_PASSES_SANITIZE_LEAK false
1278-
then
1279-
BAIL_OUT "Please, set TEST_PASSES_SANITIZE_LEAK before sourcing test-lib.sh"
1280-
fi
1281-
12821261
if test "$test_fixed" != 0
12831262
then
12841263
say_color error "# $test_fixed known breakage(s) vanished; please update test(s)"
@@ -1515,51 +1494,8 @@ then
15151494
test_done
15161495
fi
15171496

1518-
BAIL_OUT_ENV_NEEDS_SANITIZE_LEAK () {
1519-
BAIL_OUT "$1 has no effect except when compiled with SANITIZE=leak"
1520-
}
1521-
15221497
if test -n "$SANITIZE_LEAK"
15231498
then
1524-
# Normalize with test_bool_env
1525-
passes_sanitize_leak=
1526-
1527-
# We need to see TEST_PASSES_SANITIZE_LEAK in "test-tool
1528-
# env-helper" (via test_bool_env)
1529-
export TEST_PASSES_SANITIZE_LEAK
1530-
if test_bool_env TEST_PASSES_SANITIZE_LEAK false
1531-
then
1532-
passes_sanitize_leak=t
1533-
fi
1534-
1535-
if test "$GIT_TEST_PASSING_SANITIZE_LEAK" = "check" ||
1536-
test "$GIT_TEST_PASSING_SANITIZE_LEAK" = "check-failing"
1537-
then
1538-
if test "$GIT_TEST_PASSING_SANITIZE_LEAK" = "check-failing" &&
1539-
test -n "$passes_sanitize_leak"
1540-
then
1541-
skip_all="skipping leak-free $this_test under GIT_TEST_PASSING_SANITIZE_LEAK=check-failing"
1542-
test_done
1543-
fi
1544-
1545-
sanitize_leak_check=t
1546-
if test -n "$invert_exit_code"
1547-
then
1548-
BAIL_OUT "cannot use --invert-exit-code under GIT_TEST_PASSING_SANITIZE_LEAK=check"
1549-
fi
1550-
1551-
if test -z "$passes_sanitize_leak"
1552-
then
1553-
say "in GIT_TEST_PASSING_SANITIZE_LEAK=check mode, setting --invert-exit-code for TEST_PASSES_SANITIZE_LEAK != true"
1554-
invert_exit_code=t
1555-
fi
1556-
elif test -z "$passes_sanitize_leak" &&
1557-
test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false
1558-
then
1559-
skip_all="skipping $this_test under GIT_TEST_PASSING_SANITIZE_LEAK=true"
1560-
test_done
1561-
fi
1562-
15631499
rm -rf "$TEST_RESULTS_SAN_DIR"
15641500
if ! mkdir -p "$TEST_RESULTS_SAN_DIR"
15651501
then
@@ -1574,12 +1510,6 @@ then
15741510
prepend_var LSAN_OPTIONS : log_exe_name=1
15751511
prepend_var LSAN_OPTIONS : log_path="'$TEST_RESULTS_SAN_FILE'"
15761512
export LSAN_OPTIONS
1577-
1578-
elif test "$GIT_TEST_PASSING_SANITIZE_LEAK" = "check" ||
1579-
test "$GIT_TEST_PASSING_SANITIZE_LEAK" = "check-failing" ||
1580-
test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false
1581-
then
1582-
BAIL_OUT_ENV_NEEDS_SANITIZE_LEAK "GIT_TEST_PASSING_SANITIZE_LEAK=true"
15831513
fi
15841514

15851515
if test "${GIT_TEST_CHAIN_LINT:-1}" != 0 &&

0 commit comments

Comments
 (0)