Add FXIOS-13434 Refactor the LaunchScreenViewModel to enhance the logic, making it easier to comprehend and facilitating unit testing.
#5
Conversation
…sive tests - Add documentation comments to all public APIs - Use IntroScreenManagerProtocol for better testability - Change launchOrder from optional to empty array for clarity - Add @mainactor to class level - Make properties immutable where possible - Add comprehensive unit tests covering edge cases, sequences, and priority logic - Fix MockLaunchScreenManager initializer to match updated signature
- Extract FxNimbus TOS feature configuration into setTermsOfServiceFeatureEnabled helper - Set TOS feature to false in setUp() to ensure clean test state - Update all tests using TOS feature to use helper method - Improves maintainability: only helper needs updating if YAML config changes
- Add test-only Equatable conformance to LaunchType using @retroactive - Add assertSavedLaunchType helper method that uses LaunchType directly - Replace all guard case statements with cleaner helper method calls - Improves test readability and maintainability
Add hasFinishedLoading flag to prevent loadNextLaunchType from calling launchBrowser() prematurely when called before startLoading() completes. This fixes UI test failures in testCloseTour, testFirstRunTour, and testOnboardingSignIn.
Replace Equatable-based assertions with pattern matching using switch statements, following the pattern used elsewhere in the codebase to avoid adding Equatable conformance just for testing purposes.
…n matching Replace LaunchTypeCase enum with direct LaunchType parameter and use tuple pattern matching in switch statement to reduce code repetition. This makes the helper more idiomatic and easier to maintain.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| } | ||
|
|
||
| let launchType = launchOrder.removeFirst() | ||
| delegate?.launchWith(launchType: launchType) |
There was a problem hiding this comment.
Race condition causes premature browser launch before loading completes
High Severity
The refactored loadNextLaunchType() calls launchBrowser() when launchOrder is empty, but cannot distinguish between "not yet loaded" and "loaded with no screens". In LaunchScreenViewController, viewWillAppear calls loadNextLaunchType() while startLoading() runs asynchronously in a Task. Previously, an empty launchOrder (being nil) caused an early return with no action. Now, the empty array triggers launchBrowser() prematurely, potentially skipping onboarding screens. The hasFinishedLoading flag was added to address this but is never checked.
Benchmark PR from qodo-benchmark#26
Note
Modernizes launch flow and improves testability.
LaunchScreenViewModelto use an explicit, non-optionallaunchOrder(default empty) and simplifiesloadNextLaunchType()to launch the browser when emptylaunchOrder; now immediately callslaunchBrowserwhen no screens, otherwisefinishedLoadingLaunchOrderIntroScreenManagerviaIntroScreenManagerProtocol; updates init signature and stores managers asletMockLaunchScreenViewModelto new initializer and behavior trackingWritten by Cursor Bugbot for commit 7177b7e. Configure here.