[flutter_plugin_tool] Add more failure test coverage (#4132)

Many commands had insufficient failure testing. This adds new tests that
ensure that for every Process call, at least one test fails if a failure
from that process were ignored (with the exception of calls in the
`publish` command, which has a custom process mocking system, so was out
of scope here; it already has more coverage than most tests did though.)

For a few existing failure tests, adds output checks to ensure that they
are testing for the *right* failures.

Other changes:
- Adds convenience constructors to MockProcess for the common cases of a
  mock process that just exits with a 0 or 1 status, to reduce test
  verbosity.
- Fixes a few bugs that were found by the new tests.
- Minor test cleanup, especially cases where a mock process was being
  set up just to make all calls succeed, which is the default as of
  recent changes.

Author: stuartmorgan-g

script/tool/lib/src/analyze_command.dart

@@ -66,7 +66,7 @@ class AnalyzeCommand extends PackageLoopingCommand {
       }
 
       printError(
-          'Found an extra analysis_options.yaml in ${file.absolute.path}.');
+          'Found an extra analysis_options.yaml at ${file.absolute.path}.');
       printError(
           'If this was deliberate, pass the package to the analyze command '
           'with the --$_customAnalysisFlag flag and try again.');
@@ -105,7 +105,7 @@ class AnalyzeCommand extends PackageLoopingCommand {
 
     print('Fetching dependencies...');
     if (!await _runPackagesGetOnTargetPackages()) {
-      printError('Unabled to get dependencies.');
+      printError('Unable to get dependencies.');
       throw ToolExit(_exitPackagesGetFailed);
     }
 

script/tool/lib/src/firebase_test_lab_command.dart

@@ -135,13 +135,13 @@ class FirebaseTestLabCommand extends PackageLoopingCommand {
 
     // Ensures that gradle wrapper exists
     if (!await _ensureGradleWrapperExists(androidDirectory)) {
-      PackageResult.fail(<String>['Unable to build example apk']);
+      return PackageResult.fail(<String>['Unable to build example apk']);
     }
 
     await _configureFirebaseProject();
 
     if (!await _runGradle(androidDirectory, 'app:assembleAndroidTest')) {
-      PackageResult.fail(<String>['Unable to assemble androidTest']);
+      return PackageResult.fail(<String>['Unable to assemble androidTest']);
     }
 
     final List<String> errors = <String>[];
@@ -236,7 +236,7 @@ class FirebaseTestLabCommand extends PackageLoopingCommand {
         : null;
 
     final int exitCode = await processRunner.runAndStream(
-        p.join(directory.path, _gradleWrapper),
+        directory.childFile(_gradleWrapper).path,
         <String>[
           target,
           '-Pverbose=true',

script/tool/lib/src/java_test_command.dart

@@ -54,7 +54,8 @@ class JavaTestCommand extends PackageLoopingCommand {
       print('\nRUNNING JAVA TESTS for $exampleName');
 
       final Directory androidDirectory = example.childDirectory('android');
-      if (!androidDirectory.childFile(_gradleWrapper).existsSync()) {
+      final File gradleFile = androidDirectory.childFile(_gradleWrapper);
+      if (!gradleFile.existsSync()) {
         printError('ERROR: Run "flutter build apk" on $exampleName, or run '
             'this tool\'s "build-examples --apk" command, '
             'before executing tests.');
@@ -63,8 +64,7 @@ class JavaTestCommand extends PackageLoopingCommand {
       }
 
       final int exitCode = await processRunner.runAndStream(
-          p.join(androidDirectory.path, _gradleWrapper),
-          <String>['testDebugUnitTest', '--info'],
+          gradleFile.path, <String>['testDebugUnitTest', '--info'],
           workingDir: androidDirectory);
       if (exitCode != 0) {
         errors.add('$exampleName tests failed.');

script/tool/lib/src/version_check_command.dart

@@ -128,7 +128,7 @@ class VersionCheckCommand extends PackageLoopingCommand {
           'that intentionally has no version should be marked '
           '"publish_to: none".');
       // No remaining checks make sense, so fail immediately.
-      PackageResult.fail(<String>['No pubspec.yaml version.']);
+      return PackageResult.fail(<String>['No pubspec.yaml version.']);
     }
 
     final List<String> errors = <String>[];

script/tool/test/analyze_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';
@@ -33,9 +35,6 @@ void main() {
     final Directory plugin1Dir = createFakePlugin('a', packagesDir);
     final Directory plugin2Dir = createFakePlugin('b', packagesDir);
 
-    final MockProcess mockProcess = MockProcess();
-    mockProcess.exitCodeCompleter.complete(0);
-    processRunner.processToReturn = mockProcess;
     await runCapturingPrint(runner, <String>['analyze']);
 
     expect(
@@ -55,9 +54,6 @@ void main() {
   test('skips flutter pub get for examples', () async {
     final Directory plugin1Dir = createFakePlugin('a', packagesDir);
 
-    final MockProcess mockProcess = MockProcess();
-    mockProcess.exitCodeCompleter.complete(0);
-    processRunner.processToReturn = mockProcess;
     await runCapturingPrint(runner, <String>['analyze']);
 
     expect(
@@ -74,9 +70,6 @@ void main() {
     final Directory plugin1Dir = createFakePlugin('a', packagesDir);
     final Directory plugin2Dir = createFakePlugin('example', packagesDir);
 
-    final MockProcess mockProcess = MockProcess();
-    mockProcess.exitCodeCompleter.complete(0);
-    processRunner.processToReturn = mockProcess;
     await runCapturingPrint(runner, <String>['analyze']);
 
     expect(
@@ -96,9 +89,6 @@ void main() {
   test('uses a separate analysis sdk', () async {
     final Directory pluginDir = createFakePlugin('a', packagesDir);
 
-    final MockProcess mockProcess = MockProcess();
-    mockProcess.exitCodeCompleter.complete(0);
-    processRunner.processToReturn = mockProcess;
     await runCapturingPrint(
         runner, <String>['analyze', '--analysis-sdk', 'foo/bar/baz']);
 
@@ -124,25 +114,46 @@ void main() {
       createFakePlugin('foo', packagesDir,
           extraFiles: <String>['analysis_options.yaml']);
 
-      await expectLater(() => runCapturingPrint(runner, <String>['analyze']),
-          throwsA(isA<ToolExit>()));
+      Error? commandError;
+      final List<String> output = await runCapturingPrint(
+          runner, <String>['analyze'], errorHandler: (Error e) {
+        commandError = e;
+      });
+
+      expect(commandError, isA<ToolExit>());
+      expect(
+        output,
+        containsAllInOrder(<Matcher>[
+          contains(
+              'Found an extra analysis_options.yaml at /packages/foo/analysis_options.yaml'),
+        ]),
+      );
     });
 
     test('fails .analysis_options', () async {
       createFakePlugin('foo', packagesDir,
           extraFiles: <String>['.analysis_options']);
 
-      await expectLater(() => runCapturingPrint(runner, <String>['analyze']),
-          throwsA(isA<ToolExit>()));
+      Error? commandError;
+      final List<String> output = await runCapturingPrint(
+          runner, <String>['analyze'], errorHandler: (Error e) {
+        commandError = e;
+      });
+
+      expect(commandError, isA<ToolExit>());
+      expect(
+        output,
+        containsAllInOrder(<Matcher>[
+          contains(
+              'Found an extra analysis_options.yaml at /packages/foo/.analysis_options'),
+        ]),
+      );
     });
 
     test('takes an allow list', () async {
       final Directory pluginDir = createFakePlugin('foo', packagesDir,
           extraFiles: <String>['analysis_options.yaml']);
 
-      final MockProcess mockProcess = MockProcess();
-      mockProcess.exitCodeCompleter.complete(0);
-      processRunner.processToReturn = mockProcess;
       await runCapturingPrint(
           runner, <String>['analyze', '--custom-analysis', 'foo']);
 
@@ -161,14 +172,55 @@ void main() {
       createFakePlugin('foo', packagesDir,
           extraFiles: <String>['analysis_options.yaml']);
 
-      final MockProcess mockProcess = MockProcess();
-      mockProcess.exitCodeCompleter.complete(0);
-      processRunner.processToReturn = mockProcess;
-
       await expectLater(
           () => runCapturingPrint(
               runner, <String>['analyze', '--custom-analysis', '']),
           throwsA(isA<ToolExit>()));
     });
   });
+
+  test('fails if "packages get" fails', () async {
+    createFakePlugin('foo', packagesDir);
+
+    processRunner.mockProcessesForExecutable['flutter'] = <io.Process>[
+      MockProcess.failing() // flutter packages get
+    ];
+
+    Error? commandError;
+    final List<String> output = await runCapturingPrint(
+        runner, <String>['analyze'], errorHandler: (Error e) {
+      commandError = e;
+    });
+
+    expect(commandError, isA<ToolExit>());
+    expect(
+      output,
+      containsAllInOrder(<Matcher>[
+        contains('Unable to get dependencies'),
+      ]),
+    );
+  });
+
+  test('fails if "analyze" fails', () async {
+    createFakePlugin('foo', packagesDir);
+
+    processRunner.mockProcessesForExecutable['dart'] = <io.Process>[
+      MockProcess.failing() // dart analyze
+    ];
+
+    Error? commandError;
+    final List<String> output = await runCapturingPrint(
+        runner, <String>['analyze'], errorHandler: (Error e) {
+      commandError = e;
+    });
+
+    expect(commandError, isA<ToolExit>());
+    expect(
+      output,
+      containsAllInOrder(<Matcher>[
+        contains('The following packages had errors:'),
+        contains('  foo'),
+      ]),
+    );
+  });
 }