[libvirt] [PATCH 0/2] add VIR_TEST_EXPENSIVE for faster 'make check' by default

Based on discussion of Peter's recent test additions, and whether we should sleep waiting for a timeout by default. I did _not_ wire up the use of VIR_TEST_EXPENSIVE as a way to bypass Peter's sleep test, although I can help with that as needed (although in Peter's case, it may be simpler to just getenv("VIR_TEST_EXPENSIVE") inside the C code, instead of trying to hack tests/Makefile.am to be conditional). Tested with 'make distcheck' in both in-tree and VPATH builds (which also the configure defaults when unpacking a tarball), and in setups where I explicitly used --enable-gnulib-tests or --disable-gnulib-tests, vs. running 'make check VIR_TEST_EXPENSIVE=1'; in all cases, the gnulib tests were skipped only when expected. Patch 1/2 is something I noticed on the side (basically side work from my first approach to compile out the directory at configure time, but that approach failed 'make check'); patch 2/2 can be applied independently, and this is only a series because both patches touch configure. I realized (only now that I am sending this patch) that my patch is one-way: you can turn on expensive tests (either at configure time or at runtime) but can't turn them off. Automake was able to have 'make V=0' and 'make V=1' override configure defaults in both directions by using nested make expansion, so if it is deemed necessary, I could respin to try that approach instead of the approach I used here, where an explicit VIR_TEST_EXPENSIVE=0 would then disable the tests regardless of the choice made during configure. On the other hand, having only a one-way road for turning on tests, with no counterpart for turning them off, seems adequate. I'm also open to bikeshedding, such as whether the configure options should be named --enable-expensive-tests instead of --enable-gnulib-tests, on the grounds that we might want to make tests/Makefile.am conditional after all. Eric Blake (2): maint: use modern autoconf idioms build: add configure option to disable gnulib tests HACKING | 9 ++++++++ autobuild.sh | 3 +++ bootstrap.conf | 3 ++- configure.ac | 60 ++++++++++++++++++++++++++++++++---------------- docs/hacking.html.in | 13 +++++++++++ gnulib/tests/Makefile.am | 13 ++++++++++- libvirt.spec.in | 1 + mingw-libvirt.spec.in | 4 ++-- 8 files changed, 82 insertions(+), 24 deletions(-) -- 1.8.3.1

Autoconf 2.59 says that AC_OUTPUT with arguments is obsolete, and we are already using the replacement for some, but not all, of our output files. * configure.ac (AC_OUTPUT): Rewrite to use AC_CONFIG_FILES. Signed-off-by: Eric Blake <eblake@redhat.com> --- configure.ac | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/configure.ac b/configure.ac index 084d864..a155790 100644 --- a/configure.ac +++ b/configure.ac @@ -2440,26 +2440,28 @@ AC_DEFINE_UNQUOTED([base64_encode_alloc],[libvirt_gl_base64_encode_alloc],[Hack AC_CONFIG_FILES([run], [chmod +x,-w run]) -AC_OUTPUT(Makefile src/Makefile include/Makefile docs/Makefile \ - docs/schemas/Makefile \ - gnulib/lib/Makefile \ - gnulib/tests/Makefile \ - libvirt.pc libvirt.spec mingw-libvirt.spec \ - po/Makefile.in \ - include/libvirt/Makefile include/libvirt/libvirt.h \ - python/Makefile \ - daemon/Makefile \ - tools/Makefile \ - tests/Makefile \ - examples/apparmor/Makefile \ - examples/domain-events/events-c/Makefile \ - examples/domsuspend/Makefile \ - examples/dominfo/Makefile \ - examples/openauth/Makefile \ - examples/python/Makefile \ - examples/hellolibvirt/Makefile \ - examples/systemtap/Makefile \ - examples/xml/nwfilter/Makefile) +AC_CONFIG_FILES([\ + Makefile src/Makefile include/Makefile docs/Makefile \ + docs/schemas/Makefile \ + gnulib/lib/Makefile \ + gnulib/tests/Makefile \ + libvirt.pc libvirt.spec mingw-libvirt.spec \ + po/Makefile.in \ + include/libvirt/Makefile include/libvirt/libvirt.h \ + python/Makefile \ + daemon/Makefile \ + tools/Makefile \ + tests/Makefile \ + examples/apparmor/Makefile \ + examples/domain-events/events-c/Makefile \ + examples/domsuspend/Makefile \ + examples/dominfo/Makefile \ + examples/openauth/Makefile \ + examples/python/Makefile \ + examples/hellolibvirt/Makefile \ + examples/systemtap/Makefile \ + examples/xml/nwfilter/Makefile]) +AC_OUTPUT AC_MSG_NOTICE([]) AC_MSG_NOTICE([Configuration summary]) -- 1.8.3.1

On 08/01/13 00:48, Eric Blake wrote:
Autoconf 2.59 says that AC_OUTPUT with arguments is obsolete, and we are already using the replacement for some, but not all, of our output files.
* configure.ac (AC_OUTPUT): Rewrite to use AC_CONFIG_FILES.
Signed-off-by: Eric Blake <eblake@redhat.com> --- configure.ac | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-)
I'm not an autotools expert but this construction works and contains all entries from the previous construction. ACK. Peter

On 08/01/2013 04:40 AM, Peter Krempa wrote:
On 08/01/13 00:48, Eric Blake wrote:
Autoconf 2.59 says that AC_OUTPUT with arguments is obsolete, and we are already using the replacement for some, but not all, of our output files.
* configure.ac (AC_OUTPUT): Rewrite to use AC_CONFIG_FILES.
Signed-off-by: Eric Blake <eblake@redhat.com> --- configure.ac | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-)
I'm not an autotools expert but this construction works and contains all entries from the previous construction.
ACK.
Thanks; I've pushed this one. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

The gnulib testsuite is relatively stable - the only times it is likely to have a test change from pass to fail is on a gnulib submodule update or a major system change (such as moving from Fedora 18 to 19, or other large change to libc). While it is an important test for end users on arbitrary machines (to make sure that the portability glue works for their machine), it mostly wastes time for development testing (as most developers aren't making any of the major changes that would cause gnulib tests to alter behavior). Thus, it pays to make the tests optional at configure time, defaulting to off for development, on for tarballs, and where autobuilders can force the tests on. It also helps to make the configure choice have a runtime-override, by using 'make check VIR_TEST_EXPENSIVE=1'. Automake has some pretty hard-coded magic with regards to the TESTS variable; I had quite a job figuring out how to keep 'make distcheck' passing regardless of the configure option setting in use, while still disabling the tests at runtime when I did not configure them on and did not use the override variable. Thankfully, we require GNU make, which lets me hide some information from Automake's magic handling of TESTS. build: add configure option to disable gnulib tests * configure.ac (--enable-gnulib-tests): Add new enable switch. (WITH_GNULIB_TESTS): Set new witness. * gnulib/tests/Makefile.am (TESTS): Make tests conditional on oth configure settings and the VIR_TEST_EXPENSIVE variable. * autobuild.sh: Enable tests during autobuilds. * libvirt.spec.in (%configure): Likewise. * mingw-libvirt.spec.in (%mingw_configure): Likewise. * docs/hacking.html.in: Document the option. * HACKING: Regenerate. Signed-off-by: Eric Blake <eblake@redhat.com> --- HACKING | 9 +++++++++ autobuild.sh | 3 +++ bootstrap.conf | 3 ++- configure.ac | 18 ++++++++++++++++++ docs/hacking.html.in | 13 +++++++++++++ gnulib/tests/Makefile.am | 13 ++++++++++++- libvirt.spec.in | 1 + mingw-libvirt.spec.in | 4 ++-- 8 files changed, 60 insertions(+), 4 deletions(-) diff --git a/HACKING b/HACKING index 207b9ed..58ab5e0 100644 --- a/HACKING +++ b/HACKING @@ -107,6 +107,15 @@ and run the tests: Valgrind <http://valgrind.org/> is a test that checks for memory management issues, such as leaks or use of uninitialized variables. +Some tests are skipped by default in a development environment, based on the +time they take in comparison to the likelihood that those tests will turn up +problems during incremental builds. These tests default to being run when when +building from a tarball or with the configure option --enable-gnulib-tests; +you can also force a one-time run of these tests in a development environment +by using: + + make check VIR_TEST_EXPENSIVE=1 + If you encounter any failing tests, the VIR_TEST_DEBUG environment variable may provide extra information to debug the failures. Larger values of VIR_TEST_DEBUG may provide larger amounts of information: diff --git a/autobuild.sh b/autobuild.sh index 7da8cb5..d676380 100755 --- a/autobuild.sh +++ b/autobuild.sh @@ -18,6 +18,7 @@ cd build # Run with options not normally exercised by the rpm build, for # more complete code coverage. ../autogen.sh --prefix="$AUTOBUILD_INSTALL_ROOT" \ + --enable-gnulib-tests \ --enable-test-coverage \ --disable-nls \ --enable-werror \ @@ -76,6 +77,7 @@ if test -x /usr/bin/i686-w64-mingw32-gcc ; then --build=$(uname -m)-w64-linux \ --host=i686-w64-mingw32 \ --prefix="$AUTOBUILD_INSTALL_ROOT/i686-w64-mingw32/sys-root/mingw" \ + --enable-gnulib-tests \ --enable-werror \ --without-libvirtd \ --without-python @@ -96,6 +98,7 @@ if test -x /usr/bin/x86_64-w64-mingw32-gcc ; then --build=$(uname -m)-w64-linux \ --host=x86_64-w64-mingw32 \ --prefix="$AUTOBUILD_INSTALL_ROOT/x86_64-w64-mingw32/sys-root/mingw" \ + --enable-gnulib-tests \ --enable-werror \ --without-libvirtd \ --without-python diff --git a/bootstrap.conf b/bootstrap.conf index f166a53..a1d1f07 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -244,9 +244,10 @@ gnulib_extra_files=" bootstrap_epilogue() { # Change paths in gnulib/tests/gnulib.mk from "../../.." to "../..", + # and make tests conditional by changing "TESTS" to "GNULIB_TESTS", # then ensure that gnulib/tests/Makefile.in is up-to-date. m=gnulib/tests/gnulib.mk - sed 's,\.\./\.\./\.\.,../..,g' $m > $m-t + sed 's,\.\./\.\./\.\.,../..,g; s/^TESTS /GNULIB_TESTS /' $m > $m-t mv -f $m-t $m ${AUTOMAKE-automake} gnulib/tests/Makefile } diff --git a/configure.ac b/configure.ac index a155790..f61bb76 100644 --- a/configure.ac +++ b/configure.ac @@ -1987,6 +1987,24 @@ fi AC_MSG_RESULT([$withval]) AM_CONDITIONAL([WITH_TESTS], [test "$withval" = "yes"]) +AC_ARG_ENABLE([gnulib-tests], + [AC_HELP_STRING([--enable-gnulib-tests], + [enable tests of gnulib portability layer @<:@default=check@:>@])], + [case $enableval in + yes|no) ;; + check) + # gnulib doesn't change often enough to need developers running it + # on a regular basis; but releases should always test things + if test -d $srcdir/.git ; then + enableval=no + else + enableval=yes + fi ;; + *) AC_MSG_ERROR([bad value ${enableval} for enable-gnulib-tests option]) ;; + esac], [enableval=check]) +enable_gnulib_tests=$enableval +AM_CONDITIONAL([WITH_GNULIB_TESTS], [test "$enable_gnulib_tests" = "yes"]) + AC_ARG_ENABLE([test-coverage], AC_HELP_STRING([--enable-test-coverage], [turn on code coverage instrumentation @<:@default=no@:>@]), [case "${enableval}" in diff --git a/docs/hacking.html.in b/docs/hacking.html.in index 8120b19..3fda9f0 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -119,6 +119,18 @@ </p> <p> + Some tests are skipped by default in a development environment, + based on the time they take in comparison to the likelihood + that those tests will turn up problems during incremental builds. + These tests default to being run when when building from a + tarball or with the configure option --enable-gnulib-tests; you + can also force a one-time run of these tests in a development + environment by using: + </p> +<pre> + make check VIR_TEST_EXPENSIVE=1 +</pre> + <p> If you encounter any failing tests, the VIR_TEST_DEBUG environment variable may provide extra information to debug the failures. Larger values of VIR_TEST_DEBUG may provide @@ -136,6 +148,7 @@ <pre> ./qemuxml2xmltest </pre> + <p>There is also a <code>./run</code> script at the top level, to make it easier to run programs that have not yet been installed, as well as to wrap invocations of various tests diff --git a/gnulib/tests/Makefile.am b/gnulib/tests/Makefile.am index 6a2f51b..f49523b 100644 --- a/gnulib/tests/Makefile.am +++ b/gnulib/tests/Makefile.am @@ -1,4 +1,4 @@ -## Makefile for gnulib/lib -*-Makefile-*- +## Makefile for gnulib/lib ## Copyright (C) 2011, 2013 Red Hat, Inc. ## @@ -19,3 +19,14 @@ include gnulib.mk INCLUDES = $(GETTEXT_CPPFLAGS) + +if WITH_GNULIB_TESTS +## Automake is able to conditionally run tests based on configure, and +## unconditionally ship test-related files in the tarball, with this line +TESTS = $(GNULIB_TESTS) +else +## Use GNU make constructs to hide our actions from Automake, which lacks a +## way to conditionally define tests based on the current environment rather +## than the results of configure +$(eval TESTS=$(shell test $${VIR_TEST_EXPENSIVE+set} && echo $(GNULIB_TESTS))) +endif diff --git a/libvirt.spec.in b/libvirt.spec.in index fce7f91..eecdae1 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1408,6 +1408,7 @@ of recent versions of Linux (and other OSes). --with-qemu-user=%{qemu_user} \ --with-qemu-group=%{qemu_group} \ %{?enable_werror} \ + --enable-gnulib-tests \ %{init_scripts} make %{?_smp_mflags} gzip -9 ChangeLog diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in index aa39231..80e3bd6 100644 --- a/mingw-libvirt.spec.in +++ b/mingw-libvirt.spec.in @@ -156,8 +156,8 @@ autoreconf -if --without-parallels \ --without-netcf \ --without-audit \ - --without-dtrace - + --without-dtrace \ + --enable-gnulib-tests %mingw_make %{?_smp_mflags} -- 1.8.3.1

On 08/01/13 00:48, Eric Blake wrote:
The gnulib testsuite is relatively stable - the only times it is likely to have a test change from pass to fail is on a gnulib submodule update or a major system change (such as moving from Fedora 18 to 19, or other large change to libc). While it is an important test for end users on arbitrary machines (to make sure that the portability glue works for their machine), it mostly wastes time for development testing (as most developers aren't making any of the major changes that would cause gnulib tests to alter behavior). Thus, it pays to make the tests optional at configure time, defaulting to off for development, on for tarballs, and where autobuilders can force the tests on. It also helps to make the configure choice have a runtime-override, by using 'make check VIR_TEST_EXPENSIVE=1'.
Automake has some pretty hard-coded magic with regards to the TESTS variable; I had quite a job figuring out how to keep 'make distcheck' passing regardless of the configure option setting in use, while still disabling the tests at runtime when I did not configure them on and did not use the override variable. Thankfully, we require GNU make, which lets me hide some information from Automake's magic handling of TESTS.
build: add configure option to disable gnulib tests * configure.ac (--enable-gnulib-tests): Add new enable switch. (WITH_GNULIB_TESTS): Set new witness. * gnulib/tests/Makefile.am (TESTS): Make tests conditional on oth configure settings and the VIR_TEST_EXPENSIVE variable. * autobuild.sh: Enable tests during autobuilds. * libvirt.spec.in (%configure): Likewise. * mingw-libvirt.spec.in (%mingw_configure): Likewise. * docs/hacking.html.in: Document the option. * HACKING: Regenerate.
Signed-off-by: Eric Blake <eblake@redhat.com> --- HACKING | 9 +++++++++ autobuild.sh | 3 +++ bootstrap.conf | 3 ++- configure.ac | 18 ++++++++++++++++++ docs/hacking.html.in | 13 +++++++++++++ gnulib/tests/Makefile.am | 13 ++++++++++++- libvirt.spec.in | 1 + mingw-libvirt.spec.in | 4 ++-- 8 files changed, 60 insertions(+), 4 deletions(-)
...
diff --git a/docs/hacking.html.in b/docs/hacking.html.in index 8120b19..3fda9f0 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in
...
@@ -136,6 +148,7 @@ <pre> ./qemuxml2xmltest </pre> +
Stray newline addition? Or is the XSL transformation playing weird again?
<p>There is also a <code>./run</code> script at the top level, to make it easier to run programs that have not yet been installed, as well as to wrap invocations of various tests
When I configure libvirt with --enable-gnulib-tests then the VIR_TEST_EXPENSIVE doesn't get set and the expensive tests are not being run. Otherwise I really like this addition. I'll be sending some tests checking the VIR_TEST_EXPENSIVE env variable today. Peter

On 08/01/2013 07:10 AM, Peter Krempa wrote:
On 08/01/13 00:48, Eric Blake wrote:
The gnulib testsuite is relatively stable - the only times it is likely to have a test change from pass to fail is on a gnulib submodule update or a major system change (such as moving from Fedora 18 to 19, or other large change to libc). While it is an important test for end users on arbitrary machines (to make sure that the portability glue works for their machine), it mostly wastes time for development testing (as most developers aren't making any of the major changes that would cause gnulib tests to alter behavior). Thus, it pays to make the tests optional at configure time, defaulting to off for development, on for tarballs, and where autobuilders can force the tests on. It also helps to make the configure choice have a runtime-override, by using 'make check VIR_TEST_EXPENSIVE=1'.
@@ -136,6 +148,7 @@ <pre> ./qemuxml2xmltest </pre> +
Stray newline addition? Or is the XSL transformation playing weird again?
Stray newline; will fix in v2.
<p>There is also a <code>./run</code> script at the top level, to make it easier to run programs that have not yet been installed, as well as to wrap invocations of various tests
When I configure libvirt with --enable-gnulib-tests then the VIR_TEST_EXPENSIVE doesn't get set and the expensive tests are not being run.
Yeah, I've decided to send a v2, that tries to be more like automake's "V=0" or "V=1" overrides (the configure option sets a default, but the default is overridable in both directions), and with the option named closer to the override name. But here's what I used for testing this v1: ./configure; make -C gnulib/tests check - from a tarball, lots of tests run; from a git checkout, 0 tests run ./configure --enable-gnulib-tests; make -C gnulib/tests check - lots of tests run whether from a tarball or git checkout ./configure --disable-gnulib-tests; make -C gnulib/tests check - no tests run, whether from a tarball or git checkout in all situations where no tests run, make -C gnulib/tests check VIR_TEST_EXPENSIVE=1 - lots of tests run
Otherwise I really like this addition. I'll be sending some tests checking the VIR_TEST_EXPENSIVE env variable today.
Then I'd better hurry up and post my v2 :) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake
-
Peter Krempa