Sean Kelly wrote:
> On May 3, 2010, at 4:11 PM, Andrei Alexandrescu wrote:
>>> 2. many asserts are not meant at all to be unit tests, and there's no reason to think otherwise when they are outside the unittest block
>> I guess but I have the feeling you are taking a couple of wrong turns that reflect lack of experience with unittests.
>>
>> 1. A failing unittest only prints and continues
>>
>> 2. Top-level asserts are different from other asserts
>>
>> I'm not claiming to be an expert in unittesting but from what I know from other unittest facilities, I believe the following is the correct behavior:
>>
>> 1. A failing unittest means that particular unittest is terminated immediately and counted as "failed" exactly once. A failed unittest has an identity, you can't say "this unittest has failed 3 times in the following places".
>>
>> 2. unittest-level assert and any-level assert are the same deal. It's basic (de)composability and what anyone expects since the invention of the stack.
>>
>> Please let's not introduce odd behavior unless we're based on something.
> 
> I'm unsure about the current behavior with asserts.  On the one hand I agree with you, on the other I see the unittest-level asserts as a way of reporting a case failure, so it may be reasonable to just continue.  However, perhaps "assert" isn't the best candidate for both operations.

That makes the definition of assert() dependent of lexical context. That's not impossible and not unjustifiable - it's just weird without good reason.

assert() is assert() is assert(). Why complicate things without reason?

Unittest frameworks have invented a wonderful mechanism for defining assertions that behave differently from one another: assigning different names for them.

assert(expr); // terminates current unittest
expect(expr); // outputs to stderr if failed, continue unittest

Walter, please: go with the established ways. Don't invent stuff just because you can.


Andrei

----- Original Message ----
> From: Walter Bright <walter at digitalmars.com>
>
> I'm reluctant to transform all asserts because:
> 
> 1. this means all asserts, whether meant for unit testing or not, have to have return code, negatively affecting optimization all the time
> 
> 2. many asserts are not meant at all to be unit tests, and there's no reason to think otherwise when they are outside the unittest block

We're forgetting 2 things here:

1. unit tests run independently of any user input, they are completely defined by the unit test code itself.

2. Nobody, and I mean nobody, should ever compile unittest code for production.  In fact, I'd say you probably have not understood the meaning of unit tests if your main function looks anything unlike:

void main() {}

Therefore, you shouldn't be concerned with the optimization quality of unittest code during asserts.

What if we solve this with a completely global solution -- when compiling with unit tests enabled, all asserts do a unittest style assert (decided at link), when compiling without unittests, asserts function as they do now.  Can this be done?

-Steve

On Mon, 3 May 2010, Steve Schveighoffer wrote:

> ----- Original Message ----
> > From: Walter Bright <walter at digitalmars.com>
> >
> > I'm reluctant to transform all asserts because:
> > 
> > 1. this means all asserts, whether meant for unit testing or not, have to have return code, negatively affecting optimization all the time
> > 
> > 2. many asserts are not meant at all to be unit tests, and there's no reason to think otherwise when they are outside the unittest block
> 
> We're forgetting 2 things here:
> 
> 1. unit tests run independently of any user input, they are completely defined by the unit test code itself.
> 
> 2. Nobody, and I mean nobody, should ever compile unittest code for production.  In fact, I'd say you probably have not understood the meaning of unit tests if your main function looks anything unlike:
> 
> void main() {}
> 
> Therefore, you shouldn't be concerned with the optimization quality of unittest code during asserts.
> 
> What if we solve this with a completely global solution -- when compiling with unit tests enabled, all asserts do a unittest style assert (decided at link), when compiling without unittests, asserts function as they do now.  Can this be done?
> 
> -Steve

Careful.. don't conflate building unittests into production code with building production code with asserts enabled or disabled.

I run LOTS of important code with asserts enabled in production.  I'm willing, in fact eager, to risk a production system exiting rather than it continuing to run past an assertable state.  I will say that there's a set of asserts that I do disable in production systems, those that are really expensive to execute -- most of mine are dead trivial.  Admitedly, I'm not talking about D here, but rather C++.  But the philosophy doesn't differ based on language choice.

