fix: remote config e2e stabilization and error handling #8673
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
previously it was attempting to reject but using an undefined helper function to do so, and there was no e2e test to probe it
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
mikehardy
force-pushed
the
remoteConfig-e2e-stabilization
branch
from
dd365b4 to
c5d56e0
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8673 +/- ##
==========================================
+ Coverage 80.06% 80.21% +0.15%
==========================================
Files 181 181
Lines 7637 7637
Branches 1787 1756 -31
==========================================
+ Hits 6114 6125 +11
+ Misses 1401 1394 -7
+ Partials 122 118 -4 🚀 New features to boost your workflow:
|
previously was not a correctly formatted error object and not thrown simply returned - e2e test was probing it incorrectly so this slipped through despite a lot of attention!
this is a non-functional change but brings the helper function codebase up to spec also uses node 22 now and has updated firebase-tools
…rrect version - previously we were still using the old v1 definitions, so no updates were used - previously default instances/concurrency were allowed, so parallel e2e runs could update the template in between the update/alter/publish causing publish errors
previously they were left stale because the delete was not correct
this removes duplicate code in the utility function
…fier this allows arbitrary verifier functions to be passed in, with the default just being one that checks the callback count on the spy
they should handle multiple parallel e2e test runs without blowing up now, so we can hammer on the e2e suite without having to disable these
mikehardy
force-pushed
the
remoteConfig-e2e-stabilization
branch
from
7b32d05 to
41459c0
Compare
|
The other platform bug fixes on error handling are related to work a customer is interested in so I want to merge this and keep moving but as ever, happy to entertain any comments and post a follow-on |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
The flake hammer (big parallel runs of e2e workflows in CI) turned up flakiness in remote-config onConfigUpdated testing
The root cause is that the listener on one test run can (and should!) get events for updates from other test runs, but it was assuming that it would only ever see the events for it's own local test run.
The cloud function was also susceptible to failing under concurrency.
I re-worked the test suite so it uses individualized remote-config params for each local run, and only verifies if those params were updated when it triggers updates - ignoring all the other possible updates.
This turned up a few little errors in our helper suite:
It also exposed that a couple Other platform APIs still weren't returning that they weren't supported as correct errors
Everything should be repaired now
Related issues
Release Summary
Checklist
AndroidiOSOther(macOS, web)e2etests added or updated inpackages/\*\*/e2ejesttests added or updated inpackages/\*\*/__tests__Test Plan
The whole point was to re-enable a big chunk of the remote-config test suite so it's almost all tests.
But the side goal was to fix any errors that it exposed in all the packages and repair them - backed by testing
If e2e passes a flake hammer run, it's good
Think
react-native-firebaseis great? Please consider supporting the project with any of the below:React Native FirebaseandInvertaseon Twitter