config/auto/icu.pm: Write unit tests

Coleoid just pasted this failure report:


# Failed test 'Got expected return value: failed'
# at t/steps/auto_icu-04.t line 48.
# got: 'no'
# expected: 'failed'
# Looks like you failed 1 test of 12.
Dubious, test returned 1
(wstat 256, 0x100)
Failed 1/12 subtests

Am looking into it.

kid51

Thanks to *considerable* testing assistance from Jason Cole in several
rounds of off-list correspondence, I have been able to identify the
problem that Jason reported and to trace it to a subtle, long-standing
bug in config/auto/icu.pm.

To diagnose this problem I created a new 'reautoicu' branch in the
repository and added considerable verbose output. Jason and I then
re-configured repeatedly with , as well as
running prove -v t/steps/auto_icu*.t. The verbose output that jason was
getting on Win32 was self-contradictory:

Determining whether ICU is installedDiscovered icu-config
returns 1
icu-config found good!
Trying icu-config with ''
icushared: captured
For icushared, found and 1
Trying icu-config with ''
icuheaders: captured
For icuheaders, found \include and 1
not found.

The line 'icu-config found good!' suggested that Jason had ICU
installed. But it was contradicted by the '1' values in three locations
-- system failure indicators rather than Perl truth -- and by the final
result: 'not found'. If someone really doesn't have ICU installed,
then the step should have exited with a different message at a point
*earlier* than the point at which 'not found' is the result.

To somewhat shorten a long story, there was code deep inside
config/auto/icu.pm like this:

( $arg->{ret} == -1 )
||
( ( $arg->{ret} >8 ) != 0 )

which seems to reflect what's stated here in the man page:

perldoc -f system

The return value is the exit status of the program as
returned
by the "wait" call. To get the actual exit value, shift
right
by eight

That sounded correct, until I asked the question: How is the value of
$arg->{ret} determined? Ultimately, it is set by an invocation of


my ( undef, undef, $ret ) =
capture_output( $self->{icuconfig_default}, "" );

But if we look at the source code for capture_output() (lines 238-260 of
lib/Parrot/Configure/Utils.pm), we see at line 246 that capture_output
*already applies the bitshifting* to the system command it calls. That
means that this line in config/auto/icu.pm:

( ( $arg->{ret} >8 ) != 0 )

is *one bit-shifting too many*!

I tore out the superfluous bit-shifting and both Jason and I were able
to get the expected verbose output during actual configuration and pass
all the tests. As a side benefit, I was able to rip out some branches
and conditions for which I had been unable to write tests and which were
lowering the coverage results.

So I have merged the 'reautoicu' branch into trunk as of r29083. Thanks
again to Coleoid for assistance with testing.

kid51