pg.ddx.io pgsql-hackers@postgresql.org mailing list archivehelp / color / mirror / Atom feed
TestLib condition for deleting temporary directories 2+ messages / 2 participants [nested] [flat]
Commits from this thread: 2020-02-05 bf989aaf35 78a26c3edd 887657d183 d8efc5900f 048c7ccd7d When a TAP file has non-zero exit status, retain temporary directories.
* TestLib condition for deleting temporary directories @ 2020-02-02 17:01 Noah Misch <noah@leadboat.com> 0 siblings, 1 reply; 2+ messages in thread From: Noah Misch @ 2020-02-02 17:01 UTC (permalink / raw) To: pgsql-hackers 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+X4bUO7g@mail.gmail.com [2] https://postgr.es/m/FEA925B2-C3AE-4BA9-9194-5F5616AD0794@yesql.se Attachments:^ permalink raw reply [nested|flat] 2+ messages in threadperl-temporaries-v1.patch (text/plain, 2.2K) download
Author: Noah Misch <noah@leadboat.com> Commit: Noah Misch <noah@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
* Re: TestLib condition for deleting temporary directories @ 2020-02-02 17:19 Daniel Gustafsson <daniel@yesql.se> parent: Noah Misch <noah@leadboat.com> 0 siblings, 0 replies; 2+ messages in thread From: Daniel Gustafsson @ 2020-02-02 17:19 UTC (permalink / raw) To: Noah Misch <noah@leadboat.com>; +Cc: pgsql-hackers > On 2 Feb 2020, at 18:01, Noah Misch <noah@leadboat.com> wrote: > > Forking thread "logical decoding : exceeded maxAllocatedDescs for .spill > files" for this side issue: Thanks, I hadn't seen this. > 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. I'm travelling and haven't been able to test, but this makes sense from reading. +1 on backpatching. cheers ./daniel ^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2020-02-02 17:19 UTC | newest] Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-02-02 17:01 TestLib condition for deleting temporary directories Noah Misch <noah@leadboat.com> 2020-02-02 17:19 ` Daniel Gustafsson <daniel@yesql.se>
This inbox is served by DDX for PostgreSQL; see mirroring instructions for how to clone and mirror all data and code used for this inbox