Hello, I'm studying this. Two hunks in 0003 needed a fix but the other part applied cleanly on master.
At Fri, 22 Jan 2016 15:17:51 +0900, Michael Paquier <michael.paqu...@gmail.com> wrote in <CAB7nPqTTAtVCEXAoyMtF4Xu9g=mXY4cjnP=+hy7jgYfnFzM=j...@mail.gmail.com> > On Mon, Dec 21, 2015 at 4:45 PM, Michael Paquier > <michael.paqu...@gmail.com> wrote: > > As this thread is stalling a bit, please find attached a series of > > patch gathering all the pending issues for this thread: > > - 0001, fix config_default.pl for MSVC builds to take into account TAP tests > > - 0002, append a node name in get_new_node (per Noah's request) > > - 0003, the actual recovery test suite > > Hopefully this facilitates future reviews. > > Patch 2 has been pushed as c8642d9 (thanks Alvaro). The remaining two > patches still apply and pass cleanly. The TAP test framework doesn't remove existing temporary directories when a test script suite (or a prove?) starts, and it in turn removes all temprorary directories it has made even if ended with fairures. It would be sometimes inconvenient to find the cause of the failures and inconsistent with the behavior of the ordinary(?) make check, as far as my understanding goes. tmp_check is left remained but it would be ok to preserve logs, which is located in tmp_check differently than the ordinary regressions. One annoyance is the name of data directories is totally meaningless. We cannot investigate them even if it is left behind. Addition to them, maybe it is useful that a test script can get stderr content from PostgresNode->psql(). Setting client_min_messages lower can give a plenty of useful information about how server is working. So, I'd like to propose four (or five) changes to this harness. - prove_check to remove all in tmp_check - TestLib to preserve temporary directories/files if the current test fails. - PostgresNode::get_new_node to create data directory with meaningful basenames. - PostgresNode::psql to return a list of ($stdout, $stderr) if requested. (The previous behavior is not changed) - (recovery/t/00x_* gives test number to node name) As a POC, the attached diff will appliy on the 0001 and (fixed) 0003 patches. It might be good to give test number to the name of temp dirs by any automated way, but it is not included in it. Opinions? Thoughts? regards, -- Kyotaro Horiguchi NTT Open Source Software Center
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index 4602e3e..75ddcaf 100644 --- a/src/test/perl/PostgresNode.pm +++ b/src/test/perl/PostgresNode.pm @@ -51,7 +51,7 @@ sub new my $self = { _port => $pgport, _host => $pghost, - _basedir => TestLib::tempdir, + _basedir => TestLib::tempdir($name), _name => $name, _logfile => "$TestLib::log_path/${testname}_${name}.log" }; @@ -513,10 +513,16 @@ sub psql print "#### Begin standard error\n"; print $stderr; print "#### End standard error\n"; + if (wantarray) + { + chomp $stderr; + $stderr =~ s/\r//g if $Config{osname} eq 'msys'; + } } chomp $stdout; $stdout =~ s/\r//g if $Config{osname} eq 'msys'; - return $stdout; + + return wantarray ? ($stdout, $stderr) : $stdout; } # Run a query once a second, until it returns 't' (i.e. SQL boolean true). diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm index 3d11cbb..841dc06 100644 --- a/src/test/perl/TestLib.pm +++ b/src/test/perl/TestLib.pm @@ -107,13 +107,24 @@ INIT autoflush TESTLOG 1; } +END +{ + # Preserve temporary directory for this test if failure + if (!Test::More->builder->is_passing) + { + $File::Temp::KEEP_ALL = 1; + } +} + # # Helper functions # sub tempdir { + my ($prefix) = @_; + $prefix = "tmp_test" if (!$prefix); return File::Temp::tempdir( - 'tmp_testXXXX', + $prefix.'_XXXX', DIR => $tmp_check, CLEANUP => 1); } diff --git a/src/test/recovery/t/001_stream_rep.pl b/src/test/recovery/t/001_stream_rep.pl index 3ed9be3..8a00d00 100644 --- a/src/test/recovery/t/001_stream_rep.pl +++ b/src/test/recovery/t/001_stream_rep.pl @@ -6,16 +6,18 @@ use TestLib; use Test::More tests => 4; # Initialize master node -my $node_master = get_new_node('master'); +my $node_master = get_new_node('001_master'); $node_master->init(allows_streaming => 1); $node_master->start; my $backup_name = 'my_backup'; +$File::Temp::KEEP_ALL = 0; + # Take backup $node_master->backup($backup_name); # Create streaming standby linking to master -my $node_standby_1 = get_new_node('standby_1'); +my $node_standby_1 = get_new_node('001_standby_1'); $node_standby_1->init_from_backup($node_master, $backup_name, has_streaming => 1); $node_standby_1->start; @@ -25,7 +27,7 @@ $node_standby_1->start; $node_standby_1->backup($backup_name); # Create second standby node linking to standby 1 -my $node_standby_2 = get_new_node('standby_2'); +my $node_standby_2 = get_new_node('001_standby_2'); $node_standby_2->init_from_backup($node_standby_1, $backup_name, has_streaming => 1); $node_standby_2->start; @@ -43,11 +45,11 @@ $caughtup_query = "SELECT pg_last_xlog_replay_location() = write_location FROM p $node_standby_1->poll_query_until('postgres', $caughtup_query) or die "Timed out while waiting for standby 2 to catch up"; -my $result = $node_standby_1->psql('postgres', "SELECT count(*) FROM tab_int"); +my $result = $node_standby_1->psql('postgres', "set client_min_messages to debug5;SELECT count(*) FROM tab_int"); print "standby 1: $result\n"; -is($result, qq(1002), 'check streamed content on standby 1'); +isnt($result[0], qq(1002), 'check streamed content on standby 1'); -$result = $node_standby_2->psql('postgres', "SELECT count(*) FROM tab_int"); +my $result = $node_standby_2->psql('postgres', "SELECT count(*) FROM tab_int"); print "standby 2: $result\n"; is($result, qq(1002), 'check streamed content on standby 2'); diff --git a/src/test/recovery/t/002_archiving.pl b/src/test/recovery/t/002_archiving.pl index 930125c..bb620c6 100644 --- a/src/test/recovery/t/002_archiving.pl +++ b/src/test/recovery/t/002_archiving.pl @@ -7,7 +7,7 @@ use Test::More tests => 1; use File::Copy; # Initialize master node, doing archives -my $node_master = get_new_node('master'); +my $node_master = get_new_node('002_master'); $node_master->init(has_archiving => 1, allows_streaming => 1); my $backup_name = 'my_backup'; @@ -19,7 +19,7 @@ $node_master->start; $node_master->backup($backup_name); # Initialize standby node from backup, fetching WAL from archives -my $node_standby = get_new_node('standby'); +my $node_standby = get_new_node('002_standby'); $node_standby->init_from_backup($node_master, $backup_name, has_restoring => 1); $node_standby->append_conf('postgresql.conf', qq( diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl index 293603a..4d59277 100644 --- a/src/test/recovery/t/003_recovery_targets.pl +++ b/src/test/recovery/t/003_recovery_targets.pl @@ -16,7 +16,7 @@ sub test_recovery_standby my $num_rows = shift; my $until_lsn = shift; - my $node_standby = get_new_node($node_name); + my $node_standby = get_new_node("003_".$node_name); $node_standby->init_from_backup($node_master, 'my_backup', has_restoring => 1); @@ -43,7 +43,7 @@ sub test_recovery_standby } # Initialize master node -my $node_master = get_new_node('master'); +my $node_master = get_new_node('003_master'); $node_master->init(has_archiving => 1, allows_streaming => 1); # Start it diff --git a/src/test/recovery/t/004_timeline_switch.pl b/src/test/recovery/t/004_timeline_switch.pl index c58c602..0a2362b 100644 --- a/src/test/recovery/t/004_timeline_switch.pl +++ b/src/test/recovery/t/004_timeline_switch.pl @@ -11,7 +11,7 @@ use Test::More tests => 1; $ENV{PGDATABASE} = 'postgres'; # Initialize master node -my $node_master = get_new_node('master'); +my $node_master = get_new_node('004_master'); $node_master->init(allows_streaming => 1); $node_master->start; @@ -20,11 +20,11 @@ my $backup_name = 'my_backup'; $node_master->backup($backup_name); # Create two standbys linking to it -my $node_standby_1 = get_new_node('standby_1'); +my $node_standby_1 = get_new_node('004_standby_1'); $node_standby_1->init_from_backup($node_master, $backup_name, has_streaming => 1); $node_standby_1->start; -my $node_standby_2 = get_new_node('standby_2'); +my $node_standby_2 = get_new_node('004_standby_2'); $node_standby_2->init_from_backup($node_master, $backup_name, has_streaming => 1); $node_standby_2->start; diff --git a/src/test/recovery/t/005_replay_delay.pl b/src/test/recovery/t/005_replay_delay.pl index 14d9b29..ae209e4 100644 --- a/src/test/recovery/t/005_replay_delay.pl +++ b/src/test/recovery/t/005_replay_delay.pl @@ -6,7 +6,7 @@ use TestLib; use Test::More tests => 2; # Initialize master node -my $node_master = get_new_node(); +my $node_master = get_new_node('005_master'); $node_master->init(allows_streaming => 1); $node_master->start; @@ -18,7 +18,7 @@ my $backup_name = 'my_backup'; $node_master->backup($backup_name); # Create streaming standby from backup -my $node_standby = get_new_node(); +my $node_standby = get_new_node('005_standby'); $node_standby->init_from_backup($node_master, $backup_name, has_streaming => 1); $node_standby->append_conf('recovery.conf', qq(
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers