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

[Junio C Hamano <gitster@pobox.com>]

Æ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.

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.

I am afraid that the proposed change will disincentivize that by
sweeping the problematic ones under the rug.  Perhaps you can
collect what tests are terminated in the middle because they run for
too long and show the list of them at the end, or something?

Also, I thought that it was a no-no to say "to_skil=all" with
skipped-reason in the middle of a test when the test is run under
prove?

Oh, by the way, is "date +%s" even portable?  I thought not.