test: migrate all packages from Flutter driver to integration tests #9649

nilsreichardt · 2022-10-01T12:35:35Z

Description

The integration_test package is the new way to write e2e tests. Therefore, this PR migrates the tests from driver tests to integration tests.

As side effect, we see that the Android CI e2e tests pipeline is way more stable.

Screen.Recording.2022-10-02.at.11.05.28.mov

ToDos:

Related Issues

Closes #8909

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]).
This will ensure a smooth and quick review process. Updating the pubspec.yaml and changelogs is not required.

I read the Contributor Guide and followed the process outlined there for submitting PRs.
My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
All existing and new tests are passing.
I updated/added relevant documentation (doc comments with ///).
The analyzer (melos run analyze) does not report any problems on my PR.
I read and followed the Flutter Style Guide.
I signed the CLA.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

Yes, this is a breaking change.
No, this is not a breaking change.

…ase`

…json`

.vscode/tasks.json

CONTRIBUTING.md

packages/cloud_firestore_odm/cloud_firestore_odm/example/build.yaml

nilsreichardt · 2022-10-01T13:01:40Z

packages/cloud_functions/cloud_functions/pubspec.yaml

  flutter_test:
    sdk: flutter
  plugin_platform_interface: ^2.0.0
-  test: any


I removed the test dependencies in all packages (except _flutterfire_internals) because the packages already have flutter_test as a dependency. Or is there a reason why we have two test dependencies?

Makes sense to me I think.

tests/android/app/build.gradle

nilsreichardt · 2022-10-01T13:03:11Z

packages/flutterfire_ui/example/test_driver/integration_test.dart

+import 'package:integration_test/integration_test_driver.dart';
+
+Future<void> main() => integrationDriver();


Are the tests of flutterfire used? They are not in the CI, right?

We don't have integration tests for flutterfire_ui if that is your question 😄

But there are some tests in example folder of the flutterfire_ui 🤔 https://github.com/firebase/flutterfire/tree/master/packages/flutterfire_ui/example/test_driver

Huh, they aren't part of our CI e2e tests. I think you can leave them for now and we'll integrate them at some other point. Unless @lesnitsky thinks otherwise?

Should we add them as well? If yes, I can make a new issue and integrate them in a separate PR into our CI 👍

tests/integration_test/firebase_auth/firebase_auth_multi_factor_e2e_test.dart

nilsreichardt · 2022-10-01T13:05:46Z

packages/firebase_auth/firebase_auth/example/test_driver/user_e2e.dart

@@ -1,898 +0,0 @@
-// ignore_for_file: require_trailing_commas


Removing the tests from the firebase_auth from packages/firebase_auth/firebase_auth/example/ because the source of truth should be in tests, right?

Makes sense to me. Double check with @Salakar as he set it up and might have a reason why e2e tests remain in example app.

nilsreichardt · 2022-10-01T21:09:01Z

tests/integration_test/cloud_functions/cloud_functions_e2e_test.dart

+        },
+        // Android skip because it's flaky. See:
+        // https://github.com/firebase/flutterfire/issues/9652
+        skip: defaultTargetPlatform == TargetPlatform.android,


Skipping this test because it's flaky. See: #9652

nilsreichardt · 2022-10-01T21:09:23Z

tests/integration_test/firebase_messaging/firebase_messaging_e2e_test.dart

+          // Android skipped because it times out with a likelihood of 98%. See: https://github.com/firebase/flutterfire/issues/9651
+          skip: kIsWeb ||
+              defaultTargetPlatform == TargetPlatform.macOS ||
+              defaultTargetPlatform == TargetPlatform.android,


Android skipped because it times out with a likelihood of 98%. See: #9651

nilsreichardt · 2022-10-01T21:09:33Z

tests/integration_test/firebase_messaging/firebase_messaging_e2e_test.dart

