Skip to content

Refactor FXIOS-12796 [Swift 6 Migration] Fix main actor isolation warnings that are Swift 6 errors in the XCUITests suite - Batch 2 #2

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.

Sign up for GitHub

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

Open
tomerqodo wants to merge 6 commits into
base: claude_claude_vs_qodo_base__refactor_fxios-12796_swift_6_migration_fix_main_actor_isolation_warnings_that_are_swift_6_errors_in_the_xcuitests_suite_-_batch_2_pr2
Choose a base branch
from
Open

Refactor FXIOS-12796 [Swift 6 Migration] Fix main actor isolation warnings that are Swift 6 errors in the XCUITests suite - Batch 2 #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
479bf8f
Fix main actor isolation warnings that are Swift 6 errors in the XCUI…
ih-codes Dec 1, 2025
a036560
no message
ih-codes Dec 1, 2025
a7d891e
Fix setup/teardown warnings in the XCUITests suite.
ih-codes Dec 1, 2025
b5b21d0
Add main actor isolation to register* UI helper methods.
ih-codes Dec 1, 2025
ed8f2a5
Fix a few more warnings.
ih-codes Dec 1, 2025
81d28b8
update pr
tomerqodo Jan 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions firefox-ios/firefox-ios-tests/Tests/XCUITests/A11yOnboardingTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,17 @@ class A11yOnboardingTests: BaseTestCase {
return "\(AccessibilityIdentifiers.Onboarding.onboarding)\(currentScreen)"
}

