Configurable package folding (#650)

Author: grouma

CHANGELOG.md

@@ -1,3 +1,8 @@
+## 0.12.23
+
+* Add a `fold_stack_frames` field for `dart_test.yaml`. This will
+  allow users to customize which packages' frames are folded.
+  
 ## 0.12.22+2
 
 * Properly allocate ports when debugging Chrome and Dartium in an IPv6-only

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,35 @@ reporter: expanded
 This field is not supported in the
 [global configuration file](#global-configuration).
 
+### `fold_stack_frames`
+
+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:
+  except:
+  - test 
+  - stream_channel 
+```
+
+Sample stack trace, note the absence of `package:test`
+and `package:stream_channel`:
+```
+test/sample_test.dart 7:5   main.<fn>
+===== asynchronous gap ===========================
+dart:async                  _Completer.completeError
+test/sample_test.dart 8:3   main.<fn>
+===== asynchronous gap ===========================
+dart:async                  _asyncThenWrapperHelper
+test/sample_test.dart 5:27  main.<fn>
+```
+
+
 ## Configuring Tags
 
 ### `tags`

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,8 @@ 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);
+          var text = "${error.error}\n$chain";
           return prefixLines(text, "  ", first: "! ");
         }
       }).join("\n");

lib/src/frontend/test_chain.dart

@@ -0,0 +1,52 @@
+// 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';
+
+import '../backend/invoker.dart';
+import '../util/stack_trace_mapper.dart';
+
+/// Converts [trace] into a Dart stack trace
+StackTraceMapper _mapper;
+
+/// The list of packages to fold when producing [Chain]s.
+Set<String> _exceptPackages = new Set.from(['test', 'stream_channel']);
+
+/// If non-empty, all packages not in this list will be folded when producing
+/// [Chain]s.
+Set<String> _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<String> exceptPackages,
+    Set<String> onlyPackages}) {
+  if (mapper != null) _mapper = mapper;
+  if (exceptPackages != null) _exceptPackages = exceptPackages;
+  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);
+  return new Chain.forTrace(testTrace).foldFrames((frame) {
+    if (_onlyPackages.isNotEmpty) {
+      return !_onlyPackages.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 ?? true);

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.
 ///

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 (_) {

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.
+  Set<String> get foldTraceExcept => _foldTraceExcept ?? new Set();
+  final Set<String> _foldTraceExcept;
+
+  /// If non-empty, all packages not in this list will be folded when producing
+  /// [StackTrace]s.
+  Set<String> get foldTraceOnly => _foldTraceOnly ?? new Set();
+  final Set<String> _foldTraceOnly;
+
   /// The paths from which to load tests.
   List<String> get paths => _paths ?? ["test"];
   final List<String> _paths;
@@ -198,6 +207,8 @@ class Configuration {
       int shardIndex,
       int totalShards,
       Iterable<String> paths,
+      Iterable<String> foldTraceExcept,
+      Iterable<String> foldTraceOnly,
       Glob filename,
       Iterable<String> chosenPresets,
       Map<String, Configuration> presets,
@@ -238,6 +249,8 @@ class Configuration {
         shardIndex: shardIndex,
         totalShards: totalShards,
         paths: paths,
+        foldTraceExcept: foldTraceExcept,
+        foldTraceOnly: foldTraceOnly,
         filename: filename,
         chosenPresets: chosenPresetSet,
         presets: _withChosenPresets(presets, chosenPresetSet),
@@ -290,6 +303,8 @@ class Configuration {
       this.shardIndex,
       this.totalShards,
       Iterable<String> paths,
+      Iterable<String> foldTraceExcept,
+      Iterable<String> foldTraceOnly,
       Glob filename,
       Iterable<String> chosenPresets,
       Map<String, Configuration> presets,
@@ -307,6 +322,8 @@ class Configuration {
             : Uri.parse("http://localhost:$pubServePort"),
         _concurrency = concurrency,
         _paths = _list(paths),
+        _foldTraceExcept = _set(foldTraceExcept),
+        _foldTraceOnly = _set(foldTraceOnly),
         _filename = filename,
         chosenPresets =
             new UnmodifiableSetView(chosenPresets?.toSet() ?? new Set()),
@@ -347,6 +364,14 @@ class Configuration {
     return list;
   }
 
+  /// Returns a set from [input].
+  static Set<T> _set<T>(Iterable<T> input) {
+    if (input == null) return null;
+    var set = new Set<T>.from(input);
+    if (set.isEmpty) return null;
+    return set;
+  }
+
   /// Returns an unmodifiable copy of [input] or an empty unmodifiable map.
   static Map/*<K, V>*/ _map/*<K, V>*/(Map/*<K, V>*/ input) {
     if (input == null || input.isEmpty) return const {};
@@ -369,6 +394,22 @@ class Configuration {
     if (this == Configuration.empty) return other;
     if (other == Configuration.empty) return this;
 
+    var foldTraceOnly = other._foldTraceOnly ?? _foldTraceOnly;
+    var foldTraceExcept = other._foldTraceExcept ?? _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._(
         help: other._help ?? _help,
         version: other._version ?? _version,
@@ -382,6 +423,8 @@ class Configuration {
         shardIndex: other.shardIndex ?? shardIndex,
         totalShards: other.totalShards ?? totalShards,
         paths: other._paths ?? _paths,
+        foldTraceExcept: foldTraceExcept,
+        foldTraceOnly: foldTraceOnly,
         filename: other._filename ?? _filename,
         chosenPresets: chosenPresets.union(other.chosenPresets),
         presets: _mergeConfigMaps(presets, other.presets),
@@ -412,6 +455,8 @@ class Configuration {
       int shardIndex,
       int totalShards,
       Iterable<String> paths,
+      Iterable<String> exceptPackages,
+      Iterable<String> onlyPackages,
       Glob filename,
       Iterable<String> chosenPresets,
       Map<String, Configuration> presets,
@@ -450,6 +495,8 @@ class Configuration {
         shardIndex: shardIndex ?? this.shardIndex,
         totalShards: totalShards ?? this.totalShards,
         paths: paths ?? _paths,
+        foldTraceExcept: exceptPackages ?? _foldTraceExcept,
+        foldTraceOnly: onlyPackages ?? _foldTraceOnly,
         filename: filename ?? _filename,
         chosenPresets: chosenPresets ?? this.chosenPresets,
         presets: presets ?? this.presets,

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
@@ -74,6 +87,7 @@ class _ConfigurationLoader {
   Configuration _loadGlobalTestConfig() {
     var verboseTrace = _getBool("verbose_trace");
     var chainStackTraces = _getBool("chain_stack_traces");
+    var foldStackFrames = _loadFoldedStackFrames();
     var jsTrace = _getBool("js_trace");
 
     var timeout = _parseValue("timeout", (value) => new Timeout.parse(value));
@@ -108,7 +122,9 @@ class _ConfigurationLoader {
             jsTrace: jsTrace,
             timeout: timeout,
             presets: presets,
-            chainStackTraces: chainStackTraces)
+            chainStackTraces: chainStackTraces,
+            foldTraceExcept: foldStackFrames["except"],
+            foldTraceOnly: foldStackFrames["only"])
         .merge(_extractPresets/*<PlatformSelector>*/(
             onPlatform, (map) => new Configuration(onPlatform: map)));
 
@@ -263,6 +279,44 @@ 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<String, List<String>> _loadFoldedStackFrames() {
+    var foldOptionSet = false;
+    return _getMap("fold_stack_frames", key: (keyNode) {
+      _validate(keyNode, "Must be a string", (value) => value is String);
+      _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".',
+            keyNode.span,
+            _source);
+      }
+      foldOptionSet = true;
+      return keyNode.value;
+    }, value: (valueNode) {
+      _validate(
+          valueNode,
+          "Folded packages must be strings.",
+          (valueList) =>
+              valueList is YamlList &&
+              valueList.every((value) => value is String));
+
+      _validate(
+          valueNode,
+          "Invalid package name.",
+          (valueList) =>
+              valueList.every((value) => _packageName.hasMatch(value)));
+
+      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)) {