GNOME Bugzilla – Bug 606720
Add code coverage support
Last modified: 2012-09-02 16:04:43 UTC
Add gcov (or bcov?) support in a macro which can be used by modules to avoid copying lots of boilerplate code. See: * http://live.gnome.org/GnomeGoals/AddCodeCoverage * http://mail.gnome.org/archives/desktop-devel-list/2010-January/msg00001.html
Patch?
I don't have one, but before someone writes a patch we need to decide between gcov and bcov, and also what things should go into the generated report.
If you're considering bcov, I think you should take a look at kcov as well. It's based on the same principle (in fact, started as a fork of bcov), but generates output data directly (i.e., no post-processing step) and supports more features than bcov (testing coverage in shared libraries, pruning uninteresting files etc). It can also generate cobertura-compatible output for integration into Jenkins. http://simonkagstrom.github.com/kcov/index.html And yes, there is a disclaimer: I'm the kcov developer :-)
Created attachment 219845 [details] [review] Add a code coverage macro to gnome-common I guess comment #3 spurred me on to create a patch, but it’s using lcov and it’s only really a proof of concept (though it works for GLib and libgdata, which are the two modules I’ve ported so far). It should be written so that it’s not specific to any particular code coverage tool, though, so if we decide we want to go with kcov then only gnome-common needs modifying.
An improvement to this would be to put the current git hash into the lcov report title.
The .make file should probably also be copied into the root directory of tarballs when `make dist` is run.
Created attachment 221940 [details] [review] Add a code coverage macro to gnome-common (updated_ Updated version which doesn’t use an external Makefile (which would have caused problems with `make dist`). Instead, the Makefile rules are now defined as GNOME_CODE_COVERAGE_RULES, and they should be substituted into projects’ Makefile.ams. The only other real change is to use $(PACKAGE_NAME)-$(PACKAGE_VERSION) everywhere instead of just $(PACKAGE_NAME). By using git-version-gen, projects can include git hashes in the titles and naming of their code coverage reports, to allow easier comparison of two sets of code coverage.
Review of attachment 221940 [details] [review]: I don't see any major problems, but to be really clear: I've never tried gcov/lcov myself, so this is just a quick review of the Makefile. Did you write this from scratch? If you didn't, can you list where you got the original code, with copyright/attribution? Which projects have you tried modifying to use this? Matthias should probably look at this, since he's played the most with code coverage for GLib. ::: macros2/gnome-code-coverage.m4 @@ +27,3 @@ + AC_MSG_RESULT($enable_code_coverage) + + if test "x$enable_code_coverage" = "xyes"; then Needs to use AS_IF(). See http://git.gnome.org/browse/glib/commit/?id=ddfcfa66ae602c11ce9c4bfc426a79d668653278 @@ +40,3 @@ + + if test "$gcc_ccache" = "yes" && (test -z "$CCACHE_DISABLE" || test "$CCACHE_DISABLE" != "1"); then + AC_MSG_ERROR([ccache must be disabled when the --enable-code-coverage option is used. You can disable ccache by setting the environment variable CCACHE_DISABLE=1.]) Interesting. Why is that? Seems like it'd have to be a ccache bug. Note this would impact Fedora/RHEL because the developer package set comes with ccache. @@ +114,3 @@ + +# Use recursive makes in order to ignore errors during check +code-coverage: Can you call this "check-code-coverage" so it's clearer it's running "make check" in a special mode?
Created attachment 222084 [details] [review] Add a code coverage macro to gnome-common (updated again) (In reply to comment #8) > Did you write this from scratch? If you didn't, can you list where you got the > original code, with copyright/attribution? No, I pinched most of it from GLib. Sorry, I should’ve said. The licencing (LGPLv2.1+ → GPLv3) is OK. Grepping through `git log -p` for ‘lcov’ gives the following as the people who’ve touched it: • Simon McVittie • Stef Walter • Matthias Clasen • Patrick Hulin (original author) > Which projects have you tried modifying to use this? Matthias should probably > look at this, since he's played the most with code coverage for GLib. GLib, libgdata, gcr, totem-pl-parser. > ::: macros2/gnome-code-coverage.m4 > @@ +27,3 @@ > + AC_MSG_RESULT($enable_code_coverage) > + > + if test "x$enable_code_coverage" = "xyes"; then > > Needs to use AS_IF(). See > http://git.gnome.org/browse/glib/commit/?id=ddfcfa66ae602c11ce9c4bfc426a79d668653278 Fixed. > @@ +40,3 @@ > + > + if test "$gcc_ccache" = "yes" && (test -z "$CCACHE_DISABLE" || test > "$CCACHE_DISABLE" != "1"); then > + AC_MSG_ERROR([ccache must be disabled when the > --enable-code-coverage option is used. You can disable ccache by setting the > environment variable CCACHE_DISABLE=1.]) > > Interesting. Why is that? Seems like it'd have to be a ccache bug. The check was in the original build tooling I copied from GLib, and I decided to leave it in. Stupidly, I didn’t check why it was there in the first place. Apparently it was a well-known bug about 5 years ago that ccache didn’t work with -fprofile-arcs and friends: https://bugzilla.redhat.com/show_bug.cgi?id=231462. It’s been fixed for a long time, though, so I’ve removed the check from the macro. > @@ +114,3 @@ > + > +# Use recursive makes in order to ignore errors during check > +code-coverage: > > Can you call this "check-code-coverage" so it's clearer it's running "make > check" in a special mode? Makes sense. Done.
(In reply to comment #9) > Created an attachment (id=222084) [details] [review] > Add a code coverage macro to gnome-common (updated again) > > (In reply to comment #8) > > Did you write this from scratch? If you didn't, can you list where you got the > > original code, with copyright/attribution? > > No, I pinched most of it from GLib. Sorry, I should’ve said. The licencing > (LGPLv2.1+ → GPLv3) is OK. Ok, just add this as a comment then? Like "Derived from glib/Makefile.decl". If you want to add a license header that's probably a good idea. Other than than seems fine to me. I'm interested in trying it.
Review of attachment 222084 [details] [review]: Marking reviewed based on previous comment
Created attachment 223110 [details] [review] Add a code coverage macro to gnome-common (updated again) Updated to specify the licence in the file header.
Review of attachment 223110 [details] [review]: Looks good, thanks!
Comment on attachment 223110 [details] [review] Add a code coverage macro to gnome-common (updated again) commit 493d55921f26ac3a9a3b7cc33756c88daace329e Author: Philip Withnall <philip@tecnocode.co.uk> Date: Sun Jul 29 17:58:21 2012 +0200 Bug 606720 — Add code coverage support Add a GNOME_CODE_COVERAGE m4 macro to allow projects to easily add code coverage support using lcov. This is heavily based on the code coverage tooling from GLib (LGPLv2.1+), originally written by Patrick Hulin and modified by Matthias Clasen, Stef Walter and Simon McVittie since. Closes: https://bugzilla.gnome.org/show_bug.cgi?id=606720
I’ll file some bugs against other modules to port them to using the new macro soon.
Bugs filed for: • libgdata: bug #683209 • totem-pl-parser: bug #683212 • gcr: bug #683211 Take a look at them for examples of porting applications.