diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md index d64655f5b8..db9ecf4930 100644 --- a/script/tool/CHANGELOG.md +++ b/script/tool/CHANGELOG.md @@ -5,6 +5,8 @@ files, only `integration_test/*_test.dart`. - Add a summary to the end of successful command runs for commands using the new output format. +- Fixed some cases where a failure in a command for a single package would + immediately abort the test. ## 0.3.0 diff --git a/script/tool/lib/src/analyze_command.dart b/script/tool/lib/src/analyze_command.dart index 29e78f3ea4..adaeb1e616 100644 --- a/script/tool/lib/src/analyze_command.dart +++ b/script/tool/lib/src/analyze_command.dart @@ -12,6 +12,7 @@ import 'common/package_looping_command.dart'; import 'common/process_runner.dart'; const int _exitBadCustomAnalysisFile = 2; +const int _exitPackagesGetFailed = 3; /// A command to run Dart analysis on packages. class AnalyzeCommand extends PackageLoopingCommand { @@ -75,7 +76,7 @@ class AnalyzeCommand extends PackageLoopingCommand { /// Ensures that the dependent packages have been fetched for all packages /// (including their sub-packages) that will be analyzed. - Future _runPackagesGetOnTargetPackages() async { + Future _runPackagesGetOnTargetPackages() async { final List packageDirectories = await getPackages().toList(); final Set packagePaths = packageDirectories.map((Directory dir) => dir.path).toSet(); @@ -87,9 +88,14 @@ class AnalyzeCommand extends PackageLoopingCommand { packagePaths.contains(directory.parent.path); }); for (final Directory package in packageDirectories) { - await processRunner.runAndStream('flutter', ['packages', 'get'], - workingDir: package, exitOnError: true); + final int exitCode = await processRunner.runAndStream( + 'flutter', ['packages', 'get'], + workingDir: package); + if (exitCode != 0) { + return false; + } } + return true; } @override @@ -98,7 +104,10 @@ class AnalyzeCommand extends PackageLoopingCommand { _validateAnalysisOptions(); print('Fetching dependencies...'); - await _runPackagesGetOnTargetPackages(); + if (!await _runPackagesGetOnTargetPackages()) { + printError('Unabled to get dependencies.'); + throw ToolExit(_exitPackagesGetFailed); + } // Use the Dart SDK override if one was passed in. final String? dartSdk = argResults![_analysisSdk] as String?; diff --git a/script/tool/lib/src/drive_examples_command.dart b/script/tool/lib/src/drive_examples_command.dart index dc9774d846..1e19535f4a 100644 --- a/script/tool/lib/src/drive_examples_command.dart +++ b/script/tool/lib/src/drive_examples_command.dart @@ -203,7 +203,7 @@ class DriveExamplesCommand extends PackageLoopingCommand { final ProcessResult result = await processRunner.run( flutterCommand, ['devices', '--machine'], - stdoutEncoding: utf8, exitOnError: true); + stdoutEncoding: utf8); if (result.exitCode != 0) { return deviceIds; } @@ -295,8 +295,7 @@ class DriveExamplesCommand extends PackageLoopingCommand { '--target', p.relative(target.path, from: example.path), ], - workingDir: example, - exitOnError: true); + workingDir: example); if (exitCode != 0) { failures.add(target); } diff --git a/script/tool/lib/src/firebase_test_lab_command.dart b/script/tool/lib/src/firebase_test_lab_command.dart index 8d1ba995f1..3d68c85fbd 100644 --- a/script/tool/lib/src/firebase_test_lab_command.dart +++ b/script/tool/lib/src/firebase_test_lab_command.dart @@ -13,6 +13,8 @@ import 'common/core.dart'; import 'common/package_looping_command.dart'; import 'common/process_runner.dart'; +const int _exitGcloudAuthFailed = 2; + /// A command to run tests via Firebase test lab. class FirebaseTestLabCommand extends PackageLoopingCommand { /// Creates an instance of the test runner command. @@ -84,16 +86,19 @@ class FirebaseTestLabCommand extends PackageLoopingCommand { if (serviceKey.isEmpty) { print('No --service-key provided; skipping gcloud authorization'); } else { - await processRunner.run( + final io.ProcessResult result = await processRunner.run( 'gcloud', [ 'auth', 'activate-service-account', '--key-file=$serviceKey', ], - exitOnError: true, logOnError: true, ); + if (result.exitCode != 0) { + printError('Unable to activate gcloud account.'); + throw ToolExit(_exitGcloudAuthFailed); + } final int exitCode = await processRunner.runAndStream('gcloud', [ 'config', 'set', diff --git a/script/tool/lib/src/lint_podspecs_command.dart b/script/tool/lib/src/lint_podspecs_command.dart index 0bdb7c972c..82cce0bd13 100644 --- a/script/tool/lib/src/lint_podspecs_command.dart +++ b/script/tool/lib/src/lint_podspecs_command.dart @@ -14,6 +14,7 @@ import 'common/package_looping_command.dart'; import 'common/process_runner.dart'; const int _exitUnsupportedPlatform = 2; +const int _exitPodNotInstalled = 3; /// Lint the CocoaPod podspecs and run unit tests. /// @@ -53,13 +54,16 @@ class LintPodspecsCommand extends PackageLoopingCommand { throw ToolExit(_exitUnsupportedPlatform); } - await processRunner.run( + final ProcessResult result = await processRunner.run( 'which', ['pod'], workingDir: packagesDir, - exitOnError: true, logOnError: true, ); + if (result.exitCode != 0) { + printError('Unable to find "pod". Make sure it is in your path.'); + throw ToolExit(_exitPodNotInstalled); + } } @override diff --git a/script/tool/lib/src/publish_plugin_command.dart b/script/tool/lib/src/publish_plugin_command.dart index 740178829c..6de53ba269 100644 --- a/script/tool/lib/src/publish_plugin_command.dart +++ b/script/tool/lib/src/publish_plugin_command.dart @@ -355,7 +355,6 @@ Safe to ignore if the package is deleted in this commit. 'git', ['tag', tag], workingDir: packageDir, - exitOnError: false, logOnError: true, ); if (result.exitCode != 0) { @@ -402,7 +401,6 @@ Safe to ignore if the package is deleted in this commit. ['status', '--porcelain', '--ignored', packageDir.absolute.path], workingDir: packageDir, logOnError: true, - exitOnError: false, ); if (statusResult.exitCode != 0) { return false; @@ -423,9 +421,11 @@ Safe to ignore if the package is deleted in this commit. 'git', ['remote', 'get-url', remote], workingDir: packagesDir, - exitOnError: true, logOnError: true, ); + if (getRemoteUrlResult.exitCode != 0) { + return null; + } return getRemoteUrlResult.stdout as String?; } @@ -498,7 +498,6 @@ Safe to ignore if the package is deleted in this commit. 'git', ['push', remote.name, tag], workingDir: packagesDir, - exitOnError: false, logOnError: true, ); if (result.exitCode != 0) { diff --git a/script/tool/lib/src/xctest_command.dart b/script/tool/lib/src/xctest_command.dart index c7a454ab75..cd3b674f8d 100644 --- a/script/tool/lib/src/xctest_command.dart +++ b/script/tool/lib/src/xctest_command.dart @@ -184,7 +184,7 @@ class XCTestCommand extends PackageLoopingCommand { '$_kXCRunCommand ${xctestArgs.join(' ')}'; print(completeTestCommand); return processRunner.runAndStream(_kXCRunCommand, xctestArgs, - workingDir: example, exitOnError: false); + workingDir: example); } Future _findAvailableIphoneSimulator() async { diff --git a/script/tool/test/drive_examples_command_test.dart b/script/tool/test/drive_examples_command_test.dart index 97a39c6b2b..eeac96e56e 100644 --- a/script/tool/test/drive_examples_command_test.dart +++ b/script/tool/test/drive_examples_command_test.dart @@ -2,6 +2,8 @@ // 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; + import 'package:args/command_runner.dart'; import 'package:file/file.dart'; import 'package:file/memory.dart'; @@ -53,7 +55,9 @@ void main() { final MockProcess mockDevicesProcess = MockProcess(); mockDevicesProcess.exitCodeCompleter.complete(0); mockDevicesProcess.stdoutController.close(); // ignore: unawaited_futures - processRunner.processToReturn = mockDevicesProcess; + processRunner.mockProcessesForExecutable['flutter'] = [ + mockDevicesProcess + ]; processRunner.resultStdout = output; } @@ -110,7 +114,31 @@ void main() { ); }); - test('fails if Android if no Android devices are present', () async { + test('fails for iOS if getting devices fails', () async { + setMockFlutterDevicesOutput(hasIosDevice: false); + + // Simulate failure from `flutter devices`. + final MockProcess mockProcess = MockProcess(); + mockProcess.exitCodeCompleter.complete(1); + processRunner.processToReturn = mockProcess; + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['drive-examples', '--ios'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('No iOS devices'), + ]), + ); + }); + + test('fails for Android if no Android devices are present', () async { + setMockFlutterDevicesOutput(hasAndroidDevice: false); Error? commandError; final List output = await runCapturingPrint( runner, ['drive-examples', '--android'], diff --git a/script/tool/test/firebase_test_lab_command_test.dart b/script/tool/test/firebase_test_lab_command_test.dart index 4285a0fee3..711c383f2d 100644 --- a/script/tool/test/firebase_test_lab_command_test.dart +++ b/script/tool/test/firebase_test_lab_command_test.dart @@ -33,10 +33,12 @@ void main() { runner.addCommand(command); }); - test('retries gcloud set', () async { + test('fails if gcloud auth fails', () async { final MockProcess mockProcess = MockProcess(); mockProcess.exitCodeCompleter.complete(1); - processRunner.processToReturn = mockProcess; + processRunner.mockProcessesForExecutable['gcloud'] = [ + mockProcess + ]; createFakePlugin('plugin', packagesDir, extraFiles: [ 'example/integration_test/foo_test.dart', 'example/android/gradlew', @@ -50,6 +52,31 @@ void main() { }); expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('Unable to activate gcloud account.'), + ])); + }); + + test('retries gcloud set', () async { + final MockProcess mockAuthProcess = MockProcess(); + mockAuthProcess.exitCodeCompleter.complete(0); + final MockProcess mockConfigProcess = MockProcess(); + mockConfigProcess.exitCodeCompleter.complete(1); + processRunner.mockProcessesForExecutable['gcloud'] = [ + mockAuthProcess, + mockConfigProcess, + ]; + createFakePlugin('plugin', packagesDir, extraFiles: [ + 'example/integration_test/foo_test.dart', + 'example/android/gradlew', + 'example/android/app/src/androidTest/MainActivityTest.java', + ]); + + final List output = + await runCapturingPrint(runner, ['firebase-test-lab']); + expect( output, containsAllInOrder([ diff --git a/script/tool/test/lint_podspecs_command_test.dart b/script/tool/test/lint_podspecs_command_test.dart index 90a662d750..c61d6a9d92 100644 --- a/script/tool/test/lint_podspecs_command_test.dart +++ b/script/tool/test/lint_podspecs_command_test.dart @@ -2,6 +2,8 @@ // 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; + import 'package:args/command_runner.dart'; import 'package:file/file.dart'; import 'package:file/memory.dart'; @@ -160,6 +162,34 @@ void main() { expect(output, contains('Linting plugin1.podspec')); }); + test('fails if pod is missing', () async { + createFakePlugin('plugin1', packagesDir, + extraFiles: ['plugin1.podspec']); + + // Simulate failure from `which pod`. + final MockProcess mockWhichProcess = MockProcess(); + mockWhichProcess.exitCodeCompleter.complete(1); + processRunner.mockProcessesForExecutable['which'] = [ + mockWhichProcess + ]; + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['podspecs'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + + expect( + output, + containsAllInOrder( + [ + contains('Unable to find "pod". Make sure it is in your path.'), + ], + )); + }); + test('fails if linting fails', () async { createFakePlugin('plugin1', packagesDir, extraFiles: ['plugin1.podspec']); @@ -167,7 +197,9 @@ void main() { // Simulate failure from `pod`. final MockProcess mockDriveProcess = MockProcess(); mockDriveProcess.exitCodeCompleter.complete(1); - processRunner.processToReturn = mockDriveProcess; + processRunner.mockProcessesForExecutable['pod'] = [ + mockDriveProcess + ]; Error? commandError; final List output = await runCapturingPrint( diff --git a/script/tool/test/util.dart b/script/tool/test/util.dart index e71a26fa4e..b65b1fcaa8 100644 --- a/script/tool/test/util.dart +++ b/script/tool/test/util.dart @@ -252,15 +252,22 @@ Future> runCapturingPrint( /// A mock [ProcessRunner] which records process calls. class RecordingProcessRunner extends ProcessRunner { - io.Process? processToReturn; final List recordedCalls = []; + /// Maps an executable to a list of processes that should be used for each + /// successive call to it via [run], [runAndStream], or [start]. + final Map> mockProcessesForExecutable = + >{}; + /// Populate for [io.ProcessResult] to use a String [stdout] instead of a [List] of [int]. String? resultStdout; /// Populate for [io.ProcessResult] to use a String [stderr] instead of a [List] of [int]. String? resultStderr; + // Deprecated--do not add new uses. Use mockProcessesForExecutable instead. + io.Process? processToReturn; + @override Future runAndStream( String executable, @@ -269,11 +276,17 @@ class RecordingProcessRunner extends ProcessRunner { bool exitOnError = false, }) async { recordedCalls.add(ProcessCall(executable, args, workingDir?.path)); - return Future.value( - processToReturn == null ? 0 : await processToReturn!.exitCode); + final io.Process? processToReturn = _getProcessToReturn(executable); + final int exitCode = + processToReturn == null ? 0 : await processToReturn.exitCode; + if (exitOnError && (exitCode != 0)) { + throw io.ProcessException(executable, args); + } + return Future.value(exitCode); } - /// Returns [io.ProcessResult] created from [processToReturn], [resultStdout], and [resultStderr]. + /// Returns [io.ProcessResult] created from [mockProcessesForExecutable], + /// [resultStdout], and [resultStderr]. @override Future run( String executable, @@ -286,12 +299,16 @@ class RecordingProcessRunner extends ProcessRunner { }) async { recordedCalls.add(ProcessCall(executable, args, workingDir?.path)); - final io.Process? process = processToReturn; + final io.Process? process = _getProcessToReturn(executable); final io.ProcessResult result = process == null - ? io.ProcessResult(1, 1, '', '') + ? io.ProcessResult(1, 0, '', '') : io.ProcessResult(process.pid, await process.exitCode, resultStdout ?? process.stdout, resultStderr ?? process.stderr); + if (exitOnError && (result.exitCode != 0)) { + throw io.ProcessException(executable, args); + } + return Future.value(result); } @@ -299,7 +316,17 @@ class RecordingProcessRunner extends ProcessRunner { Future start(String executable, List args, {Directory? workingDirectory}) async { recordedCalls.add(ProcessCall(executable, args, workingDirectory?.path)); - return Future.value(processToReturn); + return Future.value(_getProcessToReturn(executable)); + } + + io.Process? _getProcessToReturn(String executable) { + io.Process? process; + final List? processes = mockProcessesForExecutable[executable]; + if (processes != null && processes.isNotEmpty) { + process = mockProcessesForExecutable[executable]!.removeAt(0); + } + // Fall back to `processToReturn` for backwards compatibility. + return process ?? processToReturn; } }