Michael, * Michael Paquier (michael.paqu...@gmail.com) wrote: > On Tue, Apr 4, 2017 at 9:30 PM, Stephen Frost <sfr...@snowman.net> wrote: > > * Michael Paquier (michael.paqu...@gmail.com) wrote: > >> On Tue, Apr 4, 2017 at 7:38 AM, Stephen Frost <sfr...@snowman.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
signature.asc
Description: Digital signature