Unverified Commit fb5632dc authored by Jonah Williams's avatar Jonah Williams Committed by GitHub

[flutter_tools] Only remove ProcessUtils from injection (#48975)

parent d0161464
...@@ -13,7 +13,6 @@ import 'package:flutter_tools/src/android/android_sdk.dart'; ...@@ -13,7 +13,6 @@ import 'package:flutter_tools/src/android/android_sdk.dart';
import 'package:flutter_tools/src/application_package.dart'; import 'package:flutter_tools/src/application_package.dart';
import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/io.dart'; import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/base/process.dart';
import 'package:flutter_tools/src/build_info.dart'; import 'package:flutter_tools/src/build_info.dart';
import 'package:flutter_tools/src/device.dart'; import 'package:flutter_tools/src/device.dart';
import 'package:flutter_tools/src/project.dart'; import 'package:flutter_tools/src/project.dart';
...@@ -44,45 +43,6 @@ class MockAndroidApk extends Mock implements AndroidApk { ...@@ -44,45 +43,6 @@ class MockAndroidApk extends Mock implements AndroidApk {
File get file => MockFile(); File get file => MockFile();
} }
class MockProcessUtils extends Mock implements ProcessUtils {
@override
Future<RunResult> run(
List<String> cmd, {
bool throwOnError = false,
RunResultChecker whiteListFailures,
String workingDirectory,
bool allowReentrantFlutter = false,
Map<String, String> environment,
Duration timeout,
int timeoutRetries = 0,
}) async {
if (cmd.contains('version')) {
return RunResult(ProcessResult(0, 0, 'Android Debug Bridge version 1.0.41', ''), cmd);
}
if (cmd.contains('android.intent.action.RUN')) {
_runCmd = cmd;
}
return RunResult(ProcessResult(0, 0, '', ''), cmd);
}
@override
Future<int> stream(
List<String> cmd, {
String workingDirectory,
bool allowReentrantFlutter = false,
String prefix = '',
bool trace = false,
RegExp filter,
StringConverter mapFunction,
Map<String, String> environment,
}) async {
return 0;
}
List<String> _runCmd;
List<String> get runCmd => _runCmd;
}
class MockAndroidSdkVersion extends Mock implements AndroidSdkVersion {} class MockAndroidSdkVersion extends Mock implements AndroidSdkVersion {}
void main() { void main() {
...@@ -97,13 +57,11 @@ void main() { ...@@ -97,13 +57,11 @@ void main() {
MockAndroidApk mockApk; MockAndroidApk mockApk;
MockProcessManager mockProcessManager; MockProcessManager mockProcessManager;
MockAndroidSdk mockAndroidSdk; MockAndroidSdk mockAndroidSdk;
MockProcessUtils mockProcessUtils;
setUp(() { setUp(() {
mockApk = MockAndroidApk(); mockApk = MockAndroidApk();
mockProcessManager = MockProcessManager(); mockProcessManager = MockProcessManager();
mockAndroidSdk = MockAndroidSdk(); mockAndroidSdk = MockAndroidSdk();
mockProcessUtils = MockProcessUtils();
}); });
testUsingContext('succeeds with --cache-sksl', () async { testUsingContext('succeeds with --cache-sksl', () async {
...@@ -124,6 +82,20 @@ void main() { ...@@ -124,6 +82,20 @@ void main() {
)).thenAnswer((_) async { )).thenAnswer((_) async {
return ProcessResult(0, 0, '[ro.build.version.sdk]: [24]', ''); return ProcessResult(0, 0, '[ro.build.version.sdk]: [24]', '');
}); });
when(mockProcessManager.run(
any,
workingDirectory: anyNamed('workingDirectory'),
environment: anyNamed('environment')
)).thenAnswer((_) async {
return ProcessResult(0, 0, '', '');
});
when(mockProcessManager.start(
any,
workingDirectory: anyNamed('workingDirectory'),
environment: anyNamed('environment')
)).thenAnswer((_) async {
return FakeProcess();
});
final LaunchResult launchResult = await device.startApp( final LaunchResult launchResult = await device.startApp(
mockApk, mockApk,
...@@ -134,18 +106,14 @@ void main() { ...@@ -134,18 +106,14 @@ void main() {
), ),
platformArgs: <String, dynamic>{}, platformArgs: <String, dynamic>{},
); );
expect(launchResult.started, isTrue);
final int cmdIndex = mockProcessUtils.runCmd.indexOf('cache-sksl'); expect(launchResult.started, isTrue);
expect( expect(verify(mockProcessManager.run(captureAny)).captured.last.join(','),
mockProcessUtils.runCmd.sublist(cmdIndex - 1, cmdIndex + 2), contains(<String>['--ez', 'cache-sksl', 'true'].join(',')));
equals(<String>['--ez', 'cache-sksl', 'true']),
);
}, overrides: <Type, Generator>{ }, overrides: <Type, Generator>{
AndroidSdk: () => mockAndroidSdk, AndroidSdk: () => mockAndroidSdk,
FileSystem: () => MemoryFileSystem(), FileSystem: () => MemoryFileSystem(),
ProcessManager: () => mockProcessManager, ProcessManager: () => mockProcessManager,
ProcessUtils: () => mockProcessUtils,
}); });
testUsingContext('can run a release build on x64', () async { testUsingContext('can run a release build on x64', () async {
...@@ -166,6 +134,20 @@ void main() { ...@@ -166,6 +134,20 @@ void main() {
)).thenAnswer((_) async { )).thenAnswer((_) async {
return ProcessResult(0, 0, '[ro.build.version.sdk]: [24]\n[ro.product.cpu.abi]: [x86_64]', ''); return ProcessResult(0, 0, '[ro.build.version.sdk]: [24]\n[ro.product.cpu.abi]: [x86_64]', '');
}); });
when(mockProcessManager.run(
any,
workingDirectory: anyNamed('workingDirectory'),
environment: anyNamed('environment')
)).thenAnswer((_) async {
return ProcessResult(0, 0, '', '');
});
when(mockProcessManager.start(
any,
workingDirectory: anyNamed('workingDirectory'),
environment: anyNamed('environment')
)).thenAnswer((_) async {
return FakeProcess();
});
final LaunchResult launchResult = await device.startApp( final LaunchResult launchResult = await device.startApp(
mockApk, mockApk,
...@@ -180,7 +162,6 @@ void main() { ...@@ -180,7 +162,6 @@ void main() {
AndroidSdk: () => mockAndroidSdk, AndroidSdk: () => mockAndroidSdk,
FileSystem: () => MemoryFileSystem(), FileSystem: () => MemoryFileSystem(),
ProcessManager: () => mockProcessManager, ProcessManager: () => mockProcessManager,
ProcessUtils: () => mockProcessUtils,
}); });
}); });
}); });
......
...@@ -12,7 +12,6 @@ import 'package:flutter_tools/src/base/common.dart'; ...@@ -12,7 +12,6 @@ import 'package:flutter_tools/src/base/common.dart';
import 'package:flutter_tools/src/base/context.dart'; import 'package:flutter_tools/src/base/context.dart';
import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/io.dart'; import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/base/process.dart';
import 'package:flutter_tools/src/base/os.dart'; import 'package:flutter_tools/src/base/os.dart';
import 'package:flutter_tools/src/base/time.dart'; import 'package:flutter_tools/src/base/time.dart';
import 'package:flutter_tools/src/build_info.dart'; import 'package:flutter_tools/src/build_info.dart';
...@@ -30,8 +29,8 @@ import 'package:flutter_tools/src/project.dart'; ...@@ -30,8 +29,8 @@ import 'package:flutter_tools/src/project.dart';
import 'package:flutter_tools/src/vmservice.dart'; import 'package:flutter_tools/src/vmservice.dart';
import 'package:meta/meta.dart'; import 'package:meta/meta.dart';
import 'package:mockito/mockito.dart'; import 'package:mockito/mockito.dart';
import 'package:process/process.dart';
import 'package:platform/platform.dart'; import 'package:platform/platform.dart';
import 'package:process/process.dart';
import '../../src/common.dart'; import '../../src/common.dart';
import '../../src/context.dart'; import '../../src/context.dart';
...@@ -40,11 +39,10 @@ void main() { ...@@ -40,11 +39,10 @@ void main() {
group('fuchsia device', () { group('fuchsia device', () {
MemoryFileSystem memoryFileSystem; MemoryFileSystem memoryFileSystem;
MockFile sshConfig; MockFile sshConfig;
MockProcessUtils mockProcessUtils;
setUp(() { setUp(() {
memoryFileSystem = MemoryFileSystem(); memoryFileSystem = MemoryFileSystem();
sshConfig = MockFile(); sshConfig = MockFile();
mockProcessUtils = MockProcessUtils();
when(sshConfig.absolute).thenReturn(sshConfig); when(sshConfig.absolute).thenReturn(sshConfig);
}); });
...@@ -118,27 +116,27 @@ void main() { ...@@ -118,27 +116,27 @@ void main() {
}); });
testUsingContext('targetPlatform arm64 works', () async { testUsingContext('targetPlatform arm64 works', () async {
when(mockProcessUtils.run(any)).thenAnswer((Invocation _) { when(globals.processManager.run(any)).thenAnswer((Invocation _) async {
return Future<RunResult>.value(RunResult(ProcessResult(1, 0, 'aarch64', ''), <String>[''])); return ProcessResult(1, 0, 'aarch64', '');
}); });
final FuchsiaDevice device = FuchsiaDevice('123'); final FuchsiaDevice device = FuchsiaDevice('123');
expect(await device.targetPlatform, TargetPlatform.fuchsia_arm64); expect(await device.targetPlatform, TargetPlatform.fuchsia_arm64);
}, overrides: <Type, Generator>{ }, overrides: <Type, Generator>{
FuchsiaArtifacts: () => FuchsiaArtifacts(sshConfig: sshConfig), FuchsiaArtifacts: () => FuchsiaArtifacts(sshConfig: sshConfig),
FuchsiaSdk: () => MockFuchsiaSdk(), FuchsiaSdk: () => MockFuchsiaSdk(),
ProcessUtils: () => mockProcessUtils, ProcessManager: () => MockProcessManager(),
}); });
testUsingContext('targetPlatform x64 works', () async { testUsingContext('targetPlatform x64 works', () async {
when(mockProcessUtils.run(any)).thenAnswer((Invocation _) { when(globals.processManager.run(any)).thenAnswer((Invocation _) async {
return Future<RunResult>.value(RunResult(ProcessResult(1, 0, 'x86_64', ''), <String>[''])); return ProcessResult(1, 0, 'x86_64', '');
}); });
final FuchsiaDevice device = FuchsiaDevice('123'); final FuchsiaDevice device = FuchsiaDevice('123');
expect(await device.targetPlatform, TargetPlatform.fuchsia_x64); expect(await device.targetPlatform, TargetPlatform.fuchsia_x64);
}, overrides: <Type, Generator>{ }, overrides: <Type, Generator>{
FuchsiaArtifacts: () => FuchsiaArtifacts(sshConfig: sshConfig), FuchsiaArtifacts: () => FuchsiaArtifacts(sshConfig: sshConfig),
FuchsiaSdk: () => MockFuchsiaSdk(), FuchsiaSdk: () => MockFuchsiaSdk(),
ProcessUtils: () => mockProcessUtils, ProcessManager: () => MockProcessManager(),
}); });
}); });
...@@ -326,8 +324,8 @@ void main() { ...@@ -326,8 +324,8 @@ void main() {
}); });
}); });
group('screenshot', () { group('screenshot', () {
MockProcessManager mockProcessManager; MockProcessManager mockProcessManager;
setUp(() { setUp(() {
mockProcessManager = MockProcessManager(); mockProcessManager = MockProcessManager();
...@@ -550,6 +548,7 @@ void main() { ...@@ -550,6 +548,7 @@ void main() {
}, testOn: 'posix'); }, testOn: 'posix');
}); });
group(FuchsiaIsolateDiscoveryProtocol, () { group(FuchsiaIsolateDiscoveryProtocol, () {
MockPortForwarder portForwarder; MockPortForwarder portForwarder;
MockVMService vmService; MockVMService vmService;
...@@ -960,8 +959,6 @@ class MockProcessManager extends Mock implements ProcessManager {} ...@@ -960,8 +959,6 @@ class MockProcessManager extends Mock implements ProcessManager {}
class MockProcessResult extends Mock implements ProcessResult {} class MockProcessResult extends Mock implements ProcessResult {}
class MockProcessUtils extends Mock implements ProcessUtils {}
class MockFile extends Mock implements File {} class MockFile extends Mock implements File {}
class MockProcess extends Mock implements Process {} class MockProcess extends Mock implements Process {}
......
...@@ -10,9 +10,11 @@ import 'package:file/memory.dart'; ...@@ -10,9 +10,11 @@ import 'package:file/memory.dart';
import 'package:flutter_tools/src/base/context.dart'; import 'package:flutter_tools/src/base/context.dart';
import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/error_handling_file_system.dart'; import 'package:flutter_tools/src/base/error_handling_file_system.dart';
import 'package:flutter_tools/src/base/process.dart';
import 'package:flutter_tools/src/globals.dart' as globals; import 'package:flutter_tools/src/globals.dart' as globals;
import '../src/common.dart'; import '../src/common.dart';
import '../src/context.dart';
import '../src/testbed.dart'; import '../src/testbed.dart';
void main() { void main() {
...@@ -87,6 +89,14 @@ void main() { ...@@ -87,6 +89,14 @@ void main() {
timer.cancel(); timer.cancel();
}); });
}); });
test('Throws if ProcessUtils is injected',() {
final Testbed testbed = Testbed(overrides: <Type, Generator>{
ProcessUtils: () => null,
});
expect(() => testbed.run(() {}), throwsA(isInstanceOf<StateError>()));
});
}); });
} }
......
...@@ -8,7 +8,6 @@ import 'package:built_collection/built_collection.dart'; ...@@ -8,7 +8,6 @@ import 'package:built_collection/built_collection.dart';
import 'package:dwds/asset_handler.dart'; import 'package:dwds/asset_handler.dart';
import 'package:dwds/dwds.dart'; import 'package:dwds/dwds.dart';
import 'package:flutter_tools/src/base/os.dart'; import 'package:flutter_tools/src/base/os.dart';
import 'package:flutter_tools/src/base/process.dart';
import 'package:flutter_tools/src/build_info.dart'; import 'package:flutter_tools/src/build_info.dart';
import 'package:flutter_tools/src/dart/pub.dart'; import 'package:flutter_tools/src/dart/pub.dart';
import 'package:flutter_tools/src/project.dart'; import 'package:flutter_tools/src/project.dart';
...@@ -18,8 +17,10 @@ import 'package:flutter_tools/src/globals.dart' as globals; ...@@ -18,8 +17,10 @@ import 'package:flutter_tools/src/globals.dart' as globals;
import 'package:http_multi_server/http_multi_server.dart'; import 'package:http_multi_server/http_multi_server.dart';
import 'package:meta/meta.dart'; import 'package:meta/meta.dart';
import 'package:mockito/mockito.dart'; import 'package:mockito/mockito.dart';
import 'package:process/process.dart';
import '../../src/common.dart'; import '../../src/common.dart';
import '../../src/mocks.dart';
import '../../src/testbed.dart'; import '../../src/testbed.dart';
void main() { void main() {
...@@ -30,7 +31,7 @@ void main() { ...@@ -30,7 +31,7 @@ void main() {
MockHttpMultiServer mockHttpMultiServer; MockHttpMultiServer mockHttpMultiServer;
MockBuildDaemonClient mockBuildDaemonClient; MockBuildDaemonClient mockBuildDaemonClient;
MockOperatingSystemUtils mockOperatingSystemUtils; MockOperatingSystemUtils mockOperatingSystemUtils;
MockProcessUtils mockProcessUtils; MockProcessManager mockProcessManager;
bool lastInitializePlatform; bool lastInitializePlatform;
dynamic lastAddress; dynamic lastAddress;
int lastPort; int lastPort;
...@@ -45,7 +46,7 @@ void main() { ...@@ -45,7 +46,7 @@ void main() {
mockBuildDaemonClient = MockBuildDaemonClient(); mockBuildDaemonClient = MockBuildDaemonClient();
mockOperatingSystemUtils = MockOperatingSystemUtils(); mockOperatingSystemUtils = MockOperatingSystemUtils();
mockDwds = MockDwds(); mockDwds = MockDwds();
mockProcessUtils = MockProcessUtils(); mockProcessManager = MockProcessManager();
when(mockBuildDaemonCreator.startBuildDaemon(any, release: anyNamed('release'), initializePlatform: anyNamed('initializePlatform'))) when(mockBuildDaemonCreator.startBuildDaemon(any, release: anyNamed('release'), initializePlatform: anyNamed('initializePlatform')))
.thenAnswer((Invocation invocation) async { .thenAnswer((Invocation invocation) async {
lastInitializePlatform = invocation.namedArguments[#initializePlatform] as bool; lastInitializePlatform = invocation.namedArguments[#initializePlatform] as bool;
...@@ -54,15 +55,14 @@ void main() { ...@@ -54,15 +55,14 @@ void main() {
when(mockOperatingSystemUtils.findFreePort()).thenAnswer((Invocation _) async { when(mockOperatingSystemUtils.findFreePort()).thenAnswer((Invocation _) async {
return 1234; return 1234;
}); });
when(mockProcessUtils.stream( when(mockProcessManager.start(
any, any,
workingDirectory: anyNamed('workingDirectory'), workingDirectory: anyNamed('workingDirectory'),
mapFunction: anyNamed('mapFunction'),
environment: anyNamed('environment'), environment: anyNamed('environment'),
)).thenAnswer((Invocation invocation) async { )).thenAnswer((Invocation invocation) async {
final String workingDirectory = invocation.namedArguments[#workingDirectory] as String; final String workingDirectory = invocation.namedArguments[#workingDirectory] as String;
globals.fs.file(globals.fs.path.join(workingDirectory, '.packages')).createSync(recursive: true); globals.fs.file(globals.fs.path.join(workingDirectory, '.packages')).createSync(recursive: true);
return 0; return FakeProcess();
}); });
when(mockBuildDaemonClient.buildResults).thenAnswer((Invocation _) { when(mockBuildDaemonClient.buildResults).thenAnswer((Invocation _) {
return Stream<BuildResults>.fromFuture(Future<BuildResults>.value( return Stream<BuildResults>.fromFuture(Future<BuildResults>.value(
...@@ -93,7 +93,7 @@ void main() { ...@@ -93,7 +93,7 @@ void main() {
OperatingSystemUtils: () => mockOperatingSystemUtils, OperatingSystemUtils: () => mockOperatingSystemUtils,
BuildDaemonCreator: () => mockBuildDaemonCreator, BuildDaemonCreator: () => mockBuildDaemonCreator,
ChromeLauncher: () => mockChromeLauncher, ChromeLauncher: () => mockChromeLauncher,
ProcessUtils: () => mockProcessUtils, ProcessManager: () => mockProcessManager,
HttpMultiServerFactory: () => (dynamic address, int port) async { HttpMultiServerFactory: () => (dynamic address, int port) async {
lastAddress = address; lastAddress = address;
lastPort = port; lastPort = port;
...@@ -226,5 +226,5 @@ class MockDwds extends Mock implements Dwds {} ...@@ -226,5 +226,5 @@ class MockDwds extends Mock implements Dwds {}
class MockHttpMultiServer extends Mock implements HttpMultiServer {} class MockHttpMultiServer extends Mock implements HttpMultiServer {}
class MockChromeLauncher extends Mock implements ChromeLauncher {} class MockChromeLauncher extends Mock implements ChromeLauncher {}
class MockOperatingSystemUtils extends Mock implements OperatingSystemUtils {} class MockOperatingSystemUtils extends Mock implements OperatingSystemUtils {}
class MockProcessUtils extends Mock implements ProcessUtils {}
class MockPub extends Mock implements Pub {} class MockPub extends Mock implements Pub {}
class MockProcessManager extends Mock implements ProcessManager {}
...@@ -12,6 +12,7 @@ import 'package:flutter_tools/src/base/file_system.dart'; ...@@ -12,6 +12,7 @@ import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/io.dart'; import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/base/os.dart'; import 'package:flutter_tools/src/base/os.dart';
import 'package:flutter_tools/src/base/process.dart';
import 'package:flutter_tools/src/base/signals.dart'; import 'package:flutter_tools/src/base/signals.dart';
import 'package:flutter_tools/src/base/terminal.dart'; import 'package:flutter_tools/src/base/terminal.dart';
import 'package:flutter_tools/src/base/time.dart'; import 'package:flutter_tools/src/base/time.dart';
...@@ -63,6 +64,9 @@ void testUsingContext( ...@@ -63,6 +64,9 @@ void testUsingContext(
'that you are dealing with in your test.' 'that you are dealing with in your test.'
); );
} }
if (overrides.containsKey(ProcessUtils)) {
throw StateError('Do not inject ProcessUtils for testing, use ProcessManager instead.');
}
// Ensure we don't rely on the default [Config] constructor which will // Ensure we don't rely on the default [Config] constructor which will
// leak a sticky $HOME/.flutter_settings behind! // leak a sticky $HOME/.flutter_settings behind!
......
...@@ -12,6 +12,7 @@ import 'package:flutter_tools/src/base/file_system.dart'; ...@@ -12,6 +12,7 @@ import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/io.dart'; import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/base/os.dart'; import 'package:flutter_tools/src/base/os.dart';
import 'package:flutter_tools/src/base/process.dart';
import 'package:flutter_tools/src/base/signals.dart'; import 'package:flutter_tools/src/base/signals.dart';
import 'package:flutter_tools/src/base/terminal.dart'; import 'package:flutter_tools/src/base/terminal.dart';
...@@ -113,6 +114,9 @@ class Testbed { ...@@ -113,6 +114,9 @@ class Testbed {
// Add the test-specific overrides // Add the test-specific overrides
...?overrides, ...?overrides,
}; };
if (testOverrides.containsKey(ProcessUtils)) {
throw StateError('Do not inject ProcessUtils for testing, use ProcessManager instead.');
}
// Cache the original flutter root to restore after the test case. // Cache the original flutter root to restore after the test case.
final String originalFlutterRoot = Cache.flutterRoot; final String originalFlutterRoot = Cache.flutterRoot;
// Track pending timers to verify that they were correctly cleaned up. // Track pending timers to verify that they were correctly cleaned up.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment