Skip to content

Commit

Permalink
[flutter_plugin_tools] Make having no Java unit tests a failure (#4310)
Browse files Browse the repository at this point in the history
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 #85469
  • Loading branch information
stuartmorgan committed Sep 10, 2021
1 parent b38c4e4 commit a4f0e88
Show file tree
Hide file tree
Showing 5 changed files with 119 additions and 12 deletions.
3 changes: 2 additions & 1 deletion .cirrus.yml
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,8 @@ task:
- 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.
Expand Down
11 changes: 11 additions & 0 deletions script/configs/exclude_native_unit_android.yaml
Original file line number Diff line number Diff line change
@@ -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
5 changes: 5 additions & 0 deletions script/tool/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
15 changes: 11 additions & 4 deletions script/tool/lib/src/native_test_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,8 @@ this command.

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) {
Expand Down Expand Up @@ -289,7 +290,8 @@ this command.
printError('$exampleName unit tests failed.');
failed = true;
}
ranTests = true;
ranUnitTests = true;
ranAnyTests = true;
}

if (runIntegrationTests) {
Expand All @@ -311,7 +313,7 @@ this command.
printError('$exampleName integration tests failed.');
failed = true;
}
ranTests = true;
ranAnyTests = true;
}
}

Expand All @@ -321,7 +323,12 @@ this command.
? '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);
Expand Down
97 changes: 90 additions & 7 deletions script/tool/test/native_test_command_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,8 @@ void main() {
],
);

await runCapturingPrint(runner, <String>['native-test', '--android']);
await runCapturingPrint(
runner, <String>['native-test', '--android', '--no-unit']);

final Directory androidFolder =
plugin.childDirectory('example').childDirectory('android');
Expand Down Expand Up @@ -467,7 +468,8 @@ void main() {
],
);

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.
Expand Down Expand Up @@ -641,7 +643,11 @@ void main() {
);

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,
Expand All @@ -654,7 +660,7 @@ void main() {
]));
});

test('fails when a test fails', () async {
test('fails when a unit test fails', () async {
final Directory pluginDir = createFakePlugin(
'plugin',
packagesDir,
Expand Down Expand Up @@ -695,6 +701,84 @@ void main() {
);
});

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',
Expand All @@ -713,7 +797,7 @@ void main() {
);
});

test('skips when running no tests', () async {
test('skips when running no tests in integration-only mode', () async {
createFakePlugin(
'plugin',
packagesDir,
Expand All @@ -723,12 +807,11 @@ void main() {
);

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.'),
]),
Expand Down

0 comments on commit a4f0e88

Please sign in to comment.