From ed3c6b5ada674489766f7501a28c4e5a25e24724 Mon Sep 17 00:00:00 2001
From: Natalie Weizenbaum <nex342@gmail.com>
Date: Thu, 29 Sep 2016 15:55:16 -0700
Subject: [PATCH] Refactor Configuration. (#473)
This introduces a zone-scoped `Configuration.current`. This makes it
much easier to propagate the configuration everywhere it needs to go,
especially to plugins.
---
lib/src/runner.dart | 37 ++++++-------
lib/src/runner/browser/compiler_pool.dart | 10 ++--
lib/src/runner/browser/platform.dart | 27 +++++-----
lib/src/runner/configuration.dart | 18 +++++++
lib/src/runner/debugger.dart | 15 +++---
lib/src/runner/loader.dart | 17 +++---
lib/src/runner/reporter/compact.dart | 64 ++++++-----------------
lib/src/runner/reporter/expanded.dart | 2 +
lib/src/runner/reporter/json.dart | 36 ++++---------
lib/src/runner/vm/platform.dart | 4 +-
test/runner/browser/loader_test.dart | 10 ++--
test/runner/loader_test.dart | 4 +-
12 files changed, 106 insertions(+), 138 deletions(-)
diff --git a/lib/src/runner.dart b/lib/src/runner.dart
index 5fda1aeb..67343efb 100644
--- a/lib/src/runner.dart
+++ b/lib/src/runner.dart
@@ -34,11 +34,11 @@ import 'utils.dart';
/// the other, as well as printing out tests with a [CompactReporter] or an
/// [ExpandedReporter].
class Runner {
- /// The configuration for the runner.
- final Configuration _config;
+ /// The test runner configuration.
+ final _config = Configuration.current;
/// The loader that loads the test suites from the filesystem.
- final Loader _loader;
+ final _loader = new Loader();
/// The engine that runs the test suites.
final Engine _engine;
@@ -66,21 +66,15 @@ class Runner {
bool get _closed => _closeMemo.hasRun;
/// Creates a new runner based on [configuration].
- factory Runner(Configuration config) {
- var loader = new Loader(config);
+ factory Runner(Configuration config) => config.asCurrent(() {
var engine = new Engine(
concurrency: config.concurrency,
runSkipped: config.runSkipped);
var reporter;
switch (config.reporter) {
- case "compact":
case "expanded":
- var watch = config.reporter == "compact"
- ? CompactReporter.watch
- : ExpandedReporter.watch;
-
- reporter = watch(
+ reporter = ExpandedReporter.watch(
engine,
color: config.color,
verboseTrace: config.verboseTrace,
@@ -89,24 +83,25 @@ class Runner {
printPlatform: config.platforms.length > 1);
break;
+ case "compact":
+ reporter = CompactReporter.watch(engine);
+ break;
+
case "json":
- reporter = JsonReporter.watch(engine,
- verboseTrace: config.verboseTrace,
- jsLocations: !config.jsTrace,
- runSkipped: config.runSkipped);
+ reporter = JsonReporter.watch(engine);
break;
}
- return new Runner._(config, loader, engine, reporter);
- }
+ return new Runner._(engine, reporter);
+ });
- Runner._(this._config, this._loader, this._engine, this._reporter);
+ Runner._(this._engine, this._reporter);
/// Starts the runner.
///
/// This starts running tests and printing their progress. It returns whether
/// or not they ran successfully.
- Future<bool> run() async {
+ Future<bool> run() => _config.asCurrent(() async {
if (_closed) {
throw new StateError("run() may not be called on a closed Runner.");
}
@@ -142,7 +137,7 @@ class Runner {
// Explicitly check "== true" here because [Engine.run] can return `null`
// if the engine was closed prematurely.
return success == true;
- }
+ });
/// Emits a warning if the user is trying to run on a platform that's
/// unsupported for the entire package.
@@ -374,7 +369,7 @@ class Runner {
}
_suiteSubscription = suites.asyncMap((loadSuite) async {
- _debugOperation = debug(_config, _engine, _reporter, loadSuite);
+ _debugOperation = debug(_engine, _reporter, loadSuite);
await _debugOperation.valueOrCancellation();
}).listen(null);
diff --git a/lib/src/runner/browser/compiler_pool.dart b/lib/src/runner/browser/compiler_pool.dart
index 39b32f35..2c444a67 100644
--- a/lib/src/runner/browser/compiler_pool.dart
+++ b/lib/src/runner/browser/compiler_pool.dart
@@ -23,12 +23,12 @@ final _dart2jsStatus =
///
/// This limits the number of compiler instances running concurrently.
class CompilerPool {
+ /// The test runner configuration.
+ final _config = Configuration.current;
+
/// The internal pool that controls the number of process running at once.
final Pool _pool;
- /// The test runner configuration.
- final Configuration _config;
-
/// The currently-active dart2js processes.
final _processes = new Set<Process>();
@@ -39,9 +39,7 @@ class CompilerPool {
final _closeMemo = new AsyncMemoizer();
/// Creates a compiler pool that multiple instances of `dart2js` at once.
- CompilerPool(Configuration config)
- : _pool = new Pool(config.concurrency),
- _config = config;
+ CompilerPool() : _pool = new Pool(Configuration.current.concurrency);
/// Compile the Dart code at [dartPath] to [jsPath].
///
diff --git a/lib/src/runner/browser/platform.dart b/lib/src/runner/browser/platform.dart
index 313cdaa2..656912fb 100644
--- a/lib/src/runner/browser/platform.dart
+++ b/lib/src/runner/browser/platform.dart
@@ -39,12 +39,15 @@ class BrowserPlatform extends PlatformPlugin {
///
/// [root] is the root directory that the server should serve. It defaults to
/// the working directory.
- static Future<BrowserPlatform> start(Configuration config, {String root})
+ static Future<BrowserPlatform> start({String root})
async {
var server = new shelf_io.IOServer(await HttpMultiServer.loopback(0));
- return new BrowserPlatform._(server, config, root: root);
+ return new BrowserPlatform._(server, root: root);
}
+ /// The test runner configuration.
+ final _config = Configuration.current;
+
/// The underlying server.
final shelf.Server _server;
@@ -57,9 +60,6 @@ class BrowserPlatform extends PlatformPlugin {
/// The URL for this server.
Uri get url => _server.url.resolve(_secret + "/");
- /// The test runner configuration.
- Configuration _config;
-
/// A [OneOffHandler] for servicing WebSocket connections for
/// [BrowserManager]s.
///
@@ -111,12 +111,15 @@ class BrowserPlatform extends PlatformPlugin {
/// Mappers for Dartifying stack traces, indexed by test path.
final _mappers = new Map<String, StackTraceMapper>();
- BrowserPlatform._(this._server, Configuration config, {String root})
+ BrowserPlatform._(this._server, {String root})
: _root = root == null ? p.current : root,
- _config = config,
- _compiledDir = config.pubServeUrl == null ? createTempDir() : null,
- _http = config.pubServeUrl == null ? null : new HttpClient(),
- _compilers = new CompilerPool(config) {
+ _compiledDir = Configuration.current.pubServeUrl == null
+ ? createTempDir()
+ : null,
+ _http = Configuration.current.pubServeUrl == null
+ ? null
+ : new HttpClient(),
+ _compilers = new CompilerPool() {
var cascade = new shelf.Cascade()
.add(_webSocketHandler.handler);
@@ -127,8 +130,8 @@ class BrowserPlatform extends PlatformPlugin {
.add(createStaticHandler(_root))
.add(_wrapperHandler);
- if (config.precompiledPath != null) {
- cascade = cascade.add(createStaticHandler(config.precompiledPath));
+ if (_config.precompiledPath != null) {
+ cascade = cascade.add(createStaticHandler(_config.precompiledPath));
}
}
diff --git a/lib/src/runner/configuration.dart b/lib/src/runner/configuration.dart
index e31b0666..7c3ed3bf 100644
--- a/lib/src/runner/configuration.dart
+++ b/lib/src/runner/configuration.dart
@@ -2,6 +2,7 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
+import 'dart:async';
import 'dart:io';
import 'package:boolean_selector/boolean_selector.dart';
@@ -18,6 +19,9 @@ import 'configuration/args.dart' as args;
import 'configuration/load.dart';
import 'configuration/values.dart';
+/// The key used to look up [Configuration.current] in a zone.
+final _currentKey = new Object();
+
/// A class that encapsulates the command-line configuration of the test runner.
class Configuration {
/// An empty configuration with only default values.
@@ -253,6 +257,13 @@ class Configuration {
yield* presets.values;
}
+ /// Returns the current configuration, or a default configuration if no
+ /// current configuration is set.
+ ///
+ /// The current configuration is set using [asCurrent].
+ static Configuration get current =>
+ Zone.current[_currentKey] ?? new Configuration();
+
/// Parses the configuration from [args].
///
/// Throws a [FormatException] if [args] are invalid.
@@ -463,6 +474,13 @@ class Configuration {
return new Map.unmodifiable(input);
}
+ /// Runs [body] with [this] as [Configuration.current].
+ ///
+ /// This is zone-scoped, so [this] will be the current configuration in any
+ /// asynchronous callbacks transitively created by [body].
+ /*=T*/ asCurrent/*<T>*/(/*=T*/ body()) =>
+ runZoned(body, zoneValues: {_currentKey: this});
+
/// Merges this with [other].
///
/// For most fields, if both configurations have values set, [other]'s value
diff --git a/lib/src/runner/debugger.dart b/lib/src/runner/debugger.dart
index 6e850c63..542ad0a5 100644
--- a/lib/src/runner/debugger.dart
+++ b/lib/src/runner/debugger.dart
@@ -24,8 +24,8 @@ import 'runner_suite.dart';
/// Returns a [CancelableOperation] that will complete once the suite has
/// finished running. If the operation is canceled, the debugger will clean up
/// any resources it allocated.
-CancelableOperation debug(Configuration config, Engine engine,
- Reporter reporter, LoadSuite loadSuite) {
+CancelableOperation debug(Engine engine, Reporter reporter,
+ LoadSuite loadSuite) {
var debugger;
var canceled = false;
return new CancelableOperation.fromFuture(() async {
@@ -36,7 +36,7 @@ CancelableOperation debug(Configuration config, Engine engine,
var suite = await loadSuite.suite;
if (canceled || suite == null) return;
- debugger = new _Debugger(config, engine, reporter, suite);
+ debugger = new _Debugger(engine, reporter, suite);
await debugger.run();
}(), onCancel: () {
canceled = true;
@@ -49,8 +49,8 @@ CancelableOperation debug(Configuration config, Engine engine,
// breakpoints.
/// A debugger for a single test suite.
class _Debugger {
- /// The user configuration for the test runner.
- final Configuration _config;
+ /// The test runner configuration.
+ final _config = Configuration.current;
/// The engine that will run the suite.
final Engine _engine;
@@ -73,9 +73,8 @@ class _Debugger {
/// Whether [close] has been called.
bool _closed = false;
- _Debugger(Configuration config, this._engine, this._reporter, this._suite)
- : _config = config,
- _console = new Console(color: config.color) {
+ _Debugger(this._engine, this._reporter, this._suite)
+ : _console = new Console(color: Configuration.current.color) {
_console.registerCommand(
"restart", "Restart the current test after it finishes running.",
_restartTest);
diff --git a/lib/src/runner/loader.dart b/lib/src/runner/loader.dart
index 608a7b92..6756b40b 100644
--- a/lib/src/runner/loader.dart
+++ b/lib/src/runner/loader.dart
@@ -29,7 +29,7 @@ import 'vm/platform.dart';
/// A class for finding test files and loading them into a runnable form.
class Loader {
/// The test runner configuration.
- final Configuration _config;
+ final _config = Configuration.current;
/// All suites that have been created by the loader.
final _suites = new Set<RunnerSuite>();
@@ -42,12 +42,13 @@ class Loader {
/// These are passed to the plugins' async memoizers when a plugin is needed.
final _platformCallbacks = <TestPlatform, AsyncFunction>{};
- /// Creates a new loader that loads tests on platforms defined in [_config].
+ /// Creates a new loader that loads tests on platforms defined in
+ /// [Configuration.current].
///
/// [root] is the root directory that will be served for browser tests. It
/// defaults to the working directory.
- Loader(this._config, {String root}) {
- registerPlatformPlugin([TestPlatform.vm], () => new VMPlatform(_config));
+ Loader({String root}) {
+ registerPlatformPlugin([TestPlatform.vm], () => new VMPlatform());
registerPlatformPlugin([
TestPlatform.dartium,
TestPlatform.contentShell,
@@ -56,7 +57,7 @@ class Loader {
TestPlatform.firefox,
TestPlatform.safari,
TestPlatform.internetExplorer
- ], () => BrowserPlatform.start(_config, root: root));
+ ], () => BrowserPlatform.start(root: root));
platformCallbacks.forEach((platform, plugin) {
registerPlatformPlugin([platform], plugin);
@@ -110,7 +111,9 @@ class Loader {
/// [RunnerSuite]s defined in the file.
///
/// This will emit a [LoadException] if the file fails to load.
- Stream<LoadSuite> loadFile(String path) async* {
+ Stream<LoadSuite> loadFile(String path) =>
+ // Ensure that the right config is current when invoking platform plugins.
+ _config.asCurrent(() async* {
var suiteMetadata;
try {
suiteMetadata = parseMetadata(path);
@@ -162,7 +165,7 @@ class Loader {
}
}, path: path, platform: platform);
}
- }
+ });
Future closeEphemeral() async {
await Future.wait(_platformPlugins.values.map((memo) async {
diff --git a/lib/src/runner/reporter/compact.dart b/lib/src/runner/reporter/compact.dart
index 0222e71a..6de11d94 100644
--- a/lib/src/runner/reporter/compact.dart
+++ b/lib/src/runner/reporter/compact.dart
@@ -11,6 +11,7 @@ import '../../backend/message.dart';
import '../../backend/state.dart';
import '../../utils.dart';
import '../../utils.dart' as utils;
+import '../configuration.dart';
import '../engine.dart';
import '../load_exception.dart';
import '../load_suite.dart';
@@ -24,45 +25,39 @@ const _lineLength = 100;
/// A reporter that prints test results to the console in a single
/// continuously-updating line.
class CompactReporter implements Reporter {
- /// Whether the reporter should emit terminal color escapes.
- final bool _color;
+ final _config = Configuration.current;
/// The terminal escape for green text, or the empty string if this is Windows
/// or not outputting to a terminal.
- final String _green;
+ String get _green => _config.color ? '\u001b[32m' : '';
/// The terminal escape for red text, or the empty string if this is Windows
/// or not outputting to a terminal.
- final String _red;
+ String get _red => _config.color ? '\u001b[31m' : '';
/// The terminal escape for yellow text, or the empty string if this is
/// Windows or not outputting to a terminal.
- final String _yellow;
+ String get _yellow => _config.color ? '\u001b[33m' : '';
/// The terminal escape for gray text, or the empty string if this is
/// Windows or not outputting to a terminal.
- final String _gray;
+ String get _gray => _config.color ? '\u001b[1;30m' : '';
/// The terminal escape for bold text, or the empty string if this is
/// Windows or not outputting to a terminal.
- final String _bold;
+ String get _bold => _config.color ? '\u001b[1m' : '';
/// The terminal escape for removing test coloring, or the empty string if
/// this is Windows or not outputting to a terminal.
- final String _noColor;
+ String get _noColor => _config.color ? '\u001b[0m' : '';
- /// Whether to use verbose stack traces.
- final bool _verboseTrace;
+ /// Whether the path to each test's suite should be printed.
+ final bool _printPath = Configuration.current.paths.length > 1 ||
+ new Directory(Configuration.current.paths.single).existsSync();
/// The engine used to run the tests.
final Engine _engine;
- /// Whether the path to each test's suite should be printed.
- final bool _printPath;
-
- /// Whether the platform each test is running on should be printed.
- final bool _printPlatform;
-
/// A stopwatch that tracks the duration of the full run.
final _stopwatch = new Stopwatch();
@@ -105,35 +100,10 @@ class CompactReporter implements Reporter {
/// Watches the tests run by [engine] and prints their results to the
/// terminal.
- ///
- /// If [color] is `true`, this will use terminal colors; if it's `false`, it
- /// won't. If [verboseTrace] is `true`, this will print core library frames.
- /// If [printPath] is `true`, this will print the path name as part of the
- /// test description. Likewise, if [printPlatform] is `true`, this will print
- /// the platform as part of the test description.
- static CompactReporter watch(Engine engine, {bool color: true,
- bool verboseTrace: false, bool printPath: true,
- bool printPlatform: true}) {
- return new CompactReporter._(
- engine,
- color: color,
- verboseTrace: verboseTrace,
- printPath: printPath,
- printPlatform: printPlatform);
- }
+ static CompactReporter watch(Engine engine) =>
+ new CompactReporter._(engine);
- CompactReporter._(this._engine, {bool color: true, bool verboseTrace: false,
- bool printPath: true, bool printPlatform: true})
- : _verboseTrace = verboseTrace,
- _printPath = printPath,
- _printPlatform = printPlatform,
- _color = color,
- _green = color ? '\u001b[32m' : '',
- _red = color ? '\u001b[31m' : '',
- _yellow = color ? '\u001b[33m' : '',
- _gray = color ? '\u001b[1;30m' : '',
- _bold = color ? '\u001b[1m' : '',
- _noColor = color ? '\u001b[0m' : '' {
+ CompactReporter._(this._engine) {
_subscriptions.add(_engine.onTestStarted.listen(_onTestStarted));
/// Convert the future to a stream so that the subscription can be paused or
@@ -234,12 +204,12 @@ class CompactReporter implements Reporter {
if (error is! LoadException) {
print(indent(error.toString()));
- var chain = terseChain(stackTrace, verbose: _verboseTrace);
+ var chain = terseChain(stackTrace, verbose: _config.verboseTrace);
print(indent(chain.toString()));
return;
}
- print(indent(error.toString(color: _color)));
+ print(indent(error.toString(color: _config.color)));
// Only print stack traces for load errors that come from the user's code.
if (error.innerError is! IOException &&
@@ -380,7 +350,7 @@ class CompactReporter implements Reporter {
name = "${liveTest.suite.path}: $name";
}
- if (_printPlatform && liveTest.suite.platform != null) {
+ if (_config.platforms.length > 1 && liveTest.suite.platform != null) {
name = "[${liveTest.suite.platform.name}] $name";
}
diff --git a/lib/src/runner/reporter/expanded.dart b/lib/src/runner/reporter/expanded.dart
index 9220cd87..5fba7cf9 100644
--- a/lib/src/runner/reporter/expanded.dart
+++ b/lib/src/runner/reporter/expanded.dart
@@ -95,6 +95,8 @@ class ExpandedReporter implements Reporter {
/// The set of all subscriptions to various streams.
final _subscriptions = new Set<StreamSubscription>();
+ // TODO(nweiz): Get configuration from [Configuration.current] once we have
+ // cross-platform imports.
/// Watches the tests run by [engine] and prints their results to the
/// terminal.
///
diff --git a/lib/src/runner/reporter/json.dart b/lib/src/runner/reporter/json.dart
index 1d6be91b..94acb0cc 100644
--- a/lib/src/runner/reporter/json.dart
+++ b/lib/src/runner/reporter/json.dart
@@ -14,6 +14,7 @@ import '../../backend/suite.dart';
import '../../backend/test_platform.dart';
import '../../frontend/expect.dart';
import '../../utils.dart';
+import '../configuration.dart';
import '../engine.dart';
import '../load_suite.dart';
import '../reporter.dart';
@@ -21,14 +22,8 @@ import '../version.dart';
/// A reporter that prints machine-readable JSON-formatted test results.
class JsonReporter implements Reporter {
- /// Whether to use verbose stack traces.
- final bool _verboseTrace;
-
- /// Whether to emit location information for JS tests.
- final bool _jsLocations;
-
- /// Whether skipped tests are run anyway.
- final bool _runSkipped;
+ /// The test runner configuration.
+ final _config = Configuration.current;
/// The engine used to run the tests.
final Engine _engine;
@@ -61,23 +56,9 @@ class JsonReporter implements Reporter {
var _nextID = 0;
/// Watches the tests run by [engine] and prints their results as JSON.
- ///
- /// If [verboseTrace] is `true`, this will print core library frames. If
- /// [jsLocations] is `false`, this will not emit location information for JS
- /// tests.
- static JsonReporter watch(Engine engine, {bool verboseTrace: false,
- bool jsLocations: true, bool runSkipped: false}) {
- return new JsonReporter._(engine,
- verboseTrace: verboseTrace,
- jsLocations: jsLocations,
- runSkipped: runSkipped);
- }
+ static JsonReporter watch(Engine engine) => new JsonReporter._(engine);
- JsonReporter._(this._engine, {bool verboseTrace: false,
- bool jsLocations: true, bool runSkipped: false})
- : _verboseTrace = verboseTrace,
- _jsLocations = jsLocations,
- _runSkipped = runSkipped {
+ JsonReporter._(this._engine) {
_subscriptions.add(_engine.onTestStarted.listen(_onTestStarted));
/// Convert the future to a stream so that the subscription can be paused or
@@ -229,7 +210,7 @@ class JsonReporter implements Reporter {
}
/// Serializes [metadata] into a JSON-protocol-compatible map.
- Map _serializeMetadata(Metadata metadata) => _runSkipped
+ Map _serializeMetadata(Metadata metadata) => _config.runSkipped
? {"skip": false, "skipReason": null}
: {"skip": metadata.skip, "skipReason": metadata.skipReason};
@@ -251,7 +232,8 @@ class JsonReporter implements Reporter {
_emit("error", {
"testID": _liveTestIDs[liveTest],
"error": error.toString(),
- "stackTrace": terseChain(stackTrace, verbose: _verboseTrace).toString(),
+ "stackTrace": terseChain(stackTrace, verbose: _config.verboseTrace)
+ .toString(),
"isFailure": error is TestFailure
});
}
@@ -281,7 +263,7 @@ class JsonReporter implements Reporter {
Map<String, dynamic> _addFrameInfo(Map<String, dynamic> map,
GroupEntry entry, TestPlatform platform) {
var frame = entry.trace?.frames?.first;
- if (!_jsLocations && platform.isJS) frame = null;
+ if (_config.jsTrace && platform.isJS) frame = null;
map["line"] = frame?.line;
map["column"] = frame?.column;
diff --git a/lib/src/runner/vm/platform.dart b/lib/src/runner/vm/platform.dart
index 78ce654b..27d01c3b 100644
--- a/lib/src/runner/vm/platform.dart
+++ b/lib/src/runner/vm/platform.dart
@@ -17,9 +17,9 @@ import '../plugin/platform.dart';
/// A platform that loads tests in isolates spawned within this Dart process.
class VMPlatform extends PlatformPlugin {
/// The test runner configuration.
- final Configuration _config;
+ final _config = Configuration.current;
- VMPlatform(this._config);
+ VMPlatform();
StreamChannel loadChannel(String path, TestPlatform platform) {
assert(platform == TestPlatform.vm);
diff --git a/test/runner/browser/loader_test.dart b/test/runner/browser/loader_test.dart
index 4eae04b1..51273201 100644
--- a/test/runner/browser/loader_test.dart
+++ b/test/runner/browser/loader_test.dart
@@ -36,8 +36,8 @@ void main() {
void main() {
setUp(() async {
_sandbox = createTempDir();
- _loader = new Loader(new Configuration(platforms: [TestPlatform.chrome]),
- root: _sandbox);
+ _loader = new Configuration(platforms: [TestPlatform.chrome])
+ .asCurrent(() => new Loader(root: _sandbox));
/// TODO(nweiz): Use scheduled_test for this once it's compatible with this
/// version of test.
new File(p.join(_sandbox, 'a_test.dart')).writeAsStringSync(_tests);
@@ -124,9 +124,9 @@ Future main() {
});
test("loads a suite both in the browser and the VM", () async {
- var loader = new Loader(
- new Configuration(platforms: [TestPlatform.vm, TestPlatform.chrome]),
- root: _sandbox);
+ var loader = new Configuration(
+ platforms: [TestPlatform.vm, TestPlatform.chrome])
+ .asCurrent(() => new Loader(root: _sandbox));
var path = p.join(_sandbox, 'a_test.dart');
try {
diff --git a/test/runner/loader_test.dart b/test/runner/loader_test.dart
index 182d2e2f..cb77dad4 100644
--- a/test/runner/loader_test.dart
+++ b/test/runner/loader_test.dart
@@ -35,9 +35,7 @@ void main() {
void main() {
setUp(() async {
_sandbox = createTempDir();
- _loader = new Loader(
- new Configuration(),
- root: _sandbox);
+ _loader = new Loader(root: _sandbox);
});
tearDown(() {
--
GitLab