From 18bb4d7254f89c02ef77b635dbc6fd93ab507ff6 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Mon, 12 Oct 2020 15:53:16 -0700 Subject: [PATCH] Revert "[flutter_tools] reland: fold process resolution logic into the flutter tool (#67957)" (#67968) This reverts commit bd8138797e445627d7a92ec139ac661af1b6b584. --- .../lib/src/base/error_handling_io.dart | 214 ++------ packages/flutter_tools/lib/src/base/os.dart | 10 +- .../flutter_tools/lib/src/base/process.dart | 9 - .../flutter_tools/lib/src/context_runner.dart | 3 +- .../flutter_tools/lib/src/ios/ios_deploy.dart | 4 + .../lib/src/linux/build_linux.dart | 11 +- .../lib/src/linux/linux_doctor.dart | 11 +- .../flutter_tools/lib/src/macos/xcode.dart | 8 + .../flutter_tools/lib/src/web/chrome.dart | 6 +- .../lib/src/windows/build_windows.dart | 11 +- .../lib/src/windows/visual_studio.dart | 4 + .../hermetic/build_linux_test.dart | 5 +- .../base/error_handling_io_test.dart | 464 ++++-------------- .../test/general.shard/base/os_test.dart | 8 +- .../test/general.shard/macos/xcode_test.dart | 2 +- 15 files changed, 158 insertions(+), 612 deletions(-) diff --git a/packages/flutter_tools/lib/src/base/error_handling_io.dart b/packages/flutter_tools/lib/src/base/error_handling_io.dart index 195407429f9..f6816b65c42 100644 --- a/packages/flutter_tools/lib/src/base/error_handling_io.dart +++ b/packages/flutter_tools/lib/src/base/error_handling_io.dart @@ -11,10 +11,7 @@ import 'package:path/path.dart' as p; // ignore: package_path_import import 'package:process/process.dart'; import 'common.dart' show throwToolExit; -import 'io.dart'; -import 'logger.dart'; import 'platform.dart'; -import 'process.dart'; // The Flutter tool hits file system and process errors that only the end-user can address. // We would like these errors to not hit crash logging. In these cases, we @@ -56,11 +53,11 @@ class ErrorHandlingFileSystem extends ForwardingFileSystem { /// This can be used to bypass the [ErrorHandlingFileSystem] permission exit /// checks for situations where failure is acceptable, such as the flutter /// persistent settings cache. - static T noExitOnFailure(T Function() operation) { + static void noExitOnFailure(void Function() operation) { final bool previousValue = ErrorHandlingFileSystem._noExitOnFailure; try { ErrorHandlingFileSystem._noExitOnFailure = true; - return operation(); + operation(); } finally { ErrorHandlingFileSystem._noExitOnFailure = previousValue; } @@ -504,44 +501,6 @@ T _runSync(T Function() op, { } } -/// Type signature for [io.Process.killPid]. -typedef ProcessKillPid = bool Function(int pid, [io.ProcessSignal signal]); - -/// Type signature for [io.Process.run]. -typedef ProcessRun = Future Function( - String executable, - List arguments, { - String workingDirectory, - Map environment, - bool includeParentEnvironment, - bool runInShell, - Encoding stdoutEncoding, - Encoding stderrEncoding, -}); - -/// Type signature for [io.Process.runSync]. -typedef ProcessRunSync = io.ProcessResult Function( - String executable, - List arguments, { - String workingDirectory, - Map environment, - bool includeParentEnvironment, - bool runInShell, - Encoding stdoutEncoding, - Encoding stderrEncoding, -}); - -/// Type signature for [io.Process.start]. -typedef ProcessStart = Future Function( - String executable, - List arguments, { - String workingDirectory, - Map environment, - bool includeParentEnvironment, - bool runInShell, - io.ProcessStartMode mode, -}); - /// A [ProcessManager] that throws a [ToolExit] on certain errors. /// /// If a [ProcessException] is not caused by the Flutter tool, and can only be @@ -550,47 +509,28 @@ typedef ProcessStart = Future Function( /// /// See also: /// * [ErrorHandlingFileSystem], for a similar file system strategy. -class ErrorHandlingProcessManager implements ProcessManager { +class ErrorHandlingProcessManager extends ProcessManager { ErrorHandlingProcessManager({ + @required ProcessManager delegate, @required Platform platform, - @required FileSystem fileSystem, - @required Logger logger, - @visibleForTesting ProcessKillPid killPid = io.Process.killPid, - @visibleForTesting ProcessRun run = io.Process.run, - @visibleForTesting ProcessRunSync runSync = io.Process.runSync, - @visibleForTesting ProcessStart start = io.Process.start, - }) : _platform = platform, - _fileSytem = fileSystem, - _logger = logger, - _killPid = killPid, - _processRun = run, - _processRunSync = runSync, - _processStart = start; + }) : _delegate = delegate, + _platform = platform; - final Logger _logger; - final FileSystem _fileSytem; + final ProcessManager _delegate; final Platform _platform; - final ProcessKillPid _killPid; - final ProcessRun _processRun; - final ProcessRunSync _processRunSync; - final ProcessStart _processStart; - - // Cached executable paths, the first is the working directory, the next is a map from - // executable name to actual path. - final Map> _resolvedExecutables = >{}; @override bool canRun(dynamic executable, {String workingDirectory}) { - return _runSync( - () => _getCommandPath([executable], workingDirectory, strict: true) != null, + return _runSync( + () => _delegate.canRun(executable, workingDirectory: workingDirectory), platform: _platform, ); } @override bool killPid(int pid, [io.ProcessSignal signal = io.ProcessSignal.sigterm]) { - return _runSync( - () => _killPid(pid, signal), + return _runSync( + () => _delegate.killPid(pid, signal), platform: _platform, ); } @@ -605,18 +545,15 @@ class ErrorHandlingProcessManager implements ProcessManager { Encoding stdoutEncoding = io.systemEncoding, Encoding stderrEncoding = io.systemEncoding, }) { - return _run(() { - return _processRun( - _getCommandPath(command, workingDirectory), - _getArguments(command), - workingDirectory: workingDirectory, - environment: environment, - includeParentEnvironment: includeParentEnvironment, - runInShell: runInShell, - stdoutEncoding: stdoutEncoding, - stderrEncoding: stderrEncoding, - ); - }, platform: _platform); + return _run(() => _delegate.run( + command, + workingDirectory: workingDirectory, + environment: environment, + includeParentEnvironment: includeParentEnvironment, + runInShell: runInShell, + stdoutEncoding: stdoutEncoding, + stderrEncoding: stderrEncoding, + ), platform: _platform); } @override @@ -628,16 +565,13 @@ class ErrorHandlingProcessManager implements ProcessManager { bool runInShell = false, io.ProcessStartMode mode = io.ProcessStartMode.normal, }) { - return _run(() { - return _processStart( - _getCommandPath(command, workingDirectory), - _getArguments(command), - workingDirectory: workingDirectory, - environment: environment, - includeParentEnvironment: includeParentEnvironment, - runInShell: runInShell, - ); - }, platform: _platform); + return _run(() => _delegate.start( + command, + workingDirectory: workingDirectory, + environment: environment, + includeParentEnvironment: includeParentEnvironment, + runInShell: runInShell, + ), platform: _platform); } @override @@ -650,91 +584,15 @@ class ErrorHandlingProcessManager implements ProcessManager { Encoding stdoutEncoding = io.systemEncoding, Encoding stderrEncoding = io.systemEncoding, }) { - return _runSync(() { - return _processRunSync( - _getCommandPath(command, workingDirectory), - _getArguments(command), - workingDirectory: workingDirectory, - environment: environment, - includeParentEnvironment: includeParentEnvironment, - runInShell: runInShell, - stdoutEncoding: stdoutEncoding, - stderrEncoding: stderrEncoding, - ); - }, platform: _platform); - } - - /// Resolve the first item of [command] to a file or link. - /// - /// This function will cache each raw command string per working directory, - /// so that repeated lookups for the same command will resolve to the same - /// executable. If a new executable is installed in a higher priority - /// location after a command has already been cached, the older executable - /// will be used until the tool restarts. - /// - /// If the entity cannot be found, return the raw command as is and do not - /// cache it. - String _getCommandPath(List command, String workingDirectory, { bool strict = false }) { - final String executable = command.first.toString(); - final Map workingDirectoryCache = _resolvedExecutables[workingDirectory] ??= {}; - final String resolvedExecutable = workingDirectoryCache[executable] - ?? _which(executable, workingDirectory, strict); - if (resolvedExecutable != executable) { - workingDirectoryCache[executable] = resolvedExecutable; - } - return resolvedExecutable; - } - - String _which(String execName, String workingDirectory, bool strict) { - // `where` always returns all matches, not just the first one. - ProcessResult result; - try { - result = _processRunSync( - _platform.isWindows ? 'where' : 'which', - [execName], - workingDirectory: workingDirectory, - ); - } on ProcessException catch (err) { - if (!isMissingExecutableException(err) || !_platform.isWindows) { - rethrow; - } - // `where` could be missing if system32 is not on the PATH. - throwToolExit( - 'Cannot find the executable for `where`. This can happen if the System32 ' - 'folder (e.g. C:\\Windows\\System32 ) is removed from the PATH environment ' - 'variable. Ensure that this is present and then try again after restarting ' - 'the terminal and/or IDE.' - ); - } - if (result.exitCode != 0) { - return execName; - } - final List lines = (result.stdout as String).trim().split('\n'); - for (final String line in lines) { - final String resolvedPath = line.trim(); - try { - final String candidate = ErrorHandlingFileSystem.noExitOnFailure(() { - if (_fileSytem.isFileSync(resolvedPath)) { - return resolvedPath; - } - return null; - }); - if (candidate != null) { - return candidate; - } - } on Exception { - // Do nothing and prevent permission issues from causing a tool exit. - } - } - _logger.printTrace('Could not resolve executable for $execName in $workingDirectory.'); - if (strict) { - return null; - } - return execName; - } - - List _getArguments(List command) { - return [for (dynamic arg in command.skip(1)) arg.toString()]; + return _runSync(() => _delegate.runSync( + command, + workingDirectory: workingDirectory, + environment: environment, + includeParentEnvironment: includeParentEnvironment, + runInShell: runInShell, + stdoutEncoding: stdoutEncoding, + stderrEncoding: stderrEncoding, + ), platform: _platform); } } diff --git a/packages/flutter_tools/lib/src/base/os.dart b/packages/flutter_tools/lib/src/base/os.dart index bafd3a62250..8fd9a98ac48 100644 --- a/packages/flutter_tools/lib/src/base/os.dart +++ b/packages/flutter_tools/lib/src/base/os.dart @@ -213,10 +213,7 @@ class _PosixUtils extends OperatingSystemUtils { throwOnError: true, verboseExceptions: true, ); - } on ProcessException catch (err) { - if (!isMissingExecutableException(err)) { - rethrow; - } + } on ArgumentError { // unzip is not available. this error message is modeled after the download // error in bin/internal/update_dart_sdk.sh String message = 'Please install unzip.'; @@ -300,10 +297,7 @@ class _WindowsUtils extends OperatingSystemUtils { ProcessResult result; try { result = _processManager.runSync(['where', execName]); - } on ProcessException catch (err) { - if (!isMissingExecutableException(err)) { - rethrow; - } + } on ArgumentError { // `where` could be missing if system32 is not on the PATH. throwToolExit( 'Cannot find the executable for `where`. This can happen if the System32 ' diff --git a/packages/flutter_tools/lib/src/base/process.dart b/packages/flutter_tools/lib/src/base/process.dart index 59b65a9a18e..d7691849cc5 100644 --- a/packages/flutter_tools/lib/src/base/process.dart +++ b/packages/flutter_tools/lib/src/base/process.dart @@ -610,12 +610,3 @@ class _DefaultProcessUtils implements ProcessUtils { } } } - -/// Whether the process exception was thrown due to a mising executable. -/// -/// On macOS, Linux, and Windows error code 2 indicates a missing file, which -/// means that the process resolution failed to locate the correct -/// executable. -bool isMissingExecutableException(ProcessException processException) { - return processException.errorCode == 2; -} diff --git a/packages/flutter_tools/lib/src/context_runner.dart b/packages/flutter_tools/lib/src/context_runner.dart index 492b3da2f67..8d8fa83b9cf 100644 --- a/packages/flutter_tools/lib/src/context_runner.dart +++ b/packages/flutter_tools/lib/src/context_runner.dart @@ -223,9 +223,8 @@ Future runInContext( ), ProcessInfo: () => ProcessInfo(), ProcessManager: () => ErrorHandlingProcessManager( + delegate: const LocalProcessManager(), platform: globals.platform, - fileSystem: globals.fs, - logger: globals.logger, ), ProcessUtils: () => ProcessUtils( processManager: globals.processManager, diff --git a/packages/flutter_tools/lib/src/ios/ios_deploy.dart b/packages/flutter_tools/lib/src/ios/ios_deploy.dart index d039c6e6c36..8db8b6563d8 100644 --- a/packages/flutter_tools/lib/src/ios/ios_deploy.dart +++ b/packages/flutter_tools/lib/src/ios/ios_deploy.dart @@ -361,6 +361,10 @@ class IOSDeployDebugger { _logger.printTrace('ios-deploy failed: $exception'); _debuggerState = _IOSDeployDebuggerState.detached; _debuggerOutput.addError(exception, stackTrace); + } on ArgumentError catch (exception, stackTrace) { + _logger.printTrace('ios-deploy failed: $exception'); + _debuggerState = _IOSDeployDebuggerState.detached; + _debuggerOutput.addError(exception, stackTrace); } // Wait until the debugger attaches, or the attempt fails. return debuggerCompleter.future; diff --git a/packages/flutter_tools/lib/src/linux/build_linux.dart b/packages/flutter_tools/lib/src/linux/build_linux.dart index b1cd4170c8b..e1cd4cf40be 100644 --- a/packages/flutter_tools/lib/src/linux/build_linux.dart +++ b/packages/flutter_tools/lib/src/linux/build_linux.dart @@ -6,7 +6,6 @@ import '../artifacts.dart'; import '../base/analyze_size.dart'; import '../base/common.dart'; import '../base/file_system.dart'; -import '../base/io.dart'; import '../base/logger.dart'; import '../base/process.dart'; import '../base/utils.dart'; @@ -105,10 +104,7 @@ Future _runCmake(String buildModeName, Directory sourceDir, Directory buil }, trace: true, ); - } on ProcessException catch (err) { - if (!isMissingExecutableException(err)) { - rethrow; - } + } on ArgumentError { throwToolExit("cmake not found. Run 'flutter doctor' for more information."); } if (result != 0) { @@ -135,10 +131,7 @@ Future _runBuild(Directory buildDir) async { }, trace: true, ); - } on ProcessException catch (err) { - if (!isMissingExecutableException(err)) { - rethrow; - } + } on ArgumentError { throwToolExit("ninja not found. Run 'flutter doctor' for more information."); } if (result != 0) { diff --git a/packages/flutter_tools/lib/src/linux/linux_doctor.dart b/packages/flutter_tools/lib/src/linux/linux_doctor.dart index 4ea8849755d..ac32321d94f 100644 --- a/packages/flutter_tools/lib/src/linux/linux_doctor.dart +++ b/packages/flutter_tools/lib/src/linux/linux_doctor.dart @@ -6,7 +6,6 @@ import 'package:meta/meta.dart'; import 'package:process/process.dart'; import '../base/io.dart'; -import '../base/process.dart'; import '../base/user_messages.dart'; import '../base/version.dart'; import '../doctor.dart'; @@ -169,10 +168,7 @@ class LinuxDoctorValidator extends DoctorValidator { binary, '--version', ]); - } on ProcessException catch (err) { - if (!isMissingExecutableException(err)) { - rethrow; - } + } on ArgumentError { // ignore error. } if (result == null || result.exitCode != 0) { @@ -191,10 +187,7 @@ class LinuxDoctorValidator extends DoctorValidator { '--exists', library, ]); - } on ProcessException catch (err) { - if (!isMissingExecutableException(err)) { - rethrow; - } + } on ArgumentError { // ignore error. } return (result?.exitCode ?? 1) == 0; diff --git a/packages/flutter_tools/lib/src/macos/xcode.dart b/packages/flutter_tools/lib/src/macos/xcode.dart index 1d34cf3c427..fa824af359c 100644 --- a/packages/flutter_tools/lib/src/macos/xcode.dart +++ b/packages/flutter_tools/lib/src/macos/xcode.dart @@ -84,6 +84,8 @@ class Xcode { ).stdout.trim(); } on ProcessException { // Ignored, return null below. + } on ArgumentError { + // Ignored, return null below. } } return _xcodeSelectPath; @@ -274,6 +276,8 @@ class XCDevice { ).stdout.trim(); } on ProcessException catch (exception) { _logger.printTrace('Process exception finding xcdevice:\n$exception'); + } on ArgumentError catch (exception) { + _logger.printTrace('Argument exception finding xcdevice:\n$exception'); } } return _xcdevicePath; @@ -310,6 +314,8 @@ class XCDevice { _logger.printTrace('xcdevice returned an error:\n${result.stderr}'); } on ProcessException catch (exception) { _logger.printTrace('Process exception running xcdevice list:\n$exception'); + } on ArgumentError catch (exception) { + _logger.printTrace('Argument exception running xcdevice list:\n$exception'); } return null; @@ -402,6 +408,8 @@ class XCDevice { })); } on ProcessException catch (exception, stackTrace) { _deviceIdentifierByEvent.addError(exception, stackTrace); + } on ArgumentError catch (exception, stackTrace) { + _deviceIdentifierByEvent.addError(exception, stackTrace); } } diff --git a/packages/flutter_tools/lib/src/web/chrome.dart b/packages/flutter_tools/lib/src/web/chrome.dart index 2040d4ae4ca..fdc86fc685a 100644 --- a/packages/flutter_tools/lib/src/web/chrome.dart +++ b/packages/flutter_tools/lib/src/web/chrome.dart @@ -150,7 +150,11 @@ class ChromiumLauncher { /// Whether we can locate the chrome executable. bool canFindExecutable() { final String chrome = _browserFinder(_platform, _fileSystem); - return _processManager.canRun(chrome); + try { + return _processManager.canRun(chrome); + } on ArgumentError { + return false; + } } /// The executable this launcher will use. diff --git a/packages/flutter_tools/lib/src/windows/build_windows.dart b/packages/flutter_tools/lib/src/windows/build_windows.dart index 8081b71df71..6b819ee495f 100644 --- a/packages/flutter_tools/lib/src/windows/build_windows.dart +++ b/packages/flutter_tools/lib/src/windows/build_windows.dart @@ -6,7 +6,6 @@ import '../artifacts.dart'; import '../base/analyze_size.dart'; import '../base/common.dart'; import '../base/file_system.dart'; -import '../base/io.dart'; import '../base/logger.dart'; import '../base/process.dart'; import '../base/utils.dart'; @@ -108,10 +107,7 @@ Future _runCmakeGeneration(String cmakePath, Directory buildDir, Directory ], trace: true, ); - } on ProcessException catch (err) { - if (!isMissingExecutableException(err)) { - rethrow; - } + } on ArgumentError { throwToolExit("cmake not found. Run 'flutter doctor' for more information."); } if (result != 0) { @@ -148,10 +144,7 @@ Future _runBuild(String cmakePath, Directory buildDir, String buildModeNam trace: true, stdoutErrorMatcher: errorMatcher, ); - } on ProcessException catch (err) { - if (!isMissingExecutableException(err)) { - rethrow; - } + } on ArgumentError { throwToolExit("cmake not found. Run 'flutter doctor' for more information."); } if (result != 0) { diff --git a/packages/flutter_tools/lib/src/windows/visual_studio.dart b/packages/flutter_tools/lib/src/windows/visual_studio.dart index 1f2d9d850bb..31e99e41509 100644 --- a/packages/flutter_tools/lib/src/windows/visual_studio.dart +++ b/packages/flutter_tools/lib/src/windows/visual_studio.dart @@ -303,6 +303,8 @@ class VisualStudio { return installations[0]; } } + } on ArgumentError { + // Thrown if vswhere doesn't exist; ignore and return null below. } on ProcessException { // Ignored, return null below. } on FormatException { @@ -418,6 +420,8 @@ class VisualStudio { return match.group(1).trim(); } } + } on ArgumentError { + // Thrown if reg somehow doesn't exist; ignore and return null below. } on ProcessException { // Ignored, return null below. } diff --git a/packages/flutter_tools/test/commands.shard/hermetic/build_linux_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/build_linux_test.dart index 380b8fee0d0..2d1e8119d3b 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/build_linux_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/build_linux_test.dart @@ -6,7 +6,6 @@ import 'package:args/command_runner.dart'; import 'package:file/memory.dart'; import 'package:file_testing/file_testing.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/base/utils.dart'; import 'package:flutter_tools/src/cache.dart'; @@ -153,7 +152,7 @@ void main() { setUpMockProjectFilesForBuild(); processManager = FakeProcessManager.list([ cmakeCommand('release', onRun: () { - throw const ProcessException('cmake', [], '', 2); + throw ArgumentError(); }), ]); @@ -173,7 +172,7 @@ void main() { processManager = FakeProcessManager.list([ cmakeCommand('release'), ninjaCommand('release', onRun: () { - throw const ProcessException('ninja', [], '', 2); + throw ArgumentError(); }), ]); diff --git a/packages/flutter_tools/test/general.shard/base/error_handling_io_test.dart b/packages/flutter_tools/test/general.shard/base/error_handling_io_test.dart index b51b696567e..00df1a4b766 100644 --- a/packages/flutter_tools/test/general.shard/base/error_handling_io_test.dart +++ b/packages/flutter_tools/test/general.shard/base/error_handling_io_test.dart @@ -2,16 +2,12 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'dart:io' as io; // ignore: dart_io_import import 'package:file/file.dart'; -import 'package:file/memory.dart'; import 'package:flutter_tools/src/base/common.dart'; import 'package:flutter_tools/src/base/error_handling_io.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/platform.dart'; -import 'package:flutter_tools/src/convert.dart'; import 'package:mockito/mockito.dart'; import 'package:path/path.dart' as path; // ignore: package_path_import @@ -25,24 +21,17 @@ class MockDirectory extends Mock implements Directory {} final Platform windowsPlatform = FakePlatform( operatingSystem: 'windows', - environment: { - 'PATH': '', - 'PATHEXT': '', - } + environment: {} ); final Platform linuxPlatform = FakePlatform( operatingSystem: 'linux', - environment: { - 'PATH': '', - } + environment: {} ); final Platform macOSPlatform = FakePlatform( operatingSystem: 'macos', - environment: { - 'PATH': '', - } + environment: {} ); void setupWriteMocks({ @@ -642,13 +631,16 @@ void main() { const int kUserPermissionDenied = 5; test('when the device is full', () { - final ProcessManager processManager = setUpCrashingProcessManager( - windowsPlatform, - kDeviceFull, + final MockProcessManager mockProcessManager = MockProcessManager(); + final ProcessManager processManager = ErrorHandlingProcessManager( + delegate: mockProcessManager, + platform: windowsPlatform, ); + setupProcessManagerMocks(mockProcessManager, kDeviceFull); const String expectedMessage = 'The target device is full'; - + expect(() => processManager.canRun('foo'), + throwsToolExit(message: expectedMessage)); expect(() => processManager.killPid(1), throwsToolExit(message: expectedMessage)); expect(() async => await processManager.start(['foo']), @@ -660,13 +652,16 @@ void main() { }); test('when the file is being used by another program', () { - final ProcessManager processManager = setUpCrashingProcessManager( - windowsPlatform, - kUserMappedSectionOpened, + final MockProcessManager mockProcessManager = MockProcessManager(); + final ProcessManager processManager = ErrorHandlingProcessManager( + delegate: mockProcessManager, + platform: windowsPlatform, ); + setupProcessManagerMocks(mockProcessManager, kUserMappedSectionOpened); const String expectedMessage = 'The file is being used by another program'; - + expect(() => processManager.canRun('foo'), + throwsToolExit(message: expectedMessage)); expect(() => processManager.killPid(1), throwsToolExit(message: expectedMessage)); expect(() async => await processManager.start(['foo']), @@ -678,13 +673,16 @@ void main() { }); test('when permissions are denied', () { - final ProcessManager processManager = setUpCrashingProcessManager( - windowsPlatform, - kUserPermissionDenied, + final MockProcessManager mockProcessManager = MockProcessManager(); + final ProcessManager processManager = ErrorHandlingProcessManager( + delegate: mockProcessManager, + platform: windowsPlatform, ); + setupProcessManagerMocks(mockProcessManager, kUserPermissionDenied); const String expectedMessage = 'The flutter tool cannot access the file'; - + expect(() => processManager.canRun('foo'), + throwsToolExit(message: expectedMessage)); expect(() => processManager.killPid(1), throwsToolExit(message: expectedMessage)); expect(() async => await processManager.start(['foo']), @@ -701,13 +699,16 @@ void main() { const int eacces = 13; test('when writing to a full device', () { - final ProcessManager processManager = setUpCrashingProcessManager( - linuxPlatform, - enospc, + final MockProcessManager mockProcessManager = MockProcessManager(); + final ProcessManager processManager = ErrorHandlingProcessManager( + delegate: mockProcessManager, + platform: linuxPlatform, ); + setupProcessManagerMocks(mockProcessManager, enospc); const String expectedMessage = 'The target device is full'; - + expect(() => processManager.canRun('foo'), + throwsToolExit(message: expectedMessage)); expect(() => processManager.killPid(1), throwsToolExit(message: expectedMessage)); expect(() async => await processManager.start(['foo']), @@ -719,13 +720,16 @@ void main() { }); test('when permissions are denied', () { - final ProcessManager processManager = setUpCrashingProcessManager( - linuxPlatform, - eacces, + final MockProcessManager mockProcessManager = MockProcessManager(); + final ProcessManager processManager = ErrorHandlingProcessManager( + delegate: mockProcessManager, + platform: linuxPlatform, ); + setupProcessManagerMocks(mockProcessManager, eacces); const String expectedMessage = 'The flutter tool cannot access the file'; - + expect(() => processManager.canRun('foo'), + throwsToolExit(message: expectedMessage)); expect(() => processManager.killPid(1), throwsToolExit(message: expectedMessage)); expect(() async => await processManager.start(['foo']), @@ -742,13 +746,16 @@ void main() { const int eacces = 13; test('when writing to a full device', () { - final ProcessManager processManager = setUpCrashingProcessManager( - macOSPlatform, - enospc, + final MockProcessManager mockProcessManager = MockProcessManager(); + final ProcessManager processManager = ErrorHandlingProcessManager( + delegate: mockProcessManager, + platform: macOSPlatform, ); + setupProcessManagerMocks(mockProcessManager, enospc); const String expectedMessage = 'The target device is full'; - + expect(() => processManager.canRun('foo'), + throwsToolExit(message: expectedMessage)); expect(() => processManager.killPid(1), throwsToolExit(message: expectedMessage)); expect(() async => await processManager.start(['foo']), @@ -760,13 +767,16 @@ void main() { }); test('when permissions are denied', () { - final ProcessManager processManager = setUpCrashingProcessManager( - macOSPlatform, - eacces, + final MockProcessManager mockProcessManager = MockProcessManager(); + final ProcessManager processManager = ErrorHandlingProcessManager( + delegate: mockProcessManager, + platform: linuxPlatform, ); + setupProcessManagerMocks(mockProcessManager, eacces); const String expectedMessage = 'The flutter tool cannot access the file'; - + expect(() => processManager.canRun('foo'), + throwsToolExit(message: expectedMessage)); expect(() => processManager.killPid(1), throwsToolExit(message: expectedMessage)); expect(() async => await processManager.start(['foo']), @@ -777,345 +787,41 @@ void main() { throwsToolExit(message: expectedMessage)); }); }); - - testWithoutContext('Process manager uses which on Linux to resolve executables', () { - final FileSystem fileSystem = MemoryFileSystem.test(); - final ProcessManager processManager = setUpProcessManager( - linuxPlatform, - fileSystem, - (String executable, - List arguments, { - Map environment, - bool includeParentEnvironment, - bool runInShell, - Encoding stderrEncoding, - Encoding stdoutEncoding, - String workingDirectory, - }) { - if (executable == 'which') { - return ProcessResult(0, 0, 'fizz/foo\n', ''); - } - if (executable == 'fizz/foo') { - return ProcessResult(0, 0, '', ''); - } - throw ProcessException(executable, arguments, '', 2); - }, - ); - fileSystem.file('fizz/foo').createSync(recursive: true); - - final ProcessResult result = processManager.runSync(['foo']); - - expect(result.exitCode, 0); - }); - - testWithoutContext('Process manager uses which on macOS to resolve executables', () { - final FileSystem fileSystem = MemoryFileSystem.test(); - final ProcessManager processManager = setUpProcessManager( - macOSPlatform, - fileSystem, - (String executable, - List arguments, { - Map environment, - bool includeParentEnvironment, - bool runInShell, - Encoding stderrEncoding, - Encoding stdoutEncoding, - String workingDirectory, - }) { - if (executable == 'which') { - return ProcessResult(0, 0, 'fizz/foo\n', ''); - } - if (executable == 'fizz/foo') { - return ProcessResult(0, 0, '', ''); - } - throw ProcessException(executable, arguments, '', 2); - }, - ); - fileSystem.file('fizz/foo').createSync(recursive: true); - - final ProcessResult result = processManager.runSync(['foo']); - - expect(result.exitCode, 0); - }); - - testWithoutContext('Process manager uses where on Windows to resolve executables', () { - final FileSystem fileSystem = MemoryFileSystem.test(style: FileSystemStyle.windows); - final ProcessManager processManager = setUpProcessManager( - windowsPlatform, - fileSystem, - (String executable, - List arguments, { - Map environment, - bool includeParentEnvironment, - bool runInShell, - Encoding stderrEncoding, - Encoding stdoutEncoding, - String workingDirectory, - }) { - if (executable == 'where') { - return ProcessResult(0, 0, 'C:\\fizz\\foo.exe\n', ''); - } - if (executable == 'C:\\fizz\\foo.exe') { - return ProcessResult(0, 0, '', ''); - } - throw ProcessException(executable, arguments, '', 2); - }, - ); - - fileSystem.file('C:\\fizz\\foo.exe').createSync(recursive: true); - - final ProcessResult result = processManager.runSync(['foo']); - - expect(result.exitCode, 0); - }); - - testWithoutContext('Process manager will exit if where returns exit code 2 on Windows', () { - final FileSystem fileSystem = MemoryFileSystem.test(style: FileSystemStyle.windows); - final ProcessManager processManager = setUpProcessManager( - windowsPlatform, - fileSystem, - (String executable, - List arguments, { - Map environment, - bool includeParentEnvironment, - bool runInShell, - Encoding stderrEncoding, - Encoding stdoutEncoding, - String workingDirectory, - }) { - throw ProcessException(executable, arguments, '', 2); - }, - ); - - expect(() => processManager.runSync(['any']), throwsToolExit()); - }); - - testWithoutContext('Process manager will rethrow process exception if exit code on Linux', () { - final FileSystem fileSystem = MemoryFileSystem.test(); - final ProcessManager processManager = setUpProcessManager( - linuxPlatform, - fileSystem, - (String executable, - List arguments, { - Map environment, - bool includeParentEnvironment, - bool runInShell, - Encoding stderrEncoding, - Encoding stdoutEncoding, - String workingDirectory, - }) { - throw ProcessException(executable, arguments, '', 2); - }, - ); - - expect(() => processManager.runSync(['any']), throwsA(isA())); - }); - - testWithoutContext('Process manager will return first executable that exists', () { - final FileSystem fileSystem = MemoryFileSystem.test(); - final ProcessManager processManager = setUpProcessManager( - linuxPlatform, - fileSystem, - (String executable, - List arguments, { - Map environment, - bool includeParentEnvironment, - bool runInShell, - Encoding stderrEncoding, - Encoding stdoutEncoding, - String workingDirectory, - }) { - if (executable == 'which') { - return ProcessResult(0, 0, 'fizz/foo\nbar/foo\n', ''); - } - if (executable == 'bar/foo') { - return ProcessResult(0, 0, '', ''); - } - throw ProcessException(executable, arguments, '', 2); - }, - ); - fileSystem.file('bar/foo').createSync(recursive: true); - - final ProcessResult result = processManager.runSync(['foo']); - - expect(result.exitCode, 0); - }); - - testWithoutContext('Process manager will cache executable resolution', () { - int whichCalled = 0; - final FileSystem fileSystem = MemoryFileSystem.test(); - final ProcessManager processManager = setUpProcessManager( - linuxPlatform, - fileSystem, - (String executable, - List arguments, { - Map environment, - bool includeParentEnvironment, - bool runInShell, - Encoding stderrEncoding, - Encoding stdoutEncoding, - String workingDirectory, - }) { - if (executable == 'which') { - whichCalled += 1; - return ProcessResult(0, 0, 'fizz/foo\n', ''); - } - if (executable == 'fizz/foo') { - return ProcessResult(0, 0, '', ''); - } - throw ProcessException(executable, arguments, '', 2); - }, - ); - fileSystem.file('fizz/foo').createSync(recursive: true); - - processManager.runSync(['foo']); - processManager.runSync(['foo']); - - expect(whichCalled, 1); - }); - - testWithoutContext('Process manager will not cache executable resolution if the process fails', () { - int whichCalled = 0; - final FileSystem fileSystem = MemoryFileSystem.test(); - final ProcessManager processManager = setUpProcessManager( - linuxPlatform, - fileSystem, - (String executable, - List arguments, { - Map environment, - bool includeParentEnvironment, - bool runInShell, - Encoding stderrEncoding, - Encoding stdoutEncoding, - String workingDirectory, - }) { - if (executable == 'which') { - whichCalled += 1; - return ProcessResult(0, 0, '', ''); - } - if (executable == 'foo') { - return ProcessResult(0, 0, '', ''); - } - throw ProcessException(executable, arguments, '', 2); - }, - ); - - processManager.runSync(['foo']); - processManager.runSync(['foo']); - - expect(whichCalled, 2); - }); - - testWithoutContext('Process manager can run will return false if the executable does not exist', () { - final FileSystem fileSystem = MemoryFileSystem.test(); - final ProcessManager processManager = setUpProcessManager( - linuxPlatform, - fileSystem, - (String executable, - List arguments, { - Map environment, - bool includeParentEnvironment, - bool runInShell, - Encoding stderrEncoding, - Encoding stdoutEncoding, - String workingDirectory, - }) { - if (executable == 'which') { - return ProcessResult(0, 0, 'bar/foo\n', ''); - } - throw ProcessException(executable, arguments, '', 2); - }, - ); - - expect(processManager.canRun('foo'), false); - }); - - testWithoutContext('Process manager can run will return true if the executable does exist', () { - final FileSystem fileSystem = MemoryFileSystem.test(); - fileSystem.file('bar/foo').createSync(recursive: true); - final ProcessManager processManager = setUpProcessManager( - linuxPlatform, - fileSystem, - (String executable, - List arguments, { - Map environment, - bool includeParentEnvironment, - bool runInShell, - Encoding stderrEncoding, - Encoding stdoutEncoding, - String workingDirectory, - }) { - if (executable == 'which') { - return ProcessResult(0, 0, 'bar/foo\n', ''); - } - throw ProcessException(executable, arguments, '', 2); - }, - ); - - expect(processManager.canRun('foo'), true); - }); } -ProcessManager setUpProcessManager( - Platform platform, - FileSystem fileSystem, - ProcessRunSync processRunSync, +void setupProcessManagerMocks( + MockProcessManager processManager, + int errorCode, ) { - return ErrorHandlingProcessManager( - fileSystem: fileSystem, - logger: BufferLogger.test(), - platform: platform, - runSync: processRunSync, - ); + when(processManager.canRun(any, workingDirectory: anyNamed('workingDirectory'))) + .thenThrow(ProcessException('', [], '', errorCode)); + when(processManager.killPid(any, any)) + .thenThrow(ProcessException('', [], '', errorCode)); + when(processManager.runSync( + any, + environment: anyNamed('environment'), + includeParentEnvironment: anyNamed('includeParentEnvironment'), + runInShell: anyNamed('runInShell'), + workingDirectory: anyNamed('workingDirectory'), + stdoutEncoding: anyNamed('stdoutEncoding'), + stderrEncoding: anyNamed('stderrEncoding'), + )).thenThrow(ProcessException('', [], '', errorCode)); + when(processManager.run( + any, + environment: anyNamed('environment'), + includeParentEnvironment: anyNamed('includeParentEnvironment'), + runInShell: anyNamed('runInShell'), + workingDirectory: anyNamed('workingDirectory'), + stdoutEncoding: anyNamed('stdoutEncoding'), + stderrEncoding: anyNamed('stderrEncoding'), + )).thenThrow(ProcessException('', [], '', errorCode)); + when(processManager.start( + any, + environment: anyNamed('environment'), + includeParentEnvironment: anyNamed('includeParentEnvironment'), + runInShell: anyNamed('runInShell'), + workingDirectory: anyNamed('workingDirectory'), + )).thenThrow(ProcessException('', [], '', errorCode)); } -ProcessManager setUpCrashingProcessManager( - Platform platform, - int osError, -) { - return ErrorHandlingProcessManager( - fileSystem: MemoryFileSystem.test( - style: platform.isWindows ? FileSystemStyle.windows : FileSystemStyle.posix, - ), - logger: BufferLogger.test(), - platform: platform, - killPid: (int pid, [io.ProcessSignal signal]) { - throw ProcessException('executable', [], '', osError); - }, - run: ( - String executable, - List arguments, { - Map environment, - bool includeParentEnvironment, - bool runInShell, - Encoding stderrEncoding, - Encoding stdoutEncoding, - String workingDirectory, - }) { - throw ProcessException(executable, arguments, '', osError); - }, - runSync: ( - String executable, - List arguments, { - Map environment, - bool includeParentEnvironment, - bool runInShell, - Encoding stderrEncoding, - Encoding stdoutEncoding, - String workingDirectory, - }) { - throw ProcessException(executable, arguments, '', osError); - }, - start: ( - String executable, - List arguments, { - Map environment, - bool includeParentEnvironment, - bool runInShell, - String workingDirectory, - ProcessStartMode mode, - }) { - throw ProcessException(executable, arguments, '', osError); - }, - ); -} +class MockProcessManager extends Mock implements ProcessManager {} diff --git a/packages/flutter_tools/test/general.shard/base/os_test.dart b/packages/flutter_tools/test/general.shard/base/os_test.dart index 0ef21f3709f..d2e77daefee 100644 --- a/packages/flutter_tools/test/general.shard/base/os_test.dart +++ b/packages/flutter_tools/test/general.shard/base/os_test.dart @@ -63,9 +63,9 @@ void main() { }); group('which on Windows', () { - testWithoutContext('throws tool exit if where throws a process exception with code 2', () async { + testWithoutContext('throws tool exit if where throws an argument error', () async { when(mockProcessManager.runSync(['where', kExecutable])) - .thenThrow(const ProcessException('where', [], 'Cannot find executable for where', 2)); + .thenThrow(ArgumentError('Cannot find executable for where')); final OperatingSystemUtils utils = createOSUtils(FakePlatform(operatingSystem: 'windows')); expect(() => utils.which(kExecutable), throwsA(isA())); @@ -143,11 +143,11 @@ void main() { ); }); - testWithoutContext('If unzip throws a ProcessException with code 2, display an install message', () { + testWithoutContext('If unzip throws an ArgumentError, display an install message', () { final FileSystem fileSystem = MemoryFileSystem.test(); when(mockProcessManager.runSync( ['unzip', '-o', '-q', 'foo.zip', '-d', fileSystem.currentDirectory.path], - )).thenThrow(const ProcessException('unzip', ['-o', '-q', 'foo.zip', '-d'], '', 2)); + )).thenThrow(ArgumentError()); final OperatingSystemUtils linuxOsUtils = OperatingSystemUtils( fileSystem: fileSystem, diff --git a/packages/flutter_tools/test/general.shard/macos/xcode_test.dart b/packages/flutter_tools/test/general.shard/macos/xcode_test.dart index d45b915f16c..5d13b984782 100644 --- a/packages/flutter_tools/test/general.shard/macos/xcode_test.dart +++ b/packages/flutter_tools/test/general.shard/macos/xcode_test.dart @@ -55,7 +55,7 @@ void main() { .thenThrow(const ProcessException('/usr/bin/xcode-select', ['--print-path'])); expect(xcode.xcodeSelectPath, isNull); when(processManager.runSync(['/usr/bin/xcode-select', '--print-path'])) - .thenThrow(const ProcessException('/usr/bin/xcode-select', ['--print-path'], '', 2)); + .thenThrow(ArgumentError('Invalid argument(s): Cannot find executable for /usr/bin/xcode-select')); expect(xcode.xcodeSelectPath, isNull); });