Cleaning up unittest configurations

This change does a couple fixes to configurations:
- it renames '_postMessage' so that it can be overriden by configurations that don't need it. 
- it splits the onDone callback in two: onSummary - a callback with all the results, and onDone - a callback with just the end result. The idea is that onSummary is mostly dedicated to reporting results, while onDone is about notifying that unittest is done. This split is very useful for subclasses that only want to override onSummary or onDone.

Committed: https://code.google.com/p/dart/source/detail?r=16899

[Siggi Cherem (dart-lang), 2013-01-10 02:21:42]

Hi Peter - I don't believe the rename of _postMessage should affect the dart2js
tests in any way, but I'm cc'ing you here just so you are aware (since you
introduced _postMessage in the first place).

[Jennifer Messerly, 2013-01-10 02:31:32]

https://codereview.chromium.org/11829045/diff/3/pkg/unittest/lib/compact_vm_c...
File pkg/unittest/lib/compact_vm_config.dart (right):

https://codereview.chromium.org/11829045/diff/3/pkg/unittest/lib/compact_vm_c...
pkg/unittest/lib/compact_vm_config.dart:85: } on Exception catch(ex) {
should this just be a raw catch -- or do we know for sure it won't be an Error?

https://codereview.chromium.org/11829045/diff/3/pkg/unittest/lib/compact_vm_c...
pkg/unittest/lib/compact_vm_config.dart:86: exit(1);
out of curiousity -- why do we need the explicit "exit(1)" if the base class
doesn't have it?

[Siggi Cherem (dart-lang), 2013-01-10 02:44:49]

https://codereview.chromium.org/11829045/diff/3/pkg/unittest/lib/compact_vm_c...
File pkg/unittest/lib/compact_vm_config.dart (right):

https://codereview.chromium.org/11829045/diff/3/pkg/unittest/lib/compact_vm_c...
pkg/unittest/lib/compact_vm_config.dart:85: } on Exception catch(ex) {
On 2013/01/10 02:31:32, John Messerly wrote:
> should this just be a raw catch -- or do we know for sure it won't be an
Error?

Done (see changes in vm_config). Technically we know it is Exception because
that's what we throw in config.dart, but using a raw catch works too.

https://codereview.chromium.org/11829045/diff/3/pkg/unittest/lib/compact_vm_c...
pkg/unittest/lib/compact_vm_config.dart:86: exit(1);
On 2013/01/10 02:31:32, John Messerly wrote:
> out of curiousity -- why do we need the explicit "exit(1)" if the base class
> doesn't have it?

Configuration (the base class I was using) is meant to work everywhere (drt, vm,
etc), so we couldn't use 'exit' there. Instead, Configuration throws an
exception as a way to exit with an error code.

Failure runs always have a stack trace from Configuration, so we used this catch
as a way of getting rid of it in the vm. I was doing this here and in
VMConfiguration, which made me realize that the base class for this compact
configuration should be VMConfiguration instead. I just updated the code
accordingly.

[Jennifer Messerly, 2013-01-10 03:01:18]

lgtm

https://codereview.chromium.org/11829045/diff/5001/pkg/unittest/lib/vm_config...
File pkg/unittest/lib/vm_config.dart (left):

https://codereview.chromium.org/11829045/diff/5001/pkg/unittest/lib/vm_config...
pkg/unittest/lib/vm_config.dart:26: void useVmConfiguration() {
totally optional but: is it worth having an @deprecated forward for this method,
and remove after this build goes out?

(guessing a decent number of people are using unittest, so they might be calling
this method)

[Siggi Cherem (dart-lang), 2013-01-10 03:35:31]

On 2013/01/10 03:01:18, John Messerly wrote:
> lgtm
> 
>
https://codereview.chromium.org/11829045/diff/5001/pkg/unittest/lib/vm_config...
> File pkg/unittest/lib/vm_config.dart (left):
> 
>
https://codereview.chromium.org/11829045/diff/5001/pkg/unittest/lib/vm_config...
> pkg/unittest/lib/vm_config.dart:26: void useVmConfiguration() {
> totally optional but: is it worth having an @deprecated forward for this
method,
> and remove after this build goes out?
> 
> (guessing a decent number of people are using unittest, so they might be
calling
> this method)

good idea - unfortunately right now the publish_pkg script doesn't rewrite
imports to other packages correctly (I need to import the meta package). I'll
follow up on a subsequent CL.

[ahe, 2013-01-10 09:12:37]

I understand that it is very annoying that the tests print an additional
message, but this is nothing compared to failing tests being recorded as
passing. That is what happens with this change.

I'd really appreciate if you could revert this until we can get a chance to
discuss the details in VC as suggested on Sep 22, 2012 when you first suggested
making this change.

I don't think the situation has changed since then, and I'm surprised that you
have essentially implemented exactly what I pointed out was problematic.

[Siggi Cherem (dart-lang), 2013-01-10 16:41:51]

On 2013/01/10 09:12:37, ahe wrote:
> I understand that it is very annoying that the tests print an additional
> message, but this is nothing compared to failing tests being recorded as
> passing. That is what happens with this change.
> 
> I'd really appreciate if you could revert this until we can get a chance to
> discuss the details in VC as suggested on Sep 22, 2012 when you first
suggested
> making this change.
> 
> I don't think the situation has changed since then, and I'm surprised that you
> have essentially implemented exactly what I pointed out was problematic.

Sorry for the misunderstanding - I reverted the changes to _postMessage in
r16925.

When we were talking on Sep 22 we were chatting about two options, using
wrappers or renaming the method. I was unaware of the problems with wrappers,
which you clarified, so I thought that was the main concern you had. I,
incorrectly, thought the other approach was still valid.

Let's chat when you get a chance. 
Cheers,
Siggi