Forking thread "logical decoding : exceeded maxAllocatedDescs for .spill files" for this side issue:
On Wed, Jan 08, 2020 at 09:37:04PM -0800, Noah Misch wrote: > v10 > deletes PostgresNode base directories at the end of this test file, despite > the failure[1]. > [1] It has the all_tests_passing() logic in an attempt to stop this. I'm > guessing it didn't help because the file failed by calling die "connection > error: ...", not by reporting a failure to Test::More via ok(0) or similar. That is what happened. We should test the exit status to decide whether to keep temporaries, as attached. PostgresNode does that, since commit 90627cf (thread https://postgr.es/m/flat/6205.1492883490%40sss.pgh.pa.us). That thread already discussed $SUBJECT[1] and the __DIE__ handler being redundant[2]. I plan to back-patch, since it's most useful for v10 and v9.6. [1] https://postgr.es/m/CAMsr+YFyFU=+mvfzqhthfmw22x5-h517e6ck6et+dt+x4bu...@mail.gmail.com [2] https://postgr.es/m/fea925b2-c3ae-4ba9-9194-5f5616ad0...@yesql.se
Author: Noah Misch <n...@leadboat.com> Commit: Noah Misch <n...@leadboat.com> When a TAP file has non-zero exit status, retain temporary directories. 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 88802e068017bee8cea7a5502a712794e761c7b5 introduced retention on failure. Reviewed by FIXME. Discussion: https://postgr.es/m/FIXME diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index 2e0cf4a..ff972d1 100644 --- 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 index 458b801..65ee042 100644 --- a/src/test/perl/TestLib.pm +++ b/src/test/perl/TestLib.pm @@ -183,8 +183,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(); } =pod