On 15 July 2010 06:04, Benjamin Shropshire
<benjamin at precisionsoftware.us> wrote:
> Le 2010-07-14 ? 22:42, Andrei Alexandrescu a ?crit :
>>>
>>> I agree. But also simple does not mean crappy. I think there's too much talk around a simple matter:
>>>
>>> ======
>>> Assertion failure should abort the current unittest block.
>>> ======
>>>
>
> Yes.
Yes.
>
>>> 4. DESIRED: assert is NOT hacked, any failing assert ends the current unittest, the failure message is printed, execution continues with the next unittest, program ends with error code if at least one assert failed, everybody's happy.
>>>
>>>
>
> I want that.
Yes.

On Wed, 2010-07-14 at 21:42 -0500, Andrei Alexandrescu wrote:
> On 07/14/2010 08:58 PM, Walter Bright wrote:
> >
> >
> > Sean Kelly wrote:
> >>
> >>
> >> How about this... since unit tests are run after static ctors are run, the behavior of whether to throw an AssertError or report and continue can be a run-time configurable option.
> >
> > Frankly, I don't think a configurable option is a good idea. Druntime has a lot of user configurability, and as far as I can tell, absolutely none of it has ever been used. It just adds complexity, both to the code and the documentation.
> 
> I agree. But also simple does not mean crappy. I think there's too much talk around a simple matter:
> 
> ======
> Assertion failure should abort the current unittest block.
> ======
> 
> Do we all agree about the above?

Yes.


> We keep on discussing how the usefulness and informativeness of assertion failures falls off a cliff after the first failure, and I can't seem to hear loud and clear that we all want this same thing.
> 
> So we've been through these steps:
> 
> 1. Crappy: failure to assert causes the program to abort.
> 
> 2. Awful: assert is hacked inside unittests to essentially be writeln, all unittests run regardless. Programs claims success upon exit.
> 
> 3. Mediocre: assert continues to be hacked inside unittests to essentially be writeln + set a flag. Programs exit with errors if the flag was set.
> 
> NOT YET:
> 
> 4. DESIRED: assert is NOT hacked, any failing assert ends the current unittest, the failure message is printed, execution continues with the next unittest, program ends with error code if at least one assert failed, everybody's happy.
> 
> I'm glad we made progress from 2 to 3. Could you all please share your opinion about the delta between 3 and 4?

I agree that 4 is the best solution.  It is very bad that assert has different semantics depending on where it's used.  Also, it makes a lot of sense to consider a unittest{} block to be a single unit test.

-Lars

----- Original Message ----
> From: Andrei Alexandrescu <andrei at erdani.com>
> To: Discuss the phobos library for D <phobos at puremagic.com>
> Sent: Wed, July 14, 2010 10:42:06 PM
> Subject: Re: [phobos] Silent failure of std.container unittests
> 
> On 07/14/2010 08:58 PM, Walter Bright wrote:
> > 
> > 
> > Sean  Kelly wrote:
> >> 
> >> 
> >> How about this... since unit  tests are run after static ctors are run, the behavior of whether to  throw an AssertError or report and continue can be a run-time  configurable option.
> > 
> > Frankly, I don't think a configurable  option is a good idea. Druntime has a lot of user configurability, and  as far as I can tell, absolutely none of it has ever been used. It just  adds complexity, both to the code and the documentation.
> 
> I agree.  But also simple does not mean crappy. I think there's too much talk
>around a  simple matter:
> 
> ======
> Assertion failure should abort the current  unittest block.
> ======
> 
> Do we all agree about the above? We keep on  discussing how the usefulness and
>informativeness of assertion failures falls  off a cliff after the first failure, and I can't seem to hear loud and clear  that we all want this same thing.

I agree.

> 
> So we've been through these  steps:
> 
> 1. Crappy: failure to assert causes the program to  abort.
> 
> 2. Awful: assert is hacked inside unittests to essentially be  writeln, all
>unittests run regardless. Programs claims success upon  exit.
> 
> 3. Mediocre: assert continues to be hacked inside unittests to  essentially be
>writeln + set a flag. Programs exit with errors if the flag was  set.
> 
> NOT YET:
> 
> 4. DESIRED: assert is NOT hacked, any failing assert  ends the current
>unittest, the failure message is printed, execution continues  with the next unittest, program ends with error code if at least one assert  failed, everybody's happy.

Definitely 4.  Option 1 is workable, but if it takes over a minute to compile with unit tests (as dcollections does), then I want to see all unit tests that fail so I don't have to go through a tedious compile->go-get-a-beer->run-unit-tests->see-if-I'm-sober-enough-to-fix-them cycle ;)

Here is one option that I haven't seen yet -- the compiler inserts around all unit test blocks the following framework:

try
{
   // run individual unit test block
}
catch(AssertionError ae)
{
   __onAssert(ae);
}

And then we can control exactly what happens on assert.  Someone may create some sort of graphical thing, or it could be plugged into an IDE.

-Steve

On Thursday 15 July 2010 06:57:47 Steve Schveighoffer wrote:
> 
> Here is one option that I haven't seen yet -- the compiler inserts around all unit test blocks the following framework:
> 
> try
> {
>    // run individual unit test block
> }
> catch(AssertionError ae)
> {
>    __onAssert(ae);
> }
> 
> And then we can control exactly what happens on assert.  Someone may create some sort of graphical thing, or it could be plugged into an IDE.

That sounds like a good idea. Certainly, I've worked in places where we had the IDE set up to not only run unit tests but indicated which failed and which succeeded, and dealing with all that from the IDE was quite nice. Having built in unit tests in D is great, but thus far it's led to a fairly simplistic implementation in terms of what you can do with it. Something which allowed 3rd party tools to plug in to it or which could later be reasonably extended to do so would be good. It does seem to me that #4 could be reasonably extended to this later if it weren't implemented like this now.

- Jonathan M Davis

On Jul 15, 2010, at 6:57 AM, Steve Schveighoffer wrote:
> 
> ----- Original Message ----
>> From: Andrei Alexandrescu <andrei at erdani.com>
>> To: Discuss the phobos library for D <phobos at puremagic.com>
>> Sent: Wed, July 14, 2010 10:42:06 PM
>> Subject: Re: [phobos] Silent failure of std.container unittests
>> 
>> On 07/14/2010 08:58 PM, Walter Bright wrote:
>>> 
>>> 
>>> Sean  Kelly wrote:
>>>> 
>>>> 
>>>> How about this... since unit  tests are run after static ctors are run, the behavior of whether to  throw an AssertError or report and continue can be a run-time  configurable option.
>>> 
>>> Frankly, I don't think a configurable  option is a good idea. Druntime has a lot of user configurability, and  as far as I can tell, absolutely none of it has ever been used. It just  adds complexity, both to the code and the documentation.
>> 
>> I agree.  But also simple does not mean crappy. I think there's too much talk around a  simple matter:
>> 
>> ======
>> Assertion failure should abort the current  unittest block.
>> ======
>> 
>> Do we all agree about the above? We keep on  discussing how the usefulness and informativeness of assertion failures falls  off a cliff after the first failure, and I can't seem to hear loud and clear  that we all want this same thing.
> 
> I agree.
> 
>> 
>> So we've been through these  steps:
>> 
>> 1. Crappy: failure to assert causes the program to  abort.
>> 
>> 2. Awful: assert is hacked inside unittests to essentially be  writeln, all unittests run regardless. Programs claims success upon  exit.
>> 
>> 3. Mediocre: assert continues to be hacked inside unittests to  essentially be writeln + set a flag. Programs exit with errors if the flag was  set.
>> 
>> NOT YET:
>> 
>> 4. DESIRED: assert is NOT hacked, any failing assert  ends the current unittest, the failure message is printed, execution continues  with the next unittest, program ends with error code if at least one assert  failed, everybody's happy.
> 
> Definitely 4.  Option 1 is workable, but if it takes over a minute to compile with unit tests (as dcollections does), then I want to see all unit tests that fail so I don't have to go through a tedious compile->go-get-a-beer->run-unit-tests->see-if-I'm-sober-enough-to-fix-them cycle ;)
> 
> Here is one option that I haven't seen yet -- the compiler inserts around all unit test blocks the following framework:
> 
> try
> {
>   // run individual unit test block
> }
> catch(AssertionError ae)
> {
>   __onAssert(ae);
> }
> 
> And then we can control exactly what happens on assert.  Someone may create some sort of graphical thing, or it could be plugged into an IDE.

Huh, this is kind of like the overridable assert handler in druntime already, except that it's actually useful :-)  For now, this could be achieved by overriding the module unit tester however.  It's just a bit bulkier than simply overriding the reporting mechanism.

Andrei Alexandrescu wrote:
> On 07/14/2010 08:58 PM, Walter Bright wrote:
>>
>>
>>
>> Frankly, I don't think a configurable option is a good idea. Druntime has a lot of user configurability, and as far as I can tell, absolutely none of it has ever been used. It just adds complexity, both to the code and the documentation.
>
> I agree. But also simple does not mean crappy. I think there's too much talk around a simple matter:
>
> ======
> Assertion failure should abort the current unittest block.
> ======
>
> Do we all agree about the above?

Not me.

> We keep on discussing how the usefulness and informativeness of assertion failures falls off a cliff after the first failure, and I can't seem to hear loud and clear that we all want this same thing.
>
> So we've been through these steps:
>
> 1. Crappy: failure to assert causes the program to abort.

Agreed.

>
> 2. Awful: assert is hacked inside unittests to essentially be writeln, all unittests run regardless. Programs claims success upon exit.

This was a bug, nobody intended this behavior.