I fully agree that with D that it's unlikely that I'd leave unit tests turned on in production builds -- but if the tests are fast enough to leave in, no real harm either.  Keep in mind, they don't affect anything beyond the cost of app startup, or shouldn't.

Later,
Brad

----- Original Message ----
> From: Brad Roberts <braddr at puremagic.com>
> I fully agree
> that with D that it's unlikely that I'd leave unit tests
> turned on in
> production builds -- but if the tests are fast enough to
> leave in, no real
> harm either.  Keep in mind, they don't affect anything
> beyond the cost
> of app startup, or
> shouldn't.

But we should not try to optimize unittests, they are not there for performance, they are there for diagnostics.  Make them as comprehensive and useful as possible, performance be damned.

My first point should explain why you should never leave them in -- they don't depend on anything but hard-coded values.  Therefore, they are almost entirely deterministic (you could argue that unit testing could potentially open files or something, but that is unlikely), and running them more than once is a complete waste of CPU cycles.

But I meant that when unittests were disabled, the assert handler linked would be the faster variety, and when unittests were enabled, the assert handler would be the comprehensive unittesty variety.  The object files would be identical, it's just what assert function do you link with.  I don't know if it's possible, which is why I asked.

If that's not possible, we still have another option.

An assert translates roughly to this:

if(!condition) {throwAnError();}

We all agree that when throwAnError is called, performance is out the window (nobody but the runtime should be catching asserts, and we should expect the program to exit soon after since it's in an invalid state).  Why can't the check for whether we're in unittest mode be inside that function/block as Andrei pointed out?  My larger point is simply that we should not consider performance problems to be important during unit testing, asserts can be slow, because unittests do not need to be as fast as possible.

-Steve

Steve Schveighoffer wrote:
>
> But I meant that when unittests were disabled, the assert handler linked would be the faster variety, and when unittests were enabled, the assert handler would be the comprehensive unittesty variety.  The object files would be identical, it's just what assert function do you link with.  I don't know if it's possible, which is why I asked.
>
> If that's not possible, we still have another option.
>
> An assert translates roughly to this:
>
> if(!condition) {throwAnError();}
>
> We all agree that when throwAnError is called, performance is out the window (nobody but the runtime should be catching asserts, and we should expect the program to exit soon after since it's in an invalid state).  Why can't the check for whether we're in unittest mode be inside that function/block as Andrei pointed out?  My larger point is simply that we should not consider performance problems to be important during unit testing, asserts can be slow, because unittests do not need to be as fast as possible.
>
> 

What I meant was that the compiler takes advantage of knowledge that assert() failures do not return to generate better code for the non-asserting case. I agree that the performance of the asserting case is not relevant.

On Tue, May 4, 2010 at 3:11 AM, Andrei Alexandrescu <andrei at erdani.com> wrote:
> Walter Bright wrote:
>>
>>
>> Andrei Alexandrescu wrote:
>>>
>>> Walter Bright wrote:
>>>>
>>>>
>>>> Andrei Alexandrescu wrote:
>>>>>
>>>>> Since we've been looking into this now and we have a solution in our collective mental caches, is there please a chance to effect this change for this beta? I'm telling you, it is an _important_ step forward in unittesting D programs.
>>>>>
>>>>
>>>> One thing I should mention is that dmd now calls a different function for unittest asserts than the asserts normally do, and it does not regard those calls as "terminating" calls. This means that the unittest behavior can now be controlled by adjusting the runtime library, without affecting the assert failure code.
>>>
>>> In light of Sean's latest discovery, I'm afraid there's an issue here.
>>> The asserts are only replaced syntactically at top-level unittests. My
>>> understanding was that there was a "unittest stage" global that dictates how
>>> assertion failures are handled, regardless of origin.
>>>
>>> I'm unclear on what the best policy is. Experience suggests that flat == bad.
>>>
>>
>> I'm reluctant to transform all asserts because:
>>
>> 1. this means all asserts, whether meant for unit testing or not, have to have return code, negatively affecting optimization all the time
>
> This sounds like latching onto an implementation artifact. The way I'm seeing things is:
>
> bool g_runningUnittests;
>
> // C entry point
> int _main() {
> ? ?g_runningUnittests = true;
> ? ?__runUnittests();
> ? ?g_runningUnittests = false;
> ? ?d_main();
> }
>
> The implementation of assertFailed() consults g_runningUnittests. And that's
> all there is to it.
>
>> 2. many asserts are not meant at all to be unit tests, and there's no reason to think otherwise when they are outside the unittest block
>
> I guess but I have the feeling you are taking a couple of wrong turns that reflect lack of experience with unittests.
>
> 1. A failing unittest only prints and continues
>
> 2. Top-level asserts are different from other asserts
>
> I'm not claiming to be an expert in unittesting but from what I know from other unittest facilities, I believe the following is the correct behavior:
>
> 1. A failing unittest means that particular unittest is terminated immediately and counted as "failed" exactly once. A failed unittest has an identity, you can't say "this unittest has failed 3 times in the following places".
>
> 2. unittest-level assert and any-level assert are the same deal. It's basic (de)composability and what anyone expects since the invention of the stack.
>
> Please let's not introduce odd behavior unless we're based on something.
>
>
> Andrei
> _______________________________________________
> dmd-internals mailing list
> dmd-internals at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/dmd-internals
>

What's wrong with automatically wrapping unittests with try/catch
block? Why is the "bool g_runningUnittests;" even necessary?
Anyway, I believe top-level unittest assert behavior shouldn't differ
from other asserts.

On May 5, 2010, at 4:19 AM, Denis <2korden at gmail.com> wrote:

> What's wrong with automatically wrapping unittests with try/catch block?

I believe that's what everyone except Walter wants :) if I understand correctly, a non-throwing assert is more efficient than a throwing assert. Nobody cares about performance when an assert fails, but this creates performance differences when asserts pass.



> Anyway, I believe top-level unittest assert behavior shouldn't differ from other asserts.

Me too. Anything with the same function name/signature should behave the same. Maybe the simplest solution is to have one function that throws and one that aborts?

I hate how, with the current design, someone can't write their own
helper functions that use assert. e.g.
unittest{
   foreach(x; 0..10)
     validate_foo(bar(x), x);
}

Denis wrote:
> On Tue, May 4, 2010 at 3:11 AM, Andrei Alexandrescu <andrei at erdani.com> wrote:
> What's wrong with automatically wrapping unittests with try/catch
> block? Why is the "bool g_runningUnittests;" even necessary?
> Anyway, I believe top-level unittest assert behavior shouldn't differ
> from other asserts.

That would be perfect.

Andrei

On May 5, 2010, at 8:42 AM, Jason House <jason.james.house at gmail.com> wrote:


>
> I hate how, with the current design, someone can't write their own
> helper functions that use assert. e.g.
> unittest{
>  foreach(x; 0..10)
>    validate_foo(bar(x), x);
> }
>

Another thing I dislike with the current design is that the following
would segfault:
   auto x = foo();
   assert( ! (x is null) );
   assert( x.bar == 7 );

On May 5, 2010, at 9:37 AM, Andrei Alexandrescu wrote:

> Denis wrote:
>> On Tue, May 4, 2010 at 3:11 AM, Andrei Alexandrescu <andrei at erdani.com> wrote:
>> What's wrong with automatically wrapping unittests with try/catch
>> block? Why is the "bool g_runningUnittests;" even necessary?
>> Anyway, I believe top-level unittest assert behavior shouldn't differ
>> from other asserts.
> 
> That would be perfect.

I'd happily change the onUnittestError handler to throw an AssertError instead of printing and continuing, but the better fix would be to rip this new logic out of the compiler as well, so the handler isn't even needed.  Since we don't have macros in D2, I guess we'd really need a language-level "expect" function to get the correct file and line info in the log message.