[libvirt] [PATCH 0/2] Remove --enable-test-coverage, promote 'make coverage'

This is my follow up to my message here: https://www.redhat.com/archives/libvir-list/2019-February/msg01213.html This removes our custom ./configure --enable-test-coverage and 'make cov' targets in favor of gnulib's 'make coverage' which IMO is much simpler to use. See the mail above for full explanation Patch #2 adds some hacking.html docs, see that for some details about gnulib's features here. Cole Robinson (2): configure: Remove --enable-test-coverage docs: hacking: Add 'Code coverage reports' section Makefile.am | 20 +++----------------- configure.ac | 18 ------------------ docs/hacking.html.in | 29 +++++++++++++++++++++++++++++ examples/Makefile.am | 2 +- src/Makefile.am | 3 +-- src/remote/Makefile.inc.am | 2 -- tests/Makefile.am | 2 -- tools/Makefile.am | 6 ------ 8 files changed, 34 insertions(+), 48 deletions(-) -- 2.20.1

We provide a custom configure option --enable-test-coverage and 'make cov' target to generate code coverage reports. However gnulib already provides a 'make coverage' which 'just works' and doesn't require a special configure option. This drops our custom implementation in favor of 'make coverage'. Reports are now output to cov/index.html Signed-off-by: Cole Robinson <crobinso@redhat.com> --- Makefile.am | 20 +++----------------- configure.ac | 18 ------------------ examples/Makefile.am | 2 +- src/Makefile.am | 3 +-- src/remote/Makefile.inc.am | 2 -- tests/Makefile.am | 2 -- tools/Makefile.am | 6 ------ 7 files changed, 5 insertions(+), 48 deletions(-) diff --git a/Makefile.am b/Makefile.am index 709064c6a6..3c06e2619a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -16,15 +16,15 @@ ## License along with this library. If not, see ## <http://www.gnu.org/licenses/>. -LCOV = lcov -GENHTML = genhtml - SUBDIRS = . gnulib/lib include/libvirt src tools docs gnulib/tests \ tests po examples XZ_OPT ?= -v -T0 export XZ_OPT +# have gnulib 'make coverage' output to 'cov' dir +COVERAGE_OUT = "cov" + ACLOCAL_AMFLAGS = -I m4 EXTRA_DIST = \ @@ -77,20 +77,6 @@ check-local: all tests check-access: @($(MAKE) $(AM_MAKEFLAGS) -C tests check-access) -cov: clean-cov - $(MKDIR_P) $(top_builddir)/coverage - $(LCOV) -c -o $(top_builddir)/coverage/libvirt.info.tmp \ - -d $(top_builddir)/src \ - -d $(top_builddir)/tests - $(LCOV) -r $(top_builddir)/coverage/libvirt.info.tmp \ - -o $(top_builddir)/coverage/libvirt.info - rm $(top_builddir)/coverage/libvirt.info.tmp - $(GENHTML) --show-details -t "libvirt" -o $(top_builddir)/coverage \ - --legend $(top_builddir)/coverage/libvirt.info - -clean-cov: - rm -rf $(top_builddir)/coverage - MAINTAINERCLEANFILES = .git-module-status dist-hook: gen-ChangeLog gen-AUTHORS diff --git a/configure.ac b/configure.ac index 197d9746b2..880a3a7e40 100644 --- a/configure.ac +++ b/configure.ac @@ -738,23 +738,6 @@ fi AC_SUBST([VIR_TEST_EXPENSIVE_DEFAULT]) AM_CONDITIONAL([WITH_EXPENSIVE_TESTS], [test $VIR_TEST_EXPENSIVE_DEFAULT = 1]) -LIBVIRT_ARG_ENABLE([TEST_COVERAGE], [turn on code coverage instrumentation], [no]) -case "$enable_test_coverage" in - yes|no) ;; - *) AC_MSG_ERROR([bad value ${enable_test_coverga} for test-coverage option]) ;; -esac - -if test "$enable_test_coverage" = yes; then - save_WARN_CFLAGS=$WARN_CFLAGS - WARN_CFLAGS= - gl_WARN_ADD([-fprofile-arcs]) - gl_WARN_ADD([-ftest-coverage]) - COVERAGE_FLAGS=$WARN_CFLAGS - AC_SUBST([COVERAGE_CFLAGS], [$COVERAGE_FLAGS]) - AC_SUBST([COVERAGE_LDFLAGS], [$COVERAGE_FLAGS]) - WARN_CFLAGS=$save_WARN_CFLAGS -fi - LIBVIRT_ARG_ENABLE([TEST_OOM], [memory allocation failure checking], [no]) case "$enable_test_oom" in yes|no) ;; @@ -1041,7 +1024,6 @@ LIBVIRT_WIN_RESULT_WINDRES AC_MSG_NOTICE([]) AC_MSG_NOTICE([Test suite]) AC_MSG_NOTICE([]) -AC_MSG_NOTICE([ Coverage: $enable_test_coverage]) AC_MSG_NOTICE([ Alloc OOM: $enable_test_oom]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Miscellaneous]) diff --git a/examples/Makefile.am b/examples/Makefile.am index e2ec6e7fba..b590a148ce 100644 --- a/examples/Makefile.am +++ b/examples/Makefile.am @@ -29,7 +29,7 @@ EXTRA_DIST = \ AM_CPPFLAGS = \ -I$(top_builddir)/include -I$(top_srcdir)/include -I$(top_srcdir) -LDADD = $(STATIC_BINARIES) $(WARN_CFLAGS) $(COVERAGE_LDFLAGS) \ +LDADD = $(STATIC_BINARIES) $(WARN_CFLAGS) \ $(top_builddir)/src/libvirt.la \ $(top_builddir)/src/libvirt-admin.la diff --git a/src/Makefile.am b/src/Makefile.am index 8c8dfe3dcf..a880937705 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -43,9 +43,8 @@ AM_CFLAGS = $(LIBXML_CFLAGS) \ $(WARN_CFLAGS) \ $(LOCK_CHECKING_CFLAGS) \ $(WIN32_EXTRA_CFLAGS) \ - $(COVERAGE_CFLAGS) + $(NULL) AM_LDFLAGS = $(DRIVER_MODULES_LDFLAGS) \ - $(COVERAGE_LDFLAGS) \ $(RELRO_LDFLAGS) \ $(NO_INDIRECT_LDFLAGS) \ $(CYGWIN_EXTRA_LDFLAGS) \ diff --git a/src/remote/Makefile.inc.am b/src/remote/Makefile.inc.am index 3d0ff29548..468a3f5d97 100644 --- a/src/remote/Makefile.inc.am +++ b/src/remote/Makefile.inc.am @@ -136,7 +136,6 @@ libvirtd_CFLAGS = \ $(LIBNL_CFLAGS) \ $(WARN_CFLAGS) \ $(PIE_CFLAGS) \ - $(COVERAGE_CFLAGS) \ -I$(srcdir)/access \ -I$(srcdir)/conf \ -I$(srcdir)/rpc \ @@ -145,7 +144,6 @@ libvirtd_CFLAGS = \ libvirtd_LDFLAGS = \ $(RELRO_LDFLAGS) \ $(PIE_LDFLAGS) \ - $(COVERAGE_LDFLAGS) \ $(NO_INDIRECT_LDFLAGS) \ $(NO_UNDEFINED_LDFLAGS) \ $(NULL) diff --git a/tests/Makefile.am b/tests/Makefile.am index a7f1b39a5e..c07a3866c6 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -47,7 +47,6 @@ AM_CFLAGS = \ $(SELINUX_CFLAGS) \ $(APPARMOR_CFLAGS) \ $(YAJL_CFLAGS) \ - $(COVERAGE_CFLAGS) \ $(XDR_CFLAGS) \ $(WARN_CFLAGS) @@ -275,7 +274,6 @@ endif WITH_SECDRIVER_SELINUX # This is a fake SSH we use from virnetsockettest ssh_SOURCES = ssh.c -ssh_LDADD = $(COVERAGE_LDFLAGS) if WITH_LIBXL test_programs += xlconfigtest xml2sexprtest sexpr2xmltest \ diff --git a/tools/Makefile.am b/tools/Makefile.am index f2f84f7852..95025ced43 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -27,7 +27,6 @@ WARN_CFLAGS += $(STRICT_FRAME_LIMIT_CFLAGS) AM_CFLAGS = \ $(WARN_CFLAGS) \ - $(COVERAGE_CFLAGS) \ $(PIE_CFLAGS) \ $(LIBXML_CFLAGS) \ $(NULL) @@ -137,7 +136,6 @@ libvirt_shell_la_CFLAGS = \ libvirt_shell_la_LDFLAGS = \ $(AM_LDFLAGS) \ $(PIE_LDFLAGS) \ - $(COVERAGE_LDFLAGS) \ $(NULL) libvirt_shell_la_LIBADD = \ ../src/libvirt.la \ @@ -183,7 +181,6 @@ endif ! WITH_BHYVE virt_host_validate_LDFLAGS = \ $(AM_LDFLAGS) \ $(PIE_LDFLAGS) \ - $(COVERAGE_LDFLAGS) \ $(NULL) virt_host_validate_LDADD = \ @@ -206,7 +203,6 @@ virt_login_shell_SOURCES = \ virt_login_shell_LDFLAGS = \ $(AM_LDFLAGS) \ $(PIE_LDFLAGS) \ - $(COVERAGE_LDFLAGS) \ $(NULL) virt_login_shell_LDADD = \ $(STATIC_BINARIES) \ @@ -239,7 +235,6 @@ virsh_SOURCES = \ virsh_LDFLAGS = \ $(AM_LDFLAGS) \ $(PIE_LDFLAGS) \ - $(COVERAGE_LDFLAGS) \ $(NULL) virsh_LDADD = \ $(STATIC_BINARIES) \ @@ -257,7 +252,6 @@ virt_admin_SOURCES = \ virt_admin_LDFLAGS = \ $(AM_LDFLAGS) \ - $(COVERAGE_LDFLAGS) \ $(STATIC_BINARIES) \ $(PIE_LDFLAGS) \ $(NULL) -- 2.20.1

