Here's an additional problem I noticed with auto::msvc. During coverage analysis, there were several lines of code that I was never able to cover successfully (see: http://thenceforward.net/parrot/coverage/configure-build/config-auto-msvc-pm.html).
unless ( defined $msvcref->{_MSC_VER} ) {
$self->set_result('no');
$conf->data->set( msvcversion => undef );
return 1;
}
my $major = int( $msvcref->{_MSC_VER} / 100 ); # <-- line 69
my $minor = $msvcref->{_MSC_VER} % 100;
unless ( defined $major && defined $minor ) {
print " (no) " if $verbose;
$self->set_result('no');
$conf->data->set( msvcversion => undef );
return 1;
}
By the time we get to line 69, $msvcref->{_MSC_VER} is always defined.
My hunch is that $msvcref->{_MSC_VER} should always be an integer like
'1401'. If that's the case, then $major and $minor will always be
defined as well -- in which case the block of code inside the 'unless'
block can never be reached and never covered by tests.
What if my hunch were wrong, and $msvcref->{_MSC_VER} were some other
defined value, such as a non-integer string like 'foobar'. See attached
test 'msvcdefined.t'. Although you will get a warning when you try to
divide a nonnumeric string, both $major and $minor will be defined in
that case as well.
$> prove -v msvcdefined.t
msvcdefined....Argument "foobar" isn't numeric in division (/) at
msvcdefined.t line 28.
Argument "" isn't numeric in division (/) at msvcdefined.t line 28.
ok 1 - major is defined
ok 2 - minor is defined
ok 3 - major is defined
ok 4 - minor is defined
ok 5 - major is defined
ok 6 - minor is defined
ok 7 - major is defined
ok 8 - minor is defined
ok 9 - major is defined
ok 10 - minor is defined
1..10
ok
All tests successful.
Files=1, Tests=10, 0 wallclock secs ( 0.07 cusr + 0.03 csys = 0.10 CPU)
So, unless my logic is incorrect, I think we can refactor that 'unless'
block out of config/auto/msvc.pm.
Does that seem correct?
Thank you very much.
kid51
msvcdefined.t
Description: Binary data
