Use built-in swift-testing for Xcode 16#49
Conversation
2994e5d to
557ce91
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR updates the project to use the built-in swift-testing framework available in Swift 6.0 (Xcode 16) while maintaining backward compatibility with earlier compiler versions. The changes add thread safety to shared state and enable test serialization for the new testing framework.
- Conditionally uses built-in
swift-testingfor Swift 6.0+ and falls back to the external package for earlier versions - Adds thread-safe access to the stub registry using
NSLock - Enables serialized test execution for Swift 6.0+ to prevent race conditions
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Package.swift | Conditionally includes swift-testing package dependency only for compiler versions < 6.0 |
| StubNetworkKitTests_SwiftTesting.swift | Adds .serialized trait to test suite for Swift 6.0+ to ensure sequential test execution |
| StubURLProtocol.swift | Introduces NSLock to synchronize access to shared stubs array |
| TestPlan.xctestplan | Removes repeatInNewRunnerProcess option from test configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #if compiler(<6.0) | ||
| package.dependencies.append(contentsOf: [ | ||
| .package(url: "https://github.com/apple/swift-testing", exact: "0.3.0"), | ||
| ]) | ||
| testTarget.dependencies.append(contentsOf: [ | ||
| .product(name: "Testing", package: "swift-testing"), | ||
| ]) | ||
| #endif |
There was a problem hiding this comment.
Build-time conditional compilation directives like #if compiler(<6.0) are evaluated during package manifest compilation, not during target compilation. This means the condition will be checked against the Swift compiler version used to build the Package.swift file itself, not the version used to build the targets. This will not achieve the intended backward compatibility. Consider using target-level conditions or separate package manifests for different Swift versions instead.
| #if compiler(<6.0) | |
| package.dependencies.append(contentsOf: [ | |
| .package(url: "https://github.com/apple/swift-testing", exact: "0.3.0"), | |
| ]) | |
| testTarget.dependencies.append(contentsOf: [ | |
| .product(name: "Testing", package: "swift-testing"), | |
| ]) | |
| #endif | |
| // NOTE: Conditional compilation directives like `#if compiler(<6.0)` are evaluated during manifest compilation, | |
| // not during target compilation. To support Swift < 6.0, consider using separate manifests or document this requirement. | |
| // If you need to add the swift-testing dependency only for Swift < 6.0, update the manifest manually as needed. |
557ce91 to
e9cacf4
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import os | ||
|
|
||
| // based on https://github.com/417-72KI/MultipartFormDataParser/blob/main/Tests/MultipartFormDataParserTests/StubURLProtocol.swift | ||
| final class StubURLProtocol: URLProtocol { |
There was a problem hiding this comment.
OSAllocatedUnfairLock is only available on macOS 13.0+, iOS 16.0+, watchOS 9.0+, and tvOS 16.0+. While this aligns with the package's minimum platform versions in Package.swift, the lack of an explicit @available annotation makes the platform requirements less clear and could cause issues if the minimum platform versions are lowered in the future. Consider adding an @available annotation to make the dependency explicit.
| final class StubURLProtocol: URLProtocol { | |
| final class StubURLProtocol: URLProtocol { | |
| @available(macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0, *) |
0a65bff to
1a30f01
Compare
1a30f01 to
4935c81
Compare
4935c81 to
5d992ca
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #if canImport(os) | ||
| import os | ||
|
|
||
| final class Lock<State: Sendable>: Sendable { | ||
| let lock: OSAllocatedUnfairLock<State> |
There was a problem hiding this comment.
Lock is only defined when either os or Synchronization can be imported. In the repo’s CI there is a Linux test job (Swift 5.10–6.2), where canImport(os) is false and Synchronization may not be available (e.g., Swift 5.10), leaving no Lock type and breaking the build. Add a cross-platform fallback implementation (e.g., NSLock/pthread_mutex/DispatchSemaphore) under an #else branch so the module compiles on Linux.
|
|
||
| extension Lock { | ||
| @inlinable | ||
| func withLock<R: Sendable>(_ body: @Sendable (inout State) throws -> R) rethrows -> R { |
There was a problem hiding this comment.
The withLock wrapper constrains the return type to R: Sendable, which is stricter than the underlying lock APIs and can make this utility unusable for non-Sendable intermediate values inside the module. Unless you specifically need this constraint, consider removing it to keep Lock generally usable.
| func withLock<R: Sendable>(_ body: @Sendable (inout State) throws -> R) rethrows -> R { | |
| func withLock<R>(_ body: @Sendable (inout State) throws -> R) rethrows -> R { |
No description provided.