Обсуждение: [HACKERS] Show backtrace when tap tests fail

Hi,

I've had a couple cases where tap tests died, and I couldn't easily see
where / why. For development of a new test I found it useful to show
backtraces in that case - just adding a
use Carp::Always;
at the start of the relevant module did the trick.

I'm wondering if we shouldn't always do so if the module is
installed. I.e. have PostgresNode or something do something like

# Include module showing backtraces upon failures. As it's a
non-standard module, don't fail if not installed.
eval { use Carp::Always; }

Comments?

- Andres


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 09/19/2017 01:31 PM, Andres Freund wrote:
> Hi,
>
> I've had a couple cases where tap tests died, and I couldn't easily see
> where / why. For development of a new test I found it useful to show
> backtraces in that case - just adding a
> use Carp::Always;
> at the start of the relevant module did the trick.
>
> I'm wondering if we shouldn't always do so if the module is
> installed. I.e. have PostgresNode or something do something like
>
> # Include module showing backtraces upon failures. As it's a
> non-standard module, don't fail if not installed.
> eval { use Carp::Always; }
>
> Comments?
>


Or maybe Devel::Confess ?

In an eval you need a 'require' rather than a 'use', AFAIK.


cheers

andrew

-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> On 09/19/2017 01:31 PM, Andres Freund wrote:
>> # Include module showing backtraces upon failures. As it's a
>> non-standard module, don't fail if not installed.
>> eval { use Carp::Always; }

> Or maybe Devel::Confess ?

Neither one seems to be present in a standard Perl installation :-(

> In an eval you need a 'require' rather than a 'use', AFAIK.

Yeah:

$ perl -e 'eval { use Carp::Always; }'
Can't locate Carp/Always.pm in @INC (@INC contains: /usr/local/lib64/perl5 /usr/local/share/perl5
/usr/lib64/perl5/vendor_perl/usr/share/perl5/vendor_perl /usr/lib64/perl5 /usr/share/perl5 .) at -e line 1. 
BEGIN failed--compilation aborted at -e line 1.
$ perl -e 'eval { require Carp::Always; }'
$ echo $?
0
        regards, tom lane


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:

> On 09/19/2017 01:31 PM, Andres Freund wrote:
>> Hi,
>>
>> I've had a couple cases where tap tests died, and I couldn't easily see
>> where / why. For development of a new test I found it useful to show
>> backtraces in that case - just adding a
>> use Carp::Always;
>> at the start of the relevant module did the trick.
>>
>> I'm wondering if we shouldn't always do so if the module is
>> installed. I.e. have PostgresNode or something do something like

I think TestLib would be a better place, since PostgresNode uses TestLib
and there are some tests that use TestLib but notf PostgresNode.

>> # Include module showing backtraces upon failures. As it's a
>> non-standard module, don't fail if not installed.
>> eval { use Carp::Always; }
>>
>> Comments?
>
> Or maybe Devel::Confess ?

Devel::Confess is more thorough, so +1 on that.

> In an eval you need a 'require' rather than a 'use', AFAIK.

Yes, because 'use' happens as soon as the statement is parsed at compile
time, long before the eval { } gets executed.  Devel::Confess does its
setup in the 'import' method (called implicitly by 'use'), so we'd need:
 eval { require Devel::Confess; Devel::Confess->import };

-- 
"The surreality of the universe tends towards a maximum" -- Skud's Law
"Never formulate a law or axiom that you're not prepared to live withthe consequences of."
--Skud's Meta-Law
 


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2017-09-19 21:37:26 +0100, Dagfinn Ilmari Mannsåker wrote:
> Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> 
> > On 09/19/2017 01:31 PM, Andres Freund wrote:
> >> Hi,
> >>
> >> I've had a couple cases where tap tests died, and I couldn't easily see
> >> where / why. For development of a new test I found it useful to show
> >> backtraces in that case - just adding a
> >> use Carp::Always;
> >> at the start of the relevant module did the trick.
> >>
> >> I'm wondering if we shouldn't always do so if the module is
> >> installed. I.e. have PostgresNode or something do something like
> 
> I think TestLib would be a better place, since PostgresNode uses TestLib
> and there are some tests that use TestLib but notf PostgresNode.

Fair.


> >> # Include module showing backtraces upon failures. As it's a
> >> non-standard module, don't fail if not installed.
> >> eval { use Carp::Always; }
> >>
> >> Comments?
> >
> > Or maybe Devel::Confess ?
> 
> Devel::Confess is more thorough, so +1 on that.

Or just try Devel::Confess first, and if the require fails, go to
Carp::always.

> > In an eval you need a 'require' rather than a 'use', AFAIK.
> 
> Yes, because 'use' happens as soon as the statement is parsed at compile
> time, long before the eval { } gets executed.  Devel::Confess does its
> setup in the 'import' method (called implicitly by 'use'), so we'd need:
> 
>   eval { require Devel::Confess; Devel::Confess->import };

My perl's rusty ;)

Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tom Lane <tgl@sss.pgh.pa.us> writes:

> Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
>> On 09/19/2017 01:31 PM, Andres Freund wrote:
>>> # Include module showing backtraces upon failures. As it's a
>>> non-standard module, don't fail if not installed.
>>> eval { use Carp::Always; }
>
>> Or maybe Devel::Confess ?
>
> Neither one seems to be present in a standard Perl installation :-(

No, hence making it optional via eval { }.  That way we can get more
useful output from the buildfarm (and Travis) by installing it there,
without imposing extra dependencies on end users.

We already depend on one non-core module (IPC::Run), so presumably we
could add others, but I presume the threshold for that is quite high.

- ilmari
-- 
"a disappointingly low fraction of the human race is,at any given time, on fire." - Stig Sandbeck Mathisen


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Andres Freund <andres@anarazel.de> writes:
> On 2017-09-19 21:37:26 +0100, Dagfinn Ilmari Mannsåker wrote:
>> Devel::Confess is more thorough, so +1 on that.

> Or just try Devel::Confess first, and if the require fails, go to
> Carp::always.

Meh --- Carp::Always isn't standard either, so I think this is just extra
complication with little value-add.  Let's just do the Devel::Confess
incantation as Dagfinn has it.
        regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2017-09-19 17:15:21 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2017-09-19 21:37:26 +0100, Dagfinn Ilmari Mannsåker wrote:
> >> Devel::Confess is more thorough, so +1 on that.
> 
> > Or just try Devel::Confess first, and if the require fails, go to
> > Carp::always.
> 
> Meh --- Carp::Always isn't standard either, so I think this is just extra
> complication with little value-add.  Let's just do the Devel::Confess
> incantation as Dagfinn has it.

Has ~25 times the installation base on debian tho...

https://qa.debian.org/popcon.php?package=libdevel-confess-perl (13)
vs
https://qa.debian.org/popcon.php?package=libcarp-always-perl (300)

(note this is an opt-in program)

Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Andres Freund <andres@anarazel.de> writes:
> On 2017-09-19 17:15:21 -0400, Tom Lane wrote:
>> Meh --- Carp::Always isn't standard either, so I think this is just extra
>> complication with little value-add.  Let's just do the Devel::Confess
>> incantation as Dagfinn has it.

> Has ~25 times the installation base on debian tho...

> https://qa.debian.org/popcon.php?package=libdevel-confess-perl (13)
> vs
> https://qa.debian.org/popcon.php?package=libcarp-always-perl (300)

Both of those read like "lost in the noise" to me.  I think with
either of these, we're more or less asking PG developers to install
a dependency they probably didn't have before.  We might as well
ask them to install the more useful one.
        regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 9/19/17 5:25 PM, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
>> On 2017-09-19 17:15:21 -0400, Tom Lane wrote:
>>> Meh --- Carp::Always isn't standard either, so I think this is just extra
>>> complication with little value-add.  Let's just do the Devel::Confess
>>> incantation as Dagfinn has it.
> 
>> Has ~25 times the installation base on debian tho...
> 
>> https://qa.debian.org/popcon.php?package=libdevel-confess-perl (13)
>> vs
>> https://qa.debian.org/popcon.php?package=libcarp-always-perl (300)
> 
> Both of those read like "lost in the noise" to me.  I think with
> either of these, we're more or less asking PG developers to install
> a dependency they probably didn't have before.  We might as well
> ask them to install the more useful one.

I just use:

$SIG{__DIE__} = sub {Carp::confess @_};

It also includes the stack for the confess, but that's only a single 
line and I don't care since the important information is at the top.

I have used this in production code and it doesn't seem to have any 
nasty side effects, though this is only a last resort for when a defined 
exception is not raised.  For test code it should be more than sufficient.

I have not tried this on Perls < 5.10, though.

-- 
-David
david@pgmasters.net


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Craig Ringer <craig@2ndquadrant.com> writes:

> On 20 September 2017 at 06:36, David Steele <david@pgmasters.net> wrote:
>
>>
>> I just use:
>>
>> $SIG{__DIE__} = sub {Carp::confess @_};

That is the basic idea behind both Carp::Always and Devel::Confess, but
they also avoid breaking non-string exceptions (references and objects)
being thrown and caught.  Devel::Confess jumps through even more hoops
to add backtraces to these without breaking code catching them.

See the DESCRIPTION and CAVEATS sections (and the source, if you've got
a strong stomach) of https://metacpan.org/pod/Devel::Confess for some of
the hairy details.

> That's what I patched into my TestLib.pm too, until I learned of
> Carp::Always.
>
> I'd rather have Carp::Always, but it's definitely an OK fallback.

I agree with Tom that we should just go for Devel::Confess with no
fallbacks (because of the above-mentioned potential breakage).  This is
mainly for the convenience of developers and the buildfarm, not end
users.

- ilmari
-- 
- Twitter seems more influential [than blogs] in the 'gets reported in the mainstream press' sense at least.
  - Matt McLeod
 
- That'd be because the content of a tweet is easier to condense down to a mainstream media article.
 - Calle Dybedahl
 


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 9/20/17 6:26 AM, Dagfinn Ilmari Mannsåker wrote:
> Craig Ringer <craig@2ndquadrant.com> writes:
> 
>> On 20 September 2017 at 06:36, David Steele <david@pgmasters.net> wrote:
>>
>>>
>>> I just use:
>>>
>>> $SIG{__DIE__} = sub {Carp::confess @_};
> 
> That is the basic idea behind both Carp::Always and Devel::Confess, but
> they also avoid breaking non-string exceptions (references and objects)
> being thrown and caught.  Devel::Confess jumps through even more hoops
> to add backtraces to these without breaking code catching them.

I see.  My object exceptions are always confessed so this code is just
to catch random die's from parts of the code I can't control.  I have
never seen one of them throw a non-string exception before.

-- 
-David
david@pgmasters.net


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers