-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor FXIOS-12796 [Swift 6 Migration] Fix main actor isolation warnings that are Swift 6 errors in the XCUITests suite - Batch 2 #28
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: qodo_action_req_1_base__refactor_fxios-12796_swift_6_migration_fix_main_actor_isolation_warnings_that_are_swift_6_errors_in_the_xcuitests_suite_-_batch_2_pr2
Are you sure you want to change the base?
Changes from all commits
479bf8f
a036560
a7d891e
b5b21d0
ed8f2a5
81d28b8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,9 @@ import Shared | |
| let page1 = "http://localhost:\(serverPort)/test-fixture/find-in-page-test.html" | ||
| let page2 = "http://localhost:\(serverPort)/test-fixture/test-example.html" | ||
| let serverPort = ProcessInfo.processInfo.environment["WEBSERVER_PORT"] ?? "\(Int.random(in: 1025..<65000))" | ||
| @MainActor | ||
| let urlBarAddress = XCUIApplication().textFields[AccessibilityIdentifiers.Browser.AddressToolbar.searchTextField] | ||
| @MainActor | ||
| let homepageSearchBar = XCUIApplication().cells[AccessibilityIdentifiers.FirefoxHomepage.SearchBar.itemCell] | ||
|
|
||
| func path(forTestPage page: String) -> String { | ||
|
|
@@ -107,16 +109,16 @@ class BaseTestCase: XCTestCase { | |
| } | ||
| } | ||
|
|
||
| override func setUp() { | ||
| super.setUp() | ||
| override func setUp() async throws { | ||
| try await super.setUp() | ||
| continueAfterFailure = false | ||
| setUpApp() | ||
| setUpScreenGraph() | ||
| } | ||
|
|
||
| override func tearDown() { | ||
| override func tearDown() async throws { | ||
| app.terminate() | ||
| super.tearDown() | ||
| try await super.tearDown() | ||
| } | ||
|
Comment on lines
+112
to
122
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 5. Restart helper now wrong • BaseTestCase.setUp/tearDown were migrated to async throws, but forceRestartApp() still calls synchronous tearDown()/setUp(). • This likely calls XCTest’s synchronous lifecycle methods (not BaseTestCase’s async ones), so the app may not actually restart and the screen graph may not be rebuilt. • Tests relying on forceRestartApp() to validate persistence across restart become incorrect/flaky. Agent prompt
Comment on lines
+119
to
122
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 6. Sync teardown skipped • Several BaseTestCase-derived test classes still override *synchronous* tearDown() and call super.tearDown() (sync). • With BaseTestCase now implementing tearDown() async throws, XCTest will use the async tear down path, so these synchronous overrides’ cleanup (orientation/theme reset, downloads cleanup) can be skipped. • This risks cross-test pollution and UI-test flakiness (e.g., device left in landscape). Agent prompt
|
||
|
|
||
| var skipPlatform: Bool { | ||
|
|
@@ -503,19 +505,19 @@ class BaseTestCase: XCTestCase { | |
| } | ||
|
|
||
| class IpadOnlyTestCase: BaseTestCase { | ||
| override func setUp() { | ||
| override func setUp() async throws { | ||
| specificForPlatform = .pad | ||
| if iPad() { | ||
| super.setUp() | ||
| await super.setUp() | ||
| } | ||
|
Comment on lines
+508
to
512
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 4. Missing try await • IpadOnlyTestCase.setUp() calls await super.setUp() even though the superclass setUp() is async throws. • This is a compile-time error that will block building the UI test target. Agent prompt
|
||
| } | ||
| } | ||
|
|
||
| class IphoneOnlyTestCase: BaseTestCase { | ||
| override func setUp() { | ||
| override func setUp() async throws { | ||
| specificForPlatform = .phone | ||
| if !iPad() { | ||
| super.setUp() | ||
| try await super.setUp() | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,10 +43,10 @@ class FeatureFlaggedTestSuite: FeatureFlaggedTestBase { | |
| addLaunchArgument(jsonFileName: jsonFileName, featureName: featureName) | ||
| } | ||
|
|
||
| override func setUp() { | ||
| override func setUp() async throws { | ||
| continueAfterFailure = false | ||
| setUpExperimentVariables() | ||
| setUpApp() | ||
| setUpExperimentVariables() | ||
| setUpLaunchArguments() | ||
| setUpScreenGraph() | ||
| } | ||
|
Comment on lines
+46
to
52
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 7. Feature suite order crash • FeatureFlaggedTestSuite.setUp() now calls setUpApp() before setUpExperimentVariables(). • But setUpApp() immediately uses jsonFileName/featureName, which are String! and documented to be set inside setUpExperimentVariables(). • If this base class is used, it can crash with an unexpected nil unwrap during setup. Agent prompt
|
||
|
|
||
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.
1. a11yutils.swift missing mpl header
📘 Rule violation✓ CorrectnessAgent prompt
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools