Move debug functionality to a --debug flag, disentangle --pause-after-load

Author: willdrach-wk

pkgs/test/doc/json_reporter.md

@@ -156,7 +156,7 @@ class DebugEvent extends Event {
 
 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.
+emitted if the `--debug` 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

pkgs/test/lib/src/runner/browser/platform.dart

@@ -397,12 +397,12 @@ class BrowserPlatform extends PlatformPlugin
         .resolve('packages/test/src/runner/browser/static/index.html')
         .replace(queryParameters: {
       'managerUrl': webSocketUrl.toString(),
-      'debug': _config.pauseAfterLoad.toString()
+      'debug': _config.debug.toString()
     });
 
     var future = BrowserManager.start(
         browser, hostUrl, completer.future, _browserSettings[browser],
-        debug: _config.pauseAfterLoad);
+        debug: _config.debug);
 
     // Store null values for browsers that error out so we know not to load them
     // again.

pkgs/test/test/runner/configuration/configuration_test.dart

@@ -18,6 +18,7 @@ void main() {
         expect(merged.help, isFalse);
         expect(merged.version, isFalse);
         expect(merged.pauseAfterLoad, isFalse);
+        expect(merged.debug, isFalse);
         expect(merged.color, equals(canUseSpecialChars));
         expect(merged.configurationPath, equals('dart_test.yaml'));
         expect(merged.dart2jsPath, equals(p.join(sdkDir, 'bin', 'dart2js')));
@@ -33,6 +34,7 @@ void main() {
             help: true,
             version: true,
             pauseAfterLoad: true,
+            debug: true,
             color: true,
             configurationPath: "special_test.yaml",
             dart2jsPath: "/tmp/dart2js",
@@ -45,6 +47,7 @@ void main() {
         expect(merged.help, isTrue);
         expect(merged.version, isTrue);
         expect(merged.pauseAfterLoad, isTrue);
+        expect(merged.debug, isTrue);
         expect(merged.color, isTrue);
         expect(merged.configurationPath, equals("special_test.yaml"));
         expect(merged.dart2jsPath, equals("/tmp/dart2js"));
@@ -60,6 +63,7 @@ void main() {
             help: true,
             version: true,
             pauseAfterLoad: true,
+            debug: true,
             color: true,
             configurationPath: "special_test.yaml",
             dart2jsPath: "/tmp/dart2js",
@@ -72,6 +76,7 @@ void main() {
         expect(merged.help, isTrue);
         expect(merged.version, isTrue);
         expect(merged.pauseAfterLoad, isTrue);
+        expect(merged.debug, isTrue);
         expect(merged.color, isTrue);
         expect(merged.configurationPath, equals("special_test.yaml"));
         expect(merged.dart2jsPath, equals("/tmp/dart2js"));
@@ -89,6 +94,7 @@ void main() {
             help: true,
             version: false,
             pauseAfterLoad: true,
+            debug: true,
             color: false,
             configurationPath: "special_test.yaml",
             dart2jsPath: "/tmp/dart2js",
@@ -101,6 +107,7 @@ void main() {
             help: false,
             version: true,
             pauseAfterLoad: false,
+            debug: false,
             color: true,
             configurationPath: "test_special.yaml",
             dart2jsPath: "../dart2js",
@@ -114,6 +121,7 @@ void main() {
         expect(merged.help, isFalse);
         expect(merged.version, isTrue);
         expect(merged.pauseAfterLoad, isFalse);
+        expect(merged.debug, isFalse);
         expect(merged.color, isTrue);
         expect(merged.configurationPath, equals("test_special.yaml"));
         expect(merged.dart2jsPath, equals("../dart2js"));

pkgs/test/test/runner/runner_test.dart

@@ -89,9 +89,10 @@ Usage: pub run test [files or directories...]
                                   (defaults to "30s")
 
     --pause-after-load            Pauses for debugging before any tests execute.
-                                  Implies --concurrency=1 and --timeout=none.
+                                  Implies --concurrency=1, --debug, and --timeout=none.
                                   Currently only supported for browser tests.
 
+    --debug                       Runs the VM and Chrome tests in debug mode.
     --[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

pkgs/test_core/lib/src/runner.dart

@@ -98,7 +98,7 @@ class Runner {
         // https://github.com/dart-lang/sdk/issues/31308 is resolved.
         if (!_silentObservatory &&
             runTimes.contains(Runtime.vm) &&
-            _config.pauseAfterLoad) {
+            _config.debug) {
           warn('You should set `SILENT_OBSERVATORY` to true when debugging the '
               'VM as it will output the observatory URL by '
               'default.\nThis breaks the various reporter contracts.'

pkgs/test_core/lib/src/runner/configuration.dart

@@ -51,6 +51,10 @@ class Configuration {
   bool get pauseAfterLoad => _pauseAfterLoad ?? false;
   final bool _pauseAfterLoad;
 
+  /// Whether to run browsers in their respective debug modes
+  bool get debug => pauseAfterLoad || (_debug ?? false);
+  final bool _debug;
+
   /// The path to the file from which to load more configuration information.
   ///
   /// This is *not* resolved automatically.
@@ -210,6 +214,7 @@ class Configuration {
       {bool help,
       bool version,
       bool pauseAfterLoad,
+      bool debug,
       bool color,
       String configurationPath,
       String dart2jsPath,
@@ -254,6 +259,7 @@ class Configuration {
         help: help,
         version: version,
         pauseAfterLoad: pauseAfterLoad,
+        debug: debug,
         color: color,
         configurationPath: configurationPath,
         dart2jsPath: dart2jsPath,
@@ -311,6 +317,7 @@ class Configuration {
       {bool help,
       bool version,
       bool pauseAfterLoad,
+      bool debug,
       bool color,
       String configurationPath,
       String dart2jsPath,
@@ -332,6 +339,7 @@ class Configuration {
       : _help = help,
         _version = version,
         _pauseAfterLoad = pauseAfterLoad,
+        _debug = debug,
         _color = color,
         _configurationPath = configurationPath,
         _dart2jsPath = dart2jsPath,

pkgs/test_core/lib/src/runner/configuration/args.dart

@@ -82,9 +82,12 @@ final ArgParser _parser = (() {
       defaultsTo: '30s');
   parser.addFlag("pause-after-load",
       help: 'Pauses for debugging before any tests execute.\n'
-          'Implies --concurrency=1 and --timeout=none.\n'
+          'Implies --concurrency=1, --debug, and --timeout=none.\n'
           'Currently only supported for browser tests.',
       negatable: false);
+  parser.addFlag("debug",
+      help: 'Runs the VM and Chrome tests in debug mode.',
+      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'
@@ -207,6 +210,7 @@ class _Parser {
         chainStackTraces: _ifParsed('chain-stack-traces'),
         jsTrace: _ifParsed('js-trace'),
         pauseAfterLoad: _ifParsed('pause-after-load'),
+        debug: _ifParsed('debug'),
         color: _ifParsed('color'),
         configurationPath: _ifParsed('configuration'),
         dart2jsPath: _ifParsed('dart2js-path'),

pkgs/test_core/lib/src/runner/reporter/json.dart

@@ -172,7 +172,7 @@ class JsonReporter implements Reporter {
     if (suite is LoadSuite) {
       suite.suite.then((runnerSuite) {
         _suiteIDs[runnerSuite] = id;
-        if (!_config.pauseAfterLoad) return;
+        if (!_config.debug) return;
 
         // TODO(nweiz): test this when we have a library for communicating with
         // the Chrome remote debugger, or when we have VM debug support.

pkgs/test_core/lib/src/runner/vm/platform.dart

@@ -61,7 +61,7 @@ class VMPlatform extends PlatformPlugin {
 
     VMEnvironment environment;
     IsolateRef isolateRef;
-    if (_config.pauseAfterLoad) {
+    if (_config.debug) {
       // Print an empty line because the VM prints an "Observatory listening on"
       // line and we don't want that to end up on the same line as the reporter
       // info.