On Wed, Mar 13, 2019 at 01:11:31PM -0400, Cole Robinson wrote:
We provide a custom configure option --enable-test-coverage and 'make cov' target to generate code coverage reports. However gnulib already provides a 'make coverage' which 'just works' and doesn't require a special configure option.
This drops our custom implementation in favor of 'make coverage'. Reports are now output to cov/index.html
Signed-off-by: Cole Robinson <crobinso@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

Signed-off-by: Cole Robinson <crobinso@redhat.com> --- docs/hacking.html.in | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/docs/hacking.html.in b/docs/hacking.html.in index f99d143b7b..56608fbc9e 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -1412,5 +1412,34 @@ int foo() in the same way, but still make sure they get reviewed if non-trivial. </li> </ul> + <h2><a id="coverage">Code coverage reports</a></h2> + + <p> + Code coverage HTML reports can be generated with: + </p> + +<pre> + make coverage +</pre> + + <p> + Reports will be generated in the <code>cov/</code> directory. Point a + web browser at <code>cov/index.html</code> for the full report. + </p> + + <p> + The <code>make coverage</code> target is provided by <code>gnulib</code>. + It is a convenience helper for calling the following 3 targets in order. + It may be useful to occasionally call these directly. + + <ul> + <li><code>make init-coverage</code>: run <code>make clean</code> and + remove all code coverage counter files (*.gcno, etc)</li> + <li><code>make build-coverage</code>: run <code>make</code> and + <code>make check</code> with <code>CFLAGS</code> filled in with + necessary coverage flags.</li> + <li><code>make gen-coverage</code>: generate the HTML report</li> + </ul> + </p> </body> </html> -- 2.20.1

