Thread: code coverage patch

I've attached a patch that allows the generation of code coverage
statistics. To test it, apply the patch, then:

autoconf
./configure --enable-coverage
make
make check (or execute any other application against the database to see
the coverage of that app)
make coverage
make coverage_out

You will find a coverage directory in your build directory. Open
coverage/index.html to see the code coverage.

There are a couple of issues.

gcov gets confused when source files are generated. I eliminated
src/backend/bootstrap and ../parser from coverage analysis to avoid
errors of this type. With a vpath build only, code coverage fails on
another generated .c file, src/backend/utils/misc/guc-file.c.

Also, two tests fail with the following diff when the build is
configured with --enable-coverage.
       RETURNS trigger
       AS
'/home/michelle/trunkClean/pgsql/src/test/regress/../../../contrib/spi/refint.so'
       LANGUAGE C;
+ ERROR:  could not load library
"/home/michelle/trunkClean/pgsql/src/test/regress/../../../contrib/spi/refint.so":
/home/michelle/trunkClean/pgsql/src/test/regress/../../../contrib/spi/refint.so:
undefined symbol: __gcov_merge_add
 CREATE FUNCTION check_foreign_key ()
       RETURNS trigger
       AS
'/home/michelle/trunkClean/pgsql/src/test/regress/../../../contrib/spi/refint.so'
       LANGUAGE C;
+ ERROR:  could not load library
"/home/michelle/trunkClean/pgsql/src/test/regress/../../../contrib/spi/refint.so":
/home/michelle/trunkClean/pgsql/src/test/regress/../../../contrib/spi/refint.so:
undefined symbol: __gcov_merge_add
 CREATE FUNCTION autoinc ()
       RETURNS trigger
       AS
'/home/michelle/trunkClean/pgsql/src/test/regress/../../../contrib/spi/autoinc.so'
       LANGUAGE C;
+ ERROR:  could not load library
"/home/michelle/trunkClean/pgsql/src/test/regress/../../../contrib/spi/autoinc.so":
/home/michelle/trunkClean/pgsql/src/test/regress/../../../contrib/spi/autoinc.so:
undefined symbol: __gcov_merge_add
 CREATE FUNCTION funny_dup17 ()
         RETURNS trigger
         AS '/home/michelle/trunkClean/pgsql/src/test/regress/regress.so'

Please test and comment.

-- Michelle

--
Michelle Caisse               Sun Microsystems
California, U.S.     http://sun.com/postgresql


Index: GNUmakefile.in
===================================================================
RCS file: /projects/cvsroot/pgsql/GNUmakefile.in,v
retrieving revision 1.47
diff -u -r1.47 GNUmakefile.in
--- GNUmakefile.in    18 Mar 2008 16:24:50 -0000    1.47
+++ GNUmakefile.in    27 Aug 2008 20:43:47 -0000
@@ -63,6 +63,25 @@

 ##########################################################################

+coverage: all
+    rm -rf "$(top_builddir)/coverage"
+    mkdir "$(top_builddir)/coverage"
+    $(MAKE) -C src/backend $@
+
+TRACEFILES = $(shell find src/backend -name lcov.info)
+
+coverage_out: coverage
+    $(GENHTML) --show-details --legend --output-directory=coverage --title=PostgreSQL --num-spaces=4 $(TRACEFILES)
+    cd coverage && cp index.html save.html && sed -e '/Date:/ i\
+          <td class="headerItem" width="20%">gcov output:</td> \
+          <td class="headerValue" width="80%" colspan=4><a href="gcov.out">gcov.out</a></td> \
+        </tr> \
+    <tr> ' index.html > tmp.html && mv tmp.html index.html
+    tar cf coverage.tar coverage
+
+
+##########################################################################
+
 distdir    = postgresql-$(VERSION)
 dummy    = =install=
 garbage = =*  "#"*  ."#"*  *~*  *.orig  *.rej  core  postgresql-*
