Re: Rewriting the test of pg_upgrade as a TAP test

Michael,

* Michael Paquier (michael(dot)paquier(at)gmail(dot)com) wrote:
> 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:

Ah, ok, no worries. Glad to hear that there isn't any difference in
coverage or in what's being done.

> 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!)

Presumably the check to match the old dump against the new one is also
performed?

> > 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.

Great, then I don't see any reason we can't move forward with it.

> > 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.

Fantastic, that's even better.

Thanks!

Stephen