Fix bug in the unittest's withTestEnvironment package and apply more feedback from nweiz.

Fix bug in the unittest's withTestEnvironment package and apply more feedback from nweiz.

BUG=
R=kustermann@google.com, sgjesse@google.com

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

[wibling, 2014-11-18 10:14:12]

wibling@google.com changed reviewers:
+ kevmoo@google.com, kustermann@google.com, nweiz@google.com, sgjesse@google.com

[Søren Gjesse, 2014-11-18 10:23:51]

lgtm

[kustermann, 2014-11-18 10:35:37]

lgtm

https://codereview.chromium.org/734343002/diff/20001/pkg/unittest/test/with_t...
File pkg/unittest/test/with_test_environment_test.dart (right):

https://codereview.chromium.org/734343002/diff/20001/pkg/unittest/test/with_t...
pkg/unittest/test/with_test_environment_test.dart:54: test('test1', () =>
expect(4 + 3, equals(7)));
Maybe make the second test fail and check that it will be reported as a failure
further down (so you check for 1 passing and 2 failing test)?

https://codereview.chromium.org/734343002/diff/20001/pkg/unittest/test/with_t...
pkg/unittest/test/with_test_environment_test.dart:76: .whenComplete(() {
I'd move the .whenComplete() to the previous line:

runUnittests(runTests).whenComplete(() {
  // ...
});

https://codereview.chromium.org/734343002/diff/20001/pkg/unittest/test/with_t...
pkg/unittest/test/with_test_environment_test.dart:84: .whenComplete(() {
ditto.

[wibling, 2014-11-18 10:43:03]

Committed patchset #3 (id:40001) manually as 41798 (presubmit successful).

[nweiz, 2014-11-19 21:44:18]

lgtm

https://codereview.chromium.org/734343002/diff/40001/pkg/unittest/test/with_t...
File pkg/unittest/test/with_test_environment_test.dart (right):

https://codereview.chromium.org/734343002/diff/40001/pkg/unittest/test/with_t...
pkg/unittest/test/with_test_environment_test.dart:28: .catchError((error, stack)
=> _completer.completeError(error, stack));
Indent this +4 spaces.

You can write .then(_completer.complete) and .catchError(_completer.catchError)
here.

https://codereview.chromium.org/734343002/diff/40001/pkg/unittest/test/with_t...
pkg/unittest/test/with_test_environment_test.dart:32: for (final t in _results)
{
We don't use "final" for local variables.

Also, this method is probably clearer as just "_results.any((test) =>
test.description == testName)".

[wibling, 2014-11-20 14:34:51]

Thanks for the review. 

I have sent out a new diff with the changes.

https://codereview.chromium.org/734343002/diff/40001/pkg/unittest/test/with_t...
File pkg/unittest/test/with_test_environment_test.dart (right):

https://codereview.chromium.org/734343002/diff/40001/pkg/unittest/test/with_t...
pkg/unittest/test/with_test_environment_test.dart:28: .catchError((error, stack)
=> _completer.completeError(error, stack));
On 2014/11/19 21:44:18, nweiz wrote:
> Indent this +4 spaces.
> 
> You can write .then(_completer.complete) and
.catchError(_completer.catchError)
> here.

Done.

https://codereview.chromium.org/734343002/diff/40001/pkg/unittest/test/with_t...
pkg/unittest/test/with_test_environment_test.dart:32: for (final t in _results)
{
On 2014/11/19 21:44:18, nweiz wrote:
> We don't use "final" for local variables.
> 
> Also, this method is probably clearer as just "_results.any((test) =>
> test.description == testName)".

Done.