Capture and report stdout/stderr of the browser while running a test

Capture and report stdout/stderr of the browser while running a test

Currently we have no way of getting stdout/stderr information when using the
browser controller.

This CL will capture stdout/stderr of the browser and tries to align it with the
currently executed test (which might be slightly off due to the fact that
Process.{stderr,stdout} is reported asynchronously).

R=ricow@google.com

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

[kustermann, 2013-10-18 12:02:25]

https://codereview.chromium.org/28533003/diff/1/tools/testing/dart/browser_co...
File tools/testing/dart/browser_controller.dart (left):

https://codereview.chromium.org/28533003/diff/1/tools/testing/dart/browser_co...
tools/testing/dart/browser_controller.dart:171: String get usageLog =>
_usageLog.toString();
I couldn't find any uses of these. I hope it's okay to replace them by
allBrowserOutput/testBrowserOutput.

[ricow1, 2013-10-18 14:00:42]

LGTM, very nice, I guess we should somehow surface the info from the full
browser log as well - we can still have race conditions where the browser crash
on a double reporting test.

https://codereview.chromium.org/28533003/diff/30001/tools/testing/dart/browse...
File tools/testing/dart/browser_controller.dart (right):

https://codereview.chromium.org/28533003/diff/30001/tools/testing/dart/browse...
tools/testing/dart/browser_controller.dart:834: status.currentTest = null;
shouldn't we do this right away instead of waiting for the browser to close?

https://codereview.chromium.org/28533003/diff/30001/tools/testing/dart/test_r...
File tools/testing/dart/test_runner.dart (right):

https://codereview.chromium.org/28533003/diff/30001/tools/testing/dart/test_r...
tools/testing/dart/test_runner.dart:2009: 'BrowserOutput while running the test
(this may be incorrect):',
I would prefix this line with * EXPERIMENTAL * instead of saying this may be
incorrect

https://codereview.chromium.org/28533003/diff/30001/tools/testing/dart/test_r...
tools/testing/dart/test_runner.dart:2012: .join('\n');
why do you create an array here, why don't you just use string interpolation?

[kustermann, 2013-10-18 15:00:07]

> LGTM, very nice, I guess we should somehow surface the info from the full
> browser log as well - we can still have race conditions where the browser
crash
> on a double reporting test.

Yes that is true, but I won't do it in this CL.

[kustermann, 2013-10-18 15:28:18]

Committed patchset #2 manually as r28858 (presubmit successful).

[kustermann, 2013-10-18 15:28:27]

https://codereview.chromium.org/28533003/diff/30001/tools/testing/dart/browse...
File tools/testing/dart/browser_controller.dart (right):

https://codereview.chromium.org/28533003/diff/30001/tools/testing/dart/browse...
tools/testing/dart/browser_controller.dart:834: status.currentTest = null;
On 2013/10/18 14:00:42, ricow1 wrote:
> shouldn't we do this right away instead of waiting for the browser to close?

I don't think so. We should not set the current test to 'null' before we
actually reported the result of the current test.

[And we want to wait for the browser to shutdown to capture more stdout/stderr
before reporting the result of the test].

https://codereview.chromium.org/28533003/diff/30001/tools/testing/dart/test_r...
File tools/testing/dart/test_runner.dart (right):

https://codereview.chromium.org/28533003/diff/30001/tools/testing/dart/test_r...
tools/testing/dart/test_runner.dart:2009: 'BrowserOutput while running the test
(this may be incorrect):',
On 2013/10/18 14:00:42, ricow1 wrote:
> I would prefix this line with * EXPERIMENTAL * instead of saying this may be
> incorrect

Done.

https://codereview.chromium.org/28533003/diff/30001/tools/testing/dart/test_r...
tools/testing/dart/test_runner.dart:2012: .join('\n');
On 2013/10/18 14:00:42, ricow1 wrote:
> why do you create an array here, why don't you just use string interpolation?

Done.