Re: Rewriting the test of pg_upgrade as a TAP test - Mailing list pgsql-hackers

* Michael Paquier (michael.paquier@gmail.com) wrote:
> On Tue, Apr 4, 2017 at 7:38 AM, Stephen Frost <sfrost@snowman.net> wrote:
> > Not good if it lowers the coverage, but hopefully that's fixable. Have you
> > analyzed where we're reducing coverage..?
>
> The current set of tests is just running pg_upgrade using the same
> version for the source and target instances. Based on that I am not
> lowering what is happening in this set of tests. Just doing some
> cleanup.

Ok, I'm confused.

I wrote the above in response to your statement:

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

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.

> > As for what I'm remembering, there's this:
> > https://www.postgresql.org/message-id/5669acd9-efdc-2a0f-afea-10ba6003a050@dunslane.net
> >
> > Of course, it's possible I misunderstood..
>
> This invokes directly pg_upgrade, so that's actually a third,
> different way to test pg_upgrade on top of the two existing methods
> that are used in vcregress.pl and pg_upgrade's test.sh

Ok, though I'm not sure that I see that as necessairly a bad thing.
There are only specific tools that we actually worry about being able to
work with older versions of PG, after all.

> > That seems focused on upgrading and I'd really like to see a general way to
> > do this with the TAP structure, specifically so we can test pg_dump and psql
> > against older versions.  Having the ability to then be run under the
> > coverage testing would be fantastic and would help a great deal with the
> > coverage report.
>
> I don't disagree with that. What we need first is some logic to store
> in a temporary directory the installation of all the previous major
> versions that we have. For example use a subfolder in tmp_install
> tagged with the major version number, and then when the TAP test
> starts we scan for all the versions present in tmp_install and test
> the upgrade with a full grid. One issue though is that we add
> $(bindir) in PATH and that there is currently no logic to change PATH
> automatically depending on the major/minor versions you are working
> on.

Right, I figured that what Andrew did in the above post was something
along these lines, but I've not looked at it in any depth.

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

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.

Thanks!

Stephen