Re: [Ping PATCHES] {master} Optimize tests `instspc-*.test' for speed.

[Ralf Wildenhues]

Hi Stefano,

a while ago ...

* Stefano Lattarini wrote on Tue, Jan 25, 2011 at 06:38:50PM CET:
>   <http://lists.gnu.org/archive/html/automake-patches/2010-11/msg00152.html>
>   <http://lists.gnu.org/archive/html/automake-patches/2010-12/msg00006.html>

> OK for master?

Both are OK, with nits addressed.

Thanks,
Ralf

> Subject: [PATCH 1/2] tests: optimize `instspc-*.test' for speed
> 
> After the split of `instspc.test' into various generated tests,
> the running time of the testsuite has noticeably increased, since
> all these new generated tests must run aclocal, autoconf and
> automake, whereas previously they were run only once (at the
> beginning of `instspc.test').  But luckily, since the new tests
> share the same input files for the autotools, this situation can
> be easily worked around (at the expenses of a slight increase of
> complexity for the testsuite scaffolding).
> 
> * tests/instspc-data.test: New helper test, properly calling
> the `instspc-tests.sh' script to generate input data for the
> others `instspc-*.test' tests.
> * tests/Makefile.am (TESTS): Add `instspc-data.test'.
> ($(instspc_tests:.test=.log)): Depend on its log file.
> (instspc-data.log): Depend on `instspc-tests.sh'.
> * tests/instspc-tests.sh: Recognize new action `generate-data',
> and use it to create hand-written and autotools-generated static
> files shared by all the `instspc-*.test' tests.
> When sourced by the `instspc-*.test' tests, use those previously
> created files instead of recreating them from scratch.
> (deindent, create_input_data): New subroutines.
> Some other related changes and refactorings.

> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -85,8 +85,11 @@ $(instspc_tests): Makefile.am
>         } > address@hidden
>       $(AM_V_at)chmod a+rx address@hidden && mv -f address@hidden $@
>  
> -# All instspc*.test tests work by sourcing the instspc-tests.sh script.
> -$(instspc_tests:.test=.log): instspc-tests.sh
> +# All instspc-*.test tests work by sourcing the `instspc-tests.sh'
> +# script.  Also, they all use shared data generated by the helper
> +# test `instspc-data.test', for reasons of speed.
> +instspc-data.log: instspc-tests.sh
> +$(instspc_tests:.test=.log): instspc-tests.sh instspc-data.log
>  
>  MAINTAINERCLEANFILES += $(instspc_tests)
>  EXTRA_DIST += instspc-tests.sh
> @@ -504,6 +507,7 @@ instman2.test \
>  instmany.test \
>  instmany-mans.test \
>  instmany-python.test \
> +instspc-data.test \
>  $(instspc_tests) \
>  interp.test \
>  interp2.test \

> --- /dev/null
> +++ b/tests/instspc-data.test

> +# Helper testcase which generate input data for the other test
> +# `instspc-*.test'.  It basically delegates the work to the helper
> +# script `instspc-test.sh'.

As an alternative to a helper testcase, this could also just be a helper
script whose run is a prerequisite to the instspc*.log files.  That way
you don't have a bogus test result.  E.g.:

  $(instspc_tests:.test=.log): instspc-tests.sh instspc-data.dir/.dirstamp
  instspc-data.dir/.dirstamp:
        srcdir=$(srcdir) $(SHELL) $(srcdir)/tests/instspc-setup

or so (with the script renamed to instspc-setup).  And mostlyclean-local
removing instspc-data.dir.

You decide whether this is worthwhile.

> +# Ensure proper definition of $testsrcdir.
> +. ./defs-static || exit 99
> +test -n "$testsrcdir" || exit 99 # sanity check
> +
> +instspc_action=generate-data
> +. $testsrcdir/instspc-tests.sh

> --- a/tests/instspc-tests.sh
> +++ b/tests/instspc-tests.sh

> @@ -94,6 +99,91 @@ define_problematic_string ()
>    esac
>  }
>  
> +# Helper subroutines for creation of input data files.
> +
> +deindent ()

"unindent"?  Hmm.  Both sound weird, but maybe unindent is easier to
read.  Might wanna have it in tests/defs?

> +{
> +  sed 's/^ *//' # we don't strip leading tabs -- this is deliberate!
> +}
> +
> +create_input_data ()
> +{
> +  mkdir sub
> +
> +  deindent > configure.in << 'EOF'
> +    AC_INIT([instspc], [1.0])
> +    AM_INIT_AUTOMAKE
> +    AC_CONFIG_FILES([Makefile])
> +    AC_PROG_CC
> +    AC_PROG_RANLIB
> +    AC_OUTPUT
> +EOF
> +
[...]

> +  $ACLOCAL
> +  $AUTOCONF
> +  $AUTOMAKE -a
> +
> +  : > success

This could then be .dirstamp instead.

> +}
> +
>  # Be sure to avoid interferences from the environment.
>  instspc_names_list=''
>  instspc_xfail_builds_list=''
> @@ -189,102 +279,54 @@ if test x"$instspc_action" = x"generate-makefile"; then
>    exit 0
>  fi
>  
> -###  If we are still here, we have to run a test ...
> -
> -# We'll need the full setup provided by `tests/defs'.  Temporarly disable
> +# We'll need the full setup provided by `tests/defs'.  Temporarily disable
>  # the errexit flag, since the setup code might not be prepared to deal
>  # with it.
>  set +e
>  . ./defs || Exit 99
>  set -e
>  
> +# The directory set up by the `generate-data' action should contain all
> +# the files we need.  So remove the other files created by ./defs.  And
> +# check we really are in a temporary `*.dir' directory in the build tree,
> +# since the last thing we want is to remove some random user files!
> +test -f ../defs-static && test -f ../defs || Exit 99
> +case `pwd` in *.dir);; *) Exit 99;; esac
> +rm -f *
> +
> +if test x"$instspc_action" = x"generate-data"; then
> +  # We must *not* remove the test directory, since its contents must be
> +  # used by following dependent tests.
> +  trap 'Exit $?' 0

trap '' 0
is the portable way to undo the EXIT trap, if I remember correctly.
You could also just set keep_testdirs, no?

> +  create_input_data
> +  Exit 0
> +fi

> Subject: [PATCH 2/2] tests: `instspc-*.test': do not create useless source 
> file
> 
> * tests/instspc-tests.sh (create_input_data): Do not create
> unused source file `source2.c'.