www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - std.unittests for (final?) review

reply Jonathan M Davis <jmdavisProg gmx.com> writes:
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


Both the source file and the ddoc hmtl file are included. And unlike
std.datetime, 
it's actually straightforward enough that it makes sense for your average 
program to actually look at it.

To be clear, this module has 2 purposes:

1. Improve error messages on test failure. A good example of this is 
assertEqual, which prints out both of the values being compared so that you 
don't have to add a print statement (or run a debugger) and rerun the test to 
see what the actual values were.

2. Reduce boiler-plate code in unit tests. A good example of this is 
assertExThrown. Testing whether a function call actually throws an exception 
like it's supposed to becomes a single line of code instead of being around 10 
lines.

This module does _not_ attempt to change how unit testing in D fundamentally 
works. It does not print out on success. It doesn't make it so that a unittest 
block continues after a failure occurs in that block. All of the assertXXX 
functions throw an AssertError upon failure - just like assert does, but the 
message in the AssertError is far more informative. While improvements can be 
made to how unit tests work in D, I believe that that should be addressed by 
actually making those improvements to the core language as opposed to using a 
module in Phobos to change things. You shouldn't _need_ std.unittests to write 
unit testing code.

std.unittests is intended to help you write more informative unit test code and 
save you from writing some of the boilerplate code in unit tests, but it is 
_not_ intended to fundamentally alter how unit tests in D function.

So, please have a look at the code. I've made changes according to previous 
suggestions, and I think that the result is quite a bit better than my original 
proposal. If all goes well, perhaps Andrei will put it up for vote soon, and we 
can actually have it in Phobos.

- Jonathan M Davis
Jan 02 2011
next sibling parent reply "Vladimir Panteleev" <vladimir thecybershadow.net> writes:
On Mon, 03 Jan 2011 06:44:50 +0200, Jonathan M Davis <jmdavisProg gmx.com>  
wrote:

 So, please have a look at the code.

Just one thing: wouldn't these functions also be useful in contract programming (invariants etc.)? Perhaps they should just be added to std.exception? -- Best regards, Vladimir mailto:vladimir thecybershadow.net
Jan 03 2011
next sibling parent bearophile <bearophileHUGS lycos.com> writes:
Jonathan M Davis Wrote:

 But personally, I'd be a bit worried to see more than simple assertions inside
of 
 contracts.

I agree. And I hope to see Phobos contracts as soon as possible free of enforce() and similar exceptions. It's not just a matter of performance. Otherwise enforce() risks becoming a permanent workaround for a missing but necessary feature. A way to solve the problem DMD has to use a Phobos library compiled in not-release mode when you don't use the -release switch. Bye, bearophile
Jan 03 2011
prev sibling next sibling parent Michel Fortin <michel.fortin michelf.com> writes:
On 2011-01-03 05:04:28 -0500, Jonathan M Davis <jmdavisProg gmx.com> said:

 If a lot of people really thought that unit testing functions would be useful
 and reasonable to use inside of contracts, then it would make some 
 sense for the
 unit testing functions to no longer be in a version(unittest) block. But
 personally, I'd be a bit worried to see more than simple assertions inside of
 contracts. Contracts should be efficient, since they're going to be run 
 while the
 code is running normally, whereas unit tests don't have to worry about 
 efficiency
 quite as much. I believe that the functions that I have are efficient, 
 but I can
 easily see functions intended to aid in unit testing being totally 
 inappropriate
 in a contract due to efficiency concerns. And of course, a number of the unit
 testing functions just don't make sense in contracts regardless - like
 assertExThrown.

This makes me think of an issue. In contracts, asserts use a different implementation so they can work with inheritance. I believe in unit tests too the compiler use a different implementation too for asserts which gives more control to the runtime over what to do when a test fails (remember previous behaviour where asserts in unit tests were simply printing a message without aborting?). Assertions inside a function and inside a contract and inside a unittest block being different things makes me rather hesitant when I see a function designed to be used as a special assert in a unit test... So given the choice between these two forms: unittest { assertExThrown!Exception(test); } unittest { assert(exThrown!Exception(test)); } I'd choose the second just to be sure the right assertion handler is used. -- Michel Fortin michel.fortin michelf.com http://michelf.com/
Jan 03 2011
prev sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 1/3/11 3:38 AM, Vladimir Panteleev wrote:
 On Mon, 03 Jan 2011 06:44:50 +0200, Jonathan M Davis
 <jmdavisProg gmx.com> wrote:

 So, please have a look at the code.

Just one thing: wouldn't these functions also be useful in contract programming (invariants etc.)? Perhaps they should just be added to std.exception?

In fact (without looking at std.unittest) I think it should be grouped with a simple benchmark facility. That's what the homonym frameworks in Google's and Facebook's code base do. Does anyone want to be the review manager? Refer to http://www.boost.org/community/reviews.html. Also, everybody please follow the guidelines in that document for submitting your comments. Let's collect comments and votes until February 7, when a decision will be made about inclusion into Phobos. Andrei
Jan 03 2011
parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 1/3/11 9:02 AM, Jonathan M Davis wrote:
 On Monday 03 January 2011 06:43:24 Andrei Alexandrescu wrote:
 On 1/3/11 3:38 AM, Vladimir Panteleev wrote:
 On Mon, 03 Jan 2011 06:44:50 +0200, Jonathan M Davis

 <jmdavisProg gmx.com>  wrote:
 So, please have a look at the code.

Just one thing: wouldn't these functions also be useful in contract programming (invariants etc.)? Perhaps they should just be added to std.exception?

In fact (without looking at std.unittest) I think it should be grouped with a simple benchmark facility. That's what the homonym frameworks in Google's and Facebook's code base do.

I'm afraid that I don't see what unit test helper functions have to do with benchmarking.

They both help the decision: "Is this module good to go?" At work we almost always put benchmarks in the same file as the unit tests, and run them together (subject to a --benchmark flag because benchmarks tend to take a fair amount of time). I find that very natural, although I should add that the unittest and benchmark libraries are nominally distinct.
 And I don't believe that we have a benchmarking module at the
 moment regardless, so if you want to do that, we'd need to create one. The only
 benchmarking stuff that I'm aware of is the bencharking stuff in std.datetime
that
 SHOO did, which isn't all that much code. I would have thought that unit test
 helper functions would merit their own module, particularly when I don't see
 what they have to do with benchmarks.

Adding basic support for benchmarking shouldn't be difficult and does not entail a lot of code, but I understand it if you're not all that motivated to work on that. Andrei
Jan 03 2011
prev sibling next sibling parent reply bearophile <bearophileHUGS lycos.com> writes:
Jonathan M Davis:

 While improvements can be 
 made to how unit tests work in D, I believe that that should be addressed by 
 actually making those improvements to the core language as opposed to using a 
 module in Phobos to change things. You shouldn't _need_ std.unittests to write 
 unit testing code.

I think it's wrong to design a built-in unit test system able to do most of the things a real unit test system is expected to work, because it's a lot of stuff and because 10-15 years from now the best design for an unit test system may be different, and there are different ways to create tests. So I prefer the built-in unit test features to allow the creation of a good standard library unit test system based on the built-in one, working as an extension. In little programs you are free to use the built-in one. What basic unit testing features do you think are better to become built-in? (Unit test names?) Bye, bearophile
Jan 03 2011
next sibling parent bearophile <bearophileHUGS lycos.com> writes:
Jens Mueller:

 I agree with bearophile. Unit testing can be implemented on top of the
 language and shouldn't be put into it.

This is not what I have said. I was talking about option IV I have explained here: http://www.digitalmars.com/webnews/newsgroups.php?art_group=digitalmars.D&article_id=107997 This means I accept the built-in unit test system, but I think it's wrong to make it do everything you want from an unit test system. So the built-in one has to give the basic features (like giving a name to each unit test) that are later used by the library-defined unit test system. Bye, bearophile
Jan 03 2011
prev sibling next sibling parent bearophile <bearophileHUGS lycos.com> writes:
Jonathan M Davis:

 Other changes that some people have expressed interest in are things like
having 
 it printed when a test passes, and those should not be in the language or 
 druntime itself. That's going to either have to be in helper functions that
are 
 run inside of unittest blocks or it's going to need to be possible to run 
 unittest blocks externally somehow

Few hooks may help. Built-in unit tests may call few things that are usually empty (the calls may contain a string with the unit test name, and other data). A unit test library is then free to fill those with code that shows statistics, prints, GUIs, etc. Bye, bearophile
Jan 03 2011
prev sibling parent Walter Bright <newshound2 digitalmars.com> writes:
Jens Mueller wrote:
 Unit testing can be implemented on top of the
 language and shouldn't be put into it. Somehow I have the feeling that
 too often one tries to extend the language even though the feature could
 be implemented in a library.

On the other hand, the built-in D unit test ability has been a huge success. A unit test facility that is not used is worthless, no matter how capable it is. The advantage of it being simple and built-in is it gets used, and I think there's strong evidence that this is true for D.
Jan 03 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Monday 03 January 2011 01:38:29 Vladimir Panteleev wrote:
 On Mon, 03 Jan 2011 06:44:50 +0200, Jonathan M Davis <jmdavisProg gmx.com>
 
 wrote:
 So, please have a look at the code.

Just one thing: wouldn't these functions also be useful in contract programming (invariants etc.)? Perhaps they should just be added to std.exception?

Well, they're written with unit tests in mind rather than contracts. Unit tests and contracts have similar but different purposes. I wouldn't really want to conflate the two. Also, it's std.exception now rather than std.contracts specifically because it doesn't really relate to contracts, so even if you did want to use unit testing functions in contracts, I'm not sure that std.exception is the right place to put such functions anyway. As it stands, the entire module is in a version(unittest) block, so the functions can't be used outside of unit tests. If a lot of people really thought that unit testing functions would be useful and reasonable to use inside of contracts, then it would make some sense for the unit testing functions to no longer be in a version(unittest) block. But personally, I'd be a bit worried to see more than simple assertions inside of contracts. Contracts should be efficient, since they're going to be run while the code is running normally, whereas unit tests don't have to worry about efficiency quite as much. I believe that the functions that I have are efficient, but I can easily see functions intended to aid in unit testing being totally inappropriate in a contract due to efficiency concerns. And of course, a number of the unit testing functions just don't make sense in contracts regardless - like assertExThrown. Howevr, regardless of whether the functions are deemed reasonable for use in contracts, I do think that having a module of functions designed specifically for use in unit tests is of definite value, and regardless of the current size of the module, it's quite possible that it could become fairly large at some point if enough useful unit testing functions are devised. Merging that in with std.exception seems ill-advised to me. Not to mention, it's a lot more obvious that you're dealing with functions intended to aid in unit tests when the module is named std.unittests than if those functions are lumped in with the stuff in std.exception. - Jonathan M Davis
Jan 03 2011
prev sibling next sibling parent Jens Mueller <jens.k.mueller gmx.de> writes:
bearophile wrote:
 Jonathan M Davis:
 
 While improvements can be 
 made to how unit tests work in D, I believe that that should be addressed by 
 actually making those improvements to the core language as opposed to using a 
 module in Phobos to change things. You shouldn't _need_ std.unittests to write 
 unit testing code.

I think it's wrong to design a built-in unit test system able to do most of the things a real unit test system is expected to work, because it's a lot of stuff and because 10-15 years from now the best design for an unit test system may be different, and there are different ways to create tests. So I prefer the built-in unit test features to allow the creation of a good standard library unit test system based on the built-in one, working as an extension. In little programs you are free to use the built-in one.

I agree with bearophile. Unit testing can be implemented on top of the language and shouldn't be put into it. Somehow I have the feeling that too often one tries to extend the language even though the feature could be implemented in a library. I like the basic built-in support for unit testing in D but more advanced testing should be implemented in a module ideally leading to something like GoogleTest for D. I think unittest.d does a good step into that direction. More will be needed, if there is consensus. Jens
Jan 03 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Monday 03 January 2011 01:50:48 bearophile wrote:
 Jonathan M Davis:
 While improvements can be
 made to how unit tests work in D, I believe that that should be addressed
 by actually making those improvements to the core language as opposed to
 using a module in Phobos to change things. You shouldn't _need_
 std.unittests to write unit testing code.

I think it's wrong to design a built-in unit test system able to do most of the things a real unit test system is expected to work, because it's a lot of stuff and because 10-15 years from now the best design for an unit test system may be different, and there are different ways to create tests. So I prefer the built-in unit test features to allow the creation of a good standard library unit test system based on the built-in one, working as an extension. In little programs you are free to use the built-in one. What basic unit testing features do you think are better to become built-in? (Unit test names?)

The main items that I can think of are unit test names and what happens when a unittest block fails. We can't currently name unit tests, and while it's planned that every unittest block will run even if others in the module fail, at present, if one unittest block in a module runs, none of the other unittest blocks in the module run. Other changes that some people have expressed interest in are things like having it printed when a test passes, and those should not be in the language or druntime itself. That's going to either have to be in helper functions that are run inside of unittest blocks or it's going to need to be possible to run unittest blocks externally somehow (like would be necessary if you wanted to run them in an IDE) and then whatever program runs them can choose to print test successes. But printing test successes goes against how D's unit testing framework is designed and IMHO would be very bad - particularly when the tests run before main rather than in a separate program. What I was essentially trying to say was that I'm not trying to fundamentally change how unit testing in D functions, and I don't think that it really should be changed. Some improvements can be made, but I don't think that that's the job of Phobos. Such changes either need to be made in the language/druntime, or they need to be made in a some sort of external tool which runs D's unit tests differently. All that std.unittests is trying to do is to expand on what D's unit tests already do, not fundamentally alter how they work. - Jonathan M Davis
Jan 03 2011
prev sibling next sibling parent Jens Mueller <jens.k.mueller gmx.de> writes:
Jonathan M Davis wrote:
 On Monday 03 January 2011 01:38:29 Vladimir Panteleev wrote:
 On Mon, 03 Jan 2011 06:44:50 +0200, Jonathan M Davis <jmdavisProg gmx.com>
 
 wrote:
 So, please have a look at the code.

Just one thing: wouldn't these functions also be useful in contract programming (invariants etc.)? Perhaps they should just be added to std.exception?

Well, they're written with unit tests in mind rather than contracts. Unit tests and contracts have similar but different purposes. I wouldn't really want to conflate the two. Also, it's std.exception now rather than std.contracts specifically because it doesn't really relate to contracts, so even if you did want to use unit testing functions in contracts, I'm not sure that std.exception is the right place to put such functions anyway. As it stands, the entire module is in a version(unittest) block, so the functions can't be used outside of unit tests. If a lot of people really thought that unit testing functions would be useful and reasonable to use inside of contracts, then it would make some sense for the unit testing functions to no longer be in a version(unittest) block. But personally, I'd be a bit worried to see more than simple assertions inside of contracts. Contracts should be efficient, since they're going to be run while the code is running normally, whereas unit tests don't have to worry about efficiency quite as much. I believe that the functions that I have are efficient, but I can easily see functions intended to aid in unit testing being totally inappropriate in a contract due to efficiency concerns. And of course, a number of the unit testing functions just don't make sense in contracts regardless - like assertExThrown.

