Conditional chaining (#584)

* plumb chain-stack-traces configuration through

* correctly plumb new flag through and add corresponding tests

* add example command

* chains -> chained

lib/src/backend/invoker.dart

@@ -289,6 +289,12 @@ class Invoker {
     _controller.addError(error, stackTrace);
     removeAllOutstandingCallbacks();
 
+    if (!liveTest.test.metadata.chainStackTraces) {
+      _printsOnFailure.add("Consider enabling the flag chain-stack-traces to "
+          "recieve more detailed exceptions.\n"
+          "For example, 'pub run test --chain-stack-traces'.");
+    }
+
     if (_printsOnFailure.isNotEmpty) {
       print(_printsOnFailure.join("\n\n"));
       _printsOnFailure.clear();
@@ -357,6 +363,6 @@ class Invoker {
               print: (self, parent, zone, line) =>
                   _controller.message(new Message.print(line))),
           onError: _handleError);
-    });
+    }, when: liveTest.test.metadata.chainStackTraces);
   }
 }

lib/src/backend/metadata.dart

@@ -42,6 +42,10 @@ class Metadata {
   bool get verboseTrace => _verboseTrace ?? false;
   final bool _verboseTrace;
 
+  /// Whether to chain stack traces.
+  bool get chainStackTraces => _chainStackTraces ?? true;
+  final bool _chainStackTraces;
+
   /// The user-defined tags attached to the test or suite.
   final Set<String> tags;
 
@@ -135,6 +139,7 @@ class Metadata {
       Timeout timeout,
       bool skip,
       bool verboseTrace,
+      bool chainStackTraces,
       String skipReason,
       Iterable<String> tags,
       Map<PlatformSelector, Metadata> onPlatform,
@@ -145,6 +150,7 @@ class Metadata {
         timeout: timeout,
         skip: skip,
         verboseTrace: verboseTrace,
+        chainStackTraces: chainStackTraces,
         skipReason: skipReason,
         tags: tags,
         onPlatform: onPlatform,
@@ -178,13 +184,15 @@ class Metadata {
       bool skip,
       this.skipReason,
       bool verboseTrace,
+      bool chainStackTraces,
       Iterable<String> tags,
       Map<PlatformSelector, Metadata> onPlatform,
       Map<BooleanSelector, Metadata> forTag})
       : testOn = testOn == null ? PlatformSelector.all : testOn,
         timeout = timeout == null ? const Timeout.factor(1) : timeout,
         _skip = skip,
         _verboseTrace = verboseTrace,
+        _chainStackTraces = chainStackTraces,
         tags = new UnmodifiableSetView(tags == null ? new Set() : tags.toSet()),
         onPlatform =
             onPlatform == null ? const {} : new UnmodifiableMapView(onPlatform),
@@ -201,6 +209,7 @@ class Metadata {
       Timeout timeout,
       skip,
       bool verboseTrace,
+      bool chainStackTraces,
       Map<String, dynamic> onPlatform,
       tags})
       : testOn = testOn == null
@@ -209,6 +218,7 @@ class Metadata {
         timeout = timeout == null ? const Timeout.factor(1) : timeout,
         _skip = skip == null ? null : skip != false,
         _verboseTrace = verboseTrace,
+        _chainStackTraces = chainStackTraces,
         skipReason = skip is String ? skip : null,
         onPlatform = _parseOnPlatform(onPlatform),
         tags = _parseTags(tags),
@@ -230,6 +240,7 @@ class Metadata {
         _skip = serialized['skip'],
         skipReason = serialized['skipReason'],
         _verboseTrace = serialized['verboseTrace'],
+        _chainStackTraces = serialized['chainStackTraces'],
         tags = new Set.from(serialized['tags']),
         onPlatform = new Map.fromIterable(serialized['onPlatform'],
             key: (pair) => new PlatformSelector.parse(pair.first),
@@ -272,6 +283,7 @@ class Metadata {
       skip: other._skip ?? _skip,
       skipReason: other.skipReason ?? skipReason,
       verboseTrace: other._verboseTrace ?? _verboseTrace,
+      chainStackTraces: other._chainStackTraces ?? _chainStackTraces,
       tags: tags.union(other.tags),
       onPlatform: mergeMaps(onPlatform, other.onPlatform,
           value: (metadata1, metadata2) => metadata1.merge(metadata2)),
@@ -284,6 +296,7 @@ class Metadata {
       Timeout timeout,
       bool skip,
       bool verboseTrace,
+      bool chainStackTraces,
       String skipReason,
       Map<PlatformSelector, Metadata> onPlatform,
       Set<String> tags,
@@ -292,6 +305,7 @@ class Metadata {
     timeout ??= this.timeout;
     skip ??= this._skip;
     verboseTrace ??= this._verboseTrace;
+    chainStackTraces ??= this._chainStackTraces;
     skipReason ??= this.skipReason;
     onPlatform ??= this.onPlatform;
     tags ??= this.tags;
@@ -301,6 +315,7 @@ class Metadata {
         timeout: timeout,
         skip: skip,
         verboseTrace: verboseTrace,
+        chainStackTraces: chainStackTraces,
         skipReason: skipReason,
         onPlatform: onPlatform,
         tags: tags,
@@ -335,6 +350,7 @@ class Metadata {
       'skip': _skip,
       'skipReason': skipReason,
       'verboseTrace': _verboseTrace,
+      'chainStackTraces': _chainStackTraces,
       'tags': tags.toList(),
       'onPlatform': serializedOnPlatform,
       'forTag': mapMap(forTag,

lib/src/runner/configuration.dart

@@ -213,6 +213,7 @@ class Configuration {
       // Test-level configuration
       Timeout timeout,
       bool verboseTrace,
+      bool chainStackTraces,
       bool skip,
       String skipReason,
       PlatformSelector testOn,
@@ -249,6 +250,7 @@ class Configuration {
             // Test-level configuration
             timeout: timeout,
             verboseTrace: verboseTrace,
+            chainStackTraces: chainStackTraces,
             skip: skip,
             skipReason: skipReason,
             testOn: testOn,
@@ -418,6 +420,7 @@ class Configuration {
       // Test-level configuration
       Timeout timeout,
       bool verboseTrace,
+      bool chainStackTraces,
       bool skip,
       String skipReason,
       PlatformSelector testOn,
@@ -451,6 +454,7 @@ class Configuration {
             onPlatform: onPlatform,
             timeout: timeout,
             verboseTrace: verboseTrace,
+            chainStackTraces: chainStackTraces,
             skip: skip,
             skipReason: skipReason,
             testOn: testOn,

lib/src/runner/configuration/args.dart

@@ -85,6 +85,12 @@ final ArgParser _parser = (() {
           'Implies --concurrency=1 and --timeout=none.\n'
           'Currently only supported for browser tests.',
       negatable: false);
+  parser.addFlag("chain-stack-traces",
+      help: 'Chained stack traces to provide greater exception details\n'
+          'especially for asynchronous code. It may be useful to disable\n'
+          'to provide improved test performance but at the cost of\n'
+          'debuggability.',
+      defaultsTo: true);
 
   parser.addSeparator("======== Output");
   parser.addOption("reporter",
@@ -192,6 +198,7 @@ class _Parser {
         help: _ifParsed('help'),
         version: _ifParsed('version'),
         verboseTrace: _ifParsed('verbose-trace'),
+        chainStackTraces: _ifParsed('chain-stack-traces'),
         jsTrace: _ifParsed('js-trace'),
         pauseAfterLoad: _ifParsed('pause-after-load'),
         color: _ifParsed('color'),

lib/src/runner/configuration/suite.dart

@@ -133,6 +133,7 @@ class SuiteConfiguration {
       // Test-level configuration
       Timeout timeout,
       bool verboseTrace,
+      bool chainStackTraces,
       bool skip,
       String skipReason,
       PlatformSelector testOn,
@@ -151,6 +152,7 @@ class SuiteConfiguration {
         metadata: new Metadata(
             timeout: timeout,
             verboseTrace: verboseTrace,
+            chainStackTraces: chainStackTraces,
             skip: skip,
             skipReason: skipReason,
             testOn: testOn,
@@ -254,6 +256,7 @@ class SuiteConfiguration {
       // Test-level configuration
       Timeout timeout,
       bool verboseTrace,
+      bool chainStackTraces,
       bool skip,
       String skipReason,
       PlatformSelector testOn,
@@ -272,6 +275,7 @@ class SuiteConfiguration {
         metadata: _metadata.change(
             timeout: timeout,
             verboseTrace: verboseTrace,
+            chainStackTraces: chainStackTraces,
             skip: skip,
             skipReason: skipReason,
             testOn: testOn,

test/backend/invoker_test.dart

@@ -570,6 +570,25 @@ void main() {
     });
   });
 
+  group("chainStackTraces", () {
+    test(
+        "if disabled, directs users to run with the flag enabled when "
+        "failures occur", () {
+      expect(() async {
+        var liveTest = _localTest(() {
+          expect(true, isFalse);
+        }, metadata: new Metadata(chainStackTraces: false))
+            .load(suite);
+        liveTest.onError.listen(expectAsync1((_) {}, count: 1));
+
+        await liveTest.run();
+      },
+          prints("Consider enabling the flag chain-stack-traces to "
+              "recieve more detailed exceptions.\n"
+              "For example, 'pub run test --chain-stack-traces'.\n"));
+    });
+  });
+
   group("printOnFailure:", () {
     test("doesn't print anything if the test succeeds", () {
       expect(() async {

test/runner/runner_test.dart

@@ -33,6 +33,20 @@ void main() {
 }
 """;
 
+final _asyncFailure = """
+import 'dart:async';
+
+import 'package:test/test.dart';
+
+void main() {
+  test("failure", () async{
+    await new Future((){});
+    await new Future((){});
+    throw "oh no";
+  });
+}
+""";
+
 final _defaultConcurrency = math.max(1, Platform.numberOfProcessors ~/ 2);
 
 final _browsers =
@@ -44,52 +58,58 @@ final _browsers =
 final _usage = """
 Usage: pub run test [files or directories...]
 
--h, --help                     Shows this usage information.
-    --version                  Shows the package's version.
+-h, --help                       Shows this usage information.
+    --version                    Shows the package's version.
 
 ======== Selecting Tests
--n, --name                     A substring of the name of the test to run.
-                               Regular expression syntax is supported.
-                               If passed multiple times, tests must match all substrings.
+-n, --name                       A substring of the name of the test to run.
+                                 Regular expression syntax is supported.
+                                 If passed multiple times, tests must match all substrings.
 
--N, --plain-name               A plain-text substring of the name of the test to run.
-                               If passed multiple times, tests must match all substrings.
+-N, --plain-name                 A plain-text substring of the name of the test to run.
+                                 If passed multiple times, tests must match all substrings.
 
--t, --tags                     Run only tests with all of the specified tags.
-                               Supports boolean selector syntax.
+-t, --tags                       Run only tests with all of the specified tags.
+                                 Supports boolean selector syntax.
 
--x, --exclude-tags             Don't run tests with any of the specified tags.
-                               Supports boolean selector syntax.
+-x, --exclude-tags               Don't run tests with any of the specified tags.
+                                 Supports boolean selector syntax.
 
-    --[no-]run-skipped         Run skipped tests instead of skipping them.
+    --[no-]run-skipped           Run skipped tests instead of skipping them.
 
 ======== Running Tests
--p, --platform                 The platform(s) on which to run the tests.
-                               $_browsers
+-p, --platform                   The platform(s) on which to run the tests.
+                                 $_browsers
 
--P, --preset                   The configuration preset(s) to use.
--j, --concurrency=<threads>    The number of concurrent test suites run.
-                               (defaults to "$_defaultConcurrency")
+-P, --preset                     The configuration preset(s) to use.
+-j, --concurrency=<threads>      The number of concurrent test suites run.
+                                 (defaults to "$_defaultConcurrency")
 
-    --pub-serve=<port>         The port of a pub serve instance serving "test/".
-    --timeout                  The default test timeout. For example: 15s, 2x, none
-                               (defaults to "30s")
+    --pub-serve=<port>           The port of a pub serve instance serving "test/".
+    --timeout                    The default test timeout. For example: 15s, 2x, none
+                                 (defaults to "30s")
 
-    --pause-after-load         Pauses for debugging before any tests execute.
-                               Implies --concurrency=1 and --timeout=none.
-                               Currently only supported for browser tests.
+    --pause-after-load           Pauses for debugging before any tests execute.
+                                 Implies --concurrency=1 and --timeout=none.
+                                 Currently only supported for browser tests.
+
+    --[no-]chain-stack-traces    Chained stack traces to provide greater exception details
+                                 especially for asynchronous code. It may be useful to disable
+                                 to provide improved test performance but at the cost of
+                                 debuggability.
+                                 (defaults to on)
 
 ======== Output
--r, --reporter                 The runner used to print test results.
+-r, --reporter                   The runner used to print test results.
 
-          [compact]            A single line, updated continuously.
-          [expanded]           A separate line for each update.
-          [json]               A machine-readable format (see https://goo.gl/gBsV1a).
+          [compact]              A single line, updated continuously.
+          [expanded]             A separate line for each update.
+          [json]                 A machine-readable format (see https://goo.gl/gBsV1a).
 
-    --verbose-trace            Whether to emit stack traces with core library frames.
-    --js-trace                 Whether to emit raw JavaScript stack traces for browser tests.
-    --[no-]color               Whether to use terminal colors.
-                               (auto-detected by default)
+    --verbose-trace              Whether to emit stack traces with core library frames.
+    --js-trace                   Whether to emit raw JavaScript stack traces for browser tests.
+    --[no-]color                 Whether to use terminal colors.
+                                 (auto-detected by default)
 """;
 
 void main() {
@@ -323,6 +343,27 @@ $_usage""");
   });
 
   group("runs failing tests", () {
+    test("defaults to chaining stack traces", () {
+      d.file("test.dart", _asyncFailure).create();
+
+      var test = runTest(["test.dart"]);
+      test.stdout.expect(consumeThrough(contains("asynchronous gap")));
+      test.shouldExit(1);
+    });
+
+    test("respects the chain-stack-traces flag", () {
+      d.file("test.dart", _asyncFailure).create();
+
+      var test = runTest(["test.dart", "--no-chain-stack-traces"]);
+      test.stdout.expect(containsInOrder([
+        "00:00 +0: failure",
+        "00:00 +0 -1: failure [E]",
+        "oh no",
+        "test.dart 9:5  main.<fn>",
+      ]));
+      test.shouldExit(1);
+    });
+
     test("defined in a single file", () {
       d.file("test.dart", _failure).create();