On Sun, Aug 9, 2015 at 11:19 PM, Andrew Dunstan <and...@dunslane.net> wrote:
>
>
> On 08/09/2015 08:41 AM, Michael Paquier wrote:
>>
>> On Sun, Aug 9, 2015 at 1:40 AM, Andrew Dunstan <and...@dunslane.net>
>> wrote:
>>>
>>> On 08/08/2015 09:31 AM, Robert Haas wrote:
>>>>
>>>> On Fri, Aug 7, 2015 at 7:17 PM, Andrew Dunstan <and...@dunslane.net>
>>>> wrote:
>>>>>
>>>>> That certainly isn't what happens, and given the way this is done in
>>>>> TestLib.pm, using the CLEANUP parameter of File::Temp's tempdir()
>>>>> function,
>>>>> it's not clear how we could do that easily.
>>>>
>>>> <shot-in-the-dark>
>>>>
>>>> Set cleanup to false and manually remove the directory later in the
>>>> code, so that stuff runs only if we haven't died sooner?
>>>>
>>>> </shot-in-the-dark>
>>>>
>>> Yeah, maybe. I'm thinking of trying to do it more globally, like in
>>> src/Makefile.global.in. That way we wouldn't have to add new code to
>>> every
>>> test file.
>>
>> If we rely on END to clean up the temporary data folder, there is no
>> need to impact each test file, just the test functions called in
>> TestLib.pm that could switch a flag to not perform any cleanup at the
>> end of the run should an unexpected result be found. Now I am as well
>> curious to see what you have in mind with manipulating
>> Makefile.global.
>
> See attached. If you have a better idea please post your patch.

Sure. Attached is what I have in mind. Contrary to your version we
keep around temp paths should a run succeed after one that has failed
when running make check multiple times in a row. Perhaps it does not
matter much in practice as log files get removed at each new run but I
think that this allows a finer control in case of failure. Feel free
to have a look.
-- 
Michael
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index 4927d45..11f774e 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -35,6 +35,7 @@ our @EXPORT = qw(
 
 use Cwd;
 use File::Basename;
+use File::Path qw(rmtree);
 use File::Spec;
 use File::Temp ();
 use IPC::Run qw(run start);
@@ -107,21 +108,24 @@ $ENV{PGPORT} = int($ENV{PGPORT}) % 65536;
 # Helper functions
 #
 
+my $temp_cleanup = 1;
+my @temp_dirs = ();
 
 sub tempdir
 {
-	return File::Temp::tempdir(
+	my $path = File::Temp::tempdir(
 		'tmp_testXXXX',
 		DIR => $ENV{TESTDIR} || cwd(),
-		CLEANUP => 1);
+		CLEANUP => 0);
+	push(@temp_dirs, $path);
+	return $path;
 }
 
 sub tempdir_short
 {
-
 	# Use a separate temp dir outside the build tree for the
 	# Unix-domain socket, to avoid file name length issues.
-	return File::Temp::tempdir(CLEANUP => 1);
+	return File::Temp::tempdir(CLEANUP => 0);
 }
 
 # Initialize a new cluster for testing.
@@ -217,6 +221,13 @@ END
 		system_log('pg_ctl', '-D', $test_server_datadir, '-m',
 		  'immediate', 'stop');
 	}
+
+	# Cleanup any temporary directory created
+	return if (!$temp_cleanup);
+	foreach my $temp_dir (@temp_dirs)
+	{
+		rmtree($temp_dir);
+	}
 }
 
 sub psql
@@ -278,14 +289,16 @@ sub command_ok
 {
 	my ($cmd, $test_name) = @_;
 	my $result = run_log($cmd);
-	ok($result, $test_name);
+	my $err_num = ok($result, $test_name);
+	$temp_cleanup = 0 if (!$err_num);
 }
 
 sub command_fails
 {
 	my ($cmd, $test_name) = @_;
 	my $result = run_log($cmd);
-	ok(!$result, $test_name);
+	my $err_num = ok(!$result, $test_name);
+	$temp_cleanup = 0 if (!$err_num);
 }
 
 sub command_exit_is
@@ -304,7 +317,8 @@ sub command_exit_is
 	# that, use $h->full_result on Windows instead.
 	my $result = ($Config{osname} eq "MSWin32") ?
 		($h->full_results)[0] : $h->result(0);
