Skip to content

Commit

Permalink
Remove previously deprecated --enable-test-discovery (#7391)
Browse files Browse the repository at this point in the history
This flag was deprecated for 4 years since Swift 5.4, Swift 6.0 is a good opportunity to remove it.

Closes #7389.
  • Loading branch information
MaxDesiatov committed Mar 5, 2024
1 parent 8f653eb commit 259ec78
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 129 deletions.
78 changes: 30 additions & 48 deletions Sources/Build/BuildPlan/BuildPlan+Test.swift
Expand Up @@ -33,21 +33,19 @@ extension BuildPlan {
_ observabilityScope: ObservabilityScope
) throws -> [(product: ResolvedProduct, discoveryTargetBuildDescription: SwiftTargetBuildDescription?, entryPointTargetBuildDescription: SwiftTargetBuildDescription)] {
guard buildParameters.testingParameters.testProductStyle.requiresAdditionalDerivedTestTargets,
case .entryPointExecutable(let explicitlyEnabledDiscovery, let explicitlySpecifiedPath) =
case .entryPointExecutable(let explicitlySpecifiedPath) =
buildParameters.testingParameters.testProductStyle
else {
throw InternalError("makeTestManifestTargets should not be used for build plan which does not require additional derived test targets")
}

let isEntryPointPathSpecifiedExplicitly = explicitlySpecifiedPath != nil

var isDiscoveryEnabledRedundantly = explicitlyEnabledDiscovery && !isEntryPointPathSpecifiedExplicitly
var result: [(ResolvedProduct, SwiftTargetBuildDescription?, SwiftTargetBuildDescription)] = []
for testProduct in graph.allProducts where testProduct.type == .test {
guard let package = graph.package(for: testProduct) else {
throw InternalError("package not found for \(testProduct)")
}
isDiscoveryEnabledRedundantly = isDiscoveryEnabledRedundantly && nil == testProduct.testEntryPointTarget
// If a non-explicitly specified test entry point file exists, prefer that over test discovery.
// This is designed as an escape hatch when test discovery is not appropriate and for backwards
// compatibility for projects that have existing test entry point files (e.g. XCTMain.swift, LinuxMain.swift).
Expand All @@ -58,10 +56,7 @@ extension BuildPlan {
// `--experimental-test-entry-point-path <file>`. The latter is useful because it still performs test discovery and places the discovered
// tests into a separate target/module named "<PackageName>PackageDiscoveredTests". Then, that entry point file may import that module and
// obtain that list to pass it to the `XCTMain(...)` function and avoid needing to maintain a list of tests itself.
if testProduct.testEntryPointTarget != nil && explicitlyEnabledDiscovery && !isEntryPointPathSpecifiedExplicitly {
let testEntryPointName = testProduct.underlying.testEntryPointPath?.basename ?? SwiftTarget.defaultTestEntryPointName
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>'.")
} else if testProduct.testEntryPointTarget == nil, let testEntryPointPath = explicitlySpecifiedPath, !fileSystem.exists(testEntryPointPath) {
if testProduct.testEntryPointTarget == nil, let testEntryPointPath = explicitlySpecifiedPath, !fileSystem.exists(testEntryPointPath) {
observabilityScope.emit(error: "'--experimental-test-entry-point-path' was specified but the file '\(testEntryPointPath)' could not be found.")
}

Expand Down Expand Up @@ -160,43 +155,34 @@ extension BuildPlan {
}

if let entryPointResolvedTarget = testProduct.testEntryPointTarget {
if isEntryPointPathSpecifiedExplicitly || explicitlyEnabledDiscovery {
if isEntryPointPathSpecifiedExplicitly {
// Allow using the explicitly-specified test entry point target, but still perform test discovery and thus declare a dependency on the discovery targets.
let entryPointTarget = SwiftTarget(
name: entryPointResolvedTarget.underlying.name,
dependencies: entryPointResolvedTarget.underlying.dependencies + swiftTargetDependencies,
packageAccess: entryPointResolvedTarget.packageAccess,
testEntryPointSources: entryPointResolvedTarget.underlying.sources
)
let entryPointResolvedTarget = ResolvedTarget(
packageIdentity: testProduct.packageIdentity,
underlying: entryPointTarget,
dependencies: entryPointResolvedTarget.dependencies + resolvedTargetDependencies,
defaultLocalization: testProduct.defaultLocalization,
supportedPlatforms: testProduct.supportedPlatforms,
platformVersionProvider: testProduct.platformVersionProvider
)
let entryPointTargetBuildDescription = try SwiftTargetBuildDescription(
package: package,
target: entryPointResolvedTarget,
toolsVersion: toolsVersion,
buildParameters: buildParameters,
testTargetRole: .entryPoint(isSynthesized: false),
disableSandbox: disableSandbox,
fileSystem: fileSystem,
observabilityScope: observabilityScope
)

result.append((testProduct, discoveryTargets?.buildDescription, entryPointTargetBuildDescription))
} else {
// Ignore test entry point and synthesize one, declaring a dependency on the test discovery targets created above.
let entryPointTargetBuildDescription = try generateSynthesizedEntryPointTarget(
swiftTargetDependencies: swiftTargetDependencies,
resolvedTargetDependencies: resolvedTargetDependencies
)
result.append((testProduct, discoveryTargets?.buildDescription, entryPointTargetBuildDescription))
}
if isEntryPointPathSpecifiedExplicitly {
// Allow using the explicitly-specified test entry point target, but still perform test discovery and thus declare a dependency on the discovery targets.
let entryPointTarget = SwiftTarget(
name: entryPointResolvedTarget.underlying.name,
dependencies: entryPointResolvedTarget.underlying.dependencies + swiftTargetDependencies,
packageAccess: entryPointResolvedTarget.packageAccess,
testEntryPointSources: entryPointResolvedTarget.underlying.sources
)
let entryPointResolvedTarget = ResolvedTarget(
packageIdentity: testProduct.packageIdentity,
underlying: entryPointTarget,
dependencies: entryPointResolvedTarget.dependencies + resolvedTargetDependencies,
defaultLocalization: testProduct.defaultLocalization,
supportedPlatforms: testProduct.supportedPlatforms,
platformVersionProvider: testProduct.platformVersionProvider
)
let entryPointTargetBuildDescription = try SwiftTargetBuildDescription(
package: package,
target: entryPointResolvedTarget,
toolsVersion: toolsVersion,
buildParameters: buildParameters,
testTargetRole: .entryPoint(isSynthesized: false),
disableSandbox: disableSandbox,
fileSystem: fileSystem,
observabilityScope: observabilityScope
)

result.append((testProduct, discoveryTargets?.buildDescription, entryPointTargetBuildDescription))
} else {
// Use the test entry point as-is, without performing test discovery.
let entryPointTargetBuildDescription = try SwiftTargetBuildDescription(
Expand All @@ -221,10 +207,6 @@ extension BuildPlan {
}
}

if isDiscoveryEnabledRedundantly {
observabilityScope.emit(warning: "'--enable-test-discovery' option is deprecated; tests are automatically discovered on all platforms")
}

return result
}
}
Expand Down
8 changes: 1 addition & 7 deletions Sources/Commands/Utilities/TestingSupport.swift
Expand Up @@ -215,19 +215,13 @@ extension SwiftCommandState {
) throws -> BuildParameters {
var parameters = try self.productsBuildParameters

var explicitlyEnabledDiscovery = false
var explicitlySpecifiedPath: AbsolutePath?
if case let .entryPointExecutable(
explicitlyEnabledDiscoveryValue,
explicitlySpecifiedPathValue
) = parameters.testingParameters.testProductStyle {
explicitlyEnabledDiscovery = explicitlyEnabledDiscoveryValue
if case let .entryPointExecutable(explicitlySpecifiedPathValue) = parameters.testingParameters.testProductStyle {
explicitlySpecifiedPath = explicitlySpecifiedPathValue
}
parameters.testingParameters = .init(
configuration: parameters.configuration,
targetTriple: parameters.triple,
forceTestDiscovery: explicitlyEnabledDiscovery,
testEntryPointPath: explicitlySpecifiedPath,
library: library
)
Expand Down
5 changes: 0 additions & 5 deletions Sources/CoreCommands/Options.swift
Expand Up @@ -464,13 +464,8 @@ package struct BuildOptions: ParsableArguments {
#endif
}

/// Whether to enable test discovery on platforms without Objective-C runtime.
@Flag(help: .hidden)
package var enableTestDiscovery: Bool = false

/// Path of test entry point file to use, instead of synthesizing one or using `XCTMain.swift` in the package (if
/// present).
/// This implies `--enable-test-discovery`
@Option(
name: .customLong("experimental-test-entry-point-path"),
help: .hidden
Expand Down
11 changes: 0 additions & 11 deletions Sources/CoreCommands/SwiftCommandState.swift
Expand Up @@ -371,16 +371,6 @@ package final class SwiftCommandState {
observabilityScope.emit(.mutuallyExclusiveArgumentsError(arguments: ["--arch", "--triple"]))
}

// --enable-test-discovery should never be called on darwin based platforms
#if canImport(Darwin)
if options.build.enableTestDiscovery {
observabilityScope
.emit(
warning: "'--enable-test-discovery' option is deprecated; tests are automatically discovered on all platforms"
)
}
#endif

if options.caching.shouldDisableManifestCaching {
observabilityScope
.emit(
Expand Down Expand Up @@ -777,7 +767,6 @@ package final class SwiftCommandState {
testingParameters: .init(
configuration: options.build.configuration,
targetTriple: triple,
forceTestDiscovery: options.build.enableTestDiscovery, // backwards compatibility, remove with --enable-test-discovery
testEntryPointPath: options.build.testEntryPointPath
)
)
Expand Down
16 changes: 3 additions & 13 deletions Sources/SPMBuildCore/BuildParameters/BuildParameters+Testing.swift
Expand Up @@ -28,15 +28,9 @@ extension BuildParameters {
/// `--experimental-test-entry-point-path <file>` option, that file is used, otherwise if an `XCTMain.swift`
/// (formerly `LinuxMain.swift`) file is located in the package, it is used.
///
/// - Parameter explicitlyEnabledDiscovery: Whether test discovery generation was forced by passing
/// `--enable-test-discovery`, overriding any custom test entry point file specified via other CLI options or located in
/// the package.
/// - Parameter explicitlySpecifiedPath: The path to the test entry point file, if one was specified explicitly via
/// `--experimental-test-entry-point-path <file>`.
case entryPointExecutable(
explicitlyEnabledDiscovery: Bool,
explicitlySpecifiedPath: AbsolutePath?
)
case entryPointExecutable(explicitlySpecifiedPath: AbsolutePath?)

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

public enum CodingKeys: CodingKey {
case _case
case explicitlyEnabledDiscovery
case explicitlySpecifiedPath
}

Expand All @@ -75,9 +68,8 @@ extension BuildParameters {
switch self {
case .loadableBundle:
try container.encode(DiscriminatorKeys.loadableBundle, forKey: ._case)
case .entryPointExecutable(let explicitlyEnabledDiscovery, let explicitlySpecifiedPath):
case .entryPointExecutable(let explicitlySpecifiedPath):
try container.encode(DiscriminatorKeys.entryPointExecutable, forKey: ._case)
try container.encode(explicitlyEnabledDiscovery, forKey: .explicitlyEnabledDiscovery)
try container.encode(explicitlySpecifiedPath, forKey: .explicitlySpecifiedPath)
}
}
Expand Down Expand Up @@ -122,7 +114,6 @@ extension BuildParameters {
enableCodeCoverage: Bool = false,
enableTestability: Bool? = nil,
experimentalTestOutput: Bool = false,
forceTestDiscovery: Bool = false,
testEntryPointPath: AbsolutePath? = nil,
library: Library = .xctest
) {
Expand All @@ -137,7 +128,6 @@ extension BuildParameters {
// to disable testability in `swift test`, but that requires that the tests do not use the testable imports feature
self.enableTestability = enableTestability ?? (.debug == configuration)
self.testProductStyle = (targetTriple.isDarwin() && library == .xctest) ? .loadableBundle : .entryPointExecutable(
explicitlyEnabledDiscovery: forceTestDiscovery,
explicitlySpecifiedPath: testEntryPointPath
)
self.library = library
Expand Down
30 changes: 0 additions & 30 deletions Tests/CommandsTests/TestCommandTests.swift
Expand Up @@ -202,36 +202,6 @@ final class TestCommandTests: CommandsTestCase {
}
}

func testEnableTestDiscoveryDeprecation() throws {
let compilerDiagnosticFlags = ["-Xswiftc", "-Xfrontend", "-Xswiftc", "-Rmodule-interface-rebuild"]
#if canImport(Darwin)
// should emit when LinuxMain is present
try fixture(name: "Miscellaneous/TestDiscovery/Simple") { fixturePath in
let (_, stderr) = try SwiftPM.Test.execute(["--enable-test-discovery"] + compilerDiagnosticFlags, packagePath: fixturePath)
XCTAssertMatch(stderr, .contains("warning: '--enable-test-discovery' option is deprecated"))
}

// should emit when LinuxMain is not present
try fixture(name: "Miscellaneous/TestDiscovery/Simple") { fixturePath in
try localFileSystem.writeFileContents(fixturePath.appending(components: "Tests", SwiftTarget.defaultTestEntryPointName), bytes: "fatalError(\"boom\")")
let (_, stderr) = try SwiftPM.Test.execute(["--enable-test-discovery"] + compilerDiagnosticFlags, packagePath: fixturePath)
XCTAssertMatch(stderr, .contains("warning: '--enable-test-discovery' option is deprecated"))
}
#else
// should emit when LinuxMain is present
try fixture(name: "Miscellaneous/TestDiscovery/Simple") { fixturePath in
let (_, stderr) = try SwiftPM.Test.execute(["--enable-test-discovery"] + compilerDiagnosticFlags, packagePath: fixturePath)
XCTAssertMatch(stderr, .contains("warning: '--enable-test-discovery' option is deprecated"))
}
// should not emit when LinuxMain is present
try fixture(name: "Miscellaneous/TestDiscovery/Simple") { fixturePath in
try localFileSystem.writeFileContents(fixturePath.appending(components: "Tests", SwiftTarget.defaultTestEntryPointName), bytes: "fatalError(\"boom\")")
let (_, stderr) = try SwiftPM.Test.execute(["--enable-test-discovery"] + compilerDiagnosticFlags, packagePath: fixturePath)
XCTAssertNoMatch(stderr, .contains("warning: '--enable-test-discovery' option is deprecated"))
}
#endif
}

func testList() throws {
try fixture(name: "Miscellaneous/TestDiscovery/Simple") { fixturePath in
let (stdout, stderr) = try SwiftPM.Test.execute(["list"], packagePath: fixturePath)
Expand Down
15 changes: 0 additions & 15 deletions Tests/FunctionalTests/TestDiscoveryTests.swift
Expand Up @@ -88,21 +88,6 @@ class TestDiscoveryTests: XCTestCase {
}
}

func testEntryPointOverrideIgnored() throws {
#if os(macOS)
try XCTSkipIf(true)
#endif
try fixture(name: "Miscellaneous/TestDiscovery/Simple") { fixturePath in
let manifestPath = fixturePath.appending(components: "Tests", SwiftTarget.defaultTestEntryPointName)
try localFileSystem.writeFileContents(manifestPath, string: "fatalError(\"should not be called\")")
let (stdout, stderr) = try executeSwiftTest(fixturePath, extraArgs: ["--enable-test-discovery"])
// in "swift test" build output goes to stderr
XCTAssertMatch(stderr, .contains("Build complete!"))
// in "swift test" test output goes to stdout
XCTAssertNoMatch(stdout, .contains("Executed 1 test"))
}
}

func testTestExtensions() throws {
#if os(macOS)
try XCTSkipIf(true)
Expand Down

0 comments on commit 259ec78

Please sign in to comment.