-
Notifications
You must be signed in to change notification settings - Fork 929
Fix Firestore build #3066
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
Fix Firestore build #3066
Conversation
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
@@ -1144,7 +1144,7 @@ describe('IndexedDb: allowTabSynchronization', () => { | |||
'clientA', | |||
/* multiClient= */ false, | |||
async db => { | |||
db.injectFailures = true; | |||
db.injectFailures = { updateClientMetadataAndTryBecomePrimary: true }; |
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.
Are you planning on making the other clients have the same transaction names? (Obviously they won't have the multitab related transactions, but how will this failure mechanism work anywhere else?)
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.
Yes. I am also planning to clean up our names before this happens.
Binary Size ReportAffected SDKs
Test Logs |
This is already part of #3038, but it looks like it broke CI for everyone so I pulled in out into a separate PR.
This broke because #3052 and #3020 were both passing CI on their own but not combined.