-	is($result, $expected, $test_name);
+	my $err_num = is($result, $expected, $test_name);
+	$temp_cleanup = 0 if (!$err_num);
 }
 
 sub program_help_ok
@@ -313,9 +327,13 @@ sub program_help_ok
 	my ($stdout, $stderr);
 	print("# Running: $cmd --help\n");
 	my $result = run [ $cmd, '--help' ], '>', \$stdout, '2>', \$stderr;
-	ok($result, "$cmd --help exit code 0");
-	isnt($stdout, '', "$cmd --help goes to stdout");
-	is($stderr, '', "$cmd --help nothing to stderr");
+	my $err_num;
+	$err_num = ok($result, "$cmd --help exit code 0");
+	$temp_cleanup = 0 if (!$err_num);
+	$err_num = isnt($stdout, '', "$cmd --help goes to stdout");
+	$temp_cleanup = 0 if (!$err_num);
+	$err_num = is($stderr, '', "$cmd --help nothing to stderr");
+	$temp_cleanup = 0 if (!$err_num);
 }
 
 sub program_version_ok
@@ -324,9 +342,13 @@ sub program_version_ok
 	my ($stdout, $stderr);
 	print("# Running: $cmd --version\n");
 	my $result = run [ $cmd, '--version' ], '>', \$stdout, '2>', \$stderr;
-	ok($result, "$cmd --version exit code 0");
-	isnt($stdout, '', "$cmd --version goes to stdout");
-	is($stderr, '', "$cmd --version nothing to stderr");
+	my $err_num;
+	$err_num = ok($result, "$cmd --version exit code 0");
+	$temp_cleanup = 0 if (!$err_num);
+	$err_num = isnt($stdout, '', "$cmd --version goes to stdout");
+	$temp_cleanup = 0 if (!$err_num);
+	$err_num = is($stderr, '', "$cmd --version nothing to stderr");
+	$temp_cleanup = 0 if (!$err_num);
 }
 
 sub program_options_handling_ok
@@ -335,20 +357,27 @@ sub program_options_handling_ok
 	my ($stdout, $stderr);
 	print("# Running: $cmd --not-a-valid-option\n");
 	my $result = run [ $cmd, '--not-a-valid-option' ], '>', \$stdout, '2>',
-	  \$stderr;
-	ok(!$result, "$cmd with invalid option nonzero exit code");
-	isnt($stderr, '', "$cmd with invalid option prints error message");
+	\$stderr;
+	my $err_num;
+	$err_num = ok(!$result, "$cmd with invalid option nonzero exit code");
+	$temp_cleanup = 0 if (!$err_num);
+	$err_num = isnt($stderr, '', "$cmd with invalid option prints error message");
+	$temp_cleanup = 0 if (!$err_num);
 }
 
 sub command_like
 {
 	my ($cmd, $expected_stdout, $test_name) = @_;
 	my ($stdout, $stderr);
+	my $err_num;
 	print("# Running: " . join(" ", @{$cmd}) . "\n");
 	my $result = run $cmd, '>', \$stdout, '2>', \$stderr;
-	ok($result, "@$cmd exit code 0");
+	$err_num = ok($result, "@$cmd exit code 0");
+	$temp_cleanup = 0 if (!$err_num);
 	is($stderr, '', "@$cmd no stderr");
-	like($stdout, $expected_stdout, "$test_name: matches");
+	$temp_cleanup = 0 if (!$err_num);
+	$err_num = like($stdout, $expected_stdout, "$test_name: matches");
+	$temp_cleanup = 0 if (!$err_num);
 }
 
 sub issues_sql_like
@@ -356,9 +385,12 @@ sub issues_sql_like
 	my ($cmd, $expected_sql, $test_name) = @_;
 	truncate $test_server_logfile, 0;
 	my $result = run_log($cmd);
-	ok($result, "@$cmd exit code 0");
+	my $err_num;
+	$err_num = ok($result, "@$cmd exit code 0");
+	$temp_cleanup = 0 if (!$err_num);
 	my $log = slurp_file($test_server_logfile);
-	like($log, $expected_sql, "$test_name: SQL found in server log");
+	$err_num = like($log, $expected_sql, "$test_name: SQL found in server log");
+	$temp_cleanup = 0 if (!$err_num);
 }
 
 1;
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to