On 1/2/11 10:44 PM, Jonathan M Davis wrote:
Okay. As some of you are aware, I created a module with unit testing functions
when I wrote std.datetime, and it really helped with writing the unit tests for
std.datetime. It has been at least somewhat reviewed here previously, and that
has definitely improved it. However, it has not yet been decided whether it's
going to go into Phobos along with std.datetime. I'd like it to, but it hasn't
been voted on, and I'm not sure that it's been reviewed as heavily as it should
be. So, I'm posting the most up-to-date version in the hopes of getting any
necessary changes found and made so that it can be voted on. Andrei has yet to
weigh in on std.unittests at all, so I don't know if we're actually going to get
a vote on it, but I'm presenting it for (final?) review so that it can be voted
in if that's what he wants to do.
The code: http://is.gd/jZEVl
Honest I feared I'll see a monster with a lot of useless crap, so the
surprise was nice. I appreciate that the module is small and to the
point. It contains functions that I can easily imagine needing while
unittesting while at the same time are tedious enough to emulate with
inline code.
Suggestions:
* Documentation should contain meaningful, working examples that
exercise the most expected usage patterns of the functions.
* assertEqual and assertNotEqual are misnomers. They should be
consolidated into one function called assertPredicate:
assertPredicate(1 + 1, 2); // pass, default predicate is a == b
assertPredicate!"a < b"(1 + 1, 3); // pass, 1 + 1 < 3
* To emphasize: since we have the awesome short lambdas, there's no need
to for a negation function - we can embed the negation inside the lambda.
* Remove assertCmp and fold it into assertPredicate. With trivial
metaprogramming you can distinguish an operator from an actual expression.
assertPredicate!"<"(1 + 1, 2.1);
* The explanation and the examples do not clarify for me what value
assertOpCmp adds over assertCmp.
* I'd replace assertOpBinary with a ternary predicate and fold that into
assertPredicate, too. That is a bit more difficult but worth looking into.
* I think assertOpOpAssign is too rarely necessary to warrant inclusion.
* Wow, assertPred was there at the bottom the whole time :o). Rename my
assertPredicate into assertPred, fold the existing assertPred into it
and at the end we have a useful library with a one-stop-shop function.
* getMsg() should be renamed as collectExceptionMsg() because we already
have collectException() in std.exception.
If some or all of the suggestions above are implemented (which I'd
love), the library becomes lean and mean enough to be integrated within
an existing module. I think that would be awesome.
Andrei
