git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Rubén Justo" <rjusto@gmail.com>
Cc: Git List <git@vger.kernel.org>
Subject: Re: [PATCH 1/4] t0080: mark as leak-free
Date: Mon, 29 Jan 2024 15:51:33 -0800	[thread overview]
Message-ID: <xmqqwmrrg0l6.fsf@gitster.g> (raw)
In-Reply-To: <c932fbfc-f14f-4403-bfc5-cf1d616b22de@gmail.com> ("Rubén Justo"'s message of "Tue, 30 Jan 2024 00:20:22 +0100")

Rubén Justo <rjusto@gmail.com> writes:

>> The point of the t-basic tests is to ensure the lightweight unit
>> test framework that requires nothing from Git behaves (and keeps
>> behaving) sensibly.  The point of running t[0-9][0-9][0-9][0-9]
>> tests under leak sanitizer is to exercise production Git code to
>> catch leaks in Git code.
>> 
>> So it is not quite clear if we even want to run this t0080 under
>> leak sanitizer to begin with.  t0080 is a relatively tiny test, but
>> do we even want to spend leak sanitizer cycles on it?  I dunno.
>
> IIUC, that would imply building test-tool with a different set of flags
> than Git, new artifacts ...  or running test-tool with some LSAN_OPTIONS
> options, to disable it ...  or both ... or ...
>
> And that is assuming that with test-tool we won't catch a leak in Git
> that we're not seeing in the other tests ...

But t0080 does not even run test-tool, does it?  The t-basic unit
test is about testing the unit test framework and does not even
trigger any of the half-libified Git code.  So I am not sure why
you are bringing up test-tool into the picture.

> Maybe this is tangential to this series but,  while a decision is being
> made, annotating the test makes GIT_TEST_PASSING_SANITIZE_LEAK=check
> pass, which is the objective in this series. 

One major reason why we want to set TEST_PASSES_SANITIZE_LEAK to
true is because that way the marked test will be run under the leak
sanitizer in the CI.

What do we expect to gain by running t0080, which is to run the
t-basic unit test, under the leak sanitizer?  Unlike other
t[0-9][0-9][0-9][0-9] tests that exercise Git production code, would
we care about a new leak found in t-basic run from t0080 in the
first place?

Annotating with TEST_PASSES_SANITIZE_LEAK is not a goal by itself.
Annotating the tests that we want to run under the sanitizer and see
them passing with it is.  And obviously these tests that exercise
Git production code are very good candidates for us to do so.  It is
unclear if t0080 falls into the same category.  That is why I asked
what we expect to gain by running it.

Thanks.

  reply	other threads:[~2024-01-29 23:51 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-29 21:04 [PATCH 0/4] mark tests as leak-free Rubén Justo
2024-01-29 21:08 ` [PATCH 1/4] t0080: mark " Rubén Justo
2024-01-29 22:15   ` Junio C Hamano
2024-01-29 23:20     ` Rubén Justo
2024-01-29 23:51       ` Junio C Hamano [this message]
2024-01-30 18:14         ` Rubén Justo
2024-01-30  5:53     ` Jeff King
2024-01-30 20:00       ` Junio C Hamano
2024-01-29 21:08 ` [PATCH 2/4] t5332: " Rubén Justo
2024-01-29 21:08 ` [PATCH 3/4] t6113: " Rubén Justo
2024-01-29 21:08 ` [PATCH 4/4] test-lib: check for TEST_PASSES_SANITIZE_LEAK Rubén Justo
2024-01-29 22:25   ` Junio C Hamano
2024-01-30  5:54 ` [PATCH 0/4] mark tests as leak-free Jeff King
2024-01-30 16:01   ` Junio C Hamano

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=xmqqwmrrg0l6.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=rjusto@gmail.com \
    /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).