On Wed, Mar 13, 2019 at 01:11:32PM -0400, Cole Robinson wrote:
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- docs/hacking.html.in | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
diff --git a/docs/hacking.html.in b/docs/hacking.html.in index f99d143b7b..56608fbc9e 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -1412,5 +1412,34 @@ int foo() in the same way, but still make sure they get reviewed if non-trivial. </li> </ul> + <h2><a id="coverage">Code coverage reports</a></h2> + + <p> + Code coverage HTML reports can be generated with: + </p> + +<pre> + make coverage +</pre> + + <p> + Reports will be generated in the <code>cov/</code> directory. Point a + web browser at <code>cov/index.html</code> for the full report. + </p> + + <p> + The <code>make coverage</code> target is provided by <code>gnulib</code>. + It is a convenience helper for calling the following 3 targets in order. + It may be useful to occasionally call these directly. + + <ul> + <li><code>make init-coverage</code>: run <code>make clean</code> and + remove all code coverage counter files (*.gcno, etc)</li>
s/etc/etc./
+ <li><code>make build-coverage</code>: run <code>make</code> and + <code>make check</code> with <code>CFLAGS</code> filled in with + necessary coverage flags.</li>
remove the full stop at the end so that it is the same for all three lines With the above fixed: Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
+ <li><code>make gen-coverage</code>: generate the HTML report</li> + </ul> + </p> </body> </html> -- 2.20.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (2)
-
Cole Robinson
-
Martin Kletzander