Refactor test.dart.

The goal here is to rearrange StandardTestSuite so that it's possible override some of its behavior for running package tests while still keeping the rest of the behavior.

This patch is just the refactoring. I'll do the actual pub stuff in a separate patch.

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

[Bob Nystrom, 2012-11-09 00:44:04]

OK, I think this should be enough for now. The idea is that now I'll be able to
override enqueueTests() in a subclass of StandardTestSuite and do the work I
need to do for packages there.

[Emily Fortuna, 2012-11-09 01:41:44]

looks nice! just a few comments.
lgtm

https://codereview.chromium.org/11275217/diff/5001/samples/tests/dartc/test_c...
File samples/tests/dartc/test_config.dart (right):

https://codereview.chromium.org/11275217/diff/5001/samples/tests/dartc/test_c...
samples/tests/dartc/test_config.dart:50: bool get listRecursively => true;
:-D

https://codereview.chromium.org/11275217/diff/5001/tools/testing/dart/multite...
File tools/testing/dart/multitest.dart (right):

https://codereview.chromium.org/11275217/diff/5001/tools/testing/dart/multite...
tools/testing/dart/multitest.dart:242: return null;
shouldn't we get rid of these lines if we're returning a Future at line 217?

https://codereview.chromium.org/11275217/diff/5001/tools/testing/dart/test_su...
File tools/testing/dart/test_suite.dart (left):

https://codereview.chromium.org/11275217/diff/5001/tools/testing/dart/test_su...
tools/testing/dart/test_suite.dart:62: // TODO(1030): remove once in the
corelib.
:-)

https://codereview.chromium.org/11275217/diff/5001/tools/testing/dart/test_su...
File tools/testing/dart/test_suite.dart (right):

https://codereview.chromium.org/11275217/diff/5001/tools/testing/dart/test_su...
tools/testing/dart/test_suite.dart:482: Future waitForDartium() {
nit: maybe just call this updateDartium since in not all cases to we actually
wait.

[Bob Nystrom, 2012-11-09 20:56:26]

Thanks!

https://codereview.chromium.org/11275217/diff/5001/tools/testing/dart/multite...
File tools/testing/dart/multitest.dart (right):

https://codereview.chromium.org/11275217/diff/5001/tools/testing/dart/multite...
tools/testing/dart/multitest.dart:242: return null;
On 2012/11/09 01:41:44, Emily Fortuna wrote:
> shouldn't we get rid of these lines if we're returning a Future at line 217?

This code's a bit hard to read. So the return here is for the function
expression that gets passed to transform(). It works like:

1. futureCopies is a built up list of futures.
2. We pass that to Futures.wait(). That returns a new future that will complete
when everything in the list does.
3. We get that resulting future and call transform() on it.
4. We pass in a function to transform that takes (and discards) the list of
futures, does the work we actually care about and then returns null.
5. transform() returns a future that calls that callback.
6. We return the result of 6 from the method.

Async code is hard to follow. :(

https://codereview.chromium.org/11275217/diff/5001/tools/testing/dart/test_su...
File tools/testing/dart/test_suite.dart (right):

https://codereview.chromium.org/11275217/diff/5001/tools/testing/dart/test_su...
tools/testing/dart/test_suite.dart:482: Future waitForDartium() {
On 2012/11/09 01:41:44, Emily Fortuna wrote:
> nit: maybe just call this updateDartium since in not all cases to we actually
> wait.

Done.

[Emily Fortuna, 2012-11-09 21:03:14]

https://codereview.chromium.org/11275217/diff/5001/tools/testing/dart/multite...
File tools/testing/dart/multitest.dart (right):

https://codereview.chromium.org/11275217/diff/5001/tools/testing/dart/multite...
tools/testing/dart/multitest.dart:242: return null;
On 2012/11/09 20:56:26, Bob Nystrom wrote:
> On 2012/11/09 01:41:44, Emily Fortuna wrote:
> > shouldn't we get rid of these lines if we're returning a Future at line 217?
> 
> This code's a bit hard to read. So the return here is for the function
> expression that gets passed to transform(). It works like:
> 
> 1. futureCopies is a built up list of futures.
> 2. We pass that to Futures.wait(). That returns a new future that will
complete
> when everything in the list does.
> 3. We get that resulting future and call transform() on it.
> 4. We pass in a function to transform that takes (and discards) the list of
> futures, does the work we actually care about and then returns null.
> 5. transform() returns a future that calls that callback.
> 6. We return the result of 6 from the method.
> 
> Async code is hard to follow. :(

Oh, I see. And I apparently misread the indentation. Carry on!

[Bill Hesse, 2012-11-11 14:02:55]

Looks nice - cleaner than before.  Thanks.