pg_rewind tests

On Tue, Mar 31, 2015 at 9:59 AM, Peter Eisentraut <peter_e@gmx.net> wrote:

There are some small issues with the pg_rewind tests.

This technique

check: all
$(prove_check) :: local
$(prove_check) :: remote

for passing arguments to "prove" does not work with the tools included
in Perl 5.8.

While sorting out the portability issues in the TAP framework during the
9.4 release cycle, we had set 5.8 as the oldest Perl version that is
supported. (It's the Perl version in RHEL 5.) I suggest using
environment variables instead, unless we want to change that.

That's good to know. And I think that by using environment variables it is
necessary to pass an additional flag to prove_check (see
PG_PROVE_TEST_MODE) in the patch attached because prove_check kicks several
instructions, a command mkdir to begin with.

Moreover,

if ($test_mode == "local")
...
elsif ($test_mode == "remote")

don't work, because those are numerical comparisons, not string
comparisons. So the remote branch is never actually run.

That's "eq" I guess.

Finally, RewindTest.pm should use

use strict;
use warnings;

and the warnings caused by that should be addressed.

All those things addressed give more or less the patch attached.

While looking at that I noticed two additional issues:
- In remote mode, the connection string to the promoted standby was
incorrect when running pg_rewind, leading to connection errors
- At least in my environment, a sleep of 1 after the standby promotion was
not sufficient to make the tests work.
Regards,
--
Michael

On Tue, Mar 31, 2015 at 4:37 PM, Michael Paquier <michael.paquier@gmail.com>
wrote:

While looking at that I noticed two additional issues:
- In remote mode, the connection string to the promoted standby was
incorrect when running pg_rewind, leading to connection errors
- At least in my environment, a sleep of 1 after the standby promotion was
not sufficient to make the tests work.

While working on another patch for TAP tests, I noticed that relying on
environment variables to run tests is a bad idea as well, as other tests do
not do it, so instead is a patch that refactors the tests so as they do not
use environment variables and so as it is not necessary to pass arguments
to prove.
The trick is to use sub-routines in each test, and invoke this subroutine
for both 'local' and 'remote'. This changes the order the tests are run,
but I guess that's not a big deal as long as the tests are run, and this
approach looks more solid to me as it makes pg_rewind's tests more
consistent with the rest. The log files of each test are still separated
the same way as before.
Regards,
--
Michael

Peter Eisentraut wrote:

There are some small issues with the pg_rewind tests.

Do these tests work in VPATH builds? I now get a failure in "make
check-world" (both with and without Michael's patch). "make check" in
src/bin/pg_rewind dies with the output below. If I change "./pg_rewind"
to "pg_rewind", tests pass, but I wonder if the full path to the
temp-install bindir should be used instead.

make -C ../../.. DESTDIR='/home/alvherre/Code/pgsql/build/master/src/bin/pg_rewind'/tmp_check/install install >'/home/alvherre/Code/pgsql/build/master/src/bin/pg_rewind'/tmp_check/log/install.log 2>&1
cd /pgsql/source/master/src/bin/pg_rewind && TESTDIR='/home/alvherre/Code/pgsql/build/master/src/bin/pg_rewind' PATH="/home/alvherre/Code/pgsql/build/master/src/bin/pg_rewind/tmp_check/install/pgsql/install/master/bin:$PATH" LD_LIBRARY_PATH="/home/alvherre/Code/pgsql/build/master/src/bin/pg_rewind/tmp_check/install/pgsql/install/master/lib" top_builddir='/home/alvherre/Code/pgsql/build/master/src/bin/pg_rewind/../../..' PGPORT='655432' prove -I /pgsql/source/master/src/test/perl/ --verbose t/*.pl :: local
t/001_basic.pl .......
1..4
file not found: ./pg_rewind at RewindTest.pm line 220.
# Looks like your test exited with 2 before it could output anything.
Dubious, test returned 2 (wstat 512, 0x200)
Failed 4/4 subtests
t/002_databases.pl ...
1..2
file not found: ./pg_rewind at RewindTest.pm line 220.
# Looks like your test exited with 2 before it could output anything.
Dubious, test returned 2 (wstat 512, 0x200)
Failed 2/2 subtests
t/003_extrafiles.pl ..
1..2
file not found: ./pg_rewind at RewindTest.pm line 220.
# Looks like your test exited with 2 before it could output anything.
Dubious, test returned 2 (wstat 512, 0x200)
Failed 2/2 subtests

Test Summary Report
-------------------
t/001_basic.pl (Wstat: 512 Tests: 0 Failed: 0)
Non-zero exit status: 2
Parse errors: Bad plan. You planned 4 tests but ran 0.
t/002_databases.pl (Wstat: 512 Tests: 0 Failed: 0)
Non-zero exit status: 2
Parse errors: Bad plan. You planned 2 tests but ran 0.
t/003_extrafiles.pl (Wstat: 512 Tests: 0 Failed: 0)
Non-zero exit status: 2
Parse errors: Bad plan. You planned 2 tests but ran 0.
Files=3, Tests=0, 26 wallclock secs ( 0.02 usr 0.00 sys + 4.41 cusr 0.66 csys = 5.09 CPU)
Result: FAIL
Makefile:51: recipe for target 'check' failed
make: *** [check] Error 1

--
�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 04/10/2015 04:01 PM, Alvaro Herrera wrote:

Do these tests work in VPATH builds? I now get a failure in "make
check-world" (both with and without Michael's patch). "make check" in
src/bin/pg_rewind dies with the output below. If I change "./pg_rewind"
to "pg_rewind", tests pass, but I wonder if the full path to the
temp-install bindir should be used instead.

Just "pg_rewind" seems correct, that's what all the other TAP tests do.
The Makefile target that calls the perl script sets PATH to point to the
temporary installation's bin dir. Fixed.

- 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 04/02/2015 04:56 AM, Michael Paquier wrote:

On Tue, Mar 31, 2015 at 4:37 PM, Michael Paquier <michael.paquier@gmail.com>
wrote:

While looking at that I noticed two additional issues:
- In remote mode, the connection string to the promoted standby was
incorrect when running pg_rewind, leading to connection errors
- At least in my environment, a sleep of 1 after the standby promotion was
not sufficient to make the tests work.

While working on another patch for TAP tests, I noticed that relying on
environment variables to run tests is a bad idea as well, as other tests do
not do it, so instead is a patch that refactors the tests so as they do not
use environment variables and so as it is not necessary to pass arguments
to prove.
The trick is to use sub-routines in each test, and invoke this subroutine
for both 'local' and 'remote'. This changes the order the tests are run,
but I guess that's not a big deal as long as the tests are run, and this
approach looks more solid to me as it makes pg_rewind's tests more
consistent with the rest. The log files of each test are still separated
the same way as before.

Applied, thanks!

- 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 Tue, Apr 14, 2015 at 12:32 AM, Heikki Linnakangas wrote:

Applied, thanks!

Thanks. Now, for Windows...
--
Michael

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