May 03, 2010
On May 2, 2010, at 1:23 PM, 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.

I think the current behavior is a good first step, but should probably be modified a bit further.  For example:

    void fnA()
    {
        assert( false, "fnA 1" );
        assert( false, "fnA 2 ");
    }

    unittest
    {
        assert( false, "unittest 1" );
        assert( false, "unittest 2" );
        fnA();
    }

    unittest
    {
        assert( false, "unittest 3" );
    }

    void main()
    {

    }

Running this will print:

    test.d(9): unittest 1
    test.d(10): unittest 2
    core.exception.AssertError at test.d(3): fnA 1

So if an assert error occurs inside some code being tested, the app will still exit.  I tried modifying the unit testing code to eat the exception and continue, but then it simply prints:

    test.d(9): unittest 1
    test.d(10): unittest 2

So the best we can currently do is move on to test another module--there's no way to execute the remaining unit tests within the module.  Could we perhaps have an array of unittests within the ModuleInfo struct instead of just a single function pointer?  Or is the rationale really that if implementation code throws, the app should assume it's in an invalid state and exit?
May 03, 2010
On May 2, 2010, at 1:23 PM, 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.

For now, I've modified the code to make this behavior not overridable.  I'll probably change it back though--this occurred as a side-effect of a bunch of other changes I'm working on.
May 03, 2010
Preliminary support added (in the form of a stack trace) in Druntime revision 290.  I'll see about making it prettier before the next release.

On May 1, 2010, at 10:35 AM, Sean Kelly wrote:

> Sure.
> 
> On May 1, 2010, at 5:55 AM, 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.
>> 
>> Thanks!
>> 
>> Andrei
>> 
>> On 05/01/2010 01:19 AM, Sean Kelly wrote:
>>> On Apr 30, 2010, at 1:40 PM, Steve Schveighoffer wrote:
>>>> 
>>>> ----- Original Message ----
>>>>> From: Sean Kelly<sean at invisibleduck.org>
>>>>> 
>>>>> On Apr 28, 2010, at 1:25 PM, Andrei Alexandrescu wrote:
>>>>> It's not hard to write a segfault handler that
>>>>> does this, but it involves doing technically illegal stuff in a signal handler
>>>>> (either IO or throwing an exception) so I don't want to make it a built-in
>>>>> feature.
>>>> 
>>>> syscalls are always legal because syscalls exit when signals occur (technically, they aren't even calls, they are software interrupts).  In other words, printf is illegal, write is not.
>>> 
>>> I think it's a bit more restrictive than that, but you're right.  If it helps, the list of signal-safe functions I use for reference is here:
>>> 
>>> http://www.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_04.html
>>> _______________________________________________
>>> dmd-internals mailing list
>>> dmd-internals at puremagic.com
>>> http://lists.puremagic.com/mailman/listinfo/dmd-internals
> 
> _______________________________________________
> dmd-internals mailing list
> dmd-internals at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/dmd-internals

May 03, 2010

Sean Kelly wrote:
>
> I think the current behavior is a good first step, but should probably be modified a bit further.  For example:
>
>     void fnA()
>     {
>         assert( false, "fnA 1" );
>         assert( false, "fnA 2 ");
>     }
>
>     unittest
>     {
>         assert( false, "unittest 1" );
>         assert( false, "unittest 2" );
>         fnA();
>     }
>
>     unittest
>     {
>         assert( false, "unittest 3" );
>     }
>
>     void main()
>     {
> 
>     }
>
> Running this will print:
>
>     test.d(9): unittest 1
>     test.d(10): unittest 2
>     core.exception.AssertError at test.d(3): fnA 1
>
> So if an assert error occurs inside some code being tested, the app will still exit.

No, if an assert occurs lexically outside of a unittest block, the regular assert failure code will be called.

>   I tried modifying the unit testing code to eat the exception and continue, but then it simply prints:
>
>     test.d(9): unittest 1
>     test.d(10): unittest 2
>
> So the best we can currently do is move on to test another module--there's no way to execute the remaining unit tests within the module.  Could we perhaps have an array of unittests within the ModuleInfo struct instead of just a single function pointer?  Or is the rationale really that if implementation code throws, the app should assume it's in an invalid state and exit?
> 

It's simpler than that. The idea is if an assert fails within a unittest block, the unittests should continue. If an assert fails outside, then the app should stop.

May 03, 2010
On May 3, 2010, at 1:45 PM, Walter Bright wrote:
> 
> Sean Kelly wrote:
> 
>>  I tried modifying the unit testing code to eat the exception and continue, but then it simply prints:
>> 
>>    test.d(9): unittest 1
>>    test.d(10): unittest 2
>> 
>> So the best we can currently do is move on to test another module--there's no way to execute the remaining unit tests within the module.  Could we perhaps have an array of unittests within the ModuleInfo struct instead of just a single function pointer?  Or is the rationale really that if implementation code throws, the app should assume it's in an invalid state and exit?
>> 
> 
> It's simpler than that. The idea is if an assert fails within a unittest block, the unittests should continue. If an assert fails outside, then the app should stop.

So let's say I'm doing boundary testing for code that has 'in' contracts.  I guess if I want to test that it rejects out of range values then I should explicitly catch the AssertError?
May 03, 2010

Sean Kelly wrote:
> On May 3, 2010, at 1:45 PM, Walter Bright wrote:
> 
>> Sean Kelly wrote:
>>
>> 
>>>  I tried modifying the unit testing code to eat the exception and continue, but then it simply prints:
>>>
>>>    test.d(9): unittest 1
>>>    test.d(10): unittest 2
>>>
>>> So the best we can currently do is move on to test another module--there's no way to execute the remaining unit tests within the module.  Could we perhaps have an array of unittests within the ModuleInfo struct instead of just a single function pointer?  Or is the rationale really that if implementation code throws, the app should assume it's in an invalid state and exit?
>>> 
>>> 
>> It's simpler than that. The idea is if an assert fails within a unittest block, the unittests should continue. If an assert fails outside, then the app should stop.
>> 
>
> So let's say I'm doing boundary testing for code that has 'in' contracts.  I guess if I want to test that it rejects out of range values then I should explicitly catch the AssertError?
>
> 

I'd say yes, though note that the d_assertm() function cannot return, as the code gen for it assumes it never does.
May 03, 2010
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.


Andrei
May 03, 2010

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

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

May 03, 2010
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
May 03, 2010
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.

What I would like in terms of continuing after a unittest failure is:

    void op() { assert( false ); }

    unittest // test 1
    {
        op(); // terminates the current test
    }

    unittest // test 2
    {
        // this test is run even though the one above fails
    }

Then I think we'd see a bunch of small unittest blocks per module, each testing an individual thing.  If there are multiple tests in one block then the user is declaring that if any of the tests in the block fail, he doesn't want the rest to be executed.  So you might do:

    unittest
    {
        loadThingToTest();
        runTestIfLoadedA();
        runTestIfLoadedB();
        ...
    }