Migration of testing scripts in tools/ to libv2

Migration of testing scripts in tools/ to libv2

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

[Bill Hesse, 2013-01-09 17:06:33]

A lot of cases where the return values of Future.then and Future.catchError are
not used need to be fixed.

https://codereview.chromium.org/11817012/diff/1015/tools/release/version.dart
File tools/release/version.dart (right):

https://codereview.chromium.org/11817012/diff/1015/tools/release/version.dart...
tools/release/version.dart:56: completeError(msg);
c.completeError(msg)

https://codereview.chromium.org/11817012/diff/1015/tools/test-runtime.dart
File tools/test-runtime.dart (right):

https://codereview.chromium.org/11817012/diff/1015/tools/test-runtime.dart#ne...
tools/test-runtime.dart:68: ['compiler', 'runtime', 'mode',
'arch'].mappedBy((name) => conf[name]);
You need toList here, unless you change settings to an iterator, don't you?

https://codereview.chromium.org/11817012/diff/1015/tools/test.dart
File tools/test.dart (right):

https://codereview.chromium.org/11817012/diff/1015/tools/test.dart#newcode100
tools/test.dart:100: ['compiler', 'runtime', 'mode', 'arch'].mappedBy((name) =>
conf[name]);
toList()

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/drt_upd...
File tools/testing/dart/drt_updater.dart (right):

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/drt_upd...
tools/testing/dart/drt_updater.dart:34: });
Need to use the return value of catchError, not the receiver, in future uses. 
In this case, .then(_onUpdatedHandler) will call the handler
and throw an exception, because there is no error catcher on the original
future.

