Обсуждение: coverage.postgresql.org not being refreshed
coverage.postgresql.org reports that its "current" coverage report is from 2021-01-11 19:06:43. I imagine that the backend infrastructure that runs lcov is having issues. -- Peter Geoghegan
On Sat, Jan 23, 2021 at 6:51 PM Peter Geoghegan <pg@bowt.ie> wrote: > > coverage.postgresql.org reports that its "current" coverage report is > from 2021-01-11 19:06:43. I imagine that the backend infrastructure > that runs lcov is having issues. It's indeed failing, and it seems we don't have any monitoring on it :/ All runs since the 11th have failed with a dead regression test: test connect/test1 ... stderr FAILED 202 ms I notice that the coverage scripts explicitly run: make -C src/interfaces/ecpg/test checktcp And that's what's been failing. And it is also failing for me locally. So it seems we have a test that's failing on debian at least. But we never run this on the buildfarm anywhere? Looking at the git log of that directory, I'm guessing it's related to some of the error message changes from Tom. (And I've now added monitoring) -- Magnus Hagander Me: https://www.hagander.net/ Work: https://www.redpill-linpro.com/
Magnus Hagander <magnus@hagander.net> writes:
> All runs since the 11th have failed with a dead regression test:
> test connect/test1 ... stderr FAILED 202 ms
> I notice that the coverage scripts explicitly run:
> make -C src/interfaces/ecpg/test checktcp
Ugh, no I never run that test either. Will fix.
regards, tom lane
On Sat, Jan 23, 2021 at 11:45 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Ugh, no I never run that test either. Will fix. Thanks for taking care of this so quickly, Tom and Magnus. -- Peter Geoghegan
On Sat, Jan 23, 2021 at 8:45 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Magnus Hagander <magnus@hagander.net> writes: > > All runs since the 11th have failed with a dead regression test: > > test connect/test1 ... stderr FAILED 202 ms > > > I notice that the coverage scripts explicitly run: > > make -C src/interfaces/ecpg/test checktcp > > Ugh, no I never run that test either. Will fix. Thanks! You missed the cron by a couple of minutes, but it recovered on the next run. Is there a particular reason we don't run it on the buildfarm, at least on some animals? Seems like that would be a much better way of finding these problems... -- Magnus Hagander Me: https://www.hagander.net/ Work: https://www.redpill-linpro.com/
Magnus Hagander <magnus@hagander.net> writes:
> On Sat, Jan 23, 2021 at 8:45 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Ugh, no I never run that test either. Will fix.
> Is there a particular reason we don't run it on the buildfarm, at
> least on some animals? Seems like that would be a much better way of
> finding these problems...
IIUC, the reason it's not run by default is that opening up a port
on localhost is insecure on multi-user systems. While that might
be a problem for buildfarm animals too, the ones that are running
things like the SSL checks already have the same hazard. Seems
like we could add a buildfarm client option to run this test.
Or we could just flush the test. It's not very clear to me what
coverage it's adding.
regards, tom lane
On 2021-Jan-23, Tom Lane wrote: > Or we could just flush the test. It's not very clear to me what > coverage it's adding. Hmm, I know I enabled it because it did cover something, though I no longer remember what that is. Is there a convenient way to compare two coverage runs? -- Álvaro Herrera Valdivia, Chile
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> Is there a convenient way to compare two coverage runs?
Hmm ... diff'ing the html output is really painful, but it
looks like using checktcp instead of check causes 48 lines
in ecpg to change from not-covered to covered.
$ grep -r lineNoCov coverage.std/src/interfaces/ecpg | wc
12555 120279 2212944
$ grep -r lineNoCov coverage.tcp/src/interfaces/ecpg | wc
12507 119723 2153410
regards, tom lane
I wrote:
> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
>> Is there a convenient way to compare two coverage runs?
> Hmm ... diff'ing the html output is really painful, but it
> looks like using checktcp instead of check causes 48 lines
> in ecpg to change from not-covered to covered.
Enlarging on that a little: the coverage in src/interfaces/ecpg
changes from
11860/(11860+12555.) = 0.48577
to
11908/(11908+12507.) = 0.48773
Meanwhile, the coverage in src/interfaces/libpq changes from
2421/(2421+4488.) = 0.35041
to
2481/(2481+4428.) = 0.35910
and the whole-system coverage from
256632/(256632+163564.) = 0.61074
to
256839/(256839+163357.) = 0.61124
This is comparing coverage reports for the core regression tests
plus ecpg "make check" versus core plus ecpg "make checktcp".
The increase in coverage outside ecpg is presumably because
we otherwise aren't hitting TCP-socket code paths at all,
while Unix-socket code is covered in both cases. So it's only
fair to include the non-ecpg coverage increase as a benefit
if you suppose that no other tests will hit the TCP code paths.
That might well have been true awhile ago, but I think that the
SSL and Kerberos tests are now covering much of that territory
(if coverage.postgresql.org is running those?)
regards, tom lane
On 2021-Jan-23, Tom Lane wrote: > Enlarging on that a little: the coverage in src/interfaces/ecpg > changes from > 11860/(11860+12555.) = 0.48577 > to > 11908/(11908+12507.) = 0.48773 Hm, a 0.2% increase ... sounds marginal. The other two increases are more interesting, but if the code is already covered by ssl/ldap/kerberos, then the checktcp test isn't buying much there either. So the script has this: make -j12 make -j12 -C contrib make -j12 -Otarget check-world PG_TEST_EXTRA="ssl ldap kerberos" make -C src/interfaces/ecpg/test checktcp make coverage-html I mentioned in https://postgr.es/m/20190530202311.GA22421@alvherre.pgsql that ecpg coverage was bad, and that it increased with "checktcp". I didn't quote exactly what was the change, though :-( If what this test mode adds is really just a 0.2% increase, then that doesn't seem worth the risk. I think the only extra test it runs is connect/test1. -- Álvaro Herrera Valdivia, Chile "Ed is the standard text editor." http://groups.google.com/group/alt.religion.emacs/msg/8d94ddab6a9b0ad3
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> Hm, a 0.2% increase ... sounds marginal.
> The other two increases are more interesting, but if the code is already
> covered by ssl/ldap/kerberos, then the checktcp test isn't buying much there
> either.
Since those are all non-default build options, I suppose that the
checktcp target does have some reason to live: it's the only test
suite that will exercise TCP-related code paths in a minimal build.
(On non-Windows platforms, anyway.)
However, if nobody knows about it and no buildfarm animals test it,
that argument seems pretty hypothetical :-(
regards, tom lane