Index: configure
===================================================================
RCS file: /projects/cvsroot/pgsql/configure,v
retrieving revision 1.602
diff -u -r1.602 configure
--- configure    21 Aug 2008 13:53:27 -0000    1.602
+++ configure    27 Aug 2008 20:44:18 -0000
@@ -672,6 +672,10 @@
 enable_rpath
 enable_debug
 enable_profiling
+GCOV
+LCOV
+GENHTML
+enable_coverage
 DTRACE
 DTRACEFLAGS
 enable_dtrace
@@ -1356,6 +1360,7 @@
   --disable-spinlocks     do not use spinlocks
   --enable-debug          build with debugging symbols (-g)
   --enable-profiling      build with profiling enabled
+  --enable-coverage       build with coverage enabled
   --enable-dtrace         build with DTrace support
   --enable-depend         turn on automatic dependency tracking
   --enable-cassert        enable assertion checks (for debugging)
@@ -2469,6 +2474,178 @@


 #
+# --enable-coverage enables generation of code coverage metrics with gcov
+#
+
+pgac_args="$pgac_args enable_coverage"
+
+# Check whether --enable-coverage was given.
+if test "${enable_coverage+set}" = set; then
+  enableval=$enable_coverage;
+  case $enableval in
+    yes)
+      :
+      ;;
+    no)
+      :
+      ;;
+    *)
+      { { echo "$as_me:$LINENO: error: no argument expected for --enable-coverage option" >&5
+echo "$as_me: error: no argument expected for --enable-coverage option" >&2;}
+   { (exit 1); exit 1; }; }
+      ;;
+  esac
+
+else
+  enable_coverage=no
+
+fi
+
+
+for ac_prog in gcov
+do
+  # Extract the first word of "$ac_prog", so it can be a program name with args.
+set dummy $ac_prog; ac_word=$2
+{ echo "$as_me:$LINENO: checking for $ac_word" >&5
+echo $ECHO_N "checking for $ac_word... $ECHO_C" >&6; }
+if test "${ac_cv_prog_GCOV+set}" = set; then
+  echo $ECHO_N "(cached) $ECHO_C" >&6
+else
+  if test -n "$GCOV"; then
+  ac_cv_prog_GCOV="$GCOV" # Let the user override the test.
+else
+as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
+for as_dir in $PATH
+do
+  IFS=$as_save_IFS
+  test -z "$as_dir" && as_dir=.
+  for ac_exec_ext in '' $ac_executable_extensions; do
+  if { test -f "$as_dir/$ac_word$ac_exec_ext" && $as_test_x "$as_dir/$ac_word$ac_exec_ext"; }; then
+    ac_cv_prog_GCOV="$ac_prog"
+    echo "$as_me:$LINENO: found $as_dir/$ac_word$ac_exec_ext" >&5
+    break 2
+  fi
+done
+done
+IFS=$as_save_IFS
+
+fi
+fi
+GCOV=$ac_cv_prog_GCOV
+if test -n "$GCOV"; then
+  { echo "$as_me:$LINENO: result: $GCOV" >&5
+echo "${ECHO_T}$GCOV" >&6; }
+else
+  { echo "$as_me:$LINENO: result: no" >&5
+echo "${ECHO_T}no" >&6; }
+fi
+
+
+  test -n "$GCOV" && break
+done
+
+if test -z "$GCOV"; then
+  { { echo "$as_me:$LINENO: error: gcov not found" >&5
+echo "$as_me: error: gcov not found" >&2;}
+   { (exit 1); exit 1; }; }
+fi
+for ac_prog in lcov
+do
+  # Extract the first word of "$ac_prog", so it can be a program name with args.
+set dummy $ac_prog; ac_word=$2
+{ echo "$as_me:$LINENO: checking for $ac_word" >&5
+echo $ECHO_N "checking for $ac_word... $ECHO_C" >&6; }
+if test "${ac_cv_prog_LCOV+set}" = set; then
+  echo $ECHO_N "(cached) $ECHO_C" >&6
+else
+  if test -n "$LCOV"; then
+  ac_cv_prog_LCOV="$LCOV" # Let the user override the test.
+else
+as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
+for as_dir in $PATH
+do
+  IFS=$as_save_IFS
+  test -z "$as_dir" && as_dir=.
+  for ac_exec_ext in '' $ac_executable_extensions; do
+  if { test -f "$as_dir/$ac_word$ac_exec_ext" && $as_test_x "$as_dir/$ac_word$ac_exec_ext"; }; then
+    ac_cv_prog_LCOV="$ac_prog"
+    echo "$as_me:$LINENO: found $as_dir/$ac_word$ac_exec_ext" >&5
+    break 2
+  fi
+done
+done
+IFS=$as_save_IFS
+
+fi
+fi
+LCOV=$ac_cv_prog_LCOV
+if test -n "$LCOV"; then
+  { echo "$as_me:$LINENO: result: $LCOV" >&5
+echo "${ECHO_T}$LCOV" >&6; }
+else
+  { echo "$as_me:$LINENO: result: no" >&5
+echo "${ECHO_T}no" >&6; }
+fi
+
+
+  test -n "$LCOV" && break
+done
+
+if test -z "$LCOV"; then
+  { { echo "$as_me:$LINENO: error: lcov not found" >&5
+echo "$as_me: error: lcov not found" >&2;}
+   { (exit 1); exit 1; }; }
+fi
+for ac_prog in genhtml
+do
+  # Extract the first word of "$ac_prog", so it can be a program name with args.
+set dummy $ac_prog; ac_word=$2
+{ echo "$as_me:$LINENO: checking for $ac_word" >&5
+echo $ECHO_N "checking for $ac_word... $ECHO_C" >&6; }
+if test "${ac_cv_prog_GENHTML+set}" = set; then
+  echo $ECHO_N "(cached) $ECHO_C" >&6
+else
+  if test -n "$GENHTML"; then
+  ac_cv_prog_GENHTML="$GENHTML" # Let the user override the test.
+else
+as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
+for as_dir in $PATH
+do
+  IFS=$as_save_IFS
+  test -z "$as_dir" && as_dir=.
+  for ac_exec_ext in '' $ac_executable_extensions; do
+  if { test -f "$as_dir/$ac_word$ac_exec_ext" && $as_test_x "$as_dir/$ac_word$ac_exec_ext"; }; then
+    ac_cv_prog_GENHTML="$ac_prog"
+    echo "$as_me:$LINENO: found $as_dir/$ac_word$ac_exec_ext" >&5
+    break 2
+  fi
+done
+done
+IFS=$as_save_IFS
+
+fi
+fi
+GENHTML=$ac_cv_prog_GENHTML
+if test -n "$GENHTML"; then
+  { echo "$as_me:$LINENO: result: $GENHTML" >&5
+echo "${ECHO_T}$GENHTML" >&6; }
+else
+  { echo "$as_me:$LINENO: result: no" >&5
+echo "${ECHO_T}no" >&6; }
+fi
+
+
+  test -n "$GENHTML" && break
+done
+
+if test -z "$GENHTML"; then
+  { { echo "$as_me:$LINENO: error: genhtml not found" >&5
+echo "$as_me: error: genhtml not found" >&2;}
+   { (exit 1); exit 1; }; }
+fi
+
+
+#
 # DTrace
 #

