Add a --pause-after-load flag for debugging.

Currently this only support windowed browsers and the runner can only be
unpaused from the command line.

See #294

[email protected]

Review URL: https://codereview.chromium.org//1248073003 .

Author: nex3

CHANGELOG.md

@@ -1,3 +1,9 @@
+## 0.12.4
+
+* Add a `--pause-after-load` flag that pauses the test runner after each suite
+  is loaded so that breakpoints and other debugging annotations can be added.
+  Currently this is only supported on browsers.
+
 ## 0.12.3+8
 
 * Fix an uncaught error that could crop up when killing the test runner process

lib/src/backend/live_test_controller.dart

@@ -151,7 +151,15 @@ class LiveTestController {
   }
 
   /// Emits a line printed by the test over [LiveTest.onPrint].
-  void print(String line) => _onPrintController.add(line);
+  void print(String line) {
+    if (_onPrintController.hasListener) {
+      _onPrintController.add(line);
+    } else {
+      // Make sure all prints get surfaced one way or another to aid in
+      // debugging.
+      Zone.ROOT.print(line);
+    }
+  }
 
   /// A wrapper for [_onRun] that ensures that it follows the guarantees for
   /// [LiveTest.run].

lib/src/executable.dart

@@ -16,6 +16,7 @@ import 'runner.dart';
 import 'runner/application_exception.dart';
 import 'runner/configuration.dart';
 import 'util/exit_codes.dart' as exit_codes;
+import 'util/io.dart';
 import 'utils.dart';
 
 /// A merged stream of all signals that tell the test runner to shut down
@@ -109,6 +110,7 @@ transformers:
     if (signalSubscription == null) return;
     signalSubscription.cancel();
     signalSubscription = null;
+    stdinLines.cancel(immediate: true);
     await runner.close();
   }
 

lib/src/runner.dart

@@ -7,20 +7,26 @@ library test.runner;
 import 'dart:async';
 import 'dart:io';
 
-import 'package:async/async.dart';
-
 import 'backend/metadata.dart';
+import 'backend/suite.dart';
+import 'backend/test_platform.dart';
 import 'runner/application_exception.dart';
 import 'runner/configuration.dart';
 import 'runner/engine.dart';
 import 'runner/load_exception.dart';
 import 'runner/load_suite.dart';
 import 'runner/loader.dart';
+import 'runner/reporter.dart';
 import 'runner/reporter/compact.dart';
 import 'runner/reporter/expanded.dart';
 import 'util/async_thunk.dart';
+import 'util/io.dart';
 import 'utils.dart';
 
+/// The set of platforms for which debug flags are (currently) not supported.
+final _debugUnsupportedPlatforms = new Set.from(
+    [TestPlatform.vm, TestPlatform.phantomJS, TestPlatform.contentShell]);
+
 /// A class that loads and runs tests based on a [Configuration].
 ///
 /// This maintains a [Loader] and an [Engine] and passes test suites from one to
