Date: Tue, 4 Apr 2017 08:30:12 -0400
From: Stephen Frost <sfrost@snowman.net>
To: Michael Paquier <michael.paquier@gmail.com>
Cc: Craig Ringer <craig@2ndquadrant.com>, Noah Misch <noah@leadboat.com>,
	PostgreSQL mailing lists <pgsql-hackers@postgresql.org>
Subject: Re: Rewriting the test of pg_upgrade as a TAP test
Message-ID: <20170404123011.GX9812@tamriel.snowman.net>
References: 
 <CAB7nPqRdaN1A1YNjxNL9T1jUEWct8ttqq29dNv8W_o37+e8wfA@mail.gmail.com>
 <CAMsr+YGn5tu+rhvC7hBgn2zmc7Y0egrQ79J6DUKsaeFLWdqrGQ@mail.gmail.com>
 <20170403151237.GK9812@tamriel.snowman.net>
 <CAB7nPqQKUoV6Vce4Ba9OTf8KA7n96rPM_EOD665hGHmaGFerDg@mail.gmail.com>
 <CAOuzzgoTyw+tW063W4A_UAj9SAqtHBTqgU_cE-4eGSS7V7kK_g@mail.gmail.com>
 <CAB7nPqS2Xj8Zs48mk7NjtKQa69Of3wtGJ+tKpgs2A-Ne9Zy4ww@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="uCl3rT4XoI4EtTp8"
Content-Disposition: inline
In-Reply-To: 
 <CAB7nPqS2Xj8Zs48mk7NjtKQa69Of3wtGJ+tKpgs2A-Ne9Zy4ww@mail.gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org


--uCl3rT4XoI4EtTp8
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

* 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..?
>=20
> 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-10ba6003a=
050@dunslane.net
> >
> > Of course, it's possible I misunderstood..
>=20
> 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 wa=
y 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.
>=20
> 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


--uCl3rT4XoI4EtTp8
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: Digital signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBCgAGBQJY45HTAAoJEO1sijiDR2RVab8P/24KO8BD0npywfcCb8+WbFQ4
VW1aPkUQSBlsvsn3uIUApCu9BDLTwmiTn/kX4MDhNdZwalUWsOHorn8+0HFRXQKI
sWjhVT5BfRhFI9h0x4Xtd90kcIh1dTxrs2y5emnOJTEWM7CEgXPhddBjNTuFUQXc
c7lBBYeGrtDnil9aIr7Rhj4/6a+M6tsuvOBTF+Y4DWtLQhKvL6IBW5w5Pcz8avp8
MLeMT5faYX6e1vuw3sB5h0NgJqUh6EEE4YdCKj9oJuogkA/6CBl/3T8fNQnnDH27
1SYeS7/nLeYEMJhoS30xcOMTstksovv80IinlUqKvIKZgUu1l4FR2xSGhWGdpWwi
RmhBQxRWBXMPE33i6wO7/nc66qQ9aC+OPH2C9eeF8UCeXvVOK9ws6+y8TQrZlNcC
zAsd0geSfmGAcvUe0+KpdVjodRju6jexSdOxmWLDp/uUypzIXu/X26vHj2w8ATiB
JJcOlJI6AZmxpRsAentcYwy71dbifZpzzBqx58Cxzx1jcfwxQvoO/mb7LXEu9HMh
cQ+ubMueSLOxBPBNcrG6KS/vN6Ld9fNnkkYe/qUI83PHhlDzM5dboVWYZETKzx63
zSsQArrJbheBO0qDG3dkV/VKC1eqM5S9oYAxuQ+IgcHDbrdI/1jmSozDVphovz0o
5utOAueZzl5/3liGbDke
=Ve40
-----END PGP SIGNATURE-----

--uCl3rT4XoI4EtTp8--