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

Remove previously deprecated --enable-test-discovery #7391

Merged
merged 7 commits into from Mar 5, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 3 additions & 12 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,7 +155,7 @@ extension BuildPlan {
}

if let entryPointResolvedTarget = testProduct.testEntryPointTarget {
if isEntryPointPathSpecifiedExplicitly || explicitlyEnabledDiscovery {
if isEntryPointPathSpecifiedExplicitly {
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(
Expand Down Expand Up @@ -221,10 +216,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
4 changes: 0 additions & 4 deletions Sources/CoreCommands/Options.swift
Expand Up @@ -464,10 +464,6 @@ package struct BuildOptions: ParsableArguments {
#endif
}

/// Whether to enable test discovery on platforms without Objective-C runtime.
MaxDesiatov marked this conversation as resolved.
Show resolved Hide resolved
@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`
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)
MaxDesiatov marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Up @@ -33,10 +33,7 @@ extension BuildParameters {
/// 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 +51,7 @@ extension BuildParameters {
switch self {
case .loadableBundle:
return nil
case .entryPointExecutable(explicitlyEnabledDiscovery: _, explicitlySpecifiedPath: let entryPointPath):
case .entryPointExecutable(explicitlySpecifiedPath: let entryPointPath):
return entryPointPath
}
}
Expand All @@ -75,9 +72,9 @@ 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(false, forKey: .explicitlyEnabledDiscovery)
grynspan marked this conversation as resolved.
Show resolved Hide resolved
try container.encode(explicitlySpecifiedPath, forKey: .explicitlySpecifiedPath)
}
}
Expand Down Expand Up @@ -122,7 +119,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 +133,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