1. Configuration step gen::revision was misnamed. It didn't generate any files; it simply probed the system automatically. So I renamed the class, which meant repositioning it within the order of configuration steps in Parrot::Configure::Step::List and changing the names of two test files. This was done in r22700 and r22701.
2. Refactored auto::revision a little to handle weird edge cases. Wrote tests for auto::revision in t/configure/152-auto_revision.t. Code coverage is 100%. This was done in r22702. The patch that did this is attached. kid51
Index: t/configure/152-auto_revision.t =================================================================== --- t/configure/152-auto_revision.t (revision 22701) +++ t/configure/152-auto_revision.t (revision 22702) @@ -5,11 +5,80 @@ use strict; use warnings; -use Test::More tests => 2; +use Test::More tests => 20; use Carp; -use lib qw( lib ); +use lib qw( lib t/configure/testlib ); +use_ok('config::init::defaults'); use_ok('config::auto::revision'); +use Parrot::Configure; +use Parrot::Configure::Options qw( process_options ); +use Parrot::Configure::Test qw( test_step_thru_runstep); +my $args = process_options( + { + argv => [ ], + mode => q{configure}, + } +); + +my $conf = Parrot::Configure->new; + +test_step_thru_runstep( $conf, q{init::defaults}, $args ); + +my $pkg = q{auto::revision}; + +$conf->add_steps($pkg); +$conf->options->set( %{$args} ); + +my ( $task, $step_name, @step_params, $step); +$task = $conf->steps->[1]; +$step_name = $task->step; [EMAIL PROTECTED] = @{ $task->params }; + +$step = $step_name->new(); +ok( defined $step, "$step_name constructor returned defined value" ); +isa_ok( $step, $step_name ); +ok( $step->description(), "$step_name has description" ); + +my ($testrev, $ret); +{ + $testrev = 99999; + local $Parrot::Revision::current = $testrev; + $ret = $step->runstep($conf); + ok( $ret, "$step_name runstep() returned true value" ); + is($conf->data->get('revision'), $testrev, + "'revision' element was set correctly"); + is($step->result(), qq{r$testrev}, "Expected result was set"); +} + +{ + $testrev = 0; + local $Parrot::Revision::current = $testrev; + $ret = $step->runstep($conf); + ok( $ret, "$step_name runstep() returned true value" ); + is($conf->data->get('revision'), $testrev, + "'revision' element was set correctly"); + is($step->result(), q{done}, "Expected result was set"); +} + +{ + $testrev = q{foobar}; + local $Parrot::Revision::current = $testrev; + eval { $ret = $step->runstep($conf); }; + like($@, qr/Cannot use non-numeric revision number/, + "Got expected error message"); +} + +{ + $testrev = undef; + local $Parrot::Revision::current = $testrev; + $ret = $step->runstep($conf); + ok( $ret, "$step_name runstep() returned true value" ); + ok(! defined($conf->data->get('revision')), + "'revision' element is undefined as expected"); + is($step->result(), q{done}, "Expected result was set"); +} + pass("Completed all tests in $0"); ################### DOCUMENTATION ################### Index: config/auto/revision.pm =================================================================== --- config/auto/revision.pm (revision 22701) +++ config/auto/revision.pm (revision 22702) @@ -35,9 +35,13 @@ my $revision = $Parrot::Revision::current; - $conf->data->set( revision => $revision, ); + if ( defined($revision) and $revision !~ /^\d+$/ ) { + die "Cannot use non-numeric revision number: $!"; + } - if ( $revision >= 1 ) { + $conf->data->set( revision => $revision ); + + if ( defined $revision and $revision >= 1 ) { $self->set_result("r$revision"); } else {