-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor FXIOS-14466 FXIOS-14463 [Swift 6 Migration] Migrate ShareTo and BrowserKit Package to Swift 6 #20
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
base: cursor_combined_20260121_qodo_grep_cursor_copilot_1_base_refactor_fxios-14466_fxios-14463_swift_6_migration_migrate_shareto_and_browserkit_package_to_swift_6_pr220
Are you sure you want to change the base?
Refactor FXIOS-14466 FXIOS-14463 [Swift 6 Migration] Migrate ShareTo and BrowserKit Package to Swift 6 #20
Conversation
…es are enabled in Swift 6.2 by default.
…t does not seem to require @mainactor initialization anymore (and calling `startMockImageServer` on the main thread will causes the addHandler callback to crash for not being called on the main thread).
…currency warnings.
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.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| XCTAssertEqual(result.originalData, imageData) | ||
| XCTAssertEqual(result.url, mockedURL) | ||
| XCTAssertEqual(result.image.size, expectedRasterSize) | ||
| exp.fulfill() |
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.
Test missing expectation for async callback assertions
Medium Severity
The testDownloadingSVGImage_withKingfisherProcessor_forStandardSVGCase test removed all expectation handling while other tests in the same file retained theirs. The downloadImage call is asynchronous (network download), but the test no longer waits for the completion callback. The assertions inside the callback (XCTAssertEqual for originalData, url, and image.size) will never execute before the test completes, causing the test to pass without verifying anything.
| throw MockImageServerError.noAssetData | ||
| } | ||
|
|
||
| // GDCWebServer must be initialized for the first time on the main thread |
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.
Removed crash-preventing @mainactor from GCDWebServer initialization
Low Severity
The @MainActor annotation was removed from startMockImageServer along with a comment explicitly documenting that "GDCWebServer must be initialized for the first time on the main thread" and that the annotation "ensures that if this test file is run alone it will not crash." Since the test class is @unchecked Sendable (not @MainActor isolated), there's no guarantee tests run on the main thread in Swift 6, potentially causing intermittent crashes during test execution.
Benchmark PR from qodo-benchmark#220
Note
Swift 6 migration and concurrency updates
Package.swiftto tools6.2; remove experimental flags (StrictConcurrency,RegionBasedIsolation,InferSendableFromCaptures) across targetsSWIFT_VERSION = 6.0for ShareTo-related Xcode build configurations inproject.pbxprojExecutableContentBlockingGeneratorto usenonisolated(unsafe)staticgeneratorto address thread-safety (FXIOS-14548)Tests and API adjustments
await fulfillment(...)withwait(for: ...)@Sendableto decision handler closures and adjust call sites; remove@MainActorfrom mock server helperLabelButtonHeaderView.configurenow takes a@Sendable @MainActoraction and wraps it before passing toActionButtonViewModelWritten by Cursor Bugbot for commit 5439cb2. Configure here.