Skip to content

Commit a5f9b6c

Browse files
bnbarhambnbarham
authored
Revert "Remove previously deprecated --enable-test-discovery" (#7395)
Reverts #7391, which blocks CI as various projects are still using `--enable-test-discovery`.
1 parent 259ec78 commit a5f9b6c

File tree

7 files changed

+129
-34
lines changed

7 files changed

+129
-34
lines changed

‎Sources/Build/BuildPlan/BuildPlan+Test.swift

Lines changed: 48 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -33,19 +33,21 @@ extension BuildPlan {
3333
_ observabilityScope: ObservabilityScope
3434
) throws -> [(product: ResolvedProduct, discoveryTargetBuildDescription: SwiftTargetBuildDescription?, entryPointTargetBuildDescription: SwiftTargetBuildDescription)] {
3535
guard buildParameters.testingParameters.testProductStyle.requiresAdditionalDerivedTestTargets,
36-
case .entryPointExecutable(let explicitlySpecifiedPath) =
36+
case .entryPointExecutable(let explicitlyEnabledDiscovery, let explicitlySpecifiedPath) =
3737
buildParameters.testingParameters.testProductStyle
3838
else {
3939
throw InternalError("makeTestManifestTargets should not be used for build plan which does not require additional derived test targets")
4040
}
4141

4242
let isEntryPointPathSpecifiedExplicitly = explicitlySpecifiedPath != nil
4343

44+
var isDiscoveryEnabledRedundantly = explicitlyEnabledDiscovery && !isEntryPointPathSpecifiedExplicitly
4445
var result: [(ResolvedProduct, SwiftTargetBuildDescription?, SwiftTargetBuildDescription)] = []
4546
for testProduct in graph.allProducts where testProduct.type == .test {
4647
guard let package = graph.package(for: testProduct) else {
4748
throw InternalError("package not found for \(testProduct)")
4849
}
50+
isDiscoveryEnabledRedundantly = isDiscoveryEnabledRedundantly && nil == testProduct.testEntryPointTarget
4951
// If a non-explicitly specified test entry point file exists, prefer that over test discovery.
5052
// This is designed as an escape hatch when test discovery is not appropriate and for backwards
5153
// compatibility for projects that have existing test entry point files (e.g. XCTMain.swift, LinuxMain.swift).
@@ -56,7 +58,10 @@ extension BuildPlan {
5658
// `--experimental-test-entry-point-path <file>`. The latter is useful because it still performs test discovery and places the discovered
5759
// tests into a separate target/module named "<PackageName>PackageDiscoveredTests". Then, that entry point file may import that module and
5860
// obtain that list to pass it to the `XCTMain(...)` function and avoid needing to maintain a list of tests itself.
59-
if testProduct.testEntryPointTarget == nil, let testEntryPointPath = explicitlySpecifiedPath, !fileSystem.exists(testEntryPointPath) {
61+
if testProduct.testEntryPointTarget != nil && explicitlyEnabledDiscovery && !isEntryPointPathSpecifiedExplicitly {
62+
let testEntryPointName = testProduct.underlying.testEntryPointPath?.basename ?? SwiftTarget.defaultTestEntryPointName
63+
observabilityScope.emit(warning: "'--enable-test-discovery' was specified so the '\(testEntryPointName)' entry point file for '\(testProduct.name)' will be ignored and an entry point will be generated automatically. To use test discovery with a custom entry point file, pass '--experimental-test-entry-point-path <file>'.")
64+
} else if testProduct.testEntryPointTarget == nil, let testEntryPointPath = explicitlySpecifiedPath, !fileSystem.exists(testEntryPointPath) {
6065
observabilityScope.emit(error: "'--experimental-test-entry-point-path' was specified but the file '\(testEntryPointPath)' could not be found.")
6166
}
6267

@@ -155,34 +160,43 @@ extension BuildPlan {
155160
}
156161

157162
if let entryPointResolvedTarget = testProduct.testEntryPointTarget {
158-
if isEntryPointPathSpecifiedExplicitly {
159-
// Allow using the explicitly-specified test entry point target, but still perform test discovery and thus declare a dependency on the discovery targets.
160-
let entryPointTarget = SwiftTarget(
161-
name: entryPointResolvedTarget.underlying.name,
162-
dependencies: entryPointResolvedTarget.underlying.dependencies + swiftTargetDependencies,
163-
packageAccess: entryPointResolvedTarget.packageAccess,
164-
testEntryPointSources: entryPointResolvedTarget.underlying.sources
165-
)
166-
let entryPointResolvedTarget = ResolvedTarget(
167-
packageIdentity: testProduct.packageIdentity,
168-
underlying: entryPointTarget,
169-
dependencies: entryPointResolvedTarget.dependencies + resolvedTargetDependencies,
170-
defaultLocalization: testProduct.defaultLocalization,
171-
supportedPlatforms: testProduct.supportedPlatforms,
172-
platformVersionProvider: testProduct.platformVersionProvider
173-
)
174-
let entryPointTargetBuildDescription = try SwiftTargetBuildDescription(
175-
package: package,
176-
target: entryPointResolvedTarget,
177-
toolsVersion: toolsVersion,
178-
buildParameters: buildParameters,
179-
testTargetRole: .entryPoint(isSynthesized: false),
180-
disableSandbox: disableSandbox,
181-
fileSystem: fileSystem,
182-
observabilityScope: observabilityScope
183-
)
184-
185-
result.append((testProduct, discoveryTargets?.buildDescription, entryPointTargetBuildDescription))
163+
if isEntryPointPathSpecifiedExplicitly || explicitlyEnabledDiscovery {
164+
if isEntryPointPathSpecifiedExplicitly {
165+
// Allow using the explicitly-specified test entry point target, but still perform test discovery and thus declare a dependency on the discovery targets.
166+
let entryPointTarget = SwiftTarget(
167+
name: entryPointResolvedTarget.underlying.name,
168+
dependencies: entryPointResolvedTarget.underlying.dependencies + swiftTargetDependencies,
169+
packageAccess: entryPointResolvedTarget.packageAccess,
170+
testEntryPointSources: entryPointResolvedTarget.underlying.sources
171+
)
172+
let entryPointResolvedTarget = ResolvedTarget(
173+
packageIdentity: testProduct.packageIdentity,
174+
underlying: entryPointTarget,
175+
dependencies: entryPointResolvedTarget.dependencies + resolvedTargetDependencies,
176+
defaultLocalization: testProduct.defaultLocalization,
177+
supportedPlatforms: testProduct.supportedPlatforms,
178+
platformVersionProvider: testProduct.platformVersionProvider
179+
)
180+
let entryPointTargetBuildDescription = try SwiftTargetBuildDescription(
181+
package: package,
182+
target: entryPointResolvedTarget,
183+
toolsVersion: toolsVersion,
184+
buildParameters: buildParameters,
185+
testTargetRole: .entryPoint(isSynthesized: false),
186+
disableSandbox: disableSandbox,
187+
fileSystem: fileSystem,
188+
observabilityScope: observabilityScope
189+
)
190+
191+
result.append((testProduct, discoveryTargets?.buildDescription, entryPointTargetBuildDescription))
192+
} else {
193+
// Ignore test entry point and synthesize one, declaring a dependency on the test discovery targets created above.
194+
let entryPointTargetBuildDescription = try generateSynthesizedEntryPointTarget(
195+
swiftTargetDependencies: swiftTargetDependencies,
196+
resolvedTargetDependencies: resolvedTargetDependencies
197+
)
198+
result.append((testProduct, discoveryTargets?.buildDescription, entryPointTargetBuildDescription))
199+
}
186200
} else {
187201
// Use the test entry point as-is, without performing test discovery.
188202
let entryPointTargetBuildDescription = try SwiftTargetBuildDescription(
@@ -207,6 +221,10 @@ extension BuildPlan {
207221
}
208222
}
209223

224+
if isDiscoveryEnabledRedundantly {
225+
observabilityScope.emit(warning: "'--enable-test-discovery' option is deprecated; tests are automatically discovered on all platforms")
226+
}
227+
210228
return result
211229
}
212230
}

‎Sources/Commands/Utilities/TestingSupport.swift

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,13 +215,19 @@ extension SwiftCommandState {
215215
) throws -> BuildParameters {
216216
var parameters = try self.productsBuildParameters
217217

218+
var explicitlyEnabledDiscovery = false
218219
var explicitlySpecifiedPath: AbsolutePath?
219-
if case let .entryPointExecutable(explicitlySpecifiedPathValue) = parameters.testingParameters.testProductStyle {
220+
if case let .entryPointExecutable(
221+
explicitlyEnabledDiscoveryValue,
222+
explicitlySpecifiedPathValue
223+
) = parameters.testingParameters.testProductStyle {
224+
explicitlyEnabledDiscovery = explicitlyEnabledDiscoveryValue
220225
explicitlySpecifiedPath = explicitlySpecifiedPathValue
221226
}
222227
parameters.testingParameters = .init(
223228
configuration: parameters.configuration,
224229
targetTriple: parameters.triple,
230+
forceTestDiscovery: explicitlyEnabledDiscovery,
225231
testEntryPointPath: explicitlySpecifiedPath,
226232
library: library
227233
)

‎Sources/CoreCommands/Options.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,8 +464,13 @@ package struct BuildOptions: ParsableArguments {
464464
#endif
465465
}
466466

467+
/// Whether to enable test discovery on platforms without Objective-C runtime.
468+
@Flag(help: .hidden)
469+
package var enableTestDiscovery: Bool = false
470+
467471
/// Path of test entry point file to use, instead of synthesizing one or using `XCTMain.swift` in the package (if
468472
/// present).
473+
/// This implies `--enable-test-discovery`
469474
@Option(
470475
name: .customLong("experimental-test-entry-point-path"),
471476
help: .hidden

‎Sources/CoreCommands/SwiftCommandState.swift

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,16 @@ package final class SwiftCommandState {
371371
observabilityScope.emit(.mutuallyExclusiveArgumentsError(arguments: ["--arch", "--triple"]))
372372
}
373373

374+
// --enable-test-discovery should never be called on darwin based platforms
375+
#if canImport(Darwin)
376+
if options.build.enableTestDiscovery {
377+
observabilityScope
378+
.emit(
379+
warning: "'--enable-test-discovery' option is deprecated; tests are automatically discovered on all platforms"
380+
)
381+
}
382+
#endif
383+
374384
if options.caching.shouldDisableManifestCaching {
375385
observabilityScope
376386
.emit(
@@ -767,6 +777,7 @@ package final class SwiftCommandState {
767777
testingParameters: .init(
768778
configuration: options.build.configuration,
769779
targetTriple: triple,
780+
forceTestDiscovery: options.build.enableTestDiscovery, // backwards compatibility, remove with --enable-test-discovery
770781
testEntryPointPath: options.build.testEntryPointPath
771782
)
772783
)

‎Sources/SPMBuildCore/BuildParameters/BuildParameters+Testing.swift

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,15 @@ extension BuildParameters {
2828
/// `--experimental-test-entry-point-path <file>` option, that file is used, otherwise if an `XCTMain.swift`
2929
/// (formerly `LinuxMain.swift`) file is located in the package, it is used.
3030
///
31+
/// - Parameter explicitlyEnabledDiscovery: Whether test discovery generation was forced by passing
32+
/// `--enable-test-discovery`, overriding any custom test entry point file specified via other CLI options or located in
33+
/// the package.
3134
/// - Parameter explicitlySpecifiedPath: The path to the test entry point file, if one was specified explicitly via
3235
/// `--experimental-test-entry-point-path <file>`.
33-
case entryPointExecutable(explicitlySpecifiedPath: AbsolutePath?)
36+
case entryPointExecutable(
37+
explicitlyEnabledDiscovery: Bool,
38+
explicitlySpecifiedPath: AbsolutePath?
39+
)
3440

3541
/// Whether this test product style requires additional, derived test targets, i.e. there must be additional test targets, beyond those
3642
/// listed explicitly in the package manifest, created in order to add additional behavior (such as entry point logic).
@@ -48,7 +54,7 @@ extension BuildParameters {
4854
switch self {
4955
case .loadableBundle:
5056
return nil
51-
case .entryPointExecutable(explicitlySpecifiedPath: let entryPointPath):
57+
case .entryPointExecutable(explicitlyEnabledDiscovery: _, explicitlySpecifiedPath: let entryPointPath):
5258
return entryPointPath
5359
}
5460
}
@@ -60,6 +66,7 @@ extension BuildParameters {
6066

6167
public enum CodingKeys: CodingKey {
6268
case _case
69+
case explicitlyEnabledDiscovery
6370
case explicitlySpecifiedPath
6471
}
6572

@@ -68,8 +75,9 @@ extension BuildParameters {
6875
switch self {
6976
case .loadableBundle:
7077
try container.encode(DiscriminatorKeys.loadableBundle, forKey: ._case)
71-
case .entryPointExecutable(let explicitlySpecifiedPath):
78+
case .entryPointExecutable(let explicitlyEnabledDiscovery, let explicitlySpecifiedPath):
7279
try container.encode(DiscriminatorKeys.entryPointExecutable, forKey: ._case)
80+
try container.encode(explicitlyEnabledDiscovery, forKey: .explicitlyEnabledDiscovery)
7381
try container.encode(explicitlySpecifiedPath, forKey: .explicitlySpecifiedPath)
7482
}
7583
}
@@ -114,6 +122,7 @@ extension BuildParameters {
114122
enableCodeCoverage: Bool = false,
115123
enableTestability: Bool? = nil,
116124
experimentalTestOutput: Bool = false,
125+
forceTestDiscovery: Bool = false,
117126
testEntryPointPath: AbsolutePath? = nil,
118127
library: Library = .xctest
119128
) {
@@ -128,6 +137,7 @@ extension BuildParameters {
128137
// to disable testability in `swift test`, but that requires that the tests do not use the testable imports feature
129138
self.enableTestability = enableTestability ?? (.debug == configuration)
130139
self.testProductStyle = (targetTriple.isDarwin() && library == .xctest) ? .loadableBundle : .entryPointExecutable(
140+
explicitlyEnabledDiscovery: forceTestDiscovery,
131141
explicitlySpecifiedPath: testEntryPointPath
132142
)
133143
self.library = library

‎Tests/CommandsTests/TestCommandTests.swift

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,36 @@ final class TestCommandTests: CommandsTestCase {
202202
}
203203
}
204204

205+
func testEnableTestDiscoveryDeprecation() throws {
206+
let compilerDiagnosticFlags = ["-Xswiftc", "-Xfrontend", "-Xswiftc", "-Rmodule-interface-rebuild"]
207+
#if canImport(Darwin)
208+
// should emit when LinuxMain is present
209+
try fixture(name: "Miscellaneous/TestDiscovery/Simple") { fixturePath in
210+
let (_, stderr) = try SwiftPM.Test.execute(["--enable-test-discovery"] + compilerDiagnosticFlags, packagePath: fixturePath)
211+
XCTAssertMatch(stderr, .contains("warning: '--enable-test-discovery' option is deprecated"))
212+
}
213+
214+
// should emit when LinuxMain is not present
215+
try fixture(name: "Miscellaneous/TestDiscovery/Simple") { fixturePath in
216+
try localFileSystem.writeFileContents(fixturePath.appending(components: "Tests", SwiftTarget.defaultTestEntryPointName), bytes: "fatalError(\"boom\")")
217+
let (_, stderr) = try SwiftPM.Test.execute(["--enable-test-discovery"] + compilerDiagnosticFlags, packagePath: fixturePath)
218+
XCTAssertMatch(stderr, .contains("warning: '--enable-test-discovery' option is deprecated"))
219+
}
220+
#else
221+
// should emit when LinuxMain is present
222+
try fixture(name: "Miscellaneous/TestDiscovery/Simple") { fixturePath in
223+
let (_, stderr) = try SwiftPM.Test.execute(["--enable-test-discovery"] + compilerDiagnosticFlags, packagePath: fixturePath)
224+
XCTAssertMatch(stderr, .contains("warning: '--enable-test-discovery' option is deprecated"))
225+
}
226+
// should not emit when LinuxMain is present
227+
try fixture(name: "Miscellaneous/TestDiscovery/Simple") { fixturePath in
228+
try localFileSystem.writeFileContents(fixturePath.appending(components: "Tests", SwiftTarget.defaultTestEntryPointName), bytes: "fatalError(\"boom\")")
229+
let (_, stderr) = try SwiftPM.Test.execute(["--enable-test-discovery"] + compilerDiagnosticFlags, packagePath: fixturePath)
230+
XCTAssertNoMatch(stderr, .contains("warning: '--enable-test-discovery' option is deprecated"))
231+
}
232+
#endif
233+
}
234+
205235
func testList() throws {
206236
try fixture(name: "Miscellaneous/TestDiscovery/Simple") { fixturePath in
207237
let (stdout, stderr) = try SwiftPM.Test.execute(["list"], packagePath: fixturePath)

‎Tests/FunctionalTests/TestDiscoveryTests.swift

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,21 @@ class TestDiscoveryTests: XCTestCase {
8888
}
8989
}
9090

91+
func testEntryPointOverrideIgnored() throws {
92+
#if os(macOS)
93+
try XCTSkipIf(true)
94+
#endif
95+
try fixture(name: "Miscellaneous/TestDiscovery/Simple") { fixturePath in
96+
let manifestPath = fixturePath.appending(components: "Tests", SwiftTarget.defaultTestEntryPointName)
97+
try localFileSystem.writeFileContents(manifestPath, string: "fatalError(\"should not be called\")")
98+
let (stdout, stderr) = try executeSwiftTest(fixturePath, extraArgs: ["--enable-test-discovery"])
99+
// in "swift test" build output goes to stderr
100+
XCTAssertMatch(stderr, .contains("Build complete!"))
101+
// in "swift test" test output goes to stdout
102+
XCTAssertNoMatch(stdout, .contains("Executed 1 test"))
103+
}
104+
}
105+
91106
func testTestExtensions() throws {
92107
#if os(macOS)
93108
try XCTSkipIf(true)

0 commit comments

Comments
 (0)