Thread: Rewriting the test of pg_upgrade as a TAP test

Hi all,

The $subject has been mentioned a couple of times already, the last
one being here:
https://www.postgresql.org/message-id/20170401072814.GA2528448@tornado.leadboat.com

The code tree has to maintain now two set of scripts for the same
test: test.sh for all *nix platforms, and vcregress.pl for MSVC.
Attached is a patch to remove that and replace the existing test by a
TAP test. The size of the patch speaks by itself:
 6 files changed, 98 insertions(+), 389 deletions(-)

I had for some time a WIP patch on which dust has accumulated, so
attached is a more polished version. In more details, here is what
happens:
- test.sh is removed.
- vcregress.pl loses upgradecheck.
- The new test is added. In the case of MSVC this is now part of bincheck.
Patch has been tested on macos and Windows.

I am parking that in the next commit fest.

Regards,
-- 
Michael

On 4/3/17 09:07, Michael Paquier wrote:
> I had for some time a WIP patch on which dust has accumulated, so
> attached is a more polished version. In more details, here is what
> happens:
> - test.sh is removed.
> - vcregress.pl loses upgradecheck.
> - The new test is added. In the case of MSVC this is now part of bincheck.
> Patch has been tested on macos and Windows.

This is a useful start.  What I'd really like to see is that instead of
running the full serial tests to populate the pre-upgrade database, we
determine a useful subset of what that ends up generating and just
populate with that.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Peter,

* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
> On 4/3/17 09:07, Michael Paquier wrote:
> > I had for some time a WIP patch on which dust has accumulated, so
> > attached is a more polished version. In more details, here is what
> > happens:
> > - test.sh is removed.
> > - vcregress.pl loses upgradecheck.
> > - The new test is added. In the case of MSVC this is now part of bincheck.
> > Patch has been tested on macos and Windows.
>
> This is a useful start.  What I'd really like to see is that instead of
> running the full serial tests to populate the pre-upgrade database, we
> determine a useful subset of what that ends up generating and just
> populate with that.

In the past, we've had the notion that the regression tests are intended
to also cover pg_upgrade/pg_dump by "leaving things around".  What I
found in my efforts to provide better coverage in pg_dump is that there
was quite a bit of coverage missing using that approach.

Perhaps that could be fixed, but I tend to think it's a better approach
to have a complete set of pg_upgrade/pg_dump tests in one place that
doesn't also have a bunch of other tests mixed in (and would also mean
that the regular regression tests could be 'clean').

I could also see us defining one set of commands to run which create
every type of object in the system that pg_dump understands and then
using that to perform the pg_dump and pg_upgrade tests.  Those commands
would have to be annotated with minimum major version and maximum major
version, assuming we're going to use them cross-version, but that should
be reasonably straight-forward to do.

Another question is how much sense it makes to test this logic,
essentially, twice.  The testing of pg_dump covers the pg_dump code,
which is what pg_upgrade uses anyway.  The pg_upgrade tests really need
to cover the non-pg_dump-related parts, assuming we have appropriate
coverage in the pg_dump tests for the --binary-upgrade mode.  Of course,
if we don't, then we should go about fixing that.  There are certainly
some tests now but perhaps we need more or need to have improvmenets
made there.

Thanks!

Stephen