From 1371b8dc3cf3448226c8d2ab0f097741e8364734 Mon Sep 17 00:00:00 2001 From: Christopher Fujino Date: Mon, 18 Jul 2022 16:10:06 -0700 Subject: [PATCH] [flutter_tools] Fix null check errors in attach command (#107864) --- .../lib/src/commands/attach.dart | 20 +- .../flutter_tools/lib/src/mdns_discovery.dart | 2 +- .../commands.shard/hermetic/attach_test.dart | 262 ++++++++++++------ 3 files changed, 197 insertions(+), 87 deletions(-) diff --git a/packages/flutter_tools/lib/src/commands/attach.dart b/packages/flutter_tools/lib/src/commands/attach.dart index bc50508185..55e350958a 100644 --- a/packages/flutter_tools/lib/src/commands/attach.dart +++ b/packages/flutter_tools/lib/src/commands/attach.dart @@ -215,7 +215,11 @@ known, it can be explicitly provided to attach via the command-line, e.g. Future runCommand() async { await _validateArguments(); - final Device device = await (findTargetDevice() as FutureOr); + final Device? device = await findTargetDevice(); + + if (device == null) { + throwToolExit('Did not find any valid target devices.'); + } final Artifacts? overrideArtifacts = device.artifactOverrides ?? globals.artifacts; await context.run( @@ -272,7 +276,7 @@ known, it can be explicitly provided to attach via the command-line, e.g. } else if ((device is IOSDevice) || (device is IOSSimulator) || (device is MacOSDesignedForIPadDevice)) { final Uri? uriFromMdns = await MDnsObservatoryDiscovery.instance!.getObservatoryUri( - appId!, + appId, device, usesIpv6: usesIpv6, deviceVmservicePort: deviceVmservicePort, @@ -317,12 +321,12 @@ known, it can be explicitly provided to attach via the command-line, e.g. try { int? result; if (daemon != null) { - final ResidentRunner runner = await (createResidentRunner( + final ResidentRunner runner = await createResidentRunner( observatoryUris: observatoryUri, device: device, flutterProject: flutterProject, usesIpv6: usesIpv6, - ) as FutureOr); + ); late AppInstance app; try { app = await daemon.appDomain.launch( @@ -351,12 +355,12 @@ known, it can be explicitly provided to attach via the command-line, e.g. return; } while (true) { - final ResidentRunner runner = await (createResidentRunner( + final ResidentRunner runner = await createResidentRunner( observatoryUris: observatoryUri, device: device, flutterProject: flutterProject, usesIpv6: usesIpv6, - ) as FutureOr); + ); final Completer onAppStart = Completer.sync(); TerminalHandler? terminalHandler; unawaited(onAppStart.future.whenComplete(() { @@ -400,7 +404,7 @@ known, it can be explicitly provided to attach via the command-line, e.g. } } - Future createResidentRunner({ + Future createResidentRunner({ required Stream observatoryUris, required Device device, required FlutterProject flutterProject, @@ -452,7 +456,7 @@ known, it can be explicitly provided to attach via the command-line, e.g. } class HotRunnerFactory { - HotRunner? build( + HotRunner build( List devices, { required String target, required DebuggingOptions debuggingOptions, diff --git a/packages/flutter_tools/lib/src/mdns_discovery.dart b/packages/flutter_tools/lib/src/mdns_discovery.dart index ce7d8c2602..13882b923b 100644 --- a/packages/flutter_tools/lib/src/mdns_discovery.dart +++ b/packages/flutter_tools/lib/src/mdns_discovery.dart @@ -145,7 +145,7 @@ class MDnsObservatoryDiscovery { } } - Future getObservatoryUri(String applicationId, Device device, { + Future getObservatoryUri(String? applicationId, Device device, { bool usesIpv6 = false, int? hostVmservicePort, int? deviceVmservicePort, 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 2575d7f527..0610be0b62 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/attach_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/attach_test.dart @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -// @dart = 2.8 - import 'dart:async'; import 'package:file/memory.dart'; @@ -13,6 +11,7 @@ import 'package:flutter_tools/src/artifacts.dart'; import 'package:flutter_tools/src/base/common.dart'; import 'package:flutter_tools/src/base/dds.dart'; import 'package:flutter_tools/src/base/file_system.dart'; +import 'package:flutter_tools/src/base/io.dart'; import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/base/terminal.dart'; import 'package:flutter_tools/src/build_info.dart'; @@ -24,11 +23,13 @@ import 'package:flutter_tools/src/globals.dart' as globals; import 'package:flutter_tools/src/ios/application_package.dart'; import 'package:flutter_tools/src/ios/devices.dart'; import 'package:flutter_tools/src/macos/macos_ipad_device.dart'; +import 'package:flutter_tools/src/mdns_discovery.dart'; import 'package:flutter_tools/src/project.dart'; +import 'package:flutter_tools/src/reporting/reporting.dart'; import 'package:flutter_tools/src/resident_runner.dart'; import 'package:flutter_tools/src/run_hot.dart'; import 'package:flutter_tools/src/vmservice.dart'; -import 'package:meta/meta.dart'; +import 'package:multicast_dns/multicast_dns.dart'; import 'package:test/fake.dart'; import 'package:vm_service/vm_service.dart' as vm_service; @@ -43,8 +44,8 @@ void main() { }); group('attach', () { - StreamLogger logger; - FileSystem testFileSystem; + late StreamLogger logger; + late FileSystem testFileSystem; setUp(() { Cache.disableLocking(); @@ -62,10 +63,10 @@ void main() { const int devicePort = 499; const int hostPort = 42; - FakeDeviceLogReader fakeLogReader; - RecordingPortForwarder portForwarder; - FakeDartDevelopmentService fakeDds; - FakeAndroidDevice device; + late FakeDeviceLogReader fakeLogReader; + late RecordingPortForwarder portForwarder; + late FakeDartDevelopmentService fakeDds; + late FakeAndroidDevice device; setUp(() { fakeLogReader = FakeDeviceLogReader(); @@ -80,6 +81,45 @@ void main() { fakeLogReader.dispose(); }); + testUsingContext('succeeds with iOS device', () async { + final FakeIOSDevice device = FakeIOSDevice( + logReader: fakeLogReader, + portForwarder: portForwarder, + onGetLogReader: () { + fakeLogReader.addLine('Foo'); + fakeLogReader.addLine('The Dart VM service is listening on http://127.0.0.1:$devicePort'); + return fakeLogReader; + }, + ); + + testDeviceManager.addDevice(device); + final Completer completer = Completer(); + final StreamSubscription loggerSubscription = logger.stream.listen((String message) { + if (message == '[verbose] Observatory URL on device: http://127.0.0.1:$devicePort') { + // The "Observatory URL on device" message is output by the ProtocolDiscovery when it found the observatory. + completer.complete(); + } + }); + final Future task = createTestCommandRunner(AttachCommand()).run(['attach']); + await completer.future; + + expect(portForwarder.devicePort, devicePort); + expect(portForwarder.hostPort, hostPort); + + await fakeLogReader.dispose(); + await expectLoggerInterruptEndsTask(task, logger); + await loggerSubscription.cancel(); + }, overrides: { + FileSystem: () => testFileSystem, + ProcessManager: () => FakeProcessManager.any(), + Logger: () => logger, + MDnsObservatoryDiscovery: () => MDnsObservatoryDiscovery( + mdnsClient: FakeMDnsClient([], >{}), + logger: logger, + flutterUsage: TestUsage(), + ), + }); + testUsingContext('finds observatory port and forwards', () async { device.onGetLogReader = () { fakeLogReader.addLine('Foo'); @@ -139,8 +179,8 @@ void main() { final FakeHotRunner hotRunner = FakeHotRunner(); hotRunner.onAttach = ( - Completer connectionInfoCompleter, - Completer appStartedCompleter, + Completer? connectionInfoCompleter, + Completer? appStartedCompleter, bool allowExistingDdsInstance, bool enableDevTools, ) async => 0; @@ -224,8 +264,8 @@ void main() { group('forwarding to given port', () { const int devicePort = 499; const int hostPort = 42; - RecordingPortForwarder portForwarder; - FakeAndroidDevice device; + late RecordingPortForwarder portForwarder; + late FakeAndroidDevice device; setUp(() { final FakeDartDevelopmentService fakeDds = FakeDartDevelopmentService(); @@ -408,8 +448,8 @@ void main() { final FakeHotRunnerFactory hotRunnerFactory = FakeHotRunnerFactory() ..hotRunner = hotRunner; hotRunner.onAttach = ( - Completer connectionInfoCompleter, - Completer appStartedCompleter, + Completer? connectionInfoCompleter, + Completer? appStartedCompleter, bool allowExistingDdsInstance, bool enableDevTools, ) async { @@ -438,8 +478,8 @@ void main() { ..hotRunner = hotRunner; hotRunner.onAttach = ( - Completer connectionInfoCompleter, - Completer appStartedCompleter, + Completer? connectionInfoCompleter, + Completer? appStartedCompleter, bool allowExistingDdsInstance, bool enableDevTools, ) async { @@ -447,7 +487,6 @@ void main() { throw vm_service.RPCError('flutter._listViews', RPCErrorCodes.kInvalidParams, ''); }; - testDeviceManager.addDevice(device); testFileSystem.file('lib/main.dart').createSync(); @@ -463,7 +502,7 @@ void main() { } class FakeHotRunner extends Fake implements HotRunner { - Future Function(Completer, Completer, bool, bool) onAttach; + late Future Function(Completer?, Completer?, bool, bool) onAttach; @override bool exited = false; @@ -473,8 +512,8 @@ class FakeHotRunner extends Fake implements HotRunner { @override Future attach({ - Completer connectionInfoCompleter, - Completer appStartedCompleter, + Completer? connectionInfoCompleter, + Completer? appStartedCompleter, bool allowExistingDdsInstance = false, bool enableDevTools = false, bool needsFullRestart = true, @@ -484,25 +523,25 @@ class FakeHotRunner extends Fake implements HotRunner { } class FakeHotRunnerFactory extends Fake implements HotRunnerFactory { - HotRunner hotRunner; - String dillOutputPath; - String projectRootPath; - List devices; + late HotRunner hotRunner; + String? dillOutputPath; + String? projectRootPath; + late List devices; @override HotRunner build( List devices, { - String target, - DebuggingOptions debuggingOptions, + required String target, + required DebuggingOptions debuggingOptions, bool benchmarkMode = false, - File applicationBinary, + File? applicationBinary, bool hostIsIde = false, - String projectRootPath, - String packagesFilePath, - String dillOutputPath, + String? projectRootPath, + String? packagesFilePath, + String? dillOutputPath, bool stayResident = true, bool ipv6 = false, - FlutterProject flutterProject, + FlutterProject? flutterProject, }) { this.devices = devices; this.dillOutputPath = dillOutputPath; @@ -514,17 +553,17 @@ class FakeHotRunnerFactory extends Fake implements HotRunnerFactory { class RecordingPortForwarder implements DevicePortForwarder { RecordingPortForwarder([this.hostPort]); - int devicePort; - int hostPort; + int? devicePort; + int? hostPort; @override Future dispose() async { } @override - Future forward(int devicePort, {int hostPort}) async { + Future forward(int devicePort, {int? hostPort}) async { this.devicePort = devicePort; this.hostPort ??= hostPort; - return this.hostPort; + return this.hostPort!; } @override @@ -541,12 +580,12 @@ class StreamLogger extends Logger { @override void printError( String message, { - StackTrace stackTrace, - bool emphasis, - TerminalColor color, - int indent, - int hangingIndent, - bool wrap, + StackTrace? stackTrace, + bool? emphasis, + TerminalColor? color, + int? indent, + int? hangingIndent, + bool? wrap, }) { hadErrorOutput = true; _log('[stderr] $message'); @@ -555,11 +594,11 @@ class StreamLogger extends Logger { @override void printWarning( String message, { - bool emphasis, - TerminalColor color, - int indent, - int hangingIndent, - bool wrap, + bool? emphasis, + TerminalColor? color, + int? indent, + int? hangingIndent, + bool? wrap, }) { hadWarningOutput = true; _log('[stderr] $message'); @@ -568,12 +607,12 @@ class StreamLogger extends Logger { @override void printStatus( String message, { - bool emphasis, - TerminalColor color, - bool newline, - int indent, - int hangingIndent, - bool wrap, + bool? emphasis, + TerminalColor? color, + bool? newline, + int? indent, + int? hangingIndent, + bool? wrap, }) { _log('[stdout] $message'); } @@ -581,7 +620,7 @@ class StreamLogger extends Logger { @override void printBox( String message, { - String title, + String? title, }) { if (title == null) { _log('[stdout] $message'); @@ -598,8 +637,8 @@ class StreamLogger extends Logger { @override Status startProgress( String message, { - @required Duration timeout, - String progressId, + Duration? timeout, + String? progressId, bool multilineOutput = false, bool includeTiming = true, int progressIndicatorPadding = kDefaultStatusPadding, @@ -612,9 +651,9 @@ class StreamLogger extends Logger { @override Status startSpinner({ - VoidCallback onFinish, - Duration timeout, - SlowWarningCallback slowWarningCallback, + VoidCallback? onFinish, + Duration? timeout, + SlowWarningCallback? slowWarningCallback, }) { return SilentStatus( stopwatch: Stopwatch(), @@ -641,7 +680,7 @@ class StreamLogger extends Logger { Stream get stream => _controller.stream; @override - void sendEvent(String name, [Map args]) { } + void sendEvent(String name, [Map? args]) { } @override bool get supportsColor => throw UnimplementedError(); @@ -676,10 +715,10 @@ class FakeDartDevelopmentService extends Fake implements DartDevelopmentService @override Future startDartDevelopmentService( Uri observatoryUri, { - @required Logger logger, - int hostPort, - bool ipv6, - bool disableServiceAuthCodes, + required Logger logger, + int? hostPort, + bool? ipv6, + bool? disableServiceAuthCodes, bool cacheStartupProfile = false, }) async {} @@ -691,10 +730,10 @@ class FakeDartDevelopmentService extends Fake implements DartDevelopmentService // Until we fix that, we have to also ignore related lints here. // ignore: avoid_implementing_value_types class FakeAndroidDevice extends Fake implements AndroidDevice { - FakeAndroidDevice({@required this.id}); + FakeAndroidDevice({required this.id}); @override - DartDevelopmentService dds; + late DartDevelopmentService dds; @override final String id; @@ -727,20 +766,25 @@ class FakeAndroidDevice extends Fake implements AndroidDevice { bool isSupportedForProject(FlutterProject flutterProject) => true; @override - DevicePortForwarder portForwarder; + DevicePortForwarder? portForwarder; - DeviceLogReader Function() onGetLogReader; + DeviceLogReader Function()? onGetLogReader; @override FutureOr getLogReader({ - AndroidApk app, + AndroidApk? app, bool includePastLogs = false, }) { - return onGetLogReader(); + if (onGetLogReader == null) { + throw UnimplementedError( + 'Called getLogReader but no onGetLogReader callback was supplied in the constructor to FakeAndroidDevice.', + ); + } + return onGetLogReader!(); } @override - OverrideArtifacts get artifactOverrides => null; + OverrideArtifacts? get artifactOverrides => null; @override final PlatformType platformType = PlatformType.android; @@ -753,24 +797,40 @@ class FakeAndroidDevice extends Fake implements AndroidDevice { // Until we fix that, we have to also ignore related lints here. // ignore: avoid_implementing_value_types class FakeIOSDevice extends Fake implements IOSDevice { - FakeIOSDevice({this.dds, this.portForwarder, this.logReader}); + FakeIOSDevice({ + DevicePortForwarder? portForwarder, + DeviceLogReader? logReader, + this.onGetLogReader, + }) : _portForwarder = portForwarder, _logReader = logReader; + + final DevicePortForwarder? _portForwarder; @override - final DevicePortForwarder portForwarder; + DevicePortForwarder get portForwarder => _portForwarder!; @override - final DartDevelopmentService dds; + DartDevelopmentService get dds => throw UnimplementedError('getter dds not implemented'); - final DeviceLogReader logReader; + final DeviceLogReader? _logReader; + DeviceLogReader get logReader => _logReader!; + + final DeviceLogReader Function()? onGetLogReader; @override DeviceLogReader getLogReader({ - IOSApp app, + IOSApp? app, bool includePastLogs = false, - }) => logReader; + }) { + if (onGetLogReader == null) { + throw UnimplementedError( + 'Called getLogReader but no onGetLogReader callback was supplied in the constructor to FakeIOSDevice', + ); + } + return onGetLogReader!(); + } @override - OverrideArtifacts get artifactOverrides => null; + OverrideArtifacts? get artifactOverrides => null; @override final String name = 'name'; @@ -781,3 +841,49 @@ class FakeIOSDevice extends Fake implements IOSDevice { @override final PlatformType platformType = PlatformType.ios; } + +class FakeMDnsClient extends Fake implements MDnsClient { + FakeMDnsClient(this.ptrRecords, this.srvResponse, { + this.txtResponse = const >{}, + this.osErrorOnStart = false, + }); + + final List ptrRecords; + final Map> srvResponse; + final Map> txtResponse; + final bool osErrorOnStart; + + @override + Future start({ + InternetAddress? listenAddress, + NetworkInterfacesFactory? interfacesFactory, + int mDnsPort = 5353, + InternetAddress? mDnsAddress, + }) async { + if (osErrorOnStart) { + throw const OSError('Operation not supported on socket', 102); + } + } + + @override + Stream lookup( + ResourceRecordQuery query, { + Duration timeout = const Duration(seconds: 5), + }) { + if (T == PtrResourceRecord && query.fullyQualifiedName == MDnsObservatoryDiscovery.dartObservatoryName) { + return Stream.fromIterable(ptrRecords) as Stream; + } + if (T == SrvResourceRecord) { + final String key = query.fullyQualifiedName; + return Stream.fromIterable(srvResponse[key] ?? []) as Stream; + } + if (T == TxtResourceRecord) { + final String key = query.fullyQualifiedName; + return Stream.fromIterable(txtResponse[key] ?? []) as Stream; + } + throw UnsupportedError('Unsupported query type $T'); + } + + @override + void stop() {} +}