Differential code coverage between 16 and HEAD

From: Andres Freund <andres(at)anarazel(dot)de>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Differential code coverage between 16 and HEAD
Date: 2024-04-14 22:33:05
Message-ID: 20240414223305.m3i5eju6zylabvln@awork3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

To see how well we're doing testing newly introduced code, I computed the
differential code coverage between REL_16_STABLE and HEAD.

While arguably comparing HEAD to the the merge-base between REL_16_STABLE and
HEAD would be more accurate, I chose REL_16_STABLE because we've backpatched
bugfixes with tests etc.

I first got some nonsensical differences. That turns out to be due to
immediate shutdowns in the tests, which

a) can loose coverage, e.g. there were no hits for most of walsummarizer.c,
because the test shuts always shuts it down immediately
b) can cause corrupted coverage files if a process is shut down while writing
out coverage files

I partially worked around a) by writing out coverage files during abnormal
shutdowns. That requires some care, I'll send a separate email about that. I
worked around b) by rerunning tests until that didn't occur.

The differential code coverage view in lcov is still somewhat raw. I had to
weaken two error checks to get it to succeed in postgres. You can hover over
the code coverage columns to get more details about what the various three
letter acronyms mean. The most important ones are:
- UNC - uncovered new code, i.e. we added code that's not tested
- LBC - lost baseline coverage, i.e previously tested code isn't anymore
- UBC - untested baseline, i.e. code that's still not tested
- GBC - gained baseline coverage - unchanged code that's now tested
- GNC - gained new coverage - new code that's tested

https://anarazel.de/postgres/cov/16-vs-HEAD-2024-04-14/

This includes "branch coverage" - I'm not sure that's worth the additional
clutter it generates.

Looking at the differential coverage results, at least the following seem
notable:

- We added a bit less uncovered code than last year, but it's not quite a fair
comparison, because I ran the numbers for 16 2023-04-08. Since the feature
freeze, 17's coverage has improved by a few hundred lines (8225c2fd40c).

- A good bit of the newly uncovered code is in branches that are legitimately
hard to reach (unlikely errors etc).

- Some of the new walsummary code could use more tests.
https://anarazel.de/postgres/cov/16-vs-HEAD-2024-04-14/src/backend/backup/walsummaryfuncs.c.gcov.html#L69
https://anarazel.de/postgres/cov/16-vs-HEAD-2024-04-14/src/bin/pg_combinebackup/pg_combinebackup.c.gcov.html#L424
https://anarazel.de/postgres/cov/16-vs-HEAD-2024-04-14/src/common/blkreftable.c.gcov.html#L790

- the new buffer eviction paths aren't tested at all:
https://anarazel.de/postgres/cov/16-vs-HEAD-2024-04-14/src/backend/storage/buffer/bufmgr.c.gcov.html#L6023
https://anarazel.de/postgres/cov/16-vs-HEAD-2024-04-14/contrib/pg_buffercache/pg_buffercache_pages.c.gcov.html#L356
It looks like it should be fairly trivial to test at least the basics?

- Coverage for some of the new unicode code is pretty poor:
https://anarazel.de/postgres/cov/16-vs-HEAD-2024-04-14/src/common/unicode_category.c.gcov.html#L122

- Some of the new nbtree code could use a bit more tests:
https://anarazel.de/postgres/cov/16-vs-HEAD-2024-04-14/src/backend/access/nbtree/nbtutils.c.gcov.html#L1468

- Our coverage of the non-default copy modes of pg_upgrade, pg_combinebackup
is nonexistent, and that got worse with the introduction of a new method
this release:
https://anarazel.de/postgres/cov/16-vs-HEAD-2024-04-14/src/bin/pg_upgrade/file.c.gcov.html#L360
https://anarazel.de/postgres/cov/16-vs-HEAD-2024-04-14/src/bin/pg_upgrade/file.c.gcov.html#L400
https://anarazel.de/postgres/cov/16-vs-HEAD-2024-04-14/src/bin/pg_combinebackup/copy_file.c.gcov.html#L209

- Code coverage of acl.c is atrocious and got worse.

- The new bump allocator has a fair amount of uncovered functionality:
https://anarazel.de/postgres/cov/16-vs-HEAD-2024-04-14/src/backend/utils/mmgr/bump.c.gcov.html#L293
https://anarazel.de/postgres/cov/16-vs-HEAD-2024-04-14/src/backend/utils/mmgr/bump.c.gcov.html#L613

- A lot of the new resowner functions aren't covered, but I guess the
equivalent functionality wasn't covered before, either:

https://anarazel.de/postgres/cov/16-vs-HEAD-2024-04-14/src/backend/utils/cache/catcache.c.gcov.html#L2317
https://anarazel.de/postgres/cov/16-vs-HEAD-2024-04-14/src/backend/utils/cache/relcache.c.gcov.html#L6868
https://anarazel.de/postgres/cov/16-vs-HEAD-2024-04-14/src/backend/storage/buffer/bufmgr.c.gcov.html#L3608
https://anarazel.de/postgres/cov/16-vs-HEAD-2024-04-14/src/backend/storage/buffer/bufmgr.c.gcov.html#L5978
...

Greetings,

Andres Freund

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ranier Vilela 2024-04-14 23:17:35 Fix out-of-bounds in the function GetCommandTagName
Previous Message Heikki Linnakangas 2024-04-14 20:34:55 White-box testing heap pruning