Re: [PATCH] t7610: fix flaky timeout issue, don't clone from example.com

From: Taylor Blau <me@ttaylorr.com>
Date: Nov 9, 2022, 22:47 UTC
Message-ID: <Y2wuEDGMRg99Ifef@nand.local>
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:

Show 5 quoted lines

> 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