From f4580211621d2a9c07daf191f789480b9999dddb Mon Sep 17 00:00:00 2001 From: Gary Roumanis Date: Fri, 16 Jun 2017 12:54:17 -0700 Subject: [PATCH 01/20] customizable package folding --- lib/src/frontend/stream_matcher.dart | 6 ++- lib/src/frontend/test_chain.dart | 25 +++++++++ lib/src/frontend/throws_matcher.dart | 8 ++- lib/src/runner/browser/browser_manager.dart | 2 +- lib/src/runner/configuration.dart | 23 ++++++++ lib/src/runner/configuration/load.dart | 28 +++++++++- lib/src/runner/plugin/platform_helpers.dart | 24 ++++----- lib/src/runner/remote_listener.dart | 23 ++++++-- lib/src/runner/reporter/compact.dart | 6 +-- lib/src/runner/reporter/expanded.dart | 6 +-- lib/src/runner/reporter/json.dart | 4 +- lib/src/runner/runner_test.dart | 19 ++----- lib/src/util/stack_trace_mapper.dart | 58 ++++++++++++++++++++- lib/src/utils.dart | 19 ------- 14 files changed, 184 insertions(+), 67 deletions(-) create mode 100644 lib/src/frontend/test_chain.dart diff --git a/lib/src/frontend/stream_matcher.dart b/lib/src/frontend/stream_matcher.dart index 6c906744b..32a0c7362 100644 --- a/lib/src/frontend/stream_matcher.dart +++ b/lib/src/frontend/stream_matcher.dart @@ -8,6 +8,8 @@ import 'package:async/async.dart'; import 'package:matcher/matcher.dart'; import '../utils.dart'; +import '../backend/invoker.dart'; +import 'test_chain.dart'; import 'async_matcher.dart'; /// The type for [_StreamMatcher._matchQueue]. @@ -164,7 +166,9 @@ class _StreamMatcher extends AsyncMatcher implements StreamMatcher { return addBullet(event.asValue.value.toString()); } else { var error = event.asError; - var text = "${error.error}\n${testChain(error.stackTrace)}"; + var chain = testChain(error.stackTrace, + verbose: Invoker.current.liveTest.test.metadata.verboseTrace); + var text = "${error.error}\n$chain"; return prefixLines(text, " ", first: "! "); } }).join("\n"); diff --git a/lib/src/frontend/test_chain.dart b/lib/src/frontend/test_chain.dart new file mode 100644 index 000000000..6b6c5e914 --- /dev/null +++ b/lib/src/frontend/test_chain.dart @@ -0,0 +1,25 @@ +import 'package:stack_trace/stack_trace.dart'; + +typedef StackTrace Mapper(StackTrace trace); + +/// Converts [trace] into a Dart stack trace +Mapper currentMapper = (trace) => trace; + +/// The list of packages to fold when producing [StackTrace]s. +Set exceptPackages = new Set.from(['test', 'stream_channel']); + +/// If non-empty, all packages not in this list will be folded when producing +/// [StackTrace]s. +Set onlyPackages = new Set(); + +/// Converts [stackTrace] to a [Chain] following the test's configuration. +Chain testChain(StackTrace stackTrace, {bool verbose: false}) { + var testTrace = currentMapper(stackTrace); + if (verbose) return new Chain.forTrace(testTrace); + return new Chain.forTrace(testTrace).foldFrames((frame) { + if (onlyPackages.isNotEmpty) { + return !onlyPackages.contains(frame.package); + } + return exceptPackages.contains(frame.package); + }, terse: true); +} diff --git a/lib/src/frontend/throws_matcher.dart b/lib/src/frontend/throws_matcher.dart index 8518c0f94..97ddcd70a 100644 --- a/lib/src/frontend/throws_matcher.dart +++ b/lib/src/frontend/throws_matcher.dart @@ -8,6 +8,8 @@ import 'package:matcher/matcher.dart'; import '../utils.dart'; import 'async_matcher.dart'; +import '../frontend/test_chain.dart'; +import '../backend/invoker.dart'; /// This function is deprecated. /// @@ -93,7 +95,11 @@ class Throws extends AsyncMatcher { var buffer = new StringBuffer(); buffer.writeln(indent(prettyPrint(error), first: 'threw ')); if (trace != null) { - buffer.writeln(indent(testChain(trace).toString(), first: 'stack ')); + buffer.writeln(indent( + testChain(trace, + verbose: Invoker.current.liveTest.test.metadata.verboseTrace) + .toString(), + first: 'stack ')); } if (result.isNotEmpty) buffer.writeln(indent(result, first: 'which ')); return buffer.toString().trimRight(); diff --git a/lib/src/runner/browser/browser_manager.dart b/lib/src/runner/browser/browser_manager.dart index 7f25b3886..841b0b3b5 100644 --- a/lib/src/runner/browser/browser_manager.dart +++ b/lib/src/runner/browser/browser_manager.dart @@ -238,7 +238,7 @@ class BrowserManager { try { controller = await deserializeSuite( path, _platform, suiteConfig, await _environment, suiteChannel, - mapTrace: mapper?.mapStackTrace); + mapper: mapper); _controllers.add(controller); return controller.suite; } catch (_) { diff --git a/lib/src/runner/configuration.dart b/lib/src/runner/configuration.dart index fc43ed03e..078784258 100644 --- a/lib/src/runner/configuration.dart +++ b/lib/src/runner/configuration.dart @@ -97,6 +97,15 @@ class Configuration { /// See [shardIndex] for details. final int totalShards; + /// The list of packages to fold when producing [StackTrace]s. + List get exceptPackages => _exceptPackages ?? []; + final List _exceptPackages; + + /// If non-empty, all packages not in this list will be folded when producing + /// [StackTrace]s. + List get onlyPackages => _onlyPackages ?? []; + final List _onlyPackages; + /// The paths from which to load tests. List get paths => _paths ?? ["test"]; final List _paths; @@ -198,6 +207,8 @@ class Configuration { int shardIndex, int totalShards, Iterable paths, + Iterable exceptPackages, + Iterable onlyPackages, Glob filename, Iterable chosenPresets, Map presets, @@ -238,6 +249,8 @@ class Configuration { shardIndex: shardIndex, totalShards: totalShards, paths: paths, + exceptPackages: exceptPackages, + onlyPackages: onlyPackages, filename: filename, chosenPresets: chosenPresetSet, presets: _withChosenPresets(presets, chosenPresetSet), @@ -290,6 +303,8 @@ class Configuration { this.shardIndex, this.totalShards, Iterable paths, + Iterable exceptPackages, + Iterable onlyPackages, Glob filename, Iterable chosenPresets, Map presets, @@ -307,6 +322,8 @@ class Configuration { : Uri.parse("http://localhost:$pubServePort"), _concurrency = concurrency, _paths = _list(paths), + _exceptPackages = _list(exceptPackages), + _onlyPackages = _list(onlyPackages), _filename = filename, chosenPresets = new UnmodifiableSetView(chosenPresets?.toSet() ?? new Set()), @@ -382,6 +399,8 @@ class Configuration { shardIndex: other.shardIndex ?? shardIndex, totalShards: other.totalShards ?? totalShards, paths: other._paths ?? _paths, + exceptPackages: other._exceptPackages ?? _exceptPackages, + onlyPackages: other._onlyPackages ?? _onlyPackages, filename: other._filename ?? _filename, chosenPresets: chosenPresets.union(other.chosenPresets), presets: _mergeConfigMaps(presets, other.presets), @@ -412,6 +431,8 @@ class Configuration { int shardIndex, int totalShards, Iterable paths, + Iterable exceptPackages, + Iterable onlyPackages, Glob filename, Iterable chosenPresets, Map presets, @@ -450,6 +471,8 @@ class Configuration { shardIndex: shardIndex ?? this.shardIndex, totalShards: totalShards ?? this.totalShards, paths: paths ?? _paths, + exceptPackages: exceptPackages ?? _exceptPackages, + onlyPackages: onlyPackages ?? _onlyPackages, filename: filename ?? _filename, chosenPresets: chosenPresets ?? this.chosenPresets, presets: presets ?? this.presets, diff --git a/lib/src/runner/configuration/load.dart b/lib/src/runner/configuration/load.dart index 00e89ad60..c7e974027 100644 --- a/lib/src/runner/configuration/load.dart +++ b/lib/src/runner/configuration/load.dart @@ -74,6 +74,30 @@ class _ConfigurationLoader { Configuration _loadGlobalTestConfig() { var verboseTrace = _getBool("verbose_trace"); var chainStackTraces = _getBool("chain_stack_traces"); + var foldStackFrames = _getMap("fold_stack_frames", key: (keyNode) { + _validate(keyNode, "fold_stack_frames key must be a string", + (value) => value is String); + if (keyNode.value != "only" && keyNode.value != "except") { + throw new SourceSpanFormatException( + 'Invalid fold_stack_frames key: ' + 'Must either be `only` or `except`.', + keyNode.span, + _source); + } + return keyNode.value; + }, value: (valueNode) { + if (valueNode is! YamlList || + (valueNode as YamlList).firstWhere((value) => value is! String, + orElse: () => null) != + null) { + throw new SourceSpanFormatException( + 'Invalid option for fold_stack_frames value: ' + 'Must be a list of strings.', + valueNode.span, + _source); + } + return valueNode.value; + }); var jsTrace = _getBool("js_trace"); var timeout = _parseValue("timeout", (value) => new Timeout.parse(value)); @@ -108,7 +132,9 @@ class _ConfigurationLoader { jsTrace: jsTrace, timeout: timeout, presets: presets, - chainStackTraces: chainStackTraces) + chainStackTraces: chainStackTraces, + exceptPackages: foldStackFrames["except"], + onlyPackages: foldStackFrames["only"]) .merge(_extractPresets/**/( onPlatform, (map) => new Configuration(onPlatform: map))); diff --git a/lib/src/runner/plugin/platform_helpers.dart b/lib/src/runner/plugin/platform_helpers.dart index bddaea79b..08c490ee8 100644 --- a/lib/src/runner/plugin/platform_helpers.dart +++ b/lib/src/runner/plugin/platform_helpers.dart @@ -14,6 +14,7 @@ import '../../backend/test.dart'; import '../../backend/test_platform.dart'; import '../../util/io.dart'; import '../../util/remote_exception.dart'; +import '../../util/stack_trace_mapper.dart'; import '../application_exception.dart'; import '../configuration.dart'; import '../configuration/suite.dart'; @@ -46,9 +47,7 @@ Future deserializeSuite( SuiteConfiguration suiteConfig, Environment environment, StreamChannel channel, - {StackTrace mapTrace(StackTrace trace)}) async { - if (mapTrace == null) mapTrace = (trace) => trace; - + {StackTraceMapper mapper}) async { var disconnector = new Disconnector(); var suiteChannel = new MultiChannel(channel.transform(disconnector)); @@ -60,6 +59,9 @@ Future deserializeSuite( 'path': path, 'collectTraces': Configuration.current.reporter == 'json', 'noRetry': Configuration.current.noRetry, + 'stackTraceMapper': mapper?.serialize(), + 'exceptPackages': Configuration.current.exceptPackages, + 'onlyPackages': Configuration.current.onlyPackages, }); var completer = new Completer(); @@ -72,9 +74,9 @@ Future deserializeSuite( // If we've already provided a controller, send the error to the // LoadSuite. This will cause the virtual load test to fail, which will // notify the user of the error. - loadSuiteZone.handleUncaughtError(error, mapTrace(stackTrace)); + loadSuiteZone.handleUncaughtError(error, stackTrace); } else { - completer.completeError(error, mapTrace(stackTrace)); + completer.completeError(error); } } @@ -93,11 +95,11 @@ Future deserializeSuite( case "error": var asyncError = RemoteException.deserialize(response["error"]); handleError(new LoadException(path, asyncError.error), - mapTrace(asyncError.stackTrace)); + asyncError.stackTrace); break; case "success": - var deserializer = new _Deserializer(suiteChannel, mapTrace); + var deserializer = new _Deserializer(suiteChannel); completer.complete(deserializer.deserializeGroup(response["root"])); break; } @@ -130,10 +132,7 @@ class _Deserializer { /// The channel over which tests communicate. final MultiChannel _channel; - /// The function used to errors' map stack traces. - final _MapTrace _mapTrace; - - _Deserializer(this._channel, this._mapTrace); + _Deserializer(this._channel); /// Deserializes [group] into a concrete [Group]. Group deserializeGroup(Map group) { @@ -160,7 +159,6 @@ class _Deserializer { var metadata = new Metadata.deserialize(test['metadata']); var trace = test['trace'] == null ? null : new Trace.parse(test['trace']); var testChannel = _channel.virtualChannel(test['channel']); - return new RunnerTest( - test['name'], metadata, trace, testChannel, _mapTrace); + return new RunnerTest(test['name'], metadata, trace, testChannel); } } diff --git a/lib/src/runner/remote_listener.dart b/lib/src/runner/remote_listener.dart index 5812174d3..fa56501d9 100644 --- a/lib/src/runner/remote_listener.dart +++ b/lib/src/runner/remote_listener.dart @@ -16,7 +16,9 @@ import '../backend/suite.dart'; import '../backend/test.dart'; import '../backend/test_platform.dart'; import '../util/remote_exception.dart'; +import '../util/stack_trace_mapper.dart'; import '../utils.dart'; +import '../frontend/test_chain.dart'; class RemoteListener { /// The test suite to run. @@ -78,6 +80,17 @@ class RemoteListener { noRetry: message['noRetry']); await declarer.declare(main); + if (message['stackTraceMapper'] != null) { + var mapper = + await StackTraceMapper.deserialize(message['stackTraceMapper']); + currentMapper = mapper.mapStackTrace; + } + if (message['exceptPackages'] != null) { + exceptPackages = new Set.from(message["exceptPackages"]); + } + if (message['onlyPackages'] != null) { + onlyPackages = new Set.from(message["onlyPackages"]); + } var os = message['os'] == null ? null : OperatingSystem.find(message['os']); var platform = TestPlatform.find(message['platform']); @@ -105,7 +118,7 @@ class RemoteListener { static void _sendError(StreamChannel channel, error, StackTrace stackTrace) { channel.sink.add({ "type": "error", - "error": RemoteException.serialize(error, stackTrace) + "error": RemoteException.serialize(error, testChain(stackTrace)) }); } @@ -179,11 +192,13 @@ class RemoteListener { }); }); - liveTest.onError.listen((asyncError) { + liveTest.onError.listen((asyncError) async { channel.sink.add({ "type": "error", - "error": - RemoteException.serialize(asyncError.error, asyncError.stackTrace) + "error": RemoteException.serialize( + asyncError.error, + testChain(asyncError.stackTrace, + verbose: liveTest.test.metadata.verboseTrace)) }); }); diff --git a/lib/src/runner/reporter/compact.dart b/lib/src/runner/reporter/compact.dart index a964672b4..cb4cc9acb 100644 --- a/lib/src/runner/reporter/compact.dart +++ b/lib/src/runner/reporter/compact.dart @@ -212,9 +212,7 @@ class CompactReporter implements Reporter { if (error is! LoadException) { print(indent(error.toString())); - var chain = - terseChain(stackTrace, verbose: liveTest.test.metadata.verboseTrace); - print(indent(chain.toString())); + print(indent('$stackTrace')); return; } @@ -225,7 +223,7 @@ class CompactReporter implements Reporter { error.innerError is! IsolateSpawnException && error.innerError is! FormatException && error.innerError is! String) { - print(indent(terseChain(stackTrace).toString())); + print(indent('$stackTrace')); } } diff --git a/lib/src/runner/reporter/expanded.dart b/lib/src/runner/reporter/expanded.dart index 35f289de3..00d7915be 100644 --- a/lib/src/runner/reporter/expanded.dart +++ b/lib/src/runner/reporter/expanded.dart @@ -201,9 +201,7 @@ class ExpandedReporter implements Reporter { if (error is! LoadException) { print(indent(error.toString())); - var chain = - terseChain(stackTrace, verbose: liveTest.test.metadata.verboseTrace); - print(indent(chain.toString())); + print(indent('$stackTrace')); return; } @@ -213,7 +211,7 @@ class ExpandedReporter implements Reporter { if (error.innerError is! IsolateSpawnException && error.innerError is! FormatException && error.innerError is! String) { - print(indent(terseChain(stackTrace).toString())); + print(indent('$stackTrace')); } } diff --git a/lib/src/runner/reporter/json.dart b/lib/src/runner/reporter/json.dart index 374ad2136..fba28a6ca 100644 --- a/lib/src/runner/reporter/json.dart +++ b/lib/src/runner/reporter/json.dart @@ -250,9 +250,7 @@ class JsonReporter implements Reporter { _emit("error", { "testID": _liveTestIDs[liveTest], "error": error.toString(), - "stackTrace": - terseChain(stackTrace, verbose: liveTest.test.metadata.verboseTrace) - .toString(), + "stackTrace": '$stackTrace', "isFailure": error is TestFailure }); } diff --git a/lib/src/runner/runner_test.dart b/lib/src/runner/runner_test.dart index 70b8eaba1..d3b4eddb2 100644 --- a/lib/src/runner/runner_test.dart +++ b/lib/src/runner/runner_test.dart @@ -19,8 +19,6 @@ import '../util/remote_exception.dart'; import '../utils.dart'; import 'spawn_hybrid.dart'; -typedef StackTrace _MapTrace(StackTrace trace); - /// A test running remotely, controlled by a stream channel. class RunnerTest extends Test { final String name; @@ -30,16 +28,9 @@ class RunnerTest extends Test { /// The channel used to communicate with the test's [IframeListener]. final MultiChannel _channel; - /// The function used to reformat errors' stack traces. - final _MapTrace _mapTrace; - - RunnerTest( - this.name, this.metadata, Trace trace, this._channel, _MapTrace mapTrace) - : trace = trace == null ? null : new Trace.from(mapTrace(trace)), - _mapTrace = mapTrace; + RunnerTest(this.name, this.metadata, this.trace, this._channel); - RunnerTest._( - this.name, this.metadata, this.trace, this._channel, this._mapTrace); + RunnerTest._(this.name, this.metadata, this.trace, this._channel); LiveTest load(Suite suite, {Iterable groups}) { var controller; @@ -54,7 +45,7 @@ class RunnerTest extends Test { switch (message['type']) { case 'error': var asyncError = RemoteException.deserialize(message['error']); - var stackTrace = _mapTrace(asyncError.stackTrace); + var stackTrace = asyncError.stackTrace; controller.addError(asyncError.error, stackTrace); break; @@ -108,7 +99,7 @@ class RunnerTest extends Test { Test forPlatform(TestPlatform platform, {OperatingSystem os}) { if (!metadata.testOn.evaluate(platform, os: os)) return null; - return new RunnerTest._(name, metadata.forPlatform(platform, os: os), trace, - _channel, _mapTrace); + return new RunnerTest._( + name, metadata.forPlatform(platform, os: os), trace, _channel); } } diff --git a/lib/src/util/stack_trace_mapper.dart b/lib/src/util/stack_trace_mapper.dart index 2299f4704..20794a69a 100644 --- a/lib/src/util/stack_trace_mapper.dart +++ b/lib/src/util/stack_trace_mapper.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 'package:package_resolver/package_resolver.dart'; import 'package:source_map_stack_trace/source_map_stack_trace.dart' as mapper; import 'package:source_maps/source_maps.dart'; @@ -17,9 +18,16 @@ class StackTraceMapper { /// The URI of the SDK root from which dart2js loaded its sources. final Uri _sdkRoot; - StackTraceMapper(String contents, + /// The contents of the source map. + final String _contents; + + /// The URI of the source map. + final Uri _mapUrl; + + StackTraceMapper(this._contents, {Uri mapUrl, SyncPackageResolver packageResolver, Uri sdkRoot}) - : _mapping = parseExtended(contents, mapUrl: mapUrl), + : _mapping = parseExtended(_contents, mapUrl: mapUrl), + _mapUrl = mapUrl, _packageResolver = packageResolver, _sdkRoot = sdkRoot; @@ -27,4 +35,50 @@ class StackTraceMapper { StackTrace mapStackTrace(StackTrace trace) => mapper.mapStackTrace(_mapping, trace, packageResolver: _packageResolver, sdkRoot: _sdkRoot); + + /// Returns a Map representation which is suitable for JSON serialization. + Map serialize() { + return { + 'contents': _contents, + 'sdkRoot': _sdkRoot?.toString(), + 'packageConfigMap': + _serializablePackageConfigMap(_packageResolver.packageConfigMap), + 'packageRoot': _packageResolver.packageRoot?.toString(), + 'mapUrl': _mapUrl?.toString(), + }; + } + + /// Returns a Future which will resolve to a [StackTraceMapper] contained in + /// the provided serialized representation. + static Future deserialize(Map serialized) async { + String packageRoot = serialized['packageRoot'] ?? ''; + return new StackTraceMapper(serialized['contents'], + sdkRoot: Uri.parse(serialized['sdkRoot']), + packageResolver: packageRoot.isNotEmpty + ? await new PackageResolver.root( + Uri.parse(serialized['packageRoot'])) + .asSync + : await new PackageResolver.config(_deserializePackageConfigMap( + serialized['packageConfigMap'])) + .asSync, + mapUrl: Uri.parse(serialized['mapUrl'])); + } + + static Map _serializablePackageConfigMap( + Map packageConfigMap) { + var result = {}; + for (var key in packageConfigMap.keys) { + result[key] = '${packageConfigMap[key]}'; + } + return result; + } + + static Map _deserializePackageConfigMap( + Map serialized) { + var result = {}; + for (var key in serialized.keys) { + result[key] = Uri.parse(serialized[key]); + } + return result; + } } diff --git a/lib/src/utils.dart b/lib/src/utils.dart index 17330a4f6..9bf67606f 100644 --- a/lib/src/utils.dart +++ b/lib/src/utils.dart @@ -10,7 +10,6 @@ import 'dart:typed_data'; import 'package:async/async.dart' hide StreamQueue; import 'package:matcher/matcher.dart'; import 'package:path/path.dart' as p; -import 'package:stack_trace/stack_trace.dart'; import 'package:term_glyph/term_glyph.dart' as glyph; import 'backend/invoker.dart'; @@ -169,24 +168,6 @@ final _colorCode = new RegExp('\u001b\\[[0-9;]+m'); /// Returns [str] without any color codes. String withoutColors(String str) => str.replaceAll(_colorCode, ''); -/// Returns [stackTrace] converted to a [Chain] with all irrelevant frames -/// folded together. -/// -/// If [verbose] is `true`, returns the chain for [stackTrace] unmodified. -Chain terseChain(StackTrace stackTrace, {bool verbose: false}) { - if (verbose) return new Chain.forTrace(stackTrace); - return new Chain.forTrace(stackTrace).foldFrames( - (frame) => frame.package == 'test' || frame.package == 'stream_channel', - terse: true); -} - -/// Converts [stackTrace] to a [Chain] following the test's configuration. -Chain testChain(StackTrace stackTrace) { - // TODO(nweiz): Follow more configuration when #527 is fixed. - return terseChain(stackTrace, - verbose: Invoker.current.liveTest.test.metadata.verboseTrace); -} - /// Flattens nested [Iterable]s inside an [Iterable] into a single [List] /// containing only non-[Iterable] elements. List flatten(Iterable nested) { From fd923458fae3874a39dec63c16525bcbf778db69 Mon Sep 17 00:00:00 2001 From: Gary Roumanis Date: Fri, 16 Jun 2017 13:14:04 -0700 Subject: [PATCH 02/20] fix null errors --- lib/src/util/stack_trace_mapper.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/src/util/stack_trace_mapper.dart b/lib/src/util/stack_trace_mapper.dart index 20794a69a..f014867eb 100644 --- a/lib/src/util/stack_trace_mapper.dart +++ b/lib/src/util/stack_trace_mapper.dart @@ -66,6 +66,7 @@ class StackTraceMapper { static Map _serializablePackageConfigMap( Map packageConfigMap) { + if (packageConfigMap == null) return null; var result = {}; for (var key in packageConfigMap.keys) { result[key] = '${packageConfigMap[key]}'; @@ -75,6 +76,7 @@ class StackTraceMapper { static Map _deserializePackageConfigMap( Map serialized) { + if (serialized == null) return null; var result = {}; for (var key in serialized.keys) { result[key] = Uri.parse(serialized[key]); From 349061326e8178be275b3aaab05afcaa965eda71 Mon Sep 17 00:00:00 2001 From: Gary Roumanis Date: Fri, 16 Jun 2017 13:54:13 -0700 Subject: [PATCH 03/20] fix compact reporter tests --- lib/src/runner/remote_listener.dart | 8 ++++++-- test/runner/compact_reporter_test.dart | 16 ++++++++++------ 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/lib/src/runner/remote_listener.dart b/lib/src/runner/remote_listener.dart index fa56501d9..8a61afa13 100644 --- a/lib/src/runner/remote_listener.dart +++ b/lib/src/runner/remote_listener.dart @@ -86,10 +86,14 @@ class RemoteListener { currentMapper = mapper.mapStackTrace; } if (message['exceptPackages'] != null) { - exceptPackages = new Set.from(message["exceptPackages"]); + if (message['exceptPackages'].isNotEmpty) { + exceptPackages = new Set.from(message["exceptPackages"]); + } } if (message['onlyPackages'] != null) { - onlyPackages = new Set.from(message["onlyPackages"]); + if (message['onlyPackages'].isNotEmpty) { + onlyPackages = new Set.from(message["onlyPackages"]); + } } var os = message['os'] == null ? null : OperatingSystem.find(message['os']); diff --git a/test/runner/compact_reporter_test.dart b/test/runner/compact_reporter_test.dart index 79f078b02..1ea27acaa 100644 --- a/test/runner/compact_reporter_test.dart +++ b/test/runner/compact_reporter_test.dart @@ -247,9 +247,11 @@ void main() { }); scheduleMicrotask(() { - print("five"); - print("six"); - completer.complete(); + scheduleMicrotask(() { + print("five"); + print("six"); + completer.complete(); + }); }); print("one"); @@ -264,12 +266,14 @@ void main() { one two + +0 -1: test + three + four + +0 -1: test [E] first error - test.dart 24:11 main. + test.dart 26:11 main. - three - four second error test.dart 13:13 main.. ===== asynchronous gap =========================== From 512d160a6ad9887f69a778b16dc000a3f456a53a Mon Sep 17 00:00:00 2001 From: Gary Roumanis Date: Fri, 16 Jun 2017 14:11:16 -0700 Subject: [PATCH 04/20] remove unecessary async --- lib/src/runner/remote_listener.dart | 6 ++++-- test/runner/compact_reporter_test.dart | 16 ++++++---------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/lib/src/runner/remote_listener.dart b/lib/src/runner/remote_listener.dart index 8a61afa13..66ea674a1 100644 --- a/lib/src/runner/remote_listener.dart +++ b/lib/src/runner/remote_listener.dart @@ -78,7 +78,6 @@ class RemoteListener { metadata: metadata, collectTraces: message['collectTraces'], noRetry: message['noRetry']); - await declarer.declare(main); if (message['stackTraceMapper'] != null) { var mapper = @@ -95,6 +94,9 @@ class RemoteListener { onlyPackages = new Set.from(message["onlyPackages"]); } } + + await declarer.declare(main); + var os = message['os'] == null ? null : OperatingSystem.find(message['os']); var platform = TestPlatform.find(message['platform']); @@ -196,7 +198,7 @@ class RemoteListener { }); }); - liveTest.onError.listen((asyncError) async { + liveTest.onError.listen((asyncError) { channel.sink.add({ "type": "error", "error": RemoteException.serialize( diff --git a/test/runner/compact_reporter_test.dart b/test/runner/compact_reporter_test.dart index 1ea27acaa..79f078b02 100644 --- a/test/runner/compact_reporter_test.dart +++ b/test/runner/compact_reporter_test.dart @@ -247,11 +247,9 @@ void main() { }); scheduleMicrotask(() { - scheduleMicrotask(() { - print("five"); - print("six"); - completer.complete(); - }); + print("five"); + print("six"); + completer.complete(); }); print("one"); @@ -266,14 +264,12 @@ void main() { one two - +0 -1: test - three - four - +0 -1: test [E] first error - test.dart 26:11 main. + test.dart 24:11 main. + three + four second error test.dart 13:13 main.. ===== asynchronous gap =========================== From 5e8e0ec91fb9ca29ba59e72197c6560ba2cae953 Mon Sep 17 00:00:00 2001 From: Gary Roumanis Date: Fri, 16 Jun 2017 14:51:50 -0700 Subject: [PATCH 05/20] add test chain tests --- test/frontend/test_chain_test.dart | 103 ++++++++++++++++++ .../configuration/top_level_error_test.dart | 43 ++++++++ 2 files changed, 146 insertions(+) create mode 100644 test/frontend/test_chain_test.dart diff --git a/test/frontend/test_chain_test.dart b/test/frontend/test_chain_test.dart new file mode 100644 index 000000000..a87bd5327 --- /dev/null +++ b/test/frontend/test_chain_test.dart @@ -0,0 +1,103 @@ +// Copyright (c) 2015, the Dart project authors. Please see the AUTHORS file +// 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. + +@TestOn("vm") + +import 'dart:convert'; + +import 'package:test_descriptor/test_descriptor.dart' as d; +import 'package:test/test.dart'; + +import '../io.dart'; + +final _asyncFailure = """ +import 'dart:async'; + +import 'package:test/test.dart'; + +void main() { + test("failure", () async{ + await new Future((){}); + await new Future((){}); + throw "oh no"; + }); +} +"""; + +void main() { + test("folds packages contained in the except list", () async { + await d + .file( + "dart_test.yaml", + JSON.encode({ + "fold_stack_frames": { + "except": ["stream_channel"] + } + })) + .create(); + await d.file("test.dart", _asyncFailure).create(); + + var test = await runTest(["test.dart"]); + var testOutput = (await test.stdoutStream().toList()); + for (var output in testOutput) { + expect(output.contains('package:stream_channel'), isFalse); + } + + await test.shouldExit(1); + }); + + test("by default folds both stream_channel and test packages", () async { + await d.file("test.dart", _asyncFailure).create(); + + var test = await runTest(["test.dart"]); + var testOutput = (await test.stdoutStream().toList()); + for (var output in testOutput) { + expect(output.contains('package:test'), isFalse); + expect(output.contains('package:stream_channel'), isFalse); + } + await test.shouldExit(1); + }); + + test("folds all packages not contained in the only list", () async { + await d + .file( + "dart_test.yaml", + JSON.encode({ + "fold_stack_frames": { + "only": ["test"] + } + })) + .create(); + await d.file("test.dart", _asyncFailure).create(); + + var test = await runTest(["test.dart"]); + var testOutput = (await test.stdoutStream().toList()); + for (var output in testOutput) { + expect(output.contains('package:stream_channel'), isFalse); + } + await test.shouldExit(1); + }); + + test("does not fold packages in the only list", () async { + await d + .file( + "dart_test.yaml", + JSON.encode({ + "fold_stack_frames": { + "only": ["test"] + } + })) + .create(); + await d.file("test.dart", _asyncFailure).create(); + + var test = await runTest(["test.dart"]); + var hasPackageTest = false; + var testOutput = (await test.stdoutStream().toList()); + for (var output in testOutput) { + if (output.contains('package:test')) hasPackageTest = true; + } + expect(hasPackageTest, isTrue); + await test.shouldExit(1); + }); +} diff --git a/test/runner/configuration/top_level_error_test.dart b/test/runner/configuration/top_level_error_test.dart index 3f6b643aa..26fdaec70 100644 --- a/test/runner/configuration/top_level_error_test.dart +++ b/test/runner/configuration/top_level_error_test.dart @@ -13,6 +13,49 @@ import 'package:test/test.dart'; import '../../io.dart'; void main() { + test("rejects an invalid fold_stack_frames", () async { + await d + .file("dart_test.yaml", JSON.encode({"fold_stack_frames": "flup"})) + .create(); + + var test = await runTest(["test.dart"]); + expect(test.stderr, + containsInOrder(["fold_stack_frames must be a map", "^^^^^^"])); + await test.shouldExit(exit_codes.data); + }); + + test("rejects an invalid fold_stack_frames key", () async { + await d + .file( + "dart_test.yaml", + JSON.encode({ + "fold_stack_frames": {"invalid": "blah"} + })) + .create(); + + var test = await runTest(["test.dart"]); + expect(test.stderr, + containsInOrder(["Invalid fold_stack_frames key", "^^^^^^"])); + await test.shouldExit(exit_codes.data); + }); + + test("rejects an invalid fold_stack_frames values", () async { + await d + .file( + "dart_test.yaml", + JSON.encode({ + "fold_stack_frames": {"only": "blah"} + })) + .create(); + + var test = await runTest(["test.dart"]); + expect( + test.stderr, + containsInOrder( + ["Invalid option for fold_stack_frames value", "^^^^^^"])); + await test.shouldExit(exit_codes.data); + }); + test("rejects an invalid pause_after_load", () async { await d .file("dart_test.yaml", JSON.encode({"pause_after_load": "flup"})) From 1a450424633cc81f75cdafd93089d1297c0f863a Mon Sep 17 00:00:00 2001 From: Gary Roumanis Date: Fri, 16 Jun 2017 15:02:14 -0700 Subject: [PATCH 06/20] fix grammar --- test/runner/configuration/top_level_error_test.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/runner/configuration/top_level_error_test.dart b/test/runner/configuration/top_level_error_test.dart index 26fdaec70..aed933b88 100644 --- a/test/runner/configuration/top_level_error_test.dart +++ b/test/runner/configuration/top_level_error_test.dart @@ -24,7 +24,7 @@ void main() { await test.shouldExit(exit_codes.data); }); - test("rejects an invalid fold_stack_frames key", () async { + test("rejects invalid fold_stack_frames keys", () async { await d .file( "dart_test.yaml", @@ -39,7 +39,7 @@ void main() { await test.shouldExit(exit_codes.data); }); - test("rejects an invalid fold_stack_frames values", () async { + test("rejects invalid fold_stack_frames values", () async { await d .file( "dart_test.yaml", From 082bb8bc46829199300345d1e72649e8d552aa4d Mon Sep 17 00:00:00 2001 From: Gary Roumanis Date: Fri, 16 Jun 2017 15:11:35 -0700 Subject: [PATCH 07/20] add changelog and configuration details --- CHANGELOG.md | 5 +++++ doc/configuration.md | 17 +++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index effe894f9..4403f6fe4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +## 0.12.23 + +* Add a `fold_stack_frame` field for `dart_test.yaml`. This will + allow users to customize which packages' frames are folded. + ## 0.12.22 * Add a `retry` option to `test()` and `group()` functions, as well diff --git a/doc/configuration.md b/doc/configuration.md index 7f294429b..a1578b44d 100644 --- a/doc/configuration.md +++ b/doc/configuration.md @@ -45,6 +45,7 @@ tags: * [`run_skipped`](#run_skipped) * [`pub_serve`](#pub_serve) * [`reporter`](#reporter) + * [`fold_stack_frames`](#fold_stack_frames) * [Configuring Tags](#configuring-tags) * [`tags`](#tags) * [`add_tags`](#add_tags) @@ -394,6 +395,22 @@ reporter: expanded This field is not supported in the [global configuration file](#global-configuration). +### `fold_stack_frames` + +This field indicates which packages will be folded when producing stack traces. +Packages contained in the `exclude` option will be folded. If `only` is provided, +all packages not contained in this list will be folded. If no options are provided, +we fold `package:test` and `package:stream_channel` by default. + +```yaml +fold_stack_frames: + only: + - some_package + exclude: + - a_package + - another_package +``` + ## Configuring Tags ### `tags` From ea6db49189eca0cd960e51dde99ae569f5e9839d Mon Sep 17 00:00:00 2001 From: Gary Roumanis Date: Mon, 19 Jun 2017 11:20:17 -0700 Subject: [PATCH 08/20] doc changes and other minor fixes --- CHANGELOG.md | 2 +- doc/configuration.md | 31 ++++++--- lib/src/frontend/test_chain.dart | 4 ++ lib/src/runner/configuration/load.dart | 17 ++--- lib/src/runner/remote_listener.dart | 2 +- lib/src/util/stack_trace_mapper.dart | 49 +++++++------- test/frontend/test_chain_test.dart | 66 +++++++------------ .../configuration/top_level_error_test.dart | 8 +-- 8 files changed, 83 insertions(+), 96 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a7350069..da4c657fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ ## 0.12.23 -* Add a `fold_stack_frame` field for `dart_test.yaml`. This will +* Add a `fold_stack_frames` field for `dart_test.yaml`. This will allow users to customize which packages' frames are folded. ## 0.12.22+1 diff --git a/doc/configuration.md b/doc/configuration.md index a1578b44d..24cf7c038 100644 --- a/doc/configuration.md +++ b/doc/configuration.md @@ -397,20 +397,33 @@ This field is not supported in the ### `fold_stack_frames` -This field indicates which packages will be folded when producing stack traces. -Packages contained in the `exclude` option will be folded. If `only` is provided, -all packages not contained in this list will be folded. If no options are provided, -we fold `package:test` and `package:stream_channel` by default. +This field controls which packages' stack frames will be folded away +when displaying stack traces. Packages contained in the `exclude` +option will be folded. If `only` is provided, all packages not +contained in this list will be folded. By default, +frames from the `test` package and the `stream_channel` +package are folded. ```yaml fold_stack_frames: - only: - - some_package - exclude: - - a_package - - another_package + except: + - test + - stream_channel ``` +Sample stack trace, note the absence of `package:test` +and `package:stream_channel`: +``` +test/sample_test.dart 7:5 main. +===== asynchronous gap =========================== +dart:async _Completer.completeError +test/sample_test.dart 8:3 main. +===== asynchronous gap =========================== +dart:async _asyncThenWrapperHelper +test/sample_test.dart 5:27 main. +``` + + ## Configuring Tags ### `tags` diff --git a/lib/src/frontend/test_chain.dart b/lib/src/frontend/test_chain.dart index 6b6c5e914..b242a8117 100644 --- a/lib/src/frontend/test_chain.dart +++ b/lib/src/frontend/test_chain.dart @@ -1,3 +1,7 @@ +// Copyright (c) 2017, the Dart project authors. Please see the AUTHORS file +// 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 'package:stack_trace/stack_trace.dart'; typedef StackTrace Mapper(StackTrace trace); diff --git a/lib/src/runner/configuration/load.dart b/lib/src/runner/configuration/load.dart index c7e974027..5d78e4053 100644 --- a/lib/src/runner/configuration/load.dart +++ b/lib/src/runner/configuration/load.dart @@ -75,26 +75,17 @@ class _ConfigurationLoader { var verboseTrace = _getBool("verbose_trace"); var chainStackTraces = _getBool("chain_stack_traces"); var foldStackFrames = _getMap("fold_stack_frames", key: (keyNode) { - _validate(keyNode, "fold_stack_frames key must be a string", - (value) => value is String); + _validate(keyNode, "Must be a string", (value) => value is String); if (keyNode.value != "only" && keyNode.value != "except") { throw new SourceSpanFormatException( - 'Invalid fold_stack_frames key: ' - 'Must either be `only` or `except`.', - keyNode.span, - _source); + 'Must be "only" or "except".', keyNode.span, _source); } return keyNode.value; }, value: (valueNode) { if (valueNode is! YamlList || - (valueNode as YamlList).firstWhere((value) => value is! String, - orElse: () => null) != - null) { + (valueNode as YamlList).any((value) => value is! String)) { throw new SourceSpanFormatException( - 'Invalid option for fold_stack_frames value: ' - 'Must be a list of strings.', - valueNode.span, - _source); + 'Folded packages must be strings.', valueNode.span, _source); } return valueNode.value; }); diff --git a/lib/src/runner/remote_listener.dart b/lib/src/runner/remote_listener.dart index 66ea674a1..0d56f3b9f 100644 --- a/lib/src/runner/remote_listener.dart +++ b/lib/src/runner/remote_listener.dart @@ -15,10 +15,10 @@ import '../backend/operating_system.dart'; import '../backend/suite.dart'; import '../backend/test.dart'; import '../backend/test_platform.dart'; +import '../frontend/test_chain.dart'; import '../util/remote_exception.dart'; import '../util/stack_trace_mapper.dart'; import '../utils.dart'; -import '../frontend/test_chain.dart'; class RemoteListener { /// The test suite to run. diff --git a/lib/src/util/stack_trace_mapper.dart b/lib/src/util/stack_trace_mapper.dart index f014867eb..11a6866e0 100644 --- a/lib/src/util/stack_trace_mapper.dart +++ b/lib/src/util/stack_trace_mapper.dart @@ -3,6 +3,8 @@ // BSD-style license that can be found in the LICENSE file. import 'dart:async'; +import 'package:collection/collection.dart'; + import 'package:package_resolver/package_resolver.dart'; import 'package:source_map_stack_trace/source_map_stack_trace.dart' as mapper; import 'package:source_maps/source_maps.dart'; @@ -10,39 +12,40 @@ import 'package:source_maps/source_maps.dart'; /// A class for mapping JS stack traces to Dart stack traces using source maps. class StackTraceMapper { /// The parsed source map. - final Mapping _mapping; + Mapping _mapping; /// The package resolution information passed to dart2js. final SyncPackageResolver _packageResolver; - /// The URI of the SDK root from which dart2js loaded its sources. + /// The URL of the SDK root from which dart2js loaded its sources. final Uri _sdkRoot; /// The contents of the source map. - final String _contents; + final String _mapContents; - /// The URI of the source map. + /// The URL of the source map. final Uri _mapUrl; - StackTraceMapper(this._contents, + StackTraceMapper(this._mapContents, {Uri mapUrl, SyncPackageResolver packageResolver, Uri sdkRoot}) - : _mapping = parseExtended(_contents, mapUrl: mapUrl), - _mapUrl = mapUrl, + : _mapUrl = mapUrl, _packageResolver = packageResolver, _sdkRoot = sdkRoot; /// Converts [trace] into a Dart stack trace. - StackTrace mapStackTrace(StackTrace trace) => - mapper.mapStackTrace(_mapping, trace, - packageResolver: _packageResolver, sdkRoot: _sdkRoot); + StackTrace mapStackTrace(StackTrace trace) { + _mapping ??= parseExtended(_mapContents, mapUrl: _mapUrl); + return mapper.mapStackTrace(_mapping, trace, + packageResolver: _packageResolver, sdkRoot: _sdkRoot); + } /// Returns a Map representation which is suitable for JSON serialization. Map serialize() { return { - 'contents': _contents, + 'mapContents': _mapContents, 'sdkRoot': _sdkRoot?.toString(), 'packageConfigMap': - _serializablePackageConfigMap(_packageResolver.packageConfigMap), + _serializePackageConfigMap(_packageResolver.packageConfigMap), 'packageRoot': _packageResolver.packageRoot?.toString(), 'mapUrl': _mapUrl?.toString(), }; @@ -51,8 +54,8 @@ class StackTraceMapper { /// Returns a Future which will resolve to a [StackTraceMapper] contained in /// the provided serialized representation. static Future deserialize(Map serialized) async { - String packageRoot = serialized['packageRoot'] ?? ''; - return new StackTraceMapper(serialized['contents'], + String packageRoot = serialized['packageRoot'] as String ?? ''; + return new StackTraceMapper(serialized['mapContents'], sdkRoot: Uri.parse(serialized['sdkRoot']), packageResolver: packageRoot.isNotEmpty ? await new PackageResolver.root( @@ -64,23 +67,19 @@ class StackTraceMapper { mapUrl: Uri.parse(serialized['mapUrl'])); } - static Map _serializablePackageConfigMap( + /// Converts a [packageConfigMap] into a format suitable for JSON + /// serialization. + static Map _serializePackageConfigMap( Map packageConfigMap) { if (packageConfigMap == null) return null; - var result = {}; - for (var key in packageConfigMap.keys) { - result[key] = '${packageConfigMap[key]}'; - } - return result; + return mapMap(packageConfigMap, value: (_, value) => '$value'); } + /// Converts a serialized package config map into a format suitable for + /// the [PackageResolver] static Map _deserializePackageConfigMap( Map serialized) { if (serialized == null) return null; - var result = {}; - for (var key in serialized.keys) { - result[key] = Uri.parse(serialized[key]); - } - return result; + return mapMap(serialized, value: (_, value) => Uri.parse(value)); } } diff --git a/test/frontend/test_chain_test.dart b/test/frontend/test_chain_test.dart index a87bd5327..1a3603937 100644 --- a/test/frontend/test_chain_test.dart +++ b/test/frontend/test_chain_test.dart @@ -1,4 +1,4 @@ -// Copyright (c) 2015, the Dart project authors. Please see the AUTHORS file +// Copyright (c) 2017, the Dart project authors. Please see the AUTHORS file // 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. @@ -11,21 +11,26 @@ import 'package:test/test.dart'; import '../io.dart'; -final _asyncFailure = """ -import 'dart:async'; - -import 'package:test/test.dart'; - void main() { - test("failure", () async{ - await new Future((){}); - await new Future((){}); - throw "oh no"; + setUp(() async { + await d + .file( + "test.dart", + """ + import 'dart:async'; + + import 'package:test/test.dart'; + + void main() { + test("failure", () async{ + await new Future((){}); + await new Future((){}); + throw "oh no"; + }); + } + """) + .create(); }); -} -"""; - -void main() { test("folds packages contained in the except list", () async { await d .file( @@ -36,26 +41,15 @@ void main() { } })) .create(); - await d.file("test.dart", _asyncFailure).create(); - var test = await runTest(["test.dart"]); - var testOutput = (await test.stdoutStream().toList()); - for (var output in testOutput) { - expect(output.contains('package:stream_channel'), isFalse); - } - + expect(test.stdoutStream(), neverEmits(contains('package:stream_channel'))); await test.shouldExit(1); }); test("by default folds both stream_channel and test packages", () async { - await d.file("test.dart", _asyncFailure).create(); - var test = await runTest(["test.dart"]); - var testOutput = (await test.stdoutStream().toList()); - for (var output in testOutput) { - expect(output.contains('package:test'), isFalse); - expect(output.contains('package:stream_channel'), isFalse); - } + expect(test.stdoutStream(), neverEmits(contains('package:test'))); + expect(test.stdoutStream(), neverEmits(contains('package:stream_channel'))); await test.shouldExit(1); }); @@ -69,13 +63,8 @@ void main() { } })) .create(); - await d.file("test.dart", _asyncFailure).create(); - var test = await runTest(["test.dart"]); - var testOutput = (await test.stdoutStream().toList()); - for (var output in testOutput) { - expect(output.contains('package:stream_channel'), isFalse); - } + expect(test.stdoutStream(), neverEmits(contains('package:stream_channel'))); await test.shouldExit(1); }); @@ -89,15 +78,8 @@ void main() { } })) .create(); - await d.file("test.dart", _asyncFailure).create(); - var test = await runTest(["test.dart"]); - var hasPackageTest = false; - var testOutput = (await test.stdoutStream().toList()); - for (var output in testOutput) { - if (output.contains('package:test')) hasPackageTest = true; - } - expect(hasPackageTest, isTrue); + expect(test.stdoutStream(), emitsThrough(contains('package:test'))); await test.shouldExit(1); }); } diff --git a/test/runner/configuration/top_level_error_test.dart b/test/runner/configuration/top_level_error_test.dart index 783417057..29991481b 100644 --- a/test/runner/configuration/top_level_error_test.dart +++ b/test/runner/configuration/top_level_error_test.dart @@ -35,7 +35,7 @@ void main() { var test = await runTest(["test.dart"]); expect(test.stderr, - containsInOrder(["Invalid fold_stack_frames key", "^^^^^^"])); + containsInOrder(['Must be "only" or "except".', "^^^^^^"])); await test.shouldExit(exit_codes.data); }); @@ -49,10 +49,8 @@ void main() { .create(); var test = await runTest(["test.dart"]); - expect( - test.stderr, - containsInOrder( - ["Invalid option for fold_stack_frames value", "^^^^^^"])); + expect(test.stderr, + containsInOrder(["Folded packages must be strings", "^^^^^^"])); await test.shouldExit(exit_codes.data); }); From 1ddcf6f24ea9c4c02c3058bf778c0ca687746bdd Mon Sep 17 00:00:00 2001 From: Gary Roumanis Date: Mon, 19 Jun 2017 12:13:57 -0700 Subject: [PATCH 09/20] check for only one of only or except --- lib/src/runner/configuration/load.dart | 42 ++++++++++++------- .../configuration/top_level_error_test.dart | 20 +++++++++ 2 files changed, 47 insertions(+), 15 deletions(-) diff --git a/lib/src/runner/configuration/load.dart b/lib/src/runner/configuration/load.dart index 5d78e4053..4f96fb9b1 100644 --- a/lib/src/runner/configuration/load.dart +++ b/lib/src/runner/configuration/load.dart @@ -74,21 +74,7 @@ class _ConfigurationLoader { Configuration _loadGlobalTestConfig() { var verboseTrace = _getBool("verbose_trace"); var chainStackTraces = _getBool("chain_stack_traces"); - var foldStackFrames = _getMap("fold_stack_frames", key: (keyNode) { - _validate(keyNode, "Must be a string", (value) => value is String); - if (keyNode.value != "only" && keyNode.value != "except") { - throw new SourceSpanFormatException( - 'Must be "only" or "except".', keyNode.span, _source); - } - return keyNode.value; - }, value: (valueNode) { - if (valueNode is! YamlList || - (valueNode as YamlList).any((value) => value is! String)) { - throw new SourceSpanFormatException( - 'Folded packages must be strings.', valueNode.span, _source); - } - return valueNode.value; - }); + var foldStackFrames = _loadFoldedStackFrames(); var jsTrace = _getBool("js_trace"); var timeout = _parseValue("timeout", (value) => new Timeout.parse(value)); @@ -280,6 +266,32 @@ class _ConfigurationLoader { excludeTags: excludeTags); } + Map _loadFoldedStackFrames() { + bool foldOptionSet = false; + return _getMap("fold_stack_frames", key: (keyNode) { + _validate(keyNode, "Must be a string", (value) => value is String); + if (keyNode.value != "only" && keyNode.value != "except") { + throw new SourceSpanFormatException( + 'Must be "only" or "except".', keyNode.span, _source); + } + if (foldOptionSet) { + throw new SourceSpanFormatException( + 'Can only contain one of "only" or "except".', + keyNode.span, + _source); + } + foldOptionSet = true; + return keyNode.value; + }, value: (valueNode) { + if (valueNode is! YamlList || + (valueNode as YamlList).any((value) => value is! String)) { + throw new SourceSpanFormatException( + 'Folded packages must be strings.', valueNode.span, _source); + } + return valueNode.value; + }); + } + /// Throws an exception with [message] if [test] returns `false` when passed /// [node]'s value. void _validate(YamlNode node, String message, bool test(value)) { diff --git a/test/runner/configuration/top_level_error_test.dart b/test/runner/configuration/top_level_error_test.dart index 29991481b..712cf5ec9 100644 --- a/test/runner/configuration/top_level_error_test.dart +++ b/test/runner/configuration/top_level_error_test.dart @@ -24,6 +24,26 @@ void main() { await test.shouldExit(exit_codes.data); }); + test("rejects multiple fold_stack_frames keys", () async { + await d + .file( + "dart_test.yaml", + JSON.encode({ + "fold_stack_frames": { + "except": ["blah"], + "only": ["blah"] + } + })) + .create(); + + var test = await runTest(["test.dart"]); + expect( + test.stderr, + containsInOrder( + ['Can only contain one of "only" or "except".', "^^^^^^"])); + await test.shouldExit(exit_codes.data); + }); + test("rejects invalid fold_stack_frames keys", () async { await d .file( From 7ebb648c6fec2034d746c95f8e8bd960241ab05c Mon Sep 17 00:00:00 2001 From: Gary Roumanis Date: Mon, 19 Jun 2017 13:08:36 -0700 Subject: [PATCH 10/20] change configuration logic --- lib/src/runner/configuration.dart | 57 +++++++++++++++------ lib/src/runner/configuration/load.dart | 4 +- lib/src/runner/plugin/platform_helpers.dart | 4 +- 3 files changed, 45 insertions(+), 20 deletions(-) diff --git a/lib/src/runner/configuration.dart b/lib/src/runner/configuration.dart index 078784258..bc2f1c2e4 100644 --- a/lib/src/runner/configuration.dart +++ b/lib/src/runner/configuration.dart @@ -98,13 +98,13 @@ class Configuration { final int totalShards; /// The list of packages to fold when producing [StackTrace]s. - List get exceptPackages => _exceptPackages ?? []; - final List _exceptPackages; + List get foldTraceExcept => _foldTraceExcept ?? []; + final List _foldTraceExcept; /// If non-empty, all packages not in this list will be folded when producing /// [StackTrace]s. - List get onlyPackages => _onlyPackages ?? []; - final List _onlyPackages; + List get foldTraceOnly => _foldTraceOnly ?? []; + final List _foldTraceOnly; /// The paths from which to load tests. List get paths => _paths ?? ["test"]; @@ -207,8 +207,8 @@ class Configuration { int shardIndex, int totalShards, Iterable paths, - Iterable exceptPackages, - Iterable onlyPackages, + Iterable foldTraceExcept, + Iterable foldTraceOnly, Glob filename, Iterable chosenPresets, Map presets, @@ -249,8 +249,8 @@ class Configuration { shardIndex: shardIndex, totalShards: totalShards, paths: paths, - exceptPackages: exceptPackages, - onlyPackages: onlyPackages, + foldTraceExcept: foldTraceExcept, + foldTraceOnly: foldTraceOnly, filename: filename, chosenPresets: chosenPresetSet, presets: _withChosenPresets(presets, chosenPresetSet), @@ -303,8 +303,8 @@ class Configuration { this.shardIndex, this.totalShards, Iterable paths, - Iterable exceptPackages, - Iterable onlyPackages, + Iterable foldTraceExcept, + Iterable foldTraceOnly, Glob filename, Iterable chosenPresets, Map presets, @@ -322,8 +322,8 @@ class Configuration { : Uri.parse("http://localhost:$pubServePort"), _concurrency = concurrency, _paths = _list(paths), - _exceptPackages = _list(exceptPackages), - _onlyPackages = _list(onlyPackages), + _foldTraceExcept = _list(foldTraceExcept), + _foldTraceOnly = _list(foldTraceOnly), _filename = filename, chosenPresets = new UnmodifiableSetView(chosenPresets?.toSet() ?? new Set()), @@ -386,6 +386,31 @@ class Configuration { if (this == Configuration.empty) return other; if (other == Configuration.empty) return this; + List foldTraceOnly = other._foldTraceOnly ?? _foldTraceOnly; + if (other._foldTraceOnly != null && _foldTraceExcept != null) { + foldTraceOnly = new Set.from(other._foldTraceExcept) + .difference(new Set.from(_foldTraceExcept)) + .toList(); + } + + if (_foldTraceOnly != null && other._foldTraceExcept != null) { + foldTraceOnly = new Set.from(_foldTraceExcept) + .difference(new Set.from(other._foldTraceExcept)) + .toList(); + } + if (other._foldTraceOnly != null && _foldTraceOnly != null) { + foldTraceOnly = new Set.from(other._foldTraceOnly) + .intersection(new Set.from(_foldTraceOnly)) + .toList(); + } + + List foldTraceExcept = other._foldTraceExcept ?? _foldTraceExcept; + if (other._foldTraceExcept != null && _foldTraceExcept != null) { + foldTraceExcept = new Set.from(other._foldTraceExcept) + .union(new Set.from(_foldTraceExcept)) + .toList(); + } + var result = new Configuration._( help: other._help ?? _help, version: other._version ?? _version, @@ -399,8 +424,8 @@ class Configuration { shardIndex: other.shardIndex ?? shardIndex, totalShards: other.totalShards ?? totalShards, paths: other._paths ?? _paths, - exceptPackages: other._exceptPackages ?? _exceptPackages, - onlyPackages: other._onlyPackages ?? _onlyPackages, + foldTraceExcept: foldTraceExcept, + foldTraceOnly: foldTraceOnly, filename: other._filename ?? _filename, chosenPresets: chosenPresets.union(other.chosenPresets), presets: _mergeConfigMaps(presets, other.presets), @@ -471,8 +496,8 @@ class Configuration { shardIndex: shardIndex ?? this.shardIndex, totalShards: totalShards ?? this.totalShards, paths: paths ?? _paths, - exceptPackages: exceptPackages ?? _exceptPackages, - onlyPackages: onlyPackages ?? _onlyPackages, + foldTraceExcept: exceptPackages ?? _foldTraceExcept, + foldTraceOnly: onlyPackages ?? _foldTraceOnly, filename: filename ?? _filename, chosenPresets: chosenPresets ?? this.chosenPresets, presets: presets ?? this.presets, diff --git a/lib/src/runner/configuration/load.dart b/lib/src/runner/configuration/load.dart index 4f96fb9b1..c26db4a3b 100644 --- a/lib/src/runner/configuration/load.dart +++ b/lib/src/runner/configuration/load.dart @@ -110,8 +110,8 @@ class _ConfigurationLoader { timeout: timeout, presets: presets, chainStackTraces: chainStackTraces, - exceptPackages: foldStackFrames["except"], - onlyPackages: foldStackFrames["only"]) + foldTraceExcept: foldStackFrames["except"], + foldTraceOnly: foldStackFrames["only"]) .merge(_extractPresets/**/( onPlatform, (map) => new Configuration(onPlatform: map))); diff --git a/lib/src/runner/plugin/platform_helpers.dart b/lib/src/runner/plugin/platform_helpers.dart index 08c490ee8..616c34fb9 100644 --- a/lib/src/runner/plugin/platform_helpers.dart +++ b/lib/src/runner/plugin/platform_helpers.dart @@ -60,8 +60,8 @@ Future deserializeSuite( 'collectTraces': Configuration.current.reporter == 'json', 'noRetry': Configuration.current.noRetry, 'stackTraceMapper': mapper?.serialize(), - 'exceptPackages': Configuration.current.exceptPackages, - 'onlyPackages': Configuration.current.onlyPackages, + 'exceptPackages': Configuration.current.foldTraceExcept, + 'onlyPackages': Configuration.current.foldTraceOnly, }); var completer = new Completer(); From c1d6b4855866d018ebfde8c0e14e8ea4af877008 Mon Sep 17 00:00:00 2001 From: Gary Roumanis Date: Mon, 19 Jun 2017 13:15:02 -0700 Subject: [PATCH 11/20] sync package resolver --- lib/src/runner/remote_listener.dart | 3 +-- lib/src/util/stack_trace_mapper.dart | 15 ++++++--------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/lib/src/runner/remote_listener.dart b/lib/src/runner/remote_listener.dart index 0d56f3b9f..ddebd87f4 100644 --- a/lib/src/runner/remote_listener.dart +++ b/lib/src/runner/remote_listener.dart @@ -80,8 +80,7 @@ class RemoteListener { noRetry: message['noRetry']); if (message['stackTraceMapper'] != null) { - var mapper = - await StackTraceMapper.deserialize(message['stackTraceMapper']); + var mapper = StackTraceMapper.deserialize(message['stackTraceMapper']); currentMapper = mapper.mapStackTrace; } if (message['exceptPackages'] != null) { diff --git a/lib/src/util/stack_trace_mapper.dart b/lib/src/util/stack_trace_mapper.dart index 11a6866e0..f632dbcc9 100644 --- a/lib/src/util/stack_trace_mapper.dart +++ b/lib/src/util/stack_trace_mapper.dart @@ -51,19 +51,16 @@ class StackTraceMapper { }; } - /// Returns a Future which will resolve to a [StackTraceMapper] contained in - /// the provided serialized representation. - static Future deserialize(Map serialized) async { + /// Returns a [StackTraceMapper] contained in the provided serialized + /// representation. + static StackTraceMapper deserialize(Map serialized) { String packageRoot = serialized['packageRoot'] as String ?? ''; return new StackTraceMapper(serialized['mapContents'], sdkRoot: Uri.parse(serialized['sdkRoot']), packageResolver: packageRoot.isNotEmpty - ? await new PackageResolver.root( - Uri.parse(serialized['packageRoot'])) - .asSync - : await new PackageResolver.config(_deserializePackageConfigMap( - serialized['packageConfigMap'])) - .asSync, + ? new SyncPackageResolver.root(Uri.parse(serialized['packageRoot'])) + : new SyncPackageResolver.config( + _deserializePackageConfigMap(serialized['packageConfigMap'])), mapUrl: Uri.parse(serialized['mapUrl'])); } From 87ff301b5619504a1f0b63cae245b190760aae88 Mon Sep 17 00:00:00 2001 From: Gary Roumanis Date: Mon, 19 Jun 2017 14:01:33 -0700 Subject: [PATCH 12/20] refactor test_chain file --- lib/src/frontend/stream_matcher.dart | 3 +- lib/src/frontend/test_chain.dart | 33 ++++++++++++++------- lib/src/frontend/throws_matcher.dart | 6 +--- lib/src/runner/plugin/platform_helpers.dart | 4 +-- lib/src/runner/remote_listener.dart | 27 ++++++++--------- lib/src/util/stack_trace_mapper.dart | 1 + 6 files changed, 40 insertions(+), 34 deletions(-) diff --git a/lib/src/frontend/stream_matcher.dart b/lib/src/frontend/stream_matcher.dart index 32a0c7362..9e166ca49 100644 --- a/lib/src/frontend/stream_matcher.dart +++ b/lib/src/frontend/stream_matcher.dart @@ -166,8 +166,7 @@ class _StreamMatcher extends AsyncMatcher implements StreamMatcher { return addBullet(event.asValue.value.toString()); } else { var error = event.asError; - var chain = testChain(error.stackTrace, - verbose: Invoker.current.liveTest.test.metadata.verboseTrace); + var chain = testChain(error.stackTrace); var text = "${error.error}\n$chain"; return prefixLines(text, " ", first: "! "); } diff --git a/lib/src/frontend/test_chain.dart b/lib/src/frontend/test_chain.dart index b242a8117..a7c684cf9 100644 --- a/lib/src/frontend/test_chain.dart +++ b/lib/src/frontend/test_chain.dart @@ -4,26 +4,39 @@ import 'package:stack_trace/stack_trace.dart'; -typedef StackTrace Mapper(StackTrace trace); +import '../backend/invoker.dart'; +import '../util/stack_trace_mapper.dart'; /// Converts [trace] into a Dart stack trace -Mapper currentMapper = (trace) => trace; +StackTraceMapper _mapper; /// The list of packages to fold when producing [StackTrace]s. -Set exceptPackages = new Set.from(['test', 'stream_channel']); +Set _exceptPackages = new Set.from(['test', 'stream_channel']); /// If non-empty, all packages not in this list will be folded when producing /// [StackTrace]s. -Set onlyPackages = new Set(); +Set _onlyPackages = new Set(); -/// Converts [stackTrace] to a [Chain] following the test's configuration. -Chain testChain(StackTrace stackTrace, {bool verbose: false}) { - var testTrace = currentMapper(stackTrace); +void configureTestChaining( + {StackTraceMapper mapper, + Set exceptPackages, + Set onlyPackages}) { + if (mapper != null) _mapper = mapper; + if (exceptPackages != null) _exceptPackages = exceptPackages; + if (onlyPackages != null) _onlyPackages = onlyPackages; +} + +Chain terseChain(StackTrace stackTrace, {bool verbose: false}) { + var testTrace = _mapper?.mapStackTrace(stackTrace) ?? stackTrace; if (verbose) return new Chain.forTrace(testTrace); return new Chain.forTrace(testTrace).foldFrames((frame) { - if (onlyPackages.isNotEmpty) { - return !onlyPackages.contains(frame.package); + if (_onlyPackages.isNotEmpty) { + return !_onlyPackages.contains(frame.package); } - return exceptPackages.contains(frame.package); + return _exceptPackages.contains(frame.package); }, terse: true); } + +/// Converts [stackTrace] to a [Chain] following the test's configuration. +Chain testChain(StackTrace stackTrace) => terseChain(stackTrace, + verbose: Invoker.current.liveTest.test.metadata.verboseTrace); diff --git a/lib/src/frontend/throws_matcher.dart b/lib/src/frontend/throws_matcher.dart index 97ddcd70a..75c5d230b 100644 --- a/lib/src/frontend/throws_matcher.dart +++ b/lib/src/frontend/throws_matcher.dart @@ -95,11 +95,7 @@ class Throws extends AsyncMatcher { var buffer = new StringBuffer(); buffer.writeln(indent(prettyPrint(error), first: 'threw ')); if (trace != null) { - buffer.writeln(indent( - testChain(trace, - verbose: Invoker.current.liveTest.test.metadata.verboseTrace) - .toString(), - first: 'stack ')); + buffer.writeln(indent(testChain(trace).toString(), first: 'stack ')); } if (result.isNotEmpty) buffer.writeln(indent(result, first: 'which ')); return buffer.toString().trimRight(); diff --git a/lib/src/runner/plugin/platform_helpers.dart b/lib/src/runner/plugin/platform_helpers.dart index 616c34fb9..7556f55b5 100644 --- a/lib/src/runner/plugin/platform_helpers.dart +++ b/lib/src/runner/plugin/platform_helpers.dart @@ -60,8 +60,8 @@ Future deserializeSuite( 'collectTraces': Configuration.current.reporter == 'json', 'noRetry': Configuration.current.noRetry, 'stackTraceMapper': mapper?.serialize(), - 'exceptPackages': Configuration.current.foldTraceExcept, - 'onlyPackages': Configuration.current.foldTraceOnly, + 'foldTraceExcept': Configuration.current.foldTraceExcept, + 'foldTraceOnly': Configuration.current.foldTraceOnly, }); var completer = new Completer(); diff --git a/lib/src/runner/remote_listener.dart b/lib/src/runner/remote_listener.dart index ddebd87f4..2658882df 100644 --- a/lib/src/runner/remote_listener.dart +++ b/lib/src/runner/remote_listener.dart @@ -79,20 +79,10 @@ class RemoteListener { collectTraces: message['collectTraces'], noRetry: message['noRetry']); - if (message['stackTraceMapper'] != null) { - var mapper = StackTraceMapper.deserialize(message['stackTraceMapper']); - currentMapper = mapper.mapStackTrace; - } - if (message['exceptPackages'] != null) { - if (message['exceptPackages'].isNotEmpty) { - exceptPackages = new Set.from(message["exceptPackages"]); - } - } - if (message['onlyPackages'] != null) { - if (message['onlyPackages'].isNotEmpty) { - onlyPackages = new Set.from(message["onlyPackages"]); - } - } + configureTestChaining( + mapper: StackTraceMapper.deserialize(message['stackTraceMapper']), + exceptPackages: _setFromSerializedList(message['foldTraceExcept']), + onlyPackages: _setFromSerializedList(message['foldTraceOnly'])); await declarer.declare(main); @@ -112,6 +102,13 @@ class RemoteListener { return controller.foreign; } + /// Returns a [Set] from a JSON serialized list. + static Set _setFromSerializedList(List list) { + if (list == null) return null; + if (list.isEmpty) return null; + return new Set.from(list); + } + /// Sends a message over [channel] indicating that the tests failed to load. /// /// [message] should describe the failure. @@ -202,7 +199,7 @@ class RemoteListener { "type": "error", "error": RemoteException.serialize( asyncError.error, - testChain(asyncError.stackTrace, + terseChain(asyncError.stackTrace, verbose: liveTest.test.metadata.verboseTrace)) }); }); diff --git a/lib/src/util/stack_trace_mapper.dart b/lib/src/util/stack_trace_mapper.dart index f632dbcc9..f1d6d95b2 100644 --- a/lib/src/util/stack_trace_mapper.dart +++ b/lib/src/util/stack_trace_mapper.dart @@ -54,6 +54,7 @@ class StackTraceMapper { /// Returns a [StackTraceMapper] contained in the provided serialized /// representation. static StackTraceMapper deserialize(Map serialized) { + if (serialized == null) return null; String packageRoot = serialized['packageRoot'] as String ?? ''; return new StackTraceMapper(serialized['mapContents'], sdkRoot: Uri.parse(serialized['sdkRoot']), From 8de90cec121d6ec089f9007774056b0d1b527752 Mon Sep 17 00:00:00 2001 From: Gary Roumanis Date: Mon, 19 Jun 2017 14:05:34 -0700 Subject: [PATCH 13/20] account for null invoker --- lib/src/frontend/test_chain.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/frontend/test_chain.dart b/lib/src/frontend/test_chain.dart index a7c684cf9..99cccb4e6 100644 --- a/lib/src/frontend/test_chain.dart +++ b/lib/src/frontend/test_chain.dart @@ -39,4 +39,4 @@ Chain terseChain(StackTrace stackTrace, {bool verbose: false}) { /// Converts [stackTrace] to a [Chain] following the test's configuration. Chain testChain(StackTrace stackTrace) => terseChain(stackTrace, - verbose: Invoker.current.liveTest.test.metadata.verboseTrace); + verbose: Invoker.current?.liveTest?.test?.metadata?.verboseTrace ?? true); From 2582c9919b218bc27f2f875849b3b7af47b3e234 Mon Sep 17 00:00:00 2001 From: Gary Roumanis Date: Tue, 20 Jun 2017 15:35:19 -0700 Subject: [PATCH 14/20] more review changes --- lib/src/frontend/test_chain.dart | 14 ++++++-- lib/src/runner/configuration.dart | 37 +++++++++------------ lib/src/runner/configuration/load.dart | 14 +++++--- lib/src/runner/plugin/platform_helpers.dart | 4 +-- lib/src/runner/remote_listener.dart | 19 +++++++---- lib/src/util/stack_trace_mapper.dart | 4 +-- 6 files changed, 53 insertions(+), 39 deletions(-) diff --git a/lib/src/frontend/test_chain.dart b/lib/src/frontend/test_chain.dart index 99cccb4e6..41bd9e74c 100644 --- a/lib/src/frontend/test_chain.dart +++ b/lib/src/frontend/test_chain.dart @@ -10,13 +10,19 @@ import '../util/stack_trace_mapper.dart'; /// Converts [trace] into a Dart stack trace StackTraceMapper _mapper; -/// The list of packages to fold when producing [StackTrace]s. +/// The list of packages to fold when producing [Chain]s. Set _exceptPackages = new Set.from(['test', 'stream_channel']); /// If non-empty, all packages not in this list will be folded when producing -/// [StackTrace]s. +/// [Chain]s. Set _onlyPackages = new Set(); +/// Configure the resources used for test chaining. +/// +/// [mapper] is used to convert traces into Dart stack traces. +/// [exceptPackages] is the list of packages to fold when producing a [Chain]. +/// [onlyPackages] is the list of packages to keep in a [Chain]. If non-empty, +/// all packages not in this will be folded. void configureTestChaining( {StackTraceMapper mapper, Set exceptPackages, @@ -26,6 +32,10 @@ void configureTestChaining( if (onlyPackages != null) _onlyPackages = onlyPackages; } +/// Returns [stackTrace] converted to a [Chain] with all irrelevant frames +/// folded together. +/// +/// If [verbose] is `true`, returns the chain for [stackTrace] unmodified. Chain terseChain(StackTrace stackTrace, {bool verbose: false}) { var testTrace = _mapper?.mapStackTrace(stackTrace) ?? stackTrace; if (verbose) return new Chain.forTrace(testTrace); diff --git a/lib/src/runner/configuration.dart b/lib/src/runner/configuration.dart index bc2f1c2e4..e6db33aba 100644 --- a/lib/src/runner/configuration.dart +++ b/lib/src/runner/configuration.dart @@ -98,13 +98,13 @@ class Configuration { final int totalShards; /// The list of packages to fold when producing [StackTrace]s. - List get foldTraceExcept => _foldTraceExcept ?? []; - final List _foldTraceExcept; + Set get foldTraceExcept => _foldTraceExcept ?? new Set(); + final Set _foldTraceExcept; /// If non-empty, all packages not in this list will be folded when producing /// [StackTrace]s. - List get foldTraceOnly => _foldTraceOnly ?? []; - final List _foldTraceOnly; + Set get foldTraceOnly => _foldTraceOnly ?? new Set(); + final Set _foldTraceOnly; /// The paths from which to load tests. List get paths => _paths ?? ["test"]; @@ -322,8 +322,11 @@ class Configuration { : Uri.parse("http://localhost:$pubServePort"), _concurrency = concurrency, _paths = _list(paths), - _foldTraceExcept = _list(foldTraceExcept), - _foldTraceOnly = _list(foldTraceOnly), + _foldTraceExcept = foldTraceExcept != null + ? new Set.from(_list(foldTraceExcept)) + : null, + _foldTraceOnly = + foldTraceOnly != null ? new Set.from(_list(foldTraceOnly)) : null, _filename = filename, chosenPresets = new UnmodifiableSetView(chosenPresets?.toSet() ?? new Set()), @@ -386,29 +389,21 @@ class Configuration { if (this == Configuration.empty) return other; if (other == Configuration.empty) return this; - List foldTraceOnly = other._foldTraceOnly ?? _foldTraceOnly; + var foldTraceOnly = other._foldTraceOnly ?? _foldTraceOnly; if (other._foldTraceOnly != null && _foldTraceExcept != null) { - foldTraceOnly = new Set.from(other._foldTraceExcept) - .difference(new Set.from(_foldTraceExcept)) - .toList(); + foldTraceOnly = other._foldTraceOnly.difference(_foldTraceExcept); } - if (_foldTraceOnly != null && other._foldTraceExcept != null) { - foldTraceOnly = new Set.from(_foldTraceExcept) - .difference(new Set.from(other._foldTraceExcept)) - .toList(); + if (other._foldTraceExcept != null && _foldTraceOnly != null) { + foldTraceOnly = _foldTraceOnly.difference(other._foldTraceExcept); } if (other._foldTraceOnly != null && _foldTraceOnly != null) { - foldTraceOnly = new Set.from(other._foldTraceOnly) - .intersection(new Set.from(_foldTraceOnly)) - .toList(); + foldTraceOnly = other._foldTraceOnly.intersection(_foldTraceOnly); } - List foldTraceExcept = other._foldTraceExcept ?? _foldTraceExcept; + var foldTraceExcept = other._foldTraceExcept ?? _foldTraceExcept; if (other._foldTraceExcept != null && _foldTraceExcept != null) { - foldTraceExcept = new Set.from(other._foldTraceExcept) - .union(new Set.from(_foldTraceExcept)) - .toList(); + foldTraceExcept = other._foldTraceExcept.union(_foldTraceExcept); } var result = new Configuration._( diff --git a/lib/src/runner/configuration/load.dart b/lib/src/runner/configuration/load.dart index c26db4a3b..9c6ccedf1 100644 --- a/lib/src/runner/configuration/load.dart +++ b/lib/src/runner/configuration/load.dart @@ -266,14 +266,18 @@ class _ConfigurationLoader { excludeTags: excludeTags); } + /// Returns a map representation of the `fold_stack_frames` configuration. + /// + /// The key `except` will correspond to the list of packages to fold. + /// The key `only` will correspond to the list of packages to keep in a + /// test [Chain]. Map _loadFoldedStackFrames() { - bool foldOptionSet = false; + var foldOptionSet = false; return _getMap("fold_stack_frames", key: (keyNode) { _validate(keyNode, "Must be a string", (value) => value is String); - if (keyNode.value != "only" && keyNode.value != "except") { - throw new SourceSpanFormatException( - 'Must be "only" or "except".', keyNode.span, _source); - } + _validate(keyNode, 'Must be "only" or "except".', + (value) => value == "only" || value == "except"); + if (foldOptionSet) { throw new SourceSpanFormatException( 'Can only contain one of "only" or "except".', diff --git a/lib/src/runner/plugin/platform_helpers.dart b/lib/src/runner/plugin/platform_helpers.dart index 7556f55b5..841b0756a 100644 --- a/lib/src/runner/plugin/platform_helpers.dart +++ b/lib/src/runner/plugin/platform_helpers.dart @@ -60,8 +60,8 @@ Future deserializeSuite( 'collectTraces': Configuration.current.reporter == 'json', 'noRetry': Configuration.current.noRetry, 'stackTraceMapper': mapper?.serialize(), - 'foldTraceExcept': Configuration.current.foldTraceExcept, - 'foldTraceOnly': Configuration.current.foldTraceOnly, + 'foldTraceExcept': Configuration.current.foldTraceExcept.toList(), + 'foldTraceOnly': Configuration.current.foldTraceOnly.toList(), }); var completer = new Completer(); diff --git a/lib/src/runner/remote_listener.dart b/lib/src/runner/remote_listener.dart index 2658882df..a61fd0edf 100644 --- a/lib/src/runner/remote_listener.dart +++ b/lib/src/runner/remote_listener.dart @@ -48,6 +48,8 @@ class RemoteListener { new StreamChannelController(allowForeignErrors: false, sync: true); var channel = new MultiChannel(controller.local); + var verboseChain = true; + var printZone = hidePrints ? null : Zone.current; runZoned(() async { var main; @@ -57,7 +59,7 @@ class RemoteListener { _sendLoadException(channel, "No top-level main() function defined."); return; } catch (error, stackTrace) { - _sendError(channel, error, stackTrace); + _sendError(channel, error, stackTrace, verboseChain); return; } @@ -74,6 +76,7 @@ class RemoteListener { if (message['asciiGlyphs'] ?? false) glyph.ascii = true; var metadata = new Metadata.deserialize(message['metadata']); + verboseChain = metadata.verboseTrace; var declarer = new Declarer( metadata: metadata, collectTraces: message['collectTraces'], @@ -81,8 +84,8 @@ class RemoteListener { configureTestChaining( mapper: StackTraceMapper.deserialize(message['stackTraceMapper']), - exceptPackages: _setFromSerializedList(message['foldTraceExcept']), - onlyPackages: _setFromSerializedList(message['foldTraceOnly'])); + exceptPackages: _deserializeSet(message['foldTraceExcept']), + onlyPackages: _deserializeSet(message['foldTraceOnly'])); await declarer.declare(main); @@ -93,7 +96,7 @@ class RemoteListener { platform: platform, os: os, path: message['path']); new RemoteListener._(suite, printZone)._listen(channel); }, onError: (error, stackTrace) { - _sendError(channel, error, stackTrace); + _sendError(channel, error, stackTrace, verboseChain); }, zoneSpecification: new ZoneSpecification(print: (_, __, ___, line) { if (printZone != null) printZone.print(line); channel.sink.add({"type": "print", "line": line}); @@ -103,7 +106,7 @@ class RemoteListener { } /// Returns a [Set] from a JSON serialized list. - static Set _setFromSerializedList(List list) { + static Set _deserializeSet(List list) { if (list == null) return null; if (list.isEmpty) return null; return new Set.from(list); @@ -117,10 +120,12 @@ class RemoteListener { } /// Sends a message over [channel] indicating an error from user code. - static void _sendError(StreamChannel channel, error, StackTrace stackTrace) { + static void _sendError( + StreamChannel channel, error, StackTrace stackTrace, bool verboseChain) { channel.sink.add({ "type": "error", - "error": RemoteException.serialize(error, testChain(stackTrace)) + "error": RemoteException.serialize( + error, terseChain(stackTrace, verbose: verboseChain)) }); } diff --git a/lib/src/util/stack_trace_mapper.dart b/lib/src/util/stack_trace_mapper.dart index f1d6d95b2..52375d9f1 100644 --- a/lib/src/util/stack_trace_mapper.dart +++ b/lib/src/util/stack_trace_mapper.dart @@ -2,9 +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 'package:collection/collection.dart'; - import 'package:package_resolver/package_resolver.dart'; import 'package:source_map_stack_trace/source_map_stack_trace.dart' as mapper; import 'package:source_maps/source_maps.dart'; @@ -12,6 +10,8 @@ import 'package:source_maps/source_maps.dart'; /// A class for mapping JS stack traces to Dart stack traces using source maps. class StackTraceMapper { /// The parsed source map. + /// + /// This is initialized lazily in `mapStackTrace()`. Mapping _mapping; /// The package resolution information passed to dart2js. From cd33dc9c6e4bd5fbe9ccad5bb6bd92a1cd60bc91 Mon Sep 17 00:00:00 2001 From: Gary Roumanis Date: Tue, 20 Jun 2017 16:52:21 -0700 Subject: [PATCH 15/20] more review changes --- lib/src/runner/configuration.dart | 15 ++++++++++----- lib/src/runner/configuration/load.dart | 2 +- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/src/runner/configuration.dart b/lib/src/runner/configuration.dart index e6db33aba..d5bf0dffb 100644 --- a/lib/src/runner/configuration.dart +++ b/lib/src/runner/configuration.dart @@ -322,11 +322,8 @@ class Configuration { : Uri.parse("http://localhost:$pubServePort"), _concurrency = concurrency, _paths = _list(paths), - _foldTraceExcept = foldTraceExcept != null - ? new Set.from(_list(foldTraceExcept)) - : null, - _foldTraceOnly = - foldTraceOnly != null ? new Set.from(_list(foldTraceOnly)) : null, + _foldTraceExcept = _set(foldTraceExcept), + _foldTraceOnly = _set(foldTraceOnly), _filename = filename, chosenPresets = new UnmodifiableSetView(chosenPresets?.toSet() ?? new Set()), @@ -367,6 +364,14 @@ class Configuration { return list; } + /// Returns a set from [input]. + static Set _set(Iterable input) { + if (input == null) return null; + var set = new Set.from(input); + if (set.isEmpty) return null; + return set; + } + /// Returns an unmodifiable copy of [input] or an empty unmodifiable map. static Map/**/ _map/**/(Map/**/ input) { if (input == null || input.isEmpty) return const {}; diff --git a/lib/src/runner/configuration/load.dart b/lib/src/runner/configuration/load.dart index 9c6ccedf1..19e3d7ac5 100644 --- a/lib/src/runner/configuration/load.dart +++ b/lib/src/runner/configuration/load.dart @@ -271,7 +271,7 @@ class _ConfigurationLoader { /// The key `except` will correspond to the list of packages to fold. /// The key `only` will correspond to the list of packages to keep in a /// test [Chain]. - Map _loadFoldedStackFrames() { + Map> _loadFoldedStackFrames() { var foldOptionSet = false; return _getMap("fold_stack_frames", key: (keyNode) { _validate(keyNode, "Must be a string", (value) => value is String); From cab199ae8747865b8ccf9258b52d1806fbbb84fd Mon Sep 17 00:00:00 2001 From: Gary Roumanis Date: Thu, 22 Jun 2017 16:16:28 -0700 Subject: [PATCH 16/20] package validation --- lib/src/runner/configuration/load.dart | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/lib/src/runner/configuration/load.dart b/lib/src/runner/configuration/load.dart index 19e3d7ac5..d9845f933 100644 --- a/lib/src/runner/configuration/load.dart +++ b/lib/src/runner/configuration/load.dart @@ -21,6 +21,19 @@ import '../configuration.dart'; import '../configuration/suite.dart'; import 'reporters.dart'; +/// A regular expression matching a Dart identifier. +/// +/// This also matches a package name, since they must be Dart identifiers. +final identifierRegExp = new RegExp(r"[a-zA-Z_]\w*"); + +/// A regular expression matching allowed package names. +/// +/// This allows dot-separated valid Dart identifiers. The dots are there for +/// compatibility with Google's internal Dart packages, but they may not be used +/// when publishing a package to pub.dartlang.org. +final _packageName = new RegExp( + "^${identifierRegExp.pattern}(\\.${identifierRegExp.pattern})*\$"); + /// Loads configuration information from a YAML file at [path]. /// /// If [global] is `true`, this restricts the configuration file to only rules @@ -292,6 +305,11 @@ class _ConfigurationLoader { throw new SourceSpanFormatException( 'Folded packages must be strings.', valueNode.span, _source); } + if ((valueNode as YamlList) + .any((value) => !_packageName.hasMatch(value))) { + throw new SourceSpanFormatException( + 'Invalid package identifier', valueNode.span, _source); + } return valueNode.value; }); } From 70b73429f4c9dbd3282f3e5664efab06b5146233 Mon Sep 17 00:00:00 2001 From: Gary Roumanis Date: Thu, 22 Jun 2017 16:31:10 -0700 Subject: [PATCH 17/20] better validate --- lib/src/runner/configuration/load.dart | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/lib/src/runner/configuration/load.dart b/lib/src/runner/configuration/load.dart index d9845f933..49ca222c2 100644 --- a/lib/src/runner/configuration/load.dart +++ b/lib/src/runner/configuration/load.dart @@ -300,16 +300,19 @@ class _ConfigurationLoader { foldOptionSet = true; return keyNode.value; }, value: (valueNode) { - if (valueNode is! YamlList || - (valueNode as YamlList).any((value) => value is! String)) { - throw new SourceSpanFormatException( - 'Folded packages must be strings.', valueNode.span, _source); - } - if ((valueNode as YamlList) - .any((value) => !_packageName.hasMatch(value))) { - throw new SourceSpanFormatException( - 'Invalid package identifier', valueNode.span, _source); - } + _validate( + valueNode, + "Folded packages must be strings", + (valueList) => + valueList is YamlList && + !(valueList).any((value) => value is! String)); + + _validate( + valueNode, + "Invalid package identifier", + (valueList) => + !(valueList).any((value) => !_packageName.hasMatch(value))); + return valueNode.value; }); } From 9f7c729706de23626ad70aee0798c3498b53d548 Mon Sep 17 00:00:00 2001 From: Gary Roumanis Date: Thu, 22 Jun 2017 16:35:19 -0700 Subject: [PATCH 18/20] different conditionals --- lib/src/runner/configuration.dart | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/lib/src/runner/configuration.dart b/lib/src/runner/configuration.dart index d5bf0dffb..06cc76cd7 100644 --- a/lib/src/runner/configuration.dart +++ b/lib/src/runner/configuration.dart @@ -395,20 +395,19 @@ class Configuration { if (other == Configuration.empty) return this; var foldTraceOnly = other._foldTraceOnly ?? _foldTraceOnly; - if (other._foldTraceOnly != null && _foldTraceExcept != null) { - foldTraceOnly = other._foldTraceOnly.difference(_foldTraceExcept); - } - - if (other._foldTraceExcept != null && _foldTraceOnly != null) { - foldTraceOnly = _foldTraceOnly.difference(other._foldTraceExcept); - } - if (other._foldTraceOnly != null && _foldTraceOnly != null) { - foldTraceOnly = other._foldTraceOnly.intersection(_foldTraceOnly); - } - var foldTraceExcept = other._foldTraceExcept ?? _foldTraceExcept; - if (other._foldTraceExcept != null && _foldTraceExcept != null) { - foldTraceExcept = other._foldTraceExcept.union(_foldTraceExcept); + if (_foldTraceOnly != null) { + if (other._foldTraceExcept != null) { + foldTraceOnly = _foldTraceOnly.difference(other._foldTraceExcept); + } else if (other._foldTraceOnly != null) { + foldTraceOnly = other._foldTraceOnly.intersection(_foldTraceOnly); + } + } else if (_foldTraceExcept != null) { + if (other._foldTraceOnly != null) { + foldTraceOnly = other._foldTraceOnly.difference(_foldTraceExcept); + } else if (other._foldTraceExcept != null) { + foldTraceExcept = other._foldTraceExcept.union(_foldTraceExcept); + } } var result = new Configuration._( From 2d90d0bac0b91dabb59a1b8253cb566f1f3d322c Mon Sep 17 00:00:00 2001 From: Gary Roumanis Date: Thu, 22 Jun 2017 16:45:33 -0700 Subject: [PATCH 19/20] more review changes --- lib/src/runner/configuration/load.dart | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/src/runner/configuration/load.dart b/lib/src/runner/configuration/load.dart index 49ca222c2..0f5d238fe 100644 --- a/lib/src/runner/configuration/load.dart +++ b/lib/src/runner/configuration/load.dart @@ -302,16 +302,16 @@ class _ConfigurationLoader { }, value: (valueNode) { _validate( valueNode, - "Folded packages must be strings", + "Folded packages must be strings.", (valueList) => valueList is YamlList && - !(valueList).any((value) => value is! String)); + (valueList).every((value) => value is String)); _validate( valueNode, - "Invalid package identifier", + "Invalid package name.", (valueList) => - !(valueList).any((value) => !_packageName.hasMatch(value))); + (valueList).every((value) => _packageName.hasMatch(value))); return valueNode.value; }); From d1513de37f185d574bc26de8a2f4a6e6fb7ea4b4 Mon Sep 17 00:00:00 2001 From: Gary Roumanis Date: Thu, 22 Jun 2017 16:56:27 -0700 Subject: [PATCH 20/20] fix style --- lib/src/runner/configuration/load.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/src/runner/configuration/load.dart b/lib/src/runner/configuration/load.dart index 0f5d238fe..3a51edd57 100644 --- a/lib/src/runner/configuration/load.dart +++ b/lib/src/runner/configuration/load.dart @@ -305,13 +305,13 @@ class _ConfigurationLoader { "Folded packages must be strings.", (valueList) => valueList is YamlList && - (valueList).every((value) => value is String)); + valueList.every((value) => value is String)); _validate( valueNode, "Invalid package name.", (valueList) => - (valueList).every((value) => _packageName.hasMatch(value))); + valueList.every((value) => _packageName.hasMatch(value))); return valueNode.value; });