test.dart: Find cdb.exe automatically from depot_tools when running Windows tests

test.dart: Find cdb.exe automatically from depot_tools when running Windows tests

cdb is used to dump a stack trace when a test times out.

Instead of relying on cdb.exe being in the PATH, find it from the installed
depot_tools.

BUG=
R=kustermann@google.com, zra@google.com

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

[Florian Schneider, 2017-02-06 21:28:25]

fschneider@google.com changed reviewers:
+ kustermann@google.com, zra@google.com

[zra, 2017-02-06 21:56:41]

https://codereview.chromium.org/2672243005/diff/40001/tools/testing/dart/test...
File tools/testing/dart/test_configurations.dart (right):

https://codereview.chromium.org/2672243005/diff/40001/tools/testing/dart/test...
tools/testing/dart/test_configurations.dart:10: import 'dart:convert';
alphabetize

https://codereview.chromium.org/2672243005/diff/40001/tools/testing/dart/test...
File tools/testing/dart/test_runner.dart (right):

https://codereview.chromium.org/2672243005/diff/40001/tools/testing/dart/test...
tools/testing/dart/test_runner.dart:1830: Map configuration;
<String, String> ?

[Florian Schneider, 2017-02-06 22:31:46]

https://codereview.chromium.org/2672243005/diff/40001/tools/testing/dart/test...
File tools/testing/dart/test_configurations.dart (right):

https://codereview.chromium.org/2672243005/diff/40001/tools/testing/dart/test...
tools/testing/dart/test_configurations.dart:10: import 'dart:convert';
On 2017/02/06 21:56:41, zra wrote:
> alphabetize

Done.

https://codereview.chromium.org/2672243005/diff/40001/tools/testing/dart/test...
File tools/testing/dart/test_runner.dart (right):

https://codereview.chromium.org/2672243005/diff/40001/tools/testing/dart/test...
tools/testing/dart/test_runner.dart:1830: Map configuration;
On 2017/02/06 21:56:41, zra wrote:
> <String, String> ?

Not sure. Existing code uses a plain Map, so I'd rather keep it this way for
consistency.

[zra, 2017-02-06 22:35:18]

lgtm but I think we should wait for Martin to review as well.

[kustermann, 2017-02-07 15:52:53]

lgtm

https://codereview.chromium.org/2672243005/diff/60001/tools/testing/dart/test...
File tools/testing/dart/test_configurations.dart (right):

https://codereview.chromium.org/2672243005/diff/60001/tools/testing/dart/test...
tools/testing/dart/test_configurations.dart:58: final VS_TOOLCHAIN_FILE = new
Path("build/win_toolchain.json");
Maybe add a comment here where the json file is coming from. Is it via DEPS /
gclient hooks?

https://codereview.chromium.org/2672243005/diff/60001/tools/testing/dart/test...
File tools/testing/dart/test_runner.dart (right):

https://codereview.chromium.org/2672243005/diff/60001/tools/testing/dart/test...
tools/testing/dart/test_runner.dart:1919: diagnostics.add("Found cdb in
$executable");
This message is confusing, because we actually don't know if we found the
executable or not.

Maybe rephrase this to "Using $executable for obtaining stack traces".

[Florian Schneider, 2017-02-07 18:21:10]

Description was changed from

==========
test.dart: Find cdb.exe automatically from depot_tools when running Windows
tests

cdb is used to dump a stack trace when a test times out.

Instead of relying on cdb.exe being in the PATH, find it from the installed
depot_tools.

BUG=
==========

to

==========
test.dart: Find cdb.exe automatically from depot_tools when running Windows
tests

cdb is used to dump a stack trace when a test times out.

Instead of relying on cdb.exe being in the PATH, find it from the installed
depot_tools.

BUG=
R=kustermann@google.com, zra@google.com

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

[Florian Schneider, 2017-02-07 18:21:12]

Committed patchset #5 (id:80001) manually as
f1d76fce1cc265bba074a9c219795ae7db568593 (presubmit successful).

[Florian Schneider, 2017-02-14 11:12:47]

Flushing addressed comments.

https://codereview.chromium.org/2672243005/diff/60001/tools/testing/dart/test...
File tools/testing/dart/test_configurations.dart (right):

https://codereview.chromium.org/2672243005/diff/60001/tools/testing/dart/test...
tools/testing/dart/test_configurations.dart:58: final VS_TOOLCHAIN_FILE = new
Path("build/win_toolchain.json");
On 2017/02/07 15:52:53, kustermann wrote:
> Maybe add a comment here where the json file is coming from. Is it via DEPS /
> gclient hooks?

Done.

https://codereview.chromium.org/2672243005/diff/60001/tools/testing/dart/test...
File tools/testing/dart/test_runner.dart (right):

https://codereview.chromium.org/2672243005/diff/60001/tools/testing/dart/test...
tools/testing/dart/test_runner.dart:1919: diagnostics.add("Found cdb in
$executable");
On 2017/02/07 15:52:53, kustermann wrote:
> This message is confusing, because we actually don't know if we found the
> executable or not.
> 
> Maybe rephrase this to "Using $executable for obtaining stack traces".

Done.

[Bill Hesse, 2017-03-01 13:45:55]

whesse@google.com changed reviewers:
+ whesse@google.com

[Bill Hesse, 2017-03-01 13:45:56]

https://codereview.chromium.org/2672243005/diff/80001/tools/testing/dart/test...
File tools/testing/dart/test_configurations.dart (right):

https://codereview.chromium.org/2672243005/diff/80001/tools/testing/dart/test...
tools/testing/dart/test_configurations.dart:303: // stack traces of tests timing
out.
It would be good to put this in a try block, so it doesn't kill the test run if
it fails.  For example, if gclient sync hasn't been run.

[Florian Schneider, 2017-03-02 22:56:08]

https://codereview.chromium.org/2672243005/diff/80001/tools/testing/dart/test...
File tools/testing/dart/test_configurations.dart (right):

https://codereview.chromium.org/2672243005/diff/80001/tools/testing/dart/test...
tools/testing/dart/test_configurations.dart:303: // stack traces of tests timing
out.
On 2017/03/01 13:45:56, Bill Hesse wrote:
> It would be good to put this in a try block, so it doesn't kill the test run
if
> it fails.  For example, if gclient sync hasn't been run.

I'll add one here.