override func setUp() {
override func setUp() async throws {
launchArguments = [LaunchArguments.ClearProfile,
LaunchArguments.DisableAnimations,
LaunchArguments.SkipSplashScreenExperiment]
currentScreen = 0
super.setUp()
try await super.setUp()
}

override func tearDown() {
override func tearDown() async throws {
app.terminate()
super.tearDown()
try await super.tearDown()
}

func testA11yFirstRunTour() throws {
Expand Down
5 changes: 1 addition & 4 deletions firefox-ios/firefox-ios-tests/Tests/XCUITests/A11yUtils.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at http://mozilla.org/MPL/2.0/

import XCTest

@MainActor
class A11yUtils: XCTestCase {
public struct MissingAccessibilityElement {
public let elementType: String
Expand Down
10 changes: 6 additions & 4 deletions firefox-ios/firefox-ios-tests/Tests/XCUITests/ActivityStreamTest.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ class ActivityStreamTest: FeatureFlaggedTestBase {
let testWithDB = ["testTopSites2Add", "testTopSitesRemoveAllExceptDefaultClearPrivateData"]
// Using the DDDBBs created for these tests containing enough entries for the tests that used them listed above
let pagesVisited = "browserActivityStreamPages-places.db"
override func setUp() {

override func setUp() async throws {
// Test name looks like: "[Class testFunc]", parse out the function name
let parts = name.replacingOccurrences(of: "]", with: "").split(separator: " ")
let key = String(parts[1])
Expand All @@ -41,16 +42,17 @@ class ActivityStreamTest: FeatureFlaggedTestBase {
}
launchArguments.append(LaunchArguments.SkipAddingGoogleTopSite)
launchArguments.append(LaunchArguments.SkipSponsoredShortcuts)
super.setUp()
try await super.setUp()
topSites = TopSitesScreen(app: app)
contextMenu = ContextMenuScreen(app: app)
tabTray = TabTrayScreen(app: app)
browser = BrowserScreen(app: app)
toolbar = ToolbarScreen(app: app)
}
override func tearDown() {

override func tearDown() async throws {
XCUIDevice.shared.orientation = .portrait
super.tearDown()
try await super.tearDown()
}

// https://mozilla.testrail.io/index.php?/cases/view/2273342
Expand Down
18 changes: 10 additions & 8 deletions firefox-ios/firefox-ios-tests/Tests/XCUITests/BaseTestCase.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
}

var skipPlatform: Bool {
Comment on lines 109 to 124
Copy link
Copy Markdown

@claude claude Bot Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 forceRestartApp() (line 139) calls tearDown() and setUp() synchronously, but this PR changed both to async throws overrides. In Swift, sync and async methods are separate overloads, so these calls now resolve to XCTestCase's base class no-ops instead of BaseTestCase's overrides. This makes forceRestartApp() effectively a no-op — it will no longer terminate the app, relaunch it, or rebuild the screen graph. This method is called from BookmarksTests (line 401) and AuthenticationTest (line 52).

Extended reasoning...

What the bug is

The forceRestartApp() method at line 139-142 of BaseTestCase.swift calls tearDown() and setUp() synchronously:

func forceRestartApp() {
    tearDown()
    setUp()
}

This PR changed BaseTestCase.setUp() from override func setUp() to override func setUp() async throws, and similarly tearDown() to override func tearDown() async throws. In Swift, a synchronous method and an async throws method with the same name are separate overloads — they are not the same method. The synchronous calls in forceRestartApp() will resolve to XCTestCase's base class synchronous setUp() and tearDown(), which are no-ops.

Step-by-step proof

  1. Before this PR, BaseTestCase overrode func setUp() (sync). When forceRestartApp() called tearDown() and setUp(), Swift's method resolution found the sync overrides in BaseTestCase, which executed app.terminate() (in tearDown) and setUpApp() / setUpScreenGraph() (in setUp).

  2. After this PR, BaseTestCase only overrides func setUp() async throws and func tearDown() async throws. The sync versions no longer exist in BaseTestCase.

  3. When forceRestartApp() (a non-async function) calls tearDown(), Swift looks for a synchronous tearDown() method. It does NOT find one in BaseTestCase (only async throws version exists), so it resolves to XCTestCase.tearDown(), which is a no-op.

  4. Similarly, the setUp() call resolves to XCTestCase.setUp(), another no-op.

  5. Result: forceRestartApp() does nothing — it doesn't terminate the app, doesn't relaunch it, and doesn't rebuild the screen graph.

Impact

This method is called in at least two test files:

  • BookmarksTests.swift line 401: calls forceRestartApp() to restart the app during a bookmarks link validation test
  • AuthenticationTest.swift line 52: calls forceRestartApp() to verify that BasicAuth login persists after an app restart

In both cases, the tests will silently pass without actually restarting the app, meaning they are no longer testing what they intend to test. The AuthenticationTest in particular is supposed to verify persistence across app restarts, which this regression completely undermines.

How to fix

The simplest fix is to make forceRestartApp() async and await the calls:

func forceRestartApp() async throws {
    try await tearDown()
    try await setUp()
}

Then update callers to use await. Alternatively, inline the actual teardown/setup logic (e.g., app.terminate() + setUpApp() + setUpScreenGraph()) directly in forceRestartApp() without going through the async overrides.

Expand Down Expand Up @@ -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()
Copy link
Copy Markdown

@claude claude Bot Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Bug: IpadOnlyTestCase.setUp() at line 511 calls await super.setUp() without try, but BaseTestCase.setUp() is declared async throws. This is a Swift compile error -- calling a throwing function without try is illegal. The sibling IphoneOnlyTestCase correctly uses try await super.setUp() on line 520; this line should match.

Extended reasoning...

What the bug is

At line 511 of BaseTestCase.swift, the IpadOnlyTestCase.setUp() method calls await super.setUp() without the try keyword. The method itself is declared as override func setUp() async throws, and the parent BaseTestCase.setUp() is also declared as async throws (line 112). In Swift, calling a function that can throw without using try is a compile-time error.

The specific code path

The PR changed IpadOnlyTestCase.setUp() from a synchronous override func setUp() (which called super.setUp()) to an asynchronous override func setUp() async throws. During this conversion, the call was updated from super.setUp() to await super.setUp(), but the try keyword was accidentally omitted. The corresponding IphoneOnlyTestCase class was correctly updated to use try await super.setUp() at line 520.

Why existing code does not prevent it

This is a straightforward oversight during the async/throws migration. The old synchronous setUp() did not need try because it was not declared as throwing. The new version requires both await (for async) and try (for throws), but only await was added for IpadOnlyTestCase.

Impact

This will cause a Swift compile error: "Call can throw but is not marked with try". The entire XCUITests suite will fail to compile, blocking all iPad-specific test cases that inherit from IpadOnlyTestCase (such as DragAndDropTestIpad and MultiWindowTests).

Step-by-step proof

  1. IpadOnlyTestCase.setUp() is declared async throws (line 508)
  2. It calls await super.setUp() (line 511)
  3. super.setUp() resolves to BaseTestCase.setUp() which is declared async throws (line 112)
  4. BaseTestCase.setUp() itself calls try await super.setUp() (line 113), confirming it throws
  5. Swift requires try when calling any function declared with throws
  6. Missing try results in a compile error

Fix

Change line 511 from await super.setUp() to try await super.setUp(), matching the pattern used in IphoneOnlyTestCase at line 520.

}
}
}

class IphoneOnlyTestCase: BaseTestCase {
override func setUp() {
override func setUp() async throws {
specificForPlatform = .phone
if !iPad() {
super.setUp()
try await super.setUp()
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions firefox-ios/firefox-ios-tests/Tests/XCUITests/BookmarksTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ class BookmarksTests: FeatureFlaggedTestBase {
private var homepageSettingsScreen: HomepageSettingsScreen!
private var firefoxHomeScreen: FirefoxHomePageScreen!

override func setUp() {
super.setUp()
override func setUp() async throws {
try await super.setUp()
topSitesScreen = TopSitesScreen(app: app)
browserScreen = BrowserScreen(app: app)
toolbarScreen = ToolbarScreen(app: app)
Expand All @@ -28,9 +28,9 @@ class BookmarksTests: FeatureFlaggedTestBase {
firefoxHomeScreen = FirefoxHomePageScreen(app: app)
}

override func tearDown() {
override func tearDown() async throws {
XCUIDevice.shared.orientation = .portrait
super.tearDown()
try await super.tearDown()
}

private func checkBookmarked() {
Expand Down
4 changes: 2 additions & 2 deletions firefox-ios/firefox-ios-tests/Tests/XCUITests/BrowsingPDFTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ class BrowsingPDFTests: BaseTestCase {
private var contextMenu: ContextMenuScreen!
private var library: LibraryScreen!

override func setUp() {
override func setUp() async throws {
// Test name looks like: "[Class testFunc]", parse out the function name
super.setUp()
try await super.setUp()
topSites = TopSitesScreen(app: app)
contextMenu = ContextMenuScreen(app: app)
pdf = PDFScreen(app: app)
Expand Down
9 changes: 5 additions & 4 deletions firefox-ios/firefox-ios-tests/Tests/XCUITests/C_AddressesTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ class O_AddressesTests: BaseTestCase {
private var settingsScreen: SettingScreen!
private var browserScreen: BrowserScreen!

override func setUp() {
super.setUp()
override func setUp() async throws {
try await super.setUp()

if #available(iOS 16, *) {
if !name.contains("testAddressOptionIsAvailableInSettingsMenu") {
navigator.nowAt(NewTabScreen)
Expand All @@ -34,13 +35,13 @@ class O_AddressesTests: BaseTestCase {
browserScreen = BrowserScreen(app: app)
}

override func tearDown() {
override func tearDown() async throws {
if name.contains("testAddressOptionIsAvailableInSettingsMenu") {
switchThemeToDarkOrLight(theme: "Light")
XCUIDevice.shared.orientation = .portrait
}
app.terminate()
super.tearDown()
try await super.tearDown()
}

// https://mozilla.testrail.io/index.php?/cases/view/2618637
Expand Down
4 changes: 2 additions & 2 deletions firefox-ios/firefox-ios-tests/Tests/XCUITests/CookiePersistenceTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@ final class CookiePersistenceTests: BaseTestCase {
let cookieSiteURL = "http://localhost:\(serverPort)/test-fixture/test-cookie-store.html"
let topSitesTitle = ["Facebook", "YouTube", "Wikipedia"]

override func setUp() {
override func setUp() async throws {
// Fresh install the app
// removeApp() does not work on iOS 15 and 16 intermittently
if #available(iOS 17, *) {
removeApp()
}

// The app is correctly installed
super.setUp()
try await super.setUp()
}

func testCookiePersistenceBasic() {
Expand Down
4 changes: 2 additions & 2 deletions firefox-ios/firefox-ios-tests/Tests/XCUITests/DatabaseFixtureTest.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class DatabaseFixtureTest: BaseTestCase {
"testHistoryDatabaseFixture": "testHistoryDatabase100-places.db"
]

override func setUp() {
override func setUp() async throws {
// Test name looks like: "[Class testFunc]", parse out the function name
let parts = name.replacingOccurrences(of: "]", with: "").split(separator: " ")
let key = String(parts[1])
Expand All @@ -21,7 +21,7 @@ class DatabaseFixtureTest: BaseTestCase {
LaunchArguments.LoadDatabasePrefix + fixtures[key]!,
LaunchArguments.SkipContextualHints,
LaunchArguments.DisableAnimations]
super.setUp()
try await super.setUp()
}

// https://mozilla.testrail.io/index.php?/cases/view/2458579
Expand Down
4 changes: 2 additions & 2 deletions firefox-ios/firefox-ios-tests/Tests/XCUITests/DesktopModeTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,10 @@ class DesktopModeTestsIphone: BaseTestCase {
var toolbarScreen: ToolbarScreen!
var mainMenuScreen: MainMenuScreen!

override func setUp() {
override func setUp() async throws {
specificForPlatform = .phone
if !iPad() {
super.setUp()
try await super.setUp()
}
browserScreen = BrowserScreen(app: app)
toolbarScreen = ToolbarScreen(app: app)
Expand Down
4 changes: 2 additions & 2 deletions firefox-ios/firefox-ios-tests/Tests/XCUITests/DisplaySettingsTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import XCTest

class DisplaySettingTests: BaseTestCase {
override func setUp() {
override func setUp() async throws {
// Fresh install the app
// removeApp() does not work on iOS 15 and 16 intermittently
if name.contains("testCheckDisplaySettingsDefault") {
Expand All @@ -14,7 +14,7 @@ class DisplaySettingTests: BaseTestCase {
}
}
// The app is correctly installed
super.setUp()
try await super.setUp()
}

// https://mozilla.testrail.io/index.php?/cases/view/2337485
Expand Down
4 changes: 2 additions & 2 deletions firefox-ios/firefox-ios-tests/Tests/XCUITests/DomainAutocompleteTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class DomainAutocompleteTests: BaseTestCase {
// This DB contains 3 entries mozilla.com/github.com/git.es
let historyDB = "browserAutocomplete-places.db"

override func setUp() {
override func setUp() async throws {
// Test name looks like: "[Class testFunc]", parse out the function name
let parts = name.replacingOccurrences(of: "]", with: "").split(separator: " ")
let key = String(parts[1])
Expand All @@ -41,7 +41,7 @@ class DomainAutocompleteTests: BaseTestCase {
LaunchArguments.SkipContextualHints,
LaunchArguments.DisableAnimations]
}
super.setUp()
try await super.setUp()
}

// https://mozilla.testrail.io/index.php?/cases/view/2334558
Expand Down
8 changes: 4 additions & 4 deletions firefox-ios/firefox-ios-tests/Tests/XCUITests/DragAndDropTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ class DragAndDropTestIpad: IpadOnlyTestCase {
// This DDBB contains those 4 websites listed in the name
let historyAndBookmarksDB = "browserYoutubeTwitterMozillaExample-places.db"

override func setUp() {
override func setUp() async throws {
Comment on lines 291 to +294
Copy link
Copy Markdown

@claude claude Bot Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Bug: DragAndDropTests.tearDown() (line 28) was not converted to async throws like every other tearDown override in this PR. Since BaseTestCase.tearDown() is now async throws, the synchronous super.tearDown() call bypasses BaseTestCase entirely and calls XCTestCase.tearDown() instead, meaning app.terminate() from BaseTestCase.tearDown() may not execute properly for these tests. The inner class DragAndDropTestIpad (line 310) was correctly converted — this is just a missed conversion on the outer class.

Extended reasoning...

What the bug is

The PR converts setUp and tearDown overrides across the XCUITests suite from synchronous to async throws to match the new BaseTestCase signatures. However, DragAndDropTests.tearDown() at line 28 was missed — it still uses the synchronous override func tearDown() pattern with a synchronous super.tearDown() call.

The specific code path

At line 28-31 of DragAndDropTests.swift:

override func tearDown() {
    XCUIDevice.shared.orientation = UIDeviceOrientation.portrait
    super.tearDown()
}

Meanwhile, BaseTestCase.tearDown() (line 125-128 of BaseTestCase.swift) was converted to:

override func tearDown() async throws {
    app.terminate()
    try await super.tearDown()
}

Why existing code doesn't prevent it

When a class hierarchy has both sync and async versions of tearDown, XCTest's behavior becomes nuanced. The synchronous super.tearDown() in DragAndDropTests resolves to XCTestCase.tearDown() (the sync version), not BaseTestCase.tearDown() async throws. This means the synchronous DragAndDropTests.tearDown() code — specifically setting orientation to portrait — may not execute in the expected order relative to the async tearDown chain, or may not execute at all depending on how XCTest dispatches async vs sync tearDown methods.

Step-by-step proof

  1. BaseTestCase.tearDown() is now declared as override func tearDown() async throws
  2. DragAndDropTests inherits from BaseTestCase
  3. DragAndDropTests.tearDown() is declared as override func tearDown() (synchronous)
  4. When XCTest tears down a DragAndDropTests instance, it sees both sync and async tearDown in the hierarchy
  5. The synchronous super.tearDown() call dispatches to XCTestCase.tearDown() (sync), NOT to BaseTestCase.tearDown() async throws
  6. XCTest independently calls the async tearDown chain (BaseTestCase.tearDown), so app.terminate() still runs, but the orientation reset in DragAndDropTests.tearDown() may not execute at all — XCTest only calls the async version when both exist in the hierarchy
  7. This contradicts what the verifiers noted: the orientation reset (XCUIDevice.shared.orientation = .portrait) is the code that will be SKIPPED, not app.terminate()

Impact

The device orientation will not be properly reset to portrait between DragAndDropTests test cases, potentially causing layout-dependent test failures in subsequent tests. Additionally, this leaves a synchronous tearDown that the PR is specifically trying to migrate away from, defeating the purpose of the Swift 6 migration for this class. Every other test class with a tearDown override was correctly converted.

How to fix

Convert the method signature to match the pattern used everywhere else in this PR:

override func tearDown() async throws {
    XCUIDevice.shared.orientation = UIDeviceOrientation.portrait
    try await super.tearDown()
}

This is the exact same pattern already correctly applied to the inner DragAndDropTestIpad class at lines 310-313 of the same file.

// Test name looks like: "[Class testFunc]", parse out the function name
let parts = name.replacingOccurrences(of: "]", with: "").split(separator: " ")
let key = String(parts[1])
Expand All @@ -304,12 +304,12 @@ class DragAndDropTestIpad: IpadOnlyTestCase {
LaunchArguments.SkipContextualHints,
LaunchArguments.DisableAnimations]
}
super.setUp()
try await super.setUp()
}

override func tearDown() {
override func tearDown() async throws {
XCUIDevice.shared.orientation = UIDeviceOrientation.portrait
super.tearDown()
try await super.tearDown()
}

// https://mozilla.testrail.io/index.php?/cases/view/2307024
Expand Down
4 changes: 2 additions & 2 deletions firefox-ios/firefox-ios-tests/Tests/XCUITests/EngagementNotificationTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@
import XCTest

class EngagementNotificationTests: BaseTestCase {
override func setUp() {
override func setUp() async throws {
// Fresh install the app
// removeApp() does not work on iOS 15 and 16 intermittently
if #available(iOS 17, *) {
removeApp()
}
// The app is correctly installed
super.setUp()
try await super.setUp()
}

// https://mozilla.testrail.io/index.php?/cases/view/2307101
Expand Down
4 changes: 2 additions & 2 deletions firefox-ios/firefox-ios-tests/Tests/XCUITests/FeatureFlaggedTestBase.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Comment on lines 48 to +49
Copy link
Copy Markdown

@claude claude Bot Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Bug: FeatureFlaggedTestSuite.setUp() swapped the call order of setUpApp() and setUpExperimentVariables(). Since setUpApp() calls addLaunchArgument(jsonFileName:featureName:) which force-unwraps jsonFileName and featureName (both String!), and these are only set by subclasses in setUpExperimentVariables(), calling setUpApp() first will crash on nil force-unwrap. Restore the original order: call setUpExperimentVariables() before setUpApp().

Extended reasoning...

What the bug is

In FeatureFlaggedTestSuite.setUp() (line 48-49), the PR swapped the call order of setUpApp() and setUpExperimentVariables(). The original code called setUpExperimentVariables() first, then setUpApp(). The new code calls setUpApp() first, then setUpExperimentVariables().

Why this causes a crash

FeatureFlaggedTestSuite declares two properties as implicitly unwrapped optionals:

var jsonFileName: String!
var featureName: String!

These are initialized to nil by default. Subclasses override setUpExperimentVariables() to set these values. Then setUpApp() is supposed to consume them:

override func setUpApp() {
    addLaunchArgument(jsonFileName: jsonFileName, featureName: featureName)
}

The addLaunchArgument method signature takes non-optional String parameters:

func addLaunchArgument(jsonFileName: String, featureName: String)

When String! values that are nil are passed to a function expecting String, Swift performs an implicit force-unwrap. This triggers a fatal error at runtime.

Step-by-step proof

  1. A test class like ActivityStreamTest extends FeatureFlaggedTestBase (not FeatureFlaggedTestSuite), so it is unaffected.
  2. However, any test class that extends FeatureFlaggedTestSuite and overrides setUpExperimentVariables() to set jsonFileName and featureName will hit this bug.
  3. When setUp() runs, continueAfterFailure is set to false.
  4. setUpApp() is called — this calls addLaunchArgument(jsonFileName: jsonFileName, featureName: featureName).
  5. At this point, setUpExperimentVariables() has NOT been called yet, so jsonFileName is nil and featureName is nil.
  6. Passing nil implicitly unwrapped optionals to non-optional String parameters triggers a force-unwrap crash.
  7. The test suite crashes before any test can run.

Impact

Every test class that extends FeatureFlaggedTestSuite will crash during setUp(), meaning none of those tests can execute. This is a complete blocker for any feature-flagged test suite.

Fix

Swap lines 48 and 49 back to the original order:

setUpExperimentVariables()
setUpApp()

setUpLaunchArguments()
setUpScreenGraph()
}
Expand Down
8 changes: 8 additions & 0 deletions firefox-ios/firefox-ios-tests/Tests/XCUITests/FxScreenGraph.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import Foundation
import MappaMundi
import XCTest

@MainActor
func createScreenGraph(for test: XCTestCase, with app: XCUIApplication) -> MMScreenGraph<FxUserState> {
let map = MMScreenGraph(for: test, with: FxUserState.self)

Expand Down Expand Up @@ -123,31 +124,38 @@ let allHomePanels = [
LibraryPanel_Downloads
]

@MainActor
let iOS_Settings = XCUIApplication(bundleIdentifier: "com.apple.Preferences")
@MainActor
let springboard = XCUIApplication(bundleIdentifier: "com.apple.springboard")

@MainActor
func navigationControllerBackAction(for app: XCUIApplication) -> () -> Void {
return {
app.navigationBars.element(boundBy: 0).buttons.element(boundBy: 0).waitAndTap()
}
}

@MainActor
func cancelBackAction(for app: XCUIApplication) -> () -> Void {
return {
app.otherElements["PopoverDismissRegion"].waitAndTap()
}
}

@MainActor
func dismissContextMenuAction(app: XCUIApplication) -> () -> Void {
return {
app.coordinate(withNormalizedOffset: CGVector(dx: 0.5, dy: 0.25)).tap()
}
}

@MainActor
func select(rows: Int, in app: XCUIApplication) {
app.staticTexts[String(rows)].firstMatch.waitAndTap()
}

@MainActor
func type(text: String, in app: XCUIApplication) {
text.forEach { char in
app.keys[String(char)].waitAndTap()
Expand Down
4 changes: 2 additions & 2 deletions firefox-ios/firefox-ios-tests/Tests/XCUITests/HistoryTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class HistoryTests: BaseTestCase {
var browserScreen: BrowserScreen!
var historyScreen: HistoryScreen!

override func setUp() {
override func setUp() async throws {
// Test name looks like: "[Class testFunc]", parse out the function name
let parts = name.replacingOccurrences(of: "]", with: "").split(separator: " ")
let key = String(parts[1])
Expand All @@ -57,7 +57,7 @@ class HistoryTests: BaseTestCase {
LaunchArguments.SkipContextualHints,
LaunchArguments.DisableAnimations]
}
super.setUp()
try await super.setUp()
toolbarScreen = ToolbarScreen(app: app)
settingScreen = SettingScreen(app: app)
browserScreen = BrowserScreen(app: app)
Expand Down
Loading