Date: Wed, 5 Apr 2017 09:24:26 -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: <20170405132426.GW9812@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>
 <20170404123011.GX9812@tamriel.snowman.net>
 <CAB7nPqToUk6z=+jk11rNiEShoL5963Jwx4jg+svKuKHvZ3hhtQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="9fao37YCYkYVbMEP"
Content-Disposition: inline
In-Reply-To: 
 <CAB7nPqToUk6z=+jk11rNiEShoL5963Jwx4jg+svKuKHvZ3hhtQ@mail.gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org


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

Michael,

* Michael Paquier (michael.paquier@gmail.com) wrote:
> On Tue, Apr 4, 2017 at 9:30 PM, Stephen Frost <sfrost@snowman.net> wrote:
> > * Michael Paquier (michael.paquier@gmail.com) wrote:
> >> On Tue, Apr 4, 2017 at 7:38 AM, Stephen Frost <sfrost@snowman.net> wro=
te:
> >> 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.
>=20
> 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.
>=20
> 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.
>=20
> I did not do a coverage test first, but surely this patch needs
> numbers, so here you go.
>=20
> 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)
>=20
> 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

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

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

iQIcBAEBCgAGBQJY5PAJAAoJEO1sijiDR2RVYdgQAJcjqZIwH7v7kHkTiGxOJrJS
zCFdcm8al7tM9U9bqml2sGh5agMPjTm8BlsoemORW+mfLlhltHjXkAO1S/vfHpKy
k5m9BPOBG3jcrDHbCkO5WhLH8IvIoDZcM3prZoC/nBAs1gaAEgj8eS5jl/4aQsyo
7Ve9GfaeCfxVbChsqRsrPNj/vIytC89xw0oVHwcZW2NWiqG87Oe6YBIxFhM7Dt7S
ZZSR8dIja07MEy17FR+YtSgZkiGlBJj9xLd5jzREo/Rokr/+Llly/t6tdkz/c/xZ
h01YLqeuAMJEulWv+s3RCYd1EcAEwfLgXKC9lZoZZsT7Kneo0OYWVMqfKwQYM43z
zf8BadPU7bfUspEK+3hv8ermHRBXV2LGZtRQOQjXDPmeiOiM7qJyLbV2j19AkcpD
7n1USMvbJzRKXn5kwV4Q9Q+kdPpNu3AMxG+dmm8oFAL/yNKGLTKcIBwBNVPAyhuv
JmpODbjZYnVRZwqhi0VcsDrFKEpkCZ05n+jMAJs3EMOf7rCsmZF5C1L4XFV0Fzv0
qsWiRjL/55vnIDI73IXlA9GCEVouJ6Pmk68NPsfkARrk0hhjEYCS374ajdsnq9SQ
Q/uh9G+zlWOig0pUhTjI/k9IP6DoZGb6jVDdlmbvlnooga/C4gGgA0/5rnZF+ZOJ
hdJsSdxStng4D0itkPnA
=rU8V
-----END PGP SIGNATURE-----

--9fao37YCYkYVbMEP--