The patch attached significantly improves the test coverage of
configuration step class auto::msvc.  To make this class more testable,
the guts of its runstep() method have been refactored out into two
subroutines

The first is a subroutine, _probe_for_msvc(), which wraps around the
construction and execution of the C program which probes for attributes.

The second is an internal method, _evaluate_msvc(), which accepts the
output of the first subroutine as one of its arguments.  But the
refactoring permits us to mock the output of the C probe and thereby
test more of the nooks and crannies in _evaluate_msvc() than was
possible when the code was found entirely within runstep().

Three new test files supersede one earlier test file.

This should have no impact on functionality, but I'd appreciate hearing
from Win32 developers before I commit this to trunk.

Thank you very much.
kid51

Index: MANIFEST
===================================================================
--- MANIFEST    (revision 22529)
+++ MANIFEST    (working copy)
@@ -1,7 +1,7 @@
 # ex: set ro:
 # $Id$
 #
-# generated by tools/dev/mk_manifest_and_skip.pl Sat Oct 27 12:12:46 2007 UT
+# generated by tools/dev/mk_manifest_and_skip.pl Sat Oct 27 17:58:34 2007 UT
 #
 # See tools/dev/install_files.pl for documentation on the
 # format of this file.
@@ -3006,7 +3006,9 @@
 t/configure/110-inter_yacc-05.t                             []
 t/configure/111-auto_gcc.t                                  []
 t/configure/112-auto_backtrace.t                            []
-t/configure/113-auto_msvc.t                                 []
+t/configure/113-auto_msvc-01.t                              []
+t/configure/113-auto_msvc-02.t                              []
+t/configure/113-auto_msvc-03.t                              []
 t/configure/114-auto_attributes.t                           []
 t/configure/115-auto_warnings.t                             []
 t/configure/116-init_optimize-01.t                          []
Index: t/configure/113-auto_msvc-01.t
===================================================================
--- t/configure/113-auto_msvc-01.t      (revision 0)
+++ t/configure/113-auto_msvc-01.t      (revision 0)
@@ -0,0 +1,82 @@
+#! perl
+# Copyright (C) 2007, The Perl Foundation.
+# $Id$
+# 113-auto_msvc-01.t
+
+use strict;
+use warnings;
+use Test::More qw(no_plan); # tests => 11;
+use Carp;
+use lib qw( lib t/configure/testlib );
+use_ok('config::init::defaults');
+use_ok('config::auto::msvc');
+use Parrot::Configure;
+use Parrot::Configure::Options qw( process_options );
+use Parrot::Configure::Test qw( test_step_thru_runstep);
+
+=for hints_for_testing Testing config::auto::msvc::runstep() may be
+meaningless if you are not on Windows.  Consider writing a SKIP block.  Check
+latest reports of Parrot configuration tools testing coverage to see where
+your time available for writing tests is spent.
+
+=cut
+
+my $args = process_options( {
+    argv            => [],
+    mode            => q{configure},
+} );
+
+my $conf = Parrot::Configure->new();
+
+test_step_thru_runstep($conf, q{init::defaults}, $args);
+
+my ($task, $step_name, @step_params, $step, $ret);
+my $pkg = q{auto::msvc};
+
+$conf->add_steps($pkg);
+$conf->options->set(%{$args});
+$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");
+
+ok($step->runstep($conf), "runstep() returned true value");
+
+pass("Completed all tests in $0");
+
+################### DOCUMENTATION ###################
+
+=head1 NAME
+
+113-auto_msvc-01.t - test config::auto::msvc
+
+=head1 SYNOPSIS
+
+    % prove t/configure/113-auto_msvc-01.t
+
+=head1 DESCRIPTION
+
+The files in this directory test functionality used by F<Configure.pl>.
+
+The tests in this file test subroutines exported by config::auto::msvc.
+
+=head1 AUTHOR
+
+James E Keenan
+
+=head1 SEE ALSO
+
+config::auto::msvc, F<Configure.pl>.
+
+=cut
+
+# Local Variables:
+#   mode: cperl
+#   cperl-indent-level: 4
+#   fill-column: 100
+# End:
+# vim: expandtab shiftwidth=4:

Property changes on: t/configure/113-auto_msvc-01.t
___________________________________________________________________
Name: svn:mime-type
   + text/plain
Name: svn:keywords
   + Author Date Id Revision
Name: svn:eol-style
   + native

