Conversation
Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
…ng, made tests compatible with Swift 6 Added @test(...) attributes with descriptive names, Removed test prefix from method names, Used #expect() instead of XCTAssert* macros Used nonisolated(unsafe) for mutable properties in the mock delegate, Made the mock delegate conform to @unchecked Sendable Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
|
@TAKeanice: Please have a look at this PR. |
Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
…dy enabled as of Swift version 6 Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the openHAB iOS app project from Swift 5.10 to Swift 6, implementing strict concurrency checking and modernizing the testing framework.
- Updates Swift toolchain version to 6.0/6.1 across project files and CI configuration
- Replaces XCTest with Swift Testing framework in test files
- Removes OpenAPI dependencies and simplifies Swift upcoming feature flags to essential ones only
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| openHAB.xcodeproj/project.pbxproj | Updates Swift version to 6.0 and removes OpenAPI package dependencies and most Swift upcoming feature flags |
| fastlane/Fastfile | Updates iOS simulator version from 18.1 to 18.4 for testing |
| ServerCertificateManagerTests.swift | Migrates from XCTest to Swift Testing framework with proper concurrency annotations |
| MockURLProtocol.swift | Removes unused mock protocol class |
| ClampedTests.swift | Adds new test file using Swift Testing framework |
| OpenAPIService.swift | Introduces type eraser AnyAsyncSequence for API compatibility |
| Package.swift | Updates Swift tools version to 6.1 and simplifies upcoming feature flags |
| AGENT.md | Updates documentation to reflect Swift 6 usage |
| .github/workflows/pull_requests.yml | Pins Xcode version to 16.4 for CI consistency |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| final class MockServerCertificateDelegate: ServerCertificateManagerDelegate, @unchecked Sendable { | ||
| nonisolated(unsafe) var lastCall = "" | ||
| nonisolated(unsafe) var expectedResult: ServerCertificateManager.EvaluateResult = .permitOnce | ||
| nonisolated(unsafe) var acceptedChangedCalled = false | ||
|
|
||
| func evaluateServerTrust(summary: String?, forDomain domain: String?) async -> ServerCertificateManager.EvaluateResult { | ||
| nonisolated func evaluateServerTrust(summary: String?, forDomain domain: String?) async -> ServerCertificateManager.EvaluateResult { | ||
| lastCall = "evaluateServerTrust" | ||
| return expectedResult | ||
| } | ||
|
|
||
| func evaluateCertificateMismatch(summary: String?, forDomain domain: String?) async -> ServerCertificateManager.EvaluateResult { | ||
| nonisolated func evaluateCertificateMismatch(summary: String?, forDomain domain: String?) async -> ServerCertificateManager.EvaluateResult { | ||
| lastCall = "evaluateCertificateMismatch" | ||
| return expectedResult | ||
| } | ||
|
|
||
| func acceptedServerCertificatesChanged() { | ||
| nonisolated func acceptedServerCertificatesChanged() { |
There was a problem hiding this comment.
Using @unchecked Sendable with nonisolated(unsafe) properties creates potential data races in concurrent test execution. Consider using actor isolation or proper synchronization mechanisms instead of bypassing Swift's concurrency safety.
There was a problem hiding this comment.
I know that I am bypassing concurrency safety. At least it unblocks the current situation for testing.
Signed-off-by: Tim Bert <5411131+timbms@users.noreply.github.com>
|
We should not merge yet. It compiles on GitHub with Xcode 16.4 but not with Xcode 26.0! |
I will work on it tomorrow early morning! |
That morning was spent trying to get XCode26 onto my system... I will try and solve the problem tomorrow, but I already got a pretty good idea about the problem from the description on the swiftlang discussion. |
Swift 6.2 throws concurrency checker errors on static properties in actors, since they are not isolated Signed-off-by: Tassilo Karge <tassilo.karge@web.de>
Signed-off-by: Tassilo Karge <tassilo.karge@web.de>
Signed-off-by: Tassilo Karge <tassilo.karge@web.de>
Signed-off-by: Tassilo Karge <tassilo.karge@web.de>
Signed-off-by: Tassilo Karge <tassilo.karge@web.de>
to avoid problems with calls from synchronous main-actor isolated code Signed-off-by: Tassilo Karge <tassilo.karge@web.de>
|
My XCode 16 now compiles the code, and my phone seems to run everything properly. |
|
That is a big change. Impressive piece of work @TAKeanice |
I go along with the big, but don't find it impressive, since I gave up in the end and synced everything to the main actor instead of having preferences be a separate one. That's partially because actor boundaries can only be crossed in async functions, but not all Apple framework functions have a sensible way of making them async (e.g. combine, or app delegate callbacks). That means I would have to use Task excessively, and that makes me afraid of breaking things that were previously assumed to happen in sequence. |
|
Still. You found a solution! |
|
According to Copilot Explain:
And indeed, if I build with the Release configuration, I can confirm that I get this compiler error. Marking ServerCertificateManager @unchecked Sendable |
|
I don´t understand that difference between debug and release mode yet. Both the method |
|
Have a look at #968. |
No description provided.