On Wed, Nov 09 2022, Taylor Blau wrote:

On Sat, Nov 05, 2022 at 12:54:21PM +0100, Ævar Arnfjörð Bjarmason wrote:

quoted

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.

I agree with that in general, but the expected failure case for all
those other things that are broken with the missing setup is just that
we'll predictably error out immediately.

Whereas in this case we'll end up hanging on a connect() to some box
that IANA's maintaining for the example.com landing page, and after we
reach the connect() timeout we move onto the next failing test, which
will do that again. The test takes a long time to finally report an
overall failure.

I think that's worth fixing in general, aside from the leak
detection. I.e. yes in general we aren't good about running tests 3..9
successfully if the 2nd test fails.

But we generally just fail some or all of 3..9 pretty fast, and don't
start taking 20 minutes to run the test, when it took 10s before (or
whatever).