Clean up test.dart framework.

Fix usesWebDriver check, and the usage of config['browser'] vs config['runtime']
Committed: https://code.google.com/p/dart/source/detail?r=14110

[Emily Fortuna, 2012-10-25 01:11:26]

Turns out this change I thought I had committed was tucked away in a git branch.


This also reapplies https://codereview.chromium.org/11098075/ -- it works
locally, so I'll be checking carefully to see if this fails on the bot again

The reason we were getting those random timeouts actually had nothing to do with
the framework! The IE driver is not threadsafe, so only one browser/test can be
open at a time. This is controlled in tools/bots/compiler.py, but when we
renamed the bots to ie9, the script was still checking for "ie" in one place,
and hence running multiple IE windows, causing problems. My sincerest apologies
for the redness and confusion.

[Mads Ager (google), 2012-10-25 08:11:54]

LGTM

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

https://codereview.chromium.org/11275025/diff/5001/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:80: bool usesWebDriver;
I'm not sure what we gain by having this as a member instead of the getter. It
seems to be a lot more code for the same functionality?

https://codereview.chromium.org/11275025/diff/5001/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:1145: browserUsed =
test.configuration['runtime'];
Thank you! I hope this will work out on the bots.

[Emily Fortuna, 2012-10-25 18:21:20]

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

https://codereview.chromium.org/11275025/diff/5001/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:80: bool usesWebDriver;
On 2012/10/25 08:11:55, Mads Ager wrote:
> I'm not sure what we gain by having this as a member instead of the getter. It
> seems to be a lot more code for the same functionality?

I agree. The reason I changed this is I was finding I was having to add "ie10"
in an awful lot of extra places sprinkled throughout our test framework, so this
was an attempt to avoid that. But perhaps the extra spots are worth having less
code.?

[Mads Ager (google), 2012-10-25 18:47:44]

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

https://codereview.chromium.org/11275025/diff/5001/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:80: bool usesWebDriver;
On 2012/10/25 18:21:20, Emily Fortuna wrote:
> On 2012/10/25 08:11:55, Mads Ager wrote:
> > I'm not sure what we gain by having this as a member instead of the getter.
It
> > seems to be a lot more code for the same functionality?
> 
> I agree. The reason I changed this is I was finding I was having to add "ie10"
> in an awful lot of extra places sprinkled throughout our test framework, so
this
> was an attempt to avoid that. But perhaps the extra spots are worth having
less
> code.?

I strongly agree with your use of TestUtils.usesWebDriver as the one place to
have the list of browser runtimes!

What I was wondering is if it would be better to just keep the getter on line
167 of the 'before' part of this change list and not introduce the argument to
the constructor and the field. It seems to me that the getter will give the
correct result and that the amount of places where we have the string 'ie9' will
be the same (no changed line in this CL contains browser names as strings)? That
would reduce this patch set to only the change to fix the use of 'browserUsed'.
I'm fine either way so I'll leave it to your judgement.