[tool] Check Java integration test configuration (flutter#3499)

[tool] Check Java integration test configuration

Author: stuartmorgan-g

script/tool/lib/src/native_test_command.dart

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 import 'package:file/file.dart';
+import 'package:meta/meta.dart';
 import 'package:platform/platform.dart';
 
 import 'common/cmake.dart';
@@ -21,6 +22,16 @@ const String _iOSDestinationFlag = 'ios-destination';
 
 const int _exitNoIOSSimulators = 3;
 
+/// The error message logged when a FlutterTestRunner test is not annotated with
+/// @DartIntegrationTest.
+@visibleForTesting
+const String misconfiguredJavaIntegrationTestErrorExplanation =
+    'The following files use @RunWith(FlutterTestRunner.class) '
+    'but not @DartIntegrationTest, which will cause hangs when run with '
+    'this command. See '
+    'https://github.com/flutter/flutter/wiki/Plugin-Tests#enabling-android-ui-tests '
+    'for instructions.';
+
 /// The command to run native tests for plugins:
 /// - iOS and macOS: XCTests (XCUnitTest and XCUITest)
 /// - Android: JUnit tests
@@ -211,7 +222,17 @@ this command.
               .existsSync();
     }
 
-    bool exampleHasNativeIntegrationTests(RepositoryPackage example) {
+    _JavaTestInfo getJavaTestInfo(File testFile) {
+      final List<String> contents = testFile.readAsLinesSync();
+      return _JavaTestInfo(
+          usesFlutterTestRunner: contents.any((String line) =>
+              line.trimLeft().startsWith('@RunWith(FlutterTestRunner.class)')),
+          hasDartIntegrationTestAnnotation: contents.any((String line) =>
+              line.trimLeft().startsWith('@DartIntegrationTest')));
+    }
+
+    Map<File, _JavaTestInfo> findIntegrationTestFiles(
+        RepositoryPackage example) {
       final Directory integrationTestDirectory = example
           .platformDirectory(FlutterPlatform.android)
           .childDirectory('app')
@@ -220,25 +241,30 @@ this command.
       // There are two types of integration tests that can be in the androidTest
       // directory:
       // - FlutterTestRunner.class tests, which bridge to Dart integration tests
-      // - Purely native tests
+      // - Purely native integration tests
       // Only the latter is supported by this command; the former will hang if
       // run here because they will wait for a Dart call that will never come.
       //
-      // This repository uses a convention of putting the former in a
-      // *ActivityTest.java file, so ignore that file when checking for tests.
-      // Also ignore DartIntegrationTest.java, which defines the annotation used
-      // below for filtering the former out when running tests.
+      // Find all test files, and determine which kind of test they are based on
+      // the annotations they use.
       //
-      // If those are the only files, then there are no tests to run here.
-      return integrationTestDirectory.existsSync() &&
-          integrationTestDirectory
-              .listSync(recursive: true)
-              .whereType<File>()
-              .any((File file) {
-            final String basename = file.basename;
-            return !basename.endsWith('ActivityTest.java') &&
-                basename != 'DartIntegrationTest.java';
-          });
+      // Ignore DartIntegrationTest.java, which defines the annotation used
+      // below for filtering the former out when running tests.
+      if (!integrationTestDirectory.existsSync()) {
+        return <File, _JavaTestInfo>{};
+      }
+      final Iterable<File> integrationTestFiles = integrationTestDirectory
+          .listSync(recursive: true)
+          .whereType<File>()
+          .where((File file) {
+        final String basename = file.basename;
+        return basename != 'DartIntegrationTest.java' &&
+            basename != 'DartIntegrationTest.kt';
+      });
+      return <File, _JavaTestInfo>{
+        for (final File file in integrationTestFiles)
+          file: getJavaTestInfo(file)
+      };
     }
 
     final Iterable<RepositoryPackage> examples = plugin.getExamples();
@@ -247,10 +273,17 @@ this command.
     bool ranAnyTests = false;
     bool failed = false;
     bool hasMissingBuild = false;
+    bool hasMisconfiguredIntegrationTest = false;
+    // Iterate through all examples (in the rare case that there is more than
+    // one example); running any tests found for each one. Requirements on what
+    // tests are present are enforced at the overall package level, not a per-
+    // example level. E.g., it's fine for only one example to have unit tests.
     for (final RepositoryPackage example in examples) {
       final bool hasUnitTests = exampleHasUnitTests(example);
-      final bool hasIntegrationTests =
-          exampleHasNativeIntegrationTests(example);
+      final Map<File, _JavaTestInfo> candidateIntegrationTestFiles =
+          findIntegrationTestFiles(example);
+      final bool hasIntegrationTests = candidateIntegrationTestFiles.values
+          .any((_JavaTestInfo info) => !info.hasDartIntegrationTestAnnotation);
 
       if (mode.unit && !hasUnitTests) {
         _printNoExampleTestsMessage(example, 'Android unit');
@@ -295,24 +328,41 @@ this command.
 
       if (runIntegrationTests) {
         // FlutterTestRunner-based tests will hang forever if run in a normal
-        // app build, since they wait for a Dart call from integration_test that
-        // will never come. Those tests have an extra annotation to allow
+        // app build, since they wait for a Dart call from integration_test
+        // that will never come. Those tests have an extra annotation to allow
         // filtering them out.
-        const String filter =
-            'notAnnotation=io.flutter.plugins.DartIntegrationTest';
-
-        print('Running integration tests...');
-        final int exitCode = await project.runCommand(
-          'app:connectedAndroidTest',
-          arguments: <String>[
-            '-Pandroid.testInstrumentationRunnerArguments.$filter',
-          ],
-        );
-        if (exitCode != 0) {
-          printError('$exampleName integration tests failed.');
-          failed = true;
+        final List<File> misconfiguredTestFiles = candidateIntegrationTestFiles
+            .entries
+            .where((MapEntry<File, _JavaTestInfo> entry) =>
+                entry.value.usesFlutterTestRunner &&
+                !entry.value.hasDartIntegrationTestAnnotation)
+            .map((MapEntry<File, _JavaTestInfo> entry) => entry.key)
+            .toList();
+        if (misconfiguredTestFiles.isEmpty) {
+          // Ideally we would filter out @RunWith(FlutterTestRunner.class)
+          // tests directly, but there doesn't seem to be a way to filter based
+          // on annotation contents, so the DartIntegrationTest annotation was
+          // created as a proxy for that.
+          const String filter =
+              'notAnnotation=io.flutter.plugins.DartIntegrationTest';
+
+          print('Running integration tests...');
+          final int exitCode = await project.runCommand(
+            'app:connectedAndroidTest',
+            arguments: <String>[
+              '-Pandroid.testInstrumentationRunnerArguments.$filter',
+            ],
+          );
+          if (exitCode != 0) {
+            printError('$exampleName integration tests failed.');
+            failed = true;
+          }
+          ranAnyTests = true;
+        } else {
+          hasMisconfiguredIntegrationTest = true;
+          printError('$misconfiguredJavaIntegrationTestErrorExplanation\n'
+              '${misconfiguredTestFiles.map((File f) => '  ${f.path}').join('\n')}');
         }
-        ranAnyTests = true;
       }
     }
 
@@ -322,6 +372,10 @@ this command.
               ? 'Examples must be built before testing.'
               : null);
     }
+    if (hasMisconfiguredIntegrationTest) {
+      return _PlatformResult(RunState.failed,
+          error: 'Misconfigured integration test.');
+    }
     if (!mode.integrationOnly && !ranUnitTests) {
       printError('No unit tests ran. Plugins are required to have unit tests.');
       return _PlatformResult(RunState.failed,
@@ -622,3 +676,16 @@ class _PlatformResult {
   /// Ignored unless [state] is `failed`.
   final String? error;
 }
+
+/// The state of a .java test file.
+class _JavaTestInfo {
+  const _JavaTestInfo(
+      {required this.usesFlutterTestRunner,
+      required this.hasDartIntegrationTestAnnotation});
+
+  /// Whether the test class uses the FlutterTestRunner.
+  final bool usesFlutterTestRunner;
+
+  /// Whether the test class has the @DartIntegrationTest annotation.
+  final bool hasDartIntegrationTestAnnotation;
+}

script/tool/test/native_test_command_test.dart

@@ -13,6 +13,7 @@ import 'package:flutter_plugin_tools/src/common/core.dart';
 import 'package:flutter_plugin_tools/src/common/file_utils.dart';
 import 'package:flutter_plugin_tools/src/common/plugin_utils.dart';
 import 'package:flutter_plugin_tools/src/native_test_command.dart';
+import 'package:path/path.dart' as p;
 import 'package:platform/platform.dart';
 import 'package:test/test.dart';
 
@@ -511,9 +512,11 @@ void main() {
       });
 
       test(
-          'ignores Java integration test files associated with integration_test',
+          'ignores Java integration test files using (or defining) DartIntegrationTest',
           () async {
-        createFakePlugin(
+        const String dartTestDriverRelativePath =
+            'android/app/src/androidTest/java/io/flutter/plugins/plugin/FlutterActivityTest.java';
+        final RepositoryPackage plugin = createFakePlugin(
           'plugin',
           packagesDir,
           platformSupport: <String, PlatformDetails>{
@@ -522,11 +525,24 @@ void main() {
           extraFiles: <String>[
             'example/android/gradlew',
             'example/android/app/src/androidTest/java/io/flutter/plugins/DartIntegrationTest.java',
-            'example/android/app/src/androidTest/java/io/flutter/plugins/plugin/FlutterActivityTest.java',
-            'example/android/app/src/androidTest/java/io/flutter/plugins/plugin/MainActivityTest.java',
+            'example/android/app/src/androidTest/java/io/flutter/plugins/DartIntegrationTest.kt',
+            'example/$dartTestDriverRelativePath',
           ],
         );
 
+        final File dartTestDriverFile = childFileWithSubcomponents(
+            plugin.getExamples().first.directory,
+            p.posix.split(dartTestDriverRelativePath));
+        dartTestDriverFile.writeAsStringSync('''
+import io.flutter.plugins.DartIntegrationTest;
+import org.junit.runner.RunWith;
+
+@DartIntegrationTest
+@RunWith(FlutterTestRunner.class)
+public class FlutterActivityTest {
+}
+''');
+
         await runCapturingPrint(
             runner, <String>['native-test', '--android', '--no-unit']);
 
@@ -538,6 +554,53 @@ void main() {
         );
       });
 
+      test(
+          'fails for Java integration tests Using FlutterTestRunner without @DartIntegrationTest',
+          () async {
+        const String dartTestDriverRelativePath =
+            'android/app/src/androidTest/java/io/flutter/plugins/plugin/FlutterActivityTest.java';
+        final RepositoryPackage plugin = createFakePlugin(
+          'plugin',
+          packagesDir,
+          platformSupport: <String, PlatformDetails>{
+            platformAndroid: const PlatformDetails(PlatformSupport.inline)
+          },
+          extraFiles: <String>[
+            'example/android/gradlew',
+            'example/$dartTestDriverRelativePath',
+          ],
+        );
+
+        final File dartTestDriverFile = childFileWithSubcomponents(
+            plugin.getExamples().first.directory,
+            p.posix.split(dartTestDriverRelativePath));
+        dartTestDriverFile.writeAsStringSync('''
+import io.flutter.plugins.DartIntegrationTest;
+import org.junit.runner.RunWith;
+
+@RunWith(FlutterTestRunner.class)
+public class FlutterActivityTest {
+}
+''');
+
+        Error? commandError;
+        final List<String> output = await runCapturingPrint(
+            runner, <String>['native-test', '--android', '--no-unit'],
+            errorHandler: (Error e) {
+          commandError = e;
+        });
+
+        expect(commandError, isA<ToolExit>());
+        expect(
+            output,
+            contains(
+                contains(misconfiguredJavaIntegrationTestErrorExplanation)));
+        expect(
+            output,
+            contains(contains(
+                'example/android/app/src/androidTest/java/io/flutter/plugins/plugin/FlutterActivityTest.java')));
+      });
+
       test('runs all tests when present', () async {
         final RepositoryPackage plugin = createFakePlugin(
           'plugin',