-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[Linux] Teach SwiftPM how to handle actor-isolated test functions #5525
base: main
Are you sure you want to change the base?
Conversation
…f test function signature (e.g. actor-isolated functions.)
@swift-ci please smoke test |
class MainActorIsolatedTestCaseTests: XCTestCase { | ||
@MainActor | ||
func test_actorIsolatedToMain() { } | ||
} |
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 is a clever way to test, but probably best to create a fixture instead: see FunctionalTests/TestDiscoveryTests
for examples on how to set such up
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.
Sounds good, will refactor.
Build failure does not appear related to my changes (timeout?) |
looks like its failing on the change:
|
Blergh, fair. I think that's using the deprecated path? I can make the same change on that path but I'm not sure when one path or the other is used. Any thoughts? |
i guess if you remove these tests and instead make them into a fixture, then that would use the just-built SwiftPM to run those and should help resolve it? |
@grynspan do you need any more information on how to create such fixture? |
I might—I have had to change focus and haven't been looking at this PR but will come back to it when I can. |
I need to circle back to this at some point! |
### Goals ⚽ This PR restores the Linux and Android build actions, which were disabled in #3920. It also updates the Android build action to use [swift-android-action@v2](https://github.com/marketplace/actions/swift-android-action), which enables it to run on Linux rather than requiring macOS. ### Implementation Details 🚧 Linux and Android toolchains cannot build test methods annotated with `@MainActor`, as discussed at swiftlang/swift-package-manager#5525. Rather than disabling building the tests, we instead trim out the `@MainActor` declarations with a simple `sed` command prior to performing the build on those platforms. ### Testing Details 🔍 I didn't add or remove any tests. The builds at passing in my fork for [Android](https://github.com/marcprux/Alamofire/actions/runs/12614003719/job/35152459756) and [Linux](https://github.com/marcprux/Alamofire/actions/runs/12614003719/job/35152460366).
This change teaches SwiftPM how to handle actor-isolated test cases in a package on Linux.
Motivation:
Given the following
XCTestCase
subclass:Or this one:
SwiftPM currently does not handle them properly, leading to a build error when compiling the test suite of the form:
Modifications:
This change emits a helper function into the synthesized test binary that takes either a synchronous or asynchronous member function of a test case class and returns a function of the appropriate form. It then uses that function when synthesizing the
__allTests__
array instead of trying to check if a functionisAsync
.Since a function of the form
@MainActor func testX() { }
lives in the twilight zone between sync andasync
, but can be trivially converted to anasync
function (with a synthesized thunk), this allows such functions to be added to the array in question.Result:
This change should allow test suites with actor-isolated tests to build and be tested successfully.