Bug #38355 for Test-Harness: interaction of rules and test order

Sat Aug 09 07:56:08 2008 nick [...] ccl4.org - Ticket created

Subject:

interaction of rules and test order

I was hoping that I could get the core tests running in parallel, slowest first. So I tried this patch to t/harness ==== //depot/perl/t/harness#50 - /Volumes/Stuff/p4perl/perl/t/harness ==== --- /tmp/tmp.25985.99 2008-08-09 12:46:23.000000000 +0100 +++ /Volumes/Stuff/p4perl/perl/t/harness 2008-08-09 12:46:13.000000000 +0100 @@ -188,8 +188,16 @@ if ($^O eq 'MSWin32') { my $jobs = $ENV{TEST_JOBS}; if ($jobs) { eval 'use TAP::Harness 3.13; 1' or die $@; + require App::Prove::State; my $h = TAP::Harness->new({ jobs => $jobs, rules => $rules}); - $h->runtests(@tests); + my $s = App::Prove::State->new({ store => 'test_state' }); + $s->apply_switch('slow', 'save'); + $h->callback( + after_test => sub { + $s->observe_test(@_); + } + ); + $h->runtests($s->get_tests(0, @tests)); } else { Test::Harness::runtests @tests; } First time through I got the message about no saved state, and at the end of the first run, I got the state written out. Second run, expectations high, but I see that tests continue to run in the same order as before. :-( In particular, op/regexp_qr_embed_thr.t is still the last test to finish of the first batch in parallel. I printed out the output of App::Prove::State::get_tests - here are the top ten: ../ext/Compress/Raw/Zlib/t/07bufsize.t op/regexp_qr_embed_thr.t ../ext/B/t/concise-xs.t ../lib/locale.t op/pat.t op/pat_thr.t ../ext/Time/HiRes/t/HiRes.t ../lib/Benchmark.t ../ext/IO_Compress_Zlib/t/106prime-zip.t ../lib/Memoize/t/expmod_t.t ../lib/Module/Build/t/compat.t ../ext/IO/t/io_sock.t ../lib/File/Temp/t/fork.t ../lib/Memoize/t/speed.t ../ext/threads/shared/t/wait.t ../lib/AutoLoader/t/02AutoSplit.t ../lib/Thread/Queue/t/01_basic.t ../lib/ExtUtils/t/Constant.t ../ext/threads/t/free.t So I know that the state file is being written out correctly, and with good data. So how easy (or hard) is it to merge the order of the passed in tests with the constraints of the ruleset, and hence run the slowest available test within parallel groups? Nicholas Clark

Sat Aug 09 08:25:24 2008 nick [...] ccl4.org - Correspondence added

