-
Notifications
You must be signed in to change notification settings - Fork 23
Adopt SwiftTesting to improve test and runtime stability #245
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: master
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR adopts SwiftTesting framework to replace XCTest throughout the test suite, aiming to improve test stability and runtime performance. The migration includes removing XCTestCase dependencies, converting test assertions to SwiftTesting syntax, and introducing trait-based test configuration for better test isolation and reusability.
- Converts all test files from XCTest to SwiftTesting framework
- Introduces trait-based test configuration system for better test isolation
- Refactors test utilities to work with both XCTest and SwiftTesting
- Improves concurrent testing capabilities with new helper functions
Reviewed Changes
Copilot reviewed 106 out of 106 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/TestCommon/XCTestCase+Extensions.swift | Converts XCTestCase extension methods to standalone public functions |
| Tests/TestCommon/URLSessionMock.swift | Adds UUID to queue label and new requests filtering method |
| Tests/TestCommon/Traits/*.swift | Introduces SwiftTesting traits for test configuration and dependency injection |
| Tests/TestCommon/SwiftTesting+Extensions.swift | Adds new helper functions for async testing with confirmations |
| Tests/TestCommon/TimeInterval+Extensions.swift | Adds new accuracy comparison methods for TimeInterval and Int |
| Tests/TestCommon/MockTimeCoordinator.swift | New mock time coordinator for testing time-dependent functionality |
| Tests/*/*Tests.swift | Converts all test files from XCTest to SwiftTesting syntax |
Comments suppressed due to low confidence (2)
Tests/TestCommon/XCTestCase+Extensions.swift:178
- [nitpick] The function name
performConcurrentshould be more descriptive. Consider renaming it toperformConcurrentTestsorrunConcurrentOperationsto better indicate its purpose.
public func performConcurrent(queueCount: Int = ProcessInfo.processInfo.activeProcessorCount,
Tests/AuthFoundationTests/TokenTests.swift:36
- [nitpick] The parameter name
testis ambiguous. Consider renaming it tobodyoroperationto better indicate that it's the test closure to execute within the configured environment.
func withTokenTestEnvironment<T>(_ test: (OpenIdConfiguration) throws -> T) throws -> T {
| let userDefaults: UserDefaults | ||
| let storage: UserDefaultsTokenStorage | ||
|
|
||
| init() async throws { |
Copilot
AI
Jul 29, 2025
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.
Using async initializers can be problematic in test suites as they may not work correctly with all test frameworks. Consider using a setUp method or factory function instead.
| init() async throws { | |
| func setUp() async throws { |
| init() { | ||
| token = try! Token(id: "TokenId", |
Copilot
AI
Jul 29, 2025
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.
The initializer contains a force unwrap (try!) which could cause tests to crash if token creation fails. Consider using proper error handling or making this a throwing initializer.
| init() { | |
| token = try! Token(id: "TokenId", | |
| init() throws { | |
| token = try Token(id: "TokenId", |
| let openIdConfiguration = try OpenIdConfiguration.jsonDecoder.decode( | ||
| OpenIdConfiguration.self, | ||
| from: try data(from: .module, | ||
| from: try data(from: Bundle.module, | ||
| for: "openid-configuration", | ||
| in: "MockResponses")) | ||
|
|
Copilot
AI
Jul 29, 2025
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.
[nitpick] The nested try statements make error handling unclear. Consider extracting the data loading to a separate variable for better readability and error handling.
This includes the use Swift6 toolchain for Swift 5 testing, and dropping support for Xcode 15.4. This is acceptable because Xcode 15 is no longer supported by Apple, therefore it is no longer supported by us.
01df4d8 to
664f39f
Compare
Begin migrating the Swift SDK to use the new SwiftTesting framework, away from the legacy XCTest toolchain. This has a few advantages: