Devel--Cover icon indicating copy to clipboard operation
Devel--Cover copied to clipboard

Condition coverage of 'not(a) or not(b)' differs from not( a and b)

Open thairman opened this issue 10 years ago • 6 comments

Devel::Cover fails to acknowledge the case of both conditions being true when the statement is constructed as

not( A ) or not( B )

The equivalent statement

not( A and B )

works correctly. The result is that condition coverage is falsely reported as 67% when all three possibilities have actually been exercised.

thairman avatar Jan 01 '15 20:01 thairman

I am having trouble figuring out how to attach a file. Here is a testcase that tickles the bug:


#!/usr/bin/env perl use warnings; use strict;

empty(); empty(''); empty('string');

sub empty { my( $arg ) = @_;

print "Arg is '", defined( $arg ) ? $arg : 'undef', "', ";
if ( defined $arg ) {

    print "Arg is defined, Length is ", length( $arg ), "\n";
}
else {

    print "Arg is not defined, Length is *\n";
}

print "   empty 1\n"
    if not( defined $arg )
    or not( length  $arg );

if (
    not( defined $arg )
    or not( length  $arg )
   ) {

    print "   empty 2\n";
}

if (
    not( defined( $arg ) and length( $arg ))
   ) {

    print "   empty 3\n";
}

print "\n";

return;

}

When run, the above testcase produces this output:

Arg is 'undef', Arg is not defined, Length is * empty 1 empty 2 empty 3

Arg is '', Arg is defined, Length is 0 empty 1 empty 2 empty 3

Arg is 'string', Arg is defined, Length is 6


The command used (from bash under Linux) cover -delete; perl -MDevel::Cover /tmp/testcase ; cover -report html


This is Devel::Cover VERSION 1.08 running under Perl This is perl 5, version 18, subversion 2 (v5.18.2) built for x86_64-linux-gnu-thread-multi

thairman avatar Jan 01 '15 20:01 thairman

Sorry aabout the fake close, wrong button.

thairman avatar Jan 01 '15 20:01 thairman

Thanks very much for reporting this. You are correct that the two statements should produce identical coverage and, athough the optrees differ, the B::Deparse output for the two is identical.

I will look into getting a fix for this.

pjcj avatar Aug 03 '15 23:08 pjcj

I think I've found another incantation of this bug, or I think it's the same bug. I'm having trouble producing a simple test case, but here's what the HTML report shows for the 2 different instances of (functionally) the same code:

61  8   100 67          22      die "No ip or pk defined" if ( !$opt{-ip} && !$opt{-pk} );

61  67  A   B   dec    $opt{-'ip'} or $opt{-'pk'}
        0   0   0 (green)
        0   1   1 (red)
        1   X   1 (green)

The first line is from the screen for the file. You can see that that both branches are covered, but more importantly only 67% of the conditionals are covered. The 2nd part is from clicking on the conditional link. There are 2 issues here: (1) it reversed my code (an example of bug #173); (2) it shows false+true as being uncovered when I gave it that data.

To fully illustrate it, I commented out that line of code from my test file and instead did this:

61                          #    die "No ip or pk defined" if ( !$opt{-ip} && !$opt{-pk} );
62  8   100              14     if ( !$opt{-ip} ) {
63  2   100               4         if ( !$opt{-pk} ) {
64  1                     7             die "No ip or pk defined";
65                                  }
66                                  else {
67  1                     3             print "have pk only\n";
68                                  }
69                              }
70                              else {
71  6                    22         print "have ip, pk unknown\n";
72                              }

As you can see, I do visit every line and I believe this is functionally equivalent to the original line 61. The data given is the same in both cases.

This might be important in some way: $ perl -v This is perl 5, version 18, subversion 4 (v5.18.4) built for x86_64-linux-thread-multi-ld

kbrannen avatar Sep 20 '17 21:09 kbrannen

Thanks for the update. Am I right that this is the standard html report? If so, could you try with the html_basic report?

The html report tries to be clever and, as it seems, is either too clever, or not clever enough, depending on how you look at it.

My plan is to move to the html_basic report as the default. This report doesn't try to be clever here and as such is, at least, correct.

pjcj avatar Oct 04 '17 22:10 pjcj

This issue seems to be a duplicate of: https://rt.cpan.org/Ticket/Display.html?id=78801

I just discovered that it must have been fixed in the meantime (broken in 1.18, fixed in 1.29).

mhasch avatar May 07 '19 07:05 mhasch