code coverage patch

Started by Michelle Caisseover 17 years ago16 messages
#1Michelle Caisse
Michelle.Caisse@Sun.COM
1 attachment(s)

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

Attachments:

coverage.difftext/plain; name=coverage.diffDownload
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&nbsp;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-check coverage
 .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
#2Peter Eisentraut
peter_e@gmx.net
In reply to: Michelle Caisse (#1)
Re: code coverage patch

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?

#3Peter Eisentraut
peter_e@gmx.net
In reply to: Michelle Caisse (#1)
Re: code coverage patch

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?

#4Gregory Stark
stark@enterprisedb.com
In reply to: Peter Eisentraut (#3)
Re: code coverage patch

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's PostgreSQL training!

#5Korry Douglas
korry.douglas@enterprisedb.com
In reply to: Peter Eisentraut (#2)
Re: code coverage patch

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

#6Peter Eisentraut
peter_e@gmx.net
In reply to: Gregory Stark (#4)
Re: code coverage patch

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.

#7Peter Eisentraut
peter_e@gmx.net
In reply to: Korry Douglas (#5)
Re: code coverage patch

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.

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#7)
Re: code coverage patch

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

#9Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#8)
Re: code coverage patch

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.

#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#9)
Re: code coverage patch

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

#11Peter Eisentraut
peter_e@gmx.net
In reply to: Peter Eisentraut (#2)
Re: code coverage patch

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.

#12Alvaro Herrera
alvherre@commandprompt.com
In reply to: Michelle Caisse (#1)
Re: code coverage patch

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.

#13Michelle Caisse
Michelle.Caisse@Sun.COM
In reply to: Alvaro Herrera (#12)
Re: code coverage patch

Thanks, I'll take a look at these issues.

-- Michelle

Alvaro Herrera wrote:

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.

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

#14Peter Eisentraut
peter_e@gmx.net
In reply to: Michelle Caisse (#13)
Re: code coverage patch

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.

#15Gregory Stark
stark@enterprisedb.com
In reply to: Peter Eisentraut (#14)
Re: code coverage patch

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's PostgreSQL training!

#16Alvaro Herrera
alvherre@commandprompt.com
In reply to: Gregory Stark (#15)
Re: code coverage patch

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.