Improving test coverage of extensions with pg_dump

On Tue, Mar 3, 2015 at 2:40 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

Those patches are really simple, but then perhaps there are better or
simpler ways than what is attached, so feel free to comment if you
have any ideas.

Attached are new patches somewhat based on the comments provided by
Peter Eisentraut
(/messages/by-id/54F62C3F.8070702@gmx.net).
- 0001 makes prove_check install the contents of the path located at
$(CURDIR)/t/extra if present, which would be the path that test
developers could use to store extensions, plugins or modules that
would then be usable by a given set of TAP tests as they are installed
in the temporary installation before launching the tests.
- 0002 is the test for pg_dump checking extensions containing FK
tables, this time integrated as a TAP test in src/bin/pg_dump, with
the extension in src/bin/pg_dump/t/extra.
IMO, this approach is scalable enough thinking long-term. And I think
that the location of those extra extensions is fine in t/ as an
hardcoded subfolder (any fresh idea being of course welcome) as this
makes the stuff in extra/ dedicated to only on set of TAP tests, and
it is even possible to set multiple extensions/modules.
Regards,
--
Michael

On 03/07/2015 02:34 PM, Michael Paquier wrote:

On Tue, Mar 3, 2015 at 2:40 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

Those patches are really simple, but then perhaps there are better or
simpler ways than what is attached, so feel free to comment if you
have any ideas.

Attached are new patches somewhat based on the comments provided by
Peter Eisentraut
(/messages/by-id/54F62C3F.8070702@gmx.net).
- 0001 makes prove_check install the contents of the path located at
$(CURDIR)/t/extra if present, which would be the path that test
developers could use to store extensions, plugins or modules that
would then be usable by a given set of TAP tests as they are installed
in the temporary installation before launching the tests.
- 0002 is the test for pg_dump checking extensions containing FK
tables, this time integrated as a TAP test in src/bin/pg_dump, with
the extension in src/bin/pg_dump/t/extra.
IMO, this approach is scalable enough thinking long-term. And I think
that the location of those extra extensions is fine in t/ as an
hardcoded subfolder (any fresh idea being of course welcome) as this
makes the stuff in extra/ dedicated to only on set of TAP tests, and
it is even possible to set multiple extensions/modules.

Hmm. I think it'd be better to put the tables_fk extension into
src/test/modules, and the test case under src/test/tables_fk/t/. I'm
inclined to think of this as a test case for an extension that contains
a table, which includes testing that pg_dump/restore works, rather than
as a test of pg_dump itself.

- Heikki

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Jul 8, 2015 at 1:32 AM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

Hmm. I think it'd be better to put the tables_fk extension into
src/test/modules, and the test case under src/test/tables_fk/t/. I'm
inclined to think of this as a test case for an extension that contains a
table, which includes testing that pg_dump/restore works, rather than as a
test of pg_dump itself.

The first version of this patch actually did that:
/messages/by-id/CAB7nPqQrxhy3+wvUmA69KJXiRPpV5qWJi-3cLn3ZJgByqe_BQQ@mail.gmail.com
The reason why it has been changed to this way of doing is that there
were concerns about bloating src/test/modules with many similar tests
in the future, like imagine pg_dump_test_1, pg_dump_test_2.

Attached is an updated version that can be used in src/test/modules as
well. Makefile needed also an extra EXTRA_INSTALL pointing to
src/test/modules/tables_fk. Nothing amazing. I have also reworded
one-two things at the same time while looking at that again.
--
Michael

I have reviewed this patch and it compiles runs and the new test case
passes. The code is also clean and the test seems like a useful
regression test.

What I do not like though is how the path src/test/tables_fk/t/ tells us
nothing about what features are of PostgreSQL are tested here. For this
I personally prefer the earlier versions where I think that was clear.

Another though: would it be worthwhile to also add an assertion to check
if the data really was restored properly or would that just be redundant
code?

--
Andreas

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Jul 30, 2015 at 5:54 AM, Andreas Karlsson <andreas@proxel.se> wrote:

What I do not like though is how the path src/test/tables_fk/t/ tells us
nothing about what features are of PostgreSQL are tested here. For this I
personally prefer the earlier versions where I think that was clear.

+Simple module used to check data dump ordering of pg_dump with tables
+linked with foreign keys.
The README mentions that this is to test pg_dump, perhaps referring to
the TAP tests makes sense as well?

Another thought: would it be worthwhile to also add an assertion to check if
the data really was restored properly or would that just be redundant code?

That seems worth doing, hence added something for it. Thanks for the suggestion.

At the same time I have added an entry in .gitignore for the generated
tmp_check/.
Regards,
--
Michael

On 07/30/2015 04:48 AM, Michael Paquier wrote:

On Thu, Jul 30, 2015 at 5:54 AM, Andreas Karlsson <andreas@proxel.se> wrote:

What I do not like though is how the path src/test/tables_fk/t/ tells us
nothing about what features are of PostgreSQL are tested here. For this I
personally prefer the earlier versions where I think that was clear.

+Simple module used to check data dump ordering of pg_dump with tables
+linked with foreign keys.
The README mentions that this is to test pg_dump, perhaps referring to
the TAP tests makes sense as well?

The comment is good, but what I personally do not like is that the path
to the test suite is non-obvious and not self explanatory I would not
expect src/test/tables_fk/t/ to test pg_dump and extensions. I find it
to confusing to regard the test suite as testing an extension called
"tables_fk" since in my mind that is just a necessary tool built to test
something else.

This is obviously a subjective thing, and I see that for example Heikki
likes it the way it is. I am fine with setting this as ready for
committer and and let a committer weigh in on the naming.

Another thought: would it be worthwhile to also add an assertion to check if
the data really was restored properly or would that just be redundant code?

That seems worth doing, hence added something for it. Thanks for the suggestion.

At the same time I have added an entry in .gitignore for the generated
tmp_check/.

Nice changes.

--
Andreas

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Jul 31, 2015 at 6:41 AM, Andreas Karlsson <andreas@proxel.se> wrote:

The comment is good, but what I personally do not like is that the path to
the test suite is non-obvious and not self explanatory I would not expect
src/test/tables_fk/t/ to test pg_dump and extensions. I find it to confusing
to regard the test suite as testing an extension called "tables_fk" since in
my mind that is just a necessary tool built to test something else.

This is obviously a subjective thing, and I see that for example Heikki
likes it the way it is. I am fine with setting this as ready for committer
and and let a committer weigh in on the naming.

Well, honestly, I would just have it in src/test/modules and call it a
deal. Because it is now the only extension that has such interactions
with perl tests. And because if modules/ gets bloated later on, we
could extend prove_check to install modules from extra paths, just
that it does not seem worth it to do it now for one module, and there
is no guarantee that we'll have that many around. Of course there is
no guarantee either that modules/ is not going to get bloated.

Note as well that I will be fine with any decision taken by the
committer who picks up this, this test case is useful by itself in any
case.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Jul 30, 2015 at 5:35 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

Note as well that I will be fine with any decision taken by the
committer who picks up this, this test case is useful by itself in any
case.

And I just recalled that I actually did no tests for this thing on
Windows. As this uses the TAP facility, I think that it makes most
sense to run it with tapcheck instead of modulescheck in vcregress.pl
because of its dependency with IPC::Run. The compilation with MSVC is
fixed as well.
--
Michael

On 2015-08-02 19:15:58 -0700, Michael Paquier wrote:

+psql 'postgres', 'CREATE EXTENSION tables_fk';
+
+# Insert some data before running the dump, this is needed to check
+# consistent data dump of tables with foreign key dependencies
+psql 'postgres', 'INSERT INTO cc_tab_fkey VALUES (1)';
+psql 'postgres', 'INSERT INTO bb_tab_fkey VALUES (1)';
+psql 'postgres', 'INSERT INTO aa_tab_fkey VALUES (1)';
+
+# Create a table depending on a FK defined in the extension
+psql 'postgres', 'CREATE TABLE dd_tab_fkey (id int REFERENCES bb_tab_fkey(id))';
+psql 'postgres', 'INSERT INTO dd_tab_fkey VALUES (1)';
+
+# Take a dump then re-deploy it to a new database
+command_ok(['pg_dump', '-d', 'postgres', '-f', "$tempdir/dump.sql"],
+		   'taken dump with installed extension');
+command_ok(['createdb', 'foobar1'], 'created new database to redeploy dump');
+command_ok(['psql', '--set=ON_ERROR_STOP=on', '-d', 'foobar1', '-f',
+			"$tempdir/dump.sql"], 'dump restored');
+
+# And check its content
+my $result = psql 'foobar1',
+	q{SELECT id FROM aa_tab_fkey UNION ALL
+SELECT id FROM bb_tab_fkey UNION ALL
+SELECT id FROM cc_tab_fkey UNION ALL
+SELECT id FROM dd_tab_fkey};
+is($result, qq(1
+1
+1
+1),
+	'consistency of data restored');
diff --git a/src/test/modules/tables_fk/tables_fk--1.0.sql b/src/test/modules/tables_fk/tables_fk--1.0.sql
new file mode 100644
index 0000000..e424610
--- /dev/null
+++ b/src/test/modules/tables_fk/tables_fk--1.0.sql
@@ -0,0 +1,20 @@
+/* src/test/modules/tables_fk/tables_fk--1.0.sql */
+
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION tables_fk" to load this file. \quit
+
+CREATE TABLE IF NOT EXISTS cc_tab_fkey (
+	id int PRIMARY KEY
+);
+
+CREATE TABLE IF NOT EXISTS bb_tab_fkey (
+	id int PRIMARY KEY REFERENCES cc_tab_fkey(id)
+);
+
+CREATE TABLE IF NOT EXISTS aa_tab_fkey (
+	id int REFERENCES bb_tab_fkey(id)
+);
+
+SELECT pg_catalog.pg_extension_config_dump('aa_tab_fkey', '');
+SELECT pg_catalog.pg_extension_config_dump('bb_tab_fkey', '');
+SELECT pg_catalog.pg_extension_config_dump('cc_tab_fkey', '');
diff --git a/src/test/modules/tables_fk/tables_fk.control b/src/test/modules/tables_fk/tables_fk.control

Isn't a full test with a separate initdb, create extension etc. a really
heavyhanded way to test this? I mean that's a test where the setup takes
up to 10s, whereas the actual runtime is in the millisecond range?

Adding tests in this manner doesn't seem scalable to me.

I think we should rather add *one* test that does dump/restore over the
normal regression test database. Something similar to the pg_upgrade
tests. And then work at adding more content to the regression test
database - potentially sourcing from src/test/modules.

Greetings,

Andres Freund

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Andres Freund wrote:

Isn't a full test with a separate initdb, create extension etc. a really
heavyhanded way to test this? I mean that's a test where the setup takes
up to 10s, whereas the actual runtime is in the millisecond range?

I spent some time looking over this patch yesterday, and that was one
concern I had too. I was thinking that if we turn this into a single
module that can contain several extensions, or several pg_dump-related
tests, and then test multiple features without requiring an initdb for
each, it would be more palatable.

Adding tests in this manner doesn't seem scalable to me.

I was thinking in having this be renamed src/test/modules/extensions/
and then the extension contained here would be renamed ext001_fk_tables
or something like that; later we could ext002_something for testing some
other angle of extensions, not necessarily pg_dump related.

I think we should rather add *one* test that does dump/restore over the
normal regression test database. Something similar to the pg_upgrade
tests. And then work at adding more content to the regression test
database - potentially sourcing from src/test/modules.