+          // Android skipped because it times out with a likelihood of 98%. See: https://github.com/firebase/flutterfire/issues/9650
+          skip: kIsWeb ||
+              defaultTargetPlatform == TargetPlatform.macOS ||
+              defaultTargetPlatform == TargetPlatform.android,


Android skipped because it times out with a likelihood of 98%. See: #9650

russellwheatley

LGTM provided a couple of questions I'm not sure on are answered by the rest of the core team. Good work, @nilsreichardt 🎉

packages/cloud_firestore_odm/cloud_firestore_odm/example/build.yaml

russellwheatley · 2022-10-03T11:54:50Z

packages/cloud_functions/cloud_functions/pubspec.yaml

  flutter_test:
    sdk: flutter
  plugin_platform_interface: ^2.0.0
-  test: any


Makes sense to me I think.

russellwheatley · 2022-10-03T12:08:39Z

packages/flutterfire_ui/example/test_driver/integration_test.dart

+import 'package:integration_test/integration_test_driver.dart';
+
+Future<void> main() => integrationDriver();


We don't have integration tests for flutterfire_ui if that is your question 😄

russellwheatley · 2022-10-03T12:16:15Z

packages/firebase_auth/firebase_auth/example/test_driver/user_e2e.dart

@@ -1,898 +0,0 @@
-// ignore_for_file: require_trailing_commas


Makes sense to me. Double check with @Salakar as he set it up and might have a reason why e2e tests remain in example app.

nilsreichardt · 2022-10-03T18:59:12Z

Would it be possible to break this PR down into a few smaller ones?

Reviewing changes on 123 files is not reasonably doable.

Half of the migration has already been reviewed in #9230 (see #8910, #8946, #8938, #8972, #8975, #8974, #8978, #8973, #8977). However, cherry-picking the commits in separate branches should be relatively easy because I committed in very small steps.

Is still preferred? Yes, I can do this 👍

bartekpacia · 2022-10-07T12:36:58Z

...firestore_odm/cloud_firestore_odm/example/integration_test/cloud_firestore_odm_e2e_test.dart

