diff --git a/packages/flutter_tools/lib/src/ios/devices.dart b/packages/flutter_tools/lib/src/ios/devices.dart index 979b80377c..950a6afe35 100644 --- a/packages/flutter_tools/lib/src/ios/devices.dart +++ b/packages/flutter_tools/lib/src/ios/devices.dart @@ -7,6 +7,7 @@ import 'dart:math' as math; import 'package:meta/meta.dart'; import 'package:platform/platform.dart'; +import 'package:process/process.dart'; import '../application_package.dart'; import '../artifacts.dart'; @@ -365,7 +366,13 @@ class IOSDevice extends Device { } @override - DevicePortForwarder get portForwarder => _portForwarder ??= IOSDevicePortForwarder(this); + DevicePortForwarder get portForwarder => _portForwarder ??= IOSDevicePortForwarder( + processManager: globals.processManager, + logger: globals.logger, + dyLdLibEntry: globals.cache.dyLdLibEntry, + id: id, + iproxyPath: _iproxyPath, + ); @visibleForTesting set portForwarder(DevicePortForwarder forwarder) { @@ -583,20 +590,56 @@ class IOSDeviceLogReader extends DeviceLogReader { } } -@visibleForTesting +/// A [DevicePortForwarder] specialized for iOS usage with iproxy. class IOSDevicePortForwarder extends DevicePortForwarder { - IOSDevicePortForwarder(this.device) : _forwardedPorts = []; - final IOSDevice device; + /// Create a new [IOSDevicePortForwarder]. + IOSDevicePortForwarder({ + @required ProcessManager processManager, + @required Logger logger, + @required MapEntry dyLdLibEntry, + @required String id, + @required String iproxyPath, + }) : _logger = logger, + _dyLdLibEntry = dyLdLibEntry, + _id = id, + _iproxyPath = iproxyPath, + _processUtils = ProcessUtils(processManager: processManager, logger: logger); - final List _forwardedPorts; + /// Create a [IOSDevicePortForwarder] for testing. + /// + /// This specifies the path to iproxy as 'iproxy` and the dyLdLibEntry as + /// 'DYLD_LIBRARY_PATH: /path/to/libs'. + /// + /// The device id may be provided, but otherwise defaultts to '1234'. + factory IOSDevicePortForwarder.test({ + @required ProcessManager processManager, + @required Logger logger, + String id, + }) { + return IOSDevicePortForwarder( + processManager: processManager, + logger: logger, + iproxyPath: 'iproxy', + id: id ?? '1234', + dyLdLibEntry: const MapEntry( + 'DYLD_LIBRARY_PATH', '/path/to/libs', + ), + ); + } + + final ProcessUtils _processUtils; + final Logger _logger; + final MapEntry _dyLdLibEntry; + final String _id; + final String _iproxyPath; @override - List get forwardedPorts => _forwardedPorts; + List forwardedPorts = []; @visibleForTesting - void addForwardedPorts(List forwardedPorts) { - forwardedPorts.forEach(_forwardedPorts.add); + void addForwardedPorts(List ports) { + ports.forEach(forwardedPorts.add); } static const Duration _kiProxyPortForwardTimeout = Duration(seconds: 1); @@ -612,17 +655,17 @@ class IOSDevicePortForwarder extends DevicePortForwarder { bool connected = false; while (!connected) { - globals.printTrace('Attempting to forward device port $devicePort to host port $hostPort'); + _logger.printTrace('Attempting to forward device port $devicePort to host port $hostPort'); // Usage: iproxy LOCAL_TCP_PORT DEVICE_TCP_PORT UDID - process = await processUtils.start( + process = await _processUtils.start( [ - device._iproxyPath, + _iproxyPath, hostPort.toString(), devicePort.toString(), - device.id, + _id, ], environment: Map.fromEntries( - >[globals.cache.dyLdLibEntry], + >[_dyLdLibEntry], ), ); // TODO(ianh): This is a flakey race condition, https://github.com/libimobiledevice/libimobiledevice/issues/674 @@ -645,25 +688,25 @@ class IOSDevicePortForwarder extends DevicePortForwarder { final ForwardedPort forwardedPort = ForwardedPort.withContext( hostPort, devicePort, process, ); - globals.printTrace('Forwarded port $forwardedPort'); - _forwardedPorts.add(forwardedPort); + _logger.printTrace('Forwarded port $forwardedPort'); + forwardedPorts.add(forwardedPort); return hostPort; } @override Future unforward(ForwardedPort forwardedPort) async { - if (!_forwardedPorts.remove(forwardedPort)) { + if (!forwardedPorts.remove(forwardedPort)) { // Not in list. Nothing to remove. return; } - globals.printTrace('Unforwarding port $forwardedPort'); + _logger.printTrace('Unforwarding port $forwardedPort'); forwardedPort.dispose(); } @override Future dispose() async { - for (final ForwardedPort forwardedPort in _forwardedPorts) { + for (final ForwardedPort forwardedPort in forwardedPorts) { forwardedPort.dispose(); } } diff --git a/packages/flutter_tools/test/general.shard/ios/devices_test.dart b/packages/flutter_tools/test/general.shard/ios/devices_test.dart index 310b224d25..f6fdfb4b7e 100644 --- a/packages/flutter_tools/test/general.shard/ios/devices_test.dart +++ b/packages/flutter_tools/test/general.shard/ios/devices_test.dart @@ -248,7 +248,13 @@ void main() { IOSDevicePortForwarder createPortForwarder( ForwardedPort forwardedPort, IOSDevice device) { - final IOSDevicePortForwarder portForwarder = IOSDevicePortForwarder(device); + final IOSDevicePortForwarder portForwarder = IOSDevicePortForwarder( + dyLdLibEntry: mockCache.dyLdLibEntry, + id: device.id, + iproxyPath: mockArtifacts.getArtifactPath(Artifact.iproxy, platform: TargetPlatform.ios), + logger: logger, + processManager: FakeProcessManager.any(), + ); portForwarder.addForwardedPorts([forwardedPort]); return portForwarder; } @@ -482,54 +488,6 @@ void main() { Usage: () => mockUsage, }); - // By default, the .forward() method will try every port between 1024 - // and 65535; this test verifies we are killing iproxy processes when - // we timeout on a port - testUsingContext('.forward() will kill iproxy processes before invoking a second', () async { - const String deviceId = '123'; - const int devicePort = 456; - final IOSDevice device = IOSDevice( - deviceId, - artifacts: mockArtifacts, - fileSystem: mockFileSystem, - platform: macPlatform, - iosDeploy: iosDeploy, - name: 'iPhone 1', - sdkVersion: '13.3', - cpuArchitecture: DarwinArch.arm64, - ); - final IOSDevicePortForwarder portForwarder = IOSDevicePortForwarder(device); - bool firstRun = true; - final MockProcess successProcess = MockProcess( - exitCode: Future.value(0), - stdout: Stream>.fromIterable(>['Hello'.codeUnits]), - ); - final MockProcess failProcess = MockProcess( - exitCode: Future.value(1), - stdout: const Stream>.empty(), - ); - - final ProcessFactory factory = (List command) { - if (!firstRun) { - return successProcess; - } - firstRun = false; - return failProcess; - }; - mockProcessManager.processFactory = factory; - final int hostPort = await portForwarder.forward(devicePort); - // First port tried (1024) should fail, then succeed on the next - expect(hostPort, 1024 + 1); - verifyNever(successProcess.kill()); - verify(failProcess.kill()); - }, overrides: { - Artifacts: () => mockArtifacts, - Cache: () => mockCache, - Platform: () => macPlatform, - ProcessManager: () => mockProcessManager, - Usage: () => mockUsage, - }); - testUsingContext('succeeds in debug mode when mDNS fails by falling back to manual protocol discovery', () async { final IOSDevice device = IOSDevice( '123', diff --git a/packages/flutter_tools/test/general.shard/ios/ios_device_port_forwarder_test.dart b/packages/flutter_tools/test/general.shard/ios/ios_device_port_forwarder_test.dart new file mode 100644 index 0000000000..6714f7fba2 --- /dev/null +++ b/packages/flutter_tools/test/general.shard/ios/ios_device_port_forwarder_test.dart @@ -0,0 +1,46 @@ +// Copyright 2014 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:flutter_tools/src/base/logger.dart'; +import 'package:flutter_tools/src/ios/devices.dart'; + +import '../../src/common.dart'; +import '../../src/context.dart'; + +const Map kDyLdLibEntry = { + 'DYLD_LIBRARY_PATH': '/path/to/libs', +}; + +void main() { + // By default, the .forward() method will try every port between 1024 + // and 65535; this test verifies we are killing iproxy processes when + // we timeout on a port + testWithoutContext('IOSDevicePortForwarder.forward will kill iproxy processes before invoking a second', () async { + const int devicePort = 456; + final FakeProcessManager processManager = FakeProcessManager.list([ + const FakeCommand( + command: ['iproxy', '1024', '456', '1234'], + // iproxy does not exit with 0 when it cannot forward. + exitCode: 0, + stdout: null, // no stdout indicates failure. + environment: kDyLdLibEntry, + ), + const FakeCommand( + command: ['iproxy', '1025', '456', '1234'], + exitCode: 0, + stdout: 'not empty', + environment: kDyLdLibEntry, + ), + ]); + final IOSDevicePortForwarder portForwarder = IOSDevicePortForwarder.test( + processManager: processManager, + logger: BufferLogger.test(), + ); + final int hostPort = await portForwarder.forward(devicePort); + + // First port tried (1024) should fail, then succeed on the next + expect(hostPort, 1024 + 1); + expect(processManager.hasRemainingExpectations, false); + }); +} diff --git a/packages/flutter_tools/test/src/fake_process_manager.dart b/packages/flutter_tools/test/src/fake_process_manager.dart index 4b11c9af59..a0cebc440c 100644 --- a/packages/flutter_tools/test/src/fake_process_manager.dart +++ b/packages/flutter_tools/test/src/fake_process_manager.dart @@ -30,9 +30,7 @@ class FakeCommand { this.completer, }) : assert(command != null), assert(duration != null), - assert(exitCode != null), - assert(stdout != null), - assert(stderr != null); + assert(exitCode != null); /// The exact commands that must be matched for this [FakeCommand] to be /// considered correct. @@ -139,8 +137,12 @@ class _FakeProcess implements Process { } return _exitCode; }), - stderr = Stream>.value(utf8.encode(_stderr)), - stdout = Stream>.value(utf8.encode(_stdout)); + stderr = _stderr == null + ? const Stream>.empty() + : Stream>.value(utf8.encode(_stderr)), + stdout = _stdout == null + ? const Stream>.empty() + : Stream>.value(utf8.encode(_stdout)); final int _exitCode;