Skip to content
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

WIP: Implement SE-0455: SwiftPM @testable build setting #8004

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jakepetroules
Copy link
Contributor

@jakepetroules jakepetroules commented Sep 27, 2024

This introduces a new enableTesting API in SwiftSetting which allows targets to explicitly control whether testability is enabled, as some projects do not want to use the @testable feature. This can also improve debug build performance as significantly fewer symbols will be exported from the binary in the case where @testable is disabled.

The default is equivalent to enableTesting(true, .when(configuration: .debug)), so there is no behavior change from today without explicitly adopting this new API.

The --enable-testable-imports/--disable-testable-imports command line flags now acts as overrides -- if specified, they will override any build settings configured at the target level, and if unspecified, the target-level settings will be respected.

@jakepetroules
Copy link
Contributor Author

Untested draft. Will write up a proposal soon.

@dschaefer2
Copy link
Member

Thanks! Yeah, really want to get some wide community feedback on this. Pretty much everyone fights with enable testing at some point or other.

@jakepetroules
Copy link
Contributor Author

This introduces a new `enableTesting` API in SwiftSetting which allows targets to explicitly control whether testability is enabled, as some projects do not want to use the @testable feature. This can also improve debug build performance as significantly fewer symbols will be exported from the binary in the case where @testable is disabled.

The default is equivalent to `enableTesting(true, .when(configuration: .debug))`, so there is no behavior change from today without explicitly adopting this new API.

The --enable-testable-imports/--disable-testable-imports command line flags now acts as overrides -- if specified, they will override any build settings configured at the target level, and if unspecified, the target-level settings will be respected.
@jakepetroules jakepetroules force-pushed the enable-testing-build-setting branch from b29b9e1 to 0d6fa53 Compare January 30, 2025 03:50
@bkhouri bkhouri added the needs tests This change needs test coverage label Jan 30, 2025
@@ -1016,6 +1016,12 @@ public final class PackageBuilder {
}
}

for setting in manifestTarget.settings {
if case let .enableTestableImport(enable) = setting.kind, enable, setting.condition?.config == "release" {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might need to check for config==nil as well

@jakepetroules jakepetroules changed the title WIP: Introduce SwiftSetting API for controlling whether testability is enabled for a target WIP: Implement SE-0455: SwiftPM @testable build setting Jan 30, 2025
@jakepetroules jakepetroules marked this pull request as draft January 30, 2025 19:42
@jakepetroules jakepetroules removed the request for review from francescomikulis February 1, 2025 05:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs tests This change needs test coverage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants