Date: Wed, 5 Apr 2017 11:13:33 -0400
From: Stephen Frost <sfrost@snowman.net>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Michael Paquier <michael.paquier@gmail.com>,
	Peter Eisentraut <peter.eisentraut@2ndquadrant.com>,
	Andres Freund <andres@anarazel.de>,
	PostgreSQL mailing lists <pgsql-hackers@postgresql.org>,
	Noah Misch <noah@leadboat.com>
Subject: Re: Rewriting the test of pg_upgrade as a TAP test
Message-ID: <20170405151333.GF9812@tamriel.snowman.net>
References: 
 <CAB7nPqRdaN1A1YNjxNL9T1jUEWct8ttqq29dNv8W_o37+e8wfA@mail.gmail.com>
 <62a4e16e-5ea3-89c9-c94a-ee3be953b94a@2ndquadrant.com>
 <20170403153200.jqntjo7zat6x5rhh@alap3.anarazel.de>
 <42d412a2-9cd7-7ee1-0c1a-e8d230d568b0@2ndquadrant.com>
 <CAB7nPqR9JWCoU8z_CXovL2UkTxahcEx3TzyBYzk3qyXh+MTXMg@mail.gmail.com>
 <20170405135211.GZ9812@tamriel.snowman.net>
 <6679.1491401713@sss.pgh.pa.us>
 <20170405144041.GC9812@tamriel.snowman.net>
 <8402.1491404529@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="yarHmbg0H5uVLN80"
Content-Disposition: inline
In-Reply-To: <8402.1491404529@sss.pgh.pa.us>
User-Agent: Mutt/1.5.21 (2010-09-15)
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org


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

Tom,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Stephen Frost <sfrost@snowman.net> writes:
> > I'm all for adding tests into pg_dump which do things like drop columns
> > and change column names and other cases which could impact if the
> > pg_dump is correct or not, and there's nothing preventing those tests
> > from being added in the existing structure.  Certainly, before we remove
> > the coverage provided by running the serial test suite and then using
> > pg_upgrade, we should analyze what is being tested and ensure that we're
> > providing that same set of testing in the pg_dump TAP tests.
>=20
> I don't think you grasped my basic point, which is that I'm worried about
> emergent cases that we don't foresee needing to test (and that no amount
> of code coverage checking would have shown up as being overlooked).
> Admittedly, relying on the core regression tests to trigger such cases is
> a pretty haphazard strategy, but it's way better than no strategy at all.

The tests that were added to the core regression suite were done so for
a reason and hopefully we can identify cases where it'd make sense to
also run those tests for pg_upgrade/pg_dump testing.  More-or-less
anything that materially changes the catalog should be included, I would
think.  Things that are only really only working with the heap/index
files don't really need to be performed because the pg_upgrade process
doesn't change those.

> > We might also consider if there's a way to change the format for those
> > tests to make them a bit less impenetrable for non-Perl folks to work
> > with and to make it simpler to add new tests as new features are added.
>=20
> TBH, that's part of my allergy to this concept, ie that this test
> mechanism seems pretty write-only.  I do not think that people will add
> pg_dump test cases except when required to by project policy, so that
> we will end up with a very skeletal set of tests that won't find any
> unforeseen behaviors.

I certainly agree that the current structure for the tests isn't trivial
to work with and would welcome suggestions as to how to improve it.  Now
that we've had this testing structure for a year and have added quite a
bit more to it, it's definitely clear that we need to find a more
developer-friendly approach.

> The TAP tests in general are utterly developer-unfriendly from where
> I sit: not only is the code pretty unreadable, but god help you when
> you need to try to debug a failure.  I think that some serious effort
> needs to be spent on improving that situation before we imagine that
> we can throw away other test mechanisms we have today in favor of
> TAP tests.

Agreed.

Thanks!

Stephen

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

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

iQIcBAEBCgAGBQJY5QmdAAoJEO1sijiDR2RVavQQAJrhlzqJxBRQ4RhrfUAbixcU
gDtzODYTunOTXN0txjqxRALXV0/ez8T6POdI+vCwIWQ0Ld7V771VC8IUEIRCPS0E
qbJGE7zCIxgZqj0cn+XTa4ywr1ZJ4nRs6wcDJ3EyL5Q0GxOahXvkmMmhxrS2M3qZ
rYV9EDITlh0OQXscaoMYHgFN4Zy861zckNY9ihUSD0GbjHKDuf5koK7LDUCEW2I9
kX2XR7ugrAhFCKFiNotKw402O7IMzF1MsD9q/9RwCMyS1vlcCqkcX2f6iaAtFSrd
cwMijN+sjKSlxoE0SRBs2fVS3mpDUMyXW8QT6d5SnG6q4IcriuazU78G1hIcE522
ep0eXntWX7WsoC1b4jMHLQK7GPMVa4bkNdpR82AsAetkDykkx69N9fWFpEZnMEx3
2GIIKYHHHQYpdtyoeaa2tUgtBY8DaFv4MnxBxvo9n6UDR6yYijOxo3M2h7EkASpj
LOLi+5VtlUZn8hMUr24r2+kYu5VXo0xheiQc2/LNtSw8amKiwwL2ly6nFwD6XMer
ztdWjjzOayXYUJoD4duAYLnXW9CtfJdzH19bE+qP1Lfd3W9eTstrAVojb+aVm1e3
kBUMUpuBDFcCKkgXP+n4LC4jPlgIJvBjBN9tXEEgKb6qsYilesYLKq522f+Nrs57
t0BY3J0WgeH9gmVM5t7u
=vyM0
-----END PGP SIGNATURE-----

--yarHmbg0H5uVLN80--