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+$' ##### Thank you very much. Jim KeenanThread Previous | Thread Next