git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] leak tests: ignore some new leaks in a few tests
@ 2022-01-14 16:07 Elijah Newren via GitGitGadget
  2022-01-18 16:49 ` Taylor Blau
  0 siblings, 1 reply; 2+ messages in thread
From: Elijah Newren via GitGitGadget @ 2022-01-14 16:07 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

These test scripts have not changed at all recently, but topics in seen
have caused these to trigger the linux-leaks test.  It has been reported
on list, so let's fix the claim that these are leak free and when
someone investigates and fixes the problem, they can turn it back on.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
    leak tests: ignore some new leaks in a few tests
    
    linux-leaks passes when 'seen' is merged with this topic. A better fix
    is probably to bisect to determine which topic caused the memory leak
    and plug the leak in that topic (because these test scripts did not have
    any changes since the version when they previously passed), but a couple
    of quotes:
    
    """ This is sort of water under the bridge, as the other topic is
    already in 'master', but come to think of it, the strategy we used with
    TEST_PASSES_SANITIZE_LEAK variable was misguided.
    ... I am tempted to drop the "TEST_PASSES" bit from this script for now,
    """ (from https://lore.kernel.org/git/xmqqee6dz5s9.fsf@gitster.g/)
    
    """ But as to the "roadblock" I don't mind the
    TEST_PASSES_SANITIZE_LEAK=true being removed from the script at the
    slightest sign of trouble. Nobody should have to shift gears and chase
    down some memory leak... """ (from
    https://lore.kernel.org/git/211217.86a6gyyihr.gmgdl@evledraar.gmail.com/)

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1192%2Fnewren%2Fignore-leaks-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1192/newren/ignore-leaks-v1
Pull-Request: https://github.com/git/git/pull/1192

 t/t1430-bad-ref-name.sh                | 1 -
 t/t3211-peel-ref.sh                    | 1 -
 t/t6102-rev-list-unexpected-objects.sh | 1 -
 3 files changed, 3 deletions(-)

diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
index ff1c967d550..a3c689819fa 100755
--- a/t/t1430-bad-ref-name.sh
+++ b/t/t1430-bad-ref-name.sh
@@ -4,7 +4,6 @@ test_description='Test handling of ref names that check-ref-format rejects'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
-TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 test_expect_success setup '
diff --git a/t/t3211-peel-ref.sh b/t/t3211-peel-ref.sh
index 9cbc34fc583..37b9d26f4b6 100755
--- a/t/t3211-peel-ref.sh
+++ b/t/t3211-peel-ref.sh
@@ -4,7 +4,6 @@ test_description='tests for the peel_ref optimization of packed-refs'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
-TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 test_expect_success 'create annotated tag in refs/tags' '
diff --git a/t/t6102-rev-list-unexpected-objects.sh b/t/t6102-rev-list-unexpected-objects.sh
index 6f0902b8638..52cde097dd5 100755
--- a/t/t6102-rev-list-unexpected-objects.sh
+++ b/t/t6102-rev-list-unexpected-objects.sh
@@ -2,7 +2,6 @@
 
 test_description='git rev-list should handle unexpected object types'
 
-TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 test_expect_success 'setup well-formed objects' '

base-commit: 5ae4b10f85c8c2c00910cc805458c5c5464841e3
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] leak tests: ignore some new leaks in a few tests
  2022-01-14 16:07 [PATCH] leak tests: ignore some new leaks in a few tests Elijah Newren via GitGitGadget
@ 2022-01-18 16:49 ` Taylor Blau
  0 siblings, 0 replies; 2+ messages in thread
From: Taylor Blau @ 2022-01-18 16:49 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget; +Cc: git, Elijah Newren

On Fri, Jan 14, 2022 at 04:07:12PM +0000, Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@gmail.com>
>
> These test scripts have not changed at all recently, but topics in seen
> have caused these to trigger the linux-leaks test.  It has been reported
> on list, so let's fix the claim that these are leak free and when
> someone investigates and fixes the problem, they can turn it back on.

I think this problem was likely introduced by a faulty patch in
c2dbe00466 (Merge branch
'ps/avoid-unnecessary-hook-invocation-with-packed-refs' into seen,
2022-01-12), specifically 2e6b34bb1b (refs: open-code deletion of packed
refs, 2022-01-07).

But the newer version 69840cc0f7 of the same patch (merged via
17d1824959) fixes the leak. As a result, all three of those once-failing
tests now pass on 'seen' even when built with SANITIZE=leak.

We should be able to discard this patch as a result.

Thanks,
Taylor

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-01-18 16:50 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-14 16:07 [PATCH] leak tests: ignore some new leaks in a few tests Elijah Newren via GitGitGadget
2022-01-18 16:49 ` Taylor Blau

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).