Subject:	Re: [rt.cpan.org #38355] AutoReply: interaction of rules and test order
Date:	Sat, 9 Aug 2008 13:25:10 +0100
To:	Bugs in Test-Harness via RT <bug-Test-Harness [...] rt.cpan.org>
From:	Nicholas Clark <nick [...] ccl4.org>

On Sat, Aug 09, 2008 at 07:56:09AM -0400, Bugs in Test-Harness via RT wrote: Show quoted text

> ------------------------------------------------------------------------- > I was hoping that I could get the core tests running in parallel, > slowest first. So I tried this patch to t/harness

Show quoted text

> So I know that the state file is being written out correctly, and with > good data. So how easy (or hard) is it to merge the order of the passed > in tests with the constraints of the ruleset, and hence run the slowest > available test within parallel groups?

It would be quite useful, as doing the equivalent using prove: $ ../perl ../utils/prove -j3 --state=save,slow comp/*.t cmd/*.t run/*.t io/*.t op/*.t uni/*.t mro/*.t lib/*.t goes from a first run of 108 seconds to a second run of 94 seconds: All tests successful. Files=293, Tests=83915, 108 wallclock secs (25.64 usr 2.09 sys + 119.29 cusr 9.74 csys = 156.76 CPU) Result: PASS All tests successful. Files=293, Tests=83915, 94 wallclock secs (24.76 usr 2.17 sys + 120.24 cusr 10.38 csys = 157.55 CPU) Result: PASS and there could well be another 15 seconds to save in ext/ and lib/ Nicholas Clark

Sat Aug 09 12:36:51 2008 nick [...] ccl4.org - Correspondence added

Subject:	Re: [rt.cpan.org #38355] AutoReply: interaction of rules and test order
Date:	Sat, 9 Aug 2008 17:36:34 +0100
To:	Bugs in Test-Harness via RT <bug-Test-Harness [...] rt.cpan.org>
From:	Nicholas Clark <nick [...] ccl4.org>

On Sat, Aug 09, 2008 at 01:25:10PM +0100, Nicholas Clark wrote: Show quoted text

> On Sat, Aug 09, 2008 at 07:56:09AM -0400, Bugs in Test-Harness via RT wrote: >

> > ------------------------------------------------------------------------- > > I was hoping that I could get the core tests running in parallel, > > slowest first. So I tried this patch to t/harness

>

> > So I know that the state file is being written out correctly, and with > > good data. So how easy (or hard) is it to merge the order of the passed > > in tests with the constraints of the ruleset, and hence run the slowest > > available test within parallel groups?

Aha. It does. Just not how I want it to. Within a glob pattern, it keeps the tests in the same order as passed in, so for my @next = qw(comp cmd run io op uni mro lib); push @next, 'japh' if $torture; push @next, 'win32' if $^O eq 'MSWin32'; push @seq, { par => [ map { "$_/*.t" } @next ] }; I have comp/*.t ordered then cmd/*.t ordered then run/*.t ordered then io/*.t So clearly I want a *single* glob pattern {comp,cmd,run,io,op,uni,mro,lib}/*.t Which it doesn't do. Yet. Patch appended. However, when trying this, I hit 2 problems. 1: App::Prove::State::get_tests(0, @tests); will return all the tests it knew about from the previous run (in the correct order), rather than just the tests in @tests (in the correct order). Is that a bug, or a feature? 2: The way prove does things: if ($state) { $h->callback( after_test => sub { $state->observe_test(@_); } ); } gets *horribly* slow in the core (factor of 4 slowdown). I'm not sure why. This is being called once per file, isn't it? Nicholas Clark ==== //depot/perl/lib/TAP/Parser/Scheduler.pm#1 - /Volumes/Stuff/p4perl/perl/lib/TAP/Parser/Scheduler.pm ==== --- /tmp/tmp.9162.57 2008-08-09 17:23:26.000000000 +0100 +++ /Volumes/Stuff/p4perl/perl/lib/TAP/Parser/Scheduler.pm 2008-08-09 16:00:49.000000000 +0100 @@ -110,17 +110,43 @@ sub _rule_clause { ); } +sub _glob_to_regexp { + my ( $self, $glob ) = @_; + + my $pattern; + + while (1) { + if ( $glob =~ /\G\*/gc ) { + # * is zero or more characters within a filename/directory name + $pattern .= '[^/]*'; + } elsif ( $glob =~ /\G\?/gc ) { + # ? is exactly one character within a filename/directory name + $pattern .= '[^/]'; + } elsif ( $glob =~ /\G\*\*/gc ) { + # ** is any number of characters, including /, within a pathname + $pattern .= '.*?'; + } elsif ( $glob =~ /\G\{([^}]+)\}/gc ) { + # {foo,bar,baz} is any of foo, bar or baz. + # Can't cope with nested {} yet + $pattern .= '(?:' . + join ('|', map { $self->_glob_to_regexp($_) } split ',', $1) + . ')'; + } elsif ( $glob =~ /\G(\\.)/gc ) { + # A quoted literal + $pattern .= $1; + } else { + # Eat everything that is not a meta character. + $glob =~ /\G([^{?*\\]*)/gc; + $pattern .= quotemeta $1; + } + return $pattern if pos $glob == length $glob; + } +} + sub _expand { my ( $self, $name, $tests ) = @_; - $name =~ s{(\?|\*\*?|.)}{ - $1 eq '?' ? '[^/]' - : $1 eq '*' ? '[^/]*' - : $1 eq '**' ? '.*?' - : quotemeta($1); - }gex; - - my $pattern = qr{^$name$}; + my $pattern = $self->_glob_to_regexp($name); my @match = (); for ( my $ti = 0; $ti < @$tests; $ti++ ) {

Sat Aug 09 15:17:43 2008 nick [...] ccl4.org - Correspondence added

Subject:	Re: [rt.cpan.org #38355] AutoReply: interaction of rules and test order
Date:	Sat, 9 Aug 2008 20:16:22 +0100
To:	Bugs in Test-Harness via RT <bug-Test-Harness [...] rt.cpan.org>
From:	Nicholas Clark <nick [...] ccl4.org>

On Sat, Aug 09, 2008 at 05:36:33PM +0100, Nicholas Clark wrote: Show quoted text

> 1: App::Prove::State::get_tests(0, @tests); will return all the tests it knew > about from the previous run (in the correct order), rather than just the > tests in @tests (in the correct order). Is that a bug, or a feature?

Still pertinent. Show quoted text

> 2: The way prove does things: > > if ($state) { > $h->callback( > after_test => sub { > $state->observe_test(@_); > } > ); > } > > gets *horribly* slow in the core (factor of 4 slowdown). I'm not sure why. > This is being called once per file, isn't it?

It helps a lot of one anchors one's regexps. The appended patch works. It also fixes a second bug in the previous patch - ** has to come first in the parser, else it's treated as * then *. I think that it may be possible to do nested {}s by having a "fixup" in the /\G\{([^}]+)\}/gc case that spots if $1 contains {, and tries to find the next } in the trailing string. (Mmm. Not sure how it would cope with {A{B}C{D}E} let alone {A{B}C{D{E}F}G} Probably do-able) Nicholas Clark ==== //depot/perl/lib/TAP/Parser/Scheduler.pm#1 - /Volumes/Stuff/p4perl/perl/lib/TAP/Parser/Scheduler.pm ==== --- /tmp/tmp.33468.18 2008-08-09 19:40:26.000000000 +0100 +++ /Volumes/Stuff/p4perl/perl/lib/TAP/Parser/Scheduler.pm 2008-08-09 19:29:31.000000000 +0100 @@ -110,17 +110,44 @@ sub _rule_clause { ); } +sub _glob_to_regexp { + my ( $self, $glob ) = @_; + + my $pattern; + + while (1) { + if ( $glob =~ /\G\*\*/gc ) { + # ** is any number of characters, including /, within a pathname + $pattern .= '.*?'; + } elsif ( $glob =~ /\G\*/gc ) { + # * is zero or more characters within a filename/directory name + $pattern .= '[^/]*'; + } elsif ( $glob =~ /\G\?/gc ) { + # ? is exactly one character within a filename/directory name + $pattern .= '[^/]'; + } elsif ( $glob =~ /\G\{([^}]+)\}/gc ) { + # {foo,bar,baz} is any of foo, bar or baz. + # Can't cope with nested {} yet + $pattern .= '(?:' . + join ('|', map { $self->_glob_to_regexp($_) } split ',', $1) + . ')'; + } elsif ( $glob =~ /\G(\\.)/gc ) { + # A quoted literal + $pattern .= $1; + } else { + # Eat everything that is not a meta character. + $glob =~ /\G([^{?*\\]*)/gc; + $pattern .= quotemeta $1; + } + return $pattern if pos $glob == length $glob; + } +} + sub _expand { my ( $self, $name, $tests ) = @_; - $name =~ s{(\?|\*\*?|.)}{ - $1 eq '?' ? '[^/]' - : $1 eq '*' ? '[^/]*' - : $1 eq '**' ? '.*?' - : quotemeta($1); - }gex; - - my $pattern = qr{^$name$}; + my $pattern = $self->_glob_to_regexp($name); + $pattern = qr/^$pattern$/; my @match = (); for ( my $ti = 0; $ti < @$tests; $ti++ ) {

Sat Aug 09 15:43:44 2008 andy [...] hexten.net - Correspondence added

Patch applied thanks :)

Sat Aug 09 15:43:48 2008 The RT System itself - Status changed from 'new' to 'open'

Sat Aug 09 15:43:50 2008 andy [...] hexten.net - Given to ANDYA

Sat Sep 13 09:42:34 2008 andy [...] hexten.net - Correspondence added

I assume this can be closed now Nick? I think you've gone way beyond this stage.

Sat Sep 13 09:42:41 2008 andy [...] hexten.net - Status changed from 'open' to 'resolved'