Skip to content

Conversation

@tomerqodo
Copy link

@tomerqodo tomerqodo commented Jan 21, 2026

Benchmark PR from qodo-benchmark#236


Note

Swift 6 concurrency cleanup across unit tests and test mocks.

  • Convert XCTest setUp/tearDown to async variants and call super with await where needed
  • Add @MainActor to many UI-driven test classes; adjust a few where not required
  • Add @Sendable to closures and @unchecked Sendable to test mocks (e.g., MockUserNotificationCenter, ClientSyncManagerSpy, MockTabQueue); mark some mocks final
  • Update a closure in NotificationManagerTests to use a capture list and assert checks
  • Minor tidy-ups in mocks (e.g., MockURLAuthenticationChallengeSender final, MainActor/Sendable annotations on APIs)

Written by Cursor Bugbot for commit ac31d8c. Configure here.

Copy link

@cursor cursor bot left a 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 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

XCTAssertTrue(self.center.requestAuthorizationWasCalled)
notificationManager.requestAuthorization { [center] (granted, error) in
assert(granted, "Authorization should be granted")
assert(center?.requestAuthorizationWasCalled ?? false, "requestAuthorization should be called")
Copy link

Choose a reason for hiding this comment

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

Test uses assert instead of XCTAssertTrue

Medium Severity

The testRequestAuthorization test replaced XCTAssertTrue with Swift's assert() function. This is incorrect because assert() is stripped in optimized/release builds and doesn't integrate with XCTest—failures won't be reported as test failures, they'll either crash the process or be silently ignored. Every other test in this file correctly uses XCTAssertTrue and XCTAssertEqual.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants