Issue 1187103004: Allow Suites to be added to an Engine over time.

Issue 1187103004: Allow Suites to be added to an Engine over time. (Closed)

Created:
5 years, 6 months ago by nweiz

Modified:
5 years, 6 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

Allow Suites to be added to an Engine over time. Instead of providing all test suites to the Engine's constructor, the engine now has a sink that can be used to add suites over time and closed to indicate that no more suites are coming. This makes the use pattern more flexible, and is a step towards supporting a live test runner and to more memory efficiency. Reporters no longer wrap the Engine. Instead, they expose a static `watch` function which subscribes to the events the Engine emits. R=kevmoo@google.com Committed: https://github.com/dart-lang/test/commit/70fa0ceda7e6312741e650efb25e2c1f706c2706

Patch Set 1 #

Total comments: 10

Created: 5 years, 6 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+510 lines, -355 lines)			Patch
M	lib/src/executable.dart	View	5 chunks	+96 lines, -79 lines	2 comments	Download
M	lib/src/runner/browser/browser.dart	View	1 chunk	+14 lines, -0 lines	0 comments	Download
M	lib/src/runner/browser/compiler_pool.dart	View	1 chunk	+2 lines, -2 lines	0 comments	Download
M	lib/src/runner/browser/content_shell.dart	View	2 chunks	+2 lines, -4 lines	2 comments	Download
M	lib/src/runner/engine.dart	View	6 chunks	+88 lines, -47 lines	0 comments	Download
M	lib/src/runner/reporter/compact.dart	View	5 chunks	+112 lines, -105 lines	0 comments	Download
M	lib/src/runner/reporter/expanded.dart	View	5 chunks	+101 lines, -76 lines	0 comments	Download
A	lib/src/util/delegating_sink.dart	View	1 chunk	+24 lines, -0 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	+4 lines, -0 lines	0 comments	Download
M	lib/test.dart	View	2 chunks	+9 lines, -3 lines	0 comments	Download
M	pubspec.yaml	View	1 chunk	+1 line, -0 lines	2 comments	Download
M	test/io.dart	View	2 chunks	+1 line, -4 lines	2 comments	Download
M	test/runner/engine_test.dart	View	8 chunks	+41 lines, -19 lines	0 comments	Download
M	test/runner/pub_serve_test.dart	View	4 chunks	+4 lines, -4 lines	0 comments	Download
M	test/runner/signal_test.dart	View	10 chunks	+10 lines, -11 lines	2 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

kevmoo

LGTM w/ nits https://codereview.chromium.org/1187103004/diff/1/lib/src/executable.dart File lib/src/executable.dart (right): https://codereview.chromium.org/1187103004/diff/1/lib/src/executable.dart#newcode317 lib/src/executable.dart:317: }).listen((suite) => engine.suiteSink.add(suite), consider using await ...

5 years, 6 months ago (2015-06-17 22:20:04 UTC) #2

nweiz

Committed patchset #1 (id:1) manually as 70fa0ceda7e6312741e650efb25e2c1f706c2706 (presubmit successful).

5 years, 6 months ago (2015-06-17 22:30:24 UTC) #3

nweiz

5 years, 6 months ago (2015-06-17 22:41:08 UTC) #4

Message was sent while issue was closed.

https://codereview.chromium.org/1187103004/diff/1/lib/src/executable.dart
File lib/src/executable.dart (right):

https://codereview.chromium.org/1187103004/diff/1/lib/src/executable.dart#new...
lib/src/executable.dart:317: }).listen((suite) => engine.suiteSink.add(suite),
On 2015/06/17 22:20:04, kevmoo wrote:
> consider using await async here –&nbsp;won't need a completer, etc

We can't use it here, because we need to intercept errors... potentially more
than one.

https://codereview.chromium.org/1187103004/diff/1/lib/src/runner/browser/cont...
File lib/src/runner/browser/content_shell.dart (right):

https://codereview.chromium.org/1187103004/diff/1/lib/src/runner/browser/cont...
lib/src/runner/browser/content_shell.dart:8: import 'dart:convert';
On 2015/06/17 22:20:04, kevmoo wrote:
> unused import

Done.

https://codereview.chromium.org/1187103004/diff/1/pubspec.yaml
File pubspec.yaml (right):

https://codereview.chromium.org/1187103004/diff/1/pubspec.yaml#newcode11
pubspec.yaml:11: async: {git: git://github.com/dart-lang/async}
On 2015/06/17 22:20:04, kevmoo wrote:
> Is the latest published?

Not yet.

> At least a TODO to remove this before we publish the next rev

Done.

https://codereview.chromium.org/1187103004/diff/1/test/io.dart
File test/io.dart (right):

https://codereview.chromium.org/1187103004/diff/1/test/io.dart#newcode8
test/io.dart:8: import 'dart:convert';
On 2015/06/17 22:20:04, kevmoo wrote:
> unused

Done.

https://codereview.chromium.org/1187103004/diff/1/test/runner/signal_test.dart
File test/runner/signal_test.dart (right):

https://codereview.chromium.org/1187103004/diff/1/test/runner/signal_test.dar...
test/runner/signal_test.dart:9: import 'dart:convert';
On 2015/06/17 22:20:04, kevmoo wrote:
> unused

Done.

Expand Messages | Collapse Messages