coverage additions

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

I just enabled --enabled-llvm on the coverage reporting machine, which
made src/backend/jit/jit.c go from 60/71 % (line/function wise) to 78/85 % ...
and src/backend/jit/llvm from not appearing at all in the report to
78/94 %. That's a good improvement.

If there are other obvious improvements to be had, please let me know.

I was going to suggest that adding some or all of

-DCOPY_PARSE_PLAN_TREES
-DWRITE_READ_PARSE_PLAN_TREES
-DRAW_EXPRESSION_COVERAGE_TEST

to your CPPFLAGS might improve the reported coverage in backend/nodes/,
and perhaps other places.

regards, tom lane

On 2019-May-30, Tom Lane wrote:

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

I just enabled --enabled-llvm on the coverage reporting machine, which
made src/backend/jit/jit.c go from 60/71 % (line/function wise) to 78/85 % ...
and src/backend/jit/llvm from not appearing at all in the report to
78/94 %. That's a good improvement.

If there are other obvious improvements to be had, please let me know.

I was going to suggest that adding some or all of

-DCOPY_PARSE_PLAN_TREES
-DWRITE_READ_PARSE_PLAN_TREES
-DRAW_EXPRESSION_COVERAGE_TEST

to your CPPFLAGS might improve the reported coverage in backend/nodes/,
and perhaps other places.

I did that, and it certainly increased backend/nodes numbers
considerably. Thanks.

(extensible.c remains at 0% though, as does its companion nodeCustom.c).

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Apparently, for ecpg you have to do "make checktcp" in order for some of
the tests to run, and "make check-world" doesn't do that. Not sure
what's a good fix for this; do we want to add "make -C
src/interfaces/ecpg/test checktcp" to what "make check-world" does,
or do we rather what to add checktcp as a dependency of "make check" in
src/interfaces/ecpg?

Or do we just not want this test to be run by default, and thus I should
add "make -C src/interfaces/ecpg/test checktcp" to coverage.pg.org's
shell script? Maybe all we need is a way to have it run using
the PG_EXTRA_TEST thingy, but I'm not sure how that works ...?

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Apparently, for ecpg you have to do "make checktcp" in order for some of
the tests to run, and "make check-world" doesn't do that. Not sure
what's a good fix for this; do we want to add "make -C
src/interfaces/ecpg/test checktcp" to what "make check-world" does,
or do we rather what to add checktcp as a dependency of "make check" in
src/interfaces/ecpg?

Or do we just not want this test to be run by default, and thus I should
add "make -C src/interfaces/ecpg/test checktcp" to coverage.pg.org's
shell script?

I believe it's intentionally not run by default because it opens up
an externally-accessible server port.

regards, tom lane

On Thu, May 30, 2019 at 01:52:20PM -0400, Alvaro Herrera wrote:

If there are other obvious improvements to be had, please let me know.
(We have PG_TEST_EXTRA="ssl ldap" currently, do we have any more extra
tests now?)

You can add kerberos to this list, to give:
PG_TEST_EXTRA='ssl ldap kerberos'
--
Michael

On 2019-May-30, Michael Paquier wrote:

On Thu, May 30, 2019 at 01:52:20PM -0400, Alvaro Herrera wrote:

If there are other obvious improvements to be had, please let me know.
(We have PG_TEST_EXTRA="ssl ldap" currently, do we have any more extra
tests now?)

You can add kerberos to this list, to give:
PG_TEST_EXTRA='ssl ldap kerberos'

Ah, now I remember that I tried this before, but it requires some extra
packages installed in the machine I think, and those create running
services. Did you note that src/backend/libpq does not even list the
gssapi file?

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Thu, 2019-05-30 at 17:54 -0400, Tom Lane wrote:

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Apparently, for ecpg you have to do "make checktcp" in order for
some of
the tests to run, and "make check-world" doesn't do that. Not sure
what's a good fix for this; do we want to add "make -C
src/interfaces/ecpg/test checktcp" to what "make check-world" does,
or do we rather what to add checktcp as a dependency of "make
check" in
src/interfaces/ecpg?
Or do we just not want this test to be run by default, and thus I
should
add "make -C src/interfaces/ecpg/test checktcp" to
coverage.pg.org's
shell script?