@@ -3961,6 +4138,17 @@
   CFLAGS="$CFLAGS -g"
 fi

+# enable code coverage if --enable-coverage
+if test "$enable_coverage" = yes; then
+  if test "$GCC" = yes; then
+    CFLAGS="$CFLAGS -fprofile-arcs -ftest-coverage"
+  else
+    { { echo "$as_me:$LINENO: error: --enable-coverage is supported only when using GCC" >&5
+echo "$as_me: error: --enable-coverage is supported only when using GCC" >&2;}
+   { (exit 1); exit 1; }; }
+  fi
+fi
+
 # enable profiling if --enable-profiling
 if test "$enable_profiling" = yes && test "$ac_cv_prog_cc_g" = yes; then
   if test "$GCC" = yes; then
@@ -26513,6 +26701,10 @@
 enable_rpath!$enable_rpath$ac_delim
 enable_debug!$enable_debug$ac_delim
 enable_profiling!$enable_profiling$ac_delim
+GCOV!$GCOV$ac_delim
+LCOV!$LCOV$ac_delim
+GENHTML!$GENHTML$ac_delim
+enable_coverage!$enable_coverage$ac_delim
 DTRACE!$DTRACE$ac_delim
 DTRACEFLAGS!$DTRACEFLAGS$ac_delim
 enable_dtrace!$enable_dtrace$ac_delim
