[phobos] Split std.datetime in two?

[Jonathan M Davis]

On Friday, February 11, 2011 16:56:02 Andrei Alexandrescu wrote:
> On 2/11/11 4:53 PM, Jonathan M Davis wrote:
> > On Friday, February 11, 2011 14:13:29 Steve Schveighoffer wrote:
> >> The limit should be 100% coverage.  If std.datetime passes that, it
> >> should be trimmed.
> > 
> > The problem with that is that all code coverage indicates is whether each
> > line of code is run. In some cases, that may be enough, but as soon as
> > anything like math is involved, it certainly isn't. Just because you
> > covered a particular code path doesn't mean that that code path works
> > correctly. What if a particular value ends up on a particular code path
> > when it should be on another, or if it _does_ belong on that code path,
> > but the result is incorrect for that particular value even though it
> > works with other values?
> 
> Agreed.
> 
> > Also, does code coverage take into account take into account the various
> > specializations of a template? And even if it does, it could be that the
> > template is correctly written for a particular type but not another, and
> > yet both instantiate with it, generating the same code. You'd only catch
> > that if you tested both of those types with the template.
> 
> Agreed.
> 
> > Code coverage is a useful metric, but I don't think that it grasps the
> > whole picture at all. I think that a module which has 100% code coverage
> > can still be buggy, because its tests didn't cover enough cases. There
> > is a difference between testing every code path and testing that the
> > function works correctly for all inputs. Obviously, you're not going to
> > test for _all_ possible inputs - it's completely impractical to do so in
> > most cases - but often, I don't think that even the number of inputs
> > required to get 100% code coverage is sufficient.
> > 
> > By the way, with how -cov is currently set up, getting 100% test coverage
> > is often impossible anyway, simply because you end up with code paths
> > with assert(0) in them which should _never_ happen and so that code path
> > _never_ gets hit. A prime example would be a function which is nothrow
> > but calls functions which can technically throw even though you know
> > that they never will with the given  input. You're pretty much forced to
> > use a catch block with assert(0) in it. Even perfectly tested code will
> > never hit that code path, because it should never actually happen if the
> > code is logically correct.
> 
> Agreed. I was hasty to posit that coverage is enough. That doesn't
> dilute any of my other arguments and points :o).

No, it doesn't dilute them. But if someone argues that the number of tests in 
std.datetime (or anything else for that matter) should be reduced based on the 
fact that it doesn't need that many to reach 100% code coverage, I'm going to 
argue strongly against that. There may be other valid arguments for reducing the 
number of tests, and there are obviously arguments for consolidating how much 
code is used to test a particular amount (as you've been arguing in the case of 
std.datetime), but I won't buy that reaching 100% code coverage is sufficient. Low 
code coverage is definitely a bad sign, but high code coverage doesn't 
necessarily mean that the tests are anywhere near sufficient.

- Jonathan M Davis