[PATCH] t: remove unexpected SANITIZE_LEAK variables

[PATCH] t: remove unexpected SANITIZE_LEAK variables

[Karthik Nayak]

As of 1fc7ddf35b (test-lib: unconditionally enable leak checking,
2024-11-20), both the `GIT_TEST_PASSING_SANITIZE_LEAK` and
`TEST_PASSES_SANITIZE_LEAK` variables no longer have any meaning, the
leak checks are enabled by default. However, some newly added tests
include them by mistake. Let's clean this up.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 t/perf/p5313-pack-objects.sh | 3 ---
 t/perf/p5314-name-hash.sh    | 3 ---
 t/t6601-path-walk.sh         | 2 --
 3 files changed, 8 deletions(-)

diff --git a/t/perf/p5313-pack-objects.sh b/t/perf/p5313-pack-objects.sh
index be5229a0ec..786a2c1c6f 100755
--- a/t/perf/p5313-pack-objects.sh
+++ b/t/perf/p5313-pack-objects.sh
@@ -3,9 +3,6 @@
 test_description='Tests pack performance using bitmaps'
 . ./perf-lib.sh
 
-GIT_TEST_PASSING_SANITIZE_LEAK=0
-export GIT_TEST_PASSING_SANITIZE_LEAK
-
 test_perf_large_repo
 
 test_expect_success 'create rev input' '
diff --git a/t/perf/p5314-name-hash.sh b/t/perf/p5314-name-hash.sh
index 4ef0ba7711..235cdfc824 100755
--- a/t/perf/p5314-name-hash.sh
+++ b/t/perf/p5314-name-hash.sh
@@ -3,9 +3,6 @@
 test_description='Tests pack performance using bitmaps'
 . ./perf-lib.sh
 
-GIT_TEST_PASSING_SANITIZE_LEAK=0
-export GIT_TEST_PASSING_SANITIZE_LEAK
-
 test_perf_large_repo
 
 test_size 'paths at head' '
diff --git a/t/t6601-path-walk.sh b/t/t6601-path-walk.sh
index c89b0f1e19..8d187f7279 100755
--- a/t/t6601-path-walk.sh
+++ b/t/t6601-path-walk.sh
@@ -1,7 +1,5 @@
 #!/bin/sh
 
-TEST_PASSES_SANITIZE_LEAK=true
-
 test_description='direct path-walk API tests'
 
 . ./test-lib.sh

Re: [PATCH] t: remove unexpected SANITIZE_LEAK variables

[Justin Tobler]

On 25/05/20 04:40PM, Karthik Nayak wrote:
> As of 1fc7ddf35b (test-lib: unconditionally enable leak checking,
> 2024-11-20), both the `GIT_TEST_PASSING_SANITIZE_LEAK` and
> `TEST_PASSES_SANITIZE_LEAK` variables no longer have any meaning, the
> leak checks are enabled by default. However, some newly added tests
> include them by mistake. Let's clean this up.

Indeed, both `GIT_TEST_PASSING_SANITIZE_LEAK` and
`TEST_PASSES_SANITIZE_LEAK` appear not have any purpose anymore.
Removing all remaining instances where they appear makes sense and from
a quick search it looks like this patch got them all. So this looks good
to me.

-Justin

> 
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  t/perf/p5313-pack-objects.sh | 3 ---
>  t/perf/p5314-name-hash.sh    | 3 ---
>  t/t6601-path-walk.sh         | 2 --
>  3 files changed, 8 deletions(-)
> 
> diff --git a/t/perf/p5313-pack-objects.sh b/t/perf/p5313-pack-objects.sh
> index be5229a0ec..786a2c1c6f 100755
> --- a/t/perf/p5313-pack-objects.sh
> +++ b/t/perf/p5313-pack-objects.sh
> @@ -3,9 +3,6 @@
>  test_description='Tests pack performance using bitmaps'
>  . ./perf-lib.sh
>  
> -GIT_TEST_PASSING_SANITIZE_LEAK=0
> -export GIT_TEST_PASSING_SANITIZE_LEAK
> -
>  test_perf_large_repo
>  
>  test_expect_success 'create rev input' '
> diff --git a/t/perf/p5314-name-hash.sh b/t/perf/p5314-name-hash.sh
> index 4ef0ba7711..235cdfc824 100755
> --- a/t/perf/p5314-name-hash.sh
> +++ b/t/perf/p5314-name-hash.sh
> @@ -3,9 +3,6 @@
>  test_description='Tests pack performance using bitmaps'
>  . ./perf-lib.sh
>  
> -GIT_TEST_PASSING_SANITIZE_LEAK=0
> -export GIT_TEST_PASSING_SANITIZE_LEAK
> -
>  test_perf_large_repo
>  
>  test_size 'paths at head' '
> diff --git a/t/t6601-path-walk.sh b/t/t6601-path-walk.sh
> index c89b0f1e19..8d187f7279 100755
> --- a/t/t6601-path-walk.sh
> +++ b/t/t6601-path-walk.sh
> @@ -1,7 +1,5 @@
>  #!/bin/sh
>  
> -TEST_PASSES_SANITIZE_LEAK=true
> -
>  test_description='direct path-walk API tests'
>  
>  . ./test-lib.sh
> 
> 
> 
> 

