bug#7868: splitting up test suites

[Stefano Lattarini]

On Thursday 20 January 2011, Ralf Wildenhues wrote:
> Hi Stefano,
> 
> * Stefano Lattarini wrote on Thu, Jan 20, 2011 at 12:38:17PM CET:
> > On Wednesday 19 January 2011, Ralf Wildenhues wrote:
> > > We've finally reached the point where we have more than 1000
> > > tests, $(TESTS) expands to 15k characters, and where 'make check' will
> > > not work at all any more on MSYS, because it cannot spawn sh any more,
> > > presumably in 'make check TESTS="..."'.  (MSYS make doesn't export
> > > macros to the environment of spawned processes even without .NOEXPORT,
> > > presumably otherwise lots of Makefiles would be really unusable here.)
> > > This also clears up the spurious failure of sed a few days ago.
> > >  
> > > Here's a preliminary plan for multiple testsuites per Makefile.am.
> > >
> > Hmmm... while this feature might be worth having even indipendently
> > from the issue at hand (but see below for small nits), I still think
> > that in the long run it would be nicer to transparently work around
> > such command-line length issues in the test driver, if possible.
> 
> No, that is not possible with portable make.
>
Yes :-(  I've found that out while trying to write a a proper test
'parallel-tests-long-cmdline.test' (attached, just for reference).

Relevant excerpt of the log (on Linux 2.6.30 with Bash 4.1 and GNU
make 3.81):

 + automake-1.11 --foreign -Werror -Wall -a
 + ./configure
 checking for a BSD-compatible install... /usr/bin/install -c
 checking whether build environment is sane... yes
 checking for a thread-safe mkdir -p... /bin/mkdir -p
 checking for gawk... gawk
 checking whether make sets $(MAKE)... yes
 configure: creating ./config.status
 config.status: creating Makefile
 + make check
 make[1]: execvp: /bin/sh: Argument list too long
 make[1]: *** [check-TESTS] Error 127
 make: *** [check-am] Error 2

But this makes me think.  If I substitute the `$MAKE check' call
in my test with a `$MAKE dist' call, I get:

  + make dist
  { test ! -d "parallel-tests-long-cmdline-1.0" || \
    { find "parallel-tests-long-cmdline-1.0" -type d ! -perm -200 -exec chmod 
u+w {} ';' \
      && rm -fr "parallel-tests-long-cmdline-1.0"; }; }
  test -d "parallel-tests-long-cmdline-1.0" || mkdir 
"parallel-tests-long-cmdline-1.0"
  make: execvp: /bin/sh: Argument list too long
  make: *** [distdir] Error 127

which means that, with the current implementation of the `dist' target,
even breaking the testsuite in two or more testsuites *in the same
directory* won't help at "make dist" time.  Or am I missing something?

> > Do you think your patch "parallel-tests: avoid command-line length
> > limit issue" (from commit v1.11-191-g24e3b4e) could be resurrected
> > in some way?
> 
> No.  It is fundamentally flawed.  Here's why: while it may fix things at
> the level of recursion below that, eventually the rule commands which
> were changed in that patch will fail to execute, because they pose too
> long a command line for the shell.
>
Exactly :-(

> There simply is no way to get things to scale except by using GNU make
> specifics, or splitting long lists of files up manually.  Well, an
> external list of files to be read in could work,
>
I thought about that too ..

> but then we don't have make semantics easily available.
>
... and I thought about this too.  But maybe allowing the user to say,
e.g.:

 TESTS = test1 test2 test3
 EXTRA_TESTS_LIST = @file-with-list-of-test

might still be worth after all; if he prefers to lose some make semantics
in order not to have to split the testsuite, then he should be allowed to
do so.  It remains to see if that's doable (preferably in an easy way).

> In that light, I'm sorry you already wrote the patch you did, because I
> don't see how it can improve things significantly.  I didn't try the
> patch or look in detail, please ping me if you still want me to do that.
>
No need to: the approach is really fundamentally flawed (as you said).

> > > It would be nice if this worked somehow:
> > > 
> > > # These are all specified by the user:
> > > TEST_SUITE_LOGS = suite1.log suite2.log suite3.log ..
> > > TEST_EXTENSIONS = .test ...
> > > # these undergo $(EXEEXT) autoexpansion internally:
> > > suite1_log_SOURCES = all.test aclocal7.test ...
> > > suite2_log_SOURCES = suffix.test ...
> > >
> > What about using `TESTS' instead of `SOURCES' in these two last variables?
> > That would seems more natural to me, especially considering the API of the
> > current parallel-tests driver.
> 
> First off, it would make sense to review the several discussions we had
> about this back when parallel-tests was implemented (more precisely,
> ported from Akim's GNU make-specific implementation).  See e.g.,
> http://thread.gmane.org/gmane.comp.sysutils.automake.patches/3225
> http://thread.gmane.org/gmane.comp.sysutils.automake.general/8063
>
Thanks for the pointers, I'll take a look.

> The idea was to treat tests as we treat compiled languages:
> .c files are sources for programs but get turned into .o files.
> Here, .test files are sources for testsuite logs, but the intermediates
> are the per-test .log files.  And there's maybe the unification of
> suite*.log's to a final log on top of that.
>
I haven't thought in thie terms before -- nice abstraction!  But see
below.

> If we can handle the latter via automake's register_language machinery,
> then putting tests in *_SOURCES variables would be fairly natural.
> 
> OTOH, you are right in that elsewhere, _SOURCES are usually meant to be
> final, nonderived files,
>
That's not exactly true, as automke allows (as it IMHO should)
generated files to be places in *_SOURCES.  But automake also needs
to know the contents of a foo_SOURCES variable statically [1] at
Makefile.in's generation time, while it wants to allow the user to
specify *at make runtime* the list of TESTS to run.

 [1] OK, it's smart enough to resolve variable indirections and
     conditionals, but won't allow @var@ substitution to be placed
     in a *_SOURCES variable (by design).

> which doesn't fit the picture here.  So maybe suite1_log_TESTS
> is more appropriate after all.

> > OTOH, would that maybe make the implementation more difficult?
> 
> Not if it's like the current implementation, no.
> 
> > I think it would also be nice to generate separate check/recheck targets
> > for each testsuite; for example, "make check-suite1" could run all the
> > tests in $(suite1_log_SOURCES), and "make recheck-suite2" could re-run
> > all the tests in $(suite2_log_SOURCES) that failed (or xfailed) in the
> > previous run.
> > 
> > Hmm... No, wait, it would be even nicer to allow the user choose which
> > testsuite(s) to run by resetting the $(TEST_SUITE_LOGS) variable:
> > 
> >   make check TEST_SUITE_LOGS='suite1.log suite2.log'
> > 
> > Which makes me think that, perhaps, a variable like $(TEST_SUITES) would
> > be preferable:
> > 
> >   make check TEST_SUITES='suite1 suite2'
> > 
> > but then the API should be changed to something like:
> > 
> >  # These are all specified by the user:
> >  TEST_SUITES = suite1 suite2 suite3 ..
> >  TEST_EXTENSIONS = .test ...
> >  # these undergo $(EXEEXT) autoexpansion internally:
> >  suite1_TESTS = all.test aclocal7.test ...
> >  suite2_TESTS = suffix.test ...
> > 
> >  # These are then produced by automake:
> >  TEST_SUITE_LOGS = $(TEST_SUITES:=.log)
> >  TEST_SUITE_HTMLS = $(TEST_SUITES:=.html)
> >  suite1_LOGS = $(am__helper_var:.test=.log)
> 
> Unfortunately, this is error-prone, because some GNU make versions
> expand
>   empty =
>   TEST_SUITES = foo $(empty)
>   TEST_SUITE_LOGS = $(TEST_SUITES:=.log)
> 
> into `foo.log .log' rather than `foo.log '.  This happens in practice
> if `empty' is set only under some Automake conditional, as in
>   if COND
>   TESTS += bar
>   endif
>
And I guess that specifying dummy `.log:' (and maybe `.html:' etc.)
rules won't work either, right? :-(

> That is the reason the check-TESTS rule is so ugly (and recursive) in
> the first place.  I really would like to avoid more instances of this
> wart; so specifying files without extension is Not Good(TM).
>
OK, noted (and these considerations could IMHO end up somewhere in the
manual).

> Separate check/recheck targets are of course nice, but IMVHO optional
> for the first iteration.
>
I agree; the only important thing is to devise a design that will allow
them to be easily added in later refinings.

> > > In the <suite>_SOURCES, $(EXEEXT) transformation should take place
> > > (unless no-exeext is given, of course), just as is currently done for
> > > TESTS and *_PROGRAMS.  Hmm, alternatively we could also require all
> > > <suite>_SOURCES to be listed in $(TESTS), that would allow reuse of
> > > this variable as well, at the cost of some specification redundance.
> > >
> > 
> > > Open questions: how to produce nice results, both on stdout and in suite
> > > log files.  One way is to merge all logs into a final TEST_SUITE_LOG
> > > (that way we could also reuse that variable).  Another is to try to even
> > > hide the summaries of the individual suites, iff a final suite is being
> > > made.
> > >
> > These both sound sensible.  Another problem is how to avoid that, in a
> > parallel make run, the summary from a testsuite gets mixed and garbled
> > with the output/summary from another testsuite, as in e.g.:
> 
> Yes.  We definitely want to be able to run tests from different
> testsuites in parallel.  So some form of partial summary hiding is
> prudent; I hope we can achieve that without yet another make
> indirection, but I'm not optimistic.
> 
> Thanks for the feedback,
> Ralf
> 

Regards,
  Stefano

parallel-tests-long-cmdline.test
Description: application/shellscript