Conversation
Contributor
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. |
…ests The Swift Concurrency refactor dropped the explicit `loggedEventCallbackQueue: .global(qos: .userInteractive)` argument, so the unit tests fell back to the production default of `.global(qos: .background)` while still asserting against a 3 second expectation timeout. Background QoS is aggressively deprioritized on contended machines, which made the callback arrive late and caused non-deterministic CI failures: a different test timed out on each retry, and the tests that did pass swung between 0.037s and 2.837s. Toggling only this line locally on an idle machine moves the affected suites from 0.728s to 0.027s, with individual tests 40-60x faster. Restore the argument and the comment explaining why it exists.
…gate `waitUntilAllRegistered()` now returns the registered subscribers instead of requiring a second `currentSubscribers` access, so the session start path needs one actor hop rather than two. Behavior is unchanged: the snapshot is still read after the gate opens. Also document the invariants that make the actor correct, since they are load-bearing and not obvious: - `register(...)` is deliberately non-async, so the de-duplication check, the append and the continuation resume cannot interleave. - There is no lost-wakeup race, because the `withCheckedContinuation` body runs synchronously inside the actor before the caller suspends. - The gate latches, which is what lets session starts after the first one resolve without suspending, matching the previous promise behavior. - `register(subscriber:)` is now asynchronous, so returning from it no longer implies the subscriber is visible to an in-flight session start. - `SessionStartEvent` is `@unchecked Sendable` only because it is handed off between executors and never shared; it wraps manually managed nanopb pointers, so concurrent mutation would corrupt memory. Restore the init doc comment describing `loggedEventCallbackQueue`.
The existing tests asserted `XCTAssertTrue(true)`, so they verified nothing. In particular `test_waitUntilAllRegistered_waitsForDependencies` would have passed even if the gate opened immediately, because it never asserted that the waiter was still suspended, and a regression would have hung rather than failed. Replace them with tests that assert real behavior: - the gate stays closed until every expected subscriber registers - a subscriber that was never declared as a dependency cannot open the gate on its own, but is still tracked - the gate stays open for subsequent session starts - every queued continuation is resumed, not just the first - registration is idempotent per subscriber name - concurrent registrations are serialized and de-duplicated Verified by mutation: stubbing the gate to return immediately fails two of the new tests, while all three of the previous tests passed under the same mutation.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Note: This is non-breaking, but planned for Firebase 13.0 to prevent risk of introducing a regression in a final 12.x release.
Summary
Replaces the
Promiseslibrary dependency inFirebaseSessionswith native Swift Concurrency.Key Changes
FBLPromisessubscriber tracking inFirebaseSessionswith a new internalSessionsStateactor.PromisesfromPackage.swiftandFirebaseSessions.podspec.FirebaseSessionsunit test suite and base helpers to useasync/awaitand addedSessionsStateTests.#no-changelog