Re: std.unittests vote tally
On Tuesday 08 February 2011 07:27:55 Andrei Alexandrescu wrote: Vote has closed last night at 23:59:59:99, but I accepted Lars' late vote. Thanks Jonathan for responding to comments and suggestions, and for a very dedicated attitude throughout. YES votes mean wholesale acceptance of the library. NO means either partial acceptance or no acceptance at all. We have eight NOs and even YESs. (In fairness I have changed my vote after Don committed to improve assert(), but forgot to submit it.) NO: SHOO (arguments: unittest code should be easy to read without prior knowledge) Don (arguments: assertPred is harder to read than assert, don't use if you don't like doesn't apply to Phobos, Phobos becomes difficult to read if we continue adopting clever functions, something that has any appearance of being complicated needs VERY strong justification. Voted yes for assertThrown and assertNotThrown. Asked for bugzilla enhancement requests to have assert obviate assertPred) Michel Fortin Brad Roberts David Nadlinger (yes for assertThrown, 50/50 for assertNotThrown and collectExceptionMsg, no for assertPred) spir (yes to assertThrown, abstain for assertNotThrown and collectExceptionMsg) Jim (reiterates that at best assert should be improved) Lars T. Kyllingstad (on the fence with assertPred) YES: Jens Mueller bearophile Andrej Mitrovic Nick Sabalausky Andrei Alexandrescu (contingent on reducing the size of examples) Masahiro Nakagawa (with a few notes) Andrew Wiley Reviewer Manager's decision: We have had an unexpected development: we can change assert() to obviate assertPred(), and Don all but promised he'll look into it. This means if we accept the library as it is, we'll look at a function on the brink of deprecation for the sake of a short-term benefit. Perhaps this is not the best course of action. So let's not put assertPred() for now in Phobos, though Jonathan is to be commended for his work which is leading to a vast improvement to a core facility. assertThrown seems to be liked by a vast majority - please add to std.exception at your earliest convenience. assertNotThrown and collectExceptionMsg are on the fence and it's unclear whether some NO voters want them as isolated functions. Let us take a one-week vote for each. I will create one thread for each. Thanks to everyone for participating, and special thanks to Jonathan! Enhancement request for assert: http://d.puremagic.com/issues/show_bug.cgi?id=5547 Okay. I'll look at doing another proposal which has the functionality of assertPred which doesn't make sense to add to assert, though I'll probably wait until the voting for assertNotThrown and collectExceptionMsg is done. I would point out, however, that it would be rather silly to include assertThrown and not assertNotThrown. Good unit tests should test _both_ that a function succeeds as it's supposed to _and_ that it fails as it's supposed to. So, I would hope that people vote in favor of assertNotThrown. collectExceptionMsg isn't as critical, but it really does make it easy to test that exception messages are correct, since if you use collectException, you have to worry about checking for null before you can check the message. With collectExceptionMsg, it can be a an easy one-liner to check exception messages. Without it, you end up taking several lines, because you have to save and check the exception for null before you can check its message. I'll wait for the vote on assertNotThrown and collectExceptionMsg to be completed before putting assertThrown in Phobos. Then it can just all be taken care of at once. - Jonathan M Davis
Re: std.unittests vote tally
On 2/8/11 10:54 AM, Jonathan M Davis wrote: Enhancement request for assert: http://d.puremagic.com/issues/show_bug.cgi?id=5547 Thanks! Okay. I'll look at doing another proposal which has the functionality of assertPred which doesn't make sense to add to assert, though I'll probably wait until the voting for assertNotThrown and collectExceptionMsg is done. I would point out, however, that it would be rather silly to include assertThrown and not assertNotThrown. Good unit tests should test _both_ that a function succeeds as it's supposed to _and_ that it fails as it's supposed to. So, I would hope that people vote in favor of assertNotThrown. I think many people would emulate assertNotThrown by simply calling the function and... well if it throws then the unittest fails. collectExceptionMsg isn't as critical, but it really does make it easy to test that exception messages are correct, since if you use collectException, you have to worry about checking for null before you can check the message. With collectExceptionMsg, it can be a an easy one-liner to check exception messages. Without it, you end up taking several lines, because you have to save and check the exception for null before you can check its message. I'll wait for the vote on assertNotThrown and collectExceptionMsg to be completed before putting assertThrown in Phobos. Then it can just all be taken care of at once. Sounds great. Thanks! Andrei
Re: std.unittests vote tally
On 02/08/2011 04:54 PM, Jonathan M Davis wrote: I would point out, however, that it would be rather silly to include assertThrown and not assertNotThrown. Good unit tests should test_both_ that a function succeeds as it's supposed to_and_ that it fails as it's supposed to. So, I would hope that people vote in favor of assertNotThrown. I do agree failure cases must be tested (maybe even more) and are very often neglected by programmers in unittests. But in a no-throw case the proper assertion is just a regular assert (at least, in my use of unittests): // dunno the syntax assertThrown ( 1/0, DivisionByZero ); assert ( 1/1 == 1 ); If 1/1 throws DivisionByZero, I get all the information I need. Reason for my question mark about including assertNotThrown. When do I need it? What new does it bring? Sorry, I should have asked/commented earlier on this point (but had too much...). Denis -- _ vita es estrany spir.wikidot.com
Re: std.unittests vote tally
Am 08.02.2011 18:00, schrieb spir: On 02/08/2011 04:54 PM, Jonathan M Davis wrote: I would point out, however, that it would be rather silly to include assertThrown and not assertNotThrown. Good unit tests should test_both_ that a function succeeds as it's supposed to_and_ that it fails as it's supposed to. So, I would hope that people vote in favor of assertNotThrown. I do agree failure cases must be tested (maybe even more) and are very often neglected by programmers in unittests. But in a no-throw case the proper assertion is just a regular assert (at least, in my use of unittests): // dunno the syntax assertThrown ( 1/0, DivisionByZero ); assert ( 1/1 == 1 ); If 1/1 throws DivisionByZero, I get all the information I need. Reason for my question mark about including assertNotThrown. When do I need it? What new does it bring? Sorry, I should have asked/commented earlier on this point (but had too much...). Denis Maybe it can be nested like assertThrown!Exception( assertNotThrown!MyException( fun(42) ) ); to ensure that fun() doesn't throw a MyException, but does throws another Exception? Cheers, - Daniel
Re: std.unittests vote tally
On Tuesday, February 08, 2011 08:36:27 Andrei Alexandrescu wrote: On 2/8/11 10:54 AM, Jonathan M Davis wrote: Enhancement request for assert: http://d.puremagic.com/issues/show_bug.cgi?id=5547 Thanks! Okay. I'll look at doing another proposal which has the functionality of assertPred which doesn't make sense to add to assert, though I'll probably wait until the voting for assertNotThrown and collectExceptionMsg is done. I would point out, however, that it would be rather silly to include assertThrown and not assertNotThrown. Good unit tests should test _both_ that a function succeeds as it's supposed to _and_ that it fails as it's supposed to. So, I would hope that people vote in favor of assertNotThrown. True. But the test is clearer if you're explicitly testing that the function doesn't throw instead of just having a stray function call that isn't tested. For instance. assertNotThrown!DateTimeException(TimeOfDay(23, 59, 59)); is clearer than TimeOfDay(23, 59, 59); In the first case, it's clear that you're testing that the function call does not throw. In the second, it's a function call that seems to do nothing, since its result isn't saved, it takes no references, and it's not part of an assert. Also, the first one results in an AssertError that clearly states that the problem is that the function threw when it wasn't supposed to, whereas in the second, you just get a stray exception which is likely going to be a bit hard to track down - even with a stack trace - because the unit test blocks get named with seemingly random numbers rather than real names and tracking down which unittest block an exception was thrown from is a royal pain (one more reason why we really should have named unit tests). So, I think that assertNotThrown definitely helps with clarity, and it makes it much easier to track down the failure. - Jonathan M Davis
Re: std.unittests vote tally
So in the most basic form assertThrown is used to check that our functions throw on bad (user) input, and assertNotThrown is used to check that our functions work with valid input? Looks good to me.
Re: std.unittests vote tally
On Tuesday, February 08, 2011 09:26:23 Andrej Mitrovic wrote: So in the most basic form assertThrown is used to check that our functions throw on bad (user) input, and assertNotThrown is used to check that our functions work with valid input? Looks good to me. Yes. - Jonathan M Davis
Re: std.unittests vote tally
On 2011-02-08 12:26:23 -0500, Andrej Mitrovic andrej.mitrov...@gmail.com said: So in the most basic form assertThrown is used to check that our functions throw on bad (user) input, and assertNotThrown is used to check that our functions work with valid input? Most functions are said to work whey they have the desired effect, not when they do not throw. To make sure a function works, you should call it and then check the return value and/or whatever the side effects should be. -- Michel Fortin michel.for...@michelf.com http://michelf.com/
Re: std.unittests vote tally
Jonathan M Davis jmdavisp...@gmx.com wrote in message news:mailman.1401.1297185535.4748.digitalmar...@puremagic.com... On Tuesday, February 08, 2011 08:36:27 Andrei Alexandrescu wrote: On 2/8/11 10:54 AM, Jonathan M Davis wrote: Enhancement request for assert: http://d.puremagic.com/issues/show_bug.cgi?id=5547 Thanks! Okay. I'll look at doing another proposal which has the functionality of assertPred which doesn't make sense to add to assert, though I'll probably wait until the voting for assertNotThrown and collectExceptionMsg is done. I would point out, however, that it would be rather silly to include assertThrown and not assertNotThrown. Good unit tests should test _both_ that a function succeeds as it's supposed to _and_ that it fails as it's supposed to. So, I would hope that people vote in favor of assertNotThrown. True. But the test is clearer if you're explicitly testing that the function doesn't throw instead of just having a stray function call that isn't tested. For instance. assertNotThrown!DateTimeException(TimeOfDay(23, 59, 59)); is clearer than TimeOfDay(23, 59, 59); In the first case, it's clear that you're testing that the function call does not throw. In the second, it's a function call that seems to do nothing, since its result isn't saved, it takes no references, and it's not part of an assert. Also, the first one results in an AssertError that clearly states that the problem is that the function threw when it wasn't supposed to, whereas in the second, you just get a stray exception which is likely going to be a bit hard to track down - even with a stack trace - because the unit test blocks get named with seemingly random numbers rather than real names and tracking down which unittest block an exception was thrown from is a royal pain (one more reason why we really should have named unit tests). So, I think that assertNotThrown definitely helps with clarity, and it makes it much easier to track down the failure. This is why I've always felt that assert/assertPred/etc should always check whether an exception was thrown and report it accordingly. That way, the test to make sure the result is correct will *automatically* provide all the benefits of assertNotThrown, but without the developer needing to compulsively assertNotThrown every single function they write/test. However, that said, I think assertNotThrown would still be useful for void functions since those don't have a result to assert(). Plus, AIUI, assertNotThrown lets you make sure that a *specific* type of exception isn't thrown for the given arguments, which I can imagine would be useful in certain cases (for instance, if a function had been throwing the wrong type of exception upon bad input and you want to prevent regressions).
Re: std.unittests vote tally
On 2/8/11 12:20 PM, Jonathan M Davis wrote: On Tuesday, February 08, 2011 08:36:27 Andrei Alexandrescu wrote: On 2/8/11 10:54 AM, Jonathan M Davis wrote: Enhancement request for assert: http://d.puremagic.com/issues/show_bug.cgi?id=5547 Thanks! Okay. I'll look at doing another proposal which has the functionality of assertPred which doesn't make sense to add to assert, though I'll probably wait until the voting for assertNotThrown and collectExceptionMsg is done. I would point out, however, that it would be rather silly to include assertThrown and not assertNotThrown. Good unit tests should test _both_ that a function succeeds as it's supposed to _and_ that it fails as it's supposed to. So, I would hope that people vote in favor of assertNotThrown. True. But the test is clearer if you're explicitly testing that the function doesn't throw instead of just having a stray function call that isn't tested. For instance. assertNotThrown!DateTimeException(TimeOfDay(23, 59, 59)); is clearer than TimeOfDay(23, 59, 59); In the first case, it's clear that you're testing that the function call does not throw. In the second, it's a function call that seems to do nothing, since its result isn't saved, it takes no references, and it's not part of an assert. Also, the first one results in an AssertError that clearly states that the problem is that the function threw when it wasn't supposed to, whereas in the second, you just get a stray exception which is likely going to be a bit hard to track down - even with a stack trace - because the unit test blocks get named with seemingly random numbers rather than real names and tracking down which unittest block an exception was thrown from is a royal pain (one more reason why we really should have named unit tests). So, I think that assertNotThrown definitely helps with clarity, and it makes it much easier to track down the failure. - Jonathan M Davis I think I'd write that as assert(!collectException(TimeOfDay(23, 59, 59)); Andrei
