When a TAP file has non-zero exit status, retain temporary directories.

nmisch · nmisch · commit 78a26c3edd85 · 2020-02-05T08:26:44.000-08:00
PostgresNode already retained base directories in such cases. Stop using $SIG{__DIE__}, which is redundant with the exit status check, in lieu of proliferating it to TestLib. Back-patch to 9.6, where commit 88802e0 introduced retention on failure. Reviewed by Daniel Gustafsson. Discussion: https://postgr.es/m/20200202170155.GA3264196@rfd.leadboat.com
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
@@ -1206,20 +1206,6 @@ sub can_bind
 	return $ret;
 }
 
-# Retain the errno on die() if set, else assume a generic errno of 1.
-# This will instruct the END handler on how to handle artifacts left
-# behind from tests.
-$SIG{__DIE__} = sub {
-	if ($!)
-	{
-		$died = $!;
-	}
-	else
-	{
-		$died = 1;
-	}
-};
-
 # Automatically shut down any still-running nodes when the test script exits.
 # Note that this just stops the postmasters (in the same order the nodes were
 # created in).  Any temporary directories are deleted, in an unspecified
@@ -1238,8 +1224,7 @@ END
 		next if defined $ENV{'PG_TEST_NOCLEAN'};
 
 		# clean basedir on clean test invocation
-		$node->clean_node
-		  if TestLib::all_tests_passing() && !defined $died && !$exit_code;
+		$node->clean_node if $exit_code == 0 && TestLib::all_tests_passing();
 	}
 
 	$? = $exit_code;
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
@@ -131,8 +131,13 @@ INIT
 END
 {
 
-	# Preserve temporary directory for this test on failure
-	$File::Temp::KEEP_ALL = 1 unless all_tests_passing();
+	# Test files have several ways of causing prove_check to fail:
+	# 1. Exit with a non-zero status.
+	# 2. Call ok(0) or similar, indicating that a constituent test failed.
+	# 3. Deviate from the planned number of tests.
+	#
+	# Preserve temporary directories after (1) and after (2).
+	$File::Temp::KEEP_ALL = 1 unless $? == 0 && all_tests_passing();
 }
 
 sub all_tests_passing

Original file line number	Diff line number	Diff line change
`@@ -131,8 +131,13 @@ INIT`
`131`	`131`	`END`
`132`	`132`	`{`
`133`	`133`
`134`		`- # Preserve temporary directory for this test on failure`
`135`		`- $File::Temp::KEEP_ALL = 1 unless all_tests_passing();`
	`134`	`+ # Test files have several ways of causing prove_check to fail:`
	`135`	`+ # 1. Exit with a non-zero status.`
	`136`	`+ # 2. Call ok(0) or similar, indicating that a constituent test failed.`
	`137`	`+ # 3. Deviate from the planned number of tests.`
	`138`	`+ #`
	`139`	`+ # Preserve temporary directories after (1) and after (2).`
	`140`	`+ $File::Temp::KEEP_ALL = 1 unless $? == 0 && all_tests_passing();`
`136`	`141`	`}`
`137`	`142`
`138`	`143`	`sub all_tests_passing`