That's another option, but we've had this idea for many years now and it
hasn't materialized. As I recall, Andrew Dunstan has a module that
tests cross-version pg_upgrade and one thing he does is dump both and
compare; the problem is that there are differences, so he keeps a count
of how many lines he expect to differ between any two releases. Or
something like that. While it's a good enough start, I don't think it's
robust enough to be in core. How would we do it?

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2015-09-02 14:30:33 -0300, Alvaro Herrera wrote:

I was thinking in having this be renamed src/test/modules/extensions/
and then the extension contained here would be renamed ext001_fk_tables
or something like that; later we could ext002_something for testing some
other angle of extensions, not necessarily pg_dump related.

The largest dataset we have for this is the current regression test
database, it seems a waste not to include it...

That's another option, but we've had this idea for many years now and it
hasn't materialized.

But that's just minimally more complex than what's currently done in
that test and in the pg_upgrade test?

As I recall, Andrew Dunstan has a module that
tests cross-version pg_upgrade and one thing he does is dump both and
compare; the problem is that there are differences, so he keeps a count
of how many lines he expect to differ between any two releases.

I'm not suggesting to do anything cross-release - that'd indeed be
another ballpark.

Just that instead of a pg_dump test that tests some individual things we
have one that tests the whole regression test output and then does a
diff?

Greetings,

Andres Freund

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Andres Freund wrote:

As I recall, Andrew Dunstan has a module that
tests cross-version pg_upgrade and one thing he does is dump both and
compare; the problem is that there are differences, so he keeps a count
of how many lines he expect to differ between any two releases.

I'm not suggesting to do anything cross-release - that'd indeed be
another ballpark.

Ah, you're right.

Just that instead of a pg_dump test that tests some individual things we
have one that tests the whole regression test output and then does a
diff?

Sorry if I'm slow -- A diff against what?

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Sep 3, 2015 at 2:30 AM, Alvaro Herrera wrote:

Andres Freund wrote:

Isn't a full test with a separate initdb, create extension etc. a really
heavyhanded way to test this? I mean that's a test where the setup takes
up to 10s, whereas the actual runtime is in the millisecond range?

I spent some time looking over this patch yesterday, and that was one
concern I had too. I was thinking that if we turn this into a single
module that can contain several extensions, or several pg_dump-related
tests, and then test multiple features without requiring an initdb for
each, it would be more palatable.

Yeah sure. That's a statement you could generalize for all the TAP
tests, per se for example pg_rewind tests.

Adding tests in this manner doesn't seem scalable to me.

I was thinking in having this be renamed src/test/modules/extensions/
and then the extension contained here would be renamed ext001_fk_tables
or something like that; later we could ext002_something for testing some
other angle of extensions, not necessarily pg_dump related.

So, if I am following here correctly, what you are suggesting is:
1) create src/test/modules/extensions/, and include fk_tables in it for now
2) offer the possibility to run make check in this path, doing
complicated tests on a single instance, pg_dump on an instance is one
of them.
Note that for now the TAP test machinery does not allow to share a
single cluster instance across multiple pl scripts, and it seems to me
that's what you are meaning here (in my opinion that's out of sight of
this patch, and I don't think either we should do it this will make
error handling a nightmare). And we can now have a single TAP script
doing all the tests on a single instance. So which one are you
suggesting?

The patch proposed here is actually doing the second, initializing an
instance and performing pg_dump on it. If we change it your way we
should just rename the test script to something like
001_test_extensions.pl and mention in it that new tests for extensions
should be included in it. Still I would imagine that each extension
are actually going to need slightly differ test patterns to work or
cover what they are intended to cover. See for example the set of
tests added in the CREATE EXTENSION CASCADE patch: those are not
adapted to run with the TAP machinery. I may be of course wrong, we
may have other extensions in the future using that, still I am seeing
none around for now.

