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 {

Reply via email to