Skip to content

Commit

Permalink
Remove uses of temp_await from SwiftTestCommand (#7382)
Browse files Browse the repository at this point in the history
This prevents possible deadlocks that can be caused by blocking Swift Concurrency threads with semaphores.
  • Loading branch information
MaxDesiatov committed Mar 1, 2024
1 parent 68b6236 commit 1d0be84
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 24 deletions.
35 changes: 16 additions & 19 deletions Sources/Commands/SwiftTestCommand.swift
Expand Up @@ -179,7 +179,7 @@ package enum TestOutput: String, ExpressibleByArgument {
}

/// swift-test tool namespace
package struct SwiftTestCommand: SwiftCommand {
package struct SwiftTestCommand: AsyncSwiftCommand {
package static var configuration = CommandConfiguration(
commandName: "test",
_superCommandName: "swift",
Expand All @@ -200,7 +200,7 @@ package struct SwiftTestCommand: SwiftCommand {

// MARK: - XCTest

private func xctestRun(_ swiftCommandState: SwiftCommandState) throws {
private func xctestRun(_ swiftCommandState: SwiftCommandState) async throws {
// validate XCTest available on darwin based systems
let toolchain = try swiftCommandState.getTargetToolchain()
let isHostTestingAvailable = try swiftCommandState.getHostToolchain().swiftSDK.supportsTesting
Expand All @@ -218,7 +218,7 @@ package struct SwiftTestCommand: SwiftCommand {
let testProducts = try buildTestsIfNeeded(swiftCommandState: swiftCommandState, library: .xctest)
if !self.options.shouldRunInParallel {
let xctestArgs = try xctestArgs(for: testProducts, swiftCommandState: swiftCommandState)
try runTestProducts(
try await runTestProducts(
testProducts,
additionalArguments: xctestArgs,
buildParameters: buildParameters,
Expand Down Expand Up @@ -269,7 +269,7 @@ package struct SwiftTestCommand: SwiftCommand {

// process code Coverage if request
if self.options.enableCodeCoverage, runner.ranSuccessfully {
try processCodeCoverage(testProducts, swiftCommandState: swiftCommandState, library: .xctest)
try await processCodeCoverage(testProducts, swiftCommandState: swiftCommandState, library: .xctest)
}

if !runner.ranSuccessfully {
Expand Down Expand Up @@ -337,11 +337,11 @@ package struct SwiftTestCommand: SwiftCommand {

// MARK: - swift-testing

private func swiftTestingRun(_ swiftCommandState: SwiftCommandState) throws {
private func swiftTestingRun(_ swiftCommandState: SwiftCommandState) async throws {
let buildParameters = try swiftCommandState.buildParametersForTest(options: self.options, library: .swiftTesting)
let testProducts = try buildTestsIfNeeded(swiftCommandState: swiftCommandState, library: .swiftTesting)
let additionalArguments = Array(CommandLine.arguments.dropFirst())
try runTestProducts(
try await runTestProducts(
testProducts,
additionalArguments: additionalArguments,
buildParameters: buildParameters,
Expand All @@ -352,7 +352,7 @@ package struct SwiftTestCommand: SwiftCommand {

// MARK: - Common implementation

package func run(_ swiftCommandState: SwiftCommandState) throws {
package func run(_ swiftCommandState: SwiftCommandState) async throws {
do {
// Validate commands arguments
try self.validateArguments(observabilityScope: swiftCommandState.observabilityScope)
Expand All @@ -369,10 +369,10 @@ package struct SwiftTestCommand: SwiftCommand {
try command.run(swiftCommandState)
} else {
if try options.testLibraryOptions.enableSwiftTestingLibrarySupport(swiftCommandState: swiftCommandState) {
try swiftTestingRun(swiftCommandState)
try await swiftTestingRun(swiftCommandState)
}
if options.testLibraryOptions.enableXCTestSupport {
try xctestRun(swiftCommandState)
try await xctestRun(swiftCommandState)
}
}
}
Expand All @@ -383,7 +383,7 @@ package struct SwiftTestCommand: SwiftCommand {
buildParameters: BuildParameters,
swiftCommandState: SwiftCommandState,
library: BuildParameters.Testing.Library
) throws {
) async throws {
// Clean out the code coverage directory that may contain stale
// profraw files from a previous run of the code coverage tool.
if self.options.enableCodeCoverage {
Expand Down Expand Up @@ -418,7 +418,7 @@ package struct SwiftTestCommand: SwiftCommand {
}

if self.options.enableCodeCoverage, ranSuccessfully {
try processCodeCoverage(testProducts, swiftCommandState: swiftCommandState, library: library)
try await processCodeCoverage(testProducts, swiftCommandState: swiftCommandState, library: library)
}

if self.options.enableExperimentalTestOutput, !ranSuccessfully {
Expand Down Expand Up @@ -462,16 +462,13 @@ package struct SwiftTestCommand: SwiftCommand {
_ testProducts: [BuiltTestProduct],
swiftCommandState: SwiftCommandState,
library: BuildParameters.Testing.Library
) throws {
) async throws {
let workspace = try swiftCommandState.getActiveWorkspace()
let root = try swiftCommandState.getWorkspaceRoot()
let rootManifests = try temp_await {
workspace.loadRootManifests(
packages: root.packages,
observabilityScope: swiftCommandState.observabilityScope,
completion: $0
)
}
let rootManifests = try await workspace.loadRootManifests(
packages: root.packages,
observabilityScope: swiftCommandState.observabilityScope
)
guard let rootManifest = rootManifests.values.first else {
throw StringError("invalid manifests at \(root.packages)")
}
Expand Down
10 changes: 10 additions & 0 deletions Sources/SPMTestSupport/XCTAssertHelpers.swift
Expand Up @@ -170,6 +170,16 @@ package func XCTAssertAsyncFalse(
XCTAssertFalse(result, message(), file: file, line: line)
}

package func XCTAssertAsyncNil(
_ expression: @autoclosure () async throws -> Any?,
_ message: @autoclosure () -> String = "",
file: StaticString = #filePath,
line: UInt = #line
) async rethrows {
let result = try await expression()
XCTAssertNil(result, message(), file: file, line: line)
}

package func XCTAssertThrowsCommandExecutionError<T>(
_ expression: @autoclosure () throws -> T,
_ message: @autoclosure () -> String = "",
Expand Down
2 changes: 1 addition & 1 deletion Sources/swift-package-manager/SwiftPM.swift
Expand Up @@ -44,7 +44,7 @@ struct SwiftPM {
case "swift-experimental-sdk":
await SwiftSDKCommand.main()
case "swift-test":
SwiftTestCommand.main()
await SwiftTestCommand.main()
case "swift-run":
await SwiftRunCommand.main()
case "swift-package-collection":
Expand Down
5 changes: 4 additions & 1 deletion Sources/swift-test/CMakeLists.txt
Expand Up @@ -7,9 +7,12 @@
# See http://swift.org/CONTRIBUTORS.txt for Swift project authors

add_executable(swift-test
main.swift)
Entrypoint.swift)
target_link_libraries(swift-test PRIVATE
Commands)

target_compile_options(swift-test PRIVATE
-parse-as-library)

install(TARGETS swift-test
RUNTIME DESTINATION bin)
Expand Up @@ -12,4 +12,9 @@

import Commands

SwiftTestCommand.main()
@main
struct Entrypoint {
static func main() async {
await SwiftTestCommand.main()
}
}
2 changes: 1 addition & 1 deletion Tests/BuildTests/PluginsBuildPlanTests.swift
Expand Up @@ -19,7 +19,7 @@ import PackageModel
final class PluginsBuildPlanTests: XCTestCase {
func testBuildToolsDatabasePath() throws {
try fixture(name: "Miscellaneous/Plugins/MySourceGenPlugin") { fixturePath in
let (stdout, stderr) = try executeSwiftBuild(fixturePath)
let (stdout, _) = try executeSwiftBuild(fixturePath)
XCTAssertMatch(stdout, .contains("Build complete!"))
XCTAssertTrue(localFileSystem.exists(fixturePath.appending(RelativePath(".build/plugins/tools/build.db"))))
}
Expand Down
Expand Up @@ -151,7 +151,7 @@ final class PackageGraphPerfTests: XCTestCasePerf {
measure {
do {
for _ in 0..<N {
_ = try loadPackageGraph(
_ = try loadModulesGraph(
fileSystem: fs,
manifests: [root] + packageSequence,
observabilityScope: observability.topScope
Expand Down

0 comments on commit 1d0be84

Please sign in to comment.