>
> 3. Mediocre: assert continues to be hacked inside unittests to essentially be writeln + set a flag. Programs exit with errors if the flag was set.

Yes, I think this is the correct behavior.

>
> NOT YET:
>
> 4. DESIRED: assert is NOT hacked, any failing assert ends the current unittest, the failure message is printed, execution continues with the next unittest, program ends with error code if at least one assert failed, everybody's happy.
>

Everybody's happy except the guys like me who want to run all the unit tests in one go.

On 7/15/10, Walter Bright <walter at digitalmars.com> wrote:
>> Assertion failure should abort the current unittest block.
>>
>> Do we all agree about the above?
>
> Not me.

That's madness!

unittest {
    auto a = something();
    assert(a !is null);
     a.stuff(); // if a is null, this is meaningless
}

> Everybody's happy except the guys like me who want to run all the unit tests in one go.

What you could do is:

unittest { assert(my test); }
unittest { assert(my other test); }
unittest {
  // complex test
}

It is a bit verbose, but you'd get all the results you want without breaking cases like my first null assert one.

On Jul 15, 2010, at 3:00 PM, Walter Bright wrote:
> 
>> 
>> NOT YET:
>> 
>> 4. DESIRED: assert is NOT hacked, any failing assert ends the current unittest, the failure message is printed, execution continues with the next unittest, program ends with error code if at least one assert failed, everybody's happy.
>> 
> 
> Everybody's happy except the guys like me who want to run all the unit tests in one go.

All the unit tests or all the asserts?  All the unit tests are run with this approach:

    module A;

    unittest {
        assert(false, "test 1a failed");
        assert(false, "test 1b failed");
    }

    unittest {
        assert(false, "test 2 failed");
    }

This should print:

    test 1a failed
    test 2 failed

I'm not sure I understand the problem of breaking unit tests into a collection of atomic blocks.  With version(unittest) globals can even be used if state should be retained across tests.

Le 2010-07-15 ? 18:00, Walter Bright a ?crit :

>> 4. DESIRED: assert is NOT hacked, any failing assert ends the current unittest, the failure message is printed, execution continues with the next unittest, program ends with error code if at least one assert failed, everybody's happy.
> 
> Everybody's happy except the guys like me who want to run all the unit tests in one go.

I think this is the root of the misunderstanding. Either you consider each assert grouped in a unittest blocks to be a unit test in itself; or you consider each unittest block to be a test with multiple possible points of failure across the path of the test.

The former is best illustrated this way:

	unittest {
		assert(sqrt(16) == 4);
		assert(sqrt(4) == 2);
		assert(sqrt(1) == 1);
	}

Knowing which assertion fails in the given test might help to solve the problem, so it's legitimate in my view.

And here's an illustration of a second test modelling a sequence of events. If one of these test fails, all others will fail too because they depend on each other:

	unittest {
		S s = new S;
		s.giveMeAName();
		assert(s.fileName);
		s.value = 1;
		assert(s.value == 1);
		assert(s.needsToBeSaved);
		s.save();
		assert(!s.needsToBeSaved);
		s.value = 2;
		assert(s.value == 2);
		assert(s.needsToBeSaved);
		s.revert();
		assert(s.value == 1);
		assert(!s.needsToBeSaved);
		...
	}

So here you only need to know about the first error, and the rest is garbage.

It also highlights another problem about the current behaviour: what happens if one of the functions called in this test throws an exception? My guess is that it would abort unit tests for the whole module... I don't think anyone wants that.

-- 
Michel Fortin
michel.fortin at michelf.com
http://michelf.com/

On 07/15/2010 05:32 PM, Michel Fortin wrote:
>
> Le 2010-07-15 ? 18:00, Walter Bright a ?crit :
>
>>> 4. DESIRED: assert is NOT hacked, any failing assert ends the current unittest, the failure message is printed, execution continues with the next unittest, program ends with error code if at least one assert failed, everybody's happy.
>>
>> Everybody's happy except the guys like me who want to run all the unit tests in one go.
>
> I think this is the root of the misunderstanding. Either you consider each assert grouped in a unittest blocks to be a unit test in itself; or you consider each unittest block to be a test with multiple possible points of failure across the path of the test.
>
> The former is best illustrated this way:
>
> 	unittest {
> 		assert(sqrt(16) == 4);
> 		assert(sqrt(4) == 2);
> 		assert(sqrt(1) == 1);
> 	}

There's one more problem with this, which should be discussed - an assert inside e.g. sqrt or any function that sqrt transitively calls _does_ stop the entire thing. So assert has different semantics depending on locus, which I hope we agree is not a desirable trait. Also, as you mentioned, if someone throws, we're again back to a different semantics.

I think there's a disadvantage there. FWIW changing the semantics of assert that way will translate into a disincentive to use it inside unittests ("Hmm, I better use enforce() here because assert() is just weird.")


Andrei