[phobos] Silent failure of std.container unittests

[Steve Schveighoffer]

----- Original Message ----

> From: Walter Bright <walter at digitalmars.com>
> To: Discuss the phobos library for D <phobos at puremagic.com>
> Sent: Thu, July 15, 2010 7:11:28 PM
> Subject: Re: [phobos] Silent failure of std.container unittests
> 
> 
> 
> Sean Kelly wrote:
> > 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 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.
> 
> Extra work.

But then assert doesn't do what assert is supposed to do.

The problem I see here is, inside unit tests, assert means two different things:

1. Display an error if a value isn't what it's supposed to be.
2. Make sure my code is in a sane state before it goes any further.

During unit tests, assert is used for 1 mostly, but many use it for 1 and 2 
combined.

A sample from dcollections, with each assert marked as whether it is 1 and/or 2:

    static if(doUnittest) unittest
    {
        auto hs = new HashSet;
        hs.add([1, 2, 3, 4, 5]);
        auto r = hs[];
        assert(rangeEqual(r, cast(V[])[1, 2, 3, 4, 5])); // 1, 2
        assert(r.front == hs.begin.front); // 1
        assert(r.back != r.front); // 1
        auto oldfront = r.front;
        auto oldback = r.back;
        r.popFront();
        r.popBack();
        assert(r.front != r.back); // 1, 2
        assert(r.front != oldfront); // 1, 2
        assert(r.back != oldback); // 1, 2

        auto b = r.begin;
        assert(!b.empty); // 1
        assert(b.front == r.front); // 1
        auto e = r.end;
        assert(e.empty); // 1
    }


I'm almost thinking we need to dump assert as "the way to fail unit tests" and 
go with something more configurable.  Leave assert the way it is.

Then, what we need is two functions, one that "exits the current unit test", and 
one that "just prints out a failure".

I'd suggest require() and check().

Note that we do not need the special traits of assert, it should only be used 
during unit tests.

FWIW, the current mode of unit testing (assert doesn't throw) didn't hinder much 
the testing of dcollections.

-Steve