On 10/10/2016 05:09 PM, James E Keenan wrote: > On 10/10/2016 12:59 PM, pali@cpan.org wrote: >> On Thursday 29 September 2016 21:22:26 pali@cpan.org wrote: >>> Hi! > [snip] > >> >> Hi! I prepared patch for Encode which should fix those problems with >> readonly, magic, COW, ... scalars. Can you look at it? It is correct now? >> > > Since I'm not very familiar with Encode per se, I decided that, in > reviewing this patch, I would focus on the tests you have added. > > I cloned Encode from its github repository > (https://github.com/dankogai/p5-encode), built it locally and then ran > it through Devel-Cover to see how thoroughly its test suite exercised > the source code. That means I ran: > > ##### > HARNESS_PERL_SWITCHES=-MDevel::Cover make test > cover > ##### > > There's clearly room for improvement, as this excerpt from the coverage > analysis suggests: > > ##### > ------------------------------ ------ ------ ------ ------ > File stmt bran cond sub > ------------------------------ ------ ------ ------ ------ > blib/lib/Encode.pm 70.5 57.0 80.0 74.1 > ... > blib/lib/encoding.pm 61.8 36.3 30.0 92.8 > ##### > > But at least we have a baseline. > > I then created a branch and used 'git am' to apply Pali's patch, which > applied cleanly. I then rebuilt Encode and ran 'make test', wherein all > tests PASSED. > > I then called: > > ##### > cover -delete > HARNESS_PERL_SWITCHES=-MDevel::Cover make test > ##### > > ... whereat I was surprised to get test failures (attached as > 'harness.pali.txt'). > > Can anyone see why the code revisions in the patch and Devel::Cover do > not play nice with each other? > > ##### > $ perl -v | head -2 | tail -1 > This is perl 5, version 24, subversion 0 (v5.24.0) built for x86_64-linux > $ perl -MDevel::Cover -E 'say $Devel::Cover::VERSION' 2> /dev/null | ack > '^\d\.\d+$' > ##### > Some data gleaned from throwing in some debugging statements into t/Encode.t near each of the two test failures: under the harness we're gagging on these characters: \x{100} \316 Here are the individual tests failing only under the harness: "\x{100}" =~ /(.)/; ok( is_utf8($1)); # ID 20011127.151 ... my $latin1 = find_encoding('latin1'); my $orig = "\316"; $orig =~ /(.)/; is $latin1->encode($1), $orig, '[cpan #115168] passing magic regex globals to encode';Thread Previous | Thread Next