Merge pull request #483 from dart-lang/json-debug

Add debugging support for the JSON reporter.

Author: nex3

CHANGELOG.md

@@ -1,3 +1,7 @@
+## 0.12.16
+
+* Allow tools to interact with browser debuggers using the JSON reporter.
+
 ## 0.12.15+12
 
 * Fix a race condition that could cause the runner to stall for up to three

doc/json_reporter.md

@@ -132,6 +132,36 @@ A suite event is emitted before any `GroupEvent`s for groups in a given test
 suite. This is the only event that contains the full metadata about a suite;
 future events will refer to the suite by its opaque ID.
 
+### DebugEvent
+
+```
+class DebugEvent extends Event {
+  String type = "debug";
+
+  /// The suite for which debug information is reported.
+  int suiteID;
+
+  /// The HTTP URL for the Dart Observatory, or `null` if the Observatory isn't
+  /// available for this suite.
+  String observatory;
+
+  /// The HTTP URL for the remote debugger for this suite's host page, or `null`
+  /// if no remote debugger is available for this suite.
+  String remoteDebugger;
+}
+```
+
+A debug event is emitted after (although not necessarily directly after) a
+`SuiteEvent`, and includes information about how to debug that suite. It's only
+emitted if the `--pause-after-load` flag is passed to the test runner.
+
+Note that the `remoteDebugger` URL refers to a remote debugger whose protocol
+may differ based on the browser the suite is running on. You can tell which
+protocol is in use by the `Suite.platform` field for the suite with the given
+ID. Since the same browser instance is used for multiple suites, different
+suites may have the same `host` URL, although only one suite at a time will be
+active when `--pause-after-load` is passed.
+
 ### GroupEvent
 
 ```
@@ -410,3 +440,33 @@ class Metadata {
 ```
 
 The metadata class is deprecated and should not be used.
+
+## Remote Debugger APIs
+
+When running browser tests with `--pause-after-load`, the test package embeds a
+few APIs in the JavaScript context of the host page. These allow tools to
+control the debugging process in the same way a user might do from the command
+line. They can be accessed by connecting to the remote debugger using the
+[`DebugEvent.remoteDebugger`](#DebugEvent) URL.
+
+All APIs are defined as methods on the top-level `dartTest` object. The
+following methods are available:
+
+### `resume()`
+
+Calling `resume()` when the test runner is paused causes it to resume running
+tests. If the test runner is not paused, it won't do anything. When
+`--pause-after-load` is passed, the test runner will pause after loading each
+suite but before any tests are run.
+
+This gives external tools a chance to use the remote debugger protocol to set
+breakpoints before tests have begun executing. They can start the test runner
+with `--pause-after-load`, connect to the remote debugger using the
+[`DebugEvent.remoteDebugger`](#DebugEvent) URL, set breakpoints, then call
+`dartTest.resume()` in the host frame when they're finished.
+
+### `restartCurrent()`
+
+Calling `restartCurrent()` when the test runner is running a test causes it to
+re-run that test once it completes its current run. It's intended to be called
+when the browser is paused, as at a breakpoint.

json_reporter.schema.json

@@ -128,7 +128,22 @@
       "required": ["suite"],
       "properties": {
         "type": {"enum": ["suite"]},
-        "group": {"$ref": "#/definitions/Suite"}
+        "suite": {"$ref": "#/definitions/Suite"}
+      }
+    },
+
+    {
+      "title": "DebugEvent",
+      "required": ["suiteID"],
+      "properties": {
+        "type": {"enum": ["debug"]},
+        "suiteID": {"type": "integer", "minimum": 0},
+        "observatory": {
+          "oneOf": [{"type": "string", "format": "uri"}, {"type": "null"}]
+        },
+        "remoteDebugger": {
+          "oneOf": [{"type": "string", "format": "uri"}, {"type": "null"}]
+        }
       }
     },
 
@@ -194,7 +209,7 @@
           "not": {
             "enum": [
               "start", "testStart", "allSuites", "suite", "group", "print",
-              "error", "testDone", "done"
+              "error", "testDone", "done", "debug"
             ]
           }
         }

lib/src/runner/browser/browser.dart

@@ -25,18 +25,15 @@ abstract class Browser {
 
   /// The Observatory URL for this browser.
   ///
-  /// This will throw an [UnsupportedError] for browsers that aren't running the
-  /// Dart VM, and return `null` if the Observatory URL can't be found.
-  Future<Uri> get observatoryUrl =>
-      throw new UnsupportedError("$name doesn't support Observatory.");
+  /// This will return `null` for browsers that aren't running the Dart VM, or
+  /// if the Observatory URL can't be found.
+  Future<Uri> get observatoryUrl => null;
 
   /// The remote debugger URL for this browser.
   ///
-  /// This will throw an [UnsupportedError] for browsers that don't support
-  /// remote debugging, and return `null` if the remote debugging URL can't be
-  /// found.
-  Future<Uri> get remoteDebuggerUrl =>
-      throw new UnsupportedError("$name doesn't support remote debugging.");
+  /// This will return `null` for browsers that don't support remote debugging,
+  /// or if the remote debugging URL can't be found.
+  Future<Uri> get remoteDebuggerUrl => null;
 
   /// The underlying process.
   ///

lib/src/runner/browser/browser_manager.dart

@@ -68,6 +68,9 @@ class BrowserManager {
   /// screen.
   CancelableCompleter _pauseCompleter;
 
+  /// The controller for [_BrowserEnvironment.onRestart].
+  final _onRestartController = new StreamController();
+
   /// The environment to attach to each suite.
   Future<_BrowserEnvironment> _environment;
 
@@ -133,7 +136,7 @@ class BrowserManager {
       case TestPlatform.dartium: return new Dartium(url, debug: debug);
       case TestPlatform.contentShell:
         return new ContentShell(url, debug: debug);
-      case TestPlatform.chrome: return new Chrome(url);
+      case TestPlatform.chrome: return new Chrome(url, debug: debug);
       case TestPlatform.phantomJS: return new PhantomJS(url, debug: debug);
       case TestPlatform.firefox: return new Firefox(url);
       case TestPlatform.safari: return new Safari(url);
@@ -178,15 +181,8 @@ class BrowserManager {
 
   /// Loads [_BrowserEnvironment].
   Future<_BrowserEnvironment> _loadBrowserEnvironment() async {
-    var observatoryUrl;
-    if (_platform.isDartVM) observatoryUrl = await _browser.observatoryUrl;
-
-    var remoteDebuggerUrl;
-    if (_platform.isHeadless) {
-      remoteDebuggerUrl = await _browser.remoteDebuggerUrl;
-    }
-
-    return new _BrowserEnvironment(this, observatoryUrl, remoteDebuggerUrl);
+    return new _BrowserEnvironment(this, await _browser.observatoryUrl,
+        await _browser.remoteDebuggerUrl, _onRestartController.stream);
   }
 
   /// Tells the browser the load a test suite from the URL [url].
@@ -266,11 +262,22 @@ class BrowserManager {
 
   /// The callback for handling messages received from the host page.
   void _onMessage(Map message) {
-    if (message["command"] == "ping") return;
+    switch (message["command"]) {
+      case "ping": break;
+
+      case "restart":
+        _onRestartController.add(null);
+        break;
 
-    assert(message["command"] == "resume");
-    if (_pauseCompleter == null) return;
-    _pauseCompleter.complete();
+      case "resume":
+        if (_pauseCompleter != null) _pauseCompleter.complete();
+        break;
+
+      default:
+        // Unreachable.
+        assert(false);
+        break;
+    }
   }
 
   /// Closes the manager and releases any resources it owns, including closing
@@ -298,8 +305,10 @@ class _BrowserEnvironment implements Environment {
 
   final Uri remoteDebuggerUrl;
 
+  final Stream onRestart;
+
   _BrowserEnvironment(this._manager, this.observatoryUrl,
-      this.remoteDebuggerUrl);
+      this.remoteDebuggerUrl, this.onRestart);
 
   CancelableOperation displayPause() => _manager._displayPause();
 }

lib/src/runner/browser/chrome.dart

@@ -8,6 +8,7 @@ import 'dart:io';
 import 'package:path/path.dart' as p;
 
 import '../../util/io.dart';
+import '../../utils.dart';
 import 'browser.dart';
 
 // TODO(nweiz): move this into its own package?
@@ -21,36 +22,77 @@ import 'browser.dart';
 class Chrome extends Browser {
   final name = "Chrome";
 
+  final Future<Uri> remoteDebuggerUrl;
+
   /// Starts a new instance of Chrome open to the given [url], which may be a
   /// [Uri] or a [String].
   ///
   /// If [executable] is passed, it's used as the Chrome executable. Otherwise
   /// the default executable name for the current OS will be used.
-  Chrome(url, {String executable})
-      : super(() => _startBrowser(url, executable));
-
-  static Future<Process> _startBrowser(url, [String executable]) async {
-    if (executable == null) executable = _defaultExecutable();
-
-    var dir = createTempDir();
-    var process = await Process.start(executable, [
-      "--user-data-dir=$dir",
-      url.toString(),
-      "--disable-extensions",
-      "--disable-popup-blocking",
-      "--bwsi",
-      "--no-first-run",
-      "--no-default-browser-check",
-      "--disable-default-apps",
-      "--disable-translate"
-    ]);
-
-    process.exitCode
-        .then((_) => new Directory(dir).deleteSync(recursive: true));
-
-    return process;
+  factory Chrome(url, {String executable, bool debug: false}) {
+    var remoteDebuggerCompleter = new Completer<Uri>.sync();
+    return new Chrome._(() async {
+      if (executable == null) executable = _defaultExecutable();
+
+      var tryPort = ([int port]) async {
+        var dir = createTempDir();
+        var args = [
+          "--user-data-dir=$dir", url.toString(), "--disable-extensions",
+          "--disable-popup-blocking", "--bwsi", "--no-first-run",
+          "--no-default-browser-check", "--disable-default-apps",
+          "--disable-translate",
+        ];
+
+        if (port != null) {
+          args.add("--remote-debugging-port=$port");
+          // These flags cause Chrome to print a consistent line of output after
+          // its internal call to `bind()` has succeeded or failed. We wait for
+          // that output to determine whether the port we chose worked.
+          args.add("--enable-logging=stderr");
+          args.add("--vmodule=startup_browser_creator_impl=1");
+        }
+
+        var process = await Process.start(executable, args);
+
+        if (port != null) {
+          var stderr = new StreamIterator(lineSplitter.bind(process.stderr));
+
+          // Before we can consider Chrome to have started successfully, we have
+          // to make sure the remote debugging port worked. Any errors from this
+          // will always come before the "startup_browser_creater_impl" message.
+          while (await stderr.moveNext() &&
+              !stderr.current.contains("startup_browser_creator_impl")) {
+            if (stderr.current.contains("bind() returned an error")) {
+              // If we failed to bind to the port, return null to tell
+              // getUnusedPort to try another one.
+              stderr.cancel();
+              process.kill();
+              return null;
+            }
+          }
+        }
+
+        if (port != null) {
+          remoteDebuggerCompleter.complete(
+              getRemoteDebuggerUrl(Uri.parse("http://localhost:$port")));
+        } else {
+          remoteDebuggerCompleter.complete(null);
+        }
+
+        process.exitCode
+            .then((_) => new Directory(dir).deleteSync(recursive: true));
+
+        return process;
+      };
+
+      if (!debug) return tryPort();
+      return getUnusedPort/*<Future<Process>>*/(tryPort);
+    }, remoteDebuggerCompleter.future);
   }
 
+  Chrome._(Future<Process> startBrowser(), this.remoteDebuggerUrl)
+      : super(startBrowser);
+
   /// Return the default executable for the current operating system.
   static String _defaultExecutable() {
     if (Platform.isMacOS) {

lib/src/runner/browser/content_shell.dart

@@ -3,7 +3,6 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'dart:async';
-import 'dart:convert';
 import 'dart:io';
 
 import '../../util/io.dart';
@@ -56,7 +55,7 @@ class ContentShell extends Browser {
         // Before we can consider content_shell started successfully, we have to
         // make sure it's not expired and that the remote debugging port worked.
         // Any errors from this will always come before the "Running without
-        // renderer sanxbox" message.
+        // renderer sandbox" message.
         while (await stderr.moveNext() &&
             !stderr.current.endsWith("Running without renderer sandbox")) {
           if (stderr.current == "[dartToStderr]: Dartium build has expired") {
@@ -81,7 +80,7 @@ class ContentShell extends Browser {
 
         if (port != null) {
           remoteDebuggerCompleter.complete(
-              _getRemoteDebuggerUrl(Uri.parse("http://localhost:$port")));
+              getRemoteDebuggerUrl(Uri.parse("http://localhost:$port")));
         } else {
           remoteDebuggerCompleter.complete(null);
         }
@@ -95,25 +94,6 @@ class ContentShell extends Browser {
     }, observatoryCompleter.future, remoteDebuggerCompleter.future);
   }
 
-  /// Returns the full URL of the remote debugger for the host page.
-  ///
-  /// This takes the base remote debugger URL (which points to a browser-wide
-  /// page) and uses its JSON API to find the resolved URL for debugging the
-  /// host page.
-  static Future<Uri> _getRemoteDebuggerUrl(Uri base) async {
-    try {
-      var client = new HttpClient();
-      var request = await client.getUrl(base.resolve("/json/list"));
-      var response = await request.close();
-      var json = await JSON.fuse(UTF8).decoder.bind(response).single as List;
-      return base.resolve(json.first["devtoolsFrontendUrl"]);
-    } catch (_) {
-      // If we fail to talk to the remote debugger protocol, give up and return
-      // the raw URL rather than crashing.
-      return base;
-    }
-  }
-
   ContentShell._(Future<Process> startBrowser(), this.observatoryUrl,
           this.remoteDebuggerUrl)
       : super(startBrowser);