Skip to content

Refactor FXIOS-14344 [Swift 6 migration] Fixing warnings in unit tests #19

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 3 commits into
base: coderabbit_combined_20260121_augment_sentry_coderabbit_1_base_refactor_fxios-14344_swift_6_migration_fixing_warnings_in_unit_tests__pr236
Choose a base branch
from
Open

Refactor FXIOS-14344 [Swift 6 migration] Fixing warnings in unit tests #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
db550bd
Fun again
lmarceau Dec 5, 2025
adf6e79
Fix teardown methods orders
lmarceau Dec 5, 2025
ac31d8c
update pr
tomerqodo Jan 21, 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
10 changes: 5 additions & 5 deletions ...ios-tests/Tests/ClientTests/Frontend/StoriesFeed/StoriesFeedDiffableDataSourceTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ import Storage

@testable import Client

@MainActor
final class StoriesFeedDiffableDataSourceTests: XCTestCase {
var collectionView: UICollectionView?
var diffableDataSource: StoriesFeedDiffableDataSource?

override func setUpWithError() throws {
try super.setUpWithError()

override func setUp() async throws {
try await super.setUp()
collectionView = UICollectionView(frame: .zero, collectionViewLayout: UICollectionViewFlowLayout())
let collectionView = try XCTUnwrap(collectionView)
diffableDataSource = StoriesFeedDiffableDataSource(
Expand All @@ -25,11 +25,11 @@ final class StoriesFeedDiffableDataSourceTests: XCTestCase {
DependencyHelperMock().bootstrapDependencies()
}

override func tearDown() {
override func tearDown() async throws {
try await super.tearDown()
diffableDataSource = nil
collectionView = nil
DependencyHelperMock().reset()
super.tearDown()
}
Comment on lines +28 to 33
Copy link

@coderabbitai coderabbitai bot Jan 22, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Incorrect tearDown order: super.tearDown() should be called last.

The super.tearDown() call on line 29 should be the last statement in the method, not the first. Test-specific cleanup (nilling out diffableDataSource, collectionView, and resetting dependencies) should happen before calling the superclass tearDown. This is the opposite order of setUp where super is called first.

The other test files in this PR correctly place super.tearDown() at the end.

Proposed fix 
     override func tearDown() async throws {
-        try await super.tearDown()
         diffableDataSource = nil
         collectionView = nil
         DependencyHelperMock().reset()
+        try await super.tearDown()
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
override func tearDown() async throws {
try await super.tearDown()
diffableDataSource = nil
collectionView = nil
DependencyHelperMock().reset()
super.tearDown()
}
override func tearDown() async throws {
diffableDataSource = nil
collectionView = nil
DependencyHelperMock().reset()
try await super.tearDown()
}
🤖 Prompt for AI Agents 
In
`@firefox-ios/firefox-ios-tests/Tests/ClientTests/Frontend/StoriesFeed/StoriesFeedDiffableDataSourceTests.swift`
around lines 28 - 33, The tearDown method calls try await super.tearDown() too
early; move the super.tearDown() call to the end of tearDown so test-specific
cleanup runs first: nil out diffableDataSource and collectionView and call
DependencyHelperMock().reset() before invoking try await super.tearDown() in the
override of func tearDown() async throws (method name: tearDown; symbols:
diffableDataSource, collectionView, DependencyHelperMock().reset()).


func test_updateSnapshot_initialSnapshotHasNoData() throws {
Expand Down
8 changes: 4 additions & 4 deletions ...firefox-ios-tests/Tests/ClientTests/Frontend/SyncContentSettingsViewControllerTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,20 @@ class SyncContentSettingsViewControllerTests: XCTestCase {
var syncContentSettingsVC: SyncContentSettingsViewController?
let windowUUID: WindowUUID = .XCTestDefaultUUID

override func setUp() {
super.setUp()
override func setUp() async throws {
try await super.setUp()
DependencyHelperMock().bootstrapDependencies()
profile = MockProfile()
LegacyFeatureFlagsManager.shared.initializeDeveloperFeatures(with: profile)
syncContentSettingsVC = SyncContentSettingsViewController(windowUUID: windowUUID)
syncContentSettingsVC?.profile = profile
}

override func tearDown() {
override func tearDown() async throws {
DependencyHelperMock().reset()
profile = nil
syncContentSettingsVC = nil
super.tearDown()
try await super.tearDown()
}

func test_syncContentSettingsViewController_generateSettingsCount() {
Expand Down
8 changes: 4 additions & 4 deletions firefox-ios/firefox-ios-tests/Tests/ClientTests/FxAWebViewModelTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,19 @@ class FxAWebViewModelTests: XCTestCase {
var viewModel: FxAWebViewModel!
var deeplinkParams: FxALaunchParams!

override func setUp() {
super.setUp()
override func setUp() async throws {
try await super.setUp()
deeplinkParams = FxALaunchParams(entrypoint: .browserMenu, query: ["test_key": "test_value"])
viewModel = FxAWebViewModel(pageType: .settingsPage,
profile: MockProfile(),
deepLinkParams: deeplinkParams,
telemetry: FxAWebViewTelemetry(telemetryWrapper: MockTelemetryWrapper()))
}

override func tearDown() {
override func tearDown() async throws {
deeplinkParams = nil
viewModel = nil
super.tearDown()
try await super.tearDown()
}

func testCreateOutputURLWithValidFileNameAndExtension() {
Expand Down
6 changes: 3 additions & 3 deletions firefox-ios/firefox-ios-tests/Tests/ClientTests/Helpers/NotificationManagerTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ class NotificationManagerTests: XCTestCase {
}

func testRequestAuthorization() {
notificationManager.requestAuthorization { (granted, error) in
XCTAssertTrue(granted)
XCTAssertTrue(self.center.requestAuthorizationWasCalled)
notificationManager.requestAuthorization { [center] (granted, error) in
assert(granted, "Authorization should be granted")
assert(center?.requestAuthorizationWasCalled ?? false, "requestAuthorization should be called")
}
}
Comment on lines 25 to 30
Copy link

@coderabbitai coderabbitai bot Jan 22, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Using assert() instead of XCTAssert* will not report test failures correctly.

The change from XCTAssertTrue to assert() is problematic:

  1. assert() doesn't integrate with XCTest—failures won't be reported as test failures in test results
  2. assert() crashes the process rather than gracefully failing the test
  3. The completion handler executes asynchronously but the test doesn't wait for it, so assertions may not run before the test completes
Suggested fix using expectations 
     func testRequestAuthorization() {
+        let expectation = expectation(description: "Authorization callback")
         notificationManager.requestAuthorization { [center] (granted, error) in
-            assert(granted, "Authorization should be granted")
-            assert(center?.requestAuthorizationWasCalled ?? false, "requestAuthorization should be called")
+            XCTAssertTrue(granted, "Authorization should be granted")
+            XCTAssertTrue(center?.requestAuthorizationWasCalled ?? false, "requestAuthorization should be called")
+            expectation.fulfill()
         }
+        wait(for: [expectation], timeout: 1.0)
     }
🤖 Prompt for AI Agents 
In
`@firefox-ios/firefox-ios-tests/Tests/ClientTests/Helpers/NotificationManagerTests.swift`
around lines 25 - 30, Replace the synchronous assert() calls in
testRequestAuthorization with XCTest assertions and an expectation: use an
XCTestExpectation, call notificationManager.requestAuthorization and in its
completion fulfill the expectation and use XCTAssertTrue for granted and
XCTAssertTrue(center?.requestAuthorizationWasCalled ?? false) (or XCTAssertEqual
as appropriate), then waitForExpectations with a timeout; this ensures
testRequestAuthorization waits for the asynchronous completion and reports
failures via XCTest instead of crashing.


Expand Down
9 changes: 4 additions & 5 deletions firefox-ios/firefox-ios-tests/Tests/ClientTests/Helpers/RatingPromptManagerTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ class RatingPromptManagerTests: XCTestCase {
var crashTracker: MockCrashTracker!
var subject: RatingPromptManager!

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

prefs = MockProfilePrefs()
logger = CrashingMockLogger()
Expand All @@ -33,7 +33,8 @@ class RatingPromptManagerTests: XCTestCase {
userDefaults: userDefaults)
}

override func tearDown() {
override func tearDown() async throws {
try await super.tearDown()
prefs.clearAll()
subject.reset()
prefs = nil
Expand All @@ -42,8 +43,6 @@ class RatingPromptManagerTests: XCTestCase {
userDefaults = nil
crashTracker = nil
subject = nil

super.tearDown()
}

func testShouldShowPrompt_forceShow() {
Expand Down
9 changes: 5 additions & 4 deletions ...s/firefox-ios-tests/Tests/ClientTests/Library/Bookmarks/EditBookmarkDataSourceTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,26 @@
import XCTest
@testable import Client

@MainActor
final class EditBookmarkDataSourceTests: XCTestCase {
private let folders = [
Folder(title: "Parent", guid: "ParentFolder", indentation: 0),
Folder(title: "Child", guid: "ChildFolder", indentation: 1)
]
private var tableView: UITableView!

override func setUp() {
super.setUp()
override func setUp() async throws {
try await super.setUp()
tableView = UITableView()
let window = UIWindow(frame: UIScreen.main.bounds)
window.rootViewController = UIViewController()
window.rootViewController?.view.addSubview(tableView)
window.makeKeyAndVisible()
}

override func tearDown() {
override func tearDown() async throws {
tableView = nil
super.tearDown()
try await super.tearDown()
}

func testOnSnapshotUpdate() {
Expand Down
9 changes: 4 additions & 5 deletions ...os/firefox-ios-tests/Tests/ClientTests/Library/Bookmarks/EditBookmarkViewModelTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import Shared

@testable import Client

@MainActor
class EditBookmarkViewModelTests: XCTestCase {
let folder = MockFxBookmarkNode(type: .folder,
guid: "1235",
Expand All @@ -31,18 +30,18 @@ class EditBookmarkViewModelTests: XCTestCase {
var bookmarksSaver: MockBookmarksSaver!
var profile: MockProfile!

override func setUp() {
super.setUp()
override func setUp() async throws {
try await super.setUp()
folderFetcher = MockFolderHierarchyFetcher()
bookmarksSaver = MockBookmarksSaver()
profile = MockProfile()
}

override func tearDown() {
override func tearDown() async throws {
folderFetcher = nil
bookmarksSaver = nil
profile = nil
super.tearDown()
try await super.tearDown()
}

func testInit() {
Expand Down
8 changes: 4 additions & 4 deletions firefox-ios/firefox-ios-tests/Tests/ClientTests/Library/HistoryPanelTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,17 @@ import Common
class HistoryPanelTests: XCTestCase {
let windowUUID: WindowUUID = .XCTestDefaultUUID
private var notificationCenter: MockNotificationCenter!
override func setUp() {
super.setUp()
override func setUp() async throws {
try await super.setUp()
LegacyFeatureFlagsManager.shared.initializeDeveloperFeatures(with: MockProfile())
DependencyHelperMock().bootstrapDependencies()
notificationCenter = MockNotificationCenter()
}

override func tearDown() {
super.tearDown()
override func tearDown() async throws {
DependencyHelperMock().reset()
notificationCenter = nil
try await super.tearDown()
}

func testHistoryButtons() {
Expand Down
8 changes: 4 additions & 4 deletions ...-ios/firefox-ios-tests/Tests/ClientTests/MainMenu/MainMenuConfigurationUtilityTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,16 @@ final class MainMenuConfigurationUtilityTests: XCTestCase {
var configUtility: MainMenuConfigurationUtility!
let windowUUID: WindowUUID = .XCTestDefaultUUID

override func setUp() {
super.setUp()
override func setUp() async throws {
try await super.setUp()
DependencyHelperMock().bootstrapDependencies()
configUtility = MainMenuConfigurationUtility()
}

override func tearDown() {
override func tearDown() async throws {
DependencyHelperMock().reset()
configUtility = nil
super.tearDown()
try await super.tearDown()
}

func testGenerateMenuElements_returnsHomepageSections_whenIsHomepageTrue() {
Expand Down
8 changes: 4 additions & 4 deletions firefox-ios/firefox-ios-tests/Tests/ClientTests/MainMenu/MainMenuCoordinatorTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ import XCTest
final class MainMenuCoordinatorTests: XCTestCase {
private var mockRouter: MockRouter!

override func setUp() {
super.setUp()
override func setUp() async throws {
try await super.setUp()
DependencyHelperMock().bootstrapDependencies()
mockRouter = MockRouter(navigationController: MockNavigationController())
}

override func tearDown() {
override func tearDown() async throws {
DependencyHelperMock().reset()
super.tearDown()
try await super.tearDown()
}

func testInitialState() {
Expand Down
8 changes: 4 additions & 4 deletions firefox-ios/firefox-ios-tests/Tests/ClientTests/MainMenu/MainMenuMiddlewareTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,18 @@ final class MainMenuMiddlewareTests: XCTestCase, StoreTestUtility {
var mockGleanWrapper: MockGleanWrapper!
var mockStore: MockStoreForMiddleware<AppState>!

override func setUp() {
super.setUp()
override func setUp() async throws {
try await super.setUp()
mockGleanWrapper = MockGleanWrapper()
DependencyHelperMock().bootstrapDependencies()
setupStore()
}

override func tearDown() {
override func tearDown() async throws {
DependencyHelperMock().reset()
mockGleanWrapper = nil
resetStore()
super.tearDown()
try await super.tearDown()
}

func test_tapNavigateToDestination_findInPageAction_sendTelemetryData() throws {
Expand Down
1 change: 1 addition & 0 deletions firefox-ios/firefox-ios-tests/Tests/ClientTests/MainMenu/MainMenuViewControllerTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import Common

@testable import Client

@MainActor
final class MainMenuViewControllerTests: XCTestCase {
let windowUUID: WindowUUID = .XCTestDefaultUUID

Expand Down
8 changes: 4 additions & 4 deletions ...fox-ios/firefox-ios-tests/Tests/ClientTests/Microsurvey/MicrosurveyCoordinatorTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,16 @@ final class MicrosurveyCoordinatorTests: XCTestCase {
private var mockRouter: MockRouter!
private var mockTabManager: MockTabManager!

override func setUp() {
super.setUp()
override func setUp() async throws {
try await super.setUp()
DependencyHelperMock().bootstrapDependencies()
mockRouter = MockRouter(navigationController: MockNavigationController())
mockTabManager = MockTabManager()
}

override func tearDown() {
override func tearDown() async throws {
DependencyHelperMock().reset()
super.tearDown()
try await super.tearDown()
}

func testInitialState() {
Expand Down
8 changes: 4 additions & 4 deletions firefox-ios/firefox-ios-tests/Tests/ClientTests/Microsurvey/MicrosurveyMiddlewareTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,18 @@ final class MicrosurveyMiddlewareIntegrationTests: XCTestCase, StoreTestUtility
var mockStore: MockStoreForMiddleware<AppState>!
var mockMicrosurveyTelemetry: MockMicrosurveyTelemetry!

override func setUp() {
super.setUp()
override func setUp() async throws {
try await super.setUp()
mockMicrosurveyTelemetry = MockMicrosurveyTelemetry()
DependencyHelperMock().bootstrapDependencies()
setupStore()
}

override func tearDown() {
override func tearDown() async throws {
mockMicrosurveyTelemetry = nil
DependencyHelperMock().reset()
resetStore()
super.tearDown()
try await super.tearDown()
}

func testDismissSurveyAction() throws {
Expand Down
1 change: 1 addition & 0 deletions ...-ios/firefox-ios-tests/Tests/ClientTests/Microsurvey/MicrosurveyViewControllerTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import Common

@testable import Client

@MainActor
final class MicrosurveyViewControllerTests: XCTestCase {
let windowUUID: WindowUUID = .XCTestDefaultUUID

Expand Down
2 changes: 1 addition & 1 deletion ...firefox-ios-tests/Tests/ClientTests/Mocks/MockBrowserViewControllerWebViewDelegates.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class MockNavigationAction: WKNavigationAction {
}

// MARK: - MockURLAuthenticationChallengeSender
class MockURLAuthenticationChallengeSender: NSObject, URLAuthenticationChallengeSender {
final class MockURLAuthenticationChallengeSender: NSObject, URLAuthenticationChallengeSender {
func use(_ credential: URLCredential, for challenge: URLAuthenticationChallenge) {}

func continueWithoutCredential(for challenge: URLAuthenticationChallenge) {}
Expand Down
2 changes: 1 addition & 1 deletion firefox-ios/firefox-ios-tests/Tests/ClientTests/Mocks/MockProfile.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ final class MockTabQueue: TabQueue, @unchecked Sendable {
return succeed()
}

func getQueuedTabs(completion: @escaping @MainActor ([ShareItem]) -> Void) {
func getQueuedTabs(completion: @MainActor @Sendable @escaping ([ShareItem]) -> Void) {
Task { @MainActor in
completion(queuedTabs)
getQueuedTabsCalled += 1
Expand Down
2 changes: 1 addition & 1 deletion firefox-ios/firefox-ios-tests/Tests/ClientTests/Mocks/MockUserNotificationCenter.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import Foundation
import UserNotifications
@testable import Client

class MockUserNotificationCenter: UserNotificationCenterProtocol {
class MockUserNotificationCenter: UserNotificationCenterProtocol, @unchecked Sendable {
var pendingRequests = [UNNotificationRequest]()

var getSettingsWasCalled = false
Expand Down
1 change: 1 addition & 0 deletions ...efox-ios-tests/Tests/ClientTests/NativeErrorPage/NativeErrorPageViewControllerTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import Common

@testable import Client

@MainActor
final class NativeErrorPageViewControllerTests: XCTestCase {
let windowUUID: WindowUUID = .XCTestDefaultUUID

Expand Down
8 changes: 4 additions & 4 deletions ...ox-ios/firefox-ios-tests/Tests/ClientTests/OnboardingTests/IntroViewControllerTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ import XCTest
class IntroViewControllerTests: XCTestCase {
var mockNotificationCenter: MockNotificationCenter!

override func setUp() {
super.setUp()
override func setUp() async throws {
try await super.setUp()
DependencyHelperMock().bootstrapDependencies()
mockNotificationCenter = MockNotificationCenter()
}

override func tearDown() {
super.tearDown()
override func tearDown() async throws {
mockNotificationCenter = nil
try await super.tearDown()
}

// Temp. Disabled: https://mozilla-hub.atlassian.net/browse/FXIOS-7505
Expand Down
Loading