+import 'query_reference_test.dart' as query_reference_test;
+
+void main() {
+  IntegrationTestWidgetsFlutterBinding.ensureInitialized();


afaik this isn't required, because the flutter tool creates a temporary file where the call to IntegrationTestWidgetsFlutterBinding.ensureInitialized() is automatically inserted. See flutter/flutter#92254 for more details :)

Thanks for the note, @bartekpacia. However, it seems that without IntegrationTestWidgetsFlutterBinding.ensureInitialzed() the tests doesn't on web. Maybe it's related with flutter/flutter#66264

See:

https://github.com/firebase/flutterfire/actions/runs/3213776146/jobs/5253698990

https://github.com/firebase/flutterfire/actions/runs/3213776149/jobs/5253699165

https://github.com/firebase/flutterfire/actions/runs/3213776150/jobs/5253699260

…d()`" This reverts commit f117e20.

Lyokone · 2022-10-31T07:38:04Z

Reviewed carefully and everything seems fine. Some files are deleted because they were duplicates not being used anymore. LGTM

Lyokone · 2022-10-31T07:50:55Z

CONTRIBUTING.md

+cd .github/workflows/scripts
+firebase emulators:start --only auth,firestore,functions,storage,database --project flutterfire-e2e-tests


You have the melos run firebase:emulator than can be used

nilsreichardt added 30 commits October 1, 2022 11:40

Remove Flutter Driver tests

acaad19

Remove drive dependency

6e14cb9

Add integration_test as dependency

f5ab85d

Setup integration test and migrate firebase_core

cace857

Migrate firebase_analytics

8cd393a

Migrate cloud_functions

f740e69

Migrate firebase_app_check

bc2fe33

Migrate firebase_app_installations

2889d8b

Migrate firebase_auth

eaf42c5

Migrate firebase_crashlytics

74f47c6

Migrate firebase_database

96b1030

Add missing IntegrationTestWidgetsFlutterBinding to `firebase_datab…

12acdf0

…ase`

Migrate firebase_dynamic_links

a2c55e7

Migrate firebase_messaging

efb62b2

Migrate firebase_ml_model_downloader

a6f9b4f

Migrate firebase_remote_config

055bbaf

Migrate firebase_storage

3aa0992

Migrate cloud_firestore

3034fa6

Migrate cloud_firestore_odm_e2e.dart

de19624

Fix port and move to group for cloud_firestore

ddeb590

Migrate firebase_performance

6c230af

Fix android build

abc6cd8

Update test commands

d7255fc

Delete unused cloud_functions Flutter driver tests

c412716

Delete unused firebase_analytics Flutter driver tests

13e8da1

Delete unused firebase_auth Flutter driver tests

eb5cda0

Delete unused firebase_core Flutter driver tests

7a292d1

Remove unused dev_dependencies in firebase_in_app_messaging_example

6bb66ed

Remove unused dev_dependencies in firebase_ml_model_downloader

375fc02

Remove unused dev_dependencies in firebase_performance

2152519

nilsreichardt added 2 commits October 1, 2022 14:55

Add missing --driver=./test_driver/integration_test.dart in `tasks.…

913f6ec

…json`

Changed to sync cloud_firestore_odm_e2e_test.dart

2072c17

nilsreichardt commented Oct 1, 2022

View reviewed changes

nilsreichardt mentioned this pull request Oct 1, 2022

test: migrate all packages from Flutter driver to integration tests #9230

Closed

31 tasks

nilsreichardt added 4 commits October 1, 2022 21:13

Check flakiness

bfc2b59

Skip "subscribeToTopic" and "unsubscribeFromTopic" for Android

ea9c124

comment out concurrency for e2e tests

b06cd3c

Skip "times out when the provided timeout option is exceeded" test

a3da82b

nilsreichardt commented Oct 1, 2022

View reviewed changes

Remove testing stuff

a6c7714

nilsreichardt marked this pull request as ready for review October 2, 2022 09:08

nilsreichardt requested review from Salakar, kroikie and lesnitsky as code owners October 2, 2022 09:08

russellwheatley reviewed Oct 3, 2022

View reviewed changes

This comment was marked as outdated.

Sign in to view

bartekpacia reviewed Oct 7, 2022

View reviewed changes

nilsreichardt added 6 commits October 7, 2022 20:37

Merge branch 'master' into migrate-to-integration-tests-rework

7034d76

Remove IntegrationTestWidgetsFlutterBinding.ensureInitialized()

f117e20

Revert "Remove `IntegrationTestWidgetsFlutterBinding.ensureInitialize…

36487e1

…d()`" This reverts commit f117e20.

Re-trigger CI

6fdc1a3

Re-trigger CI

56a275a

Merge branch 'master' into migrate-to-integration-tests-rework

62f2e06

Lyokone approved these changes Oct 31, 2022

View reviewed changes

Lyokone reviewed Oct 31, 2022

View reviewed changes

Salakar approved these changes Oct 31, 2022

View reviewed changes

Lyokone merged commit 1f061a8 into firebase:master Oct 31, 2022

firebase locked and limited conversation to collaborators Dec 1, 2022

		import 'package:integration_test/integration_test_driver.dart';

		Future<void> main() => integrationDriver();

		@@ -1,898 +0,0 @@
		// ignore_for_file: require_trailing_commas

		cd .github/workflows/scripts
		firebase emulators:start --only auth,firestore,functions,storage,database --project flutterfire-e2e-tests

test: migrate all packages from Flutter driver to integration tests #9649

test: migrate all packages from Flutter driver to integration tests #9649

Uh oh!

Conversation

nilsreichardt commented Oct 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

ToDos:

Related Issues

Checklist

Breaking Change

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nilsreichardt Oct 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

russellwheatley left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

nilsreichardt commented Oct 3, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Lyokone commented Oct 31, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

nilsreichardt commented Oct 1, 2022 •

edited

Loading

nilsreichardt Oct 5, 2022 •

edited

Loading