public inbox for [email protected]  
help / color / mirror / Atom feed
From: Stephen Frost <[email protected]>
To: Michael Paquier <[email protected]>
Cc: Craig Ringer <[email protected]>
Cc: Noah Misch <[email protected]>
Cc: PostgreSQL mailing lists <[email protected]>
Subject: Re: Rewriting the test of pg_upgrade as a TAP test
Date: Tue, 4 Apr 2017 08:30:12 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAB7nPqS2Xj8Zs48mk7NjtKQa69Of3wtGJ+tKpgs2A-Ne9Zy4ww@mail.gmail.com>
References: <CAB7nPqRdaN1A1YNjxNL9T1jUEWct8ttqq29dNv8W_o37+e8wfA@mail.gmail.com>
	<CAMsr+YGn5tu+rhvC7hBgn2zmc7Y0egrQ79J6DUKsaeFLWdqrGQ@mail.gmail.com>
	<[email protected]>
	<CAB7nPqQKUoV6Vce4Ba9OTf8KA7n96rPM_EOD665hGHmaGFerDg@mail.gmail.com>
	<CAOuzzgoTyw+tW063W4A_UAj9SAqtHBTqgU_cE-4eGSS7V7kK_g@mail.gmail.com>
	<CAB7nPqS2Xj8Zs48mk7NjtKQa69Of3wtGJ+tKpgs2A-Ne9Zy4ww@mail.gmail.com>
List-Unsubscribe: <mailto:[email protected]?body=unsub%20pgsql-hackers>

* Michael Paquier ([email protected]) wrote:
> On Tue, Apr 4, 2017 at 7:38 AM, Stephen Frost <[email protected]> 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/[email protected]
> >
> > 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



Attachments:

  [application/pgp-signature] signature.asc (819B, ../[email protected]/2-signature.asc)
  download

view thread (113+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Rewriting the test of pg_upgrade as a TAP test
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox