From 0d0aa65421d92e1547f971b18b91c673b9e1600b Mon Sep 17 00:00:00 2001 From: Will Drach Date: Mon, 23 Sep 2019 11:33:21 -0600 Subject: [PATCH 1/5] Move debug functionality to a --debug flag, disentangle --pause-after-load --- pkgs/test/doc/json_reporter.md | 2 +- pkgs/test/lib/src/runner/browser/platform.dart | 4 ++-- .../test/runner/configuration/configuration_test.dart | 8 ++++++++ pkgs/test/test/runner/runner_test.dart | 3 ++- pkgs/test_core/lib/src/runner.dart | 2 +- pkgs/test_core/lib/src/runner/configuration.dart | 8 ++++++++ pkgs/test_core/lib/src/runner/configuration/args.dart | 6 +++++- pkgs/test_core/lib/src/runner/reporter/json.dart | 2 +- pkgs/test_core/lib/src/runner/vm/platform.dart | 2 +- 9 files changed, 29 insertions(+), 8 deletions(-) diff --git a/pkgs/test/doc/json_reporter.md b/pkgs/test/doc/json_reporter.md index 0ce1713bc..669eebe99 100644 --- a/pkgs/test/doc/json_reporter.md +++ b/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 diff --git a/pkgs/test/lib/src/runner/browser/platform.dart b/pkgs/test/lib/src/runner/browser/platform.dart index b6021b28d..1b67088d9 100644 --- a/pkgs/test/lib/src/runner/browser/platform.dart +++ b/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. diff --git a/pkgs/test/test/runner/configuration/configuration_test.dart b/pkgs/test/test/runner/configuration/configuration_test.dart index 336e69f3f..5ccb82a46 100644 --- a/pkgs/test/test/runner/configuration/configuration_test.dart +++ b/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")); diff --git a/pkgs/test/test/runner/runner_test.dart b/pkgs/test/test/runner/runner_test.dart index 064b825a2..8ce54bd0f 100644 --- a/pkgs/test/test/runner/runner_test.dart +++ b/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 diff --git a/pkgs/test_core/lib/src/runner.dart b/pkgs/test_core/lib/src/runner.dart index 580649ffa..9ea4952a1 100644 --- a/pkgs/test_core/lib/src/runner.dart +++ b/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.' diff --git a/pkgs/test_core/lib/src/runner/configuration.dart b/pkgs/test_core/lib/src/runner/configuration.dart index 7c0652251..cf18a5767 100644 --- a/pkgs/test_core/lib/src/runner/configuration.dart +++ b/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, diff --git a/pkgs/test_core/lib/src/runner/configuration/args.dart b/pkgs/test_core/lib/src/runner/configuration/args.dart index a91907169..61393979c 100644 --- a/pkgs/test_core/lib/src/runner/configuration/args.dart +++ b/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'), diff --git a/pkgs/test_core/lib/src/runner/reporter/json.dart b/pkgs/test_core/lib/src/runner/reporter/json.dart index 1312dd7b4..1509099b0 100644 --- a/pkgs/test_core/lib/src/runner/reporter/json.dart +++ b/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. diff --git a/pkgs/test_core/lib/src/runner/vm/platform.dart b/pkgs/test_core/lib/src/runner/vm/platform.dart index 166bcf572..4f521be63 100644 --- a/pkgs/test_core/lib/src/runner/vm/platform.dart +++ b/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. From 4bfa80f31c4f80ab8beb33625ccaa9afa14161dc Mon Sep 17 00:00:00 2001 From: Will Drach Date: Tue, 24 Sep 2019 13:04:56 -0600 Subject: [PATCH 2/5] Update CHANGELOG and pubspec for --debug changes --- pkgs/test/CHANGELOG.md | 3 +++ pkgs/test/pubspec.yaml | 4 ++-- pkgs/test_core/CHANGELOG.md | 3 +++ pkgs/test_core/pubspec.yaml | 2 +- 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/pkgs/test/CHANGELOG.md b/pkgs/test/CHANGELOG.md index 1482a99f0..d19c2afa4 100644 --- a/pkgs/test/CHANGELOG.md +++ b/pkgs/test/CHANGELOG.md @@ -1,3 +1,6 @@ +## 1.6.11 +* Add a `--debug` flag for running the VM/Chrome in debug mode. + ## 1.6.10 * Depend on the latest `test_core`. diff --git a/pkgs/test/pubspec.yaml b/pkgs/test/pubspec.yaml index 6376b2932..068ab20aa 100644 --- a/pkgs/test/pubspec.yaml +++ b/pkgs/test/pubspec.yaml @@ -1,5 +1,5 @@ name: test -version: 1.6.10 +version: 1.6.11 author: Dart Team description: A full featured library for writing and running Dart tests. homepage: https://github.com/dart-lang/test/blob/master/pkgs/test @@ -32,7 +32,7 @@ dependencies: yaml: ^2.0.0 # Use an exact version until the test_api and test_core package are stable. test_api: 0.2.7 - test_core: 0.2.9+1 + test_core: 0.2.10 dev_dependencies: fake_async: ^1.0.0 diff --git a/pkgs/test_core/CHANGELOG.md b/pkgs/test_core/CHANGELOG.md index c0169a1c0..42e10c2c8 100644 --- a/pkgs/test_core/CHANGELOG.md +++ b/pkgs/test_core/CHANGELOG.md @@ -1,3 +1,6 @@ +## 0.2.10 +Add a `--debug` argument for running the VM/Chrome in debug mode. + ## 0.2.9+1 * Allow the latest `package:vm_service`. diff --git a/pkgs/test_core/pubspec.yaml b/pkgs/test_core/pubspec.yaml index 68e1a5604..9e7a89c30 100644 --- a/pkgs/test_core/pubspec.yaml +++ b/pkgs/test_core/pubspec.yaml @@ -1,5 +1,5 @@ name: test_core -version: 0.2.9+1 +version: 0.2.10 author: Dart Team description: A basic library for writing tests and running them on the VM. homepage: https://github.com/dart-lang/test/blob/master/pkgs/test_core From b920c276a3451523c3922214aaba4cb39ec6cade Mon Sep 17 00:00:00 2001 From: Will Drach Date: Tue, 24 Sep 2019 13:11:10 -0600 Subject: [PATCH 3/5] Update bad merged pubspecs --- pkgs/test/pubspec.yaml | 4 ++-- pkgs/test_core/CHANGELOG.md | 1 - pkgs/test_core/pubspec.yaml | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/pkgs/test/pubspec.yaml b/pkgs/test/pubspec.yaml index d0eb27f3a..2013411e4 100644 --- a/pkgs/test/pubspec.yaml +++ b/pkgs/test/pubspec.yaml @@ -31,8 +31,8 @@ dependencies: web_socket_channel: ^1.0.0 yaml: ^2.0.0 # Use an exact version until the test_api and test_core package are stable. - test_api: 0.2.7 - test_core: 0.2.9+1 + test_api: 0.2.8 + test_core: 0.2.10 dev_dependencies: fake_async: ^1.0.0 diff --git a/pkgs/test_core/CHANGELOG.md b/pkgs/test_core/CHANGELOG.md index 8b778313f..4c81fc0d5 100644 --- a/pkgs/test_core/CHANGELOG.md +++ b/pkgs/test_core/CHANGELOG.md @@ -6,7 +6,6 @@ Add a `--debug` argument for running the VM/Chrome in debug mode. * Depend on the latest `test_api`. - ## 0.2.9+1 * Allow the latest `package:vm_service`. diff --git a/pkgs/test_core/pubspec.yaml b/pkgs/test_core/pubspec.yaml index 9e7a89c30..cf2e86b29 100644 --- a/pkgs/test_core/pubspec.yaml +++ b/pkgs/test_core/pubspec.yaml @@ -31,7 +31,7 @@ dependencies: # properly constrains all features it provides. matcher: ">=0.12.5 <0.12.6" # Use an exact version until the test_api package is stable. - test_api: 0.2.7 + test_api: 0.2.8 dependency_overrides: test_api: From cea358f31ce6df90501fb848e9a9c1eea8679683 Mon Sep 17 00:00:00 2001 From: Will Drach Date: Tue, 24 Sep 2019 13:38:35 -0600 Subject: [PATCH 4/5] Bump to 1.7.0 --- pkgs/test/CHANGELOG.md | 2 +- pkgs/test/pubspec.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/test/CHANGELOG.md b/pkgs/test/CHANGELOG.md index 42b33b1f9..765a1d3ec 100644 --- a/pkgs/test/CHANGELOG.md +++ b/pkgs/test/CHANGELOG.md @@ -1,4 +1,4 @@ -## 1.6.12 +## 1.7.0 * Add a `--debug` flag for running the VM/Chrome in debug mode. diff --git a/pkgs/test/pubspec.yaml b/pkgs/test/pubspec.yaml index 2013411e4..1f5391ef5 100644 --- a/pkgs/test/pubspec.yaml +++ b/pkgs/test/pubspec.yaml @@ -1,5 +1,5 @@ name: test -version: 1.6.12 +version: 1.7.0 author: Dart Team description: A full featured library for writing and running Dart tests. homepage: https://github.com/dart-lang/test/blob/master/pkgs/test From 43f27620301d34136ea3928ba6fb9656ec4221b6 Mon Sep 17 00:00:00 2001 From: Will Drach Date: Tue, 24 Sep 2019 13:46:13 -0600 Subject: [PATCH 5/5] Formatting --- pkgs/test_core/lib/src/runner/configuration/args.dart | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkgs/test_core/lib/src/runner/configuration/args.dart b/pkgs/test_core/lib/src/runner/configuration/args.dart index 61393979c..fe0eb9159 100644 --- a/pkgs/test_core/lib/src/runner/configuration/args.dart +++ b/pkgs/test_core/lib/src/runner/configuration/args.dart @@ -86,8 +86,7 @@ final ArgParser _parser = (() { 'Currently only supported for browser tests.', negatable: false); parser.addFlag("debug", - help: 'Runs the VM and Chrome tests in debug mode.', - negatable: false); + 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'