From 21144362f86481aadb6fb356c24e5363c72ba375 Mon Sep 17 00:00:00 2001 From: Andrew Kolos Date: Thu, 14 Nov 2024 13:29:30 -0800 Subject: [PATCH] Move platform-specific log-reading implementation details from `ResidentRunner`/`FlutterDevice` to `DeviceLogReader` implementations (#156181) Cleans up https://github.com/flutter/flutter/pull/155800. In summary, `ResidentRunner`/`FlutterDevice` have branching behavior around logging that depends on the type of `DeviceLogReader` on the `FlutterDevice` instance. Let's instead move this behavior to the `DeviceLogReader` implementations. My apologies for the large diff. Much of this is a refactor that was a bit too difficult to separate into its own commits. Here are the main two changes * Replaces the mutable `connectedVmService` field on the `DeviceLogReader` class with a new method `provideVmService`. This serves largely the same purpose as the mutable field, but it allows for asynchronous code. This is where we put the logic that used to exist in `FlutterDevice.tryInitLogReader`. * Removes the `tryInitLogReader` method from `FlutterDevice`. This method served to set the `appPid` field on the `FlutterDevice`'s `DeviceLogReader` instance. This was only used in the case of Android to filter out logs unrelated to the flutter app coming from the device, so we can move this logic to `AdbLogReader`'s implementation of `provideVmService`. --- .../lib/src/android/android_device.dart | 35 +++++++++++++--- .../lib/src/custom_devices/custom_device.dart | 4 ++ .../flutter_tools/lib/src/desktop_device.dart | 4 ++ packages/flutter_tools/lib/src/device.dart | 14 ++----- .../lib/src/drive/drive_service.dart | 4 +- .../flutter_tools/lib/src/ios/devices.dart | 14 +++---- .../flutter_tools/lib/src/ios/simulators.dart | 4 ++ .../lib/src/proxied_devices/devices.dart | 4 ++ .../lib/src/resident_runner.dart | 22 +--------- packages/flutter_tools/lib/src/run_cold.dart | 4 -- packages/flutter_tools/lib/src/run_hot.dart | 3 +- .../commands.shard/hermetic/daemon_test.dart | 8 +--- .../hermetic/proxied_devices_test.dart | 5 +-- .../android/adb_log_reader_test.dart | 31 +++++++++++++- .../android/android_device_test.dart | 32 +++++++++++++++ .../test/general.shard/cold_test.dart | 3 -- .../drive/drive_service_test.dart | 5 --- .../ios/ios_device_logger_test.dart | 16 ++++---- .../resident_runner_helpers.dart | 3 -- .../general.shard/resident_runner_test.dart | 41 ------------------- .../resident_web_runner_test.dart | 3 -- .../flutter_tools/test/src/fake_devices.dart | 4 ++ 22 files changed, 136 insertions(+), 127 deletions(-) diff --git a/packages/flutter_tools/lib/src/android/android_device.dart b/packages/flutter_tools/lib/src/android/android_device.dart index 40a291ae0b..90d2ebdab7 100644 --- a/packages/flutter_tools/lib/src/android/android_device.dart +++ b/packages/flutter_tools/lib/src/android/android_device.dart @@ -6,6 +6,7 @@ import 'dart:async'; import 'package:meta/meta.dart'; import 'package:process/process.dart'; +import 'package:vm_service/vm_service.dart'; import '../application_package.dart'; import '../base/common.dart' show throwToolExit; @@ -21,6 +22,7 @@ import '../device_port_forwarder.dart'; import '../device_vm_service_discovery_for_attach.dart'; import '../project.dart'; import '../protocol_discovery.dart'; +import '../vmservice.dart'; import 'android.dart'; import 'android_builder.dart'; import 'android_console.dart'; @@ -618,6 +620,7 @@ class AndroidDevice extends Device { await AdbLogReader.createLogReader( this, _processManager, + _logger, ), portForwarder: portForwarder, hostPort: debuggingOptions.hostVmServicePort, @@ -796,12 +799,14 @@ class AndroidDevice extends Device { return _pastLogReader ??= await AdbLogReader.createLogReader( this, _processManager, + _logger, includePastLogs: true, ); } else { return _logReader ??= await AdbLogReader.createLogReader( this, _processManager, + _logger, ); } } @@ -1042,15 +1047,20 @@ class AndroidMemoryInfo extends MemoryInfo { /// A log reader that logs from `adb logcat`. class AdbLogReader extends DeviceLogReader { - AdbLogReader._(this._adbProcess, this.name); + AdbLogReader._(this._adbProcess, this.name, this._logger); @visibleForTesting - factory AdbLogReader.test(Process adbProcess, String name) = AdbLogReader._; + factory AdbLogReader.test( + Process adbProcess, + String name, + Logger logger, + ) = AdbLogReader._; /// Create a new [AdbLogReader] from an [AndroidDevice] instance. static Future createLogReader( AndroidDevice device, - ProcessManager processManager, { + ProcessManager processManager, + Logger logger, { bool includePastLogs = false, }) async { // logcat -T is not supported on Android releases before Lollipop. @@ -1085,11 +1095,15 @@ class AdbLogReader extends DeviceLogReader { ]); } final Process process = await processManager.start(device.adbCommandForDevice(args)); - return AdbLogReader._(process, device.name); + return AdbLogReader._(process, device.name, logger); } + int? _appPid; + final Process _adbProcess; + final Logger _logger; + @override final String name; @@ -1101,6 +1115,17 @@ class AdbLogReader extends DeviceLogReader { @override Stream get logLines => _linesController.stream; + @override + Future provideVmService(FlutterVmService connectedVmService) async { + final VM? vm = await connectedVmService.getVmGuarded(); + if (vm == null) { + _logger.printError('An error occurred when setting up filtering for adb logs. ' + 'Unable to communicate with the VM service.'); + } else { + _appPid = vm.pid; + } + } + void _start() { // We expect logcat streams to occasionally contain invalid utf-8, // see: https://github.com/flutter/flutter/pull/8864. @@ -1189,7 +1214,7 @@ class AdbLogReader extends DeviceLogReader { _fatalCrash = false; } } - } else if (appPid != null && int.parse(logMatch.group(1)!) == appPid) { + } else if (_appPid != null && int.parse(logMatch.group(1)!) == _appPid) { acceptLine = !_surfaceSyncerSpam.hasMatch(line); if (_fatalLog.hasMatch(line)) { diff --git a/packages/flutter_tools/lib/src/custom_devices/custom_device.dart b/packages/flutter_tools/lib/src/custom_devices/custom_device.dart index 4de1502151..c5f7745a4f 100644 --- a/packages/flutter_tools/lib/src/custom_devices/custom_device.dart +++ b/packages/flutter_tools/lib/src/custom_devices/custom_device.dart @@ -23,6 +23,7 @@ import '../device_port_forwarder.dart'; import '../features.dart'; import '../project.dart'; import '../protocol_discovery.dart'; +import '../vmservice.dart'; import 'custom_device_config.dart'; import 'custom_device_workflow.dart'; import 'custom_devices_config.dart'; @@ -111,6 +112,9 @@ class CustomDeviceLogReader extends DeviceLogReader { @override Stream get logLines => logLinesController.stream; + + @override + Future provideVmService(FlutterVmService connectedVmService) async { } } /// A [DevicePortForwarder] that uses commands to forward / unforward a port. diff --git a/packages/flutter_tools/lib/src/desktop_device.dart b/packages/flutter_tools/lib/src/desktop_device.dart index a7579bdd08..0ec5c5a907 100644 --- a/packages/flutter_tools/lib/src/desktop_device.dart +++ b/packages/flutter_tools/lib/src/desktop_device.dart @@ -19,6 +19,7 @@ import 'device_port_forwarder.dart'; import 'globals.dart' as globals; import 'macos/macos_device.dart'; import 'protocol_discovery.dart'; +import 'vmservice.dart'; /// A partial implementation of Device for desktop-class devices to inherit /// from, containing implementations that are common to all desktop devices. @@ -384,4 +385,7 @@ class DesktopLogReader extends DeviceLogReader { void dispose() { // Nothing to dispose. } + + @override + Future provideVmService(FlutterVmService connectedVmService) async { } } diff --git a/packages/flutter_tools/lib/src/device.dart b/packages/flutter_tools/lib/src/device.dart index fa198f535a..da778aa5f0 100644 --- a/packages/flutter_tools/lib/src/device.dart +++ b/packages/flutter_tools/lib/src/device.dart @@ -1440,14 +1440,11 @@ abstract class DeviceLogReader { /// Some logs can be obtained from a VM service stream. /// Set this after the VM services are connected. - FlutterVmService? connectedVMService; + Future provideVmService(FlutterVmService connectedVmService); @override String toString() => name; - /// Process ID of the app on the device. - int? appPid; - // Clean up resources allocated by log reader e.g. subprocesses void dispose(); } @@ -1466,17 +1463,14 @@ class NoOpDeviceLogReader implements DeviceLogReader { @override final String name; - @override - int? appPid; - - @override - FlutterVmService? connectedVMService; - @override Stream get logLines => const Stream.empty(); @override void dispose() { } + + @override + Future provideVmService(FlutterVmService connectedVmService) async {} } /// Append --null_assertions to any existing Dart VM flags if diff --git a/packages/flutter_tools/lib/src/drive/drive_service.dart b/packages/flutter_tools/lib/src/drive/drive_service.dart index bf6ccb6f01..bb7473d085 100644 --- a/packages/flutter_tools/lib/src/drive/drive_service.dart +++ b/packages/flutter_tools/lib/src/drive/drive_service.dart @@ -231,9 +231,7 @@ class FlutterDriverService extends DriverService { _vmService = await _vmServiceConnector(uri, device: _device, logger: _logger); final DeviceLogReader logReader = await device.getLogReader(app: _applicationPackage); logReader.logLines.listen(_logger.printStatus); - - final vm_service.VM vm = await _vmService.service.getVM(); - logReader.appPid = vm.pid; + await logReader.provideVmService(_vmService); } @override diff --git a/packages/flutter_tools/lib/src/ios/devices.dart b/packages/flutter_tools/lib/src/ios/devices.dart index 554abf65e6..faff4c8817 100644 --- a/packages/flutter_tools/lib/src/ios/devices.dart +++ b/packages/flutter_tools/lib/src/ios/devices.dart @@ -1300,16 +1300,12 @@ class IOSDeviceLogReader extends DeviceLogReader { @override Stream get logLines => linesController.stream; - @override - FlutterVmService? get connectedVMService => _connectedVMService; - FlutterVmService? _connectedVMService; + FlutterVmService? _connectedVmService; @override - set connectedVMService(FlutterVmService? connectedVmService) { - if (connectedVmService != null) { - _listenToUnifiedLoggingEvents(connectedVmService); - } - _connectedVMService = connectedVmService; + Future provideVmService(FlutterVmService connectedVmService) async { + await _listenToUnifiedLoggingEvents(connectedVmService); + _connectedVmService = connectedVmService; } static const int minimumUniversalLoggingSdkVersion = 13; @@ -1360,7 +1356,7 @@ class IOSDeviceLogReader extends DeviceLogReader { // CoreDevice and has iOS 13 or greater. // When using `ios-deploy` and the Dart VM, prefer the more complete logs // from the attached debugger, if available. - if (connectedVMService != null && (_iosDeployDebugger == null || !_iosDeployDebugger!.debuggerAttached)) { + if (_connectedVmService != null && (_iosDeployDebugger == null || !_iosDeployDebugger!.debuggerAttached)) { return _IOSDeviceLogSources( primarySource: IOSDeviceLogSource.unifiedLogging, fallbackSource: IOSDeviceLogSource.iosDeploy, diff --git a/packages/flutter_tools/lib/src/ios/simulators.dart b/packages/flutter_tools/lib/src/ios/simulators.dart index 97d61857ca..f402b3bd7f 100644 --- a/packages/flutter_tools/lib/src/ios/simulators.dart +++ b/packages/flutter_tools/lib/src/ios/simulators.dart @@ -26,6 +26,7 @@ import '../globals.dart' as globals; import '../macos/xcode.dart'; import '../project.dart'; import '../protocol_discovery.dart'; +import '../vmservice.dart'; import 'application_package.dart'; import 'mac.dart'; import 'plist_parser.dart'; @@ -1022,6 +1023,9 @@ class _IOSSimulatorLogReader extends DeviceLogReader { void dispose() { _stop(); } + + @override + Future provideVmService(FlutterVmService connectedVmService) async { } } class _IOSSimulatorDevicePortForwarder extends DevicePortForwarder { diff --git a/packages/flutter_tools/lib/src/proxied_devices/devices.dart b/packages/flutter_tools/lib/src/proxied_devices/devices.dart index 4163beb02e..f66560d66e 100644 --- a/packages/flutter_tools/lib/src/proxied_devices/devices.dart +++ b/packages/flutter_tools/lib/src/proxied_devices/devices.dart @@ -20,6 +20,7 @@ import '../device_port_forwarder.dart'; import '../device_vm_service_discovery_for_attach.dart'; import '../project.dart'; import '../resident_runner.dart'; +import '../vmservice.dart'; import 'debounce_data_stream.dart'; import 'file_transfer.dart'; @@ -517,6 +518,9 @@ class _ProxiedLogReader extends DeviceLogReader { }); } } + + @override + Future provideVmService(FlutterVmService connectedVmService) async { } } /// A port forwarded by a [ProxiedPortForwarder]. diff --git a/packages/flutter_tools/lib/src/resident_runner.dart b/packages/flutter_tools/lib/src/resident_runner.dart index af8aabbea5..2e2ee0dc98 100644 --- a/packages/flutter_tools/lib/src/resident_runner.dart +++ b/packages/flutter_tools/lib/src/resident_runner.dart @@ -8,7 +8,6 @@ import 'package:meta/meta.dart'; import 'package:package_config/package_config.dart'; import 'package:vm_service/vm_service.dart' as vm_service; -import 'android/android_device.dart'; import 'application_package.dart'; import 'artifacts.dart'; import 'asset.dart'; @@ -346,7 +345,8 @@ class FlutterDevice { globals.printTrace('Successfully connected to service protocol: $vmServiceUri'); vmService = service; - (await device!.getLogReader(app: package)).connectedVMService = vmService; + await (await device!.getLogReader(app: package)) + .provideVmService(vmService!); completer.complete(); await subscription.cancel(); }, onError: (dynamic error) { @@ -417,24 +417,6 @@ class FlutterDevice { _loggingSubscription = null; } - /// Attempts to set up reading logs from the Flutter app on the device. - /// - /// This can fail if the device if no longer connected. - Future tryInitLogReader() async { - final vm_service.VM? vm = await vmService!.getVmGuarded(); - final DeviceLogReader logReader = await device!.getLogReader(app: package); - if (vm == null && logReader is AdbLogReader) { - // TODO(andrewkolos): This is a temporary, hacky fix for - // https://github.com/flutter/flutter/issues/155795 that emphasizes - // simplicity for the sake of being suitable for cherry-picking. - globals.printError( - 'Unable to initiate adb log filtering for device' - '${device?.name}. Logs from the device may be more verbose than usual.', - ); - } - logReader.appPid = vm?.pid; - } - Future runHot({ required HotRunner hotRunner, String? route, diff --git a/packages/flutter_tools/lib/src/run_cold.dart b/packages/flutter_tools/lib/src/run_cold.dart index 55e699cdc1..cfe5979d1e 100644 --- a/packages/flutter_tools/lib/src/run_cold.dart +++ b/packages/flutter_tools/lib/src/run_cold.dart @@ -104,7 +104,6 @@ class ColdRunner extends ResidentRunner { if (device!.vmService == null) { continue; } - await device.tryInitLogReader(); globals.printTrace('Connected to ${device.device!.name}'); } @@ -153,9 +152,6 @@ class ColdRunner extends ResidentRunner { return 2; } - for (final FlutterDevice? device in flutterDevices) { - await device!.tryInitLogReader(); - } for (final FlutterDevice? device in flutterDevices) { final List views = await device!.vmService!.getFlutterViews(); for (final FlutterView view in views) { diff --git a/packages/flutter_tools/lib/src/run_hot.dart b/packages/flutter_tools/lib/src/run_hot.dart index dc27fd0bf3..9eb5fc121d 100644 --- a/packages/flutter_tools/lib/src/run_hot.dart +++ b/packages/flutter_tools/lib/src/run_hot.dart @@ -261,8 +261,7 @@ class HotRunner extends ResidentRunner { } for (final FlutterDevice? device in flutterDevices) { - await device!.tryInitLogReader(); - device + device! .developmentShaderCompiler .configureCompiler(device.targetPlatform); } diff --git a/packages/flutter_tools/test/commands.shard/hermetic/daemon_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/daemon_test.dart index 4e62f13b90..4f998d8473 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/daemon_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/daemon_test.dart @@ -1262,12 +1262,6 @@ class FakeDeviceLogReader implements DeviceLogReader { final StreamController logLinesController = StreamController(); bool disposeCalled = false; - @override - int? appPid; - - @override - FlutterVmService? connectedVMService; - @override void dispose() { disposeCalled = true; @@ -1279,6 +1273,8 @@ class FakeDeviceLogReader implements DeviceLogReader { @override String get name => 'device'; + @override + Future provideVmService(FlutterVmService? connectedVmService) async { } } class FakeApplicationPackageFactory implements ApplicationPackageFactory { diff --git a/packages/flutter_tools/test/commands.shard/hermetic/proxied_devices_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/proxied_devices_test.dart index 35468443cb..738144f2b2 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/proxied_devices_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/proxied_devices_test.dart @@ -354,10 +354,7 @@ class FakeDeviceLogReader implements DeviceLogReader { bool disposeCalled = false; @override - int? appPid; - - @override - FlutterVmService? connectedVMService; + Future provideVmService(FlutterVmService? connectedVmService) async { } @override void dispose() { diff --git a/packages/flutter_tools/test/general.shard/android/adb_log_reader_test.dart b/packages/flutter_tools/test/general.shard/android/adb_log_reader_test.dart index 543aff1c7c..2f6d264d36 100644 --- a/packages/flutter_tools/test/general.shard/android/adb_log_reader_test.dart +++ b/packages/flutter_tools/test/general.shard/android/adb_log_reader_test.dart @@ -5,7 +5,10 @@ import 'dart:async'; import 'package:flutter_tools/src/android/android_device.dart'; +import 'package:flutter_tools/src/base/logger.dart'; +import 'package:flutter_tools/src/vmservice.dart'; import 'package:test/fake.dart'; +import 'package:vm_service/vm_service.dart'; import '../../src/common.dart'; import '../../src/fake_process_manager.dart'; @@ -18,6 +21,24 @@ const String kLastLogcatTimestamp = '11-27 15:39:04.506'; /// line as the first input to disable this behavior. const String kDummyLine = 'Contents are not important\n'; +class _FakeVm extends Fake implements VM { + _FakeVm(this._appPid); + + final int _appPid; + + @override + int? get pid => _appPid; +} + +class _FakeFlutterVmService extends Fake implements FlutterVmService { + _FakeFlutterVmService(this._appPid); + + final int _appPid; + + @override + Future getVmGuarded() async => _FakeVm(_appPid); +} + void main() { testWithoutContext('AdbLogReader ignores spam from SurfaceSyncer', () async { const int appPid = 1; @@ -44,7 +65,9 @@ void main() { final AdbLogReader logReader = await AdbLogReader.createLogReader( createFakeDevice(null), processManager, - )..appPid = appPid; + BufferLogger.test(), + ); + await logReader.provideVmService(_FakeFlutterVmService(appPid)); final Completer onDone = Completer.sync(); final List emittedLines = []; logReader.logLines.listen((String line) { @@ -76,6 +99,7 @@ void main() { await AdbLogReader.createLogReader( createFakeDevice(kLollipopVersionCode), processManager, + BufferLogger.test(), ); expect(processManager, hasNoRemainingExpectations); @@ -99,6 +123,7 @@ void main() { await AdbLogReader.createLogReader( createFakeDevice(kLollipopVersionCode - 1), processManager, + BufferLogger.test(), ); expect(processManager, hasNoRemainingExpectations); @@ -122,6 +147,7 @@ void main() { await AdbLogReader.createLogReader( createFakeDevice(null), processManager, + BufferLogger.test(), ); expect(processManager, hasNoRemainingExpectations); @@ -147,6 +173,7 @@ void main() { await AdbLogReader.createLogReader( createFakeDevice(null), processManager, + BufferLogger.test(), includePastLogs: true, ); @@ -173,6 +200,7 @@ void main() { final AdbLogReader logReader = await AdbLogReader.createLogReader( createFakeDevice(null), processManager, + BufferLogger.test(), ); final Completer onDone = Completer.sync(); logReader.logLines.listen((String _) { }, onDone: onDone.complete); @@ -207,6 +235,7 @@ void main() { final AdbLogReader logReader = await AdbLogReader.createLogReader( createFakeDevice(null), processManager, + BufferLogger.test() ); await expectLater(logReader.logLines, emitsInOrder([ 'E/AndroidRuntime(11787): FATAL EXCEPTION: main', diff --git a/packages/flutter_tools/test/general.shard/android/android_device_test.dart b/packages/flutter_tools/test/general.shard/android/android_device_test.dart index 99898c28c1..dcf49224e6 100644 --- a/packages/flutter_tools/test/general.shard/android/android_device_test.dart +++ b/packages/flutter_tools/test/general.shard/android/android_device_test.dart @@ -16,10 +16,14 @@ import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/base/platform.dart'; import 'package:flutter_tools/src/build_info.dart'; import 'package:flutter_tools/src/device.dart'; +import 'package:flutter_tools/src/globals.dart' as globals; import 'package:flutter_tools/src/project.dart'; +import 'package:flutter_tools/src/vmservice.dart'; import 'package:test/fake.dart'; +import 'package:vm_service/vm_service.dart'; import '../../src/common.dart'; +import '../../src/context.dart'; import '../../src/fake_process_manager.dart'; void main() { @@ -470,6 +474,34 @@ Uptime: 441088659 Realtime: 521464097 expect(await device.stopApp(null), isFalse); }); + testUsingContext('AdbLogReader.provideVmService catches any RPCError due to VM service disconnection', () async { + final BufferLogger logger = globals.logger as BufferLogger; + final FlutterVmService vmService = FlutterVmService(_MyFakeVmService()); + final AdbLogReader logReader = AdbLogReader.test( + FakeProcess(), + 'foo', + logger, + ); + await logReader.provideVmService(vmService); + expect( + logger.traceText, + 'VmService.getVm call failed: null: (-32000) ' + 'Service connection disposed\n', + ); + expect( + logger.errorText, + 'An error occurred when setting up filtering for adb logs. ' + 'Unable to communicate with the VM service.\n', + ); + }, overrides: { + Logger: () => BufferLogger.test(), + }); +} + +class _MyFakeVmService extends Fake implements VmService { + @override Future getVM() async { + throw RPCError(null, RPCErrorKind.kServerError.code, 'Service connection disposed'); + } } AndroidDevice setUpAndroidDevice({ diff --git a/packages/flutter_tools/test/general.shard/cold_test.dart b/packages/flutter_tools/test/general.shard/cold_test.dart index 51521645e4..6fa138d1b8 100644 --- a/packages/flutter_tools/test/general.shard/cold_test.dart +++ b/packages/flutter_tools/test/general.shard/cold_test.dart @@ -163,9 +163,6 @@ class FakeFlutterDevice extends Fake implements FlutterDevice { Future runCold({ColdRunner? coldRunner, String? route}) async { return runColdCode; } - - @override - Future tryInitLogReader() async { } } class FakeDevice extends Fake implements Device { diff --git a/packages/flutter_tools/test/general.shard/drive/drive_service_test.dart b/packages/flutter_tools/test/general.shard/drive/drive_service_test.dart index 4bc504b2d0..1065ef83e8 100644 --- a/packages/flutter_tools/test/general.shard/drive/drive_service_test.dart +++ b/packages/flutter_tools/test/general.shard/drive/drive_service_test.dart @@ -273,7 +273,6 @@ void main() { testUsingContext('Writes SkSL to file when provided with out file', () async { final MemoryFileSystem fileSystem = MemoryFileSystem.test(); final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost(requests: [ - getVM, listViews, const FakeVmServiceRequest( method: '_flutter.getSkSLs', @@ -310,7 +309,6 @@ void main() { testWithoutContext('Can connect to existing application and stop it during cleanup', () async { final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost(requests: [ - getVM, getVM, const FakeVmServiceRequest( method: 'ext.flutter.exit', @@ -333,7 +331,6 @@ void main() { testWithoutContext('Can connect to existing application using ws URI', () async { final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost(requests: [ - getVM, getVM, const FakeVmServiceRequest( method: 'ext.flutter.exit', @@ -356,7 +353,6 @@ void main() { testWithoutContext('Can connect to existing application using ws URI (no trailing slash)', () async { final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost(requests: [ - getVM, getVM, const FakeVmServiceRequest( method: 'ext.flutter.exit', @@ -379,7 +375,6 @@ void main() { testWithoutContext('Can connect to existing application using ws URI (no trailing slash, ws in auth code)', () async { final FakeVmServiceHost fakeVmServiceHost = FakeVmServiceHost(requests: [ - getVM, getVM, const FakeVmServiceRequest( method: 'ext.flutter.exit', diff --git a/packages/flutter_tools/test/general.shard/ios/ios_device_logger_test.dart b/packages/flutter_tools/test/general.shard/ios/ios_device_logger_test.dart index c113334ba0..6d3839f8b1 100644 --- a/packages/flutter_tools/test/general.shard/ios/ios_device_logger_test.dart +++ b/packages/flutter_tools/test/general.shard/ios/ios_device_logger_test.dart @@ -181,7 +181,7 @@ Runner(libsystem_asl.dylib)[297] : libMobileGestalt logger: logger, ), ); - logReader.connectedVMService = vmService; + await logReader.provideVmService(vmService); // Wait for stream listeners to fire. await expectLater(logReader.logLines, emitsInAnyOrder([ @@ -223,7 +223,7 @@ Runner(libsystem_asl.dylib)[297] : libMobileGestalt logger: logger, ), ); - logReader.connectedVMService = vmService; + await logReader.provideVmService(vmService); final FakeIOSDeployDebugger iosDeployDebugger = FakeIOSDeployDebugger(); iosDeployDebugger.debuggerAttached = true; @@ -425,7 +425,7 @@ Runner(libsystem_asl.dylib)[297] : libMobileGestalt expect(logReader.logSources.fallbackSource, IOSDeviceLogSource.unifiedLogging); }); - testWithoutContext('for iOS 13 or greater non-CoreDevice, _iosDeployDebugger not attached, and VM is connected', () { + testWithoutContext('for iOS 13 or greater non-CoreDevice, _iosDeployDebugger not attached, and VM is connected', () async { final IOSDeviceLogReader logReader = IOSDeviceLogReader.test( iMobileDevice: IMobileDevice( artifacts: artifacts, @@ -448,7 +448,7 @@ Runner(libsystem_asl.dylib)[297] : libMobileGestalt }), ]).vmService; - logReader.connectedVMService = vmService; + await logReader.provideVmService(vmService); expect(logReader.useSyslogLogging, isFalse); expect(logReader.useUnifiedLogging, isTrue); @@ -457,7 +457,7 @@ Runner(libsystem_asl.dylib)[297] : libMobileGestalt expect(logReader.logSources.fallbackSource, IOSDeviceLogSource.iosDeploy); }); - testWithoutContext('for iOS 13 or greater non-CoreDevice and _iosDeployDebugger is attached', () { + testWithoutContext('for iOS 13 or greater non-CoreDevice and _iosDeployDebugger is attached', () async { final IOSDeviceLogReader logReader = IOSDeviceLogReader.test( iMobileDevice: IMobileDevice( artifacts: artifacts, @@ -484,7 +484,7 @@ Runner(libsystem_asl.dylib)[297] : libMobileGestalt }), ]).vmService; - logReader.connectedVMService = vmService; + await logReader.provideVmService(vmService); expect(logReader.useSyslogLogging, isFalse); expect(logReader.useUnifiedLogging, isTrue); @@ -685,7 +685,7 @@ Runner(libsystem_asl.dylib)[297] : libMobileGestalt logger: logger, ), ); - logReader.connectedVMService = vmService; + await logReader.provideVmService(vmService); // Wait for stream listeners to fire. expect(logReader.useUnifiedLogging, isTrue); @@ -729,7 +729,7 @@ Runner(libsystem_asl.dylib)[297] : libMobileGestalt ), majorSdkVersion: 12, ); - logReader.connectedVMService = vmService; + await logReader.provideVmService(vmService); final List lines = await logReader.logLines.toList(); diff --git a/packages/flutter_tools/test/general.shard/resident_runner_helpers.dart b/packages/flutter_tools/test/general.shard/resident_runner_helpers.dart index e350513d2a..1aa10fc0e9 100644 --- a/packages/flutter_tools/test/general.shard/resident_runner_helpers.dart +++ b/packages/flutter_tools/test/general.shard/resident_runner_helpers.dart @@ -227,9 +227,6 @@ class FakeFlutterDevice extends Fake implements FlutterDevice { @override Future stopEchoingDeviceLog() async { } - @override - Future tryInitLogReader() async { } - @override Future setupDevFS(String fsName, Directory rootDirectory) async { return testUri!; 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 a140cc1f6f..545c6d4294 100644 --- a/packages/flutter_tools/test/general.shard/resident_runner_test.dart +++ b/packages/flutter_tools/test/general.shard/resident_runner_test.dart @@ -682,10 +682,6 @@ void main() { testUsingContext('ResidentRunner reports hot reload time details', () => testbed.run(() async { fakeVmServiceHost = FakeVmServiceHost(requests: [ listViews, - FakeVmServiceRequest( - method: 'getVM', - jsonResponse: fakeVM.toJson(), - ), listViews, listViews, FakeVmServiceRequest( @@ -1929,43 +1925,6 @@ flutter: ProcessManager: () => FakeProcessManager.any(), }); - testUsingContext('FlutterDevice does not throw when unable to initiate log reader due to VM service disconnection', () async { - fakeVmServiceHost = FakeVmServiceHost( - requests: [ - FakeVmServiceRequest( - method: 'getVM', - error: FakeRPCError( - code: vm_service.RPCErrorKind.kServerError.code, - error: 'Service connection disposed', - ), - ), - ], - ); - final TestFlutterDevice flutterDevice = TestFlutterDevice(device); - flutterDevice.vmService = fakeVmServiceHost!.vmService; - await flutterDevice.tryInitLogReader(); - - final BufferLogger logger = globals.logger as BufferLogger; - expect( - logger.traceText, - contains( - 'VmService.getVm call failed: getVM: (-32000) ' - 'Service connection disposed\n', - ), - ); - // We should not print a warning since the device does not have a connected - // adb log reader. - // TODO(andrewkolos): This test is a bit fragile, and is something that - // should be corrected in a follow-up PR (see - // https://github.com/flutter/flutter/issues/155795). - expect(logger.errorText, isEmpty); - }, overrides: { - Logger: () => BufferLogger.test(), - Artifacts: () => Artifacts.test(), - FileSystem: () => MemoryFileSystem.test(), - ProcessManager: () => FakeProcessManager.any(), - }); - testUsingContext('Uses existing DDS URI from exception field', () => testbed.run(() async { fakeVmServiceHost = FakeVmServiceHost(requests: []); final FakeDevice device = FakeDevice() diff --git a/packages/flutter_tools/test/general.shard/resident_web_runner_test.dart b/packages/flutter_tools/test/general.shard/resident_web_runner_test.dart index 6b82c518a8..b9695f7690 100644 --- a/packages/flutter_tools/test/general.shard/resident_web_runner_test.dart +++ b/packages/flutter_tools/test/general.shard/resident_web_runner_test.dart @@ -1760,9 +1760,6 @@ class FakeFlutterDevice extends Fake implements FlutterDevice { @override Future stopEchoingDeviceLog() async {} - @override - Future tryInitLogReader() async {} - @override Future setupDevFS(String fsName, Directory rootDirectory) async { return testUri; diff --git a/packages/flutter_tools/test/src/fake_devices.dart b/packages/flutter_tools/test/src/fake_devices.dart index 5586244327..1a66178165 100644 --- a/packages/flutter_tools/test/src/fake_devices.dart +++ b/packages/flutter_tools/test/src/fake_devices.dart @@ -8,6 +8,7 @@ import 'package:flutter_tools/src/application_package.dart'; import 'package:flutter_tools/src/build_info.dart'; import 'package:flutter_tools/src/device.dart'; import 'package:flutter_tools/src/project.dart'; +import 'package:flutter_tools/src/vmservice.dart'; import 'fakes.dart'; @@ -311,4 +312,7 @@ class FakeDeviceLogReader extends DeviceLogReader { await _linesController.close(); disposed = true; } + + @override + Future provideVmService(FlutterVmService? connectedVmService) async { } }