[flutter_plugin_tools] Make having no Java unit tests a failure (#4310)
This brings the native Android unit tests in line with the policy of having tests that we expect all plugins to have—unless there's a very specific reason to opt them out—fail when missing instead of skipping when missing, to help guard against errors where we silently fail to run tests we think we are running.
Adds an explicit exclusion list covering the plugins that have a reason to be opted out.
Android portion of flutter/flutter#85469
diff --git a/.cirrus.yml b/.cirrus.yml
index 9ca3a87..a118942 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -182,7 +182,8 @@
- export CIRRUS_COMMIT_MESSAGE=""
# Native integration tests are handled by firebase-test-lab below, so
# only run unit tests.
- - ./script/tool_runner.sh native-test --android --no-integration # must come after apk build
+ # Must come after build-examples.
+ - ./script/tool_runner.sh native-test --android --no-integration --exclude script/configs/exclude_native_unit_android.yaml
firebase_test_lab_script:
# Unsetting CIRRUS_CHANGE_MESSAGE and CIRRUS_COMMIT_MESSAGE as they
# might include non-ASCII characters which makes Gradle crash.
diff --git a/script/configs/exclude_native_unit_android.yaml b/script/configs/exclude_native_unit_android.yaml
new file mode 100644
index 0000000..5ec80ee
--- /dev/null
+++ b/script/configs/exclude_native_unit_android.yaml
@@ -0,0 +1,11 @@
+# Deprecated; no plan to backfill the missing files
+- android_alarm_manager
+- battery
+- device_info/device_info
+- package_info
+- sensors
+- share
+- wifi_info_flutter/wifi_info_flutter
+
+# No need for unit tests:
+- espresso
diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md
index aa73c65..c585bee 100644
--- a/script/tool/CHANGELOG.md
+++ b/script/tool/CHANGELOG.md
@@ -1,3 +1,8 @@
+## NEXT
+
+- `native-test --android` now fails plugins that don't have unit tests,
+ rather than skipping them.
+
## 0.7.1
- Add support for `.pluginToolsConfig.yaml` in the `build-examples` command.
diff --git a/script/tool/lib/src/native_test_command.dart b/script/tool/lib/src/native_test_command.dart
index e50878d..78a82af 100644
--- a/script/tool/lib/src/native_test_command.dart
+++ b/script/tool/lib/src/native_test_command.dart
@@ -242,7 +242,8 @@
final Iterable<RepositoryPackage> examples = plugin.getExamples();
- bool ranTests = false;
+ bool ranUnitTests = false;
+ bool ranAnyTests = false;
bool failed = false;
bool hasMissingBuild = false;
for (final RepositoryPackage example in examples) {
@@ -289,7 +290,8 @@
printError('$exampleName unit tests failed.');
failed = true;
}
- ranTests = true;
+ ranUnitTests = true;
+ ranAnyTests = true;
}
if (runIntegrationTests) {
@@ -311,7 +313,7 @@
printError('$exampleName integration tests failed.');
failed = true;
}
- ranTests = true;
+ ranAnyTests = true;
}
}
@@ -321,7 +323,12 @@
? 'Examples must be built before testing.'
: null);
}
- if (!ranTests) {
+ if (!mode.integrationOnly && !ranUnitTests) {
+ printError('No unit tests ran. Plugins are required to have unit tests.');
+ return _PlatformResult(RunState.failed,
+ error: 'No unit tests ran (use --exclude if this is intentional).');
+ }
+ if (!ranAnyTests) {
return _PlatformResult(RunState.skipped);
}
return _PlatformResult(RunState.succeeded);
diff --git a/script/tool/test/native_test_command_test.dart b/script/tool/test/native_test_command_test.dart
index d1ab11f..f7b2ea5 100644
--- a/script/tool/test/native_test_command_test.dart
+++ b/script/tool/test/native_test_command_test.dart
@@ -430,7 +430,8 @@
],
);
- await runCapturingPrint(runner, <String>['native-test', '--android']);
+ await runCapturingPrint(
+ runner, <String>['native-test', '--android', '--no-unit']);
final Directory androidFolder =
plugin.childDirectory('example').childDirectory('android');
@@ -467,7 +468,8 @@
],
);
- await runCapturingPrint(runner, <String>['native-test', '--android']);
+ await runCapturingPrint(
+ runner, <String>['native-test', '--android', '--no-unit']);
// Nothing should run since those files are all
// integration_test-specific.
@@ -641,7 +643,11 @@
);
final List<String> output = await runCapturingPrint(
- runner, <String>['native-test', '--android']);
+ runner, <String>['native-test', '--android'],
+ errorHandler: (Error e) {
+ // Having no unit tests is fatal, but that's not the point of this
+ // test so just ignore the failure.
+ });
expect(
output,
@@ -654,7 +660,7 @@
]));
});
- test('fails when a test fails', () async {
+ test('fails when a unit test fails', () async {
final Directory pluginDir = createFakePlugin(
'plugin',
packagesDir,
@@ -695,6 +701,84 @@
);
});
+ test('fails when an integration test fails', () async {
+ final Directory pluginDir = createFakePlugin(
+ 'plugin',
+ packagesDir,
+ platformSupport: <String, PlatformDetails>{
+ kPlatformAndroid: const PlatformDetails(PlatformSupport.inline)
+ },
+ extraFiles: <String>[
+ 'example/android/gradlew',
+ 'example/android/app/src/test/example_test.java',
+ 'example/android/app/src/androidTest/IntegrationTest.java',
+ ],
+ );
+
+ final String gradlewPath = pluginDir
+ .childDirectory('example')
+ .childDirectory('android')
+ .childFile('gradlew')
+ .path;
+ processRunner.mockProcessesForExecutable[gradlewPath] = <io.Process>[
+ MockProcess(), // unit passes
+ MockProcess(exitCode: 1), // integration fails
+ ];
+
+ Error? commandError;
+ final List<String> output = await runCapturingPrint(
+ runner, <String>['native-test', '--android'],
+ errorHandler: (Error e) {
+ commandError = e;
+ });
+
+ expect(commandError, isA<ToolExit>());
+
+ expect(
+ output,
+ containsAllInOrder(<Matcher>[
+ contains('plugin/example integration tests failed.'),
+ contains('The following packages had errors:'),
+ contains('plugin')
+ ]),
+ );
+ });
+
+ test('fails if there are no unit tests', () async {
+ createFakePlugin(
+ 'plugin',
+ packagesDir,
+ platformSupport: <String, PlatformDetails>{
+ kPlatformAndroid: const PlatformDetails(PlatformSupport.inline)
+ },
+ extraFiles: <String>[
+ 'example/android/gradlew',
+ 'example/android/app/src/androidTest/IntegrationTest.java',
+ ],
+ );
+
+ Error? commandError;
+ final List<String> output = await runCapturingPrint(
+ runner, <String>['native-test', '--android'],
+ errorHandler: (Error e) {
+ commandError = e;
+ });
+
+ expect(commandError, isA<ToolExit>());
+
+ expect(
+ output,
+ containsAllInOrder(<Matcher>[
+ contains('No Android unit tests found for plugin/example'),
+ contains(
+ 'No unit tests ran. Plugins are required to have unit tests.'),
+ contains('The following packages had errors:'),
+ contains('plugin:\n'
+ ' No unit tests ran (use --exclude if this is intentional).')
+ ]),
+ );
+ });
+
test('skips if Android is not supported', () async {
createFakePlugin(
'plugin',
@@ -713,7 +797,7 @@
);
});
- test('skips when running no tests', () async {
+ test('skips when running no tests in integration-only mode', () async {
createFakePlugin(
'plugin',
packagesDir,
@@ -723,12 +807,11 @@
);
final List<String> output = await runCapturingPrint(
- runner, <String>['native-test', '--android']);
+ runner, <String>['native-test', '--android', '--no-unit']);
expect(
output,
containsAllInOrder(<Matcher>[
- contains('No Android unit tests found for plugin/example'),
contains('No Android integration tests found for plugin/example'),
contains('SKIPPING: No tests found.'),
]),