What do you mean by "running normally"? I think since they are compiled away with -release they are not run normally.
 Howevr, regardless of whether the functions are deemed reasonable for use in 
 contracts, I do think that having a module of functions designed specifically
for 
 use in unit tests is of definite value, and regardless of the current size of
the 
 module, it's quite possible that it could become fairly large at some point if 
 enough useful unit testing functions are devised. Merging that in with 
 std.exception seems ill-advised to me. Not to mention, it's a lot more obvious 
 that you're dealing with functions intended to aid in unit tests when the
module 
 is named std.unittests than if those functions are lumped in with the stuff in 
 std.exception.

I think you're right. It definitely should stay in it's own module. I do it as follows: Build with -release -noboundscheck and no -unittest to get the production version. I.e. all contracts, asserts, boundschecks, and unittests go away. Further I have a build with -unittest which is for testing. Now I can use Jonathan's assertions with better error messages even in contracts. Jens
Jan 03 2011
prev sibling next sibling parent Jens Mueller <jens.k.mueller gmx.de> writes:
bearophile wrote:
 Jens Mueller:
 
 I agree with bearophile. Unit testing can be implemented on top of the
 language and shouldn't be put into it.

This is not what I have said. I was talking about option IV I have explained here: http://www.digitalmars.com/webnews/newsgroups.php?art_group=digitalmars.D&article_id=107997 This means I accept the built-in unit test system, but I think it's wrong to make it do everything you want from an unit test system. So the built-in one has to give the basic features (like giving a name to each unit test) that are later used by the library-defined unit test system.

I mean the same. "I like the basic built-in support for unit testing in D but more advanced testing should be implemented in a module ideally leading to something like GoogleTest for D." Advanced features should be implemented on top of the built-in one. Specifically they should not be put into the core language. Sorry for my bad wording. Jens
Jan 03 2011
prev sibling next sibling parent Jens Mueller <jens.k.mueller gmx.de> writes:
bearophile wrote:
 Jens Mueller:
 
 I agree with bearophile. Unit testing can be implemented on top of the
 language and shouldn't be put into it.

This is not what I have said. I was talking about option IV I have explained here: http://www.digitalmars.com/webnews/newsgroups.php?art_group=digitalmars.D&article_id=107997

Not sure but is this compatible with your option IV: Built advanced unit testing capabilities on top of the existing ones in a module. You want to add hooks. If you just need them because right now some things cannot be implemented (e.g. because a unit test has no name) then we perfectly agree. There might be things in the current built-in unit testing that prevent implementing some advanced feature on top of them. That needs to be done. Jens
Jan 03 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Monday 03 January 2011 02:39:59 Jens Mueller wrote:
 Jonathan M Davis wrote:
 On Monday 03 January 2011 01:38:29 Vladimir Panteleev wrote:
 On Mon, 03 Jan 2011 06:44:50 +0200, Jonathan M Davis
 <jmdavisProg gmx.com>
 
 wrote:
 So, please have a look at the code.

Just one thing: wouldn't these functions also be useful in contract programming (invariants etc.)? Perhaps they should just be added to std.exception?

Well, they're written with unit tests in mind rather than contracts. Unit tests and contracts have similar but different purposes. I wouldn't really want to conflate the two. Also, it's std.exception now rather than std.contracts specifically because it doesn't really relate to contracts, so even if you did want to use unit testing functions in contracts, I'm not sure that std.exception is the right place to put such functions anyway. As it stands, the entire module is in a version(unittest) block, so the functions can't be used outside of unit tests. If a lot of people really thought that unit testing functions would be useful and reasonable to use inside of contracts, then it would make some sense for the unit testing functions to no longer be in a version(unittest) block. But personally, I'd be a bit worried to see more than simple assertions inside of contracts. Contracts should be efficient, since they're going to be run while the code is running normally, whereas unit tests don't have to worry about efficiency quite as much. I believe that the functions that I have are efficient, but I can easily see functions intended to aid in unit testing being totally inappropriate in a contract due to efficiency concerns. And of course, a number of the unit testing functions just don't make sense in contracts regardless - like assertExThrown.

What do you mean by "running normally"? I think since they are compiled away with -release they are not run normally.

I mean when the program is running. If you're running a debug version of your program (which is perfectly normal and common), the contracts are going to run. They're _supposed_ to run while the program is running. That's the point. Naturally, you compile them out for release, but contracts need to be written in way that the program works just fine with them compiled in. Unit tests, on the other hand, don't run while the program is running. They're run once when the program starts up (assuming that -unittest was used), and then they're done. You don't want them to be super slow, but they're not getting called multiple times or running while the program is, so they don't have to be as efficient. - Jonathan M Davis
Jan 03 2011
prev sibling next sibling parent Jens Mueller <jens.k.mueller gmx.de> writes:
 On Monday 03 January 2011 02:39:59 Jens Mueller wrote:
 Jonathan M Davis wrote:
 
 What do you mean by "running normally"? I think since they are compiled
 away with -release they are not run normally.

I mean when the program is running. If you're running a debug version of your program (which is perfectly normal and common), the contracts are going to run. They're _supposed_ to run while the program is running. That's the point. Naturally, you compile them out for release, but contracts need to be written in way that the program works just fine with them compiled in.

I see. You're right. If one uses unittest.d in contracts than they won't work in all builds, namely the build with no -release and no -unittest. Jens
Jan 03 2011
prev sibling next sibling parent "Lars T. Kyllingstad" <public kyllingen.NOSPAMnet> writes:
On Sun, 02 Jan 2011 20:44:50 -0800, 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
 
 
 Both the source file and the ddoc hmtl file are included. And unlike
 std.datetime, it's actually straightforward enough that it makes sense
 for your average program to actually look at it.
 
 To be clear, this module has 2 purposes:
 
 1. Improve error messages on test failure. A good example of this is
 assertEqual, which prints out both of the values being compared so that
 you don't have to add a print statement (or run a debugger) and rerun
 the test to see what the actual values were.
 
 2. Reduce boiler-plate code in unit tests. A good example of this is
 assertExThrown. Testing whether a function call actually throws an
 exception like it's supposed to becomes a single line of code instead of
 being around 10 lines.
 
 This module does _not_ attempt to change how unit testing in D
 fundamentally works. It does not print out on success. It doesn't make
 it so that a unittest block continues after a failure occurs in that
 block. All of the assertXXX functions throw an AssertError upon failure
 - just like assert does, but the message in the AssertError is far more
 informative. While improvements can be made to how unit tests work in D,
 I believe that that should be addressed by actually making those
 improvements to the core language as opposed to using a module in Phobos
 to change things. You shouldn't _need_ std.unittests to write unit
 testing code.
 
 std.unittests is intended to help you write more informative unit test
 code and save you from writing some of the boilerplate code in unit
 tests, but it is _not_ intended to fundamentally alter how unit tests in
 D function.
 
 So, please have a look at the code. I've made changes according to
 previous suggestions, and I think that the result is quite a bit better
 than my original proposal. If all goes well, perhaps Andrei will put it
 up for vote soon, and we can actually have it in Phobos.
 
 - Jonathan M Davis

Sorry for not commenting on earlier iterations of this module. For the most part, I think it looks pretty good, and I also think it will be useful. Often, I find myself writing stuff like assert (someVar == someVal, text("Wrong value for someVar: ", someVar)); and assertEqual() will be nice to have for those cases. :) I just have some minor comments. 1. I think assertExThrown() and assertExNotThrown() should be named assertThrown() and assertNotThrown(), because they can intercept any subclass of Throwable, not just Exception, and because you rarely throw anything else, so it seems redundant. 2. I think the name getMsg() is too non-specific. I'd prefer if it was renamed to throwableMsg(), thrownMsg(), or something similar. The function should also be moved upwards a bit so its documentation follows that of the assert*Thrown() functions. Then, the various functions will be nicely grouped in the documentation -- all the exception handling tests come first, followed by the value tests. -Lars
Jan 03 2011
prev sibling next sibling parent Andrej Mitrovic <andrej.mitrovich gmail.com> writes:
On 1/3/11, Jonathan M Davis <jmdavisProg gmx.com> wrote:
 Other changes that some people have expressed interest in are things like
 having
 it printed when a test passes, and those should not be in the language or
 druntime itself.

Isn't this already done in Phobos?: unittest { debug(std_algorithm) scope(success) writeln("unittest ", __FILE__, ":", __LINE__, " done."); } --
Jan 03 2011
prev sibling next sibling parent Andrej Mitrovic <andrej.mitrovich gmail.com> writes:
Also, you know you get my vote. :)
Jan 03 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Monday 03 January 2011 05:26:14 Andrej Mitrovic wrote:
 On 1/3/11, Jonathan M Davis <jmdavisProg gmx.com> wrote:
 Other changes that some people have expressed interest in are things like
 having
 it printed when a test passes, and those should not be in the language or
 druntime itself.

Isn't this already done in Phobos?: unittest { debug(std_algorithm) scope(success) writeln("unittest ", __FILE__, ":", __LINE__, " done."); } --

I think that it's fine if someone wants to make their unittest blocks print out on success, but that doesn't work in the general case with the unit tests running before main. And if you made it so that they _did_ print success, then what would the people who only want the failures do? With the current scheme, you can make it print if you want to, but it's not forced on you. And honestly, aside from creating a simple string mixin which does what Phobos is doing in your example, I don't how you could really have helper functions for that. However, maybe adding a function which returns an appropriate string to mixin to print successes like Phobos is there would be of some value in std.unittests. Regardless, printing test successes is definitely example of where named unit tests would be nice. And being able to name unittest blocks is definitely a change that would have to be made in the language. Of course, what I _really_ want named unit tests for is so that the stack traces have recognizable names in them for unittest blocks. In any case, it's perfectly possible to do as Phobos does above and have extra code in your unittest blocks which prints upon success. - Jonathan M Davis
Jan 03 2011
prev sibling next sibling parent reply Andrej Mitrovic <andrej.mitrovich gmail.com> writes:
We can always resort to hacks!

module namedUnittest;

import std.stdio;
import std.string;

void main()
{
}

unittest // Foo
{
    scope(failure)
    {
        writefln("Unittest '%s' Failed: ",
split(split(import(.stringof[7..$] ~ ".d"),
"\r\n")[__LINE__-5..__LINE__-4][0])[2]);
    }
    assert(0 == 1, "0 != 1");
}

..what..? Why are you looking at me like that? :p
Jan 03 2011
parent reply "Nick Sabalausky" <a a.a> writes:
"Andrej Mitrovic" <andrej.mitrovich gmail.com> wrote in message 
news:mailman.389.1294063698.4748.digitalmars-d puremagic.com...
 We can always resort to hacks!

 module namedUnittest;

 import std.stdio;
 import std.string;

 void main()
 {
 }

 unittest // Foo
 {
    scope(failure)
    {
        writefln("Unittest '%s' Failed: ",
 split(split(import(.stringof[7..$] ~ ".d"),
 "\r\n")[__LINE__-5..__LINE__-4][0])[2]);
    }
    assert(0 == 1, "0 != 1");
 }

 ..what..? Why are you looking at me like that? :p

Using this: http://www.dsource.org/projects/semitwist/browser/trunk/src/semitwist/util/deferAssert.d#L185 -------------------------------- module myproj.foo; import semitwist.util.all; alias unittestSection!"MyProject_unittest" unittestMyProject; // Old: unittest { assert(x == y); } class Foo { unittest { assert(x == y); } } // New: mixin(unittestMyProject(q{ assert(x == y); })); class Foo { mixin(unittestMyProject("This is for class Foo", q{ assert(x == y); })); } -------------------------------- Compile with: -unittest -debug=MyProject_unittest Output upon running: == unittest: myproj.foo == unittest: myproj.foo: This is for class Foo (This message has been brought to you, in part, by a grant from "Reasons we need a better syntax to invoke string mixins.")
Jan 03 2011
parent reply "Nick Sabalausky" <a a.a> writes:
"Nick Sabalausky" <a a.a> wrote in message 
news:iftndt$1i68$1 digitalmars.com...
 "Andrej Mitrovic" <andrej.mitrovich gmail.com> wrote in message 
 news:mailman.389.1294063698.4748.digitalmars-d puremagic.com...
 We can always resort to hacks!

 module namedUnittest;

 import std.stdio;
 import std.string;

 void main()
 {
 }

 unittest // Foo
 {
    scope(failure)
    {
        writefln("Unittest '%s' Failed: ",
 split(split(import(.stringof[7..$] ~ ".d"),
 "\r\n")[__LINE__-5..__LINE__-4][0])[2]);
    }
    assert(0 == 1, "0 != 1");
 }

 ..what..? Why are you looking at me like that? :p

Using this: http://www.dsource.org/projects/semitwist/browser/trunk/src/semitwist/util/deferAssert.d#L185 -------------------------------- module myproj.foo; import semitwist.util.all; alias unittestSection!"MyProject_unittest" unittestMyProject; // Old: unittest { assert(x == y); } class Foo { unittest { assert(x == y); } } // New: mixin(unittestMyProject(q{ assert(x == y); })); class Foo { mixin(unittestMyProject("This is for class Foo", q{ assert(x == y); })); } -------------------------------- Compile with: -unittest -debug=MyProject_unittest Output upon running: == unittest: myproj.foo == unittest: myproj.foo: This is for class Foo (This message has been brought to you, in part, by a grant from "Reasons we need a better syntax to invoke string mixins.")

I should also mention that it's designed specifically so that reported line numbers will still be correct (as long as the "mixin(unittestMyProject(q{" part is all on one line).
Jan 03 2011
parent "Nick Sabalausky" <a a.a> writes:
"Nick Sabalausky" <a a.a> wrote in message 
news:iftntj$1j6n$1 digitalmars.com...
 I should also mention that it's designed specifically so that reported 
 line numbers will still be correct (as long as the 
 "mixin(unittestMyProject(q{" part is all on one line).

...Although I wonder if I might be able to use #line trickery to eliminate the "as long as the...is all on one line" part...I'll have to look into that...
Jan 03 2011
prev sibling next sibling parent Andrej Mitrovic <andrej.mitrovich gmail.com> writes:
P.S. that will only work with windows newlines, and fail without the
-J switch. Hehe.
Jan 03 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Monday 03 January 2011 06:43:24 Andrei Alexandrescu wrote:
 On 1/3/11 3:38 AM, Vladimir Panteleev wrote:
 On Mon, 03 Jan 2011 06:44:50 +0200, Jonathan M Davis
 
 <jmdavisProg gmx.com> wrote:
 So, please have a look at the code.

Just one thing: wouldn't these functions also be useful in contract programming (invariants etc.)? Perhaps they should just be added to std.exception?

In fact (without looking at std.unittest) I think it should be grouped with a simple benchmark facility. That's what the homonym frameworks in Google's and Facebook's code base do.

I'm afraid that I don't see what unit test helper functions have to do with benchmarking. And I don't believe that we have a benchmarking module at the moment regardless, so if you want to do that, we'd need to create one. The only benchmarking stuff that I'm aware of is the bencharking stuff in std.datetime that SHOO did, which isn't all that much code. I would have thought that unit test helper functions would merit their own module, particularly when I don't see what they have to do with benchmarks. - Jonathan M Davis
Jan 03 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Monday 03 January 2011 07:17:56 Andrei Alexandrescu wrote:
 On 1/3/11 9:02 AM, Jonathan M Davis wrote:
 On Monday 03 January 2011 06:43:24 Andrei Alexandrescu wrote:
 On 1/3/11 3:38 AM, Vladimir Panteleev wrote:
 On Mon, 03 Jan 2011 06:44:50 +0200, Jonathan M Davis
 
 <jmdavisProg gmx.com>  wrote:
 So, please have a look at the code.

Just one thing: wouldn't these functions also be useful in contract programming (invariants etc.)? Perhaps they should just be added to std.exception?

In fact (without looking at std.unittest) I think it should be grouped with a simple benchmark facility. That's what the homonym frameworks in Google's and Facebook's code base do.

I'm afraid that I don't see what unit test helper functions have to do with benchmarking.

They both help the decision: "Is this module good to go?" At work we almost always put benchmarks in the same file as the unit tests, and run them together (subject to a --benchmark flag because benchmarks tend to take a fair amount of time). I find that very natural, although I should add that the unittest and benchmark libraries are nominally distinct.
 And I don't believe that we have a benchmarking module at the
 moment regardless, so if you want to do that, we'd need to create one.
 The only benchmarking stuff that I'm aware of is the bencharking stuff
 in std.datetime that SHOO did, which isn't all that much code. I would
 have thought that unit test helper functions would merit their own
 module, particularly when I don't see what they have to do with
 benchmarks.

Adding basic support for benchmarking shouldn't be difficult and does not entail a lot of code, but I understand it if you're not all that motivated to work on that.

I'm by no means against it, but I wouldn't have a clue what to even do for benchmarking code. I've never used a benchmarking library or done much more than the occasional ad-hoc benchmarks, so I'm not at all familiar with what would be typically done for benchmarking in a library. I'm really not the person to implement such functionality. And I'm busy enough with other stuff at the moment, that I'm not in a hurry to spend time trying to figure out the best benchmarking functionality for Phobos. It could be that it's very straightforward, but using something similar to SHOO's stopwatch to time code is about as far as my experience goes. - Jonathan M Davis
Jan 03 2011
prev sibling next sibling parent Jens Mueller <jens.k.mueller gmx.de> writes:
In fact (without looking at std.unittest) I think it should be grouped
with a simple benchmark facility. That's what the homonym frameworks in
Google's and Facebook's code base do.

I'm afraid that I don't see what unit test helper functions have to do with benchmarking.

They both help the decision: "Is this module good to go?" At work we almost always put benchmarks in the same file as the unit tests, and run them together (subject to a --benchmark flag because benchmarks tend to take a fair amount of time). I find that very natural, although I should add that the unittest and benchmark libraries are nominally distinct.

How do you do it? I had similar thoughts. When I do testing I also want to add some performance tests. In D I thought about having version(performance) (maybe better version(benchmark)) inside a unittest {} to assess performance. This way compiling with -unittest -version=performance will execute the benchmarking code.
And I don't believe that we have a benchmarking module at the
moment regardless, so if you want to do that, we'd need to create one. The only
benchmarking stuff that I'm aware of is the bencharking stuff in std.datetime
that
SHOO did, which isn't all that much code. I would have thought that unit test
helper functions would merit their own module, particularly when I don't see
what they have to do with benchmarks.

Adding basic support for benchmarking shouldn't be difficult and does not entail a lot of code, but I understand it if you're not all that motivated to work on that.

What do you think is needed for basic support? I found std.perf and used it but I think it's deprecated. But std.datetime will do it as well (I haven't looked at it though). Jens
Jan 03 2011
prev sibling next sibling parent Jens Mueller <jens.k.mueller gmx.de> writes:
Walter Bright wrote:
 Jens Mueller wrote:
Unit testing can be implemented on top of the
language and shouldn't be put into it. Somehow I have the feeling that
too often one tries to extend the language even though the feature could
be implemented in a library.

On the other hand, the built-in D unit test ability has been a huge success. A unit test facility that is not used is worthless, no matter how capable it is. The advantage of it being simple and built-in is it gets used, and I think there's strong evidence that this is true for D.

Yes. I do not disagree. I like having unittest in the language. Extending the basic built-in unit testing support should not be done inside the language, if it can be done conveniently as a library. And the built-in unit testing is very helpful and necessary. But it should be possible to build extended testing frameworks (as there are in Java, C++, etc.) on top of these. Sorry. My very first sentence was very misleading. I wanted to say that further/advanced unit testing shouldn't be put into the language. We all agree on that, don't we? Maybe we can get a discussion what features are considered useful in a testing framework library and what needs to be changed in the built-in testing to make such a library happen. Jens
Jan 03 2011
prev sibling next sibling parent Andrej Mitrovic <andrej.mitrovich gmail.com> writes:
On 1/3/11, Walter Bright <newshound2 digitalmars.com> wrote:
 The advantage of it being simple and built-in is it gets used, and I think
 there's strong evidence that this is true for D.

Yup. I often read about C++ programmers who don't bother with unittests because it's a drag to use them. And people keep coming up with new "simpler" unittest C++ libraries all the time. Offtopic (sry): On the other hand, D could do the same for the build process. Bringing RDMD's abilities to DMD would simplify the build process for many people.
Jan 03 2011
prev sibling next sibling parent "Nick Sabalausky" <a a.a> writes:
"Jonathan M Davis" <jmdavisProg gmx.com> wrote in message 
news:mailman.372.1294029902.4748.digitalmars-d puremagic.com...
 std.unittests is intended to help you write more informative unit test 
 code and
 save you from writing some of the boilerplate code in unit tests, but it 
 is
 _not_ intended to fundamentally alter how unit tests in D function.

 So, please have a look at the code. I've made changes according to 
 previous
 suggestions, and I think that the result is quite a bit better than my 
 original
 proposal. If all goes well, perhaps Andrei will put it up for vote soon, 
 and we
 can actually have it in Phobos.

I'm absolutely in favor of it going into Phobos, even if it goes in 100% as-is. That said, there are three issues I have with it (in no order): 1. I completely agree with other people's suggestions of changing some of the function names. 2. The assertOp* functions are an ugly JUnit/NUnit-style kludge. I'd much rather use something that's more like: int x = 2; assert(x == 7, x); // Or assert(q{_==7}, x); // Or assert(q{ #x#==7 }); // etc, anything that involves writing a real expression... Output: Failed: [x == 7] x: 2 Some of those exact thing can't be done right now, but it's been suggested that there should be some _traits-ish way to get the actual code sent to a function's param as a string. That would make a lot of it possible. Even without that, it would also be possible with mixins and q{}. And yes, people whine about that sort of thing being ugly, and I agree, but this is just another reason to clean mixins up. And even with the mixin() and q{} noise, I'd still vastly prefer it because of the same reason I'm a proponent of operator overloading - I don't ever want to have to write a basic expression in some bizarre "Add(x, y)" style. 3. Without the ability make them non-fatal (and catch & report unexpectedly-thrown exceptions, and optionally "fatalize" after a few of them have been run), I'd never use any of those assert* functions directly. It's possible they might be useful in the implementation of an alternate unittest lib, though.
Jan 03 2011
prev sibling next sibling parent spir <denis.spir gmail.com> writes:
On Mon, 03 Jan 2011 12:27:33 -0800
Walter Bright <newshound2 digitalmars.com> wrote:

 On the other hand, the built-in D unit test ability has been a huge succe=

=20
 A unit test facility that is not used is worthless, no matter how capable=

 The advantage of it being simple and built-in is it gets used, and I thin=

 there's strong evidence that this is true for D.

Unfortunately, I find D's builtin unittest way non-convenient for the singl= e reason they are unnamed (so do other D programmers). I have one & only on= e final unittest block controlling a bunch of (named!) test funcs, in each = module: // =3D=3D=3D test =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D void testFeature1 () { ... } void testFeature2 () { ... } void testFeature3 () { ... } unittest { testFeature1(); testFeature2(); testFeature3(); } That way, at any moment, I can run exactly the test func(s) related to the = part or aspect of the app I'm currently working on. A side advantage of thi= s practice is that I can switch off all tests of a given module (esp when w= orking on another one that imports it) by simply commenting out the control= block. Let unittests be named and D's way will be, for me, perfect in both simplic= ity and practicality: unittest feature1 { ... } unittest feature2 { ... } unittest feature3 { ... } unittest { feature1; feature2; feature3; } (Note this is an additive feature, I guess: would be backward compatible, p= rovided unnamed unittests remain automatically run (with --unittest); the '= top' one would be unnamed.) Denis -- -- -- -- -- -- -- vit esse estrany =E2=98=A3 spir.wikidot.com
Jan 03 2011
prev sibling next sibling parent spir <denis.spir gmail.com> writes:
On Mon, 3 Jan 2011 22:11:34 +0100
Andrej Mitrovic <andrej.mitrovich gmail.com> wrote:

 Offtopic (sry): On the other hand, D could do the same for the build
 process. Bringing RDMD's abilities to DMD would simplify the build
 process for many people.

+++ Newcomers (like myself) can meet rdmd quite late, just by chance. If integr= ated into dmd, should even be the default mode. People coming from language= s in which linking is not automatic would love it, others just find it norm= al (!). Denis -- -- -- -- -- -- -- vit esse estrany =E2=98=A3 spir.wikidot.com
Jan 03 2011
prev sibling next sibling parent reply Jonathan M Davis <jmdavisProg gmx.com> writes:
On Monday 03 January 2011 16:39:49 Nick Sabalausky wrote:
 "Jonathan M Davis" <jmdavisProg gmx.com> wrote in message
 news:mailman.372.1294029902.4748.digitalmars-d puremagic.com...
 
 std.unittests is intended to help you write more informative unit test
 code and
 save you from writing some of the boilerplate code in unit tests, but it
 is
 _not_ intended to fundamentally alter how unit tests in D function.
 
 So, please have a look at the code. I've made changes according to
 previous
 suggestions, and I think that the result is quite a bit better than my
 original
 proposal. If all goes well, perhaps Andrei will put it up for vote soon,
 and we
 can actually have it in Phobos.

I'm absolutely in favor of it going into Phobos, even if it goes in 100% as-is. That said, there are three issues I have with it (in no order): 1. I completely agree with other people's suggestions of changing some of the function names.

