Add RunnerSuite.{on,is}Debugging properties.

Add RunnerSuite.{on,is}Debugging properties.

These is useful for #335. It will allow the test runner to detect when
it can pause normal command-line output and present an interface for
re-running tests.

R=kevmoo@google.com

Committed: https://github.com/dart-lang/test/commit/71bfbdcd1b89ab62df9bf3a1a6f48bffe8c63974

[kevmoo, 2015-11-06 18:13:56]

lgtm

https://codereview.chromium.org/1422963004/diff/1/lib/src/runner/browser/ifra...
File lib/src/runner/browser/iframe_listener.dart (right):

https://codereview.chromium.org/1422963004/diff/1/lib/src/runner/browser/ifra...
lib/src/runner/browser/iframe_listener.dart:44: new Timer.periodic(new
Duration(seconds: 1),
const Duration

https://codereview.chromium.org/1422963004/diff/1/lib/src/runner/browser/suit...
File lib/src/runner/browser/suite.dart (right):

https://codereview.chromium.org/1422963004/diff/1/lib/src/runner/browser/suit...
lib/src/runner/browser/suite.dart:46: var timer = new RestartableTimer(new
Duration(seconds: 3), () {
const

https://codereview.chromium.org/1422963004/diff/1/lib/src/runner/browser/suit...
lib/src/runner/browser/suite.dart:48: })..cancel();
What the immediate cancel?

https://codereview.chromium.org/1422963004/diff/1/lib/src/runner/browser/suit...
lib/src/runner/browser/suite.dart:97: Future<Map> _getResponse(Stream stream) {
This would be a bit cleaner using async with await for

https://codereview.chromium.org/1422963004/diff/1/lib/src/runner/browser/suit...
lib/src/runner/browser/suite.dart:115: Future _validateResponse(String path, Map
response) {
why not async?

You're being called from an async method – Future.error + Future.value seem
– too much

https://codereview.chromium.org/1422963004/diff/1/lib/src/runner/load_suite.dart
File lib/src/runner/load_suite.dart (right):

https://codereview.chromium.org/1422963004/diff/1/lib/src/runner/load_suite.d...
lib/src/runner/load_suite.dart:38: final environment = const VMEnvironment();
nit - might as well be getters – don't add to instance size

[nweiz, 2015-11-16 20:26:53]

Code review changes

[nweiz, 2015-11-16 20:27:00]

https://codereview.chromium.org/1422963004/diff/1/lib/src/runner/browser/ifra...
File lib/src/runner/browser/iframe_listener.dart (right):

https://codereview.chromium.org/1422963004/diff/1/lib/src/runner/browser/ifra...
lib/src/runner/browser/iframe_listener.dart:44: new Timer.periodic(new
Duration(seconds: 1),
On 2015/11/06 18:13:56, kevmoo wrote:
> const Duration

Generally we use "new" rather than "const" outside of contexts where constants
are required. I don't think there's any evidence that "const" is more efficient
in practice, and using it pervasively requires users to have to remember an
additional bit of information about every constructor.

https://codereview.chromium.org/1422963004/diff/1/lib/src/runner/browser/suit...
File lib/src/runner/browser/suite.dart (right):

https://codereview.chromium.org/1422963004/diff/1/lib/src/runner/browser/suit...
lib/src/runner/browser/suite.dart:48: })..cancel();
On 2015/11/06 18:13:56, kevmoo wrote:
> What the immediate cancel?

Commented.

https://codereview.chromium.org/1422963004/diff/1/lib/src/runner/browser/suit...
lib/src/runner/browser/suite.dart:97: Future<Map> _getResponse(Stream stream) {
On 2015/11/06 18:13:56, kevmoo wrote:
> This would be a bit cleaner using async with await for

I don't think that would allow it to be restructured at all, which means it
would just add tokens and overhead for no value.

https://codereview.chromium.org/1422963004/diff/1/lib/src/runner/browser/suit...
lib/src/runner/browser/suite.dart:115: Future _validateResponse(String path, Map
response) {
On 2015/11/06 18:13:56, kevmoo wrote:
> why not async?
> 
> You're being called from an async method –&nbsp;Future.error + Future.value
seem
> –&nbsp;too much

Why would it be async? We shouldn't just use async in every function that
returns a future, we should use it in functions that actually get structural
benefits from it.

https://codereview.chromium.org/1422963004/diff/1/lib/src/runner/load_suite.dart
File lib/src/runner/load_suite.dart (right):

https://codereview.chromium.org/1422963004/diff/1/lib/src/runner/load_suite.d...
lib/src/runner/load_suite.dart:38: final environment = const VMEnvironment();
On 2015/11/06 18:13:56, kevmoo wrote:
> nit -&nbsp;might as well be getters –&nbsp;don't add to instance size

The implementation can easily determine that final fields with constant values
aren't going to change. It can represent them internally however it thinks will
be most efficient, if the VM developers believe that representation will make a
difference.

I'm writing this code to be read, and it's more readable with fewer extra
characters and duplicated type annotations. I'm not going to make it less
readable for a theoretical small efficiency improvement that could be done at a
lower level anyway.

[kevmoo, 2015-11-16 23:21:23]

LGTM w/ a question

https://codereview.chromium.org/1422963004/diff/1/lib/src/runner/browser/suit...
File lib/src/runner/browser/suite.dart (right):

https://codereview.chromium.org/1422963004/diff/1/lib/src/runner/browser/suit...
lib/src/runner/browser/suite.dart:115: Future _validateResponse(String path, Map
response) {
On 2015/11/16 20:27:00, nweiz wrote:
> On 2015/11/06 18:13:56, kevmoo wrote:
> > why not async?
> > 
> > You're being called from an async method – Future.error + Future.value
> seem
> > – too much
> 
> Why would it be async? We shouldn't just use async in every function that
> returns a future, we should use it in functions that actually get structural
> benefits from it.

Consistency is nice. As is maintenance.

It's not clear why this method throws both sync and async errors.

[nweiz, 2015-11-16 23:48:14]

https://codereview.chromium.org/1422963004/diff/1/lib/src/runner/browser/suit...
File lib/src/runner/browser/suite.dart (right):

https://codereview.chromium.org/1422963004/diff/1/lib/src/runner/browser/suit...
lib/src/runner/browser/suite.dart:115: Future _validateResponse(String path, Map
response) {
On 2015/11/16 23:21:23, kevmoo wrote:
> Consistency is nice. As is maintenance.

We consistently only use async where it provides benefit. That's also consistent
with how we use any other feature.

> It's not clear why this method throws both sync and async errors.

The only async error is the one that has to attach a specific stack trace.

[nweiz, 2015-11-16 23:48:40]

Committed patchset #2 (id:20001) manually as
71bfbdcd1b89ab62df9bf3a1a6f48bffe8c63974 (presubmit successful).