enable batch mode for dartdevc tests, also fix status so ddc bots pass

enable batch mode for dartdevc tests, also fix status so ddc bots pass

R=rnystrom@google.com

Committed: https://github.com/dart-lang/sdk/commit/62303b67116fca0fb5efe97acf35af6afbe51a16

[Jennifer Messerly, 2017-08-03 04:17:22]

jmesserly@google.com changed reviewers:
+ rnystrom@google.com

[Jennifer Messerly, 2017-08-03 04:17:23]

This does not fix runtime status yet, but it's now passing this:

./tools/test.py -m release -r none -c dartdevc --strong language_2
language_strong corelib_2 corelib_strong lib_2 lib_strong

.. and performance is fairly good! this is on my laptop:

[00:58 | 100% | + 2580 | -    0]

[Jennifer Messerly, 2017-08-03 05:41:16]

jmesserly@google.com changed reviewers:
+ vsm@google.com

[Jennifer Messerly, 2017-08-03 05:41:17]

I'm going to upload a new version because runtime is now passing! :D

$ ./tools/test.py -m release -r chrome -c dartdevc --strong language_strong
lib_strong corelib_strong language_2 lib_2 corelib_2

[05:39 | 100% | + 2612 | -    0]

[Bob Nystrom, 2017-08-03 18:28:04]

LGTM!

https://codereview.chromium.org/2987393002/diff/40001/pkg/dev_compiler/bin/da...
File pkg/dev_compiler/bin/dartdevc.dart (right):

https://codereview.chromium.org/2987393002/diff/40001/pkg/dev_compiler/bin/da...
pkg/dev_compiler/bin/dartdevc.dart:57: Stream input =
stdin.transform(UTF8.decoder).transform(new LineSplitter());
Can this stream be typed more precisely, or inferred?

Or, perhaps easier, could you use stdin.readLineSync() instead?

https://codereview.chromium.org/2987393002/diff/40001/pkg/dev_compiler/bin/da...
pkg/dev_compiler/bin/dartdevc.dart:65: ++totalTests;
Random nit: Maybe it's just me, but I tend to think of postfix "++" as the
default one and only use prefix when I specifically need the prefix semantics.

A quick code search shows 741 prefix ones and 3,596 postfix ones. Not that it
really matters either way.

https://codereview.chromium.org/2987393002/diff/40001/pkg/dev_compiler/lib/sr...
File pkg/dev_compiler/lib/src/compiler/command.dart (right):

https://codereview.chromium.org/2987393002/diff/40001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/command.dart:288: Set<String>
knownAbbreviations = new Set<String>();
No "var"?

https://codereview.chromium.org/2987393002/diff/40001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/command.dart:297: for (int i = 0; i <
args.length; i++) {
Use a for-in loop?

https://codereview.chromium.org/2987393002/diff/40001/tools/testing/dart/comm...
File tools/testing/dart/command_output.dart (right):

https://codereview.chromium.org/2987393002/diff/40001/tools/testing/dart/comm...
tools/testing/dart/command_output.dart:721: return exitCode != 0
How about "== 0" and flipping the cases to avoid the mental negation?

[Jennifer Messerly, 2017-08-03 19:43:06]

thanks! addressed those. going to make sure tests still pass after those changes
& merging status stuff.

https://codereview.chromium.org/2987393002/diff/40001/pkg/dev_compiler/bin/da...
File pkg/dev_compiler/bin/dartdevc.dart (right):

https://codereview.chromium.org/2987393002/diff/40001/pkg/dev_compiler/bin/da...
pkg/dev_compiler/bin/dartdevc.dart:57: Stream input =
stdin.transform(UTF8.decoder).transform(new LineSplitter());
On 2017/08/03 18:28:04, Bob Nystrom wrote:
> Can this stream be typed more precisely, or inferred?
> 
> Or, perhaps easier, could you use stdin.readLineSync() instead?

I just copy+pasted this from the other impls of batch mode (think I took this
from Kernel as it was the simplest). I can change that to say `var`. I don't
know why they're not using `readLineSync`, that's a good idea

https://codereview.chromium.org/2987393002/diff/40001/pkg/dev_compiler/bin/da...
pkg/dev_compiler/bin/dartdevc.dart:65: ++totalTests;
On 2017/08/03 18:28:04, Bob Nystrom wrote:
> Random nit: Maybe it's just me, but I tend to think of postfix "++" as the
> default one and only use prefix when I specifically need the prefix semantics.
> 
> A quick code search shows 741 prefix ones and 3,596 postfix ones. Not that it
> really matters either way.

again this is just copied code. I don't mind changing it, but it already exists
like this elsewhere in the repo :)

https://codereview.chromium.org/2987393002/diff/40001/pkg/dev_compiler/lib/sr...
File pkg/dev_compiler/lib/src/compiler/command.dart (right):

https://codereview.chromium.org/2987393002/diff/40001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/command.dart:288: Set<String>
knownAbbreviations = new Set<String>();
On 2017/08/03 18:28:04, Bob Nystrom wrote:
> No "var"?

this is just copied code from Analyzer; obviously I'd never write that! :)

https://codereview.chromium.org/2987393002/diff/40001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/command.dart:297: for (int i = 0; i <
args.length; i++) {
On 2017/08/03 18:28:04, Bob Nystrom wrote:
> Use a for-in loop?

same as above. See TODO, I plan to remove this function and fix the one in
package:analyzer once I can land this CL

https://codereview.chromium.org/2987393002/diff/40001/tools/testing/dart/comm...
File tools/testing/dart/command_output.dart (right):

https://codereview.chromium.org/2987393002/diff/40001/tools/testing/dart/comm...
tools/testing/dart/command_output.dart:721: return exitCode != 0
On 2017/08/03 18:28:04, Bob Nystrom wrote:
> How about "== 0" and flipping the cases to avoid the mental negation?

Done.

[Bob Nystrom, 2017-08-03 20:19:49]

https://codereview.chromium.org/2987393002/diff/40001/pkg/dev_compiler/bin/da...
File pkg/dev_compiler/bin/dartdevc.dart (right):

https://codereview.chromium.org/2987393002/diff/40001/pkg/dev_compiler/bin/da...
pkg/dev_compiler/bin/dartdevc.dart:57: Stream input =
stdin.transform(UTF8.decoder).transform(new LineSplitter());
On 2017/08/03 19:43:05, Jennifer Messerly wrote:
> I don't know why they're not using `readLineSync`, that's a good idea

It was added much later in dart:io's evolution than much of the rest of the API,
so it probably wasn't there when they first wrote this code. Also, when it was
added, they didn't really tell anyone about it...

[Jennifer Messerly, 2017-08-03 21:39:50]

Description was changed from

==========
enable batch mode for dartdevc tests, also fix status so ddc bots pass
==========

to

==========
enable batch mode for dartdevc tests, also fix status so ddc bots pass

R=rnystrom@google.com

Committed:
https://github.com/dart-lang/sdk/commit/62303b67116fca0fb5efe97acf35af6afbe51a16
==========

[Jennifer Messerly, 2017-08-03 21:39:52]

Committed patchset #4 (id:60001) manually as
62303b67116fca0fb5efe97acf35af6afbe51a16 (presubmit successful).