Re: Rewriting the test of pg_upgrade as a TAP test

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: Craig Ringer <craig(at)2ndquadrant(dot)com>, Noah Misch <noah(at)leadboat(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Rewriting the test of pg_upgrade as a TAP test
Date: 2017-04-05 02:28:20
Message-ID: CAB7nPqToUk6z=+jk11rNiEShoL5963Jwx4jg+svKuKHvZ3hhtQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On Tue, Apr 4, 2017 at 9:30 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> * Michael Paquier (michael(dot)paquier(at)gmail(dot)com) wrote:
>> On Tue, Apr 4, 2017 at 7:38 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>> The patch presented here does lower the coverage we have now.
>
> I assume (perhaps mistakenly) that this statement was based on an
> analysis of before-and-after 'make coverage' runs. Here you are saying
> that you're *not* lowering the coverage.

My apologies here, when I used the work "coverage" in my previous
emails it visibly implied that I meant that I had used the coverage
stuff. But I did not because the TAP test proposed does exactly what
test.sh is doing:
1) Initialize the old cluster and start it.
2) create a bunch of databases with full range of ascii characters.
3) Run regression tests.
4) Take dump on old cluster.
4) Stop the old cluster.
5) Initialize the new cluster.
6) Run pg_upgrade.
7) Start new cluster.
8) Take dump from it.
9) Run deletion script (Oops forgot this part!)

> I understand how the current pg_upgrade tests work. I don't see
> off-hand why the TAP tests would reduce the code coverage of pg_upgrade,
> but if they do, we should be able to figure out why and correct it.

Good news is that this patch at least does not lower the bar.

>> So in short I don't think that this lack of infrastructure should be a
>> barrier for what is basically a cleanup but... I just work here.
>
> I didn't mean to imply that this patch needs to address the
> cross-version testing challenge, was merely mentioning that there's been
> some work in this area already by Andrew and that if you're interested
> in working on that problem that you should probably coordinate with him.

Sure.

> What I do think is a barrier to this patch moving forward is if it
> reduces our current code coverage testing (with the same-version
> pg_upgrade that's run in the regular regression tests). If it doesn't,
> then great, but if it does, then the patch should be updated to fix
> that.

I did not do a coverage test first, but surely this patch needs
numbers, so here you go.

Without the patch, here is the coverage of src/bin/pg_upgrade:
lines......: 57.7% (1311 of 2273 lines)
functions..: 85.3% (87 of 102 functions)

And with the patch:
lines......: 58.8% (1349 of 2294 lines)
functions..: 85.6% (89 of 104 functions)
The coverage gets a bit higher as a couple of basic code paths like
pg_upgrade --help get looked at.
--
Michael

Attachment Content-Type Size
pgupgrade-tap-test-v2.patch application/octet-stream 18.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2017-04-05 02:30:53 Re: Rewriting the test of pg_upgrade as a TAP test
Previous Message Pavan Deolasee 2017-04-05 02:21:17 Re: Patch: Write Amplification Reduction Method (WARM)