Jonathan M Davis wrote: > On Saturday 20 November 2010 10:16:55 Lutger Blijdestijn wrote: >> I'm not particularly fond of this interface and think that a solution >> with a delegate / lazy or alias template parameter would be more >> convenient. However, until we have ast macros I do see the added value in >> this approach. >> >> Some remarks about the api, not a proper review of the code itself: >> >> - core.exception and std.string must be imported to use the module, >> relevant symbols should be selectively and publicy imported instead. > > I hadn't thought of that. Good idea. > >> - exception would be better abbreviated as ex instead of exc, I believe >> this is more conventional. (assertExThrown instead of assertExcThrown) > > I'll think about it. I don't think that I've often seen it abbreviated at > all. But if Ex really is more common, then that would be better.
I know of std.exception.enforceEx, and catch(Exception ex) is also regularly used in examples. >> - assertExcThrown should fail if an exception is thrown that is not T or >> a subclass of T. (catch and throw AssertError) > > It currently catches the exact exception (well, throwable) that you tell > it to. The others are let through exactly like they would be with an > assert. It also means that you get to see exactly what the erroneous > exception was so that you can deal with it. An AssertError would be less > informative, maybe even misleading. You get an AssertError because it > didn't throw any exception, whereas you get the wrong exception if it > threw the wrong one. Suppose you want assert that a FileNotFoundException is thrown. Now if you get an Exception then: - technically the unittest has passed because no AssertError has been thrown (splitting hairs) - if for whatever reason you catch this error in the unittest, things will get screwed up - You (or possibly some other script) won' t get the same command line output, it's then harder to correlate the exception with the assertion I agree about wanting to know the original Exception though. I think this is possible by setting the .next field of AssertError with that exception. ... > >> I believe these assertions should be added: >> >> - assertExcThrown and assertExcNotThrown with a default T (should be >> Exception, not Throwable) > > That's not a bad idea, though personally, I tend to think that if someone > used an Exception rather than a derived type that they aren't be specific > enough (I'm sure that many would argue with me on that one though). It > might work to just give it a default parameter (and I do agree that the > default should be Exception rather than Throwable). I agree but it doesn't matter for general use, people will want this and this is also practice in phobos (mostly through the use of enforce I think). ... > >> - assertOpBinary, assertTrue and perhaps assertPred (where the predicate >> is an template alias parameter) > > I decided that assertTrue() was a waste of time, because that's what a > bare assert does. The same goes for assertFalse(), because all you have to > do is add ! in front of the expression. All of the functions that I have > are there to either improve the output (such as printing out the actual > values of the two values being compared in assertEqual()), or they get rid > of boilerplate code (such as the try-catch block and other details in > assertExcThrown!()). Except that assert does not print the expression which resulted in the assertion like all other functions do, so assertTrue does improve the output too. > As for assertOpBinary, are you suggesting a function which does the given > binary operation and then compares the result with the given expected > result? That would be a good addition. > > As for assertPred. I don't know what that would do. Would that take a > predicate and its parameters and then assert that the result was true? If > that's what you're looking for, then assertPredTrue and assertPredFalse > would be better. I think that you need to clarify quite what you meant > though. bool isSorted(); int[] numbers = getSortedNumbers(); assertPred!isSorted(numbers); It's the same as assert(isSorted(numbers)), except it allows for improved output. Not very important, but I find it common to use such predicates for testing so it might help. Alternatively assertTrue and assertFalse could take an optional predicate, defaulting to the identity and negation respectively: assertTrue!isSorted(numbers);
