Add a test runner executable.

Add a test runner executable.

This is still extremely bare-bones and won't work with tests that actually
import "package:unittest/unittest.dart", but it's something.

R=kevmoo@google.com
See #2

Committed: https://github.com/dart-lang/unittest/commit/533b58ce1de2a3a7fe31635a709b42e88e4588c1

[kevmoo, 2015-02-19 01:54:09]

https://codereview.chromium.org/933083002/diff/40001/bin/unittest.dart
File bin/unittest.dart (right):

https://codereview.chromium.org/933083002/diff/40001/bin/unittest.dart#newcode15
bin/unittest.dart:15: import 'package:unittest/src/io.dart';
unused import

https://codereview.chromium.org/933083002/diff/40001/bin/unittest.dart#newcode24
bin/unittest.dart:24: _parser.addFlag("help", abbr: "h", negatable: false,
Just set these up in the field declaration. Use cascades. They don't sting.

https://codereview.chromium.org/933083002/diff/40001/bin/unittest.dart#newcode44
bin/unittest.dart:44: var paths = options.rest;
Could all of this be outside of the Future.sync call?

Then your first async call can be Future.wait

https://codereview.chromium.org/933083002/diff/40001/bin/unittest.dart#newcode54
bin/unittest.dart:54: return Future.wait(paths.map((path) {
Could this be huge? Do we want to "pool" this to some reasonable number?

https://codereview.chromium.org/933083002/diff/40001/lib/src/console_reporter...
File lib/src/console_reporter.dart (right):

https://codereview.chromium.org/933083002/diff/40001/lib/src/console_reporter...
lib/src/console_reporter.dart:10: import 'package:stack_trace/stack_trace.dart';
unused import

https://codereview.chromium.org/933083002/diff/40001/lib/src/engine.dart
File lib/src/engine.dart (right):

https://codereview.chromium.org/933083002/diff/40001/lib/src/engine.dart#newc...
lib/src/engine.dart:71: Future.wait(liveTests.map((liveTest) =>
liveTest.close()));
Could this be huge? Do we want to "pool" this?

https://codereview.chromium.org/933083002/diff/40001/lib/src/io.dart
File lib/src/io.dart (right):

https://codereview.chromium.org/933083002/diff/40001/lib/src/io.dart#newcode9
lib/src/io.dart:9: import 'dart:isolate';
It seems like all of the new imports are unused.

https://codereview.chromium.org/933083002/diff/40001/lib/src/remote_exception...
File lib/src/remote_exception.dart (right):

https://codereview.chromium.org/933083002/diff/40001/lib/src/remote_exception...
lib/src/remote_exception.dart:60: 'supertype': supertype,
Do we care about the null case here?

Better to leave it out? Or put in a generic place-holder?

https://codereview.chromium.org/933083002/diff/40001/lib/src/vm_listener.dart
File lib/src/vm_listener.dart (right):

https://codereview.chromium.org/933083002/diff/40001/lib/src/vm_listener.dart...
lib/src/vm_listener.dart:34: } on NoSuchMethodError catch (error, stackTrace) {
skip the stackTrace arg if it's not used

https://codereview.chromium.org/933083002/diff/40001/test/io.dart
File test/io.dart (right):

https://codereview.chromium.org/933083002/diff/40001/test/io.dart#newcode11
test/io.dart:11: import 'package:unittest/unittest.dart';
unused import

https://codereview.chromium.org/933083002/diff/40001/test/runner_test.dart
File test/runner_test.dart (right):

https://codereview.chromium.org/933083002/diff/40001/test/runner_test.dart#ne...
test/runner_test.dart:5: import 'dart:async';
unused import

https://codereview.chromium.org/933083002/diff/40001/test/runner_test.dart#ne...
test/runner_test.dart:16: String _success = """
const

https://codereview.chromium.org/933083002/diff/40001/test/runner_test.dart#ne...
test/runner_test.dart:25: String _failure = """
const

[nweiz, 2015-02-19 02:10:11]

Code review changes

[nweiz, 2015-02-19 02:10:18]

https://codereview.chromium.org/933083002/diff/40001/bin/unittest.dart
File bin/unittest.dart (right):

https://codereview.chromium.org/933083002/diff/40001/bin/unittest.dart#newcode15
bin/unittest.dart:15: import 'package:unittest/src/io.dart';
On 2015/02/19 01:54:08, kevmoo wrote:
> unused import

Done.

https://codereview.chromium.org/933083002/diff/40001/bin/unittest.dart#newcode24
bin/unittest.dart:24: _parser.addFlag("help", abbr: "h", negatable: false,
On 2015/02/19 01:54:08, kevmoo wrote:
> Just set these up in the field declaration. Use cascades. They don't sting.

Setting them up here has several advantages. It's more flexible, so if we want
to e.g. show/hide a flag based on external considerations that's possible. It's
also more predictable, so we know exactly where the options are being added in
case there's an error in one of them.

https://codereview.chromium.org/933083002/diff/40001/bin/unittest.dart#newcode44
bin/unittest.dart:44: var paths = options.rest;
On 2015/02/19 01:54:08, kevmoo wrote:
> Could all of this be outside of the Future.sync call?
> 
> Then your first async call can be Future.wait

We need the [LoadException] to be handled by the [catchError] clause below.

https://codereview.chromium.org/933083002/diff/40001/bin/unittest.dart#newcode54
bin/unittest.dart:54: return Future.wait(paths.map((path) {
On 2015/02/19 01:54:08, kevmoo wrote:
> Could this be huge? Do we want to "pool" this to some reasonable number?

We don't run them all at once, and loading them in isolates isn't very
expensive. I think it's fine.

https://codereview.chromium.org/933083002/diff/40001/lib/src/engine.dart
File lib/src/engine.dart (right):

https://codereview.chromium.org/933083002/diff/40001/lib/src/engine.dart#newc...
lib/src/engine.dart:71: Future.wait(liveTests.map((liveTest) =>
liveTest.close()));
On 2015/02/19 01:54:08, kevmoo wrote:
> Could this be huge? Do we want to "pool" this?

It's important for the API that the caller have access to all the [LiveTest]s at
once. I don't think it's going to be that resource-intensive, either, and I
don't want to speculate on that anyway; premature optimization and so forth.

https://codereview.chromium.org/933083002/diff/40001/lib/src/io.dart
File lib/src/io.dart (right):

https://codereview.chromium.org/933083002/diff/40001/lib/src/io.dart#newcode9
lib/src/io.dart:9: import 'dart:isolate';
On 2015/02/19 01:54:08, kevmoo wrote:
> It seems like all of the new imports are unused.

Done.

https://codereview.chromium.org/933083002/diff/40001/lib/src/remote_exception...
File lib/src/remote_exception.dart (right):

https://codereview.chromium.org/933083002/diff/40001/lib/src/remote_exception...
lib/src/remote_exception.dart:60: 'supertype': supertype,
On 2015/02/19 01:54:08, kevmoo wrote:
> Do we care about the null case here?
> 
> Better to leave it out? Or put in a generic place-holder?

It's illegal to throw null.

https://codereview.chromium.org/933083002/diff/40001/lib/src/vm_listener.dart
File lib/src/vm_listener.dart (right):

https://codereview.chromium.org/933083002/diff/40001/lib/src/vm_listener.dart...
lib/src/vm_listener.dart:34: } on NoSuchMethodError catch (error, stackTrace) {
On 2015/02/19 01:54:08, kevmoo wrote:
> skip the stackTrace arg if it's not used

Done.

https://codereview.chromium.org/933083002/diff/40001/test/runner_test.dart
File test/runner_test.dart (right):

https://codereview.chromium.org/933083002/diff/40001/test/runner_test.dart#ne...
test/runner_test.dart:5: import 'dart:async';
On 2015/02/19 01:54:09, kevmoo wrote:
> unused import

Done.

https://codereview.chromium.org/933083002/diff/40001/test/runner_test.dart#ne...
test/runner_test.dart:16: String _success = """
On 2015/02/19 01:54:09, kevmoo wrote:
> const

That won't do anything. There's only one instance of this string anyway.

Also, this is a test; optimizing for extremely narrow performance gains like
const variables give you is never going to be worth even a small readability
loss.

https://codereview.chromium.org/933083002/diff/40001/test/runner_test.dart#ne...
test/runner_test.dart:25: String _failure = """
On 2015/02/19 01:54:09, kevmoo wrote:
> const

ditto

[kevmoo, 2015-02-19 02:15:44]

LGTM w/ nits

https://codereview.chromium.org/933083002/diff/40001/test/runner_test.dart
File test/runner_test.dart (right):

https://codereview.chromium.org/933083002/diff/40001/test/runner_test.dart#ne...
test/runner_test.dart:16: String _success = """
On 2015/02/19 02:10:18, nweiz wrote:
> On 2015/02/19 01:54:09, kevmoo wrote:
> > const
> 
> That won't do anything. There's only one instance of this string anyway.
> 
> Also, this is a test; optimizing for extremely narrow performance gains like
> const variables give you is never going to be worth even a small readability
> loss.

It's not about perf gains. It's about giving folks who read this test a clear
notion of if this can be mutated or not. I need to watch _sandbox and where it
can change. I don't for _success and _failure

final is fine

https://codereview.chromium.org/933083002/diff/40001/test/runner_test.dart#ne...
test/runner_test.dart:25: String _failure = """
On 2015/02/19 02:10:18, nweiz wrote:
> On 2015/02/19 01:54:09, kevmoo wrote:
> > const
> 
> ditto

ditto

[nweiz, 2015-02-19 02:18:50]

https://codereview.chromium.org/933083002/diff/40001/test/runner_test.dart
File test/runner_test.dart (right):

https://codereview.chromium.org/933083002/diff/40001/test/runner_test.dart#ne...
test/runner_test.dart:16: String _success = """
On 2015/02/19 02:15:44, kevmoo wrote:
> On 2015/02/19 02:10:18, nweiz wrote:
> > On 2015/02/19 01:54:09, kevmoo wrote:
> > > const
> > 
> > That won't do anything. There's only one instance of this string anyway.
> > 
> > Also, this is a test; optimizing for extremely narrow performance gains like
> > const variables give you is never going to be worth even a small readability
> > loss.
> 
> It's not about perf gains. It's about giving folks who read this test a clear
> notion of if this can be mutated or not. I need to watch _sandbox and where it
> can change. I don't for _success and _failure
> 
> final is fine

Done.

[nweiz, 2015-02-19 02:19:12]

New patchsets have been uploaded after l-g-t-m from kevmoo@google.com

[nweiz, 2015-02-19 02:19:14]

Code review changes

[kevmoo, 2015-02-19 02:49:04]

lgtm

[nweiz, 2015-02-19 20:01:36]

Committed patchset #5 (id:80001) manually as
533b58ce1de2a3a7fe31635a709b42e88e4588c1 (presubmit successful).