@@ -26552,10 +26744,6 @@
 LD!$LD$ac_delim
 with_gnu_ld!$with_gnu_ld$ac_delim
 ld_R_works!$ld_R_works$ac_delim
-RANLIB!$RANLIB$ac_delim
-STRIP!$STRIP$ac_delim
-STRIP_STATIC_LIB!$STRIP_STATIC_LIB$ac_delim
-STRIP_SHARED_LIB!$STRIP_SHARED_LIB$ac_delim
 _ACEOF

   if test `sed -n "s/.*$ac_delim\$/X/p" conf$$subs.sed | grep -c X` = 97; then
@@ -26597,6 +26785,10 @@
 ac_delim='%!_!# '
 for ac_last_try in false false false false false :; do
   cat >conf$$subs.sed <<_ACEOF
+RANLIB!$RANLIB$ac_delim
+STRIP!$STRIP$ac_delim
+STRIP_STATIC_LIB!$STRIP_STATIC_LIB$ac_delim
+STRIP_SHARED_LIB!$STRIP_SHARED_LIB$ac_delim
 TAR!$TAR$ac_delim
 LN_S!$LN_S$ac_delim
 AWK!$AWK$ac_delim
@@ -26647,7 +26839,7 @@
 LTLIBOBJS!$LTLIBOBJS$ac_delim
 _ACEOF

