From 27dd48f8d366aa54a2f52d332ee43470eeccb255 Mon Sep 17 00:00:00 2001 From: Bryan Oltman Date: Mon, 16 Oct 2023 16:52:33 -0400 Subject: [PATCH] feat(shorebird_cli): Add OS wrapper for which/where.exe (#1403) --- packages/shorebird_cli/bin/shorebird.dart | 2 + .../src/os/operating_system_interface.dart | 65 ++++ packages/shorebird_cli/lib/src/os/os.dart | 1 + packages/shorebird_cli/lib/src/process.dart | 80 ++++- .../os/operating_system_interface_test.dart | 143 +++++++++ .../test/src/shorebird_process_test.dart | 293 +++++++++++++----- 6 files changed, 494 insertions(+), 90 deletions(-) create mode 100644 packages/shorebird_cli/lib/src/os/operating_system_interface.dart create mode 100644 packages/shorebird_cli/lib/src/os/os.dart create mode 100644 packages/shorebird_cli/test/src/os/operating_system_interface_test.dart diff --git a/packages/shorebird_cli/bin/shorebird.dart b/packages/shorebird_cli/bin/shorebird.dart index de7ddd70..2fdcd7cc 100644 --- a/packages/shorebird_cli/bin/shorebird.dart +++ b/packages/shorebird_cli/bin/shorebird.dart @@ -11,6 +11,7 @@ import 'package:shorebird_cli/src/command_runner.dart'; import 'package:shorebird_cli/src/doctor.dart'; import 'package:shorebird_cli/src/executables/executables.dart'; import 'package:shorebird_cli/src/logger.dart'; +import 'package:shorebird_cli/src/os/os.dart'; import 'package:shorebird_cli/src/patch_diff_checker.dart'; import 'package:shorebird_cli/src/platform.dart'; import 'package:shorebird_cli/src/process.dart'; @@ -40,6 +41,7 @@ Future main(List args) async { iosDeployRef, javaRef, loggerRef, + osInterfaceRef, patchDiffCheckerRef, platformRef, processRef, diff --git a/packages/shorebird_cli/lib/src/os/operating_system_interface.dart b/packages/shorebird_cli/lib/src/os/operating_system_interface.dart new file mode 100644 index 00000000..395aa544 --- /dev/null +++ b/packages/shorebird_cli/lib/src/os/operating_system_interface.dart @@ -0,0 +1,65 @@ +import 'dart:io'; + +import 'package:mason_logger/mason_logger.dart'; +import 'package:scoped/scoped.dart'; +import 'package:shorebird_cli/src/platform.dart'; +import 'package:shorebird_cli/src/process.dart'; + +// TODO(bryanoltman): remove this once os is used. +// coverage:ignore-start +/// A reference to a [OperatingSystemInterface] instance. +final osInterfaceRef = create(OperatingSystemInterface.new); + +/// The [OperatingSystemInterface] instance available in the current zone. +OperatingSystemInterface get osInterface => read(osInterfaceRef); +// coverage:ignore-end + +/// {@template operating_system_interface} +/// A wrapper around operating system specific functionality. +/// {@endtemplate} +abstract class OperatingSystemInterface { + /// {@macro operating_system_interface} + factory OperatingSystemInterface() { + if (platform.isWindows) { + return _WindowsOperatingSystemInterface(); + } else if (platform.isMacOS || platform.isLinux) { + return _PosixOperatingSystemInterface(); + } + + throw UnsupportedError( + 'Unsupported operating system: ${Platform.operatingSystem}', + ); + } + + /// Returns the first instance of [executableName] found on the PATH. + /// + /// This is the equivalent of the `which` command on Linux and macOS and + /// `where.exe` on Windows. + String? which(String executableName); +} + +class _PosixOperatingSystemInterface implements OperatingSystemInterface { + @override + String? which(String executableName) { + final result = process.runSync('which', [executableName]); + if (result.exitCode != ExitCode.success.code) { + return null; + } + + return result.stdout as String?; + } +} + +class _WindowsOperatingSystemInterface implements OperatingSystemInterface { + @override + String? which(String executableName) { + final result = process.runSync('where.exe', [executableName]); + if (result.exitCode != ExitCode.success.code) { + return null; + } + + // By default, where.exe will list all matching executables on PATH. We want + // to return the first one. + return (result.stdout as String).split('\n').firstOrNull; + } +} diff --git a/packages/shorebird_cli/lib/src/os/os.dart b/packages/shorebird_cli/lib/src/os/os.dart new file mode 100644 index 00000000..da1ce0bf --- /dev/null +++ b/packages/shorebird_cli/lib/src/os/os.dart @@ -0,0 +1 @@ +export 'operating_system_interface.dart'; diff --git a/packages/shorebird_cli/lib/src/process.dart b/packages/shorebird_cli/lib/src/process.dart index 5e1cd72b..aca38da0 100644 --- a/packages/shorebird_cli/lib/src/process.dart +++ b/packages/shorebird_cli/lib/src/process.dart @@ -55,14 +55,11 @@ class ShorebirdProcess { String? workingDirectory, bool useVendedFlutter = true, }) { - final resolvedEnvironment = environment ?? {}; - if (useVendedFlutter) { - // Note: this will overwrite existing environment values. - resolvedEnvironment.addAll( - _environmentOverrides(executable: executable), - ); - } - + final resolvedEnvironment = _resolveEnvironment( + environment, + executable: executable, + useVendedFlutter: useVendedFlutter, + ); final resolvedExecutable = useVendedFlutter ? _resolveExecutable(executable) : executable; final resolvedArguments = @@ -80,6 +77,36 @@ class ShorebirdProcess { ); } + ShorebirdProcessResult runSync( + String executable, + List arguments, { + bool runInShell = false, + Map? environment, + String? workingDirectory, + bool useVendedFlutter = true, + }) { + final resolvedEnvironment = _resolveEnvironment( + environment, + executable: executable, + useVendedFlutter: useVendedFlutter, + ); + final resolvedExecutable = + useVendedFlutter ? _resolveExecutable(executable) : executable; + final resolvedArguments = + useVendedFlutter ? _resolveArguments(executable, arguments) : arguments; + logger.detail( + '''[Process.runSync] $resolvedExecutable ${resolvedArguments.join(' ')}${workingDirectory == null ? '' : ' (in $workingDirectory)'}''', + ); + + return processWrapper.runSync( + resolvedExecutable, + resolvedArguments, + runInShell: runInShell, + workingDirectory: workingDirectory, + environment: resolvedEnvironment, + ); + } + Future start( String executable, List arguments, { @@ -110,6 +137,22 @@ class ShorebirdProcess { ); } + Map _resolveEnvironment( + Map? baseEnvironment, { + required String executable, + required bool useVendedFlutter, + }) { + final resolvedEnvironment = baseEnvironment ?? {}; + if (useVendedFlutter) { + // Note: this will overwrite existing environment values. + resolvedEnvironment.addAll( + _environmentOverrides(executable: executable), + ); + } + + return resolvedEnvironment; + } + String _resolveExecutable(String executable) { if (executable == 'flutter') return shorebirdEnv.flutterBinaryFile.path; return executable; @@ -178,6 +221,27 @@ class ProcessWrapper { ); } + ShorebirdProcessResult runSync( + String executable, + List arguments, { + bool runInShell = false, + Map? environment, + String? workingDirectory, + }) { + final result = Process.runSync( + executable, + arguments, + environment: environment, + runInShell: runInShell, + workingDirectory: workingDirectory, + ); + return ShorebirdProcessResult( + exitCode: result.exitCode, + stdout: result.stdout, + stderr: result.stderr, + ); + } + Future start( String executable, List arguments, { diff --git a/packages/shorebird_cli/test/src/os/operating_system_interface_test.dart b/packages/shorebird_cli/test/src/os/operating_system_interface_test.dart new file mode 100644 index 00000000..83bcb151 --- /dev/null +++ b/packages/shorebird_cli/test/src/os/operating_system_interface_test.dart @@ -0,0 +1,143 @@ +import 'package:mason_logger/mason_logger.dart'; +import 'package:mocktail/mocktail.dart'; +import 'package:platform/platform.dart'; +import 'package:scoped/scoped.dart'; +import 'package:shorebird_cli/src/os/os.dart'; +import 'package:shorebird_cli/src/platform.dart'; +import 'package:shorebird_cli/src/process.dart'; +import 'package:test/test.dart'; + +import '../mocks.dart'; + +void main() { + group(OperatingSystemInterface, () { + late Platform platform; + late ShorebirdProcess process; + late ShorebirdProcessResult processResult; + late OperatingSystemInterface osInterface; + + R runWithOverrides(R Function() body) { + return runScoped( + () => body(), + values: { + platformRef.overrideWith(() => platform), + processRef.overrideWith(() => process), + }, + ); + } + + setUp(() { + platform = MockPlatform(); + process = MockShorebirdProcess(); + processResult = MockProcessResult(); + + when(() => platform.isLinux).thenReturn(false); + when(() => platform.isMacOS).thenReturn(false); + when(() => platform.isWindows).thenReturn(false); + + when(() => process.runSync(any(), any())).thenReturn(processResult); + when(() => processResult.exitCode).thenReturn(ExitCode.success.code); + }); + + group('init', () { + test('throws UnsupportedError when operating system is not supported', + () { + expect( + () => runWithOverrides(OperatingSystemInterface.new), + throwsUnsupportedError, + ); + }); + }); + + group('on macOS/Linux', () { + setUp(() { + when(() => platform.isMacOS).thenReturn(true); + + osInterface = runWithOverrides(OperatingSystemInterface.new); + }); + + group('which()', () { + group('when no executable is found on PATH', () { + setUp(() { + when(() => processResult.exitCode).thenReturn(1); + }); + + test('returns null', () { + expect( + runWithOverrides(() => osInterface.which('shorebird')), + isNull, + ); + }); + }); + + group('when executable is found on PATH', () { + const shorebirdPath = '/path/to/shorebird'; + setUp(() { + when(() => processResult.stdout).thenReturn(shorebirdPath); + }); + + test('returns path to executable', () { + expect( + runWithOverrides(() => osInterface.which('shorebird')), + shorebirdPath, + ); + }); + }); + }); + }); + + group('on Windows', () { + setUp(() { + when(() => platform.isWindows).thenReturn(true); + osInterface = runWithOverrides(OperatingSystemInterface.new); + }); + + group('which()', () { + group('when no executable is found on PATH', () { + setUp(() { + when(() => processResult.exitCode).thenReturn(1); + }); + + test('returns null', () { + expect( + runWithOverrides(() => osInterface.which('shorebird')), isNull); + }); + }); + + group('when executable is found on PATH', () { + const shorebirdPath = r'C:\path\to\shorebird'; + setUp(() { + when(() => processResult.stdout).thenReturn(shorebirdPath); + }); + + test('returns path to executable', () { + expect( + runWithOverrides(() => osInterface.which('shorebird')), + shorebirdPath, + ); + }); + }); + + group('when multiple executables are found on PATH', () { + const shorebirdPath = r'C:\path\to\shorebird'; + const shorebirdPaths = r''' +C:\path\to\shorebird +C:\path\to\shorebird1 +C:\path\to\shorebird2 +C:\path\to\shorebird3'''; + + setUp(() { + when(() => processResult.stdout).thenReturn(shorebirdPaths); + }); + + test('returns first path to executable', () { + expect( + runWithOverrides(() => osInterface.which('shorebird')), + shorebirdPath, + ); + }); + }); + }); + }); + }); +} diff --git a/packages/shorebird_cli/test/src/shorebird_process_test.dart b/packages/shorebird_cli/test/src/shorebird_process_test.dart index ddb045ad..06dbd2bb 100644 --- a/packages/shorebird_cli/test/src/shorebird_process_test.dart +++ b/packages/shorebird_cli/test/src/shorebird_process_test.dart @@ -42,24 +42,6 @@ void main() { when( () => shorebirdEnv.flutterBinaryFile, ).thenReturn(File(p.join('bin', 'cache', 'flutter', 'bin', 'flutter'))); - when( - () => processWrapper.run( - any(), - any(), - runInShell: any(named: 'runInShell'), - environment: any(named: 'environment'), - workingDirectory: any(named: 'workingDirectory'), - ), - ).thenAnswer((_) async => runProcessResult); - - when( - () => processWrapper.start( - any(), - any(), - environment: any(named: 'environment'), - runInShell: any(named: 'runInShell'), - ), - ).thenAnswer((_) async => startProcess); }); test('ShorebirdProcessResult can be instantiated as a const', () { @@ -70,6 +52,18 @@ void main() { }); group('run', () { + setUp(() { + when( + () => processWrapper.run( + any(), + any(), + runInShell: any(named: 'runInShell'), + environment: any(named: 'environment'), + workingDirectory: any(named: 'workingDirectory'), + ), + ).thenAnswer((_) async => runProcessResult); + }); + test('forwards non-flutter executables to Process.run', () async { await shorebirdProcess.run( 'git', @@ -180,35 +174,171 @@ void main() { ).called(1); }, ); + + test('adds local-engine arguments if set', () async { + final localEngineSrcPath = p.join('path', 'to', 'engine', 'src'); + shorebirdProcess = ShorebirdProcess( + processWrapper: processWrapper, + engineConfig: EngineConfig( + localEngineSrcPath: localEngineSrcPath, + localEngine: 'android_release_arm64', + ), + ); + + await runWithOverrides(() => shorebirdProcess.run('flutter', [])); + + verify( + () => processWrapper.run( + any(), + [ + '--local-engine-src-path=$localEngineSrcPath', + '--local-engine=android_release_arm64', + ], + runInShell: any(named: 'runInShell'), + environment: any(named: 'environment'), + workingDirectory: any(named: 'workingDirectory'), + ), + ).called(1); + }); }); - test('adds local-engine arguments if set', () async { - final localEngineSrcPath = p.join('path', 'to', 'engine', 'src'); - shorebirdProcess = ShorebirdProcess( - processWrapper: processWrapper, - engineConfig: EngineConfig( - localEngineSrcPath: localEngineSrcPath, - localEngine: 'android_release_arm64', - ), + group('runSync', () { + setUp(() { + when( + () => processWrapper.runSync( + any(), + any(), + runInShell: any(named: 'runInShell'), + environment: any(named: 'environment'), + workingDirectory: any(named: 'workingDirectory'), + ), + ).thenReturn(runProcessResult); + }); + + test('forwards non-flutter executables to Process.runSync', () async { + shorebirdProcess.runSync( + 'git', + ['pull'], + runInShell: true, + workingDirectory: '~', + ); + + verify( + () => processWrapper.runSync( + 'git', + ['pull'], + runInShell: true, + environment: {}, + workingDirectory: '~', + ), + ).called(1); + }); + + test('replaces "flutter" with our local flutter', () { + runWithOverrides( + () => shorebirdProcess.runSync( + 'flutter', + ['--version'], + runInShell: true, + workingDirectory: '~', + ), + ); + + verify( + () => processWrapper.runSync( + any( + that: contains( + p.join('bin', 'cache', 'flutter', 'bin', 'flutter'), + ), + ), + ['--version'], + runInShell: true, + environment: flutterStorageBaseUrlEnv, + workingDirectory: '~', + ), + ).called(1); + }); + + test( + '''does not replace flutter with our local flutter if useVendedFlutter is false''', + () { + shorebirdProcess.runSync( + 'flutter', + ['--version'], + runInShell: true, + workingDirectory: '~', + useVendedFlutter: false, + ); + + verify( + () => processWrapper.runSync( + 'flutter', + ['--version'], + runInShell: true, + environment: {}, + workingDirectory: '~', + ), + ).called(1); + }); + + test('Updates environment if useVendedFlutter is true', () { + shorebirdProcess.runSync( + 'flutter', + ['--version'], + runInShell: true, + workingDirectory: '~', + useVendedFlutter: false, + environment: {'ENV_VAR': 'asdfasdf'}, + ); + + verify( + () => processWrapper.runSync( + 'flutter', + ['--version'], + runInShell: true, + workingDirectory: '~', + environment: {'ENV_VAR': 'asdfasdf'}, + ), + ).called(1); + }); + + test( + 'Makes no changes to environment if useVendedFlutter is false', + () { + shorebirdProcess.runSync( + 'flutter', + ['--version'], + runInShell: true, + workingDirectory: '~', + useVendedFlutter: false, + environment: {'ENV_VAR': 'asdfasdf'}, + ); + + verify( + () => processWrapper.runSync( + 'flutter', + ['--version'], + runInShell: true, + workingDirectory: '~', + environment: {'ENV_VAR': 'asdfasdf'}, + ), + ).called(1); + }, ); - - await runWithOverrides(() => shorebirdProcess.run('flutter', [])); - - verify( - () => processWrapper.run( - any(), - [ - '--local-engine-src-path=$localEngineSrcPath', - '--local-engine=android_release_arm64', - ], - runInShell: any(named: 'runInShell'), - environment: any(named: 'environment'), - workingDirectory: any(named: 'workingDirectory'), - ), - ).called(1); }); group('start', () { + setUp(() { + when( + () => processWrapper.start( + any(), + any(), + environment: any(named: 'environment'), + runInShell: any(named: 'runInShell'), + ), + ).thenAnswer((_) async => startProcess); + }); + test('forwards non-flutter executables to Process.run', () async { await shorebirdProcess.start('git', ['pull'], runInShell: true); @@ -260,55 +390,54 @@ void main() { ), ).called(1); }); - }); - - test('Updates environment if useVendedFlutter is true', () async { - await runWithOverrides( - () => shorebirdProcess.start( - 'flutter', - ['--version'], - runInShell: true, - environment: {'ENV_VAR': 'asdfasdf'}, - ), - ); - - verify( - () => processWrapper.start( - any( - that: contains( - p.join('bin', 'cache', 'flutter', 'bin', 'flutter'), - ), + test('Updates environment if useVendedFlutter is true', () async { + await runWithOverrides( + () => shorebirdProcess.start( + 'flutter', + ['--version'], + runInShell: true, + environment: {'ENV_VAR': 'asdfasdf'}, ), - ['--version'], - runInShell: true, - environment: { - 'ENV_VAR': 'asdfasdf', - ...flutterStorageBaseUrlEnv, - }, - ), - ).called(1); - }); - - test( - 'Makes no changes to environment if useVendedFlutter is false', - () async { - await shorebirdProcess.start( - 'flutter', - ['--version'], - runInShell: true, - useVendedFlutter: false, - environment: {'hello': 'world'}, ); verify( () => processWrapper.start( + any( + that: contains( + p.join('bin', 'cache', 'flutter', 'bin', 'flutter'), + ), + ), + ['--version'], + runInShell: true, + environment: { + 'ENV_VAR': 'asdfasdf', + ...flutterStorageBaseUrlEnv, + }, + ), + ).called(1); + }); + + test( + 'Makes no changes to environment if useVendedFlutter is false', + () async { + await shorebirdProcess.start( 'flutter', ['--version'], runInShell: true, + useVendedFlutter: false, environment: {'hello': 'world'}, - ), - ).called(1); - }, - ); + ); + + verify( + () => processWrapper.start( + 'flutter', + ['--version'], + runInShell: true, + environment: {'hello': 'world'}, + ), + ).called(1); + }, + ); + }); }); }