-
Notifications
You must be signed in to change notification settings - Fork 90
XCTest discovery support for non-Darwin platforms #499
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: main
Are you sure you want to change the base?
Conversation
@swift-ci test |
@@ -649,7 +649,7 @@ public final class XCTestBundleProductTypeSpec : BundleProductTypeSpec, @uncheck | |||
var (tableOpt, warnings, errors) = super.overridingBuildSettings(scope, platform: platform) | |||
var table = tableOpt ?? MacroValueAssignmentTable(namespace: scope.namespace) | |||
|
|||
let isDeviceBuild = platform?.isDeploymentPlatform == true && platform?.identifier != "com.apple.platform.macosx" | |||
let isDeviceBuild = platform?.isDeploymentPlatform == true && platform?.name != scope.evaluate(BuiltinMacros.HOST_PLATFORM) |
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.
As an aside, this seems fundamentally flawed. There's no guarantee you're running the resulting binaries on the same host that built them, so it seems the host platform should enable this too.
Why simulators are excluded I'm not sure.
@@ -295,6 +295,7 @@ public enum PIF { | |||
case executable = "com.apple.product-type.tool" | |||
case hostBuildTool = "com.apple.product-type.tool.host-build" | |||
case unitTest = "com.apple.product-type.bundle.unit-test" | |||
case swiftpmTestRunner = "com.apple.product-type.tool.swiftpm-test-runner" |
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.
Why does this need to be called "SwiftPM" test runner?
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 called it that to differentiate it from other kinds of test runners like XCTRunner, but I'm not too attached to the name
#if canImport(Testing) | ||
import Testing | ||
#endif | ||
|
||
\(testObservationFragment(testOutputPath: "TODO")) |
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.
TODO still supposed to be here?
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.
Nope, fixed now so that this is provided via a CLI arg
|
||
private func testObservationFragment(testOutputPath: String) -> String { | ||
""" | ||
#if !os(Windows) // Test observation is not supported on Windows |
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.
Why? Do we have a GitHub issue for this?
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 pulled in verbatim from SwiftPM
override func createTaskAction(_ cbc: CommandBuildContext, _ delegate: any TaskGenerationDelegate) -> (any PlannedTaskAction)? { | ||
TestEntryPointGenerationTaskAction() | ||
} | ||
|
||
public func constructTasks(_ cbc: CommandBuildContext, _ delegate: any TaskGenerationDelegate, indexStorePaths: [Path], indexUnitBasePaths: [Path]) 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.
Can we inject the necessary additional arguments without overriding constructTasks? Ever bare call to createTasks adds risk that we forget to update things as its signature or default behavior changes.
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 tried to do this at first, but I couldn't find an elegant way to differentiate the input types, especially since the handling of the index store is a bit unusual since the build system doesn't track the content inside it
Add a new target type for SwiftPM test runner executables used on Linux/Windows. XCTest discovery generates an entry point based on index store data. The index store implementation is a lightly edited version of what's in TSC.
b20fe8d
to
5c687f8
Compare
Add a new target type for SwiftPM test runner executables used on Linux/Windows. XCTest discovery generates an entry point based on index store data. The index store implementation is a lightly edited version of what's in TSC. This will require some adoption work to generate the new test runner target in PIF - we should think about the best way to minimize the diff between Darwin and non-Darwin here (fyi @cmcgee1024 )