-  if test `sed -n "s/.*$ac_delim\$/X/p" conf$$subs.sed | grep -c X` = 48; then
+  if test `sed -n "s/.*$ac_delim\$/X/p" conf$$subs.sed | grep -c X` = 52; then
     break
   elif $ac_last_try; then
     { { echo "$as_me:$LINENO: error: could not make $CONFIG_STATUS" >&5
Index: configure.in
===================================================================
RCS file: /projects/cvsroot/pgsql/configure.in,v
retrieving revision 1.564
diff -u -r1.564 configure.in
--- configure.in    19 Aug 2008 19:17:40 -0000    1.564
+++ configure.in    27 Aug 2008 20:44:21 -0000
@@ -204,6 +204,25 @@
 AC_SUBST(enable_profiling)

 #
+# --enable-coverage enables generation of code coverage metrics with gcov
+#
+PGAC_ARG_BOOL(enable, coverage, no,
+              [  --enable-coverage       build with coverage enabled])
+AC_CHECK_PROGS(GCOV, gcov)
+if test -z "$GCOV"; then
+  AC_MSG_ERROR([gcov not found])
+fi
+AC_CHECK_PROGS(LCOV, lcov)
+if test -z "$LCOV"; then
+  AC_MSG_ERROR([lcov not found])
+fi
+AC_CHECK_PROGS(GENHTML, genhtml)
+if test -z "$GENHTML"; then
+  AC_MSG_ERROR([genhtml not found])
+fi
+AC_SUBST(enable_coverage)
+
+#
 # DTrace
 #
 PGAC_ARG_BOOL(enable, dtrace, no,
@@ -415,6 +434,15 @@
   CFLAGS="$CFLAGS -g"
 fi

+# enable code coverage if --enable-coverage
+if test "$enable_coverage" = yes; then
+  if test "$GCC" = yes; then
+    CFLAGS="$CFLAGS -fprofile-arcs -ftest-coverage"
+  else
+    AC_MSG_ERROR([--enable-coverage is supported only when using GCC])
+  fi
+fi
+
 # enable profiling if --enable-profiling
 if test "$enable_profiling" = yes && test "$ac_cv_prog_cc_g" = yes; then
   if test "$GCC" = yes; then
Index: src/Makefile.global.in
===================================================================
RCS file: /projects/cvsroot/pgsql/src/Makefile.global.in,v
retrieving revision 1.241
diff -u -r1.241 Makefile.global.in
--- src/Makefile.global.in    17 Feb 2008 16:36:43 -0000    1.241
+++ src/Makefile.global.in    27 Aug 2008 20:44:23 -0000
@@ -18,7 +18,7 @@
 #
 # Meta configuration

-.PHONY: all install install-strip installdirs uninstall clean distclean maintainer-clean distprep check installcheck
maintainer-check
+.PHONY: all install install-strip installdirs uninstall clean distclean maintainer-clean distprep check installcheck
maintainer-checkcoverage 
 .SILENT: installdirs

 # make `all' the default target
@@ -291,6 +291,12 @@
 NSGMLS    = @NSGMLS@
 SGMLSPL    = @SGMLSPL@

+# Code coverage
+
+GCOV = @GCOV@
+LCOV = @LCOV@
+GENHTML = @GENHTML@
+
 # Feature settings

 DEF_PGPORT = @default_port@
Index: src/backend/common.mk
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/common.mk,v
retrieving revision 1.7
diff -u -r1.7 common.mk
--- src/backend/common.mk    17 Mar 2008 18:24:56 -0000    1.7
+++ src/backend/common.mk    27 Aug 2008 20:44:23 -0000
@@ -18,6 +18,10 @@

 SUBDIROBJS = $(SUBDIRS:%=%/$(subsysfilename))

+SRC = $(OBJS:.o=.c)
+GCDA = $(OBJS:.o=.gcda)
+GCNO = $(OBJS:.o=.gcno)
+
 # top-level backend directory obviously has its own "all" target
 ifneq ($(subdir), src/backend)
 all: $(subsysfilename)
@@ -46,3 +50,15 @@
     for dir in $(SUBDIRS); do $(MAKE) -C $$dir clean || exit; done
 endif
     rm -f $(subsysfilename) $(OBJS)
+ifeq ($(enable_coverage),yes)
+    rm -f $(GCDA) $(GCNO) lcov.info *.gcov
+endif
+
+coverage:
+ifdef SUBDIRS
+    for dir in $(SUBDIRS); do $(MAKE) -C $$dir coverage || exit; done
+endif
+ifneq (,$(SRC))
+    $(GCOV) -b -f -p -o . $(GCOVFLAGS) $(SRC) >>$(top_builddir)/coverage/gcov.out
+    $(LCOV) --directory . --capture --output-file lcov.info $(LCOVFLAGS)
+endif
Index: src/backend/bootstrap/Makefile
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/bootstrap/Makefile,v
retrieving revision 1.36
diff -u -r1.36 Makefile
--- src/backend/bootstrap/Makefile    19 Feb 2008 10:30:07 -0000    1.36
+++ src/backend/bootstrap/Makefile    27 Aug 2008 20:44:24 -0000
@@ -49,3 +49,9 @@
 clean:
 # And the garbage that might have been left behind by partial build:
     @rm -f y.tab.h y.tab.c y.output lex.yy.c
+
+# Override target in common.mk. gcov gets confused with #line
+# directives indicating source files that don't exist on disk.
+# FIXME
+coverage:
+    # do nothing
Index: src/backend/parser/Makefile
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/parser/Makefile,v
retrieving revision 1.46
diff -u -r1.46 Makefile
--- src/backend/parser/Makefile    19 Feb 2008 10:30:07 -0000    1.46
+++ src/backend/parser/Makefile    27 Aug 2008 20:44:24 -0000
@@ -61,3 +61,9 @@
 clean:
 # And the garbage that might have been left behind by partial build:
     @rm -f y.tab.h y.tab.c y.output lex.yy.c
+
+# Override target in common.mk. gcov gets confused with #line
+# directives indicating source files that don't exist on disk.
+# FIXME
+coverage:
+    # do nothing

Michelle Caisse wrote:
> gcov gets confused when source files are generated. I eliminated
> src/backend/bootstrap and ../parser from coverage analysis to avoid
> errors of this type.

The problem with those files is that the source file contains lines like this:

#line 1042 "y.tab.c"

but that source file does not exist, as it is renamed to gram.c.

We could fix that in one of two ways:

1) Use bison's -o option to put the output file in the right place directly, 
if we are dealing with bison (and don't bother to support code coverage 
analysis with other yaccs), or

2) Run a pattern replacement across the grammar output files as their are 
renamed.

Comments?

Michelle Caisse wrote:
> Also, two tests fail with the following diff when the build is
> configured with --enable-coverage.
>        RETURNS trigger
>        AS
> '/home/michelle/trunkClean/pgsql/src/test/regress/../../../contrib/spi/refi
>nt.so' LANGUAGE C;
> + ERROR:  could not load library
> "/home/michelle/trunkClean/pgsql/src/test/regress/../../../contrib/spi/refi
>nt.so":
> /home/michelle/trunkClean/pgsql/src/test/regress/../../../contrib/spi/refin
>t.so: undefined symbol: __gcov_merge_add

The reason for that problem is that the shared object needs to be linked 
with -fprofile-arcs -ftest-coverage.  (One of these causes -lgcov to be 
linked, which includes the missing symbol.)  This is not done because the 
shared object link rules don't use CFLAGS.

I think for most platforms it would actually be more correct to use CFLAGS in 
linking.  There may be the odd exception, but usually CFLAGS contains some 
assortment of -O, -g, -W, and maybe -f options, which should do more good 
than harm when linking.

Any concerns about selectively adding CFLAGS to shared library linking rules?

Peter Eisentraut <peter_e@gmx.net> writes:

> The reason for that problem is that the shared object needs to be linked 
> with -fprofile-arcs -ftest-coverage.  (One of these causes -lgcov to be 
> linked, which includes the missing symbol.)  This is not done because the 
> shared object link rules don't use CFLAGS.

Shared object link rules should use another variable (LDFLAGS?) and those
options should be added that variable as well.

--  Gregory Stark EnterpriseDB          http://www.enterprisedb.com Get trained by Bruce Momjian - ask me about
EnterpriseDB'sPostgreSQL training!
 

Gregory Stark wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
> 
>> The reason for that problem is that the shared object needs to be linked 
>> with -fprofile-arcs -ftest-coverage.  (One of these causes -lgcov to be 
>> linked, which includes the missing symbol.)  This is not done because the 
>> shared object link rules don't use CFLAGS.
> 
> Shared object link rules should use another variable (LDFLAGS?) and those
> options should be added that variable as well.

When linking executables, we already use both CFLAGS and LDFLAGS.  This 
is the standard way in the GNU-enabled world.  And it does exactly the 
right thing in this gcov case.  If we invented another variable, we 
would disrupt that system and would further differentiate between 
different types of linking, while we should ultimately aim to make it 
less different.

> The problem with those files is that the source file contains lines  
> like this:
>
> #line 1042 "y.tab.c"
>
> but that source file does not exist, as it is renamed to gram.c.
>
> We could fix that in one of two ways:
>
> 1) Use bison's -o option to put the output file in the right place  
> directly,
> if we are dealing with bison (and don't bother to support code  
> coverage
> analysis with other yaccs), or
>
> 2) Run a pattern replacement across the grammar output files as  
> their are
> renamed.

