Re: prepare Automake's test suite for parallelization

[Ralf Wildenhues]

* Benoit SIGOURE wrote on Sun, Nov 04, 2007 at 07:44:10PM CET:
> On Nov 4, 2007, at 7:07 PM, Ralf Wildenhues wrote:
>
>> - for XPASS tests we do the wrong thing: remove the test directory.
>>   This is because our suite's defs.in has no idea that we are running
>>   an XPASS.  The keep_testdirs is a cheap workaround.
>>
>> I guess for the latter, check-TESTS should indicate to the test being
>> run that it is expected to fail.  A bit ugly.
>
> Admittedly it's ugly, but easy to do and it "gets the job done".  I can't 
> think of anything better.  Maybe the cleaning could be done by check-local, 

(You mean check-TESTS here, no?)

> when it really knows whether the test is a PASS or not?

Well, but that would require us to document an interface, as to how the
TESTS test suite works.  Not just for Automake's own suite, but for
everyone that uses TESTS in their Makefile.am files.  I would like to
avoid that if at all possible, it seems like a great burden for users.

OTOH, if everyone ends up having the same problem, we may as well
consider providing a suitable way out.

I would like to be able to support test suites where more than one test
uses the same directory, because they happen to depend upon each other.
Yes, I also would like to allow the user to specify interdependencies
among tests.  This needs more work.

>> FWIW2, we could use subdirectories named after the test name rather than
>> using the PID. [...]

> It's not convenient.  If there's a failure, you'll have to find out which 
> sub-directory to look at, right?

Yes, that is correct, and it's a small disadvantage, I agree.

> Most modern OSes randomize PIDs, so it's 
> going to be cumbersome.  My personal opinion would be to use something 
> based on the test name.  If you want, you can try to combine the test-name 
> and the PID?  Or maybe increment a number for each test and use that?  
> (this will need to deal nicely with parallel make check though, so it might 
> not be trivial).

Using a number is ugly for parallel, as you say.  Based on test name (or
log name) would be ok.

Jim's suggestion to use mk(d)temp (thanks!) sounds not so bad, but
currently I don't see the need for it inside Automake.  We don't
play games with multiple users or groups in the test suite.
And it raises portability issues, so if we need it, we should use
the code that `info Autoconf 'Limitations of Usual Tools'' suggests.

>> --- a/tests/Makefile.am
>> +++ b/tests/Makefile.am
>> @@ -625,5 +625,8 @@ EXTRA_DIST = ChangeLog-old $(TESTS)
>>  # Each test case depends on defs, aclocal, and automake.
>>  check_SCRIPTS = defs aclocal-$(APIVERSION) automake-$(APIVERSION)
>>
>> -distclean-local:
>> -    -rm -rf testSubDir
>> +distclean-local: check-clean-local
>> +
>> +check-clean-local:
>> +    -chmod -R a+rwx testSubDir*
>
> u+rwx is enough, there's no need in making this open to the entire world ;o

Good point.  I didn't invent this code though.  There are a few places
in Automake that make code world-accessible, one of them can be seen in
the change below to defs.in; the distdir target has similar.  I've
wondered before why that was.  I'm currently too dumb to get git to find
for me the reason why this was introduced (something with git-log -S?).

>> +    -rm -rf testSubDir*
>
>
>> diff --git a/tests/defs.in b/tests/defs.in
>> index 78cf37a..2ad4345 100644
>> --- a/tests/defs.in
>> +++ b/tests/defs.in
>> @@ -235,16 +235,35 @@ case "$srcdir" in
>>      ;;
>>  esac
>>
>> -chmod -R a+rwx testSubDir > /dev/null 2>&1
>> -rm -rf testSubDir > /dev/null 2>&1
>> -mkdir testSubDir
>> +curdir=`pwd`
>> +testSubDir=testSubDir$$
>> +chmod -R a+rwx $testSubDir > /dev/null 2>&1
>
> Ditto.
>
>> +rm -rf $testSubDir > /dev/null 2>&1
>> +mkdir $testSubDir
>> +
>> +trap 'exit_status=$?
>> +  cd "$curdir"
>> +  case $exit_status,$keep_testdirs in
>> +  0,)
>> +    chmod -R a+rwx $testSubDir > /dev/null 2>&1
>> +    rm -rf "$testSubDir" ;;
>> +  esac
>> +  test "$signal" != 0 &&
>> +    echo "$as_me: caught signal $signal"
>> +  echo "$as_me: exit $exit_status"
>> +  exit $exit_status
>> +' 0
>> +for signal in 1 2 13 15; do
>> +  trap 'signal='$signal'; { (exit 1); exit 1; }' $signal
>> +done
>> +signal=0
>
> That's great, I'm going to use this in my own testsuites!

It's not very much tested in this circumstance yet, so beware.

Cheers,
Ralf