[PATCH 0/8] Fix valgrind tests.
Since we switched to using parallel tests, our valgrind tests would definitely have found leaks in the /usr/bin/env program if it had any. Not so much in some of our own programs. Let's fix that. Rich.
Since we enabled parallel tests, you can no longer run tests under valgrind merely by doing: TESTS_ENVIRONMENT = $(top_builddir)/run --test $(VG) check-valgrind: $(MAKE) check VG="@VG@" The reason is that the parallel tests framework doesn't run ``$(TESTS_ENVIRONMENT) <test>''. It inserts some other processes in between the environment and the test, so you end up valgrinding some unrelated process (currently ``env''). --- run.in | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/run.in b/run.in index ed4109c..c911834 100755 --- a/run.in +++ b/run.in @@ -33,13 +33,10 @@ # # The script should also be used for tests like this: # -# TESTS_ENVIRONMENT = ... $(top_builddir)/run --test [$(VG)] +# TESTS_ENVIRONMENT = ... $(top_builddir)/run --test # # The --test parameter introduces a timeout, stopping tests from # running forever. -# -# Use the optional $(VG) when the tests must also be run under -# valgrind. #---------------------------------------------------------------------- -- 2.5.0
--- docs/guestfs-hacking.pod | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/docs/guestfs-hacking.pod b/docs/guestfs-hacking.pod index c357717..419a4c2 100644 --- a/docs/guestfs-hacking.pod +++ b/docs/guestfs-hacking.pod @@ -274,8 +274,7 @@ the automake documentation for details. Runs a subset of the test suite under valgrind. -Any F<Makefile.am> in the tree that has a C<check-valgrind:> target -will be run by this rule. +See L</VALGRIND> below. =item C<make check-valgrind-local-guests> @@ -383,6 +382,35 @@ Do: =back +=head2 VALGRIND + +When you do C<make check-valgrind>, it searches for any F<Makefile.am> +in the tree that has a C<check-valgrind:> target and runs it. + +Writing the F<Makefile.am> and tests correctly to use valgrind and +working with automake parallel tests is subtle. + +If your tests are run via a shell script wrapper, then in the wrapper +use: + + $VG virt-foo + +and in the F<Makefile.am> use: + + check-valgrind: + make VG="$(top_builddir)/run @VG@" check + +However, if your binaries run directly from the C<TESTS> rule, you +have to modify the F<Makefile.am> like this: + + LOG_COMPILER = $(VG) + + check-valgrind: + make VG="@VG@" check + +In either case, check that the right program is being tested by +examining the F<tmp/valgrind*> log files carefully. + =head1 SUBMITTING PATCHES Submit patches to the mailing list: -- 2.5.0
--- tests/daemon/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/daemon/Makefile.am b/tests/daemon/Makefile.am index bb380c5..053cad3 100644 --- a/tests/daemon/Makefile.am +++ b/tests/daemon/Makefile.am @@ -27,7 +27,7 @@ TESTS = \ test-daemon-start.pl \ test-btrfs.pl -TESTS_ENVIRONMENT = $(top_builddir)/run --test $(VG) +TESTS_ENVIRONMENT = $(top_builddir)/run --test EXTRA_DIST = \ captive-daemon.pm \ -- 2.5.0
No check-valgrind is used in this subdirectory anyway. --- lua/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lua/Makefile.am b/lua/Makefile.am index 4c01e80..264f170 100644 --- a/lua/Makefile.am +++ b/lua/Makefile.am @@ -64,7 +64,7 @@ guestfs.so: libluaguestfs.la ln -sf .libs/libluaguestfs.so $@ # Tests. -TESTS_ENVIRONMENT = LC_ALL=C $(top_builddir)/run --test $(VG) +TESTS_ENVIRONMENT = LC_ALL=C $(top_builddir)/run --test TESTS = \ run-bindtests \ tests/010-load.lua \ -- 2.5.0
--- ocaml/Makefile.am | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ocaml/Makefile.am b/ocaml/Makefile.am index bc1d130..2152312 100644 --- a/ocaml/Makefile.am +++ b/ocaml/Makefile.am @@ -105,7 +105,8 @@ clean-local: endif -TESTS_ENVIRONMENT = $(top_builddir)/run --test $(VG) +TESTS_ENVIRONMENT = $(top_builddir)/run --test +LOG_COMPILER = $(VG) test_progs_bc = \ t/guestfs_010_load.bc \ @@ -164,7 +165,7 @@ if HAVE_OCAMLOPT endif check-valgrind: - $(MAKE) VG="$(top_builddir)/run @VG@" TESTS="$(test_progs_all)" check + $(MAKE) VG="@VG@" TESTS="$(test_progs_all)" check CLEANFILES += bindtests.tmp -- 2.5.0
--- src/Makefile.am | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Makefile.am b/src/Makefile.am index 25c6fa3..3ebb7f5 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -253,7 +253,8 @@ endif # internal tests of utility functions. Note we don't just test what's # in utils.c, we can test other functions as well here. -TESTS_ENVIRONMENT = $(top_builddir)/run --test $(VG) +TESTS_ENVIRONMENT = $(top_builddir)/run --test +LOG_COMPILER = $(VG) TESTS = test-utils check_PROGRAMS = test-utils -- 2.5.0
--- tests/c-api/Makefile.am | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/c-api/Makefile.am b/tests/c-api/Makefile.am index 6503d94..40fe33e 100644 --- a/tests/c-api/Makefile.am +++ b/tests/c-api/Makefile.am @@ -80,7 +80,8 @@ TESTS_ENVIRONMENT = \ SKIP_TEST_COMMAND=$(shell ldd test-command | grep -sq 'not a dynamic executable' || echo 1) \ SKIP_TEST_COMMAND_LINES=$(shell ldd test-command | grep -sq 'not a dynamic executable' || echo 1) \ SKIP_TEST_COMMAND=$(shell ldd test-pwd | grep -sq 'not a dynamic executable' || echo 1) \ - $(top_builddir)/run --test $(VG) + $(top_builddir)/run --test +LOG_COMPILER = $(VG) #SKIP_TEST_CHECKSUM_8=$(shell if test `find ../initramfs -name squashfs.ko | wc -l` -eq 0; then echo 1; fi) @@ -258,4 +259,4 @@ test_add_libvirt_dom_LDADD = \ endif check-valgrind: - $(MAKE) VG="$(top_builddir)/run @VG@" check + $(MAKE) VG="@VG@" check -- 2.5.0
--- tests/mount-local/Makefile.am | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/mount-local/Makefile.am b/tests/mount-local/Makefile.am index eac2cc8..b3c86cf 100644 --- a/tests/mount-local/Makefile.am +++ b/tests/mount-local/Makefile.am @@ -21,7 +21,8 @@ if HAVE_FUSE TESTS = test-parallel-mount-local -TESTS_ENVIRONMENT = $(top_builddir)/run --test $(VG) +TESTS_ENVIRONMENT = $(top_builddir)/run --test +LOG_COMPILER = $(VG) check_PROGRAMS = $(TESTS) @@ -48,6 +49,6 @@ test_parallel_mount_local_LDADD = \ $(top_builddir)/gnulib/lib/libgnu.la check-valgrind: - $(MAKE) VG="$(top_builddir)/run @VG@" check + $(MAKE) VG="@VG@" check endif -- 2.5.0
On Monday 22 February 2016 13:59:11 Richard W.M. Jones wrote:
Since we switched to using parallel tests, our valgrind tests would definitely have found leaks in the /usr/bin/env program if it had any. Not so much in some of our own programs. Let's fix that.
The series LGTM, but do they make sense as separate patches? Most probably a single one conveying the change across directories would be better (even from a debugging/bisecting PoV). Thanks, -- Pino Toscano
On Mon, Feb 22, 2016 at 06:49:47PM +0100, Pino Toscano wrote:
On Monday 22 February 2016 13:59:11 Richard W.M. Jones wrote:
Since we switched to using parallel tests, our valgrind tests would definitely have found leaks in the /usr/bin/env program if it had any. Not so much in some of our own programs. Let's fix that.
The series LGTM, but do they make sense as separate patches? Most probably a single one conveying the change across directories would be better (even from a debugging/bisecting PoV).
Yup, I'll squash 'em a bit. Thanks, Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
participants (2)
-
Pino Toscano -
Richard W.M. Jones