Why not use the %output directive in the grammar file instead; that  
way you don't need to add any special flags to the Makefile.
        -- Korry

Korry Douglas wrote:
>> 1) Use bison's -o option to put the output file in the right place 
>> directly,
>> if we are dealing with bison (and don't bother to support code coverage
>> analysis with other yaccs), or
>>
>> 2) Run a pattern replacement across the grammar output files as their are
>> renamed.
> 
> Why not use the %output directive in the grammar file instead; that way 
> you don't need to add any special flags to the Makefile.

I think only bison supports that directive.

Peter Eisentraut <peter_e@gmx.net> writes:
> Korry Douglas wrote:
>> Why not use the %output directive in the grammar file instead; that way 
>> you don't need to add any special flags to the Makefile.

> I think only bison supports that directive.

We're pretty much assuming bison anyway, no?  It's been years since
I heard of anyone successfully building the backend grammar with plain
yacc.
        regards, tom lane

Tom Lane wrote:
> We're pretty much assuming bison anyway, no?  It's been years since
> I heard of anyone successfully building the backend grammar with plain
> yacc.

In my recollection, you were the last holdout on that with the 
occasional HP-UX yacc test.  But I seem to recall that that combination 
actually no longer worked the last time.

If no one else has any interest in maintaining other-yacc support, then 
we might as well drop it.

