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 reply 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
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 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 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 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 reply 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
parent reply 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
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 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 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 reply 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
next sibling parent reply 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
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 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 parent reply 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
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 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=


ss.

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


 it is.=20

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


k=20

 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 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 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
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 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 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 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 reply "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
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 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 reply "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
// 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
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
 // 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 reply "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
 // 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


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
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
 

 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 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 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 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 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 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
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
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 reply 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
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 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 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
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
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 reply "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
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
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
 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 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 == 

left-hand side is the expression "foo(param1, param2)", so it's value (14 in 


   [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 


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 


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()



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 


Failed: foo() < bar() && hello() < world()
foo(): 4
bar(): 3
hello(): 2
world(): 1

Or just: 4 < 3 && 2 < 1


"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 

operators and treat the other operators as functions (ie, move arithmetic 


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 parent reply 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
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 reply 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
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 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__, 






&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.


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 parent reply Jim <bitcirkel yahoo.com> writes:
I have to agree.

assert(1 + 1 < 3);

will always be easier to read than any:

assertPred!"a < b"(1 + 1, 3);

Why not just keep it simple and straight forward? Anything expressed in the
latter form can be expressed in the former.
Jan 08 2011
next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Saturday 08 January 2011 15:21:44 Jim wrote:

 I have to agree.
 
 assert(1 + 1 < 3);
 
 will always be easier to read than any:
 
 assertPred!"a < b"(1 + 1, 3);
 
 Why not just keep it simple and straight forward? Anything expressed in the
 latter form can be expressed in the former.


Well, honestly, after seeing enough stuff like the second example, I have no 
trouble reading it pretty much as quickly as the first one - especially with
more 
complex expressions -  but I can see why some would consider that an issue.

However, my take on this is that the built-in assert cannot currently give us 
the kind of error messages that assertPred!() will. So, regardless of whether 
the built-in assert is improved later, if we want that kind of functionality 
now, then we need to have something like assertPred!().

Now, it would be extremely cool if the built in assert were able to give us 
error messages similar to what assertPred!() will do, so it could very well be 
valuable to add that later, but it's an implementation detail (similar to 
improving error messages), it's not necessarily going to be very easy to do,
and 
assertPred!() will have already given us the necessary functionality, so I 
wouldn't expect such a change to be particularly high priority. It would 
probably have to be done by someone other than Walter who had the time and 
interest in doing it, since Walter has plenty of more important things an his 
plate.

So, assert my eventually be that cool, but it isn't now, and it may never be,
so 
we create assertPred!() now, and then we have a solution. If assert eventually 
_is_ that cool, then it'll be an option to use that instead of assertPred!()
for 
getting better error messages. But for now, it isn't an option.

- Jonathan M Davis
Jan 08 2011
prev sibling parent =?UTF-8?B?IkrDqXLDtG1lIE0uIEJlcmdlciI=?= <jeberger free.fr> writes:
Jim wrote:

 I have to agree.
=20
 assert(1 + 1 < 3);
=20
 will always be easier to read than any:
=20
 assertPred!"a < b"(1 + 1, 3);
=20
 Why not just keep it simple and straight forward? Anything expressed in=


 the latter form can be expressed in the former.

	True, but on the other hand:

Assertion failed!

will always be less informative than any:

Assertion failed: 3 < 2!

	It all depends on whether your priority is code or message
readability. I would tend to give priority to the error message.

		Jerome
--=20
mailto:jeberger free.fr
http://jeberger.free.fr
Jabber: jeberger jabber.fr
Jan 09 2011