Paul Johnson wrote:
>>>  1.  Find some nice way expressing what is uncoverable.  For subroutines
>>>      this is easy.  For statements it is not hard.  For branches it is
>>>      tricky and for conditions I'm somewhat stumped.  The current
>>>      method I use is based on implementation details.
>> What's the current syntaxes you're toying with?
> 
> from tests/.uncoverable:
> 
> tests/uncoverable statement 20f752895295d69b6b73da4d9921a9ed 0 0 Can't get to 
> this statement
> tests/uncoverable statement 89c43a57239a432b0517549f6b8f9feb 0 0 Can't get to 
> this statement
> tests/uncoverable statement ba33ccd5188f01ebbd59be35038fe0cb 0 0 Can't get to 
> this statement
> tests/uncoverable branch 9389290602dbf70585e0467e6cb28669 0 0 Branch can't be 
> true
> tests/uncoverable condition 9389290602dbf70585e0467e6cb28669 0 0 $x can't be 
> false
> tests/uncoverable condition 9389290602dbf70585e0467e6cb28669 0 2 $y can't be 
> false
> tests/uncoverable subroutine ba33ccd5188f01ebbd59be35038fe0cb 0 0 Can't run 
> this subroutine
> 
> that is:
> 
>   file,
>   criterion,
>   md5 of line,
>   count (nth such construct on the line),
>   type (identification of the part of the branch or condition)
>   reason

*snip*

> I suspect that the sets of those who would annotate uncoverable code inline,
> those who would use a separate file and those who wouldn't bother at all
> probably correlates fairly well with those who add pod inline, those who add
> it at the end or separately and those who don't document at all.

I don't think this is a good measure.  The reason usually given for not using 
inline POD is that since there's nearly as much POD as code it clutters up the 
code and makes it difficult to read.  This is not the case with an uncoverable 
marker.  For one, uncover markers should be rare.  Another, it should be no 
more obtrusive than an end-of-line comment.

        else {
                die "Can't get here";  # uncoverable
        }


> Well, maybe not exactly - I can probably make good arguments for inline
> annotation.

Allow me. :)

The current scheme won't track changes to the source file well.  Using file + 
md5 rather than file + line helps some, but problems remain.  Not dealing well 
with code change is a much worse problem than having to markup your source.  
You markup once.  You change code a lot.  O(1) vs O(n).

* What happens when I trivially alter an uncoverable line?  Add a comment, 
change the spacing,
  move it to a different indentation level?
* What happens when I have two duplicate lines in the same file, one I want to 
cover one I don't?
* What happens when I rename a file with uncoverable elements?
* What happens when I move code from one file to another?

And the usual drawbacks of not being inline.

* Having to drag around a separate file just for coverage.
* Having to shell out to a separate utility and describe to it which bit not to 
cover.

Additionally, the use of an md5 sum to identify the line means I cannot easily 
know which lines are marked as uncoverable even with the uncover file open in 
front of me.

I predict a lot of churn of the form:
1) Edit the code
2) Run the coverage
3) Read warnings about uncover lines no longer existing in the source file 
  (maybe, they've just been altered, maybe they've been deleted.  Who knows?)
4) Carefully scanning my coverage results to see what was marked as uncovered 
has lost 
   that marker because of a trivial change to the line.
5) Updating the uncover file.

This is no fun.

If I might make a suggestion for an inline syntax.  This is statement, 
subroutine, branch and condition respectively.

        statement;  # uncoverable "Optional Reason"

        sub foo {   # uncoverable "Optional Reason"
                ...
        }

        if( condition ) {      # uncoverable "Optional Reason"

        if( this || that ||
            unreachable        # uncoverable "Optional Reason"
        )
        {
                ...
        }


The condition coverage marker does require some reformatting of the code.  That 
probably won't be a big deal given how infrequently it should be used.  
However, maybe something a bit more explicit could be used.

        if( this || that || unreachable ) {  # uncoverable condition/3 
"Optional Reason"
                ...
        }

That states that the 3rd part of the condition on that line is uncoverable.

Reply via email to