On Wednesday, January 05, 2011 16:09:17 Andrei Alexandrescu wrote: > 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.
Well, most of the functions were created out of need when developing std.datetime rather than just because they seemed useful. If I had simply been trying to create a unit testing module to create a unit testing module, it might have been much larger. Then again, I've tried to come up with more functions which might be useful, and I couldn't think of many. As it is, several of the functions that are there are there because of suggestions in previous threads. > Suggestions: > > * Documentation should contain meaningful, working examples that > exercise the most expected usage patterns of the functions. They should for the most part, but several of them explicitly test overloaded operators, which requires a struct or class with the appropriate operator, which would have made the examples much larger, since there aren't really any in druntime or Phobos. So, in those cases, the examples don't work. Perhaps that was a poor decision though, and I should have just added minimal type definitions to the examples. > * 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. Valid point. assertEqual didn't even take a predicate originally, so it didn't occur to me to combine assertEqual and assertNotEqual. I added the predicate after someone suggested that it be more like std.algorithm.equal. > * 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. The key difference is that assertOpCmp explicitly calls the opCmp() while assertCmp just uses the given operator. This makes it possible to check that opCmp() returns 0 when it's supposed to rather than just having opEquals() checked when checking ==. Now, with a more general predicate, you could feed it opCmp and get around that problem, though it would be slightly more tedious. I fully understand the confusion between the two though. I originally just had assertOpCmp() and added assertCmp() after realizing that assertOpCmp() wouldn't work for primitive types and the like, and people are obviously going to want to be able to test primitives with comparisons other than equality. As long as I can get assertPred to test both the operators and the actual overloaded operator functions, then combining that way would certainly be ideal. > * I'd replace assertOpBinary with a ternary predicate and fold that into > assertPredicate, too. That is a bit more difficult but worth looking into. That may take some doing, but it could certainly be worth doing. I'll look into it. > * I think assertOpOpAssign is too rarely necessary to warrant inclusion. I think that I actually am using it in std.datetime. I don't think that it's something that I would have come up with otherwise. But maybe I'm just more thorough about that sort of testing than most people. You can replace it with two tests though, so getting rid of it isn't necessarily a big deal, just somewhat less efficient. > * 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. Well, it was the last one I came up with (based on a suggestion; I haven't actually used it in std.datetime), so it ended up at the bottom. But if assertPred can indeed be made powerful enough to absorb the functionality of most of the other functions, then that would definitely be worth doing. It'll take a bit of work though. > * getMsg() should be renamed as collectExceptionMsg() because we already > have collectException() in std.exception. Fine with me. I was looking for the functionality. The name isn't terribly important as long as its clear. > 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. I'll try and do most of them. It'll certainly be a challeng to implement such a powerful assertPred though. I'm just glad that D templates are so powerful. I'm not good enough with templates in C++ to even dream of doing some of the stuff that I do with D templates, and some if it just isn't possible. I'm going to be porting a portion of std.datetime to C++ for use at work, and some of the stuff in there (like the nice convert function) just can't be done the same way in C++. After using D templates, C++ templates seem really unwieldy. If I'm able to combine most of the functions into a single assertPred as you suggest, then it does definitely look like a whole module is overkill. However, we're then going to have to decide where to put it. std.exception would probably be the best place, though it does strike me as somewhat of an odd place to put them. Of course, almost all of them _do_ throw AssertError, and the one exception to that (getMsg) operates on an exception (well, Throwable), so they do at least sort of fit. - Jonathan M Davis
