datetime review part 2 [Update 4]

Jonathan M Davis jmdavisProg at gmx.com
Tue Nov 9 16:11:59 PST 2010


On Tuesday, November 09, 2010 15:48:30 bearophile wrote:
> Yao G.:
> > Ugh. The datetime.d file is 1.5 MB? 0_o

Heavy unit testing and documenting everything adds up. The implementation isn't 
all that small either, but it's the unit tests and ddoc comments which really 
increase the size. Most of that will never end up in any user program.

> It contains too-much-repetitive code like:
> 
>         assertEqual(SysTime(DateTime(2010, 8, 1, 23, 59, 59),
> FracSec.from!"msecs"(999)).dayOfGregorianCal, 733_985);
> assertEqual(SysTime(DateTime(2010, 8, 31, 23, 59, 59),
> FracSec.from!"msecs"(999)).dayOfGregorianCal, 734_015);
> assertEqual(SysTime(DateTime(2010, 9, 1, 23, 59, 59),
> FracSec.from!"msecs"(999)).dayOfGregorianCal, 734_016);
> assertEqual(SysTime(DateTime(2010, 9, 30, 23, 59, 59),
> FracSec.from!"msecs"(999)).dayOfGregorianCal, 734_045);
> assertEqual(SysTime(DateTime(2010, 10, 1, 23, 59, 59),
> FracSec.from!"msecs"(999)).dayOfGregorianCal, 734_046);
> assertEqual(SysTime(DateTime(2010, 10, 31, 23, 59, 59),
> FracSec.from!"msecs"(999)).dayOfGregorianCal, 734_076);
> assertEqual(SysTime(DateTime(2010, 11, 1, 23, 59, 59),
> FracSec.from!"msecs"(999)).dayOfGregorianCal, 734_077);
> 
> 
> My suggestions:
> - Keep only 5-10% of the tests inside the datetime.d module, and move the
> other 90-95% in a separated datetime test module. - Use deterministic
> (repeatable) random data in a loop to perform some compact fuzzy testing -
> Use alias or string mixing to greatly reduce the amount of text contained
> in the tests. Tuples help a lot. All those lines may be replaced with a
> single loop that contains something like: assertEqual(SysTime(DateTime(z0,
> x1, x2, x3, x4, x5), FracSec.from!"msecs"(x6)).dayOfGregorianCal, x7)
> Where the xn data comes from an array of typecons.Tuples like:
> Data5(2010, 11, 1, 23, 59, 59, 999, 734_077)

While there seem to be a lot of tests, they've turned out to be _really_ helpful 
and by their very nature, they _need_ to be repetitive. Testing a lot of stuff 
with slight differences is where errors are likely to be found. I've definitely 
found errors in my code where there were a lot of tests that were similar but 
one of them which was just a bit different from the ones next to it managed to 
find a bug due to whatever quirk in the logic made it a boundary of some kind in 
the calculation. Having all of those similar tests is valuable.

Now, some of those could be turned into loops, but that would complicate the 
code, it would be harder to figure out what failed (since things would have the 
same line number - though using assertEqual() instead of assert would certainly 
help), and many of the tests are different enough that it wouldn't work anyway. 
In fact, while a lot of the tests are similar, many of them aren't in a pattern 
that would turn into a loop very well.

Also, the unit testing model in D and how Phobos is set up isn't really intended 
to have the unit tests separate from the code. Doing so would be cumbersome and 
more bug prone. It's irritating enough to have to put the tests for some of the 
templated types after the type definition rather than in it. Managing the tests 
is _much_ easier when they're right next to the functions that they're testing. 
On top of that, many of them need private access, which they can't get in 
another module. So, while some could be moved out, I don't think that it would 
be a good idea, and many couldn't be moved out even if you wanted to.

I grant you that there are a lot of unit tests, and ways probably could be found 
to trim them down, but they've helped a _lot_ in ensuring the code is correct, 
they have no effect whatsoever on user code since they'll be compiled-out, and I 
have found that maintaining them next to the functions that they're testing is 
actually easier to manage that it would be to try and split them out. So, I'm 
not particularly inclined to drastically change my tests.

I think that the real question here is how good the API is.

- Jonathan M Davis


More information about the Digitalmars-d mailing list