If you are worrying about run time, we could as well have make check
add an EXTRA_INSTALL pointing to src/test/modules/extensions/, with an
additional test, say extensions.sql that creates data based on it.
Then we let pg_upgrade do the dump/restore checks. This way the run
time of make check-world is minimally impacted, and the code paths we
want tested are done by the buildfarm. That's ugly, still it won't
impact performance.

I think we should rather add *one* test that does dump/restore over the
normal regression test database. Something similar to the pg_upgrade
tests. And then work at adding more content to the regression test
database - potentially sourcing from src/test/modules.

That's another option, but we've had this idea for many years now and it
hasn't materialized. As I recall, Andrew Dunstan has a module that
tests cross-version pg_upgrade and one thing he does is dump both and
compare; the problem is that there are differences, so he keeps a count
of how many lines he expect to differ between any two releases. Or
something like that. While it's a good enough start, I don't think it's
robust enough to be in core. How would we do it?

That does not seem plausible to me for an integration in core, you
would need to keep switching between branches in a given git tree if
the test is being done within a git repository, and that would be just
not doable from a raw tarball which just stores one given version.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Sep 3, 2015 at 12:38 AM, Andres Freund wrote:

Isn't a full test with a separate initdb, create extension etc. a really
heavyhanded way to test this? I mean that's a test where the setup takes
up to 10s, whereas the actual runtime is in the millisecond range?

Adding tests in this manner doesn't seem scalable to me.

How to include such kind of tests is in the heart of the discussion
since this patch has showed up. I think we are now close to 5
different opinions with 4 different hackers on the matter, the Riggs'
theorem applies nicely.

I think we should rather add *one* test that does dump/restore over the
normal regression test database. Something similar to the pg_upgrade
tests. And then work at adding more content to the regression test
database - potentially sourcing from src/test/modules.

If you are worrying about run time, pg_upgrade already exactly does
that. What would be the point to double the amount of time to do the
same thing in two different places?
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Sep 3, 2015 at 10:35 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Thu, Sep 3, 2015 at 12:38 AM, Andres Freund wrote:

Isn't a full test with a separate initdb, create extension etc. a really
heavyhanded way to test this? I mean that's a test where the setup takes
up to 10s, whereas the actual runtime is in the millisecond range?

Adding tests in this manner doesn't seem scalable to me.

How to include such kind of tests is in the heart of the discussion
since this patch has showed up. I think we are now close to 5
different opinions with 4 different hackers on the matter, the Riggs'
theorem applies nicely.

I think we should rather add *one* test that does dump/restore over the
normal regression test database. Something similar to the pg_upgrade
tests. And then work at adding more content to the regression test
database - potentially sourcing from src/test/modules.

If you are worrying about run time, pg_upgrade already exactly does
that. What would be the point to double the amount of time to do the
same thing in two different places?

So, to summarize the state of this patch whose status is now "Waiting
on Author", we have the following possibilities:
1) Keep the test as-is, as an independent test of src/test/modules.
2) Integrate it in the test suite of src/test/regress and let
pg_upgrade make the work with dump/restore.
3) Create a new facility as src/test/modules/extensions that does a
run of the regression test suite with a set of extensions, and then a
dump/restore. For now the only extension added in the installation is
tables_fk. Future ones would be added here as well, though it is not
clear to me what they are and if we'd have some (there may be... Or
not).
4) Include it as a test of pg_dump in src/bin/pg_dump/t, with the
extension located in for example t/extensions and extend prove_check
to install as well any extensions in t/extensions.

I would still go for 1), the infrastructures included by the other
proposals may become useful depending on the types of tests that are
added in the future, but it is still unclear what those tests are, and
they may even need something else than what listed here (see that as
an example /messages/by-id/54F62C3F.8070702@gmx.net)
to run properly.
Regards,
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2015-09-07 22:55:50 +0900, Michael Paquier wrote:

So, to summarize the state of this patch whose status is now "Waiting
on Author", we have the following possibilities:
1) Keep the test as-is, as an independent test of src/test/modules.

I find that a bad option. A test that takes this long and has that much
setup for such a marginal amount of coverage just doesn't seem worth it.

