www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Documented unittests & code coverage

reply Johannes Pfau <nospam example.com> writes:
Some time ago we moved some example code from documentation comments
into documented unittests. Some of these more complicated examples are
incomplete and therefore are not expected to actually run. These are
also not very useful as real unittests as they do not contain any
asserts or verification of results. Making them documented
unittests was mainly done to make sure these examples keep compiling
when there are library changes.

I just had a quick look at https://github.com/dlang/phobos/pull/4587
and some example output:
https://codecov.io/gh/wilzbach/phobos/src/5fc9eb90076101c0266fb3491ac68527d3520fba/std/digest/digest.d#L106

And it seems that this 'idiom' messes up code coverage results: The
code in the unittest is never executed (we just want syntactically
valid code) and therefore shows up as untested code. The code coverage
shows 46 missed lines for std.digest.digest, but only 8 of these
actually need testing.

So how do we solve this?
* Ignore lines in documented unittests for code coverage?
* Make these examples completely executable, at the expense of
  documentation which will then contain useless boilerplate code
* Move these examples back to the documentation block?


And as a philosophical question: Is code coverage in unittests even a
meaningful measurement? We write unittests to test the library code. But
if there's a line in a unittests which is never executed, this does not
directly mean there's a problem in library code, as long as all library
code is still tested. It may be an oversight in the unittest in the
worst case, but how important are ovesights / bugs in the unittests if
they don't affect library code in any way?
Jul 28 2016
next sibling parent Jonathan M Davis via Digitalmars-d <digitalmars-d puremagic.com> writes:
On Thursday, July 28, 2016 12:15:27 Johannes Pfau via Digitalmars-d wrote:
 And as a philosophical question: Is code coverage in unittests even a
 meaningful measurement? We write unittests to test the library code. But
 if there's a line in a unittests which is never executed, this does not
 directly mean there's a problem in library code, as long as all library
 code is still tested. It may be an oversight in the unittest in the
 worst case, but how important are ovesights / bugs in the unittests if
 they don't affect library code in any way?
https://issues.dlang.org/show_bug.cgi?id=14856 - Jonathan M Davis
Jul 28 2016
prev sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 7/28/2016 3:15 AM, Johannes Pfau wrote:
 And as a philosophical question: Is code coverage in unittests even a
 meaningful measurement?
Yes. I've read all the arguments against code coverage testing. But in my usage of it for 30 years, it has been a dramatic and unqualified success in improving the reliability of shipping code.
Jul 28 2016
next sibling parent Seb <seb wilzba.ch> writes:
On Thursday, 28 July 2016 at 23:14:42 UTC, Walter Bright wrote:
 On 7/28/2016 3:15 AM, Johannes Pfau wrote:
 And as a philosophical question: Is code coverage in unittests 
 even a
 meaningful measurement?
Yes. I've read all the arguments against code coverage testing. But in my usage of it for 30 years, it has been a dramatic and unqualified success in improving the reliability of shipping code.
Walter: the discussion is not about code coverage in general, but whether code coverage within unittests should be reported, because we are only interested in the coverage of the library itself and as Johannes and Jonathan pointed out there are some valid patterns (e.g. scope(failure)) that are used within unittests and never called. However as Jonathan mentioned in the Bugzilla issue, the downside of not counting within unittest blocks is that potential bugs in the unittests can't be catched that easy anymore.
Jul 28 2016
prev sibling next sibling parent reply Jonathan M Davis via Digitalmars-d <digitalmars-d puremagic.com> writes:
On Thursday, July 28, 2016 16:14:42 Walter Bright via Digitalmars-d wrote:
 On 7/28/2016 3:15 AM, Johannes Pfau wrote:
 And as a philosophical question: Is code coverage in unittests even a
 meaningful measurement?
Yes. I've read all the arguments against code coverage testing. But in my usage of it for 30 years, it has been a dramatic and unqualified success in improving the reliability of shipping code.
The issue isn't whether we should have code coverage testing. We agree that that's a great thing. The issue is whether the lines in the unit tests themselves should count towards the coverage results. https://issues.dlang.org/show_bug.cgi?id=14856 gives some good examples of why having the unittest blocks themselves counted in the total percentage is problematic and can lead to dmd's code coverage tool listing than 100% coverage in a module that is fully tested. What's critical is that the code itself has the coverage testing not that the lines in the tests which are doing that testing be counted as part of the code that is or isn't covered. I know that it will frequently be the case that I will not get 100% code coverage per -cov for the code that I write simply because I frequently do stuff like use scope(failure) writefln(...) to print useful information on failure in unittest blocks so that I can debug what happened when things go wrong (including when someone reports failures on their machine that don't happen on mine). D's code coverage tools are fantastic to have, but they do need a few tweaks if we want to actually be reporting 100% code coverage for fully tested modules. A couple of other reports that I opened a while back are https://issues.dlang.org/show_bug.cgi?id=14855 https://issues.dlang.org/show_bug.cgi?id=14857 - Jonathan M Davis
Jul 28 2016
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 7/28/2016 9:48 PM, Jonathan M Davis via Digitalmars-d wrote:
 gives some good examples of why having the unittest blocks themselves
 counted in the total percentage is problematic and can lead to dmd's code
 coverage tool listing than 100% coverage in a module that is fully tested.
 What's critical is that the code itself has the coverage testing not that
 the lines in the tests which are doing that testing be counted as part of
 the code that is or isn't covered.

 I know that it will frequently be the case that I will not get 100% code
 coverage per -cov for the code that I write simply because I frequently do
 stuff like use scope(failure) writefln(...) to print useful information on
 failure in unittest blocks so that I can debug what happened when things go
 wrong (including when someone reports failures on their machine that don't
 happen on mine).

 D's code coverage tools are fantastic to have, but they do need a few tweaks
 if we want to actually be reporting 100% code coverage for fully tested
 modules. A couple of other reports that I opened a while back are
As soon as we start taking the % coverage too seriously, we are in trouble. It's never going to be cut and dried what should be tested and what is unreasonable to test, and I see no point in arguing about it. The % is a useful indicator, that is all. It is not a substitute for thought. As always, use good judgement.
Jul 28 2016
next sibling parent reply Jonathan M Davis via Digitalmars-d <digitalmars-d puremagic.com> writes:
On Thursday, July 28, 2016 22:12:58 Walter Bright via Digitalmars-d wrote:
 As soon as we start taking the % coverage too seriously, we are in trouble.
 It's never going to be cut and dried what should be tested and what is
 unreasonable to test, and I see no point in arguing about it.

 The % is a useful indicator, that is all. It is not a substitute for
 thought.

 As always, use good judgement.
True, but particularly when you start doing stuff like trying to require that modules have 100% coverage - or that the coverage not be reduced by a change - it starts mattering - especially if it's done with build tools. The current situation is far from the end of the world, but I definitely think that we'd be better off if we fixed some of these issues so that the percentage reflected the amount of the actual code that's covered rather than having unit tests, assert(0) statements, invariants, etc. start affecting code coverage when they aren't what you're trying to cover at all. - Jonathan M Davis
Jul 28 2016
next sibling parent Walter Bright <newshound2 digitalmars.com> writes:
On 7/28/2016 10:49 PM, Jonathan M Davis via Digitalmars-d wrote:
 True, but particularly when you start doing stuff like trying to require
 that modules have 100% coverage - or that the coverage not be reduced by a
 change - it starts mattering - especially if it's done with build tools. The
 current situation is far from the end of the world, but I definitely think
 that we'd be better off if we fixed some of these issues so that the
 percentage reflected the amount of the actual code that's covered rather
 than having unit tests, assert(0) statements, invariants, etc. start
 affecting code coverage when they aren't what you're trying to cover at all.
Worrying about this just serves no purpose. Code coverage percentages are a guide, an indicator, not a requirement in and of itself. Changing the code in order to manipulate the number to meet some metric means the reviewer or the programmer or both have failed.
Jul 28 2016
prev sibling parent Seb <seb wilzba.ch> writes:
On Friday, 29 July 2016 at 05:49:01 UTC, Jonathan M Davis wrote:
 On Thursday, July 28, 2016 22:12:58 Walter Bright via 
 Digitalmars-d wrote:
 As soon as we start taking the % coverage too seriously, we 
 are in trouble. It's never going to be cut and dried what 
 should be tested and what is unreasonable to test, and I see 
 no point in arguing about it.

 The % is a useful indicator, that is all. It is not a 
 substitute for thought.

 As always, use good judgement.
True, but particularly when you start doing stuff like trying to require that modules have 100% coverage - or that the coverage not be reduced by a change - it starts mattering - especially if it's done with build tools. The current situation is far from the end of the world, but I definitely think that we'd be better off if we fixed some of these issues so that the percentage reflected the amount of the actual code that's covered rather than having unit tests, assert(0) statements, invariants, etc. start affecting code coverage when they aren't what you're trying to cover at all. - Jonathan M Davis
Yep especially because I think we agree that "coverage [should] not be reduced by a change", except there is a pretty good reason to do so? It could have the negative effect that people won't use such techniques anymore (e.g. debugging in unittests, invariants, ...) as they might develop an evil smell.
Jul 30 2016
prev sibling parent reply Jack Stouffer <jack jackstouffer.com> writes:
On Friday, 29 July 2016 at 05:12:58 UTC, Walter Bright wrote:
 As soon as we start taking the % coverage too seriously, we are 
 in trouble. It's never going to be cut and dried what should be 
 tested and what is unreasonable to test, and I see no point in 
 arguing about it.

 The % is a useful indicator, that is all. It is not a 
 substitute for thought.

 As always, use good judgement.
In the context of the bug, we are not the ones interpreting the statistic, we're the ones measuring and reporting it to users, and it's being measured incorrectly. By deciding not to fix a bug that causes an inaccurate statistic to be reported, you're making a decision on the user's behalf that coverage % is unimportant without knowing their circumstances. If you're going to include coverage % in the report, then a job worth doing is worth doing well.
Jul 28 2016
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 7/28/2016 11:07 PM, Jack Stouffer wrote:
 you're making a decision on the user's behalf that coverage % is
 unimportant without knowing their circumstances.
Think of it like the airspeed indicator on an airplane. There is no right or wrong airspeed. The pilot reads the indicated value, interprets it in the context of what the other instruments say, APPLIES GOOD JUDGMENT, and flies the airplane. You won't find many pilots willing to fly without one.
Jul 29 2016
next sibling parent Chris <wendlec tcd.ie> writes:
On Friday, 29 July 2016 at 07:01:35 UTC, Walter Bright wrote:
 On 7/28/2016 11:07 PM, Jack Stouffer wrote:
 you're making a decision on the user's behalf that coverage % 
 is
 unimportant without knowing their circumstances.
