Call DumpRenderTree directly from test.dart instead of using the drt-trampoline.py indirection

Call DumpRenderTree directly from test.dart instead of using the drt-trampoline.py indirection

Currently drt-trampoline.py is used to call DRT. Unfortunately this causes
problems on Windows: If drt-trampoline.py times out, it is killed by test.dart.
But drt-trampoline.py has no way of installing a signal handler on Windows which
would kill the actual DumpRenderTree process. So sometimes we're left with one
hanging DumpRenderTree process on the Windows buildbots.

This change removes the drt-trampoline.py indirection and calls DRT directly
from test.dart.

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

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

[kustermann, 2012-12-17 12:10:17]

This code is not completely cleaned up, but please take a look nonetheless.

[ricow1, 2012-12-17 13:45:56]

As discussed offline we could make this much nicer by refactoring the
command/commandoutput classes. I don't have strong opinions about if we do that
in this cl or in another

https://codereview.chromium.org/11586012/diff/1/tools/testing/dart/test_runne...
File tools/testing/dart/test_runner.dart (right):

https://codereview.chromium.org/11586012/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:56: bool get isPixelTest => false;
file a bug for us to make this nicer by using polymorphic calls to get the right
output (it is not currently possible with the structure we have for commands and
commandoutput)

https://codereview.chromium.org/11586012/diff/1/tools/testing/dart/test_suite...
File tools/testing/dart/test_suite.dart (right):

https://codereview.chromium.org/11586012/diff/1/tools/testing/dart/test_suite...
tools/testing/dart/test_suite.dart:792: Map optionsFromFile =
info.optionsFromFile;
Add todo here to tell us to refactor this method (don't refactor in this cl)

https://codereview.chromium.org/11586012/diff/1/tools/testing/dart/test_suite...
tools/testing/dart/test_suite.dart:917: List<Command> commandSet = new
List<Command>.from(commands);
add comment describing what we use this for

[kustermann, 2012-12-17 14:00:47]

PTAL

https://codereview.chromium.org/11586012/diff/1/tools/testing/dart/test_runne...
File tools/testing/dart/test_runner.dart (right):

https://codereview.chromium.org/11586012/diff/1/tools/testing/dart/test_runne...
tools/testing/dart/test_runner.dart:56: bool get isPixelTest => false;
On 2012/12/17 13:45:57, ricow1 wrote:
> file a bug for us to make this nicer by using polymorphic calls to get the
right
> output (it is not currently possible with the structure we have for commands
and
> commandoutput)

Done. Issue 7453

https://codereview.chromium.org/11586012/diff/1/tools/testing/dart/test_suite...
File tools/testing/dart/test_suite.dart (right):

https://codereview.chromium.org/11586012/diff/1/tools/testing/dart/test_suite...
tools/testing/dart/test_suite.dart:792: Map optionsFromFile =
info.optionsFromFile;
On 2012/12/17 13:45:57, ricow1 wrote:
> Add todo here to tell us to refactor this method (don't refactor in this cl)

Done.

https://codereview.chromium.org/11586012/diff/1/tools/testing/dart/test_suite...
tools/testing/dart/test_suite.dart:917: List<Command> commandSet = new
List<Command>.from(commands);
On 2012/12/17 13:45:57, ricow1 wrote:
> add comment describing what we use this for

Done.

[ricow1, 2012-12-17 14:20:54]

LGTM
Lets file a bug for getting this cleaned up and make a high priority.

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

https://codereview.chromium.org/11586012/diff/3001/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:578: // We are either doing a pixel test or
a layout test with DumpRenderTree
move the body of this file out into a seperate method(s)

https://codereview.chromium.org/11586012/diff/3001/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:596: bool areByteArraysEqual(List<int>
buffer1,
doc style comment explaining what this does

https://codereview.chromium.org/11586012/diff/3001/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:606: for (var i=0; i<count; i++) {
space around =, space around <

https://codereview.chromium.org/11586012/diff/3001/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:909: print('');
I think the earlier print might have been here for a reason, lets not change
this until we figure out why we put it in in the first place, try asking peter
or bill

[kustermann, 2012-12-17 16:11:18]

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

https://codereview.chromium.org/11586012/diff/3001/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:578: // We are either doing a pixel test or
a layout test with DumpRenderTree
On 2012/12/17 14:20:54, ricow1 wrote:
> move the body of this file out into a seperate method(s)

Done.

https://codereview.chromium.org/11586012/diff/3001/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:596: bool areByteArraysEqual(List<int>
buffer1,
On 2012/12/17 14:20:54, ricow1 wrote:
> doc style comment explaining what this does

Done.

https://codereview.chromium.org/11586012/diff/3001/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:606: for (var i=0; i<count; i++) {
On 2012/12/17 14:20:54, ricow1 wrote:
> space around =, space around <

Done.

https://codereview.chromium.org/11586012/diff/3001/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:909: print('');
On 2012/12/17 14:20:54, ricow1 wrote:
> I think the earlier print might have been here for a reason, lets not change
> this until we figure out why we put it in in the first place, try asking peter
> or bill

Done.

[kustermann, 2012-12-17 17:53:24]

PTAL

Patch set 4 fixes two bugs related to environment variables. Unfortunately I
didn't catch them with my local testing.

@ahe: I added you now as a reviewer, since it seems you've some knowledge about
the "drt-trampoline.py" issue as well.

[ricow1, 2012-12-18 09:42:22]

LGTM
I should have seen the env issue, we will definitely not work on the buildbot if
we "delete" the existing ones