From: Taylor Blau <me@ttaylorr.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
Taylor Blau <me@ttaylorr.com>
Subject: Re: [PATCH] t7610: fix flaky timeout issue, don't clone from example.com
Date: Wed, 9 Nov 2022 17:47:44 -0500 [thread overview]
Message-ID: <Y2wuEDGMRg99Ifef@nand.local> (raw)
In-Reply-To: <patch-1.1-83eca7b5a7c-20221105T115420Z-avarab@gmail.com>
On Sat, Nov 05, 2022 at 12:54:21PM +0100, Ævar Arnfjörð Bjarmason wrote:
> The behavior of "-N" here might be surprising to some, since it's
> explained as "[if you use -N we] don’t fetch new objects from the
> remote site". But (perhaps counter-intuitively) it's only talking
> about if it needs to do so via "git fetch". In this case we'll end up
> spawning a "git clone", as we have no submodule set up.
Makes sense, though I'm not sure I agree this is worth patching as I
currently understand it.
If I run t7610 today with '--run=2-' (IOW, skipping the setup test), I
am definitely going to get failures. And I don't think we should have
every subsequent test depend on having run anything containing "setup"
before it. That is, it is not surprising that we will see some test
failures when carving up and running portions of the test instead of the
whole file.
I don't think this is substantively any different than that. Whether we
don't complete the "setup" test because we had some leak (and ran the
test suite with the appropriate LSan options), or because we neglected
to run it at all, I don't think there is a significant difference.
Either way, the end-state of the test repository isn't guaranteed to
match the intent of the "setup" test.
If this is the only such problem in-tree, sure, I think it's fine to
patch. But I would wager that there are *many* more than just this one
lurking, and patching all of them would be less straightforward than
this one.
So... I don't know. I'm certainly leaning negative on this patch, but if
you have some compelling reason that I'm missing, I'm all-ears.
Thanks,
Taylor
next prev parent reply other threads:[~2022-11-09 22:47 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-05 11:54 [PATCH] t7610: fix flaky timeout issue, don't clone from example.com Ævar Arnfjörð Bjarmason
2022-11-09 22:47 ` Taylor Blau [this message]
2022-11-09 23:55 ` Ævar Arnfjörð Bjarmason
2022-11-10 2:17 ` Taylor Blau
2022-11-15 19:42 ` Taylor Blau
2022-11-15 23:40 ` [PATCH] t7610: use "file:///dev/null", not "/dev/null", fixes MinGW Ævar Arnfjörð Bjarmason
2022-11-16 1:07 ` Taylor Blau
2022-11-17 10:08 ` Ævar Arnfjörð Bjarmason
2022-11-17 21:50 ` Taylor Blau
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=Y2wuEDGMRg99Ifef@nand.local \
--to=me@ttaylorr.com \
--cc=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.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 read-only IMAP folder(s) and NNTP newsgroup(s).