Think of it like the airspeed indicator on an airplane. There is no right or wrong airspeed. The pilot reads the indicated value, interprets it in the context of what the other instruments say, APPLIES GOOD JUDGMENT, and flies the airplane. You won't find many pilots willing to fly without one.
Maybe it would help to give more than one value, e.g. the actual code coverage, i.e. functions and branches executed in the actual program, and commands executed in the unit test. So you would have 100% code coverage 95% total commands executed (but don't worry!)
Jul 29 2016
prev sibling next sibling parent Steven Schveighoffer <schveiguy yahoo.com> writes:
On 7/29/16 3:01 AM, Walter Bright wrote:
 On 7/28/2016 11:07 PM, Jack Stouffer wrote:
 you're making a decision on the user's behalf that coverage % is
 unimportant without knowing their circumstances.
Think of it like the airspeed indicator on an airplane. There is no right or wrong airspeed. The pilot reads the indicated value, interprets it in the context of what the other instruments say, APPLIES GOOD JUDGMENT, and flies the airplane. You won't find many pilots willing to fly without one.
What if the guage was air-speed added to fuel level, and you didn't get a guage for each individually? -Steve
Jul 29 2016
prev sibling parent Jack Stouffer <jack jackstouffer.com> writes:
On Friday, 29 July 2016 at 07:01:35 UTC, Walter Bright wrote:
 The pilot reads the indicated value, interprets it in the 
 context of what the other instruments say, APPLIES GOOD 
 JUDGMENT, and flies the airplane.
Continuing with this metaphor, in this situation you're not the pilot making the judgement, you're the aerospace engineer deciding that the speedometer in the plane can be off by several hundred m/s and it's no big deal. Yes, every measurement in the real world has a margin of error. But, since we're dealing with computers this is one of the rare situations where a perfect number can actually be obtained and presented to the user.
 There is no right or wrong airspeed.
The right one is the actual speed of the plane and the wrong one is every other number.
Jul 29 2016
prev sibling parent reply Atila Neves <atila.neves gmail.com> writes:
On Thursday, 28 July 2016 at 23:14:42 UTC, Walter Bright wrote:
 On 7/28/2016 3:15 AM, Johannes Pfau wrote:
 And as a philosophical question: Is code coverage in unittests 
 even a
 meaningful measurement?
Yes. I've read all the arguments against code coverage testing. But in my usage of it for 30 years, it has been a dramatic and unqualified success in improving the reliability of shipping code.
Have you read this? http://www.linozemtseva.com/research/2014/icse/coverage/ Atila
Aug 04 2016
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 8/4/2016 1:13 AM, Atila Neves wrote:
 On Thursday, 28 July 2016 at 23:14:42 UTC, Walter Bright wrote:
 On 7/28/2016 3:15 AM, Johannes Pfau wrote:
 And as a philosophical question: Is code coverage in unittests even a
 meaningful measurement?
Yes. I've read all the arguments against code coverage testing. But in my usage of it for 30 years, it has been a dramatic and unqualified success in improving the reliability of shipping code.
Have you read this? http://www.linozemtseva.com/research/2014/icse/coverage/
I've seen the reddit discussion of it. I don't really understand from reading the paper how they arrived at their test suites, but I suspect that may have a lot to do with the poor correlations they produced. Unittests have uncovered lots of bugs for me, and code that was unittested had far, far fewer bugs showing up after release. The bugs that did turn up tended to be based on misunderstandings of the requirements. For example, the Warp project was fully unittested from the ground up. I attribute that as the reason for the remarkably short development time for it and the near complete absence of bugs in the shipped product. Unittests also enabled fearless rejiggering of the data structures trying to make Warp run faster. Not-unittested code tends to stick with the first design out of fear.
Aug 04 2016
parent reply Atila Neves <atila.neves gmail.com> writes:
On Thursday, 4 August 2016 at 10:24:39 UTC, Walter Bright wrote:
 On 8/4/2016 1:13 AM, Atila Neves wrote:
 On Thursday, 28 July 2016 at 23:14:42 UTC, Walter Bright wrote:
 On 7/28/2016 3:15 AM, Johannes Pfau wrote:
 And as a philosophical question: Is code coverage in 
 unittests even a
 meaningful measurement?
Yes. I've read all the arguments against code coverage testing. But in my usage of it for 30 years, it has been a dramatic and unqualified success in improving the reliability of shipping code.
Have you read this? http://www.linozemtseva.com/research/2014/icse/coverage/
I've seen the reddit discussion of it. I don't really understand from reading the paper how they arrived at their test suites, but I suspect that may have a lot to do with the poor correlations they produced.
I think I read the paper around a year ago, my memory is fuzzy. From what I remember they analysed existing test suites. What I do remember is having the impression that it was done well.
 Unittests have uncovered lots of bugs for me, and code that was 
 unittested had far, far fewer bugs showing up after release. 
 <snip>
No argument there, as far as I'm concerned, unit tests = good thing (TM). It think measuring unit test code coverage is a good idea, but only so it can be looked at to find lines that really should have been covered but weren't. What I take issue with is two things: 1. Code coverage metric targets (especially if the target is 100%). This leads to inane behaviours such as "testing" a print function (which itself was only used in testing) to meet the target. It's busywork that accomplishes nothing. 2. Using the code coverage numbers as a measure of unit test quality. This was always obviously wrong to me, I was glad that the research I linked to confirmed my opinion, and as far as I know (I'd be glad to be proven wrong), nobody else has published anything to convince me otherwise. Code coverage, as a measure of test quality, is fundamentally broken. It measures coupling between the production code and the tests, which is never a good idea. Consider: int div(int i, int j) { return i + j; } unittest { div(3, 2); } 100% coverage, utterly wrong. Fine, no asserts is "cheating": int div(int i, int j) { return i / j; } unittest { assert(div(4, 2) == 2); } 100% coverage. No check for division by 0. Oops. This is obviously a silly example, but the main idea is: coverage doesn't measure the quality of the sentinel values. Bad tests serve only as sanity tests, and the only way I've seen so far to make sure the tests themselves are good is mutant testing. Atila
Aug 04 2016
next sibling parent Walter Bright <newshound2 digitalmars.com> writes:
On 8/4/2016 12:04 PM, Atila Neves wrote:
 What I take issue with is two things:

 1. Code coverage metric targets (especially if the target is 100%).  This leads
 to inane behaviours such as "testing" a print function (which itself was only
 used in testing) to meet the target. It's busywork that accomplishes nothing.
Any metric that is blindly followed results in counterproductive edge cases. It doesn't mean the metric is pointless, however, it just means that "good judgment" is necessary. I don't think anyone can quote me on a claim that 100% coverage is required. I have said things like uncovered code requires some sort of credible justification. Being part of the test harness is a credible justification, as are assert(0)'s not being executed. Leaving the multi-codepoint Unicode pathway untested probably has no credible justification.
 2. Using the code coverage numbers as a measure of unit test quality. This was
 always obviously wrong to me, I was glad that the research I linked to
