Skip to content

fix regression where swift test list --skip-build is ignored #5770

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

Merged
merged 1 commit into from
Sep 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 22 additions & 22 deletions Sources/Commands/SwiftTestTool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,18 @@ extension TestError: CustomStringConvertible {
}
}

struct TestToolOptions: ParsableArguments {
struct SharedOptions: ParsableArguments {
@Flag(name: .customLong("skip-build"),
help: "Skip building the test target")
var shouldSkipBuilding: Bool = false

/// If the test target should be built before testing.
var shouldBuildTests: Bool {
!shouldSkipBuilding
}
/// The test product to use. This is useful when there are multiple test products
/// to choose from (usually in multiroot packages).
@Option(help: "Test the specified product.")
var testProduct: String?
}

struct TestToolOptions: ParsableArguments {
/// If tests should run in parallel mode.
@Flag(name: .customLong("parallel"),
help: "Run the tests in parallel.")
Expand Down Expand Up @@ -109,11 +111,6 @@ struct TestToolOptions: ParsableArguments {
help: "Path where the xUnit xml file should be generated.")
var xUnitOutput: AbsolutePath?

/// The test product to use. This is useful when there are multiple test products
/// to choose from (usually in multiroot packages).
@Option(help: "Test the specified product.")
var testProduct: String?

/// Generate LinuxMain entries and exit.
@Flag(name: .customLong("testable-imports"), inversion: .prefixedEnableDisable, help: "Enable or disable testable imports. Enabled by default.")
var enableTestableImports: Bool = true
Expand Down Expand Up @@ -156,6 +153,9 @@ public struct SwiftTestTool: SwiftCommand {
@OptionGroup()
var globalOptions: GlobalOptions

@OptionGroup()
var sharedOptions: SharedOptions

@OptionGroup()
var options: TestToolOptions

Expand Down Expand Up @@ -397,8 +397,8 @@ public struct SwiftTestTool: SwiftCommand {
let buildParameters = try swiftTool.buildParametersForTest(options: self.options)
let buildSystem = try swiftTool.createBuildSystem(customBuildParameters: buildParameters)

if options.shouldBuildTests {
let subset = options.testProduct.map(BuildSubset.product) ?? .allIncludingTests
if !self.sharedOptions.shouldSkipBuilding {
let subset = self.sharedOptions.testProduct.map(BuildSubset.product) ?? .allIncludingTests
try buildSystem.build(subset: subset)
}

Expand All @@ -408,7 +408,7 @@ public struct SwiftTestTool: SwiftCommand {
throw TestError.testsExecutableNotFound
}

if let testProductName = options.testProduct {
if let testProductName = self.sharedOptions.testProduct {
guard let selectedTestProduct = testProducts.first(where: { $0.productName == testProductName }) else {
throw TestError.testsExecutableNotFound
}
Expand Down Expand Up @@ -461,17 +461,15 @@ extension SwiftTestTool {
@OptionGroup(_hiddenFromHelp: true)
var globalOptions: GlobalOptions

/// The test product to use. This is useful when there are multiple test products
/// to choose from (usually in multiroot packages).
@Option(help: "Test the specified product.")
var testProduct: String?
@OptionGroup()
var sharedOptions: SharedOptions

// for deprecated passthrough from SwiftTestTool (parse will fail otherwise)
@Flag(name: [.customLong("list-tests"), .customShort("l")], help: .hidden)
var _deprecated_passthrough: Bool = false

func run(_ swiftTool: SwiftTool) throws {
let testProducts = try buildTests(swiftTool: swiftTool)
let testProducts = try buildTestsIfNeeded(swiftTool: swiftTool)
let testSuites = try TestingSupport.getTestSuites(
in: testProducts,
swiftTool: swiftTool,
Expand All @@ -485,20 +483,22 @@ extension SwiftTestTool {
}
}

private func buildTests(swiftTool: SwiftTool) throws -> [BuiltTestProduct] {
private func buildTestsIfNeeded(swiftTool: SwiftTool) throws -> [BuiltTestProduct] {
let buildParameters = try swiftTool.buildParametersForTest(enableCodeCoverage: false)
let buildSystem = try swiftTool.createBuildSystem(customBuildParameters: buildParameters)

let subset = self.testProduct.map(BuildSubset.product) ?? .allIncludingTests
try buildSystem.build(subset: subset)
if !self.sharedOptions.shouldSkipBuilding {
let subset = self.sharedOptions.testProduct.map(BuildSubset.product) ?? .allIncludingTests
try buildSystem.build(subset: subset)
}

// Find the test product.
let testProducts = buildSystem.builtTestProducts
guard !testProducts.isEmpty else {
throw TestError.testsExecutableNotFound
}

if let testProductName = self.testProduct {
if let testProductName = self.sharedOptions.testProduct {
guard let selectedTestProduct = testProducts.first(where: { $0.productName == testProductName }) else {
throw TestError.testsExecutableNotFound
}
Expand Down
48 changes: 48 additions & 0 deletions Tests/CommandsTests/TestToolTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -265,4 +265,52 @@ final class TestToolTests: CommandsTestCase {
""")
}
}

func testList() throws {
try fixture(name: "Miscellaneous/TestDiscovery/Simple") { fixturePath in
let (stdout, stderr) = try SwiftPMProduct.SwiftTest.execute(["list"], packagePath: fixturePath)
// build was run
XCTAssertMatch(stderr, .contains("Build complete!"))
// getting the lists
XCTAssertMatch(stdout, .contains("SimpleTests.SimpleTests/testExample1"))
XCTAssertMatch(stdout, .contains("SimpleTests.SimpleTests/test_Example2"))
XCTAssertMatch(stdout, .contains("SimpleTests.SimpleTests/testThrowing"))
}

try fixture(name: "Miscellaneous/TestDiscovery/Simple") { fixturePath in
// build first
do {
let (stdout, _) = try SwiftPMProduct.SwiftBuild.execute(["--build-tests"], packagePath: fixturePath)
XCTAssertMatch(stdout, .contains("Build complete!"))
}
// list
do {
let (stdout, stderr) = try SwiftPMProduct.SwiftTest.execute(["list"], packagePath: fixturePath)
// build was run
XCTAssertMatch(stderr, .contains("Build complete!"))
// getting the lists
XCTAssertMatch(stdout, .contains("SimpleTests.SimpleTests/testExample1"))
XCTAssertMatch(stdout, .contains("SimpleTests.SimpleTests/test_Example2"))
XCTAssertMatch(stdout, .contains("SimpleTests.SimpleTests/testThrowing"))
}
}

try fixture(name: "Miscellaneous/TestDiscovery/Simple") { fixturePath in
// build first
do {
let (stdout, _) = try SwiftPMProduct.SwiftBuild.execute(["--build-tests"], packagePath: fixturePath)
XCTAssertMatch(stdout, .contains("Build complete!"))
}
// list while skipping build
do {
let (stdout, stderr) = try SwiftPMProduct.SwiftTest.execute(["list", "--skip-build"], packagePath: fixturePath)
// build was not run
XCTAssertNoMatch(stderr, .contains("Build complete!"))
// getting the lists
XCTAssertMatch(stdout, .contains("SimpleTests.SimpleTests/testExample1"))
XCTAssertMatch(stdout, .contains("SimpleTests.SimpleTests/test_Example2"))
XCTAssertMatch(stdout, .contains("SimpleTests.SimpleTests/testThrowing"))
}
}
}
}