Peter Eisentraut <peter_e@gmx.net> writes:
> Tom Lane wrote:
>> We're pretty much assuming bison anyway, no?  It's been years since
>> I heard of anyone successfully building the backend grammar with plain
>> yacc.

> In my recollection, you were the last holdout on that with the 
> occasional HP-UX yacc test.  But I seem to recall that that combination 
> actually no longer worked the last time.

I don't think I've tried that in this century ;-).  Between the sheer
size of the grammar and the fact that we're already depending on the
behavior of several arcane %-options, I really doubt any tool besides
bison will work.  Besides, the whole point of shipping the built files
in tarballs is to ensure no one has to use any other tool.
        regards, tom lane

Peter Eisentraut wrote:
> Michelle Caisse wrote:
>> gcov gets confused when source files are generated. I eliminated
>> src/backend/bootstrap and ../parser from coverage analysis to avoid
>> errors of this type.
> 
> The problem with those files is that the source file contains lines like this:
> 
> #line 1042 "y.tab.c"
> 
> but that source file does not exist, as it is renamed to gram.c.

This problem is now fixed, so the workaround in the coverage patch can 
be dropped.

Michelle Caisse wrote:
> I've attached a patch that allows the generation of code coverage  
> statistics. To test it, apply the patch, then:
>
> autoconf
> ./configure --enable-coverage
> make
> make check (or execute any other application against the database to see  
> the coverage of that app)
> make coverage
> make coverage_out

"make clean" does not work for me; it doesn't remove the .gcda and .gcno
files.  Apparently the problem is that $(enable_coverage) is not
defined, so that part of common.mk is not called.

Note: one thing to keep in mind is directories like src/port.  There are
some .gcda and .gcno files in there too, but even if common.mk is fixed,
they will not be cleaned because src/port does not seem to use
common.mk.

Another thing that I'm unsure about is the coverage_out target.  It does
work, but is it running the coverage target each time it is invoked?
Because if so, it's removing all of ./coverage and creating it again ...
is this the desired behavior?

This patch is missing a installation.sgml patch, at a minimum.  I think
it would be useful to mention that we support gcov, and the make targets
we have, in some other part of the documentation.  I can't readily find
a good spot, but I think a new file to be inserted in the internals
chapter would be appropriate.