2) Integrate it in the test suite of src/test/regress and let
pg_upgrade make the work with dump/restore.

2b) ... and create a single src/test/modules/pg_dumprestore test that
initdbs, runs installcheck, dumps, loads and compares a repated dump.

I think 2b) is by far the best choice.

I would still go for 1), the infrastructures included by the other
proposals may become useful depending on the types of tests that are
added in the future, but it is still unclear what those tests are, and
they may even need something else than what listed here (see that as
an example /messages/by-id/54F62C3F.8070702@gmx.net)
to run properly.

By that argument we're going to add more and more isolated tests and
never merge anything.

I fail to see a single benefit of this approach.

Greetings,

Andres Freund

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Sep 2, 2015 at 02:30:33PM -0300, Alvaro Herrera wrote:

I think we should rather add *one* test that does dump/restore over the
normal regression test database. Something similar to the pg_upgrade
tests. And then work at adding more content to the regression test
database - potentially sourcing from src/test/modules.

That's another option, but we've had this idea for many years now and it
hasn't materialized. As I recall, Andrew Dunstan has a module that
tests cross-version pg_upgrade and one thing he does is dump both and
compare; the problem is that there are differences, so he keeps a count
of how many lines he expect to differ between any two releases. Or
something like that. While it's a good enough start, I don't think it's
robust enough to be in core. How would we do it?

I have shell scripts that test pg_dump restore/upgrade of every
supported PG version. I also have expected pg_dump output files for
every major version. This is explained in src/bin/pg_upgrade/TESTING.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Sep 9, 2015 at 3:33 AM, Andres Freund <andres@anarazel.de> wrote:

On 2015-09-07 22:55:50 +0900, Michael Paquier wrote:

So, to summarize the state of this patch whose status is now "Waiting
on Author", we have the following possibilities:
1) Keep the test as-is, as an independent test of src/test/modules.

I find that a bad option. A test that takes this long and has that much
setup for such a marginal amount of coverage just doesn't seem worth it.

2) Integrate it in the test suite of src/test/regress and let
pg_upgrade make the work with dump/restore.

2b) ... and create a single src/test/modules/pg_dumprestore test that
initdbs, runs installcheck, dumps, loads and compares a repated dump.

I think 2b) is by far the best choice.

And I guess that the extensions tested should be located directly in
this path, or we would need again to tweak the MSVC scripts...
Attached is an attempt to solve things by converting the previous
patch as proposed by Andres. A source and a target databases are used
on a cluster, and their respective dumps are compared with
File::Compare (available down to perl 5.8.8) at the end.
Regards,
--
Michael

On 2015-09-09 10:48:24 +0900, Michael Paquier wrote:

On Wed, Sep 9, 2015 at 3:33 AM, Andres Freund <andres@anarazel.de> wrote:

On 2015-09-07 22:55:50 +0900, Michael Paquier wrote:

So, to summarize the state of this patch whose status is now "Waiting
on Author", we have the following possibilities:
1) Keep the test as-is, as an independent test of src/test/modules.

I find that a bad option. A test that takes this long and has that much
setup for such a marginal amount of coverage just doesn't seem worth it.

2) Integrate it in the test suite of src/test/regress and let
pg_upgrade make the work with dump/restore.

2b) ... and create a single src/test/modules/pg_dumprestore test that
initdbs, runs installcheck, dumps, loads and compares a repated dump.

I think 2b) is by far the best choice.

And I guess that the extensions tested should be located directly in
this path, or we would need again to tweak the MSVC scripts...
Attached is an attempt to solve things by converting the previous
patch as proposed by Andres. A source and a target databases are used
on a cluster, and their respective dumps are compared with
File::Compare (available down to perl 5.8.8) at the end.

Unless I miss something this isn't 2b) as it does *not* actually run
the actual installcheck. Therefore the actual pg_dump/restore coverage
is neglegible.

Greetings,

Andres Freund

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers