From 8acac060bf145f95b678d70f3b19445d80c91a41 Mon Sep 17 00:00:00 2001 From: Ian Hickson Date: Mon, 1 Feb 2021 16:06:02 -0800 Subject: [PATCH] Remove the timeout when launching DevTools (#74859) --- .../lib/src/commands/attach.dart | 7 +- .../lib/src/commands/daemon.dart | 1 + .../flutter_tools/lib/src/commands/run.dart | 3 +- .../lib/src/devtools_launcher.dart | 10 +- .../lib/src/drive/web_driver_service.dart | 1 + .../lib/src/isolated/resident_web_runner.dart | 3 + .../lib/src/resident_runner.dart | 95 ++++++- packages/flutter_tools/lib/src/run_cold.dart | 69 ++--- packages/flutter_tools/lib/src/run_hot.dart | 54 ++-- .../lib/src/runner/flutter_command.dart | 41 ++- .../commands.shard/hermetic/attach_test.dart | 21 +- .../test/general.shard/cold_test.dart | 8 +- .../test/general.shard/hot_test.dart | 4 +- .../general.shard/resident_runner_test.dart | 246 +++++++++++------- .../runner/flutter_command_test.dart | 4 +- .../general.shard/terminal_handler_test.dart | 2 + 16 files changed, 353 insertions(+), 216 deletions(-) diff --git a/packages/flutter_tools/lib/src/commands/attach.dart b/packages/flutter_tools/lib/src/commands/attach.dart index 0a80363a444..3f7c69da5c9 100644 --- a/packages/flutter_tools/lib/src/commands/attach.dart +++ b/packages/flutter_tools/lib/src/commands/attach.dart @@ -103,7 +103,7 @@ class AttachCommand extends FlutterCommand { ); usesTrackWidgetCreation(verboseHelp: verboseHelp); addDdsOptions(verboseHelp: verboseHelp); - addDevToolsOptions(); + addDevToolsOptions(verboseHelp: verboseHelp); usesDeviceTimeoutOption(); hotRunnerFactory ??= HotRunnerFactory(); } @@ -207,7 +207,8 @@ known, it can be explicitly provided to attach via the command-line, e.g. body: () => _attachToDevice(device), overrides: { Artifacts: () => overrideArtifacts, - }); + }, + ); return FlutterCommandResult.success(); } @@ -327,6 +328,7 @@ known, it can be explicitly provided to attach via the command-line, e.g. connectionInfoCompleter: connectionInfoCompleter, appStartedCompleter: appStartedCompleter, allowExistingDdsInstance: true, + enableDevTools: boolArg(FlutterCommand.kEnableDevTools), ); }, device, @@ -365,6 +367,7 @@ known, it can be explicitly provided to attach via the command-line, e.g. result = await runner.attach( appStartedCompleter: onAppStart, allowExistingDdsInstance: true, + enableDevTools: boolArg(FlutterCommand.kEnableDevTools), ); if (result != 0) { throwToolExit(null, exitCode: result); diff --git a/packages/flutter_tools/lib/src/commands/daemon.dart b/packages/flutter_tools/lib/src/commands/daemon.dart index 640098662d3..1b3e721bd07 100644 --- a/packages/flutter_tools/lib/src/commands/daemon.dart +++ b/packages/flutter_tools/lib/src/commands/daemon.dart @@ -515,6 +515,7 @@ class AppDomain extends Domain { return runner.run( connectionInfoCompleter: connectionInfoCompleter, appStartedCompleter: appStartedCompleter, + enableDevTools: true, route: route, ); }, diff --git a/packages/flutter_tools/lib/src/commands/run.dart b/packages/flutter_tools/lib/src/commands/run.dart index 00a6099a227..071e667db48 100644 --- a/packages/flutter_tools/lib/src/commands/run.dart +++ b/packages/flutter_tools/lib/src/commands/run.dart @@ -136,7 +136,7 @@ abstract class RunCommandBase extends FlutterCommand with DeviceBasedDevelopment usesDeviceUserOption(); usesDeviceTimeoutOption(); addDdsOptions(verboseHelp: verboseHelp); - addDevToolsOptions(); + addDevToolsOptions(verboseHelp: verboseHelp); addAndroidSpecificBuildOptions(hide: !verboseHelp); } @@ -621,6 +621,7 @@ class RunCommand extends RunCommandBase { final int result = await runner.run( appStartedCompleter: appStartedTimeRecorder, + enableDevTools: stayResident && boolArg(FlutterCommand.kEnableDevTools), route: route, ); if (result != 0) { diff --git a/packages/flutter_tools/lib/src/devtools_launcher.dart b/packages/flutter_tools/lib/src/devtools_launcher.dart index 8cc1e53f855..791f57726d2 100644 --- a/packages/flutter_tools/lib/src/devtools_launcher.dart +++ b/packages/flutter_tools/lib/src/devtools_launcher.dart @@ -48,8 +48,7 @@ class DevtoolsServerLauncher extends DevtoolsLauncher { @override Future launch(Uri vmServiceUri) async { // Place this entire method in a try/catch that swallows exceptions because - // we do not want to block Flutter run/attach operations on a DevTools - // failure. + // this method is guaranteed not to return a Future that throws. try { bool offline = false; try { @@ -109,8 +108,7 @@ class DevtoolsServerLauncher extends DevtoolsLauncher { .transform(utf8.decoder) .transform(const LineSplitter()) .listen(_logger.printError); - devToolsUri = await completer.future - .timeout(const Duration(seconds: 10)); + devToolsUrl = await completer.future; } on Exception catch (e, st) { _logger.printError('Failed to launch DevTools: $e', stackTrace: st); } @@ -124,7 +122,6 @@ class DevtoolsServerLauncher extends DevtoolsLauncher { 'global', 'list', ]); - if (_pubGlobalListProcess.stdout.toString().contains('devtools ')) { return true; } @@ -144,7 +141,6 @@ class DevtoolsServerLauncher extends DevtoolsLauncher { if (!shouldActivate) { return false; } - final Status status = _logger.startProgress( 'Activating Dart DevTools...', ); @@ -182,7 +178,7 @@ class DevtoolsServerLauncher extends DevtoolsLauncher { @override Future close() async { - devToolsUri = null; + devToolsUrl = null; if (_devToolsProcess != null) { _devToolsProcess.kill(); await _devToolsProcess.exitCode; diff --git a/packages/flutter_tools/lib/src/drive/web_driver_service.dart b/packages/flutter_tools/lib/src/drive/web_driver_service.dart index c571ae0d73b..f9bf635141e 100644 --- a/packages/flutter_tools/lib/src/drive/web_driver_service.dart +++ b/packages/flutter_tools/lib/src/drive/web_driver_service.dart @@ -76,6 +76,7 @@ class WebDriverService extends DriverService { final Completer appStartedCompleter = Completer.sync(); final int result = await _residentRunner.run( appStartedCompleter: appStartedCompleter, + enableDevTools: false, route: route, ); _webUri = _residentRunner.uri; diff --git a/packages/flutter_tools/lib/src/isolated/resident_web_runner.dart b/packages/flutter_tools/lib/src/isolated/resident_web_runner.dart index 0461727d73e..9dcc455c3a8 100644 --- a/packages/flutter_tools/lib/src/isolated/resident_web_runner.dart +++ b/packages/flutter_tools/lib/src/isolated/resident_web_runner.dart @@ -454,6 +454,7 @@ class _ResidentWebRunner extends ResidentWebRunner { Future run({ Completer connectionInfoCompleter, Completer appStartedCompleter, + bool enableDevTools = false, // ignored, we don't yet support devtools for web String route, }) async { firstBuildTime = DateTime.now(); @@ -531,6 +532,7 @@ class _ResidentWebRunner extends ResidentWebRunner { return attach( connectionInfoCompleter: connectionInfoCompleter, appStartedCompleter: appStartedCompleter, + enableDevTools: enableDevTools, ); }); } on WebSocketException { @@ -745,6 +747,7 @@ class _ResidentWebRunner extends ResidentWebRunner { Completer connectionInfoCompleter, Completer appStartedCompleter, bool allowExistingDdsInstance = false, + bool enableDevTools = false, // ignored, we don't yet support devtools for web }) async { if (_chromiumLauncher != null) { final Chromium chrome = await _chromiumLauncher.connectedInstance; diff --git a/packages/flutter_tools/lib/src/resident_runner.dart b/packages/flutter_tools/lib/src/resident_runner.dart index 14b94402aa7..7b9bdb34e13 100644 --- a/packages/flutter_tools/lib/src/resident_runner.dart +++ b/packages/flutter_tools/lib/src/resident_runner.dart @@ -795,6 +795,8 @@ abstract class ResidentRunner { final CommandHelp commandHelp; final bool machine; + @visibleForTesting + DevtoolsLauncher get devToolsLauncher => _devToolsLauncher; DevtoolsLauncher _devToolsLauncher; bool _exited = false; @@ -878,6 +880,7 @@ abstract class ResidentRunner { Future run({ Completer connectionInfoCompleter, Completer appStartedCompleter, + bool enableDevTools = false, String route, }); @@ -885,6 +888,7 @@ abstract class ResidentRunner { Completer connectionInfoCompleter, Completer appStartedCompleter, bool allowExistingDdsInstance = false, + bool enableDevTools = false, }); bool get supportsRestart => false; @@ -1275,19 +1279,26 @@ abstract class ResidentRunner { return _devToolsLauncher.activeDevToolsServer; } - Future serveDevToolsGracefully({ - Uri devToolsServerAddress + // This must be guaranteed not to return a Future that fails. + Future serveAndAnnounceDevTools({ + Uri devToolsServerAddress, }) async { if (!supportsServiceProtocol) { return; } - _devToolsLauncher ??= DevtoolsLauncher.instance; if (devToolsServerAddress != null) { - _devToolsLauncher.devToolsUri = devToolsServerAddress; + _devToolsLauncher.devToolsUrl = devToolsServerAddress; } else { - await _devToolsLauncher.serve(); + unawaited(_devToolsLauncher.serve()); } + await _devToolsLauncher.ready; + if (_reportedDebuggers) { + // Since the DevTools only just became available, we haven't had a chance to + // report their URLs yet. Do so now. + printDebuggerList(includeObservatory: false); + } + await maybeCallDevToolsUriServiceExtension(); } Future maybeCallDevToolsUriServiceExtension() async { @@ -1417,6 +1428,39 @@ abstract class ResidentRunner { appFinished(); } + bool _reportedDebuggers = false; + + void printDebuggerList({ bool includeObservatory = true, bool includeDevtools = true }) { + final DevToolsServerAddress devToolsServerAddress = activeDevToolsServer(); + if (devToolsServerAddress == null) { + includeDevtools = false; + } + for (final FlutterDevice device in flutterDevices) { + if (device.vmService == null) { + continue; + } + if (includeObservatory) { + // Caution: This log line is parsed by device lab tests. + globals.printStatus( + 'An Observatory debugger and profiler on ${device.device.name} is available at: ' + '${device.vmService.httpAddress}', + ); + } + if (includeDevtools) { + final Uri uri = devToolsServerAddress.uri?.replace( + queryParameters: {'uri': '${device.vmService.httpAddress}'}, + ); + if (uri != null) { + globals.printStatus( + 'The Flutter DevTools debugger and profiler ' + 'on ${device.device.name} is available at: $uri', + ); + } + } + } + _reportedDebuggers = true; + } + /// Called to print help to the terminal. void printHelp({ @required bool details }); @@ -1763,24 +1807,51 @@ String nextPlatform(String currentPlatform, FeatureFlags featureFlags) { /// A launcher for the devtools debugger and analysis tool. abstract class DevtoolsLauncher { - Uri devToolsUri; + static DevtoolsLauncher get instance => context.get(); + + /// Serve Dart DevTools and return the host and port they are available on. + /// + /// This method must return a future that is guaranteed not to fail, because it + /// will be used in unawaited contexts. It may, however, return null. + Future serve(); /// Launch a Dart DevTools process, optionally targeting a specific VM Service /// URI if [vmServiceUri] is non-null. + /// + /// This method must return a future that is guaranteed not to fail, because it + /// will be used in unawaited contexts. + @visibleForTesting Future launch(Uri vmServiceUri); - /// Serve Dart DevTools and return the host and port they are available on. - Future serve(); - Future close(); - static DevtoolsLauncher get instance => context.get(); + /// Returns a future that completes when the DevTools server is ready. + /// + /// Completes when [devToolsUrl] is set. That can be set either directly, or + /// by calling [serve]. + Future get ready => _readyCompleter.future; + Completer _readyCompleter = Completer(); + Uri get devToolsUrl => _devToolsUrl; + Uri _devToolsUrl; + set devToolsUrl(Uri value) { + assert((_devToolsUrl == null) != (value == null)); + _devToolsUrl = value; + if (_devToolsUrl != null) { + _readyCompleter.complete(); + } else { + _readyCompleter = Completer(); + } + } + + /// The URL of the current DevTools server. + /// + /// Returns null if [ready] is not complete. DevToolsServerAddress get activeDevToolsServer { - if (devToolsUri == null) { + if (_devToolsUrl == null) { return null; } - return DevToolsServerAddress(devToolsUri.host, devToolsUri.port); + return DevToolsServerAddress(devToolsUrl.host, devToolsUrl.port); } } diff --git a/packages/flutter_tools/lib/src/run_cold.dart b/packages/flutter_tools/lib/src/run_cold.dart index 8064e0b85e0..40e3ac60cdb 100644 --- a/packages/flutter_tools/lib/src/run_cold.dart +++ b/packages/flutter_tools/lib/src/run_cold.dart @@ -53,6 +53,7 @@ class ColdRunner extends ResidentRunner { Future run({ Completer connectionInfoCompleter, Completer appStartedCompleter, + bool enableDevTools = false, String route, }) async { try { @@ -72,17 +73,17 @@ class ColdRunner extends ResidentRunner { return 1; } + if (enableDevTools) { + // The method below is guaranteed never to return a failing future. + unawaited(serveAndAnnounceDevTools( + devToolsServerAddress: debuggingOptions.devToolsServerAddress, + )); + } + // Connect to observatory. - if (debuggingOptions.debuggingEnabled) { + if (debuggingEnabled) { try { - await Future.wait(>[ - connectToServiceProtocol( - allowExistingDdsInstance: false, - ), - serveDevToolsGracefully( - devToolsServerAddress: debuggingOptions.devToolsServerAddress, - ), - ]); + await connectToServiceProtocol(allowExistingDdsInstance: false); } on String catch (message) { globals.printError(message); appFailedToStart(); @@ -124,7 +125,6 @@ class ColdRunner extends ResidentRunner { } if (debuggingEnabled) { - unawaited(maybeCallDevToolsUriServiceExtension()); unawaited(callConnectedVmServiceUriExtension()); } @@ -144,22 +144,26 @@ class ColdRunner extends ResidentRunner { Completer connectionInfoCompleter, Completer appStartedCompleter, bool allowExistingDdsInstance = false, + bool enableDevTools = false, }) async { _didAttach = true; try { - await Future.wait(>[ - connectToServiceProtocol( - getSkSLMethod: writeSkSL, - allowExistingDdsInstance: allowExistingDdsInstance, - ), - serveDevToolsGracefully( - devToolsServerAddress: debuggingOptions.devToolsServerAddress, - ), - ], eagerError: true); + await connectToServiceProtocol( + getSkSLMethod: writeSkSL, + allowExistingDdsInstance: allowExistingDdsInstance, + ); } on Exception catch (error) { globals.printError('Error connecting to the service protocol: $error'); return 2; } + + if (enableDevTools) { + // The method below is guaranteed never to return a failing future. + unawaited(serveAndAnnounceDevTools( + devToolsServerAddress: debuggingOptions.devToolsServerAddress, + )); + } + for (final FlutterDevice device in flutterDevices) { await device.initLogReader(); } @@ -170,7 +174,6 @@ class ColdRunner extends ResidentRunner { } } - unawaited(maybeCallDevToolsUriServiceExtension()); unawaited(callConnectedVmServiceUriExtension()); appStartedCompleter?.complete(); @@ -205,35 +208,13 @@ class ColdRunner extends ResidentRunner { if (details) { printHelpDetails(); } - commandHelp.h.print(); + commandHelp.h.print(); // TODO(ianh): print different message if details is false if (_didAttach) { commandHelp.d.print(); } commandHelp.c.print(); commandHelp.q.print(); - for (final FlutterDevice device in flutterDevices) { - final String dname = device.device.name; - if (device.vmService != null) { - // Caution: This log line is parsed by device lab tests. - globals.printStatus( - 'An Observatory debugger and profiler on $dname is available at: ' - '${device.vmService.httpAddress}', - ); - - final DevToolsServerAddress devToolsServerAddress = activeDevToolsServer(); - if (devToolsServerAddress != null) { - final Uri uri = devToolsServerAddress.uri?.replace( - queryParameters: {'uri': '${device.vmService.httpAddress}'}, - ); - if (uri != null) { - globals.printStatus( - '\nFlutter DevTools, a Flutter debugger and profiler, on ' - '${device.device.name} is available at: $uri', - ); - } - } - } - } + printDebuggerList(); } @override diff --git a/packages/flutter_tools/lib/src/run_hot.dart b/packages/flutter_tools/lib/src/run_hot.dart index 46f29354f0b..ee3c83dfb14 100644 --- a/packages/flutter_tools/lib/src/run_hot.dart +++ b/packages/flutter_tools/lib/src/run_hot.dart @@ -174,21 +174,17 @@ class HotRunner extends ResidentRunner { Completer connectionInfoCompleter, Completer appStartedCompleter, bool allowExistingDdsInstance = false, + bool enableDevTools = false, }) async { _didAttach = true; try { - await Future.wait(>[ - connectToServiceProtocol( - reloadSources: _reloadSourcesService, - restart: _restartService, - compileExpression: _compileExpressionService, - getSkSLMethod: writeSkSL, - allowExistingDdsInstance: allowExistingDdsInstance, - ), - serveDevToolsGracefully( - devToolsServerAddress: debuggingOptions.devToolsServerAddress, - ), - ]); + await connectToServiceProtocol( + reloadSources: _reloadSourcesService, + restart: _restartService, + compileExpression: _compileExpressionService, + getSkSLMethod: writeSkSL, + allowExistingDdsInstance: allowExistingDdsInstance, + ); // Catches all exceptions, non-Exception objects are rethrown. } catch (error) { // ignore: avoid_catches_without_on_clauses if (error is! Exception && error is! String) { @@ -198,6 +194,13 @@ class HotRunner extends ResidentRunner { return 2; } + if (enableDevTools) { + // The method below is guaranteed never to return a failing future. + unawaited(serveAndAnnounceDevTools( + devToolsServerAddress: debuggingOptions.devToolsServerAddress, + )); + } + for (final FlutterDevice device in flutterDevices) { await device.initLogReader(); } @@ -218,7 +221,6 @@ class HotRunner extends ResidentRunner { return 3; } - unawaited(maybeCallDevToolsUriServiceExtension()); unawaited(callConnectedVmServiceUriExtension()); final Stopwatch initialUpdateDevFSsTimer = Stopwatch()..start(); @@ -306,6 +308,7 @@ class HotRunner extends ResidentRunner { Future run({ Completer connectionInfoCompleter, Completer appStartedCompleter, + bool enableDevTools = false, String route, }) async { firstBuildTime = DateTime.now(); @@ -356,6 +359,7 @@ class HotRunner extends ResidentRunner { return attach( connectionInfoCompleter: connectionInfoCompleter, appStartedCompleter: appStartedCompleter, + enableDevTools: enableDevTools, ); } @@ -1086,7 +1090,7 @@ class HotRunner extends ResidentRunner { if (canHotRestart) { commandHelp.R.print(); } - commandHelp.h.print(); + commandHelp.h.print(); // TODO(ianh): print different message if "details" is false if (_didAttach) { commandHelp.d.print(); } @@ -1095,26 +1099,6 @@ class HotRunner extends ResidentRunner { if (details) { printHelpDetails(); } - for (final FlutterDevice device in flutterDevices) { - // Caution: This log line is parsed by device lab tests. - globals.printStatus( - 'An Observatory debugger and profiler on ${device.device.name} is available at: ' - '${device.vmService.httpAddress}', - ); - - final DevToolsServerAddress devToolsServerAddress = activeDevToolsServer(); - if (devToolsServerAddress != null) { - final Uri uri = devToolsServerAddress.uri?.replace( - queryParameters: {'uri': '${device.vmService.httpAddress}'}, - ); - if (uri != null) { - globals.printStatus( - '\nFlutter DevTools, a Flutter debugger and profiler, on ' - '${device.device.name} is available at: $uri', - ); - } - } - } globals.printStatus(''); if (debuggingOptions.buildInfo.nullSafetyMode == NullSafetyMode.sound) { globals.printStatus('💪 Running with sound null safety 💪', emphasis: true); @@ -1127,6 +1111,8 @@ class HotRunner extends ResidentRunner { 'For more information see https://dart.dev/null-safety/unsound-null-safety', ); } + globals.printStatus(''); + printDebuggerList(); } Future _evictDirtyAssets() async { diff --git a/packages/flutter_tools/lib/src/runner/flutter_command.dart b/packages/flutter_tools/lib/src/runner/flutter_command.dart index 54afac9758e..eee8df5a2de 100644 --- a/packages/flutter_tools/lib/src/runner/flutter_command.dart +++ b/packages/flutter_tools/lib/src/runner/flutter_command.dart @@ -133,6 +133,9 @@ abstract class FlutterCommand extends Command { /// The option name for a custom DevTools server address. static const String kDevToolsServerAddress = 'devtools-server-address'; + /// The flag name for whether to launch the DevTools or not. + static const String kEnableDevTools = 'devtools'; + /// The flag name for whether or not to use ipv6. static const String ipv6Flag = 'ipv6'; @@ -327,27 +330,39 @@ abstract class FlutterCommand extends Command { _usesPortOption = true; } - void addDevToolsOptions() { - argParser.addOption(kDevToolsServerAddress, + void addDevToolsOptions({@required bool verboseHelp}) { + argParser.addFlag( + kEnableDevTools, + hide: !verboseHelp, + defaultsTo: true, + help: 'Enable (or disable, with --no-$kEnableDevTools) the launching of the ' + 'Flutter DevTools debugger and profiler. ' + 'If specified, --$kDevToolsServerAddress is ignored.' + ); + argParser.addOption( + kDevToolsServerAddress, + hide: !verboseHelp, help: 'When this value is provided, the Flutter tool will not spin up a ' - 'new DevTools server instance, but instead will use the one provided ' - 'at this address.'); + 'new DevTools server instance, and will instead use the one provided ' + 'at the given address. Ignored if --no-$kEnableDevTools is specified.' + ); } void addDdsOptions({@required bool verboseHelp}) { argParser.addOption('dds-port', help: 'When this value is provided, the Dart Development Service (DDS) will be ' - 'bound to the provided port.\nSpecifying port 0 (the default) will find ' - 'a random free port.'); + 'bound to the provided port.\n' + 'Specifying port 0 (the default) will find a random free port.' + ); argParser.addFlag( - 'disable-dds', + 'disable-dds', // TODO(ianh): this should be called `dds` and default to true (see style guide about double negatives) hide: !verboseHelp, - help: 'Disable the Dart Developer Service (DDS). This flag should only be provided' - ' when attaching to an application with an existing DDS instance (e.g.,' - ' attaching to an application currently connected to by "flutter run") or' - ' when running certain tests.\n' - 'Note: passing this flag may degrade IDE functionality if a DDS instance is not' - ' already connected to the target application.' + help: 'Disable the Dart Developer Service (DDS). This flag should only be provided ' + 'when attaching to an application with an existing DDS instance (e.g., ' + 'attaching to an application currently connected to by "flutter run") or ' + 'when running certain tests.\n' + 'Passing this flag may degrade IDE functionality if a DDS instance is not ' + 'already connected to the target application.' ); } diff --git a/packages/flutter_tools/test/commands.shard/hermetic/attach_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/attach_test.dart index dcb2b1cb27d..7139dd8dc1d 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/attach_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/attach_test.dart @@ -181,8 +181,11 @@ void main() { const String outputDill = '/tmp/output.dill'; final MockHotRunner mockHotRunner = MockHotRunner(); - when(mockHotRunner.attach(appStartedCompleter: anyNamed('appStartedCompleter'), allowExistingDdsInstance: true)) - .thenAnswer((_) async => 0); + when(mockHotRunner.attach( + appStartedCompleter: anyNamed('appStartedCompleter'), + allowExistingDdsInstance: true, + enableDevTools: anyNamed('enableDevTools'), + )).thenAnswer((_) async => 0); when(mockHotRunner.exited).thenReturn(false); when(mockHotRunner.isWaitingForObservatory).thenReturn(false); @@ -313,8 +316,11 @@ void main() { .thenReturn([ForwardedPort(hostPort, devicePort)]); when(portForwarder.unforward(any)) .thenAnswer((_) async {}); - when(mockHotRunner.attach(appStartedCompleter: anyNamed('appStartedCompleter'), allowExistingDdsInstance: true)) - .thenAnswer((_) async => 0); + when(mockHotRunner.attach( + appStartedCompleter: anyNamed('appStartedCompleter'), + allowExistingDdsInstance: true, + enableDevTools: anyNamed('enableDevTools'), + )).thenAnswer((_) async => 0); when(mockHotRunnerFactory.build( any, target: anyNamed('target'), @@ -397,8 +403,11 @@ void main() { .thenReturn([ForwardedPort(hostPort, devicePort)]); when(portForwarder.unforward(any)) .thenAnswer((_) async {}); - when(mockHotRunner.attach(appStartedCompleter: anyNamed('appStartedCompleter'), allowExistingDdsInstance: true)) - .thenAnswer((_) async => 0); + when(mockHotRunner.attach( + appStartedCompleter: anyNamed('appStartedCompleter'), + allowExistingDdsInstance: true, + enableDevTools: anyNamed('enableDevTools'), + )).thenAnswer((_) async => 0); when(mockHotRunnerFactory.build( any, target: anyNamed('target'), diff --git a/packages/flutter_tools/test/general.shard/cold_test.dart b/packages/flutter_tools/test/general.shard/cold_test.dart index c3b209bf129..2335ee5744e 100644 --- a/packages/flutter_tools/test/general.shard/cold_test.dart +++ b/packages/flutter_tools/test/general.shard/cold_test.dart @@ -42,7 +42,9 @@ void main() { final int exitCode = await ColdRunner(devices, debuggingOptions: DebuggingOptions.enabled(BuildInfo.debug), target: 'main.dart', - ).attach(); + ).attach( + enableDevTools: false, + ); expect(exitCode, 2); }); @@ -90,7 +92,9 @@ void main() { applicationBinary: applicationBinary, debuggingOptions: DebuggingOptions.enabled(BuildInfo.debug), target: 'main.dart', - ).run(); + ).run( + enableDevTools: false, + ); expect(result, 1); verify(mockFlutterDevice.runCold( diff --git a/packages/flutter_tools/test/general.shard/hot_test.dart b/packages/flutter_tools/test/general.shard/hot_test.dart index 05592a336f3..b6850611b8f 100644 --- a/packages/flutter_tools/test/general.shard/hot_test.dart +++ b/packages/flutter_tools/test/general.shard/hot_test.dart @@ -524,7 +524,9 @@ void main() { final int exitCode = await HotRunner(devices, debuggingOptions: DebuggingOptions.enabled(BuildInfo.debug), target: 'main.dart', - ).attach(); + ).attach( + enableDevTools: false, + ); expect(exitCode, 2); }, overrides: { HotRunnerConfig: () => TestHotRunnerConfig(successfulSetup: true), diff --git a/packages/flutter_tools/test/general.shard/resident_runner_test.dart b/packages/flutter_tools/test/general.shard/resident_runner_test.dart index f5d01a17408..40db5daba6d 100644 --- a/packages/flutter_tools/test/general.shard/resident_runner_test.dart +++ b/packages/flutter_tools/test/general.shard/resident_runner_test.dart @@ -224,21 +224,22 @@ void main() { listViews, setAssetBundlePath, ]); - final Completer onConnectionInfo = Completer.sync(); - final Completer onAppStart = Completer.sync(); + final Completer futureConnectionInfo = Completer.sync(); + final Completer futureAppStart = Completer.sync(); final Future result = residentRunner.attach( - appStartedCompleter: onAppStart, - connectionInfoCompleter: onConnectionInfo, + appStartedCompleter: futureAppStart, + connectionInfoCompleter: futureConnectionInfo, + enableDevTools: true, ); - final Future connectionInfo = onConnectionInfo.future; + final Future connectionInfo = futureConnectionInfo.future; expect(await result, 0); verify(mockFlutterDevice.initLogReader()).called(1); - expect(onConnectionInfo.isCompleted, true); + expect(futureConnectionInfo.isCompleted, true); expect((await connectionInfo).baseUri, 'foo://bar'); - expect(onAppStart.isCompleted, true); + expect(futureAppStart.isCompleted, true); expect(fakeVmServiceHost.hasRemainingExpectations, false); }), overrides: { DevtoolsLauncher: () => mockDevtoolsLauncher, @@ -278,7 +279,7 @@ void main() { return 0; }); - expect(await residentRunner.run(), 0); + expect(await residentRunner.run(enableDevTools: true), 0); verify(residentCompiler.recompile( any, any, @@ -412,7 +413,7 @@ void main() { return 0; }); - expect(await residentRunner.run(), 0); + expect(await residentRunner.run(enableDevTools: true), 0); verify(residentCompiler.recompile( any, any, @@ -477,21 +478,22 @@ void main() { ), target: 'main.dart', ); - final Completer onConnectionInfo = Completer.sync(); - final Completer onAppStart = Completer.sync(); + final Completer futureConnectionInfo = Completer.sync(); + final Completer futureAppStart = Completer.sync(); final Future result = residentRunner.attach( - appStartedCompleter: onAppStart, - connectionInfoCompleter: onConnectionInfo, + appStartedCompleter: futureAppStart, + connectionInfoCompleter: futureConnectionInfo, + enableDevTools: true, ); - final Future connectionInfo = onConnectionInfo.future; + final Future connectionInfo = futureConnectionInfo.future; expect(await result, 0); verify(mockFlutterDevice.initLogReader()).called(1); - expect(onConnectionInfo.isCompleted, true); + expect(futureConnectionInfo.isCompleted, true); expect((await connectionInfo).baseUri, 'foo://bar'); - expect(onAppStart.isCompleted, true); + expect(futureAppStart.isCompleted, true); expect(fakeVmServiceHost.hasRemainingExpectations, false); }), overrides: { DevtoolsLauncher: () => mockDevtoolsLauncher, @@ -504,13 +506,14 @@ void main() { setAssetBundlePath, listViews, ]); - final Completer onConnectionInfo = Completer.sync(); - final Completer onAppStart = Completer.sync(); + final Completer futureConnectionInfo = Completer.sync(); + final Completer futureAppStart = Completer.sync(); unawaited(residentRunner.attach( - appStartedCompleter: onAppStart, - connectionInfoCompleter: onConnectionInfo, + appStartedCompleter: futureAppStart, + connectionInfoCompleter: futureConnectionInfo, + enableDevTools: true, )); - await onAppStart.future; + await futureAppStart.future; when(mockFlutterDevice.updateDevFS( mainUri: anyNamed('mainUri'), target: anyNamed('target'), @@ -551,13 +554,13 @@ void main() { listViews, setAssetBundlePath, ]); - final Completer onConnectionInfo = Completer.sync(); - final Completer onAppStart = Completer.sync(); + final Completer futureConnectionInfo = Completer.sync(); + final Completer futureAppStart = Completer.sync(); unawaited(residentRunner.attach( - appStartedCompleter: onAppStart, - connectionInfoCompleter: onConnectionInfo, + appStartedCompleter: futureAppStart, + connectionInfoCompleter: futureConnectionInfo, )); - await onAppStart.future; + await futureAppStart.future; when(mockFlutterDevice.devFS).thenReturn(null); final OperationResult result = await residentRunner.restart(fullRestart: false); @@ -565,7 +568,7 @@ void main() { expect(result.code, 1); expect(result.message, contains('Device initialization has not completed.')); expect(fakeVmServiceHost.hasRemainingExpectations, false); - }), timeout: const Timeout(Duration(seconds: 15))); // https://github.com/flutter/flutter/issues/74539 + })); testUsingContext('ResidentRunner can handle an reload-barred exception from hot reload', () => testbed.run(() async { fakeVmServiceHost = FakeVmServiceHost(requests: [ @@ -574,13 +577,14 @@ void main() { setAssetBundlePath, listViews, ]); - final Completer onConnectionInfo = Completer.sync(); - final Completer onAppStart = Completer.sync(); + final Completer futureConnectionInfo = Completer.sync(); + final Completer futureAppStart = Completer.sync(); unawaited(residentRunner.attach( - appStartedCompleter: onAppStart, - connectionInfoCompleter: onConnectionInfo, + appStartedCompleter: futureAppStart, + connectionInfoCompleter: futureConnectionInfo, + enableDevTools: true, )); - await onAppStart.future; + await futureAppStart.future; when(mockFlutterDevice.updateDevFS( mainUri: anyNamed('mainUri'), target: anyNamed('target'), @@ -636,13 +640,14 @@ void main() { ], )), ); - final Completer onConnectionInfo = Completer.sync(); - final Completer onAppStart = Completer.sync(); + final Completer futureConnectionInfo = Completer.sync(); + final Completer futureAppStart = Completer.sync(); unawaited(residentRunner.attach( - appStartedCompleter: onAppStart, - connectionInfoCompleter: onConnectionInfo, + appStartedCompleter: futureAppStart, + connectionInfoCompleter: futureConnectionInfo, + enableDevTools: true, )); - await onAppStart.future; + await futureAppStart.future; when(mockFlutterDevice.updateDevFS( mainUri: anyNamed('mainUri'), target: anyNamed('target'), @@ -705,13 +710,14 @@ void main() { debuggingOptions: DebuggingOptions.enabled(BuildInfo.debug), target: 'main.dart', ); - final Completer onConnectionInfo = Completer.sync(); - final Completer onAppStart = Completer.sync(); + final Completer futureConnectionInfo = Completer.sync(); + final Completer futureAppStart = Completer.sync(); unawaited(residentRunner.attach( - appStartedCompleter: onAppStart, - connectionInfoCompleter: onConnectionInfo, + appStartedCompleter: futureAppStart, + connectionInfoCompleter: futureConnectionInfo, + enableDevTools: true, )); - await onAppStart.future; + await futureAppStart.future; when(mockFlutterDevice.updateDevFS( mainUri: anyNamed('mainUri'), target: anyNamed('target'), @@ -780,13 +786,14 @@ void main() { }, ), ]); - final Completer onConnectionInfo = Completer.sync(); - final Completer onAppStart = Completer.sync(); + final Completer futureConnectionInfo = Completer.sync(); + final Completer futureAppStart = Completer.sync(); unawaited(residentRunner.attach( - appStartedCompleter: onAppStart, - connectionInfoCompleter: onConnectionInfo, + appStartedCompleter: futureAppStart, + connectionInfoCompleter: futureConnectionInfo, + enableDevTools: true, )); - await onAppStart.future; + await futureAppStart.future; when(mockFlutterDevice.updateDevFS( mainUri: anyNamed('mainUri'), target: anyNamed('target'), @@ -861,13 +868,14 @@ void main() { }, ), ]); - final Completer onConnectionInfo = Completer.sync(); - final Completer onAppStart = Completer.sync(); + final Completer futureConnectionInfo = Completer.sync(); + final Completer futureAppStart = Completer.sync(); unawaited(residentRunner.attach( - appStartedCompleter: onAppStart, - connectionInfoCompleter: onConnectionInfo, + appStartedCompleter: futureAppStart, + connectionInfoCompleter: futureConnectionInfo, + enableDevTools: true, )); - await onAppStart.future; + await futureAppStart.future; when(mockFlutterDevice.updateDevFS( mainUri: anyNamed('mainUri'), target: anyNamed('target'), @@ -938,13 +946,14 @@ void main() { }, ), ]); - final Completer onConnectionInfo = Completer.sync(); - final Completer onAppStart = Completer.sync(); + final Completer futureConnectionInfo = Completer.sync(); + final Completer futureAppStart = Completer.sync(); unawaited(residentRunner.attach( - appStartedCompleter: onAppStart, - connectionInfoCompleter: onConnectionInfo, + appStartedCompleter: futureAppStart, + connectionInfoCompleter: futureConnectionInfo, + enableDevTools: true, )); - await onAppStart.future; + await futureAppStart.future; final OperationResult result = await residentRunner.restart(fullRestart: false); expect(result.fatal, false); @@ -1043,14 +1052,15 @@ void main() { ); }); - final Completer onConnectionInfo = Completer.sync(); - final Completer onAppStart = Completer.sync(); + final Completer futureConnectionInfo = Completer.sync(); + final Completer futureAppStart = Completer.sync(); unawaited(residentRunner.attach( - appStartedCompleter: onAppStart, - connectionInfoCompleter: onConnectionInfo, + appStartedCompleter: futureAppStart, + connectionInfoCompleter: futureConnectionInfo, + enableDevTools: true, )); - await onAppStart.future; + await futureAppStart.future; final OperationResult result = await residentRunner.restart(fullRestart: false); expect(result.fatal, false); @@ -1112,11 +1122,12 @@ void main() { ) ) ]); - final Completer onConnectionInfo = Completer.sync(); - final Completer onAppStart = Completer.sync(); + final Completer futureConnectionInfo = Completer.sync(); + final Completer futureAppStart = Completer.sync(); unawaited(residentRunner.attach( - appStartedCompleter: onAppStart, - connectionInfoCompleter: onConnectionInfo, + appStartedCompleter: futureAppStart, + connectionInfoCompleter: futureConnectionInfo, + enableDevTools: true, )); final OperationResult result = await residentRunner.restart(fullRestart: true); @@ -1189,11 +1200,12 @@ void main() { ) ) ]); - final Completer onConnectionInfo = Completer.sync(); - final Completer onAppStart = Completer.sync(); + final Completer futureConnectionInfo = Completer.sync(); + final Completer futureAppStart = Completer.sync(); unawaited(residentRunner.attach( - appStartedCompleter: onAppStart, - connectionInfoCompleter: onConnectionInfo, + appStartedCompleter: futureAppStart, + connectionInfoCompleter: futureConnectionInfo, + enableDevTools: true, )); final OperationResult result = await residentRunner.restart(fullRestart: true); @@ -1312,11 +1324,12 @@ void main() { ), ) ]); - final Completer onConnectionInfo = Completer.sync(); - final Completer onAppStart = Completer.sync(); + final Completer futureConnectionInfo = Completer.sync(); + final Completer futureAppStart = Completer.sync(); unawaited(residentRunner.attach( - appStartedCompleter: onAppStart, - connectionInfoCompleter: onConnectionInfo, + appStartedCompleter: futureAppStart, + connectionInfoCompleter: futureConnectionInfo, + enableDevTools: true, )); await residentRunner.restart(fullRestart: true); @@ -1334,13 +1347,14 @@ void main() { listViews, setAssetBundlePath, ]); - final Completer onConnectionInfo = Completer.sync(); - final Completer onAppStart = Completer.sync(); + final Completer futureConnectionInfo = Completer.sync(); + final Completer futureAppStart = Completer.sync(); unawaited(residentRunner.attach( - appStartedCompleter: onAppStart, - connectionInfoCompleter: onConnectionInfo, + appStartedCompleter: futureAppStart, + connectionInfoCompleter: futureConnectionInfo, + enableDevTools: true, )); - await onAppStart.future; + await futureAppStart.future; when(mockFlutterDevice.updateDevFS( mainUri: anyNamed('mainUri'), target: anyNamed('target'), @@ -1479,9 +1493,11 @@ void main() { commandHelp.v, commandHelp.P, commandHelp.a, + '', + '💪 Running with sound null safety 💪', + '', 'An Observatory debugger and profiler on FakeDevice is available at: null', - '\n💪 Running with sound null safety 💪', - '' + '', ].join('\n') )); })); @@ -1577,7 +1593,7 @@ void main() { listViews, setAssetBundlePath, ]); - final Future result = residentRunner.attach(); + final Future result = residentRunner.attach(enableDevTools: true); expect(await result, 0); // Verify DevTools was served. @@ -1611,7 +1627,7 @@ void main() { return 0; }); - final Future result = residentRunner.attach(); + final Future result = residentRunner.attach(enableDevTools: true); expect(await result, 0); // Verify DevTools was served. @@ -1624,6 +1640,51 @@ void main() { DevtoolsLauncher: () => mockDevtoolsLauncher, }); + testUsingContext('ResidentRunner ignores DevToolsLauncher when attaching with enableDevTools: false', () => testbed.run(() async { + fakeVmServiceHost = FakeVmServiceHost(requests: [ + listViews, + listViews, + setAssetBundlePath, + ]); + final Future result = residentRunner.attach(enableDevTools: false); + expect(await result, 0); + + // Verify DevTools was served. + verifyNever(mockDevtoolsLauncher.serve()); + }), overrides: { + DevtoolsLauncher: () => mockDevtoolsLauncher, + }); + + testUsingContext('ResidentRunner ignores DevtoolsLauncher when attaching with enableDevTools: false - cold mode', () => testbed.run(() async { + fakeVmServiceHost = FakeVmServiceHost(requests: [ + listViews, + listViews, + setAssetBundlePath, + ]); + residentRunner = ColdRunner( + [ + mockFlutterDevice, + ], + stayResident: false, + debuggingOptions: DebuggingOptions.enabled(BuildInfo.profile, vmserviceOutFile: 'foo'), + target: 'main.dart', + ); + when(mockFlutterDevice.runCold( + coldRunner: anyNamed('coldRunner'), + route: anyNamed('route'), + )).thenAnswer((Invocation invocation) async { + return 0; + }); + + final Future result = residentRunner.attach(enableDevTools: false); + expect(await result, 0); + + // Verify DevTools was served. + verifyNever(mockDevtoolsLauncher.serve()); + }), overrides: { + DevtoolsLauncher: () => mockDevtoolsLauncher, + }); + testUsingContext('ResidentRunner invokes DevtoolsLauncher when running and shutting down - cold mode', () => testbed.run(() async { fakeVmServiceHost = FakeVmServiceHost(requests: [ listViews, @@ -1645,7 +1706,7 @@ void main() { return 0; }); - final Future result = residentRunner.run(); + final Future result = residentRunner.run(enableDevTools: true); expect(await result, 0); // Verify DevTools was served. @@ -2277,7 +2338,7 @@ void main() { )).thenAnswer((Invocation invocation) async { return 0; }); - await residentRunner.run(); + await residentRunner.run(enableDevTools: true); expect(await globals.fs.file('foo').readAsString(), testUri.toString()); }), overrides: { @@ -2319,7 +2380,7 @@ void main() { )).thenAnswer((Invocation invocation) async { return 0; }); - await residentRunner.run(); + await residentRunner.run(enableDevTools: true); expect(await globals.fs.file(globals.fs.path.join('build', 'cache.dill')).readAsString(), 'ABC'); }), overrides: { @@ -2368,7 +2429,7 @@ void main() { )).thenAnswer((Invocation invocation) async { return 0; }); - await residentRunner.run(); + await residentRunner.run(enableDevTools: true); expect(await globals.fs.file(globals.fs.path.join( 'build', '187ef4436122d1cc2f40dc2b92f0eba0.cache.dill')).readAsString(), 'ABC'); @@ -2418,7 +2479,7 @@ void main() { )).thenAnswer((Invocation invocation) async { return 0; }); - await residentRunner.run(); + await residentRunner.run(enableDevTools: true); expect(await globals.fs.file(globals.fs.path.join( 'build', '3416d3007730479552122f01c01e326d.cache.dill')).readAsString(), 'ABC'); @@ -2462,7 +2523,7 @@ void main() { )).thenAnswer((Invocation invocation) async { return 0; }); - await residentRunner.run(); + await residentRunner.run(enableDevTools: true); expect(globals.fs.file(globals.fs.path.join('build', 'cache.dill')), isNot(exists)); }), overrides: { @@ -2509,7 +2570,7 @@ void main() { )).thenAnswer((Invocation invocation) async { return 0; }); - await residentRunner.run(); + await residentRunner.run(enableDevTools: true); expect(await globals.fs.file(globals.fs.path.join('build', 'cache.dill.track.dill')).readAsString(), 'ABC'); }), overrides: { @@ -2565,7 +2626,7 @@ void main() { )).thenAnswer((Invocation invocation) async { return 0; }); - await residentRunner.run(); + await residentRunner.run(enableDevTools: true); expect(testLogger.errorText, contains('Failed to write vmservice-out-file at foo')); expect(fakeVmServiceHost.hasRemainingExpectations, false); @@ -2608,9 +2669,10 @@ void main() { )).thenAnswer((Invocation invocation) async { return 0; }); - await residentRunner.run(); + await residentRunner.run(enableDevTools: true); // Await a short delay so that we don't try to exit before all the expected // VM service requests have been fired. + // TODO(ianh): remove this delay, it's almost certainly going to cause flakes await Future.delayed(const Duration(milliseconds: 200)); expect(await globals.fs.file('foo').readAsString(), testUri.toString()); diff --git a/packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart b/packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart index 12691ce3dfd..98759c6238c 100644 --- a/packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart +++ b/packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart @@ -289,7 +289,7 @@ void main() { }); testUsingContext('devToolsServerAddress returns parsed uri', () async { - final DummyFlutterCommand command = DummyFlutterCommand()..addDevToolsOptions(); + final DummyFlutterCommand command = DummyFlutterCommand()..addDevToolsOptions(verboseHelp: false); await createTestCommandRunner(command).run([ 'dummy', '--${FlutterCommand.kDevToolsServerAddress}', @@ -299,7 +299,7 @@ void main() { }); testUsingContext('devToolsServerAddress returns null for bad input', () async { - final DummyFlutterCommand command = DummyFlutterCommand()..addDevToolsOptions(); + final DummyFlutterCommand command = DummyFlutterCommand()..addDevToolsOptions(verboseHelp: false); final CommandRunner runner = createTestCommandRunner(command); await runner.run([ 'dummy', diff --git a/packages/flutter_tools/test/general.shard/terminal_handler_test.dart b/packages/flutter_tools/test/general.shard/terminal_handler_test.dart index de69d2f2cf3..3e06cac135c 100644 --- a/packages/flutter_tools/test/general.shard/terminal_handler_test.dart +++ b/packages/flutter_tools/test/general.shard/terminal_handler_test.dart @@ -341,6 +341,7 @@ class TestRunner extends Mock implements ResidentRunner { Future run({ Completer connectionInfoCompleter, Completer appStartedCompleter, + bool enableDevTools = false, String route, }) async => null; @@ -349,5 +350,6 @@ class TestRunner extends Mock implements ResidentRunner { Completer connectionInfoCompleter, Completer appStartedCompleter, bool allowExistingDdsInstance = false, + bool enableDevTools = false, }) async => null; }