Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(971)

Issue 2672243005: test.dart: Find cdb.exe automatically from depot_tools when running Windows tests (Closed)

Created:
3 years, 10 months ago by Florian Schneider
Modified:
3 years, 9 months ago
Reviewers:
zra, Bill Hesse, kustermann
CC:
reviews_dartlang.org
Target Ref:
refs/heads/master
Visibility:
Public.

Description

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

Patch Set 1 #

Patch Set 2 : formatting #

Patch Set 3 : fix 32/64 bit distinction #

Total comments: 4

Patch Set 4 : address comment #

Total comments: 4

Patch Set 5 : add comments #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+26 lines, -4 lines) Patch
M tools/testing/dart/test_configurations.dart View 1 2 3 4 3 chunks +11 lines, -0 lines 2 comments Download
M tools/testing/dart/test_runner.dart View 1 2 3 4 3 chunks +15 lines, -4 lines 0 comments Download

Messages

Total messages: 12 (3 generated)
Florian Schneider
3 years, 10 months ago (2017-02-06 21:28:26 UTC) #2
zra
https://codereview.chromium.org/2672243005/diff/40001/tools/testing/dart/test_configurations.dart File tools/testing/dart/test_configurations.dart (right): https://codereview.chromium.org/2672243005/diff/40001/tools/testing/dart/test_configurations.dart#newcode10 tools/testing/dart/test_configurations.dart:10: import 'dart:convert'; alphabetize https://codereview.chromium.org/2672243005/diff/40001/tools/testing/dart/test_runner.dart File tools/testing/dart/test_runner.dart (right): https://codereview.chromium.org/2672243005/diff/40001/tools/testing/dart/test_runner.dart#newcode1830 tools/testing/dart/test_runner.dart:1830: ...
3 years, 10 months ago (2017-02-06 21:56:41 UTC) #3
Florian Schneider
https://codereview.chromium.org/2672243005/diff/40001/tools/testing/dart/test_configurations.dart File tools/testing/dart/test_configurations.dart (right): https://codereview.chromium.org/2672243005/diff/40001/tools/testing/dart/test_configurations.dart#newcode10 tools/testing/dart/test_configurations.dart:10: import 'dart:convert'; On 2017/02/06 21:56:41, zra wrote: > alphabetize ...
3 years, 10 months ago (2017-02-06 22:31:46 UTC) #4
zra
lgtm but I think we should wait for Martin to review as well.
3 years, 10 months ago (2017-02-06 22:35:18 UTC) #5
kustermann
lgtm https://codereview.chromium.org/2672243005/diff/60001/tools/testing/dart/test_configurations.dart File tools/testing/dart/test_configurations.dart (right): https://codereview.chromium.org/2672243005/diff/60001/tools/testing/dart/test_configurations.dart#newcode58 tools/testing/dart/test_configurations.dart:58: final VS_TOOLCHAIN_FILE = new Path("build/win_toolchain.json"); Maybe add a ...
3 years, 10 months ago (2017-02-07 15:52:53 UTC) #6
Florian Schneider
Committed patchset #5 (id:80001) manually as f1d76fce1cc265bba074a9c219795ae7db568593 (presubmit successful).
3 years, 10 months ago (2017-02-07 18:21:12 UTC) #8
Florian Schneider
Flushing addressed comments. https://codereview.chromium.org/2672243005/diff/60001/tools/testing/dart/test_configurations.dart File tools/testing/dart/test_configurations.dart (right): https://codereview.chromium.org/2672243005/diff/60001/tools/testing/dart/test_configurations.dart#newcode58 tools/testing/dart/test_configurations.dart:58: final VS_TOOLCHAIN_FILE = new Path("build/win_toolchain.json"); On ...
3 years, 10 months ago (2017-02-14 11:12:47 UTC) #9
Bill Hesse
https://codereview.chromium.org/2672243005/diff/80001/tools/testing/dart/test_configurations.dart File tools/testing/dart/test_configurations.dart (right): https://codereview.chromium.org/2672243005/diff/80001/tools/testing/dart/test_configurations.dart#newcode303 tools/testing/dart/test_configurations.dart:303: // stack traces of tests timing out. It would ...
3 years, 9 months ago (2017-03-01 13:45:56 UTC) #11
Florian Schneider
3 years, 9 months ago (2017-03-02 22:56:08 UTC) #12
Message was sent while issue was closed.
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.

Powered by Google App Engine
This is Rietveld 408576698