From d992d6de2031a243c79c76475df40b4b34c4d8b0 Mon Sep 17 00:00:00 2001 From: stuartmorgan Date: Wed, 9 Oct 2019 16:28:10 -0700 Subject: [PATCH] Make desktop stopApp only apply to processes Flutter started (#41519) --- packages/flutter_tools/lib/src/desktop.dart | 74 ------- .../flutter_tools/lib/src/desktop_device.dart | 172 +++++++++++++++ .../lib/src/linux/linux_device.dart | 113 ++-------- .../lib/src/macos/macos_device.dart | 142 +++---------- .../lib/src/windows/windows_device.dart | 117 ++--------- .../general.shard/desktop_device_test.dart | 198 ++++++++++++++++++ .../linux/linux_device_test.dart | 60 ++---- .../macos/macos_device_test.dart | 106 ++-------- .../windows/windows_device_test.dart | 60 ++---- 9 files changed, 490 insertions(+), 552 deletions(-) delete mode 100644 packages/flutter_tools/lib/src/desktop.dart create mode 100644 packages/flutter_tools/lib/src/desktop_device.dart create mode 100644 packages/flutter_tools/test/general.shard/desktop_device_test.dart diff --git a/packages/flutter_tools/lib/src/desktop.dart b/packages/flutter_tools/lib/src/desktop.dart deleted file mode 100644 index 69a9bf864b..0000000000 --- a/packages/flutter_tools/lib/src/desktop.dart +++ /dev/null @@ -1,74 +0,0 @@ -// Copyright 2019 The Chromium 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 'dart:async'; - -import 'base/io.dart'; -import 'base/process_manager.dart'; -import 'convert.dart'; -import 'device.dart'; - -/// Kills a process on linux or macOS. -Future killProcess(String executable) async { - if (executable == null) { - return false; - } - final RegExp whitespace = RegExp(r'\s+'); - bool succeeded = true; - try { - final ProcessResult result = await processManager.run([ - 'ps', 'aux', - ]); - if (result.exitCode != 0) { - return false; - } - final List lines = result.stdout.split('\n'); - for (String line in lines) { - if (!line.contains(executable)) { - continue; - } - final List values = line.split(whitespace); - if (values.length < 2) { - continue; - } - final String processPid = values[1]; - final String currentRunningProcessPid = pid.toString(); - // Don't kill the flutter tool process - if (processPid == currentRunningProcessPid) { - continue; - } - - final ProcessResult killResult = await processManager.run([ - 'kill', processPid, - ]); - succeeded &= killResult.exitCode == 0; - } - return true; - } on ArgumentError { - succeeded = false; - } - return succeeded; -} - -class DesktopLogReader extends DeviceLogReader { - final StreamController> _inputController = StreamController>.broadcast(); - - void initializeProcess(Process process) { - process.stdout.listen(_inputController.add); - process.stderr.listen(_inputController.add); - process.exitCode.then((int result) { - _inputController.close(); - }); - } - - @override - Stream get logLines { - return _inputController.stream - .transform(utf8.decoder) - .transform(const LineSplitter()); - } - - @override - String get name => 'desktop'; -} diff --git a/packages/flutter_tools/lib/src/desktop_device.dart b/packages/flutter_tools/lib/src/desktop_device.dart new file mode 100644 index 0000000000..341aa1d01c --- /dev/null +++ b/packages/flutter_tools/lib/src/desktop_device.dart @@ -0,0 +1,172 @@ +// Copyright 2019 The Chromium 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 'dart:async'; + +import 'package:meta/meta.dart'; + +import 'application_package.dart'; +import 'base/common.dart'; +import 'base/io.dart'; +import 'base/os.dart'; +import 'base/process_manager.dart'; +import 'build_info.dart'; +import 'cache.dart'; +import 'convert.dart'; +import 'device.dart'; +import 'globals.dart'; +import 'protocol_discovery.dart'; + +/// A partial implementation of Device for desktop-class devices to inherit +/// from, containing implementations that are common to all desktop devices. +abstract class DesktopDevice extends Device { + DesktopDevice(String identifier, {@required PlatformType platformType, @required bool ephemeral}) : super( + identifier, + category: Category.desktop, + platformType: platformType, + ephemeral: ephemeral, + ); + + final Set _runningProcesses = {}; + + final DesktopLogReader _deviceLogReader = DesktopLogReader(); + + // Since the host and target devices are the same, no work needs to be done + // to install the application. + @override + Future isAppInstalled(ApplicationPackage app) async => true; + + // Since the host and target devices are the same, no work needs to be done + // to install the application. + @override + Future isLatestBuildInstalled(ApplicationPackage app) async => true; + + // Since the host and target devices are the same, no work needs to be done + // to install the application. + @override + Future installApp(ApplicationPackage app) async => true; + + // Since the host and target devices are the same, no work needs to be done + // to uninstall the application. + @override + Future uninstallApp(ApplicationPackage app) async => true; + + @override + Future get isLocalEmulator async => false; + + @override + Future get emulatorId async => null; + + @override + DevicePortForwarder get portForwarder => const NoOpDevicePortForwarder(); + + @override + Future get sdkNameAndVersion async => os.name; + + @override + DeviceLogReader getLogReader({ ApplicationPackage app }) { + return _deviceLogReader; + } + + @override + void clearLogs() {} + + @override + Future startApp( + ApplicationPackage package, { + String mainPath, + String route, + DebuggingOptions debuggingOptions, + Map platformArgs, + bool prebuiltApplication = false, + bool ipv6 = false, + }) async { + if (!prebuiltApplication) { + Cache.releaseLockEarly(); + await buildForDevice( + package, + buildInfo: debuggingOptions?.buildInfo, + mainPath: mainPath, + ); + } + + // Ensure that the executable is locatable. + final BuildMode buildMode = debuggingOptions?.buildInfo?.mode; + final String executable = executablePathForDevice(package, buildMode); + if (executable == null) { + printError('Unable to find executable to run'); + return LaunchResult.failed(); + } + + final Process process = await processManager.start([ + executable, + ]); + _runningProcesses.add(process); + unawaited(process.exitCode.then((_) => _runningProcesses.remove(process))); + + if (debuggingOptions?.buildInfo?.isRelease == true) { + return LaunchResult.succeeded(); + } + _deviceLogReader.initializeProcess(process); + final ProtocolDiscovery observatoryDiscovery = ProtocolDiscovery.observatory(_deviceLogReader); + try { + final Uri observatoryUri = await observatoryDiscovery.uri; + onAttached(package, buildMode, process); + return LaunchResult.succeeded(observatoryUri: observatoryUri); + } catch (error) { + printError('Error waiting for a debug connection: $error'); + return LaunchResult.failed(); + } finally { + await observatoryDiscovery.cancel(); + } + } + + @override + Future stopApp(ApplicationPackage app) async { + bool succeeded = true; + // Walk a copy of _runningProcesses, since the exit handler removes from the + // set. + for (Process process in Set.from(_runningProcesses)) { + succeeded &= process.kill(); + } + return succeeded; + } + + /// Builds the current project for this device, with the given options. + Future buildForDevice( + ApplicationPackage package, { + String mainPath, + BuildInfo buildInfo, + }); + + /// Returns the path to the executable to run for [package] on this device for + /// the given [buildMode]. + String executablePathForDevice(ApplicationPackage package, BuildMode buildMode); + + /// Called after a process is attached, allowing any device-specific extra + /// steps to be run. + void onAttached(ApplicationPackage package, BuildMode buildMode, Process process) {} +} + +class DesktopLogReader extends DeviceLogReader { + final StreamController> _inputController = StreamController>.broadcast(); + + void initializeProcess(Process process) { + process.stdout.listen(_inputController.add); + process.stderr.listen(_inputController.add); + process.exitCode.then((int result) { + _inputController.close(); + }); + } + + @override + Stream get logLines { + return _inputController.stream + .transform(utf8.decoder) + .transform(const LineSplitter()); + } + + @override + String get name => 'desktop'; +} diff --git a/packages/flutter_tools/lib/src/linux/linux_device.dart b/packages/flutter_tools/lib/src/linux/linux_device.dart index 04b85a4eef..b4e3030b5f 100644 --- a/packages/flutter_tools/lib/src/linux/linux_device.dart +++ b/packages/flutter_tools/lib/src/linux/linux_device.dart @@ -2,129 +2,54 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import '../application_package.dart'; -import '../base/io.dart'; -import '../base/os.dart'; import '../base/platform.dart'; -import '../base/process_manager.dart'; import '../build_info.dart'; -import '../desktop.dart'; +import '../desktop_device.dart'; import '../device.dart'; -import '../globals.dart'; import '../project.dart'; -import '../protocol_discovery.dart'; import 'application_package.dart'; import 'build_linux.dart'; import 'linux_workflow.dart'; /// A device that represents a desktop Linux target. -class LinuxDevice extends Device { +class LinuxDevice extends DesktopDevice { LinuxDevice() : super( 'Linux', - category: Category.desktop, platformType: PlatformType.linux, ephemeral: false, ); - @override - void clearLogs() { } - - @override - DeviceLogReader getLogReader({ ApplicationPackage app }) { - return _logReader; - } - final DesktopLogReader _logReader = DesktopLogReader(); - - // Since the host and target devices are the same, no work needs to be done - // to install the application. - @override - Future installApp(ApplicationPackage app) async => true; - - // Since the host and target devices are the same, no work needs to be done - // to install the application. - @override - Future isAppInstalled(ApplicationPackage app) async => true; - - // Since the host and target devices are the same, no work needs to be done - // to install the application. - @override - Future isLatestBuildInstalled(ApplicationPackage app) async => true; - - @override - Future get isLocalEmulator async => false; - - @override - Future get emulatorId async => null; - @override bool isSupported() => true; @override String get name => 'Linux'; - @override - DevicePortForwarder get portForwarder => const NoOpDevicePortForwarder(); - - @override - Future get sdkNameAndVersion async => os.name; - - @override - Future startApp( - covariant LinuxApp package, { - String mainPath, - String route, - DebuggingOptions debuggingOptions, - Map platformArgs, - bool prebuiltApplication = false, - bool ipv6 = false, - }) async { - _lastBuiltMode = debuggingOptions.buildInfo.mode; - if (!prebuiltApplication) { - await buildLinux( - FlutterProject.current().linux, - debuggingOptions.buildInfo, - target: mainPath, - ); - } - final Process process = await processManager.start([ - package.executable(debuggingOptions?.buildInfo?.mode), - ]); - if (debuggingOptions?.buildInfo?.isRelease == true) { - return LaunchResult.succeeded(); - } - _logReader.initializeProcess(process); - final ProtocolDiscovery observatoryDiscovery = ProtocolDiscovery.observatory(_logReader); - try { - final Uri observatoryUri = await observatoryDiscovery.uri; - return LaunchResult.succeeded(observatoryUri: observatoryUri); - } catch (error) { - printError('Error waiting for a debug connection: $error'); - return LaunchResult.failed(); - } finally { - await observatoryDiscovery.cancel(); - } - } - - @override - Future stopApp(covariant LinuxApp app) async { - return killProcess(app.executable(_lastBuiltMode)); - } - @override Future get targetPlatform async => TargetPlatform.linux_x64; - // Since the host and target devices are the same, no work needs to be done - // to uninstall the application. - @override - Future uninstallApp(ApplicationPackage app) async => true; - @override bool isSupportedForProject(FlutterProject flutterProject) { return flutterProject.linux.existsSync(); } - // Track the last built mode from startApp. - BuildMode _lastBuiltMode; + @override + Future buildForDevice( + covariant LinuxApp package, { + String mainPath, + BuildInfo buildInfo, + }) async { + await buildLinux( + FlutterProject.current().linux, + buildInfo, + target: mainPath, + ); + } + + @override + String executablePathForDevice(covariant LinuxApp package, BuildMode buildMode) { + return package.executable(buildMode); + } } class LinuxDevices extends PollingDeviceDiscovery { diff --git a/packages/flutter_tools/lib/src/macos/macos_device.dart b/packages/flutter_tools/lib/src/macos/macos_device.dart index ddd893d585..f33f43675e 100644 --- a/packages/flutter_tools/lib/src/macos/macos_device.dart +++ b/packages/flutter_tools/lib/src/macos/macos_device.dart @@ -2,145 +2,71 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import '../application_package.dart'; import '../base/io.dart'; -import '../base/os.dart'; import '../base/platform.dart'; import '../base/process_manager.dart'; import '../build_info.dart'; -import '../cache.dart'; -import '../desktop.dart'; +import '../desktop_device.dart'; import '../device.dart'; -import '../globals.dart'; import '../macos/application_package.dart'; import '../project.dart'; -import '../protocol_discovery.dart'; import 'build_macos.dart'; import 'macos_workflow.dart'; /// A device that represents a desktop MacOS target. -class MacOSDevice extends Device { +class MacOSDevice extends DesktopDevice { MacOSDevice() : super( 'macOS', - category: Category.desktop, platformType: PlatformType.macos, ephemeral: false, ); - @override - void clearLogs() { } - - @override - DeviceLogReader getLogReader({ ApplicationPackage app }) { - return _deviceLogReader; - } - final DesktopLogReader _deviceLogReader = DesktopLogReader(); - - // Since the host and target devices are the same, no work needs to be done - // to install the application. - @override - Future installApp(ApplicationPackage app) async => true; - - // Since the host and target devices are the same, no work needs to be done - // to install the application. - @override - Future isAppInstalled(ApplicationPackage app) async => true; - - // Since the host and target devices are the same, no work needs to be done - // to install the application. - @override - Future isLatestBuildInstalled(ApplicationPackage app) async => true; - - @override - Future get isLocalEmulator async => false; - - @override - Future get emulatorId async => null; - @override bool isSupported() => true; @override String get name => 'macOS'; - @override - DevicePortForwarder get portForwarder => const NoOpDevicePortForwarder(); - - @override - Future get sdkNameAndVersion async => os.name; - - @override - Future startApp( - covariant MacOSApp package, { - String mainPath, - String route, - DebuggingOptions debuggingOptions, - Map platformArgs, - bool prebuiltApplication = false, - bool ipv6 = false, - }) async { - if (!prebuiltApplication) { - Cache.releaseLockEarly(); - await buildMacOS( - flutterProject: FlutterProject.current(), - buildInfo: debuggingOptions?.buildInfo, - targetOverride: mainPath, - ); - } - - // Ensure that the executable is locatable. - final String executable = package.executable(debuggingOptions?.buildInfo?.mode); - if (executable == null) { - printError('Unable to find executable to run'); - return LaunchResult.failed(); - } - - _lastBuiltMode = debuggingOptions?.buildInfo?.mode; - final Process process = await processManager.start([ - executable, - ]); - if (debuggingOptions?.buildInfo?.isRelease == true) { - return LaunchResult.succeeded(); - } - _deviceLogReader.initializeProcess(process); - final ProtocolDiscovery observatoryDiscovery = ProtocolDiscovery.observatory(_deviceLogReader); - try { - final Uri observatoryUri = await observatoryDiscovery.uri; - // Bring app to foreground. - await processManager.run([ - 'open', package.applicationBundle(debuggingOptions?.buildInfo?.mode), - ]); - return LaunchResult.succeeded(observatoryUri: observatoryUri); - } catch (error) { - printError('Error waiting for a debug connection: $error'); - return LaunchResult.failed(); - } finally { - await observatoryDiscovery.cancel(); - } - } - - // TODO(jonahwilliams): implement using process manager. - // currently we rely on killing the isolate taking down the application. - @override - Future stopApp(covariant MacOSApp app) async { - return killProcess(app.executable(_lastBuiltMode)); - } - @override Future get targetPlatform async => TargetPlatform.darwin_x64; - // Since the host and target devices are the same, no work needs to be done - // to uninstall the application. - @override - Future uninstallApp(ApplicationPackage app) async => true; - @override bool isSupportedForProject(FlutterProject flutterProject) { return flutterProject.macos.existsSync(); } - // Track the last built mode from startApp. - BuildMode _lastBuiltMode; + @override + Future buildForDevice( + covariant MacOSApp package, { + String mainPath, + BuildInfo buildInfo, + }) async { + await buildMacOS( + flutterProject: FlutterProject.current(), + buildInfo: buildInfo, + targetOverride: mainPath, + ); + } + + @override + String executablePathForDevice(covariant MacOSApp package, BuildMode buildMode) { + return package.executable(buildMode); + } + + @override + void onAttached(covariant MacOSApp package, BuildMode buildMode, Process process) { + // Bring app to foreground. Ideally this would be done post-launch rather + // than post-attach, since this won't run for release builds, but there's + // no general-purpose way of knowing when a process is far enoug along in + // the launch process for 'open' to foreground it. + processManager.run([ + 'open', package.applicationBundle(buildMode), + ]).then((ProcessResult result) { + if (result.exitCode != 0) { + print('Failed to foreground app; open returned ${result.exitCode}'); + } + }); + } } class MacOSDevices extends PollingDeviceDiscovery { diff --git a/packages/flutter_tools/lib/src/windows/windows_device.dart b/packages/flutter_tools/lib/src/windows/windows_device.dart index 7e01d9d875..31aab39679 100644 --- a/packages/flutter_tools/lib/src/windows/windows_device.dart +++ b/packages/flutter_tools/lib/src/windows/windows_device.dart @@ -4,133 +4,56 @@ import 'package:meta/meta.dart'; -import '../application_package.dart'; import '../base/io.dart'; -import '../base/os.dart'; import '../base/platform.dart'; import '../base/process.dart'; import '../build_info.dart'; -import '../desktop.dart'; +import '../desktop_device.dart'; import '../device.dart'; -import '../globals.dart'; import '../project.dart'; -import '../protocol_discovery.dart'; import 'application_package.dart'; import 'build_windows.dart'; import 'windows_workflow.dart'; /// A device that represents a desktop Windows target. -class WindowsDevice extends Device { +class WindowsDevice extends DesktopDevice { WindowsDevice() : super( 'Windows', - category: Category.desktop, platformType: PlatformType.windows, ephemeral: false, ); - @override - void clearLogs() { } - - @override - DeviceLogReader getLogReader({ ApplicationPackage app }) { - return _logReader; - } - final DesktopLogReader _logReader = DesktopLogReader(); - - // Since the host and target devices are the same, no work needs to be done - // to install the application. - @override - Future installApp(ApplicationPackage app) async => true; - - // Since the host and target devices are the same, no work needs to be done - // to install the application. - @override - Future isAppInstalled(ApplicationPackage app) async => true; - - // Since the host and target devices are the same, no work needs to be done - // to install the application. - @override - Future isLatestBuildInstalled(ApplicationPackage app) async => true; - - @override - Future get isLocalEmulator async => false; - - @override - Future get emulatorId async => null; - @override bool isSupported() => true; @override String get name => 'Windows'; - @override - DevicePortForwarder get portForwarder => const NoOpDevicePortForwarder(); - - @override - Future get sdkNameAndVersion async => os.name; - - @override - Future startApp( - covariant WindowsApp package, { - String mainPath, - String route, - DebuggingOptions debuggingOptions, - Map platformArgs, - bool prebuiltApplication = false, - bool ipv6 = false, - }) async { - if (!prebuiltApplication) { - await buildWindows( - FlutterProject.current().windows, - debuggingOptions.buildInfo, - target: mainPath, - ); - } - final Process process = await processUtils.start([ - package.executable(debuggingOptions?.buildInfo?.mode), - ]); - if (debuggingOptions?.buildInfo?.isRelease == true) { - return LaunchResult.succeeded(); - } - _logReader.initializeProcess(process); - final ProtocolDiscovery observatoryDiscovery = ProtocolDiscovery.observatory(_logReader); - try { - final Uri observatoryUri = await observatoryDiscovery.uri; - return LaunchResult.succeeded(observatoryUri: observatoryUri); - } catch (error) { - printError('Error waiting for a debug connection: $error'); - return LaunchResult.failed(); - } finally { - await observatoryDiscovery.cancel(); - } - } - - @override - Future stopApp(covariant WindowsApp app) async { - // Assume debug for now. - final List process = runningProcess(app.executable(BuildMode.debug)); - if (process == null) { - return false; - } - final RunResult result = await processUtils.run( - ['Taskkill', '/PID', process.first, '/F'], - ); - return result.exitCode == 0; - } - @override Future get targetPlatform async => TargetPlatform.windows_x64; - // Since the host and target devices are the same, no work needs to be done - // to uninstall the application. - @override - Future uninstallApp(ApplicationPackage app) async => true; - @override bool isSupportedForProject(FlutterProject flutterProject) { return flutterProject.windows.existsSync(); } + + @override + Future buildForDevice( + covariant WindowsApp package, { + String mainPath, + BuildInfo buildInfo, + }) async { + await buildWindows( + FlutterProject.current().windows, + buildInfo, + target: mainPath, + ); + } + + @override + String executablePathForDevice(covariant WindowsApp package, BuildMode buildMode) { + return package.executable(buildMode); + } } class WindowsDevices extends PollingDeviceDiscovery { diff --git a/packages/flutter_tools/test/general.shard/desktop_device_test.dart b/packages/flutter_tools/test/general.shard/desktop_device_test.dart new file mode 100644 index 0000000000..f036588b20 --- /dev/null +++ b/packages/flutter_tools/test/general.shard/desktop_device_test.dart @@ -0,0 +1,198 @@ +// Copyright 2019 The Chromium 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 'dart:async'; +import 'dart:convert'; + +import 'package:flutter_tools/src/base/context.dart'; +import 'package:flutter_tools/src/base/logger.dart'; +import 'package:flutter_tools/src/project.dart'; +import 'package:mockito/mockito.dart'; +import 'package:process/process.dart'; + +import 'package:flutter_tools/src/application_package.dart'; +import 'package:flutter_tools/src/base/file_system.dart'; +import 'package:flutter_tools/src/base/io.dart'; +import 'package:flutter_tools/src/base/os.dart'; +import 'package:flutter_tools/src/build_info.dart'; +import 'package:flutter_tools/src/desktop_device.dart'; +import 'package:flutter_tools/src/device.dart'; + +import '../src/common.dart'; +import '../src/context.dart'; +import '../src/mocks.dart'; + +/// A trivial subclass of DesktopDevice for testing the shared functionality. +class FakeDesktopDevice extends DesktopDevice { + FakeDesktopDevice() : super( + 'dummy', + platformType: PlatformType.linux, + ephemeral: false, + ); + + /// The [mainPath] last passed to [buildForDevice]. + String lastBuiltMainPath; + + /// The [buildInfo] last passed to [buildForDevice]. + BuildInfo lastBuildInfo; + + @override + String get name => 'dummy'; + + @override + Future get targetPlatform async => TargetPlatform.tester; + + @override + bool isSupported() => true; + + @override + bool isSupportedForProject(FlutterProject flutterProject) => true; + + @override + Future buildForDevice( + ApplicationPackage package, { + String mainPath, + BuildInfo buildInfo, + }) async { + lastBuiltMainPath = mainPath; + lastBuildInfo = buildInfo; + } + + // Dummy implementation that just returns the build mode name. + @override + String executablePathForDevice(ApplicationPackage package, BuildMode buildMode) { + return buildMode == null ? 'null' : getNameForBuildMode(buildMode); + } +} + +/// A desktop device that returns a null executable path, for failure testing. +class NullExecutableDesktopDevice extends FakeDesktopDevice { + @override + String executablePathForDevice(ApplicationPackage package, BuildMode buildMode) { + return null; + } +} + +class MockAppplicationPackage extends Mock implements ApplicationPackage {} + +class MockFileSystem extends Mock implements FileSystem {} + +class MockFile extends Mock implements File {} + +class MockProcessManager extends Mock implements ProcessManager {} + +void main() { + group('Basic info', () { + test('Category is desktop', () async { + final FakeDesktopDevice device = FakeDesktopDevice(); + expect(device.category, Category.desktop); + }); + + test('Not an emulator', () async { + final FakeDesktopDevice device = FakeDesktopDevice(); + expect(await device.isLocalEmulator, false); + expect(await device.emulatorId, null); + }); + + testUsingContext('Uses OS name as SDK name', () async { + final FakeDesktopDevice device = FakeDesktopDevice(); + expect(await device.sdkNameAndVersion, os.name); + }); + }); + + group('Install', () { + test('Install checks always return true', () async { + final FakeDesktopDevice device = FakeDesktopDevice(); + expect(await device.isAppInstalled(null), true); + expect(await device.isLatestBuildInstalled(null), true); + expect(device.category, Category.desktop); + }); + + test('Install and uninstall are no-ops that report success', () async { + final FakeDesktopDevice device = FakeDesktopDevice(); + final MockAppplicationPackage package = MockAppplicationPackage(); + expect(await device.uninstallApp(package), true); + expect(await device.isAppInstalled(package), true); + expect(await device.isLatestBuildInstalled(package), true); + + expect(await device.installApp(package), true); + expect(await device.isAppInstalled(package), true); + expect(await device.isLatestBuildInstalled(package), true); + expect(device.category, Category.desktop); + }); + }); + + group('Starting and stopping application', () { + final MockFileSystem mockFileSystem = MockFileSystem(); + final MockProcessManager mockProcessManager = MockProcessManager(); + + // Configures mock environment so that startApp will be able to find and + // run an FakeDesktopDevice exectuable with for the given mode. + void setUpMockExecutable(FakeDesktopDevice device, BuildMode mode, {Future exitFuture}) { + final String executableName = device.executablePathForDevice(null, mode); + final MockFile mockFile = MockFile(); + when(mockFileSystem.file(executableName)).thenReturn(mockFile); + when(mockFile.existsSync()).thenReturn(true); + when(mockProcessManager.start([executableName])).thenAnswer((Invocation invocation) async { + return FakeProcess( + exitCode: Completer().future, + stdout: Stream>.fromIterable(>[ + utf8.encode('Observatory listening on http://127.0.0.1/0\n'), + ]), + stderr: const Stream>.empty(), + ); + }); + when(mockProcessManager.run(any)).thenAnswer((Invocation invocation) async { + return ProcessResult(0, 1, '', ''); + }); + } + + test('Stop without start is a successful no-op', () async { + final FakeDesktopDevice device = FakeDesktopDevice(); + final MockAppplicationPackage package = MockAppplicationPackage(); + expect(await device.stopApp(package), true); + }); + + testUsingContext('Can run from prebuilt application', () async { + final FakeDesktopDevice device = FakeDesktopDevice(); + final MockAppplicationPackage package = MockAppplicationPackage(); + setUpMockExecutable(device, null); + final LaunchResult result = await device.startApp(package, prebuiltApplication: true); + expect(result.started, true); + expect(result.observatoryUri, Uri.parse('http://127.0.0.1/0')); + }, overrides: { + FileSystem: () => mockFileSystem, + ProcessManager: () => mockProcessManager, + }); + + testUsingContext('Null executable path fails gracefully', () async { + final NullExecutableDesktopDevice device = NullExecutableDesktopDevice(); + final MockAppplicationPackage package = MockAppplicationPackage(); + final LaunchResult result = await device.startApp(package, prebuiltApplication: true); + expect(result.started, false); + final BufferLogger logger = context.get(); + expect(logger.errorText, contains('Unable to find executable to run')); + }); + + testUsingContext('stopApp kills process started by startApp', () async { + final FakeDesktopDevice device = FakeDesktopDevice(); + final MockAppplicationPackage package = MockAppplicationPackage(); + setUpMockExecutable(device, null); + final LaunchResult result = await device.startApp(package, prebuiltApplication: true); + expect(result.started, true); + expect(await device.stopApp(package), true); + }, overrides: { + FileSystem: () => mockFileSystem, + ProcessManager: () => mockProcessManager, + }); + }); + + test('Port forwarder is a no-op', () async { + final FakeDesktopDevice device = FakeDesktopDevice(); + final DevicePortForwarder portForwarder = device.portForwarder; + final int result = await portForwarder.forward(2); + expect(result, 2); + expect(portForwarder.forwardedPorts.isEmpty, true); + }); +} diff --git a/packages/flutter_tools/test/general.shard/linux/linux_device_test.dart b/packages/flutter_tools/test/general.shard/linux/linux_device_test.dart index c25d2e9106..aafffb923b 100644 --- a/packages/flutter_tools/test/general.shard/linux/linux_device_test.dart +++ b/packages/flutter_tools/test/general.shard/linux/linux_device_test.dart @@ -4,7 +4,6 @@ import 'package:file/memory.dart'; import 'package:flutter_tools/src/base/file_system.dart'; -import 'package:flutter_tools/src/base/io.dart'; import 'package:flutter_tools/src/base/platform.dart'; import 'package:flutter_tools/src/build_info.dart'; import 'package:flutter_tools/src/linux/application_package.dart'; @@ -12,7 +11,6 @@ import 'package:flutter_tools/src/linux/linux_device.dart'; import 'package:flutter_tools/src/device.dart'; import 'package:flutter_tools/src/project.dart'; import 'package:mockito/mockito.dart'; -import 'package:process/process.dart'; import '../../src/common.dart'; import '../../src/context.dart'; @@ -21,22 +19,8 @@ void main() { group(LinuxDevice, () { final LinuxDevice device = LinuxDevice(); final MockPlatform notLinux = MockPlatform(); - final MockProcessManager mockProcessManager = MockProcessManager(); - const String flutterToolCommand = 'flutter --someoption somevalue'; when(notLinux.isLinux).thenReturn(false); - when(mockProcessManager.run([ - 'ps', 'aux', - ])).thenAnswer((Invocation invocation) async { - // The flutter tool process is returned as output to the ps aux command - final MockProcessResult result = MockProcessResult(); - when(result.exitCode).thenReturn(0); - when(result.stdout).thenReturn('username $pid $flutterToolCommand'); - return result; - }); - when(mockProcessManager.run([ - 'kill', '$pid', - ])).thenThrow(Exception('Flutter tool process has been killed')); testUsingContext('defaults', () async { final PrebuiltLinuxApp linuxApp = PrebuiltLinuxApp(executable: 'foo'); @@ -48,24 +32,6 @@ void main() { expect(await device.isAppInstalled(linuxApp), true); expect(await device.stopApp(linuxApp), true); expect(device.category, Category.desktop); - }, overrides: { - ProcessManager: () => mockProcessManager, - }); - - test('noop port forwarding', () async { - final LinuxDevice device = LinuxDevice(); - final DevicePortForwarder portForwarder = device.portForwarder; - final int result = await portForwarder.forward(2); - expect(result, 2); - expect(portForwarder.forwardedPorts.isEmpty, true); - }); - - testUsingContext('The current running process is not killed when stopping the app', () async { - // The name of the executable is the same as a command line argument to the flutter tool - final PrebuiltLinuxApp linuxApp = PrebuiltLinuxApp(executable: 'somevalue'); - expect(await device.stopApp(linuxApp), true); - }, overrides: { - ProcessManager: () => mockProcessManager, }); testUsingContext('No devices listed if platform unsupported', () async { @@ -95,16 +61,24 @@ void main() { }, overrides: { FileSystem: () => MemoryFileSystem(), }); + + testUsingContext('executablePathForDevice uses the correct package executable', () async { + final MockLinuxApp mockApp = MockLinuxApp(); + const String debugPath = 'debug/executable'; + const String profilePath = 'profile/executable'; + const String releasePath = 'release/executable'; + when(mockApp.executable(BuildMode.debug)).thenReturn(debugPath); + when(mockApp.executable(BuildMode.profile)).thenReturn(profilePath); + when(mockApp.executable(BuildMode.release)).thenReturn(releasePath); + + expect(LinuxDevice().executablePathForDevice(mockApp, BuildMode.debug), debugPath); + expect(LinuxDevice().executablePathForDevice(mockApp, BuildMode.profile), profilePath); + expect(LinuxDevice().executablePathForDevice(mockApp, BuildMode.release), releasePath); + }, overrides: { + FileSystem: () => MemoryFileSystem(), + }); } class MockPlatform extends Mock implements Platform {} -class MockFileSystem extends Mock implements FileSystem {} - -class MockFile extends Mock implements File {} - -class MockProcessManager extends Mock implements ProcessManager {} - -class MockProcess extends Mock implements Process {} - -class MockProcessResult extends Mock implements ProcessResult {} +class MockLinuxApp extends Mock implements LinuxApp {} diff --git a/packages/flutter_tools/test/general.shard/macos/macos_device_test.dart b/packages/flutter_tools/test/general.shard/macos/macos_device_test.dart index b826dac45b..56f307db52 100644 --- a/packages/flutter_tools/test/general.shard/macos/macos_device_test.dart +++ b/packages/flutter_tools/test/general.shard/macos/macos_device_test.dart @@ -2,9 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'dart:async'; -import 'dart:convert'; - import 'package:file/memory.dart'; import 'package:flutter_tools/src/base/context.dart'; import 'package:flutter_tools/src/project.dart'; @@ -21,7 +18,6 @@ import 'package:flutter_tools/src/macos/macos_device.dart'; import '../../src/common.dart'; import '../../src/context.dart'; -import '../../src/mocks.dart'; void main() { group(MacOSDevice, () { @@ -36,93 +32,13 @@ void main() { testUsingContext('defaults', () async { final MockMacOSApp mockMacOSApp = MockMacOSApp(); - when(mockMacOSApp.executable(any)).thenReturn('foo'); expect(await device.targetPlatform, TargetPlatform.darwin_x64); expect(device.name, 'macOS'); expect(await device.installApp(mockMacOSApp), true); expect(await device.uninstallApp(mockMacOSApp), true); expect(await device.isLatestBuildInstalled(mockMacOSApp), true); expect(await device.isAppInstalled(mockMacOSApp), true); - expect(await device.stopApp(mockMacOSApp), false); expect(device.category, Category.desktop); - }, overrides: { - ProcessManager: () => mockProcessManager, - }); - - testUsingContext('stopApp', () async { - const String psOut = r''' -tester 17193 0.0 0.2 4791128 37820 ?? S 2:27PM 0:00.09 /Applications/foo -'''; - final MockMacOSApp mockMacOSApp = MockMacOSApp(); - when(mockMacOSApp.executable(any)).thenReturn('/Applications/foo'); - when(mockProcessManager.run(['ps', 'aux'])).thenAnswer((Invocation invocation) async { - return ProcessResult(1, 0, psOut, ''); - }); - when(mockProcessManager.run(['kill', '17193'])).thenAnswer((Invocation invocation) async { - return ProcessResult(2, 0, '', ''); - }); - expect(await device.stopApp(mockMacOSApp), true); - verify(mockProcessManager.run(['kill', '17193'])); - }, overrides: { - ProcessManager: () => mockProcessManager, - }); - - group('startApp', () { - final MockMacOSApp macOSApp = MockMacOSApp(); - final MockFileSystem mockFileSystem = MockFileSystem(); - final MockProcessManager mockProcessManager = MockProcessManager(); - final MockFile mockFile = MockFile(); - when(macOSApp.executable(any)).thenReturn('test'); - when(mockFileSystem.file('test')).thenReturn(mockFile); - when(mockFile.existsSync()).thenReturn(true); - when(mockProcessManager.start(['test'])).thenAnswer((Invocation invocation) async { - return FakeProcess( - exitCode: Completer().future, - stdout: Stream>.fromIterable(>[ - utf8.encode('Observatory listening on http://127.0.0.1/0\n'), - ]), - stderr: const Stream>.empty(), - ); - }); - when(mockProcessManager.run(any)).thenAnswer((Invocation invocation) async { - return ProcessResult(0, 1, '', ''); - }); - - testUsingContext('Can run from prebuilt application', () async { - final LaunchResult result = await device.startApp(macOSApp, prebuiltApplication: true); - expect(result.started, true); - expect(result.observatoryUri, Uri.parse('http://127.0.0.1/0')); - }, overrides: { - FileSystem: () => mockFileSystem, - ProcessManager: () => mockProcessManager, - }); - - testUsingContext('The current running process is not killed when stopping the app', () async { - final String psOut = ''' -tester $pid 0.0 0.2 4791128 37820 ?? S 2:27PM 0:00.09 flutter run --use-application-binary /Applications/foo -'''; - final MockMacOSApp mockMacOSApp = MockMacOSApp(); - // The name of the executable is the same as a command line argument to the flutter tool - when(mockMacOSApp.executable(any)).thenReturn('/Applications/foo'); - when(mockProcessManager.run(['ps', 'aux'])).thenAnswer((Invocation invocation) async { - return ProcessResult(1, 0, psOut, ''); - }); - when(mockProcessManager.run([ - 'kill', '$pid', - ])).thenThrow(Exception('Flutter tool process has been killed')); - - expect(await device.stopApp(mockMacOSApp), true); - }, overrides: { - ProcessManager: () => mockProcessManager, - }); - }); - - test('noop port forwarding', () async { - final MacOSDevice device = MacOSDevice(); - final DevicePortForwarder portForwarder = device.portForwarder; - final int result = await portForwarder.forward(2); - expect(result, 2); - expect(portForwarder.forwardedPorts.isEmpty, true); }); testUsingContext('No devices listed if platform unsupported', () async { @@ -151,6 +67,22 @@ tester $pid 0.0 0.2 4791128 37820 ?? S 2:27PM 0:00.09 flutter r }, overrides: { FileSystem: () => MemoryFileSystem(), }); + + testUsingContext('executablePathForDevice uses the correct package executable', () async { + final MockMacOSApp mockApp = MockMacOSApp(); + const String debugPath = 'debug/executable'; + const String profilePath = 'profile/executable'; + const String releasePath = 'release/executable'; + when(mockApp.executable(BuildMode.debug)).thenReturn(debugPath); + when(mockApp.executable(BuildMode.profile)).thenReturn(profilePath); + when(mockApp.executable(BuildMode.release)).thenReturn(releasePath); + + expect(MacOSDevice().executablePathForDevice(mockApp, BuildMode.debug), debugPath); + expect(MacOSDevice().executablePathForDevice(mockApp, BuildMode.profile), profilePath); + expect(MacOSDevice().executablePathForDevice(mockApp, BuildMode.release), releasePath); + }, overrides: { + FileSystem: () => MemoryFileSystem(), + }); }); } @@ -158,10 +90,4 @@ class MockPlatform extends Mock implements Platform {} class MockMacOSApp extends Mock implements MacOSApp {} -class MockFileSystem extends Mock implements FileSystem {} - -class MockFile extends Mock implements File {} - class MockProcessManager extends Mock implements ProcessManager {} - -class MockProcess extends Mock implements Process {} diff --git a/packages/flutter_tools/test/general.shard/windows/windows_device_test.dart b/packages/flutter_tools/test/general.shard/windows/windows_device_test.dart index 755d08f806..0a3023cf24 100644 --- a/packages/flutter_tools/test/general.shard/windows/windows_device_test.dart +++ b/packages/flutter_tools/test/general.shard/windows/windows_device_test.dart @@ -4,7 +4,6 @@ import 'package:file/memory.dart'; import 'package:flutter_tools/src/base/file_system.dart'; -import 'package:flutter_tools/src/base/io.dart'; import 'package:flutter_tools/src/base/platform.dart'; import 'package:flutter_tools/src/build_info.dart'; import 'package:flutter_tools/src/project.dart'; @@ -12,7 +11,6 @@ import 'package:flutter_tools/src/windows/application_package.dart'; import 'package:flutter_tools/src/windows/windows_device.dart'; import 'package:flutter_tools/src/device.dart'; import 'package:mockito/mockito.dart'; -import 'package:process/process.dart'; import '../../src/common.dart'; import '../../src/context.dart'; @@ -21,28 +19,9 @@ void main() { group(WindowsDevice, () { final WindowsDevice device = WindowsDevice(); final MockPlatform notWindows = MockPlatform(); - final MockProcessManager mockProcessManager = MockProcessManager(); - const String flutterToolBinary = 'flutter.exe'; when(notWindows.isWindows).thenReturn(false); when(notWindows.environment).thenReturn(const {}); - when(mockProcessManager.runSync( - ['powershell', '-script="Get-CimInstance Win32_Process"'], - workingDirectory: anyNamed('workingDirectory'), - environment: anyNamed('environment'), - )).thenAnswer((Invocation invocation) { - // The flutter tool process is returned as output to the powershell script - final MockProcessResult result = MockProcessResult(); - when(result.exitCode).thenReturn(0); - when(result.stdout).thenReturn('$pid $flutterToolBinary'); - when(result.stderr).thenReturn(''); - return result; - }); - when(mockProcessManager.run( - ['Taskkill', '/PID', '$pid', '/F'], - workingDirectory: anyNamed('workingDirectory'), - environment: anyNamed('environment'), - )).thenThrow(Exception('Flutter tool process has been killed')); testUsingContext('defaults', () async { final PrebuiltWindowsApp windowsApp = PrebuiltWindowsApp(executable: 'foo'); @@ -52,18 +31,7 @@ void main() { expect(await device.uninstallApp(windowsApp), true); expect(await device.isLatestBuildInstalled(windowsApp), true); expect(await device.isAppInstalled(windowsApp), true); - expect(await device.stopApp(windowsApp), false); expect(device.category, Category.desktop); - }, overrides: { - ProcessManager: () => mockProcessManager, - }); - - test('noop port forwarding', () async { - final WindowsDevice device = WindowsDevice(); - final DevicePortForwarder portForwarder = device.portForwarder; - final int result = await portForwarder.forward(2); - expect(result, 2); - expect(portForwarder.forwardedPorts.isEmpty, true); }); testUsingContext('No devices listed if platform unsupported', () async { @@ -93,24 +61,24 @@ void main() { FileSystem: () => MemoryFileSystem(), }); - testUsingContext('The current running process is not killed when stopping the app', () async { - // The name of the executable is the same as the flutter tool one - final PrebuiltWindowsApp windowsApp = PrebuiltWindowsApp(executable: flutterToolBinary); - expect(await device.stopApp(windowsApp), false); + testUsingContext('executablePathForDevice uses the correct package executable', () async { + final MockWindowsApp mockApp = MockWindowsApp(); + const String debugPath = 'debug/executable'; + const String profilePath = 'profile/executable'; + const String releasePath = 'release/executable'; + when(mockApp.executable(BuildMode.debug)).thenReturn(debugPath); + when(mockApp.executable(BuildMode.profile)).thenReturn(profilePath); + when(mockApp.executable(BuildMode.release)).thenReturn(releasePath); + + expect(WindowsDevice().executablePathForDevice(mockApp, BuildMode.debug), debugPath); + expect(WindowsDevice().executablePathForDevice(mockApp, BuildMode.profile), profilePath); + expect(WindowsDevice().executablePathForDevice(mockApp, BuildMode.release), releasePath); }, overrides: { - ProcessManager: () => mockProcessManager, + FileSystem: () => MemoryFileSystem(), }); }); } class MockPlatform extends Mock implements Platform {} -class MockFileSystem extends Mock implements FileSystem {} - -class MockFile extends Mock implements File {} - -class MockProcessManager extends Mock implements ProcessManager {} - -class MockProcess extends Mock implements Process {} - -class MockProcessResult extends Mock implements ProcessResult {} +class MockWindowsApp extends Mock implements WindowsApp {}