Opened 9 years ago

Closed 9 years ago

#3444 closed enhancement (fixed)

Add dump/restore testing

Reported by: strk Owned by: robe
Priority: medium Milestone: PostGIS 2.3.0
Component: QA/buildbots Version: 2.2.x
Keywords: Cc:

Description

It may be useful to add dump/restore testing to the build bots, see #3443 and #3430

Change History (31)

comment:1 by robe, 9 years ago

Milestone: PostGIS FuturePostGIS 2.3.0
Version: 2.2.x

Let's be optimistic.

comment:2 by strk, 9 years ago

I've a local script that does the testing, but I'm thinking it would be best done with an additional switch in the run_test.pl one. Like --dumprestore ? So it could be mixed with both EXTENSION and non-EXTENSION based and with --upgrade etc.

comment:3 by strk, 9 years ago

(In [14651]) Add --dumprestore switch to ./run_test.pl

See #3444

comment:4 by strk, 9 years ago

r14651 in trunk (2.3.0dev) adds --dumprestore switch to run_test.pl. Now it should be added to the testing calls...

comment:5 by strk, 9 years ago

The limit is that with --extension --dumprestore it is not predictable about which version of PostGIS is restored because pg_dump simply includes a "CREATE EXTENSION POSTGIS" in the dump, which means the version of postgis being restored depends on which version of PostGIS was installed last.

comment:6 by strk, 9 years ago

(In [14652]) Add --dumprestore switch to ./run_test.pl

See #3444

comment:7 by strk, 9 years ago

Running "make check RUNTESTFLAGS=--dumprestore" currently fails due to unexpected order in restored tables (I think). Like:

--- loader/PointZ-w.select.expected     2015-08-07 18:03:05.367654370 +0200
+++ /tmp/pgis_reg/test_3_out    2016-02-09 17:27:15.740751528 +0100
@@ -1,3 +1,3 @@
 POINT(0 1 2 3)
-POINT(9 -1 -2 -3)
 POINT(9 -1 -20 -123)
+POINT(9 -1 -2 -3)

comment:8 by strk, 9 years ago

(In [14653]) Do not rely on storage order on querying loaded shapefiles

Fixes runs with RUNTESTFLAGS=--dumprestore See #3444

comment:9 by strk, 9 years ago

Next issue: topology --dumprestore fails:

--- regress/legacy_validate_expected    2015-08-07 18:04:39.568169336 +0200
+++ /tmp/pgis_reg/test_1_out    2016-02-09 17:43:33.022704739 +0100
@@ -7,6 +7,6 @@
 Topology validation errors follow:
 End of topology validation errors
 Topology 'city_data' dropped
-t
-Empty topology errors|0
-Topology 'tt' dropped
+ERROR:  function createtopology(unknown) does not exist at character 8
+ERROR:  function validatetopology(unknown) does not exist at character 47
+ERROR:  function droptopology(unknown) does not exist at character 8

comment:10 by strk, 9 years ago

(In [14654]) Do not rely on storage order on querying loaded shapefiles

Fixes runs with RUNTESTFLAGS=--dumprestore See #3444

comment:11 by strk, 9 years ago

I confirm this fails:

cd topology/test
../../regress/run_test.pl -v --topology --dumprestore regress/legacy_validate.sql

Omitting the --dumprestore switch the test succeeds

comment:12 by strk, 9 years ago

Got it: custom topology search_path is lost on dump/reload !

comment:13 by strk, 9 years ago

Filed #3454 for the search_path loss

comment:14 by strk, 9 years ago

With search_path tweaked by run_test.pl (r14656 and r14655) there are still failures in topology, which I believe have to do with tuples order. It's surprising how a dump/reload can change order of things, despite it happening _before_ loading any data !

../../regress/run_test.pl --dumprestore --topology -v regress/st_remedgemodface.sql
...
Dumping and restoring database 'postgis_reg'
PostgreSQL 9.3.6 on x86_64-unknown-linux-gnu, compiled by gcc (Ubuntu 4.8.2-19ubuntu1) 4.8.2, 64-bit
  Postgis 2.3.0dev - r14654 - 2016-02-09 12:02:08
  scripts 2.3.0dev r14654
  GEOS: 3.6.0dev-CAPI-1.10.0 r4138
  PROJ: Rel. 4.8.0, 6 March 2012

Running tests

 regress/st_remedgemodface .. failed (diff expected obtained: /tmp/pgis_reg/test_1_diff)
-----------------------------------------------------------------------------
--- regress/st_remedgemodface_expected  2015-08-07 18:04:52.984242613 +0200
+++ /tmp/pgis_reg/test_1_out    2016-02-09 19:27:37.319120020 +0100
@@ -10,175 +10,175 @@
 ERROR:  SQL/MM Spatial exception - non-existent edge 0
 ERROR:  SQL/MM Spatial exception - non-existent edge 143
 RM(25)|1
