From 757b39ba9945c26972f2ddbfeda14e59ce0133bb Mon Sep 17 00:00:00 2001 From: Jenn Magder Date: Tue, 17 Mar 2020 16:02:45 -0700 Subject: [PATCH] Ignore fuchsia device-finder no device error (#52761) --- .../lib/src/fuchsia/fuchsia_dev_finder.dart | 43 ++++-- .../lib/src/fuchsia/fuchsia_device.dart | 12 +- .../lib/src/fuchsia/fuchsia_pm.dart | 8 +- .../lib/src/fuchsia/fuchsia_sdk.dart | 19 +-- .../lib/src/fuchsia/fuchsia_workflow.dart | 5 +- packages/flutter_tools/lib/src/globals.dart | 2 + .../fuchsia/fuchsia_dev_finder_test.dart | 135 ++++++++++++++++++ 7 files changed, 191 insertions(+), 33 deletions(-) create mode 100644 packages/flutter_tools/test/general.shard/fuchsia/fuchsia_dev_finder_test.dart diff --git a/packages/flutter_tools/lib/src/fuchsia/fuchsia_dev_finder.dart b/packages/flutter_tools/lib/src/fuchsia/fuchsia_dev_finder.dart index c439e3c0407..c6570ae18a9 100644 --- a/packages/flutter_tools/lib/src/fuchsia/fuchsia_dev_finder.dart +++ b/packages/flutter_tools/lib/src/fuchsia/fuchsia_dev_finder.dart @@ -2,9 +2,12 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:meta/meta.dart'; +import 'package:process/process.dart'; + import '../base/common.dart'; +import '../base/logger.dart'; import '../base/process.dart'; -import '../globals.dart' as globals; import 'fuchsia_sdk.dart'; // Usage: device-finder @@ -19,24 +22,42 @@ import 'fuchsia_sdk.dart'; /// A simple wrapper for the Fuchsia SDK's 'device-finder' tool. class FuchsiaDevFinder { + FuchsiaDevFinder({ + @required FuchsiaArtifacts fuchsiaArtifacts, + @required Logger logger, + @required ProcessManager processManager, + }) + : _fuchsiaArtifacts = fuchsiaArtifacts, + _logger = logger, + _processUtils = ProcessUtils(logger: logger, processManager: processManager); + + + final FuchsiaArtifacts _fuchsiaArtifacts; + final Logger _logger; + final ProcessUtils _processUtils; + /// Returns a list of attached devices as a list of strings with entries /// formatted as follows: /// 192.168.42.172 scare-cable-skip-joy Future> list({ Duration timeout }) async { - if (fuchsiaArtifacts.devFinder == null || - !fuchsiaArtifacts.devFinder.existsSync()) { + if (_fuchsiaArtifacts.devFinder == null || + !_fuchsiaArtifacts.devFinder.existsSync()) { throwToolExit('Fuchsia device-finder tool not found.'); } final List command = [ - fuchsiaArtifacts.devFinder.path, + _fuchsiaArtifacts.devFinder.path, 'list', '-full', if (timeout != null) ...['-timeout', '${timeout.inMilliseconds}ms'] ]; - final RunResult result = await processUtils.run(command); + final RunResult result = await _processUtils.run(command); if (result.exitCode != 0) { - globals.printError('device-finder failed: ${result.stderr}'); + // No devices returns error code 1. + // https://bugs.fuchsia.dev/p/fuchsia/issues/detail?id=48563 + if (!result.stderr.contains('no devices found')) { + _logger.printError('device-finder failed: ${result.stderr}'); + } return null; } return result.stdout.split('\n'); @@ -50,20 +71,20 @@ class FuchsiaDevFinder { /// The string [deviceName] should be the name of the device from the /// 'list' command, e.g. 'scare-cable-skip-joy'. Future resolve(String deviceName, {bool local = false}) async { - if (fuchsiaArtifacts.devFinder == null || - !fuchsiaArtifacts.devFinder.existsSync()) { + if (_fuchsiaArtifacts.devFinder == null || + !_fuchsiaArtifacts.devFinder.existsSync()) { throwToolExit('Fuchsia device-finder tool not found.'); } final List command = [ - fuchsiaArtifacts.devFinder.path, + _fuchsiaArtifacts.devFinder.path, 'resolve', if (local) '-local', '-device-limit', '1', deviceName, ]; - final RunResult result = await processUtils.run(command); + final RunResult result = await _processUtils.run(command); if (result.exitCode != 0) { - globals.printError('device-finder failed: ${result.stderr}'); + _logger.printError('device-finder failed: ${result.stderr}'); return null; } return result.stdout.trim(); diff --git a/packages/flutter_tools/lib/src/fuchsia/fuchsia_device.dart b/packages/flutter_tools/lib/src/fuchsia/fuchsia_device.dart index b0f0a464d41..d35d59a218a 100644 --- a/packages/flutter_tools/lib/src/fuchsia/fuchsia_device.dart +++ b/packages/flutter_tools/lib/src/fuchsia/fuchsia_device.dart @@ -558,14 +558,14 @@ class FuchsiaDevice extends Device { /// Run `command` on the Fuchsia device shell. Future shell(String command) async { - if (fuchsiaArtifacts.sshConfig == null) { + if (globals.fuchsiaArtifacts.sshConfig == null) { throwToolExit('Cannot interact with device. No ssh config.\n' 'Try setting FUCHSIA_SSH_CONFIG or FUCHSIA_BUILD_DIR.'); } return await processUtils.run([ 'ssh', '-F', - fuchsiaArtifacts.sshConfig.absolute.path, + globals.fuchsiaArtifacts.sshConfig.absolute.path, id, // Device's IP address. command, ]); @@ -573,14 +573,14 @@ class FuchsiaDevice extends Device { /// Transfer the file [origin] from the device to [destination]. Future scp(String origin, String destination) async { - if (fuchsiaArtifacts.sshConfig == null) { + if (globals.fuchsiaArtifacts.sshConfig == null) { throwToolExit('Cannot interact with device. No ssh config.\n' 'Try setting FUCHSIA_SSH_CONFIG or FUCHSIA_BUILD_DIR.'); } return await processUtils.run([ 'scp', '-F', - fuchsiaArtifacts.sshConfig.absolute.path, + globals.fuchsiaArtifacts.sshConfig.absolute.path, '$id:$origin', destination, ]); @@ -742,7 +742,7 @@ class _FuchsiaPortForwarder extends DevicePortForwarder { 'ssh', '-6', '-F', - fuchsiaArtifacts.sshConfig.absolute.path, + globals.fuchsiaArtifacts.sshConfig.absolute.path, '-nNT', '-vvv', '-f', @@ -774,7 +774,7 @@ class _FuchsiaPortForwarder extends DevicePortForwarder { final List command = [ 'ssh', '-F', - fuchsiaArtifacts.sshConfig.absolute.path, + globals.fuchsiaArtifacts.sshConfig.absolute.path, '-O', 'cancel', '-vvv', diff --git a/packages/flutter_tools/lib/src/fuchsia/fuchsia_pm.dart b/packages/flutter_tools/lib/src/fuchsia/fuchsia_pm.dart index 7dcb1818837..53875dd6469 100644 --- a/packages/flutter_tools/lib/src/fuchsia/fuchsia_pm.dart +++ b/packages/flutter_tools/lib/src/fuchsia/fuchsia_pm.dart @@ -107,14 +107,14 @@ class FuchsiaPM { /// [FuchsiaDevFinder.resolve], and [port] should be an unused port for the /// http server to bind. Future serve(String repoPath, String host, int port) async { - if (fuchsiaArtifacts.pm == null) { + if (globals.fuchsiaArtifacts.pm == null) { throwToolExit('Fuchsia pm tool not found'); } if (isIPv6Address(host.split('%').first)) { host = '[${host.replaceAll('%', '%25')}]'; } final List command = [ - fuchsiaArtifacts.pm.path, + globals.fuchsiaArtifacts.pm.path, 'serve', '-repo', repoPath, @@ -151,10 +151,10 @@ class FuchsiaPM { } Future _runPMCommand(List args) async { - if (fuchsiaArtifacts.pm == null) { + if (globals.fuchsiaArtifacts.pm == null) { throwToolExit('Fuchsia pm tool not found'); } - final List command = [fuchsiaArtifacts.pm.path, ...args]; + final List command = [globals.fuchsiaArtifacts.pm.path, ...args]; final RunResult result = await processUtils.run(command); return result.exitCode == 0; } diff --git a/packages/flutter_tools/lib/src/fuchsia/fuchsia_sdk.dart b/packages/flutter_tools/lib/src/fuchsia/fuchsia_sdk.dart index 34bb45d0c4c..1e58567c146 100644 --- a/packages/flutter_tools/lib/src/fuchsia/fuchsia_sdk.dart +++ b/packages/flutter_tools/lib/src/fuchsia/fuchsia_sdk.dart @@ -17,9 +17,6 @@ import 'fuchsia_pm.dart'; /// The [FuchsiaSdk] instance. FuchsiaSdk get fuchsiaSdk => context.get(); -/// The [FuchsiaArtifacts] instance. -FuchsiaArtifacts get fuchsiaArtifacts => context.get(); - /// Returns [true] if the current platform supports Fuchsia targets. bool isFuchsiaSupportedPlatform() { return globals.platform.isLinux || globals.platform.isMacOS; @@ -37,7 +34,11 @@ class FuchsiaSdk { /// Interface to the 'device-finder' tool. FuchsiaDevFinder _fuchsiaDevFinder; FuchsiaDevFinder get fuchsiaDevFinder => - _fuchsiaDevFinder ??= FuchsiaDevFinder(); + _fuchsiaDevFinder ??= FuchsiaDevFinder( + fuchsiaArtifacts: globals.fuchsiaArtifacts, + logger: globals.logger, + processManager: globals.processManager + ); /// Interface to the 'kernel_compiler' tool. FuchsiaKernelCompiler _fuchsiaKernelCompiler; @@ -48,8 +49,8 @@ class FuchsiaSdk { /// $ device-finder list -full /// > 192.168.42.56 paper-pulp-bush-angel Future listDevices({ Duration timeout }) async { - if (fuchsiaArtifacts.devFinder == null || - !fuchsiaArtifacts.devFinder.existsSync()) { + if (globals.fuchsiaArtifacts.devFinder == null || + !globals.fuchsiaArtifacts.devFinder.existsSync()) { return null; } final List devices = await fuchsiaDevFinder.list(timeout: timeout); @@ -67,8 +68,8 @@ class FuchsiaSdk { final StreamController controller = StreamController(onCancel: () { process.kill(); }); - if (fuchsiaArtifacts.sshConfig == null || - !fuchsiaArtifacts.sshConfig.existsSync()) { + if (globals.fuchsiaArtifacts.sshConfig == null || + !globals.fuchsiaArtifacts.sshConfig.existsSync()) { globals.printError('Cannot read device logs: No ssh config.'); globals.printError('Have you set FUCHSIA_SSH_CONFIG or FUCHSIA_BUILD_DIR?'); return null; @@ -77,7 +78,7 @@ class FuchsiaSdk { final List cmd = [ 'ssh', '-F', - fuchsiaArtifacts.sshConfig.absolute.path, + globals.fuchsiaArtifacts.sshConfig.absolute.path, id, // The device's IP. remoteCommand, ]; diff --git a/packages/flutter_tools/lib/src/fuchsia/fuchsia_workflow.dart b/packages/flutter_tools/lib/src/fuchsia/fuchsia_workflow.dart index 61edab91181..ed454819c8e 100644 --- a/packages/flutter_tools/lib/src/fuchsia/fuchsia_workflow.dart +++ b/packages/flutter_tools/lib/src/fuchsia/fuchsia_workflow.dart @@ -5,7 +5,6 @@ import '../base/context.dart'; import '../doctor.dart'; import '../globals.dart' as globals; -import 'fuchsia_sdk.dart'; /// The [FuchsiaWorkflow] instance. FuchsiaWorkflow get fuchsiaWorkflow => context.get(); @@ -21,12 +20,12 @@ class FuchsiaWorkflow implements Workflow { @override bool get canListDevices { - return fuchsiaArtifacts.devFinder != null; + return globals.fuchsiaArtifacts.devFinder != null; } @override bool get canLaunchDevices { - return fuchsiaArtifacts.devFinder != null && fuchsiaArtifacts.sshConfig != null; + return globals.fuchsiaArtifacts.devFinder != null && globals.fuchsiaArtifacts.sshConfig != null; } @override diff --git a/packages/flutter_tools/lib/src/globals.dart b/packages/flutter_tools/lib/src/globals.dart index b4f30d8c5e0..8ab4be0ff7a 100644 --- a/packages/flutter_tools/lib/src/globals.dart +++ b/packages/flutter_tools/lib/src/globals.dart @@ -21,6 +21,7 @@ import 'base/template.dart'; import 'base/terminal.dart'; import 'base/user_messages.dart'; import 'cache.dart'; +import 'fuchsia/fuchsia_sdk.dart'; import 'ios/ios_deploy.dart'; import 'ios/ios_workflow.dart'; import 'ios/mac.dart'; @@ -69,6 +70,7 @@ Platform get platform => context.get() ?? _kLocalPlatform; AndroidStudio get androidStudio => context.get(); AndroidSdk get androidSdk => context.get(); FlutterVersion get flutterVersion => context.get(); +FuchsiaArtifacts get fuchsiaArtifacts => context.get(); IMobileDevice get iMobileDevice => context.get(); IOSDeploy get iosDeploy => context.get(); IOSSimulatorUtils get iosSimulatorUtils => context.get(); diff --git a/packages/flutter_tools/test/general.shard/fuchsia/fuchsia_dev_finder_test.dart b/packages/flutter_tools/test/general.shard/fuchsia/fuchsia_dev_finder_test.dart new file mode 100644 index 00000000000..9ee52a7aa94 --- /dev/null +++ b/packages/flutter_tools/test/general.shard/fuchsia/fuchsia_dev_finder_test.dart @@ -0,0 +1,135 @@ +// 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:file/file.dart'; +import 'package:file/memory.dart'; +import 'package:flutter_tools/src/base/file_system.dart'; +import 'package:flutter_tools/src/base/logger.dart'; +import 'package:flutter_tools/src/fuchsia/fuchsia_dev_finder.dart'; +import 'package:flutter_tools/src/fuchsia/fuchsia_sdk.dart'; +import 'package:mockito/mockito.dart'; +import 'package:process/process.dart'; + +import '../../src/common.dart'; +import '../../src/context.dart'; + +void main() { + MockFuchsiaArtifacts mockFuchsiaArtifacts; + BufferLogger logger; + MemoryFileSystem memoryFileSystem; + File deviceFinder; + + setUp(() { + mockFuchsiaArtifacts = MockFuchsiaArtifacts(); + memoryFileSystem = MemoryFileSystem.test(); + logger = BufferLogger.test(); + deviceFinder = memoryFileSystem.file('device-finder'); + + when(mockFuchsiaArtifacts.devFinder).thenReturn(deviceFinder); + }); + + group('device-finder list', () { + testWithoutContext('device-finder not found', () { + final FuchsiaDevFinder fuchsiaDevFinder = FuchsiaDevFinder( + fuchsiaArtifacts: mockFuchsiaArtifacts, + logger: logger, + processManager: FakeProcessManager.any(), + ); + + expect(() async => await fuchsiaDevFinder.list(), + throwsToolExit(message: 'Fuchsia device-finder tool not found.')); + }); + + testWithoutContext('no devices', () async { + deviceFinder.createSync(); + + final ProcessManager processManager = FakeProcessManager.list([ + FakeCommand( + command: [ deviceFinder.path, 'list', '-full' ], + exitCode: 1, + stderr: 'list.go:72: no devices found', + ), + ]); + + final FuchsiaDevFinder fuchsiaDevFinder = FuchsiaDevFinder( + fuchsiaArtifacts: mockFuchsiaArtifacts, + logger: logger, + processManager: processManager, + ); + + expect(await fuchsiaDevFinder.list(), isNull); + expect(logger.errorText, isEmpty); + }); + + testWithoutContext('error', () async { + deviceFinder.createSync(); + + final ProcessManager processManager = FakeProcessManager.list([ + FakeCommand( + command: [ deviceFinder.path, 'list', '-full' ], + exitCode: 1, + stderr: 'unexpected error', + ), + ]); + + final FuchsiaDevFinder fuchsiaDevFinder = FuchsiaDevFinder( + fuchsiaArtifacts: mockFuchsiaArtifacts, + logger: logger, + processManager: processManager, + ); + + expect(await fuchsiaDevFinder.list(), isNull); + expect(logger.errorText, contains('unexpected error')); + }); + + testWithoutContext('devices found', () async { + deviceFinder.createSync(); + + final ProcessManager processManager = FakeProcessManager.list([ + FakeCommand( + command: [ deviceFinder.path, 'list', '-full' ], + exitCode: 0, + stdout: 'device1\ndevice2', + ), + ]); + + final FuchsiaDevFinder fuchsiaDevFinder = FuchsiaDevFinder( + fuchsiaArtifacts: mockFuchsiaArtifacts, + logger: logger, + processManager: processManager, + ); + + expect(await fuchsiaDevFinder.list(), ['device1', 'device2']); + expect(logger.errorText, isEmpty); + }); + + testWithoutContext('timeout', () async { + deviceFinder.createSync(); + + final ProcessManager processManager = FakeProcessManager.list([ + FakeCommand( + command: [ + deviceFinder.path, + 'list', + '-full', + '-timeout', + '2000ms', + ], + exitCode: 0, + stdout: 'device1', + ), + ]); + + final FuchsiaDevFinder fuchsiaDevFinder = FuchsiaDevFinder( + fuchsiaArtifacts: mockFuchsiaArtifacts, + logger: logger, + processManager: processManager, + ); + + expect(await fuchsiaDevFinder.list(timeout: const Duration(seconds: 2)), ['device1']); + }); + }); +} + +class MockFuchsiaArtifacts extends Mock implements FuchsiaArtifacts {}