[phobos] Silent failure of std.container unittests

[Jonathan M Davis]

On Wednesday, July 14, 2010 14:43:26 Walter Bright wrote:
> Sean Kelly wrote:
> > Right.  I was proposing that ModuleInfo have a list of unittest modules
> > instead of a reference to a single function which calls them each in
> > turn (ie. finer granularity).  I've already taken care of the assert
> > behavior on my local machine, but I don't want to commit the change
> > unless it's what people want.  I thought that since you reverted my
> > unittest changes yesterday that non-throwing asserts might actually be
> > desired. _______________________________________________
> 
> There isn't a right answer. The overwhelming feedback I received prior
> to 2.44 was that all the unittests should run, and then the program
> should exit if any of them failed, and that's what I implemented. I
> think that also makes a lot of sense - if there are a lot, simply
> redirect them to a file and then fix the ones you want to, all in one
> go. I don't think it is a big problem to ignore messages from tests that
> are redundant.
> 
> The other tenable option is to allow at most one unittest failure per
> module. Or one per unittest block. Either way, we'll get complaints. I
> suggest we go with the "run them all".
> _______________________________________________
> phobos mailing list
> phobos at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/phobos

I would have thought that when people were saying "let all the unit tests run," 
they were referring to each unittest block rather than each assertion within the 
unittest blocks. If frameworks like JUnit, each unit test (in JUnit's case, each 
unit test is a function) will be run and sucess or failure will be indicated for 
each test with failure meaning that _one_ of the assertions failed - a test does 
not continue if an assertion fails.

I think that it makes sense to treat a unittest block as a single unit test and 
treat a single assertion failure within a unittest block as failure for that 
test but have that have no effect on whether the other unittest blocks are run. 
If we ever moved towards named unit tests, this makes even more sense. e.g.

unittest(equality_test)
{
  ... //whatever tests we do for equality for whatever this test is for
}

Treating a unittest block as a single unit test makes a lot of sense IMO, and it 
lends itself quite well to good granularity. If you want to test a bunch of stuff 
in a single test and have it fail on a single assertion, then put it all in one 
unittest block. If you want to test a bunch of stuff individually without each 
test affecting the others, then put each test in a separate in a separate 
unittest block.

That would be the typical way that unit test tools work (in my experience at 
least), and I think that's more or less what most people are looking for. It 
also makes it work better to move towards named unittests in the future like a 
number of people have requested. It would even make it possible to make it so 
that you could simply print out success or failure for each test if you wanted 
to do set things up that way.

Having everything halt because of a single assertion failure makes for slow 
testing. On the other hand, never halting on assertion failures makes for a lot 
of cruft in your results. Treating each unittest block as a single unittest 
which succeeds or fails, stopping a particular block when one of its assertions 
fails and then moving on to the next block seems by far the best solution to me.

- Jonathan M Davis