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:

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

I do not expect any single person to tackle the splitting.  I just
wished that a patch inspired by this patch (or better yet, a new
version of this patch) made the tail end of "make test" output to
read like this:

   ...
   [18:32:44] t9400-git-cvsserver-server.sh ...... ok    18331 ms
   [18:32:49] t9402-git-cvsserver-refs.sh ........ ok    22902 ms
   [18:32:49] t9200-git-cvsexportcommit.sh ....... ok    25163 ms
   [18:32:51]
   All tests successful.
   Files=785, Tests=16928, 122 wallclock secs ( ...
   Result: PASS

   * The following tests took longer than 15 seconds to run.  We
     may want to look into splitting them into smaller files.

   t3404-rebase-interactive.sh ...    19 secs
   t9001-send-email.sh ...........    22 secs
   t9402-git-cvsserver-refs.sh ...    22 secs
   t9200-git-cvsexportcommit.sh ..    25 secs

when the hidden feature is _not_ used, so that wider set of people
will be forced to see that some tests take inordinate amount of
time, and entice at least some of them to look into it.

> Collecting the skipped ones is easy enough to do with a grep + for
> loop, so I don't think it's worth making the implementation more
> complex to occasionally answer the question of how many tests were
> skipped due to running into the timeout

"Easy enough" and "made to stand out so _NO_ effort is needed to
see" are very different things.

> Or you can just run in a mode where you stream out however many tests
> you're going to run as you go along, and then print "1..NUM_TESTS" at
> the end.
>
> We use the latter, so we can abort the entire test suite at any time
> with test_done, that's what this change does.

cf. bf4b7219 ("test-lib.sh: Add check for invalid use of 'skip_all'
facility", 2012-09-01)

>> Oh, by the way, is "date +%s" even portable?  I thought not.
>
> The lib-git-p4.sh lib says not, and shells out to python's time() is a
> workaround, I could replace this with perl -e 'print time', but
> thought it wasn't worth bothering with for an obscure optional feature
> like this.

Let's do the right thing, because doing so is easy.

I personally think that filter-branch being broken is not noticed
only because it is not very often used, as opposed to that we want
to encourage those who are following along with us, especially those
who are on minority platforms, to run our tests every day.  Let's
not spread sloppyness unnecessarily.