-
Notifications
You must be signed in to change notification settings - Fork 140
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
Add Wasm CI. #219
Add Wasm CI. #219
Conversation
@MainActor | ||
func testIsolationOnMinActor() async { | ||
let model = MainActorModel() | ||
let expectation = expectation(description: "observation") | ||
expectation.expectedFulfillmentCount = 2 | ||
let token = SwiftNavigation.observe { | ||
_ = model.count | ||
MainActor.assertIsolated() | ||
expectation.fulfill() | ||
func testIsolationOnMainActor() async throws { | ||
try await Task { @MainActor in |
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.
Apparently Wasm doesn't support @MainActor
on test cases. So, there is one of two things I do to emulate it. For tests that need async
, like this one, I wrap the test in:
try await Task { @MainActor in
…
}
.value
For for synchronous tests, I do:
await MainActor.run {
…
}
Open to other ideas, or if there is some way to get Wasm to recognize @MainActor
tests. The errors I got were at runtime, not compile, and looked like this:
2024-08-29T01:14:05.9592310Z Could not cast value of type '(SwiftNavigationTests.UIBindableTests) -> @Swift.MainActor () throws -> ()' (0x66aff0) to '(SwiftNavigationTests.UIBindableTests) -> () throws -> ()' (0x66b0fc).
2024-08-29T01:14:05.9594936Z Error: failed to run main module `.build/debug/swift-navigationPackageTests.wasm`
2024-08-29T01:14:05.9595800Z
2024-08-29T01:14:05.9596082Z Caused by:
2024-08-29T01:14:05.9596492Z 0: failed to invoke command default
2024-08-29T01:14:05.9596998Z 1: error while executing at wasm backtrace:
2024-08-29T01:14:05.9597833Z 0: 0xf22a4f - <unknown>!abort
2024-08-29T01:14:05.9609860Z 1: 0x65c263 - <unknown>!swift::fatalErrorv(unsigned int, char const*, void*)
2024-08-29T01:14:05.9610708Z 2: 0x65c28d - <unknown>!swift::fatalError(unsigned int, char const*, ...)
2024-08-29T01:14:05.9611836Z 3: 0x653e72 - <unknown>!swift::swift_dynamicCastFailure(void const*, char const*, void const*, char const*, char const*)
2024-08-29T01:14:05.9613263Z 4: 0x653ed9 - <unknown>!swift::swift_dynamicCastFailure(swift::TargetMetadata<swift::InProcess> const*, swift::TargetMetadata<swift::InProcess> const*, char const*)
2024-08-29T01:14:05.9614348Z 5: 0x657655 - <unknown>!swift_dynamicCast
2024-08-29T01:14:05.9614916Z 6: 0x2f377a - <unknown>!$ss15_arrayForceCastySayq_GSayxGr0_lF
2024-08-29T01:14:05.9615989Z 7: 0x2657ba - <unknown>!$s38swift_navigationPackageDiscoveredTests017__SwiftNavigatione5__allE0Say6XCTest0I4CaseCm04testJ5Class_SaySS_yAEKctG0hE0tGyF
2024-08-29T01:14:05.9617755Z 8: 0x266803 - <unknown>!$s38swift_navigationPackageDiscoveredTests05__alldE0Say6XCTest0G4CaseCm04testH5Class_SaySS_yAEKctG0fE0tGyF
2024-08-29T01:14:05.9618747Z 9: 0x2a3208 - <unknown>!$s28swift_navigationPackageTests6RunnerV4mainyyYaFZTY0_
2024-08-29T01:14:05.9619454Z 10: 0x743ed7 - <unknown>!swift_task_switch
2024-08-29T01:14:05.9620197Z 11: 0x2a31dd - <unknown>!$s28swift_navigationPackageTests6RunnerV4mainyyYaFZ
2024-08-29T01:14:05.9621008Z 12: 0x2a3389 - <unknown>!$s28swift_navigationPackageTests6RunnerV5$mainyyYaFZ
2024-08-29T01:14:05.9621626Z 13: 0x2a353e - <unknown>!async_Main
2024-08-29T01:14:05.9622247Z 14: 0x2a36e8 - <unknown>!$sIetH_yts5Error_pIegHrzo_TR
2024-08-29T01:14:05.9622866Z 15: 0x2a3a0e - <unknown>!$sIetH_yts5Error_pIegHrzo_TRTA
2024-08-29T01:14:05.9623509Z 16: 0x745b1b - <unknown>!future_adapter(swift::AsyncContext*)
2024-08-29T01:14:05.9624366Z 17: 0x742d61 - <unknown>!swift::runJobInEstablishedExecutorContext(swift::Job*)
2024-08-29T01:14:05.9625049Z 18: 0x743ab6 - <unknown>!swift_job_run
2024-08-29T01:14:05.9625499Z 19: 0x2a3669 - <unknown>!main
2024-08-29T01:14:05.9626032Z 20: 0xf2256b - <unknown>!__main_void
2024-08-29T01:14:05.9626523Z 21: 0x2dc1e - <unknown>!_start
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.
For for synchronous tests, I do:
await MainActor.run { … }
Since non-async tests run on the main thread in XCTest, can we simply omit this?
Open to other ideas, or if there is some way to get Wasm to recognize
@MainActor
tests. The errors I got were at runtime, not compile, and looked like this:
I feel like I've seen similar on Linux before, where all test cases needed to be the same signature type for a test case to work, but I thought that was fixed...then again we don't have Linux tests on this repo, so it's possible this is broken in general for non-Apple platforms.
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.
Since non-async tests run on the main thread in XCTest, can we simply omit this?
They run on the main thread, but they aren't known to be main actor to the compiler. So it won't play nicely with the isolation stuff we have for observe
.
expectation.fulfill() | ||
func testIsolationOnMainActor() async throws { | ||
try await Task { @MainActor in | ||
let model = MainActorModel() | ||
var didObserve = false | ||
let token = SwiftNavigation.observe { | ||
_ = model.count | ||
MainActor.assertIsolated() | ||
didObserve = true |
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.
Wasm also doesn't support test expectations (AFAICT), and so I've replaced them with more rudimentary tools for now.
#if !os(WASI) | ||
@MainActor | ||
func testTokenStorage() async { |
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 point of this test is specifically to the NSObject
version of observe
, and so we can just omit it from Wasm entirely.
@@ -1,9 +1,9 @@ | |||
#if canImport(SwiftUI) |
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.
I moved this test from SwiftUINavigation to SwiftNavigation. Currently the only test in here is testing SwiftUI-specific behavior, but still it feels like the test should be in this module.
@MainActor | ||
func testDynamicMemberLookupBindable() throws { | ||
func testDynamicMemberLookupBindable() { |
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.
Some tests were marked as @MainActor
that didn't need it, so removing.
Also I moved this test from UIKitNavigation to SwiftNavigation since this is a pure Swift test.
@MainActor | ||
func testInitProjectedValue() throws { |
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.
Same with this file...
XCTAssertEqual(UITransaction.current.isSet, true) | ||
} else { | ||
XCTFail() | ||
#if compiler(>=6) |
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.
I was a little torn by this. Technically this test is not running on Apple platforms in CI since we aren't dealing with any Xcode betas right now. But we are getting coverage as far as Wasm is concerned since we are running a Swift 6 snapshot. And afaict there isn't a way to write a test that can run on both Xcode 15.4 + Wasm.
We could copy-paste this test to make a version that works on Xcode 15.4. I went with this approach since hopefully soon we will be able to target Xcode 16 in CI, at which point this is all moot.
func XCTTODO(_ message: String) { | ||
XCTExpectFailure(message) | ||
} |
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.
This wasn't being used and it's not Wasm-friendly so just deleting it. We can bring it back when needed.
Co-authored-by: Stephen Celis <[email protected]>
No description provided.