@matheusbsbs / matheus-bagliano / commits / 1fc7ddf35b

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 <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Nov 20, 2024 at 14:39 UTC 1fc7ddf35b727a12f78a427ae7e829c77d43fd56

4 files changed +1 -97

ci/lib.sh

-1

@@ -384,7 +384,6 @@ linux-musl)

384 ;;

385 linux-leaks|linux-reftable-leaks)

386 export SANITIZE=leak

387 - export GIT_TEST_PASSING_SANITIZE_LEAK=true

387 ;;

388 linux-asan-ubsan)

389 export SANITIZE=address,undefined

t/README

-21

@@ -368,27 +368,6 @@ excluded as so much relies on it, but this might change in the future.

368 GIT_TEST_SPLIT_INDEX=<boolean> forces split-index mode on the whole

369 test suite. Accept any boolean values that are accepted by git-config.

370

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 -

371 GIT_TEST_PROTOCOL_VERSION=<n>, when set, makes 'protocol.version'

372 default to n.

373

t/lib-git-svn.sh

-4

@@ -1,7 +1,3 @@

1 -if test -z "$TEST_FAILS_SANITIZE_LEAK"

2 -then

3 - TEST_PASSES_SANITIZE_LEAK=true

4 -fi

1 . ./test-lib.sh

3 if test -n "$NO_SVN_TESTS"

t/test-lib.sh

+1 -71

@@ -1227,23 +1227,7 @@ check_test_results_san_file_ () {

1227 fi &&

1228 say_color error "$(cat "$TEST_RESULTS_SAN_FILE".*)" &&

1229

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

1231 then

1232 say "Our logs revealed a memory leak, exit non-zero!" &&

1233 invert_exit_code=t

@@ -1274,11 +1258,6 @@ test_done () {

1258 EOF

1259 fi

1260

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 -

1261 if test "$test_fixed" != 0

1262 then

1263 say_color error "# $test_fixed known breakage(s) vanished; please update test(s)"

@@ -1515,51 +1494,8 @@ then

1494 test_done

1495 fi

1496

1518 -BAIL_OUT_ENV_NEEDS_SANITIZE_LEAK () {

1519 - BAIL_OUT "$1 has no effect except when compiled with SANITIZE=leak"

1520 -}

1521 -

1497 if test -n "$SANITIZE_LEAK"

1498 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 -

1499 rm -rf "$TEST_RESULTS_SAN_DIR"

1500 if ! mkdir -p "$TEST_RESULTS_SAN_DIR"

1501 then

@@ -1574,12 +1510,6 @@ then

1510 prepend_var LSAN_OPTIONS : log_exe_name=1

1511 prepend_var LSAN_OPTIONS : log_path="'$TEST_RESULTS_SAN_FILE'"

1512 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"

1513 fi

1514

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