Skip to content

Enable FCM Integration Test #3145

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 37 commits into from
Jun 12, 2020
Merged

Enable FCM Integration Test #3145

merged 37 commits into from
Jun 12, 2020

Conversation

zwu52
Copy link
Member

@zwu52 zwu52 commented Jun 1, 2020

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.83). Currently covering topic operations (send/receive get, delete, update, etc)

  • 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.

  • 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.
  • Enable foreground send test
  • Run the IC against some milestone chrome/firefox version (if it makes sense) to ensure backward compatibility.

Kai Wu and others added 26 commits April 14, 2020 11:00
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
@zwu52 zwu52 requested a review from hsubox76 June 1, 2020 23:36
@zwu52 zwu52 requested review from Feiyang1 and hiranya911 as code owners June 1, 2020 23:36
@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jun 2, 2020

Binary Size Report

Affected SDKs

  • @firebase/database

    Type Base (5547861) Head (60c7dc3) Diff
    browser 268 kB 268 kB +499 B (+0.2%)
    esm2017 235 kB 235 kB +498 B (+0.2%)
    main 269 kB 269 kB +499 B (+0.2%)
    module 266 kB 267 kB +499 B (+0.2%)
  • @firebase/firestore

    Type Base (5547861) Head (60c7dc3) Diff
    browser 251 kB 252 kB +905 B (+0.4%)
    esm2017 195 kB 195 kB +350 B (+0.2%)
    main 493 kB 494 kB +1.66 kB (+0.3%)
    module 248 kB 249 kB +853 B (+0.3%)
    react-native ? 195 kB ? (?)
  • @firebase/firestore/lite

    Type Base (5547861) Head (60c7dc3) Diff
    main 6.51 kB 138 kB +132 kB (+2026.0%)
  • @firebase/firestore/memory

    Type Base (5547861) Head (60c7dc3) Diff
    browser 191 kB 192 kB +911 B (+0.5%)
    esm2017 149 kB 149 kB +362 B (+0.2%)
    main 369 kB 370 kB +1.71 kB (+0.5%)
    module 189 kB 190 kB +859 B (+0.5%)
    react-native ? 149 kB ? (?)
  • firebase

    Type Base (5547861) Head (60c7dc3) Diff
    firebase-database.js 187 kB 187 kB +306 B (+0.2%)
    firebase-firestore.js 289 kB 290 kB +851 B (+0.3%)
    firebase-firestore.memory.js 231 kB 232 kB +855 B (+0.4%)
    firebase.js 822 kB 823 kB +1.16 kB (+0.1%)

Test Logs

@zwu52 zwu52 changed the title Test Enable FCM Integration Test Jun 11, 2020
@zwu52 zwu52 merged commit 712dfde into master Jun 12, 2020
@firebase firebase locked and limited conversation to collaborators Jul 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants