Skip to content

Commit

Permalink
Issue #190 - move counter from TAP to History
Browse files Browse the repository at this point in the history
Avoid threads corruption

The whole history is locked now to prevent any corruption of
the test state
  • Loading branch information
geistteufel committed Mar 31, 2012
1 parent fb86f7a commit 7aa03c5
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 41 deletions.
4 changes: 4 additions & 0 deletions lib/TB2/EventCoordinator.pm
Expand Up @@ -218,6 +218,10 @@ sub post_event {
my $self = shift;
my $event = shift;

#prevent counter corruption between multiple threads
my $history = $self->history;
lock $history;

$event = shared_clone($event);
for my $handler ($self->all_handlers) {
$handler->accept_event($event, $self);
Expand Down
40 changes: 10 additions & 30 deletions lib/TB2/Formatter/TAP/Base.pm
Expand Up @@ -4,7 +4,6 @@ use 5.008001;

use TB2::Mouse;
use TB2::Types;
use TB2::threads::shared;
extends 'TB2::Formatter';
with 'TB2::CanLoad';

Expand Down Expand Up @@ -121,26 +120,6 @@ sub note {
}


=head3 counter
my $counter = $formatter->counter;
$formatter->counter($counter);
Gets/sets the TB2::Counter for this formatter keeping track of
the test number.
=cut

has counter =>
is => 'rw',
isa => 'TB2::Counter',
trigger => sub { shared_clone($_[1]) },
default => sub {
$_[0]->load('TB2::Counter');
return TB2::Counter->new;
},
;

=head3 use_numbers
my $use_numbers = $formatter->use_numbers;
Expand Down Expand Up @@ -220,7 +199,7 @@ sub handle_test_end {
my $event = shift;
my $ec = shift;

$self->output_plan if $self->show_footer;
$self->output_plan($ec) if $self->show_footer;

$self->output_ending_commentary($ec);

Expand All @@ -243,7 +222,7 @@ sub handle_set_plan {

# TAP only allows a plan at the very start or the very end.
# If we've already seen some results, or it's "no_plan", save it for the end.
$self->output_plan if !$self->seen_results and $self->show_header and !$event->no_plan;
$self->output_plan($ec) if !$self->seen_results and $self->show_header and !$event->no_plan;

return;
}
Expand All @@ -257,13 +236,14 @@ has did_output_plan =>

sub output_plan {
my $self = shift;
my ($ec) = @_;

return unless $self->show_plan;
return if $self->did_output_plan;

return if !$self->plan;

$self->_output_plan;
$self->_output_plan($ec);

$self->did_output_plan(1);

Expand All @@ -272,6 +252,8 @@ sub output_plan {

sub _output_plan {
my $self = shift;
my ($ec) = @_;

my $plan = $self->plan;

if( $plan->skip ) {
Expand All @@ -282,7 +264,7 @@ sub _output_plan {
$self->out($out);
}
elsif( $plan->no_plan ) {
my $seen = $self->counter->get;
my $seen = $ec->history->counter->get;
$self->out("1..$seen\n");
}
elsif( my $expected = $plan->asserts_expected ) {
Expand Down Expand Up @@ -340,7 +322,7 @@ sub output_ending_commentary {

my $plan = $self->plan;

my $tests_run = $self->counter->get;
my $tests_run = $ec->history->counter->get;
my $w_test = _inflect("test", $tests_run);

my $tests_failed = $ec->history->fail_count;
Expand Down Expand Up @@ -441,7 +423,7 @@ has test_is_todo =>

sub handle_result {
my $self = shift;
my $result = shift;
my ($result, $ec) = @_;

# FIXME: there is a lot more detail in the
# result object that I ought to do deal with.
Expand All @@ -450,9 +432,7 @@ sub handle_result {
$out .= "not " if !$result->literal_pass;
$out .= "ok";

my $counter = $self->counter;
lock $counter;
my $num = $result->test_number || $counter->increment;
my $num = $result->test_number || $ec->history->counter->get;
$out .= " ".$num if $self->use_numbers;

my $name = $result->name;
Expand Down
30 changes: 29 additions & 1 deletion lib/TB2/History.pm
Expand Up @@ -4,9 +4,11 @@ use Carp;
use TB2::Mouse;
use TB2::Types;
use TB2::StackBuilder;
use TB2::threads::shared;

with 'TB2::EventHandler',
'TB2::CanTry';
'TB2::CanTry',
'TB2::CanLoad';

our $VERSION = '1.005000_004';
$VERSION = eval $VERSION; ## no critic (BuiltinFunctions::ProhibitStringyEval)
Expand Down Expand Up @@ -189,6 +191,9 @@ sub handle_result {
my $self = shift;
my $result = shift;

my $counter = $self->counter;
$counter->increment;

$self->results_push($result);
$self->events_push($result);

Expand Down Expand Up @@ -408,6 +413,29 @@ sub done_testing {
}




=head3 counter
my $counter = $formatter->counter;
$formatter->counter($counter);
Gets/sets the TB2::Counter for this formatter keeping track of
the test number.
=cut

has counter =>
is => 'rw',
isa => 'TB2::Counter',
trigger => sub { shared_clone($_[1]) },
default => sub {
$_[0]->load('TB2::Counter');
return TB2::Counter->new;
},
;


=head3 plan
my $plan = $history->plan;
Expand Down
12 changes: 2 additions & 10 deletions lib/Test/Builder.pm
Expand Up @@ -325,16 +325,8 @@ sub history {

sub counter {
my $self = shift;

my $counter = $self->try(sub { $self->formatter->counter; });
return $counter if $counter;

# Fake a counter from the history object.
# This will not remember changes to the current_test()
$counter = TB2::Counter->new;
$counter->set($self->history->results_count);

return $counter;

return $self->history->counter;
}

=item B<object_id>
Expand Down

0 comments on commit 7aa03c5

Please sign in to comment.