@@ -36,13 +42,16 @@ class Runner {
   /// The engine that runs the test suites.
   final Engine _engine;
 
+  /// The reporter that's emitting the test runner's results.
+  final Reporter _reporter;
+
+  /// The subscription to the stream returned by [_loadSuites].
+  StreamSubscription _suiteSubscription;
+
   /// The thunk for ensuring [close] only runs once.
   final _closeThunk = new AsyncThunk();
   bool get _closed => _closeThunk.hasRun;
 
-  /// Whether [run] has been called.
-  bool _hasRun = false;
-
   /// Creates a new runner based on [configuration].
   factory Runner(Configuration configuration) {
     var metadata = new Metadata(
@@ -60,36 +69,42 @@ class Runner {
         ? CompactReporter.watch
         : ExpandedReporter.watch;
 
-    watch(
+    var reporter = watch(
         engine,
         color: configuration.color,
         verboseTrace: configuration.verboseTrace,
         printPath: configuration.paths.length > 1 ||
             new Directory(configuration.paths.single).existsSync(),
         printPlatform: configuration.platforms.length > 1);
 
-    return new Runner._(configuration, loader, engine);
+    return new Runner._(configuration, loader, engine, reporter);
   }
 
-  Runner._(this._configuration, this._loader, this._engine);
+  Runner._(this._configuration, this._loader, this._engine, this._reporter);
 
   /// Starts the runner.
   ///
   /// This starts running tests and printing their progress. It returns whether
   /// or not they ran successfully.
   Future<bool> run() async {
-    _hasRun = true;
-
     if (_closed) {
       throw new StateError("run() may not be called on a closed Runner.");
     }
 
+    var suites = _loadSuites();
+
     var success;
-    var results = await Future.wait([
-      _loadSuites(),
-      _engine.run()
-    ], eagerError: true);
-    success = results.last;
+    if (_configuration.pauseAfterLoad) {
+      // TODO(nweiz): disable timeouts when debugging.
+      success = await _loadThenPause(suites);
+    } else {
+      _suiteSubscription = suites.listen(_engine.suiteSink.add);
+      var results = await Future.wait([
+        _suiteSubscription.asFuture().then((_) => _engine.suiteSink.close()),
+        _engine.run()
+      ], eagerError: true);
+      success = results.last;
+    }
 
     if (_closed) return false;
 
@@ -118,54 +133,115 @@ class Runner {
   /// filesystem.
   Future close() => _closeThunk.run(() async {
     var timer;
-    if (_hasRun) {
+    if (!_engine.isIdle) {
       // Wait a bit to print this message, since printing it eagerly looks weird
       // if the tests then finish immediately.
       timer = new Timer(new Duration(seconds: 1), () {
-        // Print a blank line first to ensure that this doesn't interfere with
-        // the compact reporter's unfinished line.
-        print('');
+        // Pause the reporter while we print to ensure that we don't interfere
+        // with its output.
+        _reporter.pause();
         print("Waiting for current test(s) to finish.");
         print("Press Control-C again to terminate immediately.");
+        _reporter.resume();
       });
     }
 
+    if (_suiteSubscription != null) _suiteSubscription.cancel();
+    _suiteSubscription = null;
+
     // Make sure we close the engine *before* the loader. Otherwise,
     // LoadSuites provided by the loader may get into bad states.
     await _engine.close();
     if (timer != null) timer.cancel();
     await _loader.close();
   });
 
-  /// Load the test suites in [_configuration.paths] that match
-  /// [_configuration.pattern].
-  Future _loadSuites() async {
-    var group = new FutureGroup();
-
-    mergeStreams(_configuration.paths.map((path) {
+  /// Return a stream of [LoadSuite]s in [_configuration.paths].
+  ///
+  /// Only tests that match [_configuration.pattern] will be included in the
+  /// suites once they're loaded.
+  Stream<LoadSuite> _loadSuites() {
+    return mergeStreams(_configuration.paths.map((path) {
       if (new Directory(path).existsSync()) return _loader.loadDir(path);
       if (new File(path).existsSync()) return _loader.loadFile(path);
 
       return new Stream.fromIterable([
         new LoadSuite("loading $path", () =>
             throw new LoadException(path, 'Does not exist.'))
       ]);
-    })).listen((loadSuite) {
-      group.add(new Future.sync(() {
-        _engine.suiteSink.add(loadSuite.changeSuite((suite) {
-          if (_configuration.pattern == null) return suite;
-          return suite.change(tests: suite.tests.where(
-              (test) => test.name.contains(_configuration.pattern)));
-        }));
-      }));
-    }, onError: (error, stackTrace) {
-      group.add(new Future.error(error, stackTrace));
-    }, onDone: group.close);
-
-    await group.future;
-
-    // Once we've loaded all the suites, notify the engine that no more will be
-    // coming.
-    _engine.suiteSink.close();
+    })).map((loadSuite) {
+      return loadSuite.changeSuite((suite) {
+        if (_configuration.pattern == null) return suite;
+        return suite.change(tests: suite.tests.where((test) =>
+            test.name.contains(_configuration.pattern)));
+      });
+    });
+  }
+
+  /// Loads each suite in [suites] in order, pausing after load for platforms
+  /// that support debugging.
+  Future<bool> _loadThenPause(Stream<LoadSuite> suites) async {
+    var unsupportedPlatforms = _configuration.platforms
+        .where(_debugUnsupportedPlatforms.contains)
+        .map((platform) =>
+             platform == TestPlatform.vm ? "the Dart VM" : platform.name)
+        .toList();
+
+    if (unsupportedPlatforms.isNotEmpty) {
+      warn(
+          wordWrap("Debugging is currently unsupported on "
+              "${toSentence(unsupportedPlatforms)}."),
+          color: _configuration.color);
+    }
+
+    _suiteSubscription = suites.asyncMap((loadSuite) async {
+      // Make the underlying suite null so that the engine doesn't start running
+      // it immediately.
+      _engine.suiteSink.add(loadSuite.changeSuite((_) => null));
+
+      var suite = await loadSuite.suite;
+      if (suite == null) return;
+
+      await _pause(suite);
+      if (_closed) return;
+
+      _engine.suiteSink.add(suite);
+      await _engine.onIdle.first;
+    }).listen(null);
+
+    var results = await Future.wait([
+      _suiteSubscription.asFuture().then((_) => _engine.suiteSink.close()),
+      _engine.run()
+    ]);
+    return results.last;
+  }
+
+  /// Pauses the engine and the reporter so that the user can set breakpoints as
+  /// necessary.
+  ///
+  /// This is a no-op for test suites that aren't on platforms where debugging
+  /// is supported.
+  Future _pause(Suite suite) async {
+    if (suite.platform == null) return;
+    if (_debugUnsupportedPlatforms.contains(suite.platform)) return;
+
+    try {
+      _reporter.pause();
+
+      var bold = _configuration.color ? '\u001b[1m' : '';
+      var noColor = _configuration.color ? '\u001b[0m' : '';
+      print('');
+      print(wordWrap(
+          "${bold}The test runner is paused.${noColor} Open the dev console in "
+              "${suite.platform} and set breakpoints. Once you're finished, "
+              "return to this terminal and press Enter."));
+
+      // TODO(nweiz): Display something in the paused browsers indicating that
+      // they're paused.
+
+      await stdinLines.next;
+    } finally {
+      _reporter.resume();
+    }
   }
 }

lib/src/runner/configuration.dart

@@ -56,6 +56,11 @@ class Configuration {
         help: 'The port of a pub serve instance serving "test/".',
         hide: !supportsPubServe,
         valueHelp: 'port');
+    parser.addFlag("pause-after-load",
+        help: 'Pauses for debugging before any tests execute.\n'
+            'Implies --concurrency=1.\n'
+            'Currently only supported for browser tests.',
+        negatable: false);
     parser.addOption("reporter",
         abbr: 'r',
         help: 'The runner used to print test results.',
@@ -95,6 +100,9 @@ class Configuration {
   /// Dart-like traces.
   bool get jsTrace => _options['js-trace'];
 
+  /// Whether to pause for debugging after loading each test suite.
+  bool get pauseAfterLoad => _options['pause-after-load'];
+
   /// The package root for resolving "package:" URLs.
   String get packageRoot => _options['package-root'] == null
       ? p.join(p.current, 'packages')

lib/src/runner/engine.dart

@@ -16,7 +16,6 @@ import '../backend/live_test_controller.dart';
 import '../backend/state.dart';
 import '../backend/suite.dart';
 import '../backend/test.dart';
-import '../util/delegating_sink.dart';
 import 'load_suite.dart';
 
 /// An [Engine] manages a run that encompasses multiple test suites.
@@ -136,6 +135,13 @@ class Engine {
   /// This is separate from [active] because load tests aren't always surfaced.
   final _activeLoadTests = new List<LiveTest>();
 
+  /// Whether this engine is idle—that is, not currently executing a test.
+  bool get isIdle => _group.isIdle;
+
+  /// A broadcast stream that fires an event whenever [isIdle] switches from
+  /// `false` to `true`.
+  Stream get onIdle => _group.onIdle;
+
   /// Creates an [Engine] that will run all tests provided via [suiteSink].
   ///
   /// [concurrency] controls how many suites are run at once, and defaults to 1.

lib/src/runner/reporter.dart

@@ -0,0 +1,27 @@
+// 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.
+
+library test.runner.reporter;
+
+/// An interface for classes that watch the progress of an Engine and report it
+/// to the user.
+///
+/// A reporter should subscribe to the Engine's events as soon as it's created.
+abstract class Reporter {
+  /// Pauses the reporter's output.
+  ///
+  /// Subclasses should buffer any events from the engine while they're paused.
+  /// They should also ensure that this does nothing if the reporter is already
+  /// paused.
+  void pause();
+
+  /// Resumes the reporter's output after being [paused].
+  ///
+  /// Subclasses should ensure that this does nothing if the reporter isn't
+  /// paused.
+  void resume();
+
+  /// Cancels the reporter's output.
+  void cancel();
+}