Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

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

Merged
merged 6 commits into from
Jul 21, 2021
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
99c97a8
[flutter_plugin_tools] Make firebase-test-lab fail when no tests run
stuartmorgan-g Jul 20, 2021
beed326
Fix, and test, the hang on second calls to _configureFirebaseProject
stuartmorgan-g Jul 20, 2021
0a74919
Exclude everything that currently fails
stuartmorgan-g Jul 20, 2021
4a01207
CHANGELOG update
stuartmorgan-g Jul 20, 2021
dc872d9
Add two more to the list
stuartmorgan-g Jul 21, 2021
77ec6d4
Merge branch 'master' into firebase-test-lab-fail-on-missing
stuartmorgan-g Jul 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion .cirrus.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,22 @@ task:
CHANNEL: "stable"
MAPS_API_KEY: ENCRYPTED[596a9f6bca436694625ac50851dc5da6b4d34cba8025f7db5bc9465142e8cd44e15f69e3507787753accebfc4910d550]
GCLOUD_FIREBASE_TESTLAB_KEY: ENCRYPTED[07586610af1fdfc894e5969f70ef2458341b9b7e9c3b7c4225a663b4a48732b7208a4d91c3b7d45305a6b55fa2a37fc4]
# Currently missing harness files (https://github.com/flutter/flutter/issues/86749):
# camera/camera
# google_sign_in/google_sign_in
# in_app_purchase/in_app_purchase
# in_app_purchase_android
# quick_actions
# shared_preferences/shared_preferences
# url_launcher/url_launcher
# video_player/video_player
# webview_flutter
# Deprecated; no plan to backfill the missing files:
# android_intent,connectivity/connectivity,device_info/device_info,sensors,share,wifi_info_flutter/wifi_info_flutter
# No integration tests to run:
# image_picker/image_picker - Native UI is the critical functionality
# espresso - No Dart code, so no integration tests
PLUGINS_TO_EXCLUDE_INTEGRATION_TESTS: "camera/camera,google_sign_in/google_sign_in,in_app_purchase/in_app_purchase,in_app_purchase_android,quick_actions,shared_preferences/shared_preferences,url_launcher/url_launcher,video_player/video_player,webview_flutter,android_intent,connectivity/connectivity,device_info/device_info,sensors,share,wifi_info_flutter/wifi_info_flutter,image_picker/image_picker,espresso"
build_script:
# Unsetting CIRRUS_CHANGE_MESSAGE and CIRRUS_COMMIT_MESSAGE as they
# might include non-ASCII characters which makes Gradle crash.
Expand All @@ -159,7 +175,7 @@ task:
- export CIRRUS_COMMIT_MESSAGE=""
- if [[ -n "$GCLOUD_FIREBASE_TESTLAB_KEY" ]]; then
- echo $GCLOUD_FIREBASE_TESTLAB_KEY > ${HOME}/gcloud-service-key.json
- ./script/tool_runner.sh firebase-test-lab --device model=flame,version=29 --device model=starqlteue,version=26
- ./script/tool_runner.sh firebase-test-lab --device model=flame,version=29 --device model=starqlteue,version=26 --exclude $PLUGINS_TO_EXCLUDE_INTEGRATION_TESTS
- else
- echo "This user does not have permission to run Firebase Test Lab tests."
- fi
Expand Down
6 changes: 6 additions & 0 deletions script/tool/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
36 changes: 22 additions & 14 deletions script/tool/lib/src/firebase_test_lab_command.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -110,31 +109,34 @@ class FirebaseTestLabCommand extends PackageLoopingCommand {
print('');
if (exitCode == 0) {
print('Firebase project configured.');
return;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the actual fix for the bug, but the Completer was needlessly complex so I cleaned it up as well.

} 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']);
Expand Down Expand Up @@ -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);
Expand Down
160 changes: 157 additions & 3 deletions script/tool/test/firebase_test_lab_command_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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).'),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: does it make sense to say where --exclude should be used? e.g. in .cirrus.yml?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be useful locally as well; in any scenario where someone sees this message, they are trying to run a failing package, so might want to exclude it.

]),
);
});

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',
Expand All @@ -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,
Expand Down