I believe it's intentionally not run by default because it opens up
an externally-accessible server port.

Correct, iirc.

Michael
--
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael at xmpp dot meskes dot org
VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL

On 2019-Jun-02, Michael Meskes wrote:

On Thu, 2019-05-30 at 17:54 -0400, Tom Lane wrote:

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Or do we just not want this test to be run by default, and thus I
should add "make -C src/interfaces/ecpg/test checktcp" to
coverage.pg.org's shell script?

I believe it's intentionally not run by default because it opens up
an externally-accessible server port.

Correct, iirc.

Okay ... I added a "make -C src/interfaces/ecpg/test checktcp". Now
function-wise ecpg seems reasonable almost everywhere except compatlib
(though line-wise things are not so great).

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Sat, Jun 01, 2019 at 12:55:47AM -0400, Alvaro Herrera wrote:

Ah, now I remember that I tried this before, but it requires some extra
packages installed in the machine I think, and those create running
services. Did you note that src/backend/libpq does not even list the
gssapi file?

Do you mean the header file be-gssapi-common.h? It is stored in
src/backend/libpq/ which is obviously incorrect. I think that it
should be moved to src/include/libpq/be-gssapi-common.h. Its
identification marker even says that. Perhaps that's because of MSVC?
Stephen?
--
Michael

On 2019-Jun-04, Michael Paquier wrote:

On Sat, Jun 01, 2019 at 12:55:47AM -0400, Alvaro Herrera wrote:

Ah, now I remember that I tried this before, but it requires some extra
packages installed in the machine I think, and those create running
services. Did you note that src/backend/libpq does not even list the
gssapi file?

Do you mean the header file be-gssapi-common.h?

Actually, I meant be-gssapi-common.c, but I suppose having the file
appear at all would be dependent on whether the GSSAPI stuff is compiled
in, which seems to require yet another configure switch that we don't
have in the coverage machine.

But yeah, I think be-gssapi-common.h be in src/backend/libpq is against
our established practice and we should put it in src/include/libpq.

Which in turn makes me think that perhaps src/include/libpq/libpq.h
needs some splitting or something, because the be-openssl-common.c file
does not seem to have a corresponding header ...

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Tue, Jun 04, 2019 at 04:07:17PM -0400, Alvaro Herrera wrote:

On 2019-Jun-04, Michael Paquier wrote:

On Sat, Jun 01, 2019 at 12:55:47AM -0400, Alvaro Herrera wrote:

Ah, now I remember that I tried this before, but it requires some extra
packages installed in the machine I think, and those create running
services. Did you note that src/backend/libpq does not even list the
gssapi file?

Do you mean the header file be-gssapi-common.h?

Actually, I meant be-gssapi-common.c, but I suppose having the file
appear at all would be dependent on whether the GSSAPI stuff is compiled
in, which seems to require yet another configure switch that we don't
have in the coverage machine.

Not sure I still follow.. In src/backend/libpq we have
be-gssapi-common.c and be-gssapi-common.c, both getting added only if
with_gssapi is enabled.

Which in turn makes me think that perhaps src/include/libpq/libpq.h
needs some splitting or something, because the be-openssl-common.c file
does not seem to have a corresponding header ...

Yeah, it seems that there could be ways to split that in a smarter
way.
--
Michael

On Thu, Jun 06, 2019 at 06:14:45PM +0900, Michael Paquier wrote:

Not sure I still follow.. In src/backend/libpq we have
be-gssapi-common.c and be-gssapi-common.c, both getting added only if
with_gssapi is enabled.

I am going to spawn a new thread with a patch for the header file. I
think that we had better fix that before v12 ships.
--
Michael