-
Notifications
You must be signed in to change notification settings - Fork 624
Firestore: add TODO comments about removing special 'missing index' test logic for multidb #5600
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
Conversation
…est logic for multidb
📝 PRs merging into main branchOur main branch should always be in a releasable state. If you are working on a larger change, or if you don't want this change to see the light of the day just yet, consider using a feature branch first, and only merge into the main branch when the code complete and ready to be released. |
Coverage Report 1Affected ProductsNo changes between base commit (ff6cc57) and merge commit (2f1fde2).Test Logs |
Unit Test Results 180 files + 144 180 suites +144 4m 5s ⏱️ + 3m 12s Results for commit f92d45f. ± Comparison against base commit ff6cc57. This pull request removes 157 and adds 1226 tests. Note that renamed tests count towards both.
|
Size Report 1Affected ProductsNo changes between base commit (ff6cc57) and merge commit (2f1fde2).Test Logs |
Startup Time Report 1Note: Layout is sometimes suboptimal due to limited formatting support on GitHub. Please check this report on GCS. Notes
Startup Times
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Add a comment to Firestore's integration tests about removing special logic when testing for the "missing index" error message. Formerly, the "missing index" error message was different for the default database and a non-default database; however, their logic has been unified, and will be rolled out to production soon. Once rolled out, these tests should be updated to remove the special logic for non-default dbs.
Also, remove the "else" branch that verified a different error message when a non-default db was used because when that change rolls out that "else" block will start to fail, seemingly without reason. This also brings the test into parity with the js sdk.
A similar change was made to the js sdk in firebase/firebase-js-sdk#7874.
Googlers see b/316359394 for details.