Skip Menu |
 

This queue is for tickets about the Module-Build CPAN distribution.

Report information
The Basics
Id: 49080
Status: resolved
Priority: 0/
Queue: Module-Build

People
Owner: Nobody in particular
Requestors: dwheeler [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: Normal
Broken in: (no value)
Fixed in: 0.36



Subject: 'use_tap_harness' should die on test failure
Date: Thu, 27 Aug 2009 15:44:14 -0700
To: bug-module-build [...] rt.cpan.org
From: "David E. Wheeler" <dwheeler [...] cpan.org>
Download (untitled) / with headers
text/plain 570b
I'm using the "use_tap_harness" option in a project, and a colleague, who set up a pre-checkin hook to make sure that all tests pass, discovered that `./Build test` was exiting normally even when tests pass. Such is not the case when using Test::Harness, which `die`s on failure. TAP::Harness doesn't die, though; it's up to the caller to die. So the attached patch calls `exit 1` if the tests fail. I'd say that this is a bug, given that it means that the behavior of `./Build test` varies depending on whether or not one uses TAP::Harness. Best, David

Message body is not shown because sender requested not to inline it.

Download (untitled) / with headers
text/plain 322b
Thank you for the bug report and patch. I took a different approach to fixing it, since "exit 1" can't be trapped in the usual way for testing. Instead, the full die() summary message of Test::Harness is emulated. I also added tests to confirm the non-zero exit code. Patched in trunk and will be in a future release.
Subject: Re: [rt.cpan.org #49080] [PATCH] 'use_tap_harness' should die on test failure
Date: Sun, 30 Aug 2009 20:14:56 -0700
To: "bug-Module-Build [...] rt.cpan.org" <bug-Module-Build [...] rt.cpan.org>
From: "David E. Wheeler" <dwheeler [...] cpan.org>
Download (untitled) / with headers
text/plain 635b
On Aug 30, 2009, at 18:29, "David Golden via RT" <bug-Module-Build@rt.cpan.org Show quoted text
> wrote:
Show quoted text
> Thank you for the bug report and patch. I took a different approach > to > fixing it, since "exit 1" can't be trapped in the usual way for > testing. > Instead, the full die() summary message of Test::Harness is emulated. > I also added tests to confirm the non-zero exit code. > > Patched in trunk and will be in a future release.
I'm not sure that's the right approach, since TAP::Harness will have already output that information. It'll look redundant. I think that it needs to be a quiet exit, like Test::Harness does. David
On Sun Aug 30 23:15:24 2009, DWHEELER wrote: Show quoted text
> I'm not sure that's the right approach, since TAP::Harness will have > already output that information. It'll look redundant. I think that it > needs to be a quiet exit, like Test::Harness does.
If you look at Test::Harness::runtests, you can see that it constructs a very specific die() message. Under TAP::Harness::runtests, the summary output is generated by the formatter, which may or may not send information to the console. It may have similar information, or it may not. In short, there is no way for Module::Build to know whether any invocation of TAP::Harness provides any useful information or not, so providing a summary -- even redundant -- that matches that produced by Test::Harness would seem to have the "least surprised" for users (including automated tools parsing test output). That's my take on it, but I'm open to further discussion. The only absolute I have is that it must die(), not exit(), so it can be caught in eval() or a signal handler. -- David G
Subject: Re: [rt.cpan.org #49080] [PATCH] 'use_tap_harness' should die on test failure
Date: Mon, 31 Aug 2009 11:56:22 -0700
To: bug-Module-Build [...] rt.cpan.org
From: "David E. Wheeler" <dwheeler [...] cpan.org>
Download (untitled) / with headers
text/plain 1.1k
On Aug 30, 2009, at 9:44 PM, David Golden via RT wrote: Show quoted text
> If you look at Test::Harness::runtests, you can see that it > constructs a > very specific die() message. Under TAP::Harness::runtests, the > summary > output is generated by the formatter, which may or may not send > information to the console. It may have similar information, or it > may > not. In short, there is no way for Module::Build to know whether any > invocation of TAP::Harness provides any useful information or not, so > providing a summary -- even redundant -- that matches that produced by > Test::Harness would seem to have the "least surprised" for users > (including automated tools parsing test output). > > That's my take on it, but I'm open to further discussion. > > The only absolute I have is that it must die(), not exit(), so it > can be > caught in eval() or a signal handler.
Hrm. Might be worth asking for opinions on perl-qa@perl.org and/or tapx-dev@hexten.net . Die'ing is fine, I agree with that, but I'm less sure about the output. I think that it's the responsibility of all formatters to output an appropriate summary, but I'm not positive. Best, David


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.