Index: t/configure/113-auto_msvc-02.t
===================================================================
--- t/configure/113-auto_msvc-02.t      (revision 0)
+++ t/configure/113-auto_msvc-02.t      (revision 0)
@@ -0,0 +1,89 @@
+#! perl
+# Copyright (C) 2007, The Perl Foundation.
+# $Id$
+# 113-auto_msvc-02.t
+
+use strict;
+use warnings;
+use Test::More qw(no_plan); # tests => 11;
+use Carp;
+use lib qw( lib t/configure/testlib );
+use_ok('config::init::defaults');
+use_ok('config::auto::msvc');
+use Parrot::Configure;
+use Parrot::Configure::Options qw( process_options );
+use Parrot::Configure::Test qw( test_step_thru_runstep);
+
+=for hints_for_testing Testing config::auto::msvc::runstep() may be
+meaningless if you are not on Windows.  Consider writing a SKIP block.  Check
+latest reports of Parrot configuration tools testing coverage to see where
+your time available for writing tests is spent.
+
+=cut
+
+my $args = process_options( {
+    argv            => [],
+    mode            => q{configure},
+} );
+
+my $conf = Parrot::Configure->new();
+
+test_step_thru_runstep($conf, q{init::defaults}, $args);
+
+my ($task, $step_name, @step_params, $step, $ret);
+my $pkg = q{auto::msvc};
+
+$conf->add_steps($pkg);
+$conf->options->set(%{$args});
+$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 $msvcref = { _MSC_VER => 1399 };
+ok($step->_evaluate_msvc($conf, $msvcref),
+    "_evaluate_msvc returned true value");
+
+is($step->result, 'yes', "Got expected result");
+
+is($conf->data->get('msvcversion'), '13.99',
+    "Got expected msvc version string");
+
+pass("Completed all tests in $0");
+
+################### DOCUMENTATION ###################
+
+=head1 NAME
+
+113-auto_msvc-02.t - test config::auto::msvc
+
+=head1 SYNOPSIS
+
+    % prove t/configure/113-auto_msvc-02.t
+
+=head1 DESCRIPTION
+
+The files in this directory test functionality used by F<Configure.pl>.
+
+The tests in this file test subroutines exported by config::auto::msvc.
+
+=head1 AUTHOR
+
+James E Keenan
+
+=head1 SEE ALSO
+
+config::auto::msvc, F<Configure.pl>.
+
+=cut
+
+# Local Variables:
+#   mode: cperl
+#   cperl-indent-level: 4
+#   fill-column: 100
+# End:
+# vim: expandtab shiftwidth=4:

Property changes on: t/configure/113-auto_msvc-02.t
___________________________________________________________________
Name: svn:mime-type
   + text/plain
Name: svn:keywords
   + Author Date Id Revision
Name: svn:eol-style
   + native

Index: t/configure/113-auto_msvc-03.t
===================================================================
--- t/configure/113-auto_msvc-03.t      (revision 0)
+++ t/configure/113-auto_msvc-03.t      (revision 0)
@@ -0,0 +1,95 @@
+#! perl
+# Copyright (C) 2007, The Perl Foundation.
+# $Id$
+# 113-auto_msvc-03.t
+
+use strict;
+use warnings;
+use Test::More qw(no_plan); # tests => 11;
+use Carp;
+use lib qw( lib t/configure/testlib );
+use_ok('config::init::defaults');
+use_ok('config::auto::msvc');
+use Parrot::Configure;
+use Parrot::Configure::Options qw( process_options );
+use Parrot::Configure::Test qw( test_step_thru_runstep);
+
+=for hints_for_testing Testing config::auto::msvc::runstep() may be
+meaningless if you are not on Windows.  Consider writing a SKIP block.  Check
+latest reports of Parrot configuration tools testing coverage to see where
+your time available for writing tests is spent.
+
+=cut
+
+my $args = process_options( {
+    argv            => [],
+    mode            => q{configure},
+} );
+
+my $conf = Parrot::Configure->new();
+
+test_step_thru_runstep($conf, q{init::defaults}, $args);
+
+my ($task, $step_name, @step_params, $step, $ret);
+my $pkg = q{auto::msvc};
+
+$conf->add_steps($pkg);
+$conf->options->set(%{$args});
+$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 $msvcref = { _MSC_VER => 1400 };
+ok($step->_evaluate_msvc($conf, $msvcref),
+    "_evaluate_msvc returned true value");
+
+is($step->result, 'yes', "Got expected result");
+
+is($conf->data->get('msvcversion'), '14.0',
+    "Got expected msvc version string");
+
+like(
+    $conf->data->get('ccflags'),
+    qr/\-D_CRT_SECURE_NO_DEPRECATE/,
+    "ccflags appropriately adjusted given MSVC version"
+);
+
+pass("Completed all tests in $0");
+
+################### DOCUMENTATION ###################
+
+=head1 NAME
+
+113-auto_msvc-03.t - test config::auto::msvc
+
+=head1 SYNOPSIS
+
+    % prove t/configure/113-auto_msvc-03.t
+
+=head1 DESCRIPTION
+
+The files in this directory test functionality used by F<Configure.pl>.
+
+The tests in this file test subroutines exported by config::auto::msvc.
+
+=head1 AUTHOR
+
+James E Keenan
+
+=head1 SEE ALSO
+
+config::auto::msvc, F<Configure.pl>.
+
+=cut
+
+# Local Variables:
+#   mode: cperl
+#   cperl-indent-level: 4
+#   fill-column: 100
+# End:
+# vim: expandtab shiftwidth=4:

Property changes on: t/configure/113-auto_msvc-03.t
___________________________________________________________________
Name: svn:mime-type
   + text/plain
Name: svn:keywords
   + Author Date Id Revision
Name: svn:eol-style
   + native

Index: t/configure/113-auto_msvc.t
===================================================================
--- t/configure/113-auto_msvc.t (revision 22529)
+++ t/configure/113-auto_msvc.t (working copy)
@@ -1,80 +0,0 @@
-#! perl
-# Copyright (C) 2007, The Perl Foundation.
-# $Id$
-# 113-auto_msvc.t
-
-use strict;
-use warnings;
-use Test::More tests => 10;
-use Carp;
-use lib qw( lib t/configure/testlib );
-use_ok('config::init::defaults');
-use_ok('config::auto::msvc');
-use Parrot::Configure;
-use Parrot::Configure::Options qw( process_options );
-use Parrot::Configure::Test qw( test_step_thru_runstep);
-
-=for hints_for_testing Testing config::auto::msvc::runstep() may be
-meaningless if you are not on Windows.  Consider writing a SKIP block.  Check
-latest reports of Parrot configuration tools testing coverage to see where
-your time available for writing tests is spent.
-
-=cut
-
-my $args = process_options( {
-    argv            => [],
-    mode            => q{configure},
-} );
-
-my $conf = Parrot::Configure->new();
-
-test_step_thru_runstep($conf, q{init::defaults}, $args);
-
-my ($task, $step_name, @step_params, $step, $ret);
-my $pkg = q{auto::msvc};
-
-$conf->add_steps($pkg);
-$conf->options->set(%{$args});
-$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");
-
-pass("Completed all tests in $0");
-
-################### DOCUMENTATION ###################
-
-=head1 NAME
-
-113-auto_msvc.t - test config::auto::msvc
-
-=head1 SYNOPSIS
-
-    % prove t/configure/113-auto_msvc.t
-
-=head1 DESCRIPTION
-
-The files in this directory test functionality used by F<Configure.pl>.
-
-The tests in this file test subroutines exported by config::auto::msvc.
-
-=head1 AUTHOR
-
-James E Keenan
-
-=head1 SEE ALSO
-
-config::auto::msvc, F<Configure.pl>.
-
-=cut
-
-# Local Variables:
-#   mode: cperl
-#   cperl-indent-level: 4
-#   fill-column: 100
-# End:
-# vim: expandtab shiftwidth=4:
Index: config/auto/msvc.pm
===================================================================
--- config/auto/msvc.pm (revision 22529)
+++ config/auto/msvc.pm (working copy)
@@ -33,13 +33,24 @@
 sub runstep {
     my ( $self, $conf ) = ( shift, shift );
 
-    my $verbose = $conf->options->get('verbose');
+    my $msvcref = _probe_for_msvc();
 
+    $self->_evaluate_msvc($conf, $msvcref);
+
+    return 1;
+}
+
+sub _probe_for_msvc {
     cc_gen("config/auto/msvc/test_c.in");
     cc_build();
     my %msvc = eval cc_run() or die "Can't run the test program: $!";
     cc_clean();
+    return \%msvc;
+}
 
+sub _evaluate_msvc {
+    my ($self, $conf, $msvcref) = @_;
+    my $verbose = $conf->options->get('verbose');
     # Set msvcversion to undef.  This will also trigger any hints-file
     # callbacks that depend on knowing whether or not we're using Visual C++.
 
@@ -47,14 +58,14 @@
     # which should have been caught by the 'die' above.
     # Therefore, test if it's defined to see if MSVC's installed.
     # return 'no' if it's not.
-    unless ( defined $msvc{_MSC_VER} ) {
+    unless ( defined $msvcref->{_MSC_VER} ) {
         $self->set_result('no');
         $conf->data->set( msvcversion => undef );
         return 1;
     }
 
-    my $major = int( $msvc{_MSC_VER} / 100 );
-    my $minor = $msvc{_MSC_VER} % 100;
+    my $major = int( $msvcref->{_MSC_VER} / 100 );
+    my $minor = $msvcref->{_MSC_VER} % 100;
     unless ( defined $major && defined $minor ) {
         print " (no) " if $verbose;
         $self->set_result('no');
@@ -79,7 +90,6 @@
         # for details.
         $conf->data->add( " ", "ccflags", "-D_CRT_SECURE_NO_DEPRECATE" );
     }
-
     return 1;
 }
 

Reply via email to