[flutter_plugin_tools] Make firebase-test-lab fail when no tests run (#4172)

If a package supports Android, it will now report failure instead of
skip if no tests run. This matches the new behavior of drive-examples,
and is intended to prevent recurrance of situations where we are
silently failing to run tests because of, e.g., tests being in the wrong
directory.

Also fixes a long-standing but unnoticed problem where if a
run tried to run more than one package's tests, it would hang
forever (although on the bots it doesn't seem to time out, just
end logs abruptly) due to a logic error in the call to configure
gcloud.

Fixes flutter/flutter#86732

script/tool/CHANGELOG.md

@@ -3,6 +3,12 @@
 - Added an `xctest` flag to select specific test targets, to allow running only
   unit tests or integration tests.
 - Split Xcode analysis out of `xctest` and into a new `xcode-analyze` command.
+- Fixed a bug that caused `firebase-test-lab` to hang if it tried to run more
+  than one plugin's tests in a single run.
+- **Breaking change**: If `firebase-test-lab` is run on a package that supports
+  Android, but for which no tests are run, it now fails instead of skipping.
+  This matches `drive-examples`, as this command is what is used for driving
+  Android Flutter integration tests on CI.
 
 ## 0.4.1
 

script/tool/lib/src/firebase_test_lab_command.dart

@@ -76,13 +76,12 @@ class FirebaseTestLabCommand extends PackageLoopingCommand {
 
   static const String _gradleWrapper = 'gradlew';
 
-  Completer<void>? _firebaseProjectConfigured;
+  bool _firebaseProjectConfigured = false;
 
   Future<void> _configureFirebaseProject() async {
-    if (_firebaseProjectConfigured != null) {
-      return _firebaseProjectConfigured!.future;
+    if (_firebaseProjectConfigured) {
+      return;
     }
-    _firebaseProjectConfigured = Completer<void>();
 
     final String serviceKey = getStringArg('service-key');
     if (serviceKey.isEmpty) {
@@ -110,31 +109,34 @@ class FirebaseTestLabCommand extends PackageLoopingCommand {
       print('');
       if (exitCode == 0) {
         print('Firebase project configured.');
-        return;
       } else {
         logWarning(
             'Warning: gcloud config set returned a non-zero exit code. Continuing anyway.');
       }
     }
-    _firebaseProjectConfigured!.complete(null);
+    _firebaseProjectConfigured = true;
   }
 
   @override
   Future<PackageResult> runForPackage(Directory package) async {
-    if (!package
-        .childDirectory('example')
-        .childDirectory('android')
+    final Directory exampleDirectory = package.childDirectory('example');
+    final Directory androidDirectory =
+        exampleDirectory.childDirectory('android');
+    if (!androidDirectory.existsSync()) {
+      return PackageResult.skip(
+          '${getPackageDescription(exampleDirectory)} does not support Android.');
+    }
+
+    if (!androidDirectory
         .childDirectory('app')
         .childDirectory('src')
         .childDirectory('androidTest')
         .existsSync()) {
-      return PackageResult.skip('No example with androidTest directory');
+      printError('No androidTest directory found.');
+      return PackageResult.fail(
+          <String>['No tests ran (use --exclude if this is intentional).']);
     }
 
-    final Directory exampleDirectory = package.childDirectory('example');
-    final Directory androidDirectory =
-        exampleDirectory.childDirectory('android');
-
     // Ensures that gradle wrapper exists
     if (!await _ensureGradleWrapperExists(androidDirectory)) {
       return PackageResult.fail(<String>['Unable to build example apk']);
@@ -191,6 +193,12 @@ class FirebaseTestLabCommand extends PackageLoopingCommand {
         errors.add('$testName failed tests');
       }
     }
+
+    if (errors.isEmpty && resultsCounter == 0) {
+      printError('No integration tests were run.');
+      errors.add('No tests ran (use --exclude if this is intentional).');
+    }
+
     return errors.isEmpty
         ? PackageResult.success()
         : PackageResult.fail(errors);

script/tool/test/firebase_test_lab_command_test.dart

@@ -84,6 +84,85 @@ void main() {
           ]));
     });
 
+    test('only runs gcloud configuration once', () async {
+      createFakePlugin('plugin1', packagesDir, extraFiles: <String>[
+        'test/plugin_test.dart',
+        'example/integration_test/foo_test.dart',
+        'example/android/gradlew',
+        'example/android/app/src/androidTest/MainActivityTest.java',
+      ]);
+      createFakePlugin('plugin2', packagesDir, extraFiles: <String>[
+        'test/plugin_test.dart',
+        'example/integration_test/bar_test.dart',
+        'example/android/gradlew',
+        'example/android/app/src/androidTest/MainActivityTest.java',
+      ]);
+
+      final List<String> output = await runCapturingPrint(runner, <String>[
+        'firebase-test-lab',
+        '--device',
+        'model=flame,version=29',
+        '--device',
+        'model=seoul,version=26',
+        '--test-run-id',
+        'testRunId',
+        '--build-id',
+        'buildId',
+      ]);
+
+      expect(
+        output,
+        containsAllInOrder(<Matcher>[
+          contains('Running for plugin1'),
+          contains('Firebase project configured.'),
+          contains('Testing example/integration_test/foo_test.dart...'),
+          contains('Running for plugin2'),
+          contains('Testing example/integration_test/bar_test.dart...'),
+        ]),
+      );
+
+      expect(
+        processRunner.recordedCalls,
+        orderedEquals(<ProcessCall>[
+          ProcessCall(
+              'gcloud',
+              'auth activate-service-account --key-file=${Platform.environment['HOME']}/gcloud-service-key.json'
+                  .split(' '),
+              null),
+          ProcessCall(
+              'gcloud', 'config set project flutter-infra'.split(' '), null),
+          ProcessCall(
+              '/packages/plugin1/example/android/gradlew',
+              'app:assembleAndroidTest -Pverbose=true'.split(' '),
+              '/packages/plugin1/example/android'),
+          ProcessCall(
+              '/packages/plugin1/example/android/gradlew',
+              'app:assembleDebug -Pverbose=true -Ptarget=/packages/plugin1/example/integration_test/foo_test.dart'
+                  .split(' '),
+              '/packages/plugin1/example/android'),
+          ProcessCall(
+              'gcloud',
+              'firebase test android run --type instrumentation --app build/app/outputs/apk/debug/app-debug.apk --test build/app/outputs/apk/androidTest/debug/app-debug-androidTest.apk --timeout 5m --results-bucket=gs://flutter_firebase_testlab --results-dir=plugins_android_test/plugin1/buildId/testRunId/0/ --device model=flame,version=29 --device model=seoul,version=26'
+                  .split(' '),
+              '/packages/plugin1/example'),
+          ProcessCall(
+              '/packages/plugin2/example/android/gradlew',
+              'app:assembleAndroidTest -Pverbose=true'.split(' '),
+              '/packages/plugin2/example/android'),
+          ProcessCall(
+              '/packages/plugin2/example/android/gradlew',
+              'app:assembleDebug -Pverbose=true -Ptarget=/packages/plugin2/example/integration_test/bar_test.dart'
+                  .split(' '),
+              '/packages/plugin2/example/android'),
+          ProcessCall(
+              'gcloud',
+              'firebase test android run --type instrumentation --app build/app/outputs/apk/debug/app-debug.apk --test build/app/outputs/apk/androidTest/debug/app-debug-androidTest.apk --timeout 5m --results-bucket=gs://flutter_firebase_testlab --results-dir=plugins_android_test/plugin2/buildId/testRunId/0/ --device model=flame,version=29 --device model=seoul,version=26'
+                  .split(' '),
+              '/packages/plugin2/example'),
+        ]),
+      );
+    });
+
     test('runs integration tests', () async {
       createFakePlugin('plugin', packagesDir, extraFiles: <String>[
         'test/plugin_test.dart',
@@ -203,12 +282,87 @@ void main() {
       );
     });
 
-    test('skips packages with no androidTest directory', () async {
+    test('fails for packages with no androidTest directory', () async {
       createFakePlugin('plugin', packagesDir, extraFiles: <String>[
         'example/integration_test/foo_test.dart',
         'example/android/gradlew',
       ]);
 
+      Error? commandError;
+      final List<String> output = await runCapturingPrint(
+        runner,
+        <String>[
+          'firebase-test-lab',
+          '--device',
+          'model=flame,version=29',
+          '--device',
+          'model=seoul,version=26',
+          '--test-run-id',
+          'testRunId',
+          '--build-id',
+          'buildId',
+        ],
+        errorHandler: (Error e) {
+          commandError = e;
+        },
+      );
+
+      expect(commandError, isA<ToolExit>());
+      expect(
+        output,
+        containsAllInOrder(<Matcher>[
+          contains('Running for plugin'),
+          contains('No androidTest directory found.'),
+          contains('The following packages had errors:'),
+          contains('plugin:\n'
+              '    No tests ran (use --exclude if this is intentional).'),
+        ]),
+      );
+    });
+
+    test('fails for packages with no integration test files', () async {
+      createFakePlugin('plugin', packagesDir, extraFiles: <String>[
+        'example/android/gradlew',
+        'example/android/app/src/androidTest/MainActivityTest.java',
+      ]);
+
+      Error? commandError;
+      final List<String> output = await runCapturingPrint(
+        runner,
+        <String>[
+          'firebase-test-lab',
+          '--device',
+          'model=flame,version=29',
+          '--device',
+          'model=seoul,version=26',
+          '--test-run-id',
+          'testRunId',
+          '--build-id',
+          'buildId',
+        ],
+        errorHandler: (Error e) {
+          commandError = e;
+        },
+      );
+
+      expect(commandError, isA<ToolExit>());
+      expect(
+        output,
+        containsAllInOrder(<Matcher>[
+          contains('Running for plugin'),
+          contains('No integration tests were run'),
+          contains('The following packages had errors:'),
+          contains('plugin:\n'
+              '    No tests ran (use --exclude if this is intentional).'),
+        ]),
+      );
+    });
+
+    test('skips packages with no android directory', () async {
+      createFakePackage('package', packagesDir, extraFiles: <String>[
+        'example/integration_test/foo_test.dart',
+      ]);
+
       final List<String> output = await runCapturingPrint(runner, <String>[
         'firebase-test-lab',
         '--device',
@@ -224,8 +378,8 @@ void main() {
       expect(
         output,
         containsAllInOrder(<Matcher>[
-          contains('Running for plugin'),
-          contains('No example with androidTest directory'),
+          contains('Running for package'),
+          contains('package/example does not support Android'),
         ]),
       );
       expect(output,