Issue 1263503008: Add an Environment class that's exposed through RunnerSuite.

Issue 1263503008: Add an Environment class that's exposed through RunnerSuite. (Closed)

Created:
5 years, 4 months ago by nweiz

Modified:
5 years, 4 months ago

Reviewers:
kevmoo

CC:
reviews_dartlang.org

Base URL:
git@github.com:dart-lang/test@master

Target Ref:
refs/heads/master

Visibility:
Public.

More Reviews

Description

Add an Environment class that's exposed through RunnerSuite. This will later be filled out with methods to aid in debugging. R=kevmoo@google.com Committed: https://github.com/dart-lang/test/commit/9dd5ed200e1a30578b2e3d8ef2967a376016da08

Patch Set 1 #

Total comments: 4

Created: 5 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+113 lines, -45 lines)			Patch
M	lib/src/runner/browser/browser_manager.dart	View	4 chunks	+15 lines, -1 line	2 comments	Download
M	lib/src/runner/browser/iframe_listener.dart	View	3 chunks	+3 lines, -4 lines	0 comments	Download
A +	lib/src/runner/environment.dart	View	1 chunk	+6 lines, -1 line	0 comments	Download
M	lib/src/runner/load_suite.dart	View	3 chunks	+3 lines, -2 lines	0 comments	Download
M	lib/src/runner/loader.dart	View	3 chunks	+4 lines, -2 lines	2 comments	Download
M	lib/src/runner/runner_suite.dart	View	2 chunks	+8 lines, -13 lines	0 comments	Download
A +	lib/src/runner/vm/environment.dart	View	1 chunk	+8 lines, -1 line	0 comments	Download
M	lib/src/runner/vm/isolate_listener.dart	View	2 chunks	+3 lines, -3 lines	0 comments	Download
M	lib/src/util/io.dart	View	1 chunk	+1 line, -1 line	0 comments	Download
M	lib/src/utils.dart	View	2 chunks	+29 lines, -0 lines	0 comments	Download
M	lib/test.dart	View	2 chunks	+4 lines, -0 lines	0 comments	Download
M	test/runner/engine_test.dart	View	9 chunks	+23 lines, -9 lines	0 comments	Download
M	test/runner/load_suite_test.dart	View	7 chunks	+6 lines, -8 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

kevmoo

LGTM https://codereview.chromium.org/1263503008/diff/1/lib/src/runner/browser/browser_manager.dart File lib/src/runner/browser/browser_manager.dart (right): https://codereview.chromium.org/1263503008/diff/1/lib/src/runner/browser/browser_manager.dart#newcode162 lib/src/runner/browser/browser_manager.dart:162: final BrowserManager _manager; This field ends up being ...

5 years, 4 months ago (2015-07-30 01:37:24 UTC) #2

nweiz

Committed patchset #1 (id:1) manually as 9dd5ed200e1a30578b2e3d8ef2967a376016da08 (presubmit successful).

5 years, 4 months ago (2015-07-30 19:41:27 UTC) #3

nweiz

5 years, 4 months ago (2015-07-30 19:41:50 UTC) #4

Message was sent while issue was closed.

https://codereview.chromium.org/1263503008/diff/1/lib/src/runner/browser/brow...
File lib/src/runner/browser/browser_manager.dart (right):

https://codereview.chromium.org/1263503008/diff/1/lib/src/runner/browser/brow...
lib/src/runner/browser/browser_manager.dart:162: final BrowserManager _manager;
On 2015/07/30 01:37:24, kevmoo wrote:
> This field ends up being unused.
> 
> Uh...

Right, the whole point of this CL is to set the stage for Environment to
actually get members later on.

https://codereview.chromium.org/1263503008/diff/1/lib/src/runner/loader.dart
File lib/src/runner/loader.dart (right):

https://codereview.chromium.org/1263503008/diff/1/lib/src/runner/loader.dart#...
lib/src/runner/loader.dart:266: var suite = new RunnerSuite(const
VMEnvironment(),
On 2015/07/30 01:37:24, kevmoo wrote:
> nested awaits worry me –&nbsp;could make debugging that much more difficult.
Pop out
> if you think it's okay.

Part of the point of await is that it can be used in subexpressions. I don't
think that makes it any harder to debug than any other subexpression.

Expand Messages | Collapse Messages