Add Foreground Message Send/Receive Integration Tests #3216
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Context: the tests were switched off a couple of years ago due to flakiness. It has not been maintained since then. During the time, Some Selenium API methods used were deprecated and removed; the Firebase projects used are no longer managed or owned by the FCM team. Consequently, the test became unfunctional. In the effort of providing safety for the upcoming FCM releases, this PR is created to fix, deflake, refactor and improve the old tests.
This PR did the following:
- Enabled comprehensive IT for chrome (ver.80). Now we are covering send&foreground recevie for FCM messages (messages with {notification} payload, {data} payload and {notification, data} payload), delete/update token, use default/customized ServiceWorker.
- Defalaked test. The IT is now reasonably stable without retry. Previously we are retrying 1 or 3 times.
- Optimized test. Previously we create a webDriver for each test, which is slow and annoying. Because a window is created and brought to focus and killed frequently, it makes working on other tasks and testing nearly impossible (Probably using a headless browser would work but I haven't found a satisfying solution to have the app in the state of foreground and background which is a requirement for FCM functions). With the way the tests are organized, the IT only spin up a new web driver when necessary. Some data on performance: (old) 'test-send' take 74 seconds (only measured 'test-send' because the other test suites were not functional at the time); (now) 'test-send', 'test-deleteToken', 'test-updateToken', 'test-useDefaultServiceWorker', 'test-useValidManifest' takes in total 33s (10 run average).
- General refactoring. Including refactors on expect blocks, createWebDriver, use const for constants usage, etc. The code should be much easier to understand and maintain.
Future work:
- Enable test on firefox once I get the notification permission working.
- Run the IC against some milestone chrome/firefox version (if it makes sense) to ensure backward compatibility. We should try to avoid #2712 . :)
Context: FCM IT were turned off a couple years ago due to flakiness. It became mostly unfunctional as repo structure change overtime. The goal is to fix and enable IT for more confident developement flow and safer releases. This CL does the following: - Fix the IT to be functional again. The IT is derteminated from my experiements (no longer flaky). Therefore, The CL removed the retry mechasim (previously retry 3 times) which makes running IT cheaper and more enjoyable. - Include IT for test:change for FCM package: the entire IT test suites is resoanblly fast (from my exeperience 1-3 miutes to complete. As it grows larger, maybe it makes run tests in parallel in Saucelab) Futhure work: - Enable testing for firefox
js doesn't allow underscore for int
why dot.env(FCM_SECRETS) are not included in env?
Because headless chrome doesn't have the concept of foreground and background
somehow not workingin github workflow?
Binary Size ReportAffected SDKsNo changes between base commit (992432e) and head commit (8840804). Test Logs
|
…into test Sync to Head
🦋 Changeset is good to goLatest commit: fd7d493 We got this. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Not a SDK Change
hsubox76
approved these changes
Jun 26, 2020
…into test Sync to head
…into test sync to head
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Continue on PR, this PR enables the integration tests that checks the onMessage hook where foreground messages are received, including FCM messages that contain "data" payload, "notification" payload or both. ({data}, {notification}, {data, notification}).
With this PR, the FCM integration test suite covers all API the SDK offers. Note that from experiment, the "test-send" takes 5-7 minutes to complete which is fairly good given the 3 minutes waiting time induced with the background message testing.