Hello, At Fri, 26 Feb 2016 15:43:14 -0300, Alvaro Herrera <alvhe...@2ndquadrant.com> wrote in <20160226184314.GA205945@alvherre.pgsql> > Kyotaro HORIGUCHI wrote: > > > 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. > > These changes all seem very reasonable to me. I'm not so sure about the > last one. Perhaps the framework ought to generate an appropriate subdir > name using the test file name plus the node name, so that instead of > tmp_XXXX it becomes tmp_001_master_XXXX or something like that? Having > be a coding convention doesn't look real nice to me.
Thank you for mentioning this. Sorry, the last one accidentally contained garbage code to intentionally raise an error. The last one names the nodes as such like '001_master_24f8'. Maybe prefixing "tmp_" would be better. > I didn't try to apply your patch but I'm fairly certain it would > conflict with what's here now; can you please rebase and resend? This was a very small patch made on the old, uncommited patches. I remade the patch and split it into two parts. 0001-Change-behavior-... Changes of PostmasterNode.pm and TestLib.pm to add some features and change a behavior. 0002-Prefix-test-numbers-to-node- This is rather a example usage of 0001- patch (except for stderr stuff). 00n_xxx test leaves temporary directories with the names of 00n_(master|standby)_XXXX on failure. If this is considered reasonable, I'll make same patches for the other /t/nnn_*.pl tests. regards, -- Kyotaro Horiguchi NTT Open Source Software Center
>From 4e2c5123e249ef3278da8d5260bbe1e4799988b2 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horiguchi.kyot...@lab.ntt.co.jp> Date: Tue, 1 Mar 2016 16:09:25 +0900 Subject: [PATCH 1/2] Change behavior of PostgresNode.pm and TestLib.pm Modify PostgresNode.new to use a temporary directory with the name after the node name instead of tmp_dirXXXX. Modify PostgresNode.psql to allow callers to get stderr output. Make TestLib to preserve the temporary directory if a test has been failed. Modify TestLib.tempdir to allow callers to prefix the tempdir name with an arbitrary prefix. --- src/test/perl/PostgresNode.pm | 9 +++++++-- src/test/perl/TestLib.pm | 10 +++++++++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index a8e6f0c..ad27361 100644 --- a/src/test/perl/PostgresNode.pm +++ b/src/test/perl/PostgresNode.pm @@ -115,7 +115,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" }; @@ -805,10 +805,15 @@ 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; } =pod diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm index 3d11cbb..da0e617 100644 --- a/src/test/perl/TestLib.pm +++ b/src/test/perl/TestLib.pm @@ -107,13 +107,21 @@ INIT autoflush TESTLOG 1; } +END +{ + # Preserve temporary directory for this test if failure + $File::Temp::KEEP_ALL = 1 unless Test::More->builder->is_passing; +} + # # Helper functions # sub tempdir { + my ($prefix) = @_; + $prefix = "tmp_test" if (!$prefix); return File::Temp::tempdir( - 'tmp_testXXXX', + $prefix.'_XXXX', DIR => $tmp_check, CLEANUP => 1); } -- 1.8.3.1
>From fab2f28637a4392ef7e0bfbdf9bcac51f5fb5fd7 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horiguchi.kyot...@lab.ntt.co.jp> Date: Tue, 1 Mar 2016 16:50:24 +0900 Subject: [PATCH 2/2] Prefix test numbers to node directories Add prefix numbers to node names to make the replication tests leave directories with reasonable names on failure. --- src/test/recovery/t/001_stream_rep.pl | 6 +++--- src/test/recovery/t/002_archiving.pl | 4 ++-- src/test/recovery/t/003_recovery_targets.pl | 4 ++-- src/test/recovery/t/004_timeline_switch.pl | 6 +++--- src/test/recovery/t/005_replay_delay.pl | 4 ++-- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/test/recovery/t/001_stream_rep.pl b/src/test/recovery/t/001_stream_rep.pl index 7dcca65..6f2d13b 100644 --- a/src/test/recovery/t/001_stream_rep.pl +++ b/src/test/recovery/t/001_stream_rep.pl @@ -6,7 +6,7 @@ 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'; @@ -15,7 +15,7 @@ my $backup_name = 'my_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 +25,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; diff --git a/src/test/recovery/t/002_archiving.pl b/src/test/recovery/t/002_archiving.pl index 67bb7df..5dafcf2 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); @@ -20,7 +20,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( diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl index 8b581cc..8552e5b 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); @@ -46,7 +46,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 58bf580..fc5399d 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 401d17b..b04fbdb 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('master'); +my $node_master = get_new_node('005_master'); $node_master->init(allows_streaming => 1); $node_master->start; @@ -19,7 +19,7 @@ my $backup_name = 'my_backup'; $node_master->backup($backup_name); # Create streaming standby from backup -my $node_standby = get_new_node('standby'); +my $node_standby = get_new_node('005_standby'); $node_standby->init_from_backup($node_master, $backup_name, has_streaming => 1); $node_standby->append_conf( -- 1.8.3.1
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers