Re: [PATCH] test-lib: add ability to cap the runtime of tests

> On 04 Jun 2017, at 09:29, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> 
> On Sun, Jun 4, 2017 at 2:31 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>> 
>>> Speeding up the test suite by simply cataloging and skipping tests
>>> that take longer than N seconds is a hassle to maintain, and entirely
>>> skips some tests which would be nice to at least partially run,
>>> e.g. instead of entirely skipping t3404-rebase-interactive.sh we can
>>> run it for N seconds and get at least some "git rebase -i" test
>>> coverage in a fast test run.
>> 
>> I'd be more supportive to the former approach in the longer run for
>> two reasons.
>> 
>> Is it even safe to stop a test in the middle?  Won't we leave
>> leftover server processes, for example?
>> 
>>    I see start_httpd at least sets up "trap" to call stop_httpd
>>    when the shell exits, so HTTP testing via lib-httpd.sh may be
>>    safe.  I do not know about other network-y tests, though.
> 
> When this flag is in effect and you run into the timeout the code is
> semantically equivalent to not running subsequent test_expect_*
> blocks, things like the trap in lib-httpd.sh will still run, so will
> test_when_finished.
> 
> Unless we have some test killing a daemon in a test_expect_success
> block later in the test this'll work as intended.
> 
>> Granted, when a test fails, we already have the same problem, but
>> then we'd go in and investigate, and the first thing we notice would
>> be that the old leftover server instance is holding onto the port to
>> prevent the attempt to re-run the test from running, which then we'd
>> kill.  But with this option, the user is not even made aware of
>> tests being killed in the middle.
>> 
>>> While running with a timeout of 10 seconds cuts the runtime in half,
>>> over 92% of the tests are still run. The test coverage is higher than
>>> that number indicates, just taking into account the many similar tests
>>> t0027-auto-crlf.sh runs brings it up to 95%.
>> 
>> I certainly understand that but in the longer term, I'd prefer the
>> approach to call out an overly large test.  That will hopefully
>> motivate us to split it (or speed up the thing) to help folks on
>> many-core machines.
> 
> The reason I didn't document this in t/README was because I thought it
> made sense to have this as a mostly hidden feature that end users
> wouldn't be tempted to fiddle with, but would be useful to someone
> doing git development.
> 
> Realistically I'm going to submit this patch, I'm not going to take
> the much bigger project of refactoring the entire test suite so that
> no test runs under N second, and of course any such refactoring can
> only aim for a fixed instead of dynamic N.
> 
> The point of this change is that I can replace running e.g. "prove
> t[0-9]*{grep,log}*.sh" with just running the full test suite every
> time, since 30s is noticeably slow during regular hacking but once
> it's down to 15s it's perceptively fast enough.
> 
> Reading between the lines in your reply, I think you're afraid that
> regular users just testing git out will start using this, as opposed
> to power user developers who understand the trade-offs. I think that's
> mostly mitigated by not documenting it in t/README, but I could amend
> the patch to add some scary commend to test-lib.sh as well.