-RM(25)/nodes|+|21|1
 RM(25)/nodes|-|21|
-RM(25)/nodes|+|22|1
+RM(25)/nodes|+|21|1
 RM(25)/nodes|-|22|
+RM(25)/nodes|+|22|1
 RM(25)/edges|-|25|-25|25|1|1
 RM(4)|0
-RM(4)/nodes|+|5|0
 RM(4)/nodes|-|5|
+RM(4)/nodes|+|5|0
 RM(4)/edges|-|4|-5|4|0|0
-RM(4)/edges|+|5|-5|5|0|0
 RM(4)/edges|-|5|-4|5|0|0
+RM(4)/edges|+|5|-5|5|0|0
 RM(26)|1
-RM(26)/nodes|+|20|1
 RM(26)/nodes|-|20|
+RM(26)/nodes|+|20|1
 RM(26)/edges|-|26|26|-26|9|1
 RM(26)/faces|-|9|SRID=4326;POLYGON((4 31,4 34,7 34,7 31,4 31))
...

Dropping the --dumprestore switch makes the test succeed

comment:15 by strk, 9 years ago

The problem is with ORDER BY getting different order. With --dumprestore:

RM(25)/nodes|-|21|                                                              
RM(25)/nodes|+|21|1                                                             
RM(25)/nodes|-|22|                                                              
RM(25)/nodes|+|22|1   

Without:

RM(25)/nodes|+|21|1                                                             
RM(25)/nodes|-|21|                                                              
RM(25)/nodes|+|22|1                                                             
RM(25)/nodes|-|22|          

The query is (in a patch of mine):

SELECT * FROM check_nodes('RM(25)/nodes') ORDER BY 3,2;

comment:16 by strk, 9 years ago

Figured, with help from lluad on irc. Collation or character set is different before and after the dump/reload too ! select '-' > '+'; is false after dump/reload, true before.

comment:17 by strk, 9 years ago

(In [14657]) Ensure restored database is created the same as the initial one

Fixes ORDER BY consistency, probably also with the loader tests already tweaked to workaround this

See #3444

comment:18 by strk, 9 years ago

(In [14658]) Ensure restored database is created the same as the initial one

Fixes ORDER BY consistency, probably also with the loader tests already tweaked to workaround this

See #3444

comment:19 by strk, 9 years ago

As of r14657 (trunk) and r14658 (2.2), make check RUNTESTFLAGS="-v --dumprestore" completes successfully for me. Sounds about time to add the flag to the bots build line.

comment:20 by robe, 9 years ago

Can you test this on travis?

comment:21 by strk, 9 years ago

I pushed a "travis" branch on my github fork, but can't access the proprietary github GUI from this system so cannot file a PR for travis to pickup. Pretty annoying.

comment:22 by robe, 9 years ago

YOu this be separate from the extension tests e.g. right now bots have for extension testing

 make check RUNTESTFLAGS="--extension -v"

Should I change to

make check RUNTESTFLAGS="--extension -v --dumprestore"

or the dumprestore doesn't rely on extension?

comment:23 by strk, 9 years ago

--dumprestore simply adds a pg_dump and a pg_restore step to the initialization phase, so should work both with extension and with script.

The only difference is that when restoring with extension you cannot guarantee what version will be restored, because the dump will contain a "CREATE EXTENSION postgis" with no version specification, so the outcome will depend on what's the default postgis version according to postgis.control.

comment:24 by robe, 9 years ago

(In [14722]) option for debbbie to do dump restore - variable set in jenkins references #3444

comment:25 by robe, 9 years ago

okay have debbie set to dump restore. I just have two separate. So she's doing make install, run extension test and then dumprestore.

I saw a dumprestoring message so assume it's working:

http://debbie.postgis.net:8080/job/PostGIS_Regress/3265/consoleFull

I'll close this out after have done the same on winnie.

comment:26 by robe, 9 years ago

Resolution: fixed
Status: newclosed

(In [14723]) option for winnie to do dump restore - variable set in jenkins closes #3444

comment:27 by strk, 9 years ago

Resolution: fixed
Status: closedreopened

Reopening to close after travis also tests, see https://github.com/postgis/postgis/pull/101

comment:28 by strk, 9 years ago

and we shouldn't forget gitlab-ci either...

comment:29 by strk, 9 years ago

Regina check the travis side (PR above), you'll notice --dumprestore is used both with "make check" _and_ with "make installcheck". The former tests _scripts_, the latter tests _extension_.

Only the "installcheck" rule (with extension) needs a previous "install" and is affected by it.

comment:30 by strk, 9 years ago

(In [14726]) Add dump-restore testing for Travis

See #3444

Runs dump-restore tests for both scripts and extension.

comment:31 by strk, 9 years ago

Resolution: fixed
Status: reopenedclosed

(In [14727]) Add dump-restore testing for gitlab-ci

Closes #3444

Runs dump-restore tests for both scripts and extension.

Note: See TracTickets for help on using tickets.