From 99c97a85ce65cec480cbe0e18e4c175ea1127a73 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Tue, 20 Jul 2021 11:16:24 -0400 Subject: [PATCH 1/5] [flutter_plugin_tools] Make firebase-test-lab fail when no tests run 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. --- script/tool/CHANGELOG.md | 4 + .../lib/src/firebase_test_lab_command.dart | 26 ++++-- .../test/firebase_test_lab_command_test.dart | 81 ++++++++++++++++++- 3 files changed, 100 insertions(+), 11 deletions(-) diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md index 17b28927538d..cadf6a12a8a1 100644 --- a/script/tool/CHANGELOG.md +++ b/script/tool/CHANGELOG.md @@ -1,6 +1,10 @@ ## NEXT - Improved `license-check` output. +- **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.0 diff --git a/script/tool/lib/src/firebase_test_lab_command.dart b/script/tool/lib/src/firebase_test_lab_command.dart index 5e4d9f080085..0c05c0335937 100644 --- a/script/tool/lib/src/firebase_test_lab_command.dart +++ b/script/tool/lib/src/firebase_test_lab_command.dart @@ -121,20 +121,24 @@ class FirebaseTestLabCommand extends PackageLoopingCommand { @override Future 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( + ['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(['Unable to build example apk']); @@ -191,6 +195,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); diff --git a/script/tool/test/firebase_test_lab_command_test.dart b/script/tool/test/firebase_test_lab_command_test.dart index c265868bbf3e..0bbc3f784694 100644 --- a/script/tool/test/firebase_test_lab_command_test.dart +++ b/script/tool/test/firebase_test_lab_command_test.dart @@ -203,12 +203,87 @@ void main() { ); }); - test('skips packages with no androidTest directory', () async { + test('fails for packages with no androidTest directory', () async { createFakePlugin('plugin', packagesDir, extraFiles: [ 'example/integration_test/foo_test.dart', 'example/android/gradlew', ]); + Error? commandError; + final List output = await runCapturingPrint( + runner, + [ + '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()); + expect( + output, + containsAllInOrder([ + 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: [ + 'example/android/gradlew', + 'example/android/app/src/androidTest/MainActivityTest.java', + ]); + + Error? commandError; + final List output = await runCapturingPrint( + runner, + [ + '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()); + expect( + output, + containsAllInOrder([ + 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: [ + 'example/integration_test/foo_test.dart', + ]); + final List output = await runCapturingPrint(runner, [ 'firebase-test-lab', '--device', @@ -224,8 +299,8 @@ void main() { expect( output, containsAllInOrder([ - contains('Running for plugin'), - contains('No example with androidTest directory'), + contains('Running for package'), + contains('package/example does not support Android'), ]), ); expect(output, From beed326c4c422f6ade7fcb7319367a05c6efbb53 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Tue, 20 Jul 2021 16:32:03 -0400 Subject: [PATCH 2/5] Fix, and test, the hang on second calls to _configureFirebaseProject --- .../lib/src/firebase_test_lab_command.dart | 10 +-- .../test/firebase_test_lab_command_test.dart | 79 +++++++++++++++++++ 2 files changed, 83 insertions(+), 6 deletions(-) diff --git a/script/tool/lib/src/firebase_test_lab_command.dart b/script/tool/lib/src/firebase_test_lab_command.dart index 0c05c0335937..304912824960 100644 --- a/script/tool/lib/src/firebase_test_lab_command.dart +++ b/script/tool/lib/src/firebase_test_lab_command.dart @@ -76,13 +76,12 @@ class FirebaseTestLabCommand extends PackageLoopingCommand { static const String _gradleWrapper = 'gradlew'; - Completer? _firebaseProjectConfigured; + bool _firebaseProjectConfigured = false; Future _configureFirebaseProject() async { - if (_firebaseProjectConfigured != null) { - return _firebaseProjectConfigured!.future; + if (_firebaseProjectConfigured) { + return; } - _firebaseProjectConfigured = Completer(); final String serviceKey = getStringArg('service-key'); if (serviceKey.isEmpty) { @@ -110,13 +109,12 @@ 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 diff --git a/script/tool/test/firebase_test_lab_command_test.dart b/script/tool/test/firebase_test_lab_command_test.dart index 0bbc3f784694..185b9d83f0fe 100644 --- a/script/tool/test/firebase_test_lab_command_test.dart +++ b/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: [ + 'test/plugin_test.dart', + 'example/integration_test/foo_test.dart', + 'example/android/gradlew', + 'example/android/app/src/androidTest/MainActivityTest.java', + ]); + createFakePlugin('plugin2', packagesDir, extraFiles: [ + 'test/plugin_test.dart', + 'example/integration_test/bar_test.dart', + 'example/android/gradlew', + 'example/android/app/src/androidTest/MainActivityTest.java', + ]); + + final List output = await runCapturingPrint(runner, [ + 'firebase-test-lab', + '--device', + 'model=flame,version=29', + '--device', + 'model=seoul,version=26', + '--test-run-id', + 'testRunId', + '--build-id', + 'buildId', + ]); + + expect( + output, + containsAllInOrder([ + 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( + '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: [ 'test/plugin_test.dart', From 0a74919b11a046c7c2e9e0a9dbe9915ddd2392ba Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Tue, 20 Jul 2021 19:39:04 -0400 Subject: [PATCH 3/5] Exclude everything that currently fails --- .cirrus.yml | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index 8f69bd188c06..215c0e2ed357 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -137,6 +137,20 @@ 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 + # shared_preferences/shared_preferences + # 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,shared_preferences/shared_preferences,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. @@ -159,7 +173,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 From 4a012078b7b5e59bcec20b420c70b2531cee52d1 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Tue, 20 Jul 2021 19:40:17 -0400 Subject: [PATCH 4/5] CHANGELOG update --- script/tool/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md index cadf6a12a8a1..efe5acc6be7c 100644 --- a/script/tool/CHANGELOG.md +++ b/script/tool/CHANGELOG.md @@ -1,6 +1,8 @@ ## NEXT - Improved `license-check` output. +- 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 From dc872d9d41a97708ba359550938e75c7e4c1bd23 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Wed, 21 Jul 2021 07:13:17 -0400 Subject: [PATCH 5/5] Add two more to the list --- .cirrus.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index 215c0e2ed357..3a8182607683 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -142,7 +142,9 @@ task: # 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: @@ -150,7 +152,7 @@ task: # 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,shared_preferences/shared_preferences,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" + 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.