www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - std.unittests vote tally

reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
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!

Andrei
Feb 08 2011
next sibling parent reply Jonathan M Davis <jmdavisProg gmx.com> writes:
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
Feb 08 2011
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
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
Feb 08 2011
next sibling parent reply Jonathan M Davis <jmdavisProg gmx.com> writes:
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
Feb 08 2011
next sibling parent "Nick Sabalausky" <a a.a> writes:
"Jonathan M Davis" <jmdavisProg gmx.com> wrote in message 
news:mailman.1401.1297185535.4748.digitalmars-d 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).
Feb 08 2011
prev sibling parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
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
Feb 08 2011
prev sibling parent reply Andrej Mitrovic <andrej.mitrovich gmail.com> writes:
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.
Feb 08 2011
parent Michel Fortin <michel.fortin michelf.com> writes:
On 2011-02-08 12:26:23 -0500, Andrej Mitrovic 
<andrej.mitrovich 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.fortin michelf.com
http://michelf.com/
Feb 08 2011
prev sibling next sibling parent reply spir <denis.spir gmail.com> writes:
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
Feb 08 2011
parent Daniel Gibson <metalcaedes gmail.com> writes:
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
Feb 08 2011
prev sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
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
Feb 08 2011