Re: PATCH: Unlogged tables re-initialization tests - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: PATCH: Unlogged tables re-initialization tests
Date
Msg-id 20180313063113.GC23071@paquier.xyz
Whole thread Raw
In response to Re: PATCH: Unlogged tables re-initialization tests  (David Steele <david@pgmasters.net>)
List pgsql-hackers
On Mon, Mar 12, 2018 at 02:33:18PM -0400, David Steele wrote:
> On 3/12/18 11:59 AM, Dagfinn Ilmari Mannsåker wrote:
>> However, that is still wrong, because die() and BAIL_OUT() mean
>> different things: die() aborts the current test script, while BAIL_OUT()
>> aborts the entire 'prove' run, i.e. all subsequent scripts in the same
>> test directory.
>
> If that's the case, do we really want to abort all subsequent test
> modules if a single module fails?  I'm good either way, just throwing it
> out there for consideration.

I am getting that in those code paths, we want the test series to die in
a painful way which should be used for tests where things are critically
failing.  In which case, as documented by Test:More we should use
BAIL_OUT.  It is true that using die() has the advantage that one can
look at multiple failures in a single run when you want to look for
similar failure patterns, however it makes debugging harder if you a lot
of test scripts because it becomes harder to track which one was the
first to fail in a parallel test.

Also, if you look at all the code paths calling die() in the test suites
those refer to critical failures, which should cause an immediate stop
of the test.  So my take would be to switch all die() calls to
BAIL_OUT() in order to make all this code consistent.
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: David Gould
Date:
Subject: Re: [patch] BUG #15005: ANALYZE can make pg_class.reltuplesinaccurate.
Next
From: Andrey Borodin
Date:
Subject: Re: [WIP PATCH] Index scan offset optimisation using visibility map