confirmed
 my opinion, and as far as I know (I'd be glad to be proven wrong), nobody else
 has published anything to convince me otherwise.

 Code coverage, as a measure of test quality, is fundamentally broken. It
 measures coupling between the production code and the tests, which is never a
 good idea. Consider:
All that means is code coverage is necessary, but not sufficient. Even just executing code and not testing the results has *some* value, in that it verifies that the code doesn't crash, and that it is not dead code. ---- One of the interesting differences between D and C++ is that D requires template bodies to have valid syntax, while C++ requires template bodies to be both syntactically correct and partially semantically correct. The justification for the latter is so that the user won't see semantic errors when instantiating templates, but I interpret that as "so I can ship templates that were never instantiated", a justification that is unsupportable in my view :-)
Aug 04 2016
prev sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
In adding some overflow detection to Phobos, I discovered that some allocations 
were never called by the unittests. Adding a unittest for those paths, I 
discovered those paths didn't work at all for any cases.

I'm not giving up coverage testing anytime soon, regardless of what some study 
claims :-)
Aug 04 2016
next sibling parent Atila Neves <atila.neves gmail.com> writes:
On Friday, 5 August 2016 at 02:37:35 UTC, Walter Bright wrote:
 In adding some overflow detection to Phobos, I discovered that 
 some allocations were never called by the unittests. Adding a 
 unittest for those paths, I discovered those paths didn't work 
 at all for any cases.

 I'm not giving up coverage testing anytime soon, regardless of 
 what some study claims :-)
:) Like I said, measuring coverage is important, what isn't is using it as a measure of the quality of the tests themselves. The other important thing is to decide whether or not certain lines are worth covering, which of course you can only do if you have the coverage data! Mutant testing could have found those code paths you just mentioned, BTW: you'd always get surviving mutants for those paths. Atila
Aug 05 2016
prev sibling parent reply Jonathan M Davis via Digitalmars-d <digitalmars-d puremagic.com> writes:
On Thursday, August 04, 2016 19:37:35 Walter Bright via Digitalmars-d wrote:
 In adding some overflow detection to Phobos, I discovered that some
 allocations were never called by the unittests. Adding a unittest for those
 paths, I discovered those paths didn't work at all for any cases.

 I'm not giving up coverage testing anytime soon, regardless of what some
 study claims :-)
Well, like you said in the previous post, code coverage is important but it's not sufficient. It's always a bad sign when the code coverage isn't 100% (which is part of why we'd like the metrics to actually be accurate), but while testing everything is a huge step in the right direction, you have to have thorough tests to actually verify that the code behaves correctly. And since you can never test everything, it does become a bit of an art to figure out how to sufficiently test a function without going overboard. Too many folks don't test sufficiently IMHO, but then again, I probably tend to go overboard. Still, I've found that being very thorough with unit tests seriously reduces the number of bugs (e.g. very few bugs have ever been reported for std.datetime). Regardless, the fact that D has unit testing built in is a huge win, and even if too many folks don't test their code thoroughly, the fact that it's so easy to add tests to your program makes it embarassing when they don't do at least _some_ testing, and that improves the quality of D code overall even if there's still plenty of code that isn't necessarily at the level of quality that we'd like. - Jonathan M Davis
Aug 05 2016
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 8/5/2016 12:10 PM, Jonathan M Davis via Digitalmars-d wrote:
 [...]
Yes, I pretty much agree with your entire post.
Aug 05 2016
parent Jonathan M Davis via Digitalmars-d <digitalmars-d puremagic.com> writes:
On Friday, August 05, 2016 14:10:02 Walter Bright via Digitalmars-d wrote:
 On 8/5/2016 12:10 PM, Jonathan M Davis via Digitalmars-d wrote:
 [...]
Yes, I pretty much agree with your entire post.
I just wish that you'd agree that issues like these https://issues.dlang.org/show_bug.cgi?id=14855 https://issues.dlang.org/show_bug.cgi?id=14856 https://issues.dlang.org/show_bug.cgi?id=14857 should be fixed so that the code coverage metrics would be more accurate. :) Without those fixes, most of the code that I write will never have 100% code coverage even if the tests do test everything and test it very throughly. - Jonathan M Davis
Aug 05 2016