There may be a lot of places where this pattern is used, and needs to be
changed.

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

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/multite...
tools/testing/dart/multitest.dart:146: var parts =
line.split('///')[1].split(':').mappedBy((s) => s.trim());
This returns an iterable, so either use the iterable below, or call toList.

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/multite...
tools/testing/dart/multitest.dart:151: return annotation;
toList.

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/scrub_s...
File tools/testing/dart/scrub_status_file.dart (right):

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/scrub_s...
tools/testing/dart/scrub_status_file.dart:14: import "dart:async";
What uses dart:async in this file?

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/test_ru...
File tools/testing/dart/test_runner.dart (right):

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:15: import "dart:io" as io;
Add a comment that we do this to avoid a name collision with an inherited field
exitCode and io.exitCode.

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:1120: processFuture.catchError((e) {
Another instance of f.then(); f.catchError();, which needs to be changed to
f.then().catchError();

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:1397: processFuture.catchError((e) {
Another.

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:1576: });
Another.

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:1649: processFuture.catchError((e) {
Another.

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

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/test_su...
tools/testing/dart/test_suite.dart:79: futures.add(task);
Add the return value of task.catchError(...).then(...) to futures, not task.

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/test_su...
tools/testing/dart/test_suite.dart:277: processFuture.catchError((e) {
another

https://codereview.chromium.org/11817012/diff/1015/tools/version.dart
File tools/version.dart (right):

https://codereview.chromium.org/11817012/diff/1015/tools/version.dart#newcode17
tools/version.dart:17: print("Could not create version number, failed with:
$e");
'nother

[kustermann, 2013-01-09 18:02:25]

PTAL.

https://codereview.chromium.org/11817012/diff/1015/tools/release/version.dart
File tools/release/version.dart (right):

https://codereview.chromium.org/11817012/diff/1015/tools/release/version.dart...
tools/release/version.dart:56: completeError(msg);
On 2013/01/09 17:06:33, Bill Hesse wrote:
> c.completeError(msg)

Done.

https://codereview.chromium.org/11817012/diff/1015/tools/test-runtime.dart
File tools/test-runtime.dart (right):

https://codereview.chromium.org/11817012/diff/1015/tools/test-runtime.dart#ne...
tools/test-runtime.dart:68: ['compiler', 'runtime', 'mode',
'arch'].mappedBy((name) => conf[name]);
On 2013/01/09 17:06:33, Bill Hesse wrote:
> You need toList here, unless you change settings to an iterator, don't you?

Done.

https://codereview.chromium.org/11817012/diff/1015/tools/test.dart
File tools/test.dart (right):

https://codereview.chromium.org/11817012/diff/1015/tools/test.dart#newcode100
tools/test.dart:100: ['compiler', 'runtime', 'mode', 'arch'].mappedBy((name) =>
conf[name]);
On 2013/01/09 17:06:33, Bill Hesse wrote:
> toList()

Done.

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/drt_upd...
File tools/testing/dart/drt_updater.dart (right):

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/drt_upd...
tools/testing/dart/drt_updater.dart:34: });
On 2013/01/09 17:06:33, Bill Hesse wrote:
> Need to use the return value of catchError, not the receiver, in future uses. 
> In this case, .then(_onUpdatedHandler) will call the handler
> and throw an exception, because there is no error catcher on the original
> future.
> 
> There may be a lot of places where this pattern is used, and needs to be
> changed.

Done.

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

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/multite...
tools/testing/dart/multitest.dart:146: var parts =
line.split('///')[1].split(':').mappedBy((s) => s.trim());
On 2013/01/09 17:06:33, Bill Hesse wrote:
> This returns an iterable, so either use the iterable below, or call toList.

Done.

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/multite...
tools/testing/dart/multitest.dart:151: return annotation;
On 2013/01/09 17:06:33, Bill Hesse wrote:
> toList.

Done.

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/scrub_s...
File tools/testing/dart/scrub_status_file.dart (right):

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/scrub_s...
tools/testing/dart/scrub_status_file.dart:14: import "dart:async";
On 2013/01/09 17:06:33, Bill Hesse wrote:
> What uses dart:async in this file?

Done.

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/test_ru...
File tools/testing/dart/test_runner.dart (right):

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:15: import "dart:io" as io;
On 2013/01/09 17:06:33, Bill Hesse wrote:
> Add a comment that we do this to avoid a name collision with an inherited
field
> exitCode and io.exitCode.

Done.

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:1120: processFuture.catchError((e) {
On 2013/01/09 17:06:33, Bill Hesse wrote:
> Another instance of f.then(); f.catchError();, which needs to be changed to
> f.then().catchError();

Done.

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:1397: processFuture.catchError((e) {
On 2013/01/09 17:06:33, Bill Hesse wrote:
> Another.

Done.

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:1576: });
On 2013/01/09 17:06:33, Bill Hesse wrote:
> Another.

Done.

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:1649: processFuture.catchError((e) {
On 2013/01/09 17:06:33, Bill Hesse wrote:
> Another.

Done.

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

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/test_su...
tools/testing/dart/test_suite.dart:79: futures.add(task);
On 2013/01/09 17:06:33, Bill Hesse wrote:
> Add the return value of task.catchError(...).then(...) to futures, not task.

Done.

https://codereview.chromium.org/11817012/diff/1015/tools/testing/dart/test_su...
tools/testing/dart/test_suite.dart:277: processFuture.catchError((e) {
On 2013/01/09 17:06:33, Bill Hesse wrote:
> another

Done.

https://codereview.chromium.org/11817012/diff/1015/tools/version.dart
File tools/version.dart (right):

https://codereview.chromium.org/11817012/diff/1015/tools/version.dart#newcode17
tools/version.dart:17: print("Could not create version number, failed with:
$e");
On 2013/01/09 17:06:33, Bill Hesse wrote:
> 'nother

Done.

[Bill Hesse, 2013-01-09 18:21:06]

lgtm

https://codereview.chromium.org/11817012/diff/16/tools/testing/dart/test_suit...
File tools/testing/dart/test_suite.dart (right):

https://codereview.chromium.org/11817012/diff/16/tools/testing/dart/test_suit...
tools/testing/dart/test_suite.dart:80: var handledTaskFuture =
task.catchError((e) {
Couldn't you just put parentheses around all this, and add it to futures where
it was added before?

[kustermann, 2013-01-09 18:27:40]

https://codereview.chromium.org/11817012/diff/16/tools/testing/dart/test_suit...
File tools/testing/dart/test_suite.dart (right):

https://codereview.chromium.org/11817012/diff/16/tools/testing/dart/test_suit...
tools/testing/dart/test_suite.dart:80: var handledTaskFuture =
task.catchError((e) {
On 2013/01/09 18:21:06, Bill Hesse wrote:
> Couldn't you just put parentheses around all this, and add it to futures where
> it was added before?

I think it is a bit more readable this way.