I was going over our code coverage reports at work and noticed that if the following snippet of code was fully covered, we'd have 100% coverage for one of our modules (with the awkward name of SQL::Stripper):

if ( @chunks > 1 && $chunks[-2] =~ s/$comment$// ) { 
    ...
}

[download]

In this case, I noticed the following for the SQL::Stripper code coverage:

    line !l  l&&!r  l&&r  condition
    94   51  0      2     @chunks > 1 and $chunks-2 =~ s/$comment$//

The number indicates how many times each condition is exercised. The exact code is this:

    @chunks > 1 && $chunks[-2] =~ s/$comment$//

(Note that code as Devel::Cover reports it is often subtly different, but semantically equivalent, because Devel::Cover runs off of the opcodes).

That code is more or less equivalent to:

   $left && $right

There are only three possibly paths through that (note the headings in the Devel::Cover output I listed above):

$left is false
!l
$left is true, $right is $false
l&&!r
$left and $right are $true
l&&r

Because of the way the SQL::Stripper code is structured, if $left is true, $right must be true because $right will always have a $comment to remove from the @chunks element. Thus, we have a code coverage condition that cannot be exercised (l&&!r), so the actual full snippet:

  if ( @chunks > 1 && $chunks[-2] =~ s/$comment$// ) {
      pop @chunks;
  }

[download]

Might be better written as this:

  if ( @chunks > 1 ) {
      $chunks[-2] =~ s/$comment$//;   # XXX remove comment indicator
      pop @chunks;                    # XXX remove the comment
  }

[download]

That will make the code coverage tests report 100% for SQL::Stripper so I made the change. Then I realized the problem. Prior to that snippet is the following code:

        my $split = Data::Record->new(
            {
                split  => $comment,
                unless => $RE{quoted},
                chomp  => 0,
                trim   => 1,
            }
        );
        my @chunks = $split->records($line_of_sql);

[download]

That says "for this individual line of SQL, split it on comments and store the sections in @chunks. Then, if we have more than one chunk (assuming only one comment per line), the last chunk is a comment we can discard and the previous chunk just needs the comment marker removed. However, by changing my code to get 100% code coverage, I've made the code more fragile. Previously, the we effectively had an assertion that there was a comment marker in our line of SQL:

  if ( @chunks > 1 && $chunks[-2] =~ s/$comment$// ) {
      pop @chunks;
  }

[download]

But the new code merely assumes we have that comment marker:

  if ( @chunks > 1 ) {
      $chunks[-2] =~ s/$comment$//;   # XXX remove comment indicator
      pop @chunks;                    # XXX remove the comment
  }

[download]

As a result, if someone alters the code prior to this, the more rigorous code -- which could not achieve 100% code coverage -- would have been safer to use. Using something like Carp::Assert or something similar might help here. It would make the code more robust and allow the 100% code coverage.

That's part of what I like about code coverage. Now that I'm making the above change, the code is a more accurate reflection of what's really happening, though sometimes you really have to sit down and think it through.

Cheers,
Ovid

New address of my CGI Course.

In reply to When 100% Code Coverage Backfires by Ovid

Title:
Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post, it's "PerlMonks-approved HTML":


  • Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!
  • Titles consisting of a single word are discouraged, and in most cases are disallowed outright.
  • Read Where should I post X? if you're not absolutely sure you're posting in the right place.
  • Please read these before you post! —
  • Posts may use any of the Perl Monks Approved HTML tags:
    a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, details, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, summary, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr
  • You may need to use entities for some characters, as follows. (Exception: Within code tags, you can put the characters literally.)
            For:     Use:
    & &amp;
    < &lt;
    > &gt;
    [ &#91;
    ] &#93;
  • Link using PerlMonks shortcuts! What shortcuts can I use for linking?
  • See Writeup Formatting Tips and other pages linked from there for more info.