Well, I'm certainly up for that if enough people agree.
 2. The assertOp* functions are an ugly JUnit/NUnit-style kludge. I'd much
 rather use something that's more like:
 
 int x = 2;
 assert(x == 7, x);
 // Or
 assert(q{_==7}, x);
 // Or
 assert(q{ #x#==7 });
 // etc, anything that involves writing a real expression...
 
 Output:
 
 Failed: [x == 7] x: 2
 
 Some of those exact thing can't be done right now, but it's been suggested
 that there should be some _traits-ish way to get the actual code sent to a
 function's param as a string. That would make a lot of it possible. Even
 without that, it would also be possible with mixins and q{}. And yes,
 people whine about that sort of thing being ugly, and I agree, but this is
 just another reason to clean mixins up. And even with the mixin() and q{}
 noise, I'd still vastly prefer it because of the same reason I'm a
 proponent of operator overloading - I don't ever want to have to write a
 basic expression in some bizarre "Add(x, y)" style.

Well, it's not possible to overload assert, so that sort of thing really doesn't work. You're kind of stuck with something JUnit-ish in terms of names, though assertCmp and assertOpCmp both take a string for the operator, so you don't have assertLessThan, assertGreaterThan, etc. And what's there uses lazy instead of string mixins, because it's cleaner that way. lazy does the job just fine. And even if you could overload assert, you'd likely run into the same sort of problems that come up when you try and overload constructors and the like and the parameters aren't different enough to allow you to do all of the overloads that you want to do. At least JUnit-like naming is clear, and you're only going to use it unit tests, so it's fairly confined. So, if it bothers you, at least it doesn't affect your actual program code.
 3. Without the ability make them non-fatal (and catch & report
 unexpectedly-thrown exceptions, and optionally "fatalize" after a few of
 them have been run), I'd never use any of those assert* functions directly.
 It's possible they might be useful in the implementation of an alternate
 unittest lib, though.

Well, like I said, these functions are intended to work the same way that assert does. They're not intended to alter how unit testing works in D, and continuing a unittest block after an assertion/unit testing function fails would be changing how they work. It may be worthwhile to figure out a way to have some set of functions which report errors but don't actually throw an AssertError for those who really want that sort of thing, but without hooks of some kind in the built-in unit testing framework, I don't know how you'd do that very cleanly. So, if that sort of functionality is really wanted, changes are going to have to be made in the core unit testing stuff (which is implemented in druntime, I think, but it may require compiler changes as well). - Jonathan M Davis
Jan 03 2011
parent "Nick Sabalausky" <a a.a> writes:
"Jonathan M Davis" <jmdavisProg gmx.com> wrote in message 
news:mailman.405.1294111260.4748.digitalmars-d puremagic.com...
 On Monday 03 January 2011 16:39:49 Nick Sabalausky wrote:
 2. The assertOp* functions are an ugly JUnit/NUnit-style kludge. I'd much
 rather use something that's more like:

 int x = 2;
 assert(x == 7, x);
 // Or
 assert(q{_==7}, x);
 // Or
 assert(q{ #x#==7 });
 // etc, anything that involves writing a real expression...

 Output:

 Failed: [x == 7] x: 2

 Some of those exact thing can't be done right now, but it's been 
 suggested
 that there should be some _traits-ish way to get the actual code sent to 
 a
 function's param as a string. That would make a lot of it possible. Even
 without that, it would also be possible with mixins and q{}. And yes,
 people whine about that sort of thing being ugly, and I agree, but this 
 is
 just another reason to clean mixins up. And even with the mixin() and q{}
 noise, I'd still vastly prefer it because of the same reason I'm a
 proponent of operator overloading - I don't ever want to have to write a
 basic expression in some bizarre "Add(x, y)" style.

Well, it's not possible to overload assert, so that sort of thing really doesn't work. You're kind of stuck with something JUnit-ish in terms of names, though assertCmp and assertOpCmp both take a string for the operator, so you don't have assertLessThan, assertGreaterThan, etc. And what's there uses lazy instead of string mixins, because it's cleaner that way. lazy does the job just fine. And even if you could overload assert, you'd likely run into the same sort of problems that come up when you try and overload constructors and the like and the parameters aren't different enough to allow you to do all of the overloads that you want to do. At least JUnit-like naming is clear, and you're only going to use it unit tests, so it's fairly confined. So, if it bothers you, at least it doesn't affect your actual program code.

The actual name "assert" isn't really what I was getting at. I realize that's not usable, and that's fine. What I meant was, for instance, your documentatin includes the example: assertOpBinary!"+"(Foo(5), 7, Foo(12)); (Although it doesn't seem to compile for me...) That's quite a mess compared to the following: assert(Foo(5) + 7 == Foo(12)); That can currently be done, of course, but it's output on failure obviously doesn't give as much useful information as assertOpBinary. I think a middle ground is needed. For instance, with the assert utils in my SemiTwist D Tools library, you can do this: mixin(deferEnsure!(`Foo(12)`, `_ == Foo(5) + 7`)); And the output upon failure still gives you this information: Expression 'Foo(12)': Expected: _ == Foo(5) + 7 Actual: 999 And, though I haven't done so yet, that can be generalized to allow it to display what Foo(5) evaluated to. Admittedly, there's lots of room for improvement in my syntax there, but the point is that you write an actual expression and still get all the same useful information. So, just as one off-the-top-of-my-head idea, your lib could have something like: assertExpression(`#Foo(5)# + 7 == #Foo(12)#`); The # delimiters (or whatever system of delimiters would make sense) indicate what sub-expression should be displayed, ex: Assert failed: [Foo(5) + 7 == Foo(12)] Foo(5): 6 Foo(12): 12 Or: assertExpression(q(a + 7 == b}, Foo(5), Foo(12)); // (same output as before) The whole things may or may not need to be wrapped in a "mixin()", I don't know. Ie, something like that.
 3. Without the ability make them non-fatal (and catch & report
 unexpectedly-thrown exceptions, and optionally "fatalize" after a few of
 them have been run), I'd never use any of those assert* functions 
 directly.
 It's possible they might be useful in the implementation of an alternate
 unittest lib, though.

Well, like I said, these functions are intended to work the same way that assert does. They're not intended to alter how unit testing works in D, and continuing a unittest block after an assertion/unit testing function fails would be changing how they work. It may be worthwhile to figure out a way to have some set of functions which report errors but don't actually throw an AssertError for those who really want that sort of thing, but without hooks of some kind in the built-in unit testing framework, I don't know how you'd do that very cleanly. So, if that sort of functionality is really wanted, changes are going to have to be made in the core unit testing stuff (which is implemented in druntime, I think, but it may require compiler changes as well).

What about a flag that sets whether the AssertError is thrown or accumulated? For instance: // How it (presumably) is right now: void assertEqual(...) { perform test; if(test failed) { throw new AssertError(all relevant info); } } // Suggestion: bool accumulateFailures=false; AssertError[] failures; void assertEqual(...) { if(accumulateFailures) { try perform test; catch(e) { failures ~= new AssertError(all relevant info, including e); return; } } else perform test; if(test failed) { auto err = new AssertError(all relevant info); if(accumulateFailures) failures ~= err; else throw err; } } void fatalize() { if(failures.length > 0) { throw new AssertError(concat everyting in failures); } }
Jan 03 2011
prev sibling next sibling parent Jens Mueller <jens.k.mueller gmx.de> writes:
Nick Sabalausky wrote:
 "Jonathan M Davis" <jmdavisProg gmx.com> wrote in message 
 news:mailman.405.1294111260.4748.digitalmars-d puremagic.com...
 Well, it's not possible to overload assert, so that sort of thing really 
 doesn't
 work. You're kind of stuck with something JUnit-ish in terms of names, 
 though
 assertCmp and assertOpCmp both take a string for the operator, so you 
 don't have
 assertLessThan, assertGreaterThan, etc. And what's there uses lazy instead 
 of
 string mixins, because it's cleaner that way. lazy does the job just fine.

 And even if you could overload assert, you'd likely run into the same sort 
 of
 problems that come up when you try and overload constructors and the like 
 and
 the parameters aren't different enough to allow you to do all of the 
 overloads
 that you want to do. At least JUnit-like naming is clear, and you're only 
 going
 to use it unit tests, so it's fairly confined. So, if it bothers you, at 
 least it
 doesn't affect your actual program code.

The actual name "assert" isn't really what I was getting at. I realize that's not usable, and that's fine. What I meant was, for instance, your documentatin includes the example: assertOpBinary!"+"(Foo(5), 7, Foo(12)); (Although it doesn't seem to compile for me...) That's quite a mess compared to the following: assert(Foo(5) + 7 == Foo(12));

I had the same impression. But I think it should be there for consistency reasons. Because there is also assertOpOpAssign. But you're right in case of assertOpBinary I prefer writing assertEqual(Foo(5) + 7, Foo(12)); Becaues it's one line and it gives enough information for me. But this does not hold for assertOpOpAssign anymore.
 That can currently be done, of course, but it's output on failure obviously 
 doesn't give as much useful information as assertOpBinary.
 
 I think a middle ground is needed. For instance, with the assert utils in my 
 SemiTwist D Tools library, you can do this:
 
 mixin(deferEnsure!(`Foo(12)`, `_ == Foo(5) + 7`));
 
 And the output upon failure still gives you this information:
 
 Expression 'Foo(12)':
 Expected: _ == Foo(5) + 7
 Actual: 999
 
 And, though I haven't done so yet, that can be generalized to allow it to 
 display what Foo(5) evaluated to. Admittedly, there's lots of room for 
 improvement in my syntax there, but the point is that you write an actual 
 expression and still get all the same useful information.
 
 So, just as one off-the-top-of-my-head idea, your lib could have something 
 like:
 
 assertExpression(`#Foo(5)# + 7 == #Foo(12)#`);
 
 The # delimiters (or whatever system of delimiters would make sense) 
 indicate what sub-expression should be displayed, ex:
 
 Assert failed: [Foo(5) + 7 == Foo(12)]
     Foo(5): 6
     Foo(12): 12
 
 Or:
 
 assertExpression(q(a + 7 == b}, Foo(5), Foo(12));
 // (same output as before)
 
 The whole things may or may not need to be wrapped in a "mixin()", I don't 
 know.
 
 Ie, something like that.

Sounds useful and could be added later. Jens
Jan 05 2011
prev sibling next sibling parent Jens Mueller <jens.k.mueller gmx.de> writes:
Lars T. Kyllingstad wrote:
 Sorry for not commenting on earlier iterations of this module.  For the 
 most part, I think it looks pretty good, and I also think it will be 
 useful.  Often, I find myself writing stuff like
 
   assert (someVar == someVal,
     text("Wrong value for someVar: ", someVar));
 
 and assertEqual() will be nice to have for those cases. :)  I just have 
 some minor comments.
 
 1. I think assertExThrown() and assertExNotThrown() should be named 
 assertThrown() and assertNotThrown(), because they can intercept any 
 subclass of Throwable, not just Exception, and because you rarely throw 
 anything else, so it seems redundant.

I think this is a good renaming. assertThrown and assertNotThrown are better names.
 2. I think the name getMsg() is too non-specific.  I'd prefer if it was 
 renamed to throwableMsg(), thrownMsg(), or something similar.  The 
 function should also be moved upwards a bit so its documentation follows 
 that of the assert*Thrown() functions.  Then, the various functions will 
 be nicely grouped in the documentation -- all the exception handling 
 tests come first, followed by the value tests.

Right. getMsg() is a too general name. Of the given ones I like thrownMsg() best because it's concise. Jens
Jan 05 2011
prev sibling next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
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
Jan 05 2011
next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 1/5/11 6:09 PM, Andrei Alexandrescu wrote:
[snip]

One more thing. Since assert() is available and useful outside 
unittests, I don't see a reason for which assertPred and friends should 
only be available during unittesting. I can sure use them in regular code.

Andrei
Jan 05 2011
parent reply bearophile <bearophileHUGS lycos.com> writes:
Andrei:

 One more thing. Since assert() is available and useful outside 
 unittests, I don't see a reason for which assertPred and friends should 
 only be available during unittesting. I can sure use them in regular code.

I guess assertPred creates statistics to be printed at the end of the unittests? I don't think mixing the two groups of things is good. Bye, bearophile
Jan 06 2011
next sibling parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 1/6/11 3:57 AM, bearophile wrote:
 Andrei:

 One more thing. Since assert() is available and useful outside
 unittests, I don't see a reason for which assertPred and friends should
 only be available during unittesting. I can sure use them in regular code.

I guess assertPred creates statistics to be printed at the end of the unittests? I don't think mixing the two groups of things is good.

The printed stats could be wrapped in version(unittest). Andrei
Jan 06 2011
prev sibling parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 1/6/11 3:57 AM, bearophile wrote:
 Andrei:

 One more thing. Since assert() is available and useful outside
 unittests, I don't see a reason for which assertPred and friends should
 only be available during unittesting. I can sure use them in regular code.

I guess assertPred creates statistics to be printed at the end of the unittests? I don't think mixing the two groups of things is good.

Oh, and if assertXxxx doesn't do what assert does (throw an error in case things go wrong) it should not be called such. Call it expectXxx. Andrei
Jan 06 2011
prev sibling next sibling parent reply Ary Borenszweig <ary esperanto.org.ar> writes:
I prefer assert, assertFalse, assertEqual and assertNotEqual.

Compare this:
assertPredicate!"a < b"(1 + 1, 3);

To this:
assert(1 + 1 < 3)

Or to this:

assertLess(1 + 1, 3)

Ok, the first one is more generic. But so the error message for the assertion
failure will be more generic, when you want exactly the opposite to happen.
Plus your brain has to think more to read it (starting from the point that it's
longer).

assertPredicate!"<"(1 + 1, 3) reminds me of polish notation, which is very good
for machines but not very good for humans.

Sorry, you can have more specific error messages: on compile time you parse the
given string and see. If it's "<" then you give an error message regarding
less. Then you end up having a huge function, bearly understandable, and with a
huge documentation. On the other case you end up having very well defined
functions, very simple to read and write, well documented, and also with a very
simple implementation. What's the benefit of inlining all of that if it's
going to be used in unit tests or in debug mode?

I'm not a big fan of IDEs anymore but having many functions also helps with the
autocompletion, to get the code right on the first shot, instead of
assertPredicate!insertYourStringHere(...).
Jan 05 2011
next sibling parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 1/5/11 8:54 PM, Ary Borenszweig wrote:
 I prefer assert, assertFalse, assertEqual and assertNotEqual.

 Compare this:
 assertPredicate!"a<  b"(1 + 1, 3);

 To this:
 assert(1 + 1<  3)

 Or to this:

 assertLess(1 + 1, 3)

 Ok, the first one is more generic. But so the error message for the assertion
failure will be more generic, when you want exactly the opposite to happen.
 Plus your brain has to think more to read it (starting from the point that
it's longer).

 assertPredicate!"<"(1 + 1, 3) reminds me of polish notation, which is very
good for machines but not very good for humans.

One problem with using distinct names is the explosion thereof. Google unittests have EXPECT_EQ, EXPECT_NE, EXPECT_LT, EXPECT_GT, EXPECT_LE, EXPECT_GE, EXPECT_TRUE, EXPECT_FALSE and probably others. I was able to write these from memory because I used to know Fortran, which uses the same two-letter conventions for comparisons. At that point I think we can say, wait a minute, why not use the respective symbols which are impossible to forget? Andrei
Jan 06 2011
prev sibling next sibling parent Bruno Medeiros <brunodomedeiros+spam com.gmail> writes:
On 06/01/2011 02:54, Ary Borenszweig wrote:
 I prefer assert, assertFalse, assertEqual and assertNotEqual.

 Compare this:
 assertPredicate!"a<  b"(1 + 1, 3);

 To this:
 assert(1 + 1<  3)

 Or to this:

 assertLess(1 + 1, 3)

I agree with Ary here, at least with regard to assertEquals. I'm a very test and contracts minded programmer so I use asserts a lot, and specifically assertEquals (possibly assertSame as well) is common enough that I want a shortcut to type them, I don't want to have to type the predicate. And it's more readable as well. (The implementation can use assertPredicate, that's good, but that's not the issue) And yes, for "a == b" I won't have to type the predicate that it is the default: assertPredicate(1 + 1, 2); However it still suffers from the readability issue, and it won't work for a similar predicate I also use quite often as well, assertAreEqual, which accepts nulls( "(a == b) || (a != null && b != null && a.equals(b))" In practice it won't be a problem at all because we can define aliases, like Jonathan said, however it would be nice if the std library would be mindful of this and provide shortcuts for the very common predicates. -- Bruno Medeiros - Software Engineer
Jan 28 2011
prev sibling parent Bruno Medeiros <brunodomedeiros+spam com.gmail> writes:
On 06/01/2011 02:54, Ary Borenszweig wrote:
 I'm not a big fan of IDEs anymore but having many functions also helps with
the autocompletion, to get the code right on the first shot, instead of
 assertPredicate!insertYourStringHere(...).

(!) Man, what have those Ruby guys done to you? You've been seduced by the power of the Dark Side... :P -- Bruno Medeiros - Software Engineer
Jan 28 2011
prev sibling parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 1/6/11 2:59 PM, Tomek Sowiński wrote:
 Andrei Alexandrescu napisał:

 * Remove assertCmp and fold it into assertPredicate. With trivial
 metaprogramming you can distinguish an operator from an actual expression.

 assertPredicate!"<"(1 + 1, 2.1);

If it's trivial, put it into binaryFun, so that everyone else can reap rewards: sort!">"(range); auto sum = reduce!"+"(range);

Yah, I've been thinking of it.
 As for operator-only unaryFuns, don't; they bring more confusion than benefit.

Agreed. Andrei
Jan 06 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
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
Jan 05 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Wednesday, January 05, 2011 18:54:32 Ary Borenszweig wrote:
 I prefer assert, assertFalse, assertEqual and assertNotEqual.
 
 Compare this:
 assertPredicate!"a < b"(1 + 1, 3);
 
 To this:
 assert(1 + 1 < 3)
 
 Or to this:
 
 assertLess(1 + 1, 3)
 
 Ok, the first one is more generic. But so the error message for the
 assertion failure will be more generic, when you want exactly the opposite
 to happen. Plus your brain has to think more to read it (starting from the
 point that it's longer).
 
 assertPredicate!"<"(1 + 1, 3) reminds me of polish notation, which is very
 good for machines but not very good for humans.
 
 Sorry, you can have more specific error messages: on compile time you parse
 the given string and see. If it's "<" then you give an error message
 regarding less. Then you end up having a huge function, bearly
 understandable, and with a huge documentation. On the other case you end
 up having very well defined functions, very simple to read and write, well
 documented, and also with a very simple implementation. What's the benefit
 of inlining all of that if it's going to be used in unit tests or in debug
 mode?
 
 I'm not a big fan of IDEs anymore but having many functions also helps with
 the autocompletion, to get the code right on the first shot, instead of
 assertPredicate!insertYourStringHere(...).

Valid points. However, assertPred!"=="(1 + 1, 3) isn't much harder to read than assertEqual(1 + 1, 3). Also, you could create an alias for assertPred"==" to assertEqual, if you wanted to. So, we could easily add assertEqual and the like as aliases if enough people really wanted them. Also, assertPred can definitely be made to print out error messages just as informative as what I have now. You just either have specialized versions of assertPred or you have lots of static ifs inside of asertPred (or both). And actually, now that I think of it, it should be possible to have specific tests for each overloaded operator function (rather than the operator itself) in the same manner. So, assertOpOpAssign could become assertPred!"opOpAssign", and assertPred would be smart enough to handle it correctly. Assuming that I can pull if off correctly (which I should be able to given a bit of time and effort), assertPred could be quite clean and very powerful. And if the name bothers you enough, alias can fix it for you. - Jonathan M Davis
Jan 05 2011
prev sibling next sibling parent reply Michel Fortin <michel.fortin michelf.com> writes:
I'm not sold on the concept. The whole point of this module seems to 
offer a way to replace the built-in assertion mechanism with a 
customized one, with the sole purpose of giving better error messages. 
So we're basically encouraging the use of:

	assertPredicate!"a > b"(a, b, "message");

instead of:

	assert(a > b, "message");

It looks like an uglification of the language to me.

I agree that getting better error messages is important (very important 
in fact), but keeping the code clean is important too. If the built-in 
assert doesn't give us good enough error messages, perhaps it's the 
built-in assert that should be improved. The compiler could give the 
values on both side of the operator to the assertion handler, which 
would in turn print values and operator as part of the error message.

So to me this module is a temporary fix until the compiler is capable 
of giving the necessary information to the assertion handler. I sure 
hope it won't be needed for too long.

(Note: this criticism doesn't apply to those assertions dealing with 
exceptions.)

-- 
Michel Fortin
michel.fortin michelf.com
http://michelf.com/
Jan 05 2011
next sibling parent reply Michel Fortin <michel.fortin michelf.com> writes:
On 2011-01-05 22:57:00 -0500, Jonathan M Davis <jmdavisProg gmx.com> said:

 On Wednesday 05 January 2011 19:35:13 Michel Fortin wrote:
 I'm not sold on the concept. The whole point of this module seems to
 offer a way to replace the built-in assertion mechanism with a
 customized one, with the sole purpose of giving better error messages.
 So we're basically encouraging the use of:
 
 	assertPredicate!"a > b"(a, b, "message");
 
 instead of:
 
 	assert(a > b, "message");
 
 It looks like an uglification of the language to me.
 
 I agree that getting better error messages is important (very important
 in fact), but keeping the code clean is important too. If the built-in
 assert doesn't give us good enough error messages, perhaps it's the
 built-in assert that should be improved. The compiler could give the
 values on both side of the operator to the assertion handler, which
 would in turn print values and operator as part of the error message.
 
 So to me this module is a temporary fix until the compiler is capable
 of giving the necessary information to the assertion handler. I sure
 hope it won't be needed for too long.
 
 (Note: this criticism doesn't apply to those assertions dealing with
 exceptions.)

Well, I'm not about to claim that assert can't be fixed to give better error messages, but right now all it takes is a value which converts to bool for the test. a > b may obviously be convertible to something similar to assertPred!">"(a, b), but what about something like 1 + 1 < b or a < b < c. As expressions get progressively more complicated, it very quickly becomes non- obvious what someone would really want to print on error. Would 1 + 1 < b print 2 and b's value? Would it print 1, 1, and b's value? 1, 1, 2, and b's value? Sure, it may be obvious to the programmer what they intended, but it doesn't take much for it to be very difficult for the compiler to figure it out for you.

I think "assert(1+a < b)" should print the same thing as "static assert(1+a < b)" does. What "static assert(1+a < b)" prints when a == 1 and b == 0 is "(2 < 0) is false". Try it yourself.
 Also, assertPred!">"(a, b) would print out a more informative error message on
 its own. You wouldn't need to give it an additional message for it to be more
 informative. That would defeat the point. Even assertPred!"a > b"(a, b) 
 could be
 more informative (assuming that it treats a > b as a general predicate rather
 than determining that it's actually >) by printing the values that it's given.
 So, that's definitely a leg up on assert(a > b) right there.

I don't believe it to be that difficult. From inside the compiler, you have access to the expression tree. All the compiler needs to do is check whether the top level expression is a binary op, and if so decompose it this way (assuming no given message here): auto a = operand1; auto b = operand2; if (a <binaryop> b) _d_assert_msg2("(%s <binaryop> %s) is false", __FILE__, __LINE__, &a, typeid(a), &b, typeid(b)); As for other expressions it could simply print the value by lowering it this way: auto result = <expression>; if (result) _d_assert_msg1("(%s) is false", __FILE__, __LINE__, &result, typeid(result)); That would basically give you the same error messages as static assert. Currently, assertions are lowered like this instead: if (expression) _d_assertm(moduleinfo, __LINE__); or like this when a message is provided: if (expression) _d_assertm(<message>, __FILE__, __LINE__); Sure, it's more complicated than doing it for static asserts where everything is known at compile-time, but I don't believe it to be that difficult.
 By passing each of the values to assertPred, we're able to print them out on
 failure without the computer having to understand what the predicate does, even
 when the values are arbitrary expressions. That would be very hard to 
 do with an
 improved assert which just took the expression. I mean, try and write a 
 function
 that took 1 + 1 > b or a < b < c as a string and tried to correctly print out
 values which are meaningful to the programmer. That would be _really_ hard. And
 while assertPred may not be able to understand a generic predicate, it can know
 about specific operators and/or functions and therefore give more informative
 error messages than it would be able to do with a generic predicate.

It's hard to do using a function. But it's easy for 'assert' because it's a language construct handled by the compiler.
 So, correctly implemented, I think that assertPred actually makes a lot more
 sense than trying to soup up assert and getting the compiler to guess at what
 the programmer really wants.

I don't really see what the compiler has to guess. The compiler just takes the top-level expression and pass its value to the assertion handler, and for binary expressions it can pass two values plus the operator's string. What cases are not covered by that? -- Michel Fortin michel.fortin michelf.com http://michelf.com/
Jan 05 2011
parent reply Michel Fortin <michel.fortin michelf.com> writes:
On 2011-01-06 01:36:32 -0500, Jonathan M Davis <jmdavisProg gmx.com> said:

 Okay. I thought this through a bit more, and I think that if the evaluation was
 stopped when all that was left in the expression was boolean operators 
 and their
 operands, then that pretty much has to be what the programmer was trying to
 print. That being the case, you could theoretically get assert to do it, but I
 would expect that that would make assert awfully complicated. Done properly, it
 would be fantastic, but since it can be done in a library with something like
 assertPred!() much more easily, I wouldn't expect such an enhancement to assert
 to be implemented any time soon.

Some people though const(Object)ref was impossible too. I agree that assertPred!() is useful in the meanwhile.
 A few of the things that I'm thinking of doing with assertPred!() have to be
 special cased though in a way which wouldn't work with assert. For 
 instance, I'm
 thinking of doing something like assertPred!("opCmp", 0)(foo(), bar()), which
 would do what assertOpCmp!() does now. As such, opCmp() isn't a predicate, if
 you rewrote it to assert(foo().opCmp(bar()) == 0), even stopping 
 evaluation when
 all you have left is boolean operators and their operands wouldn't work, since
 then you'd get something like 1 == 0 rather than (assuming that foo() and bar()
 return something like BigInts which have the values "4" and "2" respectively)
 something like "opCmp() == 0 failed: 4 > 2".

For these cases you might prefer a custom assertion function, such as assertOpCmp!(). I think it's important that assert be useful for the common case, it doesn't mean that more specialized solutions cannot be created for more specialized cases, such as this one.
 Sure, you could make it so that assert could do that sort of thing too, 
 but then
 you're adding special cases and complicating assert even further, and it's also
 much harder to add such cases, because then you're actually changing the
 _language_ rather than the standard library, and every compiler would have to
 follow suit.

It's a quality of implementation issue. What error message an assert gives is no more part of the language than what error message the compiler outputs when it encounters some incorrect code. The compiler could simply print "error" each time it encounters an error and it'd be compliant (even though that would be extremely annoying). Beside, all the current working compilers share the same front end, so they'll all get this for free. So I think it's worth it.
 So, yes assert could theoretically be improved to do a lot more and do a lot
 better, but it won't ever do as much as I could do with assertPred!(), and even
 if assert _is_ improved as you suggest (which would certainly be cool), then
 there's still some value in assertPred!().

There's an other issue that's bothering me with these assertion functions... with 'assert', assertions behaves differently depending on where you write them. In regular code, on failure it calls _d_assert*, in a unit test on failure it calls _d_unittest*, and in contracts for virtual function... well that case I don't understand it fully but it does something very special to implement contract inheritance. What does assertPred do? It does "throw new AssertError(...)", irrespective of the context. I'm pretty sure that'll break contract inheritance if used inside a contract. To be truly a replacement for assert, assertPred would need to know in which context it is called and generate the appropriate code, although I somewhat doubt it is even possible to generate the right code for contracts without the compiler doing it. -- Michel Fortin michel.fortin michelf.com http://michelf.com/
Jan 06 2011
parent Michel Fortin <michel.fortin michelf.com> writes:
On 2011-01-06 11:34:48 -0500, Jonathan M Davis <jmdavisProg gmx.com> said:

 On Thursday 06 January 2011 05:30:56 Michel Fortin wrote:
 There's an other issue that's bothering me with these assertion
 functions... with 'assert', assertions behaves differently depending on
 where you write them. In regular code, on failure it calls _d_assert*,
 in a unit test on failure it calls _d_unittest*, and in contracts for
 virtual function... well that case I don't understand it fully but it
 does something very special to implement contract inheritance.
 
 What does assertPred do? It does "throw new AssertError(...)",
 irrespective of the context. I'm pretty sure that'll break contract
 inheritance if used inside a contract. To be truly a replacement for
 assert, assertPred would need to know in which context it is called and
 generate the appropriate code, although I somewhat doubt it is even
 possible to generate the right code for contracts without the compiler
 doing it.

I don't know anything about this. As far as I know, there's no difference between assert in unittest blocks and assert in contracts. That being said, you're probably more knowledgeable than I am about that. I would have thought was that all it would take would be for the AssertError to be handled appropriately by whatever catches it. In the case of contracts, I would think that it's an issue of the right contract code being called in the right order, and that which contract threw the test would be irrelevant (the stack track would show where it was; all that matters from the runtime's perspective is that there _was_ an AssertError thrown and that execution is therefore going to be stopping).

The thing is that for 'in' contracts you need to have *one* contract work in the inheritance chain. So if a contract fails it should just check the inherited contract, and if it fails it continues like that. If all the inherited contract fails then only then you must throw. I'm not sure exactly how DMD generates the code, but I'm pretty sure it doesn't throw then catch then throw then catch until it reaches the last inherited contract (that'd be unacceptably slow), so if you do throw in the contract then you're probably breaking how it works. That said, I'm pretty sure this only apply to 'in' contracts. Other contracts don't exhibit this all-must-fail-to-fail behaviour. Here's a test case: class A { // accepts i == 0 void fun(int i) in { assert(i == 0); } body {} } class B : A { // accepts i == 0 || i == 1 override void fun(int i) in { assert(i == 1); } body {} } B b = new B; b.fun(1); b.fun(0); b.fun(-1); // only this one breaks the contract
 As for unittest blocks, I thought that it caught the AssertError from the
 unittest block and dealt with it. If it does that, then I don't see why 
 it would
 need a special version of assert. I know that it _used_ to be 
 different, because
 Walter temporarily made it so that within unittest blocks assert set a flag
 saying that the test failed and printed the error rather than throwing an
 AssertError, but Andrei and others complained about that (both that 
 assert would
 have different behavior in different places and that a unittest block would
 continue after a failure), and it was agreed that assert would throw an
 AssertError like it normally does.

The code it generates is different: it calls different assertion failure handler in the runtime (_d_assert and derivatives vs. _d_unittest and derivatives). But druntime was later changed so that both handlers do the same thing (except for different default error messages). As long as druntime makes them do the same thing, there won't be a problem. Should there be a difference between the two? This difference could be used to distinguish failed tests from failed assertions in function the test is calling, but I'm not too sure how useful that'd be.
 So, I don't know if assert does something different depending on where it is
 called. The only special case for assert that I'm aware of is assert(0), which
 becomes the halt instruction with -release rather than going away. If 
 there is a
 difference, then we probably need to understand what it is and what issues it
 could cause. Ideally, there wouldn't be any difference.

assert(0) is one special case, assert(object) is another -- it calls the object's invariant. The rest is undocumented as far as I know.
 However, I _have_ been using these functions in unit tests, and they work fine
 there. So, as far as unit testing goes, they work. I have _not_ been using them
 in contracts. I created them specifically with unit testing in mind 
 (and in fact,
 with the current code, the entire module is in a version(unittest) block). It
 sounds like there are some folks (including Andrei) who think that it should be
 useable in normal contracts just like assert is. That's simple to fix 
 by removing
 the version(unittest) block, but I don't know if there are any issues with
 throwing an AssertError from a function called within a contract rather than
 asserting directly inside a contract. If there is, I would think that that's a
 more general problem. I've been doing that all the time with invariants (just
 not with any of my unit testing functions), and that's worked as far as I can
 tell, but I've been using structs primarily, which wouldn't have contract
 inheritance. So, I think that assert should _definitely_ work normally when
 called from a function called from a contract rather than when used directly in
 a contract, but I don't know that that never causes problems. We need someone
 who actually knows what assert does in each case to say whether there's an
 issue, I think.

As things stands now, I'm pretty sure the issue only apply if you use one of your assert derivative in 'in' contracts (see test case above). But I think we need to have Walter involved in this thread to tell us where and when and how we can use custom assert functions in place of 'assert' without breaking things. -- Michel Fortin michel.fortin michelf.com http://michelf.com/
Jan 06 2011
prev sibling next sibling parent reply Lutger Blijdestijn <lutger.blijdestijn gmail.com> writes:
Michel Fortin wrote:

 I'm not sold on the concept. The whole point of this module seems to
 offer a way to replace the built-in assertion mechanism with a
 customized one, with the sole purpose of giving better error messages.
 So we're basically encouraging the use of:
 
 assertPredicate!"a > b"(a, b, "message");
 
 instead of:
 
 assert(a > b, "message");
 
 It looks like an uglification of the language to me.

As you said, it is all about the error messages, not replacing assert perse. So this comparison would be more fair, using the syntax suggested by Andrei: assertPred!">"(a, b); vs assert(a > b, format("a > b failed: [%s] is not > [%s]", a, b) ); If you really want the error message, regular asserts are a bit uglier and duplicate code. Cutting down on the characters further, 'expect' could be used instead of assertPred. Some unittest libraries also use this, but I can't remember which one. I think it then makes sense to unify everything: expect!">"(a, b); expect!Exception( { throw new Exception(""); } () );
Jan 06 2011
parent reply Michel Fortin <michel.fortin michelf.com> writes:
On 2011-01-06 06:45:41 -0500, Lutger Blijdestijn 
<lutger.blijdestijn gmail.com> said:

 As you said, it is all about the error messages, not replacing assert perse.
 So this comparison would be more fair, using the syntax suggested by Andrei:
 
 assertPred!">"(a, b);
 
 vs
 
 assert(a > b, format("a > b failed: [%s] is not > [%s]", a, b) );
 
 If you really want the error message, regular asserts are a bit uglier and
 duplicate code.

The whole point of my proposal is to make the regular asserts print a message similar to yours *by default*, when you don't specify any message. This is possible because assert is not a function, it's a language construct handled by the compiler. The compiler has access to the whole expression tree, and it can rewrite the code to store intermediate results in variables it can later give to the assertion handler in case of failure. -- Michel Fortin michel.fortin michelf.com http://michelf.com/
Jan 06 2011
parent reply Lutger Blijdestijn <lutger.blijdestijn gmail.com> writes:
Michel Fortin wrote:

 On 2011-01-06 06:45:41 -0500, Lutger Blijdestijn
 <lutger.blijdestijn gmail.com> said:
 
 As you said, it is all about the error messages, not replacing assert
 perse. So this comparison would be more fair, using the syntax suggested
 by Andrei:
 
 assertPred!">"(a, b);
 
 vs
 
 assert(a > b, format("a > b failed: [%s] is not > [%s]", a, b) );
 
 If you really want the error message, regular asserts are a bit uglier
 and duplicate code.

The whole point of my proposal is to make the regular asserts print a message similar to yours *by default*, when you don't specify any message. This is possible because assert is not a function, it's a language construct handled by the compiler. The compiler has access to the whole expression tree, and it can rewrite the code to store intermediate results in variables it can later give to the assertion handler in case of failure.

I understood, but was reacting to the proposition that assertPred is not an improvement over the current situation, isn't that what you were saying? Just being pragmatic, it's unlikely such big changes to assert are going to land in dmd anytime soon. But if they are, yeah that would be cool.
Jan 06 2011
parent Michel Fortin <michel.fortin michelf.com> writes:
On 2011-01-06 08:44:33 -0500, Lutger Blijdestijn 
<lutger.blijdestijn gmail.com> said:

 Michel Fortin wrote:
 
 The whole point of my proposal is to make the regular asserts print a
 message similar to yours *by default*, when you don't specify any
 message. This is possible because assert is not a function, it's a
 language construct handled by the compiler. The compiler has access to
 the whole expression tree, and it can rewrite the code to store
 intermediate results in variables it can later give to the assertion
 handler in case of failure.

I understood, but was reacting to the proposition that assertPred is not an improvement over the current situation, isn't that what you were saying? Just being pragmatic, it's unlikely such big changes to assert are going to land in dmd anytime soon. But if they are, yeah that would be cool.

assertPred is an improvement for error messages but it's not without a price. Basically you trade the very straightforward assertion syntax for a more verbose and less intuitive syntax. My argument is that this is an unnecessary tradeoff since we can just "fix" assert instead. I think the consensus is that 'assert' can and should give us better error messages, but that assertPred is a good stopgap solution for most cases. -- Michel Fortin michel.fortin michelf.com http://michelf.com/
Jan 06 2011
prev sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 1/5/11 9:35 PM, Michel Fortin wrote:
 I'm not sold on the concept. The whole point of this module seems to
 offer a way to replace the built-in assertion mechanism with a
 customized one, with the sole purpose of giving better error messages.
 So we're basically encouraging the use of:

 assertPredicate!"a > b"(a, b, "message");

 instead of:

 assert(a > b, "message");

 It looks like an uglification of the language to me.

 I agree that getting better error messages is important (very important
 in fact), but keeping the code clean is important too. If the built-in
 assert doesn't give us good enough error messages, perhaps it's the
 built-in assert that should be improved. The compiler could give the
 values on both side of the operator to the assertion handler, which
 would in turn print values and operator as part of the error message.

 So to me this module is a temporary fix until the compiler is capable of
 giving the necessary information to the assertion handler. I sure hope
 it won't be needed for too long.

 (Note: this criticism doesn't apply to those assertions dealing with
 exceptions.)

I think this is a valid improvement that can be brought to the language, but we're at a point in D's history where we should ask ourselves how we can get stuff done in the existing language instead of how we can change the language to make stuff easier to get done. Andrei
Jan 06 2011
parent Michel Fortin <michel.fortin michelf.com> writes:
On 2011-01-06 10:10:46 -0500, Andrei Alexandrescu 
<SeeWebsiteForEmail erdani.org> said:

 On 1/5/11 9:35 PM, Michel Fortin wrote:
 I'm not sold on the concept. The whole point of this module seems to
 offer a way to replace the built-in assertion mechanism with a
 customized one, with the sole purpose of giving better error messages.
 So we're basically encouraging the use of:
 
 assertPredicate!"a > b"(a, b, "message");
 
 instead of:
 
 assert(a > b, "message");
 
 It looks like an uglification of the language to me.
 
 I agree that getting better error messages is important (very important
 in fact), but keeping the code clean is important too. If the built-in
 assert doesn't give us good enough error messages, perhaps it's the
 built-in assert that should be improved. The compiler could give the
 values on both side of the operator to the assertion handler, which
 would in turn print values and operator as part of the error message.
 
 So to me this module is a temporary fix until the compiler is capable of
 giving the necessary information to the assertion handler. I sure hope
 it won't be needed for too long.
 
 (Note: this criticism doesn't apply to those assertions dealing with
 exceptions.)

I think this is a valid improvement that can be brought to the language, but we're at a point in D's history where we should ask ourselves how we can get stuff done in the existing language instead of how we can change the language to make stuff easier to get done.

It's not a language change. The language doesn't mandate any particular error message for assertions. It's a quality of implementation issue. -- Michel Fortin michel.fortin michelf.com http://michelf.com/
Jan 06 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Wednesday 05 January 2011 19:35:13 Michel Fortin wrote:
 I'm not sold on the concept. The whole point of this module seems to
 offer a way to replace the built-in assertion mechanism with a
 customized one, with the sole purpose of giving better error messages.
 So we're basically encouraging the use of:
 
 	assertPredicate!"a > b"(a, b, "message");
 
 instead of:
 
 	assert(a > b, "message");
 
 It looks like an uglification of the language to me.
 
 I agree that getting better error messages is important (very important
 in fact), but keeping the code clean is important too. If the built-in
 assert doesn't give us good enough error messages, perhaps it's the
 built-in assert that should be improved. The compiler could give the
 values on both side of the operator to the assertion handler, which
 would in turn print values and operator as part of the error message.
 
 So to me this module is a temporary fix until the compiler is capable
 of giving the necessary information to the assertion handler. I sure
 hope it won't be needed for too long.
 
 (Note: this criticism doesn't apply to those assertions dealing with
 exceptions.)

Well, I'm not about to claim that assert can't be fixed to give better error messages, but right now all it takes is a value which converts to bool for the test. a > b may obviously be convertible to something similar to assertPred!">"(a, b), but what about something like 1 + 1 < b or a < b < c. As expressions get progressively more complicated, it very quickly becomes non- obvious what someone would really want to print on error. Would 1 + 1 < b print 2 and b's value? Would it print 1, 1, and b's value? 1, 1, 2, and b's value? Sure, it may be obvious to the programmer what they intended, but it doesn't take much for it to be very difficult for the compiler to figure it out for you. Also, assertPred!">"(a, b) would print out a more informative error message on its own. You wouldn't need to give it an additional message for it to be more informative. That would defeat the point. Even assertPred!"a > b"(a, b) could be more informative (assuming that it treats a > b as a general predicate rather than determining that it's actually >) by printing the values that it's given. So, that's definitely a leg up on assert(a > b) right there. By passing each of the values to assertPred, we're able to print them out on failure without the computer having to understand what the predicate does, even when the values are arbitrary expressions. That would be very hard to do with an improved assert which just took the expression. I mean, try and write a function that took 1 + 1 > b or a < b < c as a string and tried to correctly print out values which are meaningful to the programmer. That would be _really_ hard. And while assertPred may not be able to understand a generic predicate, it can know about specific operators and/or functions and therefore give more informative error messages than it would be able to do with a generic predicate. So, correctly implemented, I think that assertPred actually makes a lot more sense than trying to soup up assert and getting the compiler to guess at what the programmer really wants. - Jonathan M Davis
Jan 05 2011
prev sibling next sibling parent reply Jonathan M Davis <jmdavisProg gmx.com> writes:
On Wednesday 05 January 2011 21:09:07 Michel Fortin wrote:
 On 2011-01-05 22:57:00 -0500, Jonathan M Davis <jmdavisProg gmx.com> said:
 On Wednesday 05 January 2011 19:35:13 Michel Fortin wrote:
 I'm not sold on the concept. The whole point of this module seems to
 offer a way to replace the built-in assertion mechanism with a
 customized one, with the sole purpose of giving better error messages.
 
 So we're basically encouraging the use of:
 	assertPredicate!"a > b"(a, b, "message");
 
 instead of:
 	assert(a > b, "message");
 
 It looks like an uglification of the language to me.
 
 I agree that getting better error messages is important (very important
 in fact), but keeping the code clean is important too. If the built-in
 assert doesn't give us good enough error messages, perhaps it's the
 built-in assert that should be improved. The compiler could give the
 values on both side of the operator to the assertion handler, which
 would in turn print values and operator as part of the error message.
 
 So to me this module is a temporary fix until the compiler is capable
 of giving the necessary information to the assertion handler. I sure
 hope it won't be needed for too long.
 
 (Note: this criticism doesn't apply to those assertions dealing with
 exceptions.)

Well, I'm not about to claim that assert can't be fixed to give better error messages, but right now all it takes is a value which converts to bool for the test. a > b may obviously be convertible to something similar to assertPred!">"(a, b), but what about something like 1 + 1 < b or a < b < c. As expressions get progressively more complicated, it very quickly becomes non- obvious what someone would really want to print on error. Would 1 + 1 < b print 2 and b's value? Would it print 1, 1, and b's value? 1, 1, 2, and b's value? Sure, it may be obvious to the programmer what they intended, but it doesn't take much for it to be very difficult for the compiler to figure it out for you.

I think "assert(1+a < b)" should print the same thing as "static assert(1+a < b)" does. What "static assert(1+a < b)" prints when a == 1 and b == 0 is "(2 < 0) is false". Try it yourself.
 Also, assertPred!">"(a, b) would print out a more informative error
 message on its own. You wouldn't need to give it an additional message
 for it to be more informative. That would defeat the point. Even
 assertPred!"a > b"(a, b) could be
 more informative (assuming that it treats a > b as a general predicate
 rather than determining that it's actually >) by printing the values
 that it's given. So, that's definitely a leg up on assert(a > b) right
 there.

I don't believe it to be that difficult. From inside the compiler, you have access to the expression tree. All the compiler needs to do is check whether the top level expression is a binary op, and if so decompose it this way (assuming no given message here): auto a = operand1; auto b = operand2; if (a <binaryop> b) _d_assert_msg2("(%s <binaryop> %s) is false", __FILE__, __LINE__, &a, typeid(a), &b, typeid(b)); As for other expressions it could simply print the value by lowering it this way: auto result = <expression>; if (result) _d_assert_msg1("(%s) is false", __FILE__, __LINE__, &result, typeid(result)); That would basically give you the same error messages as static assert. Currently, assertions are lowered like this instead: if (expression) _d_assertm(moduleinfo, __LINE__); or like this when a message is provided: if (expression) _d_assertm(<message>, __FILE__, __LINE__); Sure, it's more complicated than doing it for static asserts where everything is known at compile-time, but I don't believe it to be that difficult.
 By passing each of the values to assertPred, we're able to print them out
 on failure without the computer having to understand what the predicate
 does, even when the values are arbitrary expressions. That would be very
 hard to do with an
 improved assert which just took the expression. I mean, try and write a
 function
 that took 1 + 1 > b or a < b < c as a string and tried to correctly print
 out values which are meaningful to the programmer. That would be
 _really_ hard. And while assertPred may not be able to understand a
 generic predicate, it can know about specific operators and/or functions
 and therefore give more informative error messages than it would be able
 to do with a generic predicate.

It's hard to do using a function. But it's easy for 'assert' because it's a language construct handled by the compiler.
 So, correctly implemented, I think that assertPred actually makes a lot
 more sense than trying to soup up assert and getting the compiler to
 guess at what the programmer really wants.

I don't really see what the compiler has to guess. The compiler just takes the top-level expression and pass its value to the assertion handler, and for binary expressions it can pass two values plus the operator's string. What cases are not covered by that?

If you write assertPred!"<"(foo(), 7) and it fails, it would print out the value of foo(). Something like, "5 is not less than 7". What should assert(foo() < 7) print? The value of the expression is false. We know that because the assertion failed, so there's no point in printing that. And if you want anything like "5 is not less than 7", what are you going to do? If you want it to print something like "assertion failed: 5 < 7", how does it know that you wanted to stop the evaluation of the expression at the point where foo() has been evaluated? Simply because there was only one evaluation left? That would deal with plenty of binary cases, but it wouldn't scale. What about something like a < b && c < d? If assertPred!() takes more than two parameters (as I would hope it would), then you could do something like assertPred!((a, b, c, d){return a < b && c < d;})(foo(), bar(), hello(), world()) and it could not only tell you that the predicate failed, but it could tell you that the values that it was given were 1, 5, 4, and 2. How could assert(foo() < bar() && hello() < world()) do that? It has to know where to stop the expressions evaluation to print it. Stopping with only one evaluation left (true && false) wouldn't be particularly useful, and it certainly wouldn't be what happened with assertPred!(). So, in many cases, the compiler either has to somehow guess where you want the evaluation to stop, or it's going to print sub- optimal information. assertPred!() would allow you to have control over what values get printed. The whole point of something like assertPred!() IMHO is to improve the output on error - in particular to print out the values being tested. I don't see how assert() could do that quite as nicely, even if it became as smart as you suggest. - Jonathan M Davis
Jan 05 2011
parent "Nick Sabalausky" <a a.a> writes:
"Jonathan M Davis" <jmdavisProg gmx.com> wrote in message 
news:mailman.446.1294293885.4748.digitalmars-d puremagic.com...
 What about something like a < b && c < d? If assertPred!() takes more
 than two parameters (as I would hope it would), then you could do 
 something like
 assertPred!((a, b, c, d){return a < b && c < d;})(foo(), bar(), hello(),
 world()) and it could not only tell you that the predicate failed, but it 
 could
 tell you that the values that it was given were 1, 5, 4, and 2. How could
 assert(foo() < bar() && hello() < world()) do that? It has to know where 
 to stop
 the expressions evaluation to print it.

How about this set of rules?: As the compler evaluates the assert's expression tree starting from the root: 1. Literals DO NOT have their values printed. 2. Built-in-operator expressions (arithmetic, parens, comparison, etc) DO NOT have their values printed, but the compiler traverses their immediate sub-expressions. 3. Variables DO have their values printed. 4. Things that take parameters (such as a function call or an eponymous template instantiation) DO have their values printed, but the compiler does *not* traverse the parameters (unless this is the root-level expression). I think that would cover any typical use-case, and I suspect anything that doesn't cover would probably be overly-complex anyway and should be re-written. Or, like I suggested in another branch, use some form of markup to denote what should be printed. Example: assertPred!`#foo()# < #bar()# && #hello()# < #world()#`(); assertPred!`{foo()} < {bar()} && {hello()} < {world()}`(); // Etc, whatever... That would probably need to return a string intended to be mixed in, which might be unappealing, but frankly we need a better way to instantiate string mixins anyway. I get real tired of library writers being made to feel pressure to avoid the power of string mixins just because they're (admittedly) ugly to use.
Jan 05 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Wednesday 05 January 2011 22:04:24 Jonathan M Davis wrote:
 On Wednesday 05 January 2011 21:09:07 Michel Fortin wrote:
 On 2011-01-05 22:57:00 -0500, Jonathan M Davis <jmdavisProg gmx.com> said:
 On Wednesday 05 January 2011 19:35:13 Michel Fortin wrote:
 I'm not sold on the concept. The whole point of this module seems to
 offer a way to replace the built-in assertion mechanism with a
 customized one, with the sole purpose of giving better error messages.
 
 So we're basically encouraging the use of:
 	assertPredicate!"a > b"(a, b, "message");
 
 instead of:
 	assert(a > b, "message");
 
 It looks like an uglification of the language to me.
 
 I agree that getting better error messages is important (very
 important in fact), but keeping the code clean is important too. If
 the built-in assert doesn't give us good enough error messages,
 perhaps it's the built-in assert that should be improved. The
 compiler could give the values on both side of the operator to the
 assertion handler, which would in turn print values and operator as
 part of the error message.
 
 So to me this module is a temporary fix until the compiler is capable
 of giving the necessary information to the assertion handler. I sure
 hope it won't be needed for too long.
 
 (Note: this criticism doesn't apply to those assertions dealing with
 exceptions.)

Well, I'm not about to claim that assert can't be fixed to give better error messages, but right now all it takes is a value which converts to bool for the test. a > b may obviously be convertible to something similar to assertPred!">"(a, b), but what about something like 1 + 1 < b or a < b < c. As expressions get progressively more complicated, it very quickly becomes non- obvious what someone would really want to print on error. Would 1 + 1 < b print 2 and b's value? Would it print 1, 1, and b's value? 1, 1, 2, and b's value? Sure, it may be obvious to the programmer what they intended, but it doesn't take much for it to be very difficult for the compiler to figure it out for you.

I think "assert(1+a < b)" should print the same thing as "static assert(1+a < b)" does. What "static assert(1+a < b)" prints when a == 1 and b == 0 is "(2 < 0) is false". Try it yourself.
 Also, assertPred!">"(a, b) would print out a more informative error
 message on its own. You wouldn't need to give it an additional message
 for it to be more informative. That would defeat the point. Even
 assertPred!"a > b"(a, b) could be
 more informative (assuming that it treats a > b as a general predicate
 rather than determining that it's actually >) by printing the values
 that it's given. So, that's definitely a leg up on assert(a > b) right
 there.

I don't believe it to be that difficult. From inside the compiler, you have access to the expression tree. All the compiler needs to do is check whether the top level expression is a binary op, and if so decompose it this way (assuming no given message here): auto a = operand1; auto b = operand2; if (a <binaryop> b) _d_assert_msg2("(%s <binaryop> %s) is false", __FILE__, __LINE__, &a, typeid(a), &b, typeid(b)); As for other expressions it could simply print the value by lowering it this way: auto result = <expression>; if (result) _d_assert_msg1("(%s) is false", __FILE__, __LINE__, &result, typeid(result)); That would basically give you the same error messages as static assert. Currently, assertions are lowered like this instead: if (expression) _d_assertm(moduleinfo, __LINE__); or like this when a message is provided: if (expression) _d_assertm(<message>, __FILE__, __LINE__); Sure, it's more complicated than doing it for static asserts where everything is known at compile-time, but I don't believe it to be that difficult.
 By passing each of the values to assertPred, we're able to print them
 out on failure without the computer having to understand what the
 predicate does, even when the values are arbitrary expressions. That
 would be very hard to do with an
 improved assert which just took the expression. I mean, try and write a
 function
 that took 1 + 1 > b or a < b < c as a string and tried to correctly
 print out values which are meaningful to the programmer. That would be
 _really_ hard. And while assertPred may not be able to understand a
 generic predicate, it can know about specific operators and/or
 functions and therefore give more informative error messages than it
 would be able to do with a generic predicate.

It's hard to do using a function. But it's easy for 'assert' because it's a language construct handled by the compiler.
 So, correctly implemented, I think that assertPred actually makes a lot
 more sense than trying to soup up assert and getting the compiler to
 guess at what the programmer really wants.

I don't really see what the compiler has to guess. The compiler just takes the top-level expression and pass its value to the assertion handler, and for binary expressions it can pass two values plus the operator's string. What cases are not covered by that?

If you write assertPred!"<"(foo(), 7) and it fails, it would print out the value of foo(). Something like, "5 is not less than 7". What should assert(foo() < 7) print? The value of the expression is false. We know that because the assertion failed, so there's no point in printing that. And if you want anything like "5 is not less than 7", what are you going to do? If you want it to print something like "assertion failed: 5 < 7", how does it know that you wanted to stop the evaluation of the expression at the point where foo() has been evaluated? Simply because there was only one evaluation left? That would deal with plenty of binary cases, but it wouldn't scale. What about something like a < b && c < d? If assertPred!() takes more than two parameters (as I would hope it would), then you could do something like assertPred!((a, b, c, d){return a < b && c < d;})(foo(), bar(), hello(), world()) and it could not only tell you that the predicate failed, but it could tell you that the values that it was given were 1, 5, 4, and 2. How could assert(foo() < bar() && hello() < world()) do that? It has to know where to stop the expressions evaluation to print it. Stopping with only one evaluation left (true && false) wouldn't be particularly useful, and it certainly wouldn't be what happened with assertPred!(). So, in many cases, the compiler either has to somehow guess where you want the evaluation to stop, or it's going to print sub- optimal information. assertPred!() would allow you to have control over what values get printed. The whole point of something like assertPred!() IMHO is to improve the output on error - in particular to print out the values being tested. I don't see how assert() could do that quite as nicely, even if it became as smart as you suggest.

Okay. I thought this through a bit more, and I think that if the evaluation was stopped when all that was left in the expression was boolean operators and their operands, then that pretty much has to be what the programmer was trying to print. That being the case, you could theoretically get assert to do it, but I would expect that that would make assert awfully complicated. Done properly, it would be fantastic, but since it can be done in a library with something like assertPred!() much more easily, I wouldn't expect such an enhancement to assert to be implemented any time soon. A few of the things that I'm thinking of doing with assertPred!() have to be special cased though in a way which wouldn't work with assert. For instance, I'm thinking of doing something like assertPred!("opCmp", 0)(foo(), bar()), which would do what assertOpCmp!() does now. As such, opCmp() isn't a predicate, if you rewrote it to assert(foo().opCmp(bar()) == 0), even stopping evaluation when all you have left is boolean operators and their operands wouldn't work, since then you'd get something like 1 == 0 rather than (assuming that foo() and bar() return something like BigInts which have the values "4" and "2" respectively) something like "opCmp() == 0 failed: 4 > 2". Sure, you could make it so that assert could do that sort of thing too, but then you're adding special cases and complicating assert even further, and it's also much harder to add such cases, because then you're actually changing the _language_ rather than the standard library, and every compiler would have to follow suit. So, yes assert could theoretically be improved to do a lot more and do a lot better, but it won't ever do as much as I could do with assertPred!(), and even if assert _is_ improved as you suggest (which would certainly be cool), then there's still some value in assertPred!(). - Jonathan M Davis
Jan 05 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Wednesday 05 January 2011 23:08:21 Nick Sabalausky wrote:
 "Jonathan M Davis" <jmdavisProg gmx.com> wrote in message
 news:mailman.446.1294293885.4748.digitalmars-d puremagic.com...
 
 What about something like a < b && c < d? If assertPred!() takes more
 than two parameters (as I would hope it would), then you could do
 something like
 assertPred!((a, b, c, d){return a < b && c < d;})(foo(), bar(), hello(),
 world()) and it could not only tell you that the predicate failed, but it
 could
 tell you that the values that it was given were 1, 5, 4, and 2. How could
 assert(foo() < bar() && hello() < world()) do that? It has to know where
 to stop
 the expressions evaluation to print it.

How about this set of rules?: As the compler evaluates the assert's expression tree starting from the root: 1. Literals DO NOT have their values printed. 2. Built-in-operator expressions (arithmetic, parens, comparison, etc) DO NOT have their values printed, but the compiler traverses their immediate sub-expressions. 3. Variables DO have their values printed. 4. Things that take parameters (such as a function call or an eponymous template instantiation) DO have their values printed, but the compiler does *not* traverse the parameters (unless this is the root-level expression). I think that would cover any typical use-case, and I suspect anything that doesn't cover would probably be overly-complex anyway and should be re-written. Or, like I suggested in another branch, use some form of markup to denote what should be printed. Example: assertPred!`#foo()# < #bar()# && #hello()# < #world()#`(); assertPred!`{foo()} < {bar()} && {hello()} < {world()}`(); // Etc, whatever... That would probably need to return a string intended to be mixed in, which might be unappealing, but frankly we need a better way to instantiate string mixins anyway. I get real tired of library writers being made to feel pressure to avoid the power of string mixins just because they're (admittedly) ugly to use.

Well, while it might be nice if the mixin syntax was less verbose, I think that the markup suggestion is pretty ugly even if it mixed itself in exactly as you wrote it. I find the idea of passing the individual values in separating to be far more palatable. That does seem to be a matter of taste however. - Jonathan M Davis
Jan 05 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Wednesday 05 January 2011 22:36:32 Jonathan M Davis wrote:
 On Wednesday 05 January 2011 22:04:24 Jonathan M Davis wrote:
 On Wednesday 05 January 2011 21:09:07 Michel Fortin wrote:
 On 2011-01-05 22:57:00 -0500, Jonathan M Davis <jmdavisProg gmx.com> said:
 On Wednesday 05 January 2011 19:35:13 Michel Fortin wrote:
 I'm not sold on the concept. The whole point of this module seems to
 offer a way to replace the built-in assertion mechanism with a
 customized one, with the sole purpose of giving better error
 messages.
 
 So we're basically encouraging the use of:
 	assertPredicate!"a > b"(a, b, "message");
 
 instead of:
 	assert(a > b, "message");
 
 It looks like an uglification of the language to me.
 
 I agree that getting better error messages is important (very
 important in fact), but keeping the code clean is important too. If
 the built-in assert doesn't give us good enough error messages,
 perhaps it's the built-in assert that should be improved. The
 compiler could give the values on both side of the operator to the
 assertion handler, which would in turn print values and operator as
 part of the error message.
 
 So to me this module is a temporary fix until the compiler is
 capable of giving the necessary information to the assertion
 handler. I sure hope it won't be needed for too long.
 
 (Note: this criticism doesn't apply to those assertions dealing with
 exceptions.)

Well, I'm not about to claim that assert can't be fixed to give better error messages, but right now all it takes is a value which converts to bool for the test. a > b may obviously be convertible to something similar to assertPred!">"(a, b), but what about something like 1 + 1 < b or a < b < c. As expressions get progressively more complicated, it very quickly becomes non- obvious what someone would really want to print on error. Would 1 + 1 < b print 2 and b's value? Would it print 1, 1, and b's value? 1, 1, 2, and b's value? Sure, it may be obvious to the programmer what they intended, but it doesn't take much for it to be very difficult for the compiler to figure it out for you.

I think "assert(1+a < b)" should print the same thing as "static assert(1+a < b)" does. What "static assert(1+a < b)" prints when a == 1 and b == 0 is "(2 < 0) is false". Try it yourself.
 Also, assertPred!">"(a, b) would print out a more informative error
 message on its own. You wouldn't need to give it an additional
 message for it to be more informative. That would defeat the point.
 Even assertPred!"a > b"(a, b) could be
 more informative (assuming that it treats a > b as a general
 predicate rather than determining that it's actually >) by printing
 the values that it's given. So, that's definitely a leg up on
 assert(a > b) right there.

I don't believe it to be that difficult. From inside the compiler, you have access to the expression tree. All the compiler needs to do is check whether the top level expression is a binary op, and if so decompose it this way (assuming no given message here): auto a = operand1; auto b = operand2; if (a <binaryop> b) _d_assert_msg2("(%s <binaryop> %s) is false", __FILE__, __LINE__,



 
 typeid(a), &b, typeid(b));
 
 As for other expressions it could simply print the value by lowering it
 
 this way:
 	auto result = <expression>;
 	if (result)
 	
 		_d_assert_msg1("(%s) is false", __FILE__, __LINE__, &result,
 
 typeid(result));
 
 That would basically give you the same error messages as static assert.
 
 Currently, assertions are lowered like this instead:
 	if (expression)
 	
 		_d_assertm(moduleinfo, __LINE__);
 
 or like this when a message is provided:
 	if (expression)
 	
 		_d_assertm(<message>, __FILE__, __LINE__);
 
 Sure, it's more complicated than doing it for static asserts where
 everything is known at compile-time, but I don't believe it to be that
 difficult.
 
 By passing each of the values to assertPred, we're able to print them
 out on failure without the computer having to understand what the
 predicate does, even when the values are arbitrary expressions. That
 would be very hard to do with an
 improved assert which just took the expression. I mean, try and write
 a function
 that took 1 + 1 > b or a < b < c as a string and tried to correctly
 print out values which are meaningful to the programmer. That would
 be _really_ hard. And while assertPred may not be able to understand
 a generic predicate, it can know about specific operators and/or
 functions and therefore give more informative error messages than it
 would be able to do with a generic predicate.

It's hard to do using a function. But it's easy for 'assert' because it's a language construct handled by the compiler.
 So, correctly implemented, I think that assertPred actually makes a
 lot more sense than trying to soup up assert and getting the
 compiler to guess at what the programmer really wants.

I don't really see what the compiler has to guess. The compiler just takes the top-level expression and pass its value to the assertion handler, and for binary expressions it can pass two values plus the operator's string. What cases are not covered by that?

If you write assertPred!"<"(foo(), 7) and it fails, it would print out the value of foo(). Something like, "5 is not less than 7". What should assert(foo() < 7) print? The value of the expression is false. We know that because the assertion failed, so there's no point in printing that. And if you want anything like "5 is not less than 7", what are you going to do? If you want it to print something like "assertion failed: 5 < 7", how does it know that you wanted to stop the evaluation of the expression at the point where foo() has been evaluated? Simply because there was only one evaluation left? That would deal with plenty of binary cases, but it wouldn't scale. What about something like a < b && c < d? If assertPred!() takes more than two parameters (as I would hope it would), then you could do something like assertPred!((a, b, c, d){return a < b && c < d;})(foo(), bar(), hello(), world()) and it could not only tell you that the predicate failed, but it could tell you that the values that it was given were 1, 5, 4, and 2. How could assert(foo() < bar() && hello() < world()) do that? It has to know where to stop the expressions evaluation to print it. Stopping with only one evaluation left (true && false) wouldn't be particularly useful, and it certainly wouldn't be what happened with assertPred!(). So, in many cases, the compiler either has to somehow guess where you want the evaluation to stop, or it's going to print sub- optimal information. assertPred!() would allow you to have control over what values get printed. The whole point of something like assertPred!() IMHO is to improve the output on error - in particular to print out the values being tested. I don't see how assert() could do that quite as nicely, even if it became as smart as you suggest.

Okay. I thought this through a bit more, and I think that if the evaluation was stopped when all that was left in the expression was boolean operators and their operands, then that pretty much has to be what the programmer was trying to print. That being the case, you could theoretically get assert to do it, but I would expect that that would make assert awfully complicated. Done properly, it would be fantastic, but since it can be done in a library with something like assertPred!() much more easily, I wouldn't expect such an enhancement to assert to be implemented any time soon.

Actually, for this to work, you'd also have to consider a function which returns bool as a boolean operator even if it isn't an operator, otherwise cases such as testing a function which returned a bool would end up just printing out the result, which wouldn't be particularly useful. - Jonathan m Davis
Jan 05 2011
prev sibling next sibling parent reply Jonathan M Davis <jmdavisProg gmx.com> writes:
On Wednesday 05 January 2011 23:08:21 Nick Sabalausky wrote:
 "Jonathan M Davis" <jmdavisProg gmx.com> wrote in message
 news:mailman.446.1294293885.4748.digitalmars-d puremagic.com...
 
 What about something like a < b && c < d? If assertPred!() takes more
 than two parameters (as I would hope it would), then you could do
 something like
 assertPred!((a, b, c, d){return a < b && c < d;})(foo(), bar(), hello(),
 world()) and it could not only tell you that the predicate failed, but it
 could
 tell you that the values that it was given were 1, 5, 4, and 2. How could
 assert(foo() < bar() && hello() < world()) do that? It has to know where
 to stop
 the expressions evaluation to print it.

How about this set of rules?: As the compler evaluates the assert's expression tree starting from the root: 1. Literals DO NOT have their values printed. 2. Built-in-operator expressions (arithmetic, parens, comparison, etc) DO NOT have their values printed, but the compiler traverses their immediate sub-expressions. 3. Variables DO have their values printed. 4. Things that take parameters (such as a function call or an eponymous template instantiation) DO have their values printed, but the compiler does *not* traverse the parameters (unless this is the root-level expression).

I think that that you'd have to give some examples for it to be clear what exactly you mean, but it sounds like this would print a lot more values than would be desirable in complex expressions. With assertPred, I could do something like assertPred!"=="(foo(bar(g, none("abc")) * 2, hello(world(i)) + 2 * func(funky(2 + q)), 25) and get what the actual result was. e.g. "== failed: 14 != 25". I wouldn't want all of those arguments printed. It's the final result that matters. If I understand what you're suggesting, a lot of those arguments would be printed in your suggestion, which would be less than desirable, I think. Granted, that particular example is quite ugly, but I'd like to be able to test arbitrarily complex functions and still get a reasonably informative error when the test fails. - Jonathan M Davis
Jan 05 2011
parent "Nick Sabalausky" <a a.a> writes:
"Jonathan M Davis" <jmdavisProg gmx.com> wrote in message 
news:mailman.450.1294300236.4748.digitalmars-d puremagic.com...
 On Wednesday 05 January 2011 23:08:21 Nick Sabalausky wrote:
 "Jonathan M Davis" <jmdavisProg gmx.com> wrote in message
 news:mailman.446.1294293885.4748.digitalmars-d puremagic.com...

 What about something like a < b && c < d? If assertPred!() takes more
 than two parameters (as I would hope it would), then you could do
 something like
 assertPred!((a, b, c, d){return a < b && c < d;})(foo(), bar(), 
 hello(),
 world()) and it could not only tell you that the predicate failed, but 
 it
 could
 tell you that the values that it was given were 1, 5, 4, and 2. How 
 could
 assert(foo() < bar() && hello() < world()) do that? It has to know 
 where
 to stop
 the expressions evaluation to print it.

How about this set of rules?: As the compler evaluates the assert's expression tree starting from the root: 1. Literals DO NOT have their values printed. 2. Built-in-operator expressions (arithmetic, parens, comparison, etc) DO NOT have their values printed, but the compiler traverses their immediate sub-expressions. 3. Variables DO have their values printed. 4. Things that take parameters (such as a function call or an eponymous template instantiation) DO have their values printed, but the compiler does *not* traverse the parameters (unless this is the root-level expression).

I think that that you'd have to give some examples for it to be clear what exactly you mean, but it sounds like this would print a lot more values than would be desirable in complex expressions. With assertPred, I could do something like assertPred!"=="(foo(bar(g, none("abc")) * 2, hello(world(i)) + 2 * func(funky(2 + q)), 25) and get what the actual result was. e.g. "== failed: 14 != 25". I wouldn't want all of those arguments printed. It's the final result that matters. If I understand what you're suggesting, a lot of those arguments would be printed in your suggestion, which would be less than desirable, I think. Granted, that particular example is quite ugly, but I'd like to be able to test arbitrarily complex functions and still get a reasonably informative error when the test fails.

Your example is missing a closing paren, I assume you meant: assertPred!"=="(foo(bar(g, none("abc")) * 2, hello(world(i)) + 2 * func(funky(2 + q))), 25) Ie, assertPred is given two run-time arguments, the first of which is a call to foo. And foo is given two arguments: "bar(...)*2" and "hello(...)+2*func(...)". Under my suggestion, that example would be: assert(foo(bar(g, none("abc")) * 2, hello(world(i)) + 2 * func(funky(2 + q))) == 25) If that fails, it would first print something like: file(line): Assert failed: foo(bar(g, none("abc")) * 2, hello(world(i)) + 2 * func(funky(2 + q))) == 25 Then it would inspect the expression: The root-level expression is "stuff == 25". The right-hand-side, 25, is a literal, so that's ignored (rule #1). The left-hand side is the expression "foo(param1, param2)", so it's value (14 in your example) gets displayed (rule #4): [foo(bar(g, none("abc")) * 2, hello(world(i)) + 2 * func(funky(2 + q)))]: 14 Since foo is something that takes params (it's a function call) and it's not the root expression (the == is the root expression), foo's params are ignored (rule #4) and everything is now done. Alternatively, the rules could be followed like this: Start with the whole expression, but do not print anything yet. Look at the root expression: It's "stuff == 25". The 25 is a literal, so leave it as-is (essentially rule #1). The other side is a function call, so replace the expression "foo(...,...)" with it's value, 14 (essentially rule #4). Now there's no more subexpressions left to process, so display the newly-modified expression: 14 == 25 Under the other example you had: assert(foo() < bar() && hello() < world()) The expression tree looks like this: - Root: && - LHS of &&: < - LHS of <: foo() - RHS of <: bar() - RHS of &&: < - LHS of <: hello() - RHS of <: world() The root is &&, so we dive into it's operands (rule #2). Both of those are <, so we dive into their respective operands (rule #2). Now we have four things that take params (I'm conveniently ignoring the fact that the number of params they actually take just happens to be 0), so we either display or substitute-in their values and dive no further (rule #4), and get either: Failed: foo() < bar() && hello() < world() foo(): 4 bar(): 3 hello(): 2 world(): 1 Or just: 4 < 3 && 2 < 1 Note that due to the way rule #2 is written, if you replaced "foo()" with "f() + oo()", then you would get something like: 2 + 2 < 3 && 2 < 1 Which I don't think is too bad, particularly considering the expression it came from. And is is more information, which could be helpful - you can always add them together, but if it just gives you "4" you can't un-add it. But I also like your idea of "if the evaluation was stopped when all that was left in the expression was boolean operators and their operands". I can't tell right now since it's late and I'm half asleep, but I think my suggestion is essentially very similar. In fact, it might be exactly the same if you changed my rule #2 to just be comparison and boolean-logic operators and treat the other operators as functions (ie, move arithmetic and bitwise-logic operators from rule #2 to rule #4). In any case, the details of the rules can be tweaked, but the main point is I think it's doable. That said, I still see the value in assertPred both for the meantime and to avoid the catch-22 someone mentioned of (paraphrasing) "Do we define this in the language thereby complicating the language and preventing alternate compilers from improving on it, or do we leave it out of the langauge definition and not guarantee these benefits to users of alternate compilers?"
Jan 06 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Thursday 06 January 2011 05:30:56 Michel Fortin wrote:
 There's an other issue that's bothering me with these assertion
 functions... with 'assert', assertions behaves differently depending on
 where you write them. In regular code, on failure it calls _d_assert*,
 in a unit test on failure it calls _d_unittest*, and in contracts for
 virtual function... well that case I don't understand it fully but it
 does something very special to implement contract inheritance.
 
 What does assertPred do? It does "throw new AssertError(...)",
 irrespective of the context. I'm pretty sure that'll break contract
 inheritance if used inside a contract. To be truly a replacement for
 assert, assertPred would need to know in which context it is called and
 generate the appropriate code, although I somewhat doubt it is even
 possible to generate the right code for contracts without the compiler
 doing it.

I don't know anything about this. As far as I know, there's no difference between assert in unittest blocks and assert in contracts. That being said, you're probably more knowledgeable than I am about that. I would have thought was that all it would take would be for the AssertError to be handled appropriately by whatever catches it. In the case of contracts, I would think that it's an issue of the right contract code being called in the right order, and that which contract threw the test would be irrelevant (the stack track would show where it was; all that matters from the runtime's perspective is that there _was_ an AssertError thrown and that execution is therefore going to be stopping). As for unittest blocks, I thought that it caught the AssertError from the unittest block and dealt with it. If it does that, then I don't see why it would need a special version of assert. I know that it _used_ to be different, because Walter temporarily made it so that within unittest blocks assert set a flag saying that the test failed and printed the error rather than throwing an AssertError, but Andrei and others complained about that (both that assert would have different behavior in different places and that a unittest block would continue after a failure), and it was agreed that assert would throw an AssertError like it normally does. So, I don't know if assert does something different depending on where it is called. The only special case for assert that I'm aware of is assert(0), which becomes the halt instruction with -release rather than going away. If there is a difference, then we probably need to understand what it is and what issues it could cause. Ideally, there wouldn't be any difference. However, I _have_ been using these functions in unit tests, and they work fine there. So, as far as unit testing goes, they work. I have _not_ been using them in contracts. I created them specifically with unit testing in mind (and in fact, with the current code, the entire module is in a version(unittest) block). It sounds like there are some folks (including Andrei) who think that it should be useable in normal contracts just like assert is. That's simple to fix by removing the version(unittest) block, but I don't know if there are any issues with throwing an AssertError from a function called within a contract rather than asserting directly inside a contract. If there is, I would think that that's a more general problem. I've been doing that all the time with invariants (just not with any of my unit testing functions), and that's worked as far as I can tell, but I've been using structs primarily, which wouldn't have contract inheritance. So, I think that assert should _definitely_ work normally when called from a function called from a contract rather than when used directly in a contract, but I don't know that that never causes problems. We need someone who actually knows what assert does in each case to say whether there's an issue, I think. - Jonathan M Davis
Jan 06 2011
prev sibling next sibling parent Tomek =?ISO-8859-2?Q?Sowi=F1ski?= <just ask.me> writes:
Andrei Alexandrescu napisa=B3:

 * Remove assertCmp and fold it into assertPredicate. With trivial=20
 metaprogramming you can distinguish an operator from an actual expression.
=20
 assertPredicate!"<"(1 + 1, 2.1);

If it's trivial, put it into binaryFun, so that everyone else can reap rewa= rds: sort!">"(range); auto sum =3D reduce!"+"(range); As for operator-only unaryFuns, don't; they bring more confusion than benef= it. --=20 Tomek
Jan 06 2011
prev sibling parent Jens Mueller <jens.k.mueller gmx.de> writes:
Andrei Alexandrescu wrote:
 On 1/5/11 8:54 PM, Ary Borenszweig wrote:
I prefer assert, assertFalse, assertEqual and assertNotEqual.

Compare this:
assertPredicate!"a<  b"(1 + 1, 3);

To this:
assert(1 + 1<  3)

Or to this:

assertLess(1 + 1, 3)

Ok, the first one is more generic. But so the error message for the assertion
failure will be more generic, when you want exactly the opposite to happen.
Plus your brain has to think more to read it (starting from the point that it's
longer).

assertPredicate!"<"(1 + 1, 3) reminds me of polish notation, which is very good
for machines but not very good for humans.

One problem with using distinct names is the explosion thereof. Google unittests have EXPECT_EQ, EXPECT_NE, EXPECT_LT, EXPECT_GT, EXPECT_LE, EXPECT_GE, EXPECT_TRUE, EXPECT_FALSE and probably others. I was able to write these from memory because I used to know Fortran, which uses the same two-letter conventions for comparisons. At that point I think we can say, wait a minute, why not use the respective symbols which are impossible to forget?

I partly share this. But there may be users who are not that deeply into the template stuff. Even though they miss a lot. I'd like to have testing easy and assertEqual is easy. I mean Jonathan started with those names. When I look now at the documentation the name assertPred alone does not tell much. I feel a bit lost. Why not define an alias? I mean there are people who master assertPred. Why not giving others a slow start using assertEqual and friends. Jens
Jan 30 2011