Re: [PATCH] t: remove unexpected SANITIZE_LEAK variables

[Junio C Hamano]

Justin Tobler <jltobler@gmail.com> writes:

> On 25/05/20 04:40PM, Karthik Nayak wrote:
>> As of 1fc7ddf35b (test-lib: unconditionally enable leak checking,
>> 2024-11-20), both the `GIT_TEST_PASSING_SANITIZE_LEAK` and
>> `TEST_PASSES_SANITIZE_LEAK` variables no longer have any meaning, the
>> leak checks are enabled by default. However, some newly added tests
>> include them by mistake. Let's clean this up.
>
> Indeed, both `GIT_TEST_PASSING_SANITIZE_LEAK` and
> `TEST_PASSES_SANITIZE_LEAK` appear not have any purpose anymore.
> Removing all remaining instances where they appear makes sense and from
> a quick search it looks like this patch got them all. So this looks good
> to me.

Thanks, both.

Re: [PATCH] t: remove unexpected SANITIZE_LEAK variables

[Jeff King]

On Tue, May 20, 2025 at 04:40:12PM +0200, Karthik Nayak wrote:

> As of 1fc7ddf35b (test-lib: unconditionally enable leak checking,
> 2024-11-20), both the `GIT_TEST_PASSING_SANITIZE_LEAK` and
> `TEST_PASSES_SANITIZE_LEAK` variables no longer have any meaning, the
> leak checks are enabled by default. However, some newly added tests
> include them by mistake. Let's clean this up.

Thanks, I saw these recently while looking at another topic and was
surprised. I hadn't yet confirmed that they truly are pointless, so I'm
glad that you did. :)

As a side note, we do still use the SANITIZE_LEAK prereq in a few
places, and I believe that it does actually work. It might be nice to
clean up any leaks in those few spots, though we probably want to keep
the prereq around forever (e.g., if you introduce a test which shows off
a leak and then fixes it later). All orthogonal to your patch, though,
which looks good to me.

-Peff

Re: [PATCH] t: remove unexpected SANITIZE_LEAK variables

[Jeff King]

On Thu, May 22, 2025 at 02:06:26AM -0400, Jeff King wrote:

> As a side note, we do still use the SANITIZE_LEAK prereq in a few
> places, and I believe that it does actually work. It might be nice to
> clean up any leaks in those few spots, though we probably want to keep
> the prereq around forever (e.g., if you introduce a test which shows off
> a leak and then fixes it later). All orthogonal to your patch, though,
> which looks good to me.

Ah, nevermind. These are all due to 8415595203 (t5601: work around leak
sanitizer issue, 2024-11-20). They are leak-free (and the tests pass on
my system), but apparently some upstream bug can cause issues.

-Peff

Re: [PATCH] t: remove unexpected SANITIZE_LEAK variables

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 807 bytes --]

Jeff King <peff@peff.net> writes:

> On Thu, May 22, 2025 at 02:06:26AM -0400, Jeff King wrote:
>
>> As a side note, we do still use the SANITIZE_LEAK prereq in a few
>> places, and I believe that it does actually work. It might be nice to
>> clean up any leaks in those few spots, though we probably want to keep
>> the prereq around forever (e.g., if you introduce a test which shows off
>> a leak and then fixes it later). All orthogonal to your patch, though,
>> which looks good to me.
>
> Ah, nevermind. These are all due to 8415595203 (t5601: work around leak
> sanitizer issue, 2024-11-20). They are leak-free (and the tests pass on
> my system), but apparently some upstream bug can cause issues.
>
> -Peff

Yup. Someday in the future we can cleanup the whole thing. That would be
nice!

- Karthik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]