Two hunks no longer apply, but that's OK because they were working
around a problem that no longer exists.  Other than the minor gripes
above, the patch looks OK to me.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Thanks, I'll take a look at these issues.<br /><br /> -- Michelle<br /><br /> Alvaro Herrera wrote: <blockquote
cite="mid20080904212045.GD5786@alvh.no-ip.org"type="cite"><pre wrap="">Michelle Caisse wrote: </pre><blockquote
type="cite"><prewrap="">I've attached a patch that allows the generation of code coverage  
 
statistics. To test it, apply the patch, then:

autoconf
./configure --enable-coverage
make
make check (or execute any other application against the database to see  
the coverage of that app)
make coverage
make coverage_out   </pre></blockquote><pre wrap="">
"make clean" does not work for me; it doesn't remove the .gcda and .gcno
files.  Apparently the problem is that $(enable_coverage) is not
defined, so that part of common.mk is not called.

Note: one thing to keep in mind is directories like src/port.  There are
some .gcda and .gcno files in there too, but even if common.mk is fixed,
they will not be cleaned because src/port does not seem to use
common.mk.

Another thing that I'm unsure about is the coverage_out target.  It does
work, but is it running the coverage target each time it is invoked?
Because if so, it's removing all of ./coverage and creating it again ...
is this the desired behavior?

This patch is missing a installation.sgml patch, at a minimum.  I think
it would be useful to mention that we support gcov, and the make targets
we have, in some other part of the documentation.  I can't readily find
a good spot, but I think a new file to be inserted in the internals
chapter would be appropriate.

Two hunks no longer apply, but that's OK because they were working
around a problem that no longer exists.  Other than the minor gripes
above, the patch looks OK to me.
 </pre></blockquote><br /><pre class="moz-signature" cols="72">-- 
Michelle Caisse               Sun Microsystems
California, U.S.     <a class="moz-txt-link-freetext" href="http://sun.com/postgresql">http://sun.com/postgresql</a>

</pre>

Michelle Caisse wrote:
> Thanks, I'll take a look at these issues.

I have committed your patch with some rework that mainly addresses the 
concerns also found by Alvaro with regard to cleaning and dependency 
handling.  I have renamed the out target to coverage-html, to be more in 
line with our other targets.  So the workflow is now

./configure --enable-coverage
make
make check
make coverage-html
$BROWSER coverage/index.html

There are a couple of platform-specific problems that I have come across:

* Linux (Debian) works OK
* FreeBSD build fails, apparently because libgcov.a was not compiled 
with -fPIC
* Mac OS X builds and runs OK but the server does not shut down in 
finite time at the end of the regression tests; it has to be killed 
manually.

I have uploaded an example run here: 
http://developer.postgresql.org/~petere/coverage/

Current test coverage is about 66% overall.

Peter Eisentraut <peter_e@gmx.net> writes:

> I have uploaded an example run here:
> http://developer.postgresql.org/~petere/coverage/
>
> Current test coverage is about 66% overall.

With some pretty glaring gaps: 0% coverage of geqo, 0% coverage of logtape
which implies no tuplesorts are spilling to disk, no coverage of mark/restore
on index scans...

--  Gregory Stark EnterpriseDB          http://www.enterprisedb.com Get trained by Bruce Momjian - ask me about
EnterpriseDB'sPostgreSQL training!
 

Gregory Stark wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
> 
> > I have uploaded an example run here:
> > http://developer.postgresql.org/~petere/coverage/
> >
> > Current test coverage is about 66% overall.
> 
> With some pretty glaring gaps: 0% coverage of geqo, 0% coverage of logtape
> which implies no tuplesorts are spilling to disk, no coverage of mark/restore
> on index scans...

Yah, that kinda shocked me too.  Clearly we should spend some effort to
expand the regression tests a bit.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.