Skip Menu |
 

This queue is for tickets about the TAPx-Parser CPAN distribution.

Report information
The Basics
Id: 21379
Status: resolved
Priority: 0/
Queue: TAPx-Parser

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

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

Attachments


Subject: [Patch+Test] TAPx::Parser::Streamed has leftovers from previous (unrelated) invocations
Download (untitled) / with headers
text/plain 559b
The test file in this patch demonstrates a case where TAPx::Parser::Stream is called, and then stops processing after a bailout, and afterwards a different parser is initialised and gets some of its leftover tokens. This caused a subtle bug during the conversion of Test::Run to use TAPx::Parser. The patch also corrects the problem, by converting two "static" variables into object variables. I've noticed other places where such static variables are used, and they may cause similar problems due to their Singleton nature. Regards, Shlomi Fish
Subject: TAPx-Parser-premature-bailout.diff
Index: t/80-premature-bailout.t =================================================================== --- t/80-premature-bailout.t (revision 0) +++ t/80-premature-bailout.t (revision 7) @@ -0,0 +1,111 @@ +#!/usr/bin/perl +use warnings; +use strict; + +use Test::More tests => 14; +use TAPx::Parser; + +use TAPx::Parser::Iterator; +use TAPx::Parser::Streamed; + +sub tap_to_lines +{ + my $string = shift; + my @lines = ($string =~ /.*\n/g); + return \@lines; +} + +my $tap = <<'END_TAP'; +1..4 +ok 1 - input file opened +... this is junk +not ok first line of the input valid # todo some data +# this is a comment +ok 3 - read the rest of the file +not ok 4 - this is a real failure +Bail out! We ran out of foobar. +not ok 5 +END_TAP + +my $parser = TAPx::Parser->new( + { + stream => TAPx::Parser::Iterator->new(tap_to_lines($tap)), + } +); + +# results() is sane? + +# check the test plan +my $result = $parser->next(); +# TEST +ok $result->is_plan, 'We should have a plan'; + +# a normal, passing test + +my $test = $parser->next(); +# TEST +ok $test->is_test, '... and a test'; + +# junk lines should be preserved + +my $unknown = $parser->next(); +# TEST +ok $unknown->is_unknown, '... and an unknown line'; + +# a failing test, which also happens to have a directive + +my $failed = $parser->next(); +# TEST +ok $failed->is_test, '... and another test'; + +# comments + +my $comment = $parser->next(); +# TEST +ok $comment->is_comment, '... and a comment'; + +# another normal, passing test + +$test = $parser->next(); +# TEST +ok $test->is_test, '... and another test'; + +# a failing test + +$failed = $parser->next(); +# TEST +ok $failed->is_test, '... and yet another test'; + +# ok 5 # skip we have no description +# skipped test +my $bailout = $parser->next(); +# TEST +ok $bailout->is_bailout, 'And finally we should have a bailout'; +# TEST +is $bailout->as_string, 'We ran out of foobar.', + '... and as_string() should return the explanation'; +# TEST +is ($bailout->raw, 'Bail out! We ran out of foobar.', + '... and raw() should return the explanation'); +# TEST +is ($bailout->explanation, 'We ran out of foobar.', + '... and it should have the correct explanation'); + +my $more_tap = "1..1\nok 1 - input file opened\n"; + +my $second_parser = TAPx::Parser->new( + { + stream => TAPx::Parser::Iterator->new([split(/\n/, $more_tap)]), + } +); + +$result = $second_parser->next(); +# TEST +ok $result->is_plan(), "Result is not the leftover line"; + +$result = $second_parser->next(); +# TEST +ok $result->is_test(), "Result is a test"; +# TEST +ok $result->passed(), "The event has passed"; + Property changes on: t/80-premature-bailout.t ___________________________________________________________________ Name: svn:executable + Index: MANIFEST =================================================================== --- MANIFEST (revision 5) +++ MANIFEST (revision 7) @@ -1,37 +1,35 @@ + + + + + # build info Build.PL Changes -Makefile.PL -MANIFEST -META.yml # Will be created by "make dist" -README -TODO - -# parsers -lib/TAPx/Parser.pm -lib/TAPx/Parser/Streamed.pm - -# helpers -lib/TAPx/Parser/Aggregator.pm -lib/TAPx/Parser/Iterator.pm -lib/TAPx/Parser/Source/Perl.pm examples/README +examples/t/10-stuff.t examples/tprove examples/tprove_color -examples/t/10-stuff.t - # grammars +# helpers +lib/TAPx/Parser/Aggregator.pm lib/TAPx/Parser/Grammar.pm - -# result types -lib/TAPx/Parser/Results.pm +lib/TAPx/Parser/Iterator.pm +lib/TAPx/Parser.pm +lib/TAPx/Parser/Results/Bailout.pm lib/TAPx/Parser/Results/Comment.pm lib/TAPx/Parser/Results/Plan.pm +lib/TAPx/Parser/Results.pm lib/TAPx/Parser/Results/Test.pm lib/TAPx/Parser/Results/Unknown.pm -lib/TAPx/Parser/Results/Bailout.pm - -# tests +lib/TAPx/Parser/Source/Perl.pm +lib/TAPx/Parser/Streamed.pm +Makefile.PL +MANIFEST +META.yml # Will be created by "make dist" +# parsers +README +# result types t/00-load.t t/10-lex.t t/20-parse.t @@ -40,5 +38,7 @@ t/50-streams.t t/60-aggregator.t t/70-callbacks.t +# tests +TODO t/pod-coverage.t t/pod.t Index: lib/TAPx/Parser.pm =================================================================== --- lib/TAPx/Parser.pm (revision 5) +++ lib/TAPx/Parser.pm (revision 7) @@ -32,6 +32,8 @@ plan tests_planned tests_run + _m_tokens + _current_chunk > ) { Index: lib/TAPx/Parser/Streamed.pm =================================================================== --- lib/TAPx/Parser/Streamed.pm (revision 5) +++ lib/TAPx/Parser/Streamed.pm (revision 7) @@ -33,6 +33,7 @@ $self->_stream($stream); $self->_start_tap(undef); $self->_end_tap(undef); + $self->_m_tokens([]); return $self; } @@ -58,47 +59,43 @@ return @tokens; } -{ - my ( @tokens, $current_chunk ); - - # all of this annoying current and next chunk stuff is to ensure that we - # really do know if we're at the beginning or end of a stream. - sub next { - my $self = shift; - if (@tokens) { - return shift @tokens; +# all of this annoying current and next chunk stuff is to ensure that we +# really do know if we're at the beginning or end of a stream. +sub next { + my $self = shift; + if (@{$self->_m_tokens}) { + return shift @{$self->_m_tokens}; + } + if ($self->_current_chunk) { + if ( $self->_stream_started ) { + $self->_start_tap(0); } - if ($current_chunk) { - if ( $self->_stream_started ) { - $self->_start_tap(0); - } - else { - $self->_start_tap(1); - $self->_stream_started(1); - } + else { + $self->_start_tap(1); + $self->_stream_started(1); } - my $next_chunk = $self->_stream->next; - if (! $current_chunk && $next_chunk ) { - $current_chunk = $next_chunk; - return $self->next; - } - unless ( defined $next_chunk ) { - $self->_end_tap(1); - } - if ( defined $current_chunk ) { - my @current_tokens = map { - my $result = TAPx::Parser::Results->new($_); - $self->_validate($result); - $result; - } $self->_lex($current_chunk); - my $token = shift @current_tokens; - push @tokens => @current_tokens; - $current_chunk = $next_chunk; - return $token; - } - $self->_finish; - return; } + my $next_chunk = $self->_stream->next; + if (! $self->_current_chunk() && $next_chunk ) { + $self->_current_chunk($next_chunk); + return $self->next; + } + unless ( defined $next_chunk ) { + $self->_end_tap(1); + } + if ( defined $self->_current_chunk ) { + my @current_tokens = map { + my $result = TAPx::Parser::Results->new($_); + $self->_validate($result); + $result; + } $self->_lex($self->_current_chunk()); + my $token = shift @current_tokens; + push @{$self->_m_tokens()} => @current_tokens; + $self->_current_chunk($next_chunk); + return $token; + } + $self->_finish; + return; } =head1 OVERRIDDEN METHODS
Download (untitled) / with headers
text/plain 809b
Shlomi, Thank you, patch applied and 0.21 uploaded. Your comments are spot on. I'm at a conference right now, so I'll try to address more of this later. Cheers, Ovid On Thu Sep 07 14:20:15 2006, SHLOMIF wrote: Show quoted text
> The test file in this patch demonstrates a case where > TAPx::Parser::Stream is called, and then stops processing after a > bailout, and afterwards a different parser is initialised and gets > some of its leftover tokens. > > This caused a subtle bug during the conversion of Test::Run to use > TAPx::Parser. > > The patch also corrects the problem, by converting two "static" > variables into object variables. > > I've noticed other places where such static variables are used, and > they may cause similar problems due to their Singleton nature. > > Regards, > > Shlomi Fish


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.