On Mon, Jul 11, 2005 at 01:17:48PM -0700, Michael G Schwern wrote: > On Mon, Jul 11, 2005 at 07:38:57PM +0300, Yuval Kogman wrote: > > > So what should I do to eliminate it? > > > > Maybe Just Nothing > > > > The issue is that you can't special case get_current_coords to be > > truish, as far as Devel::Cover is concerned - it might not be. > > > > Any fix that could be thought up is inherently problematic. > > > > Coverage reporting is not done for the pretty colors - a human reads > > it, and says "OK, this is logical, get_current_coords always returns > > a true value". It's not a race for greens and percentages. > > While I agree coverage is not a race, I disagree that a human should have > to disambiguate between real missing coverage and a false negative. At > least not more than once.
Quite. > I'll make the same argument "no broken windows" argument here that I do > about warnings and tests: elminate all warnings, even if they are dubious. > Ensure all tests pass eliminating all false negatives. Do not leave any > "expected warnings" or "expected failures" because this erodes the > confidence in the test suite. Warnings and test failures fail to ring alarm > bells. One "expected" warning leads to two. Then four. Then finally too > many to remember which are expected and which are not and you ignore them > all together. > > The Pragmatic Programmer does a good job with this argument. > http://www.pragmaticprogrammer.com/ppbook/extracts/no_broken_windows.html I agree with this completely. The rest of the book is pretty good too. > So goes the same with coverage. Red should be a BAD color, something you > do not want to see. You want to eliminate the red. But sometimes its a > false negative. In that case there should be some way to tell the tool > that it is, in fact, a false negative. Just like skipping tests, you > store the fact that there is a false negative to make the red go away. Red > remains a bad color and seeing it means something is wrong. The team doesn't > have to remember which bits are expected to be uncovered and which are not. > > What's missing is a way to let Devel::Cover know that a bit of coverage is > not necessary. I've called this concept "uncoverable" code, and if you grep the current Devel::Cover source code for uncoverable you'll find a bit of code designed to solve this problem. But it's not complete, so I've neither documented it nor advertised it. Things have proceeded much further in my development code and I'm hoping to have something working properly before YAPC::EU. (No promises though.) > The first way to do this which pops into my mind is a comment. > > my $foo = $bar || default(); # DC ignore X|0 > > "Hey, Devel::Cover! Ignore the case where the right side of this logic is > false." I wasn't particularly happy with the idea of needing to change the source code just to satisfy some tool. I feel the same way about doing things to shut up lint, for example, or to satisfy some arbitrary metric. That's why I've initially stored information in a .uncoverable file. But everyone I've spoken to about this has either not worried about or actively preferred to annotate the source. Many of those have also admitted to adding pod and even tests inline, which tells me that I really should be ignoring their opinions, but I try to please my users anyway, and so I'll see what I can do. > Ignored conditions would be green, but perhaps a slightly different shade of > green so they can be spotted if you're looking for them. Yes, it should be possible to easily find this uncoverable code amongst the coverage. It should also be possible to note why the code is uncoverable. I've also found it useful to have different classes of uncoverable code. Why is it that my TODO list only gets longer? -- Paul Johnson - [EMAIL PROTECTED] http://www.pjcj.net
