Skip to content

Add arguments when creating Driver for tests so that tests can run #449

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

Add arguments when creating Driver for tests so that tests can run #449

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fef6f84
augment args
Jan 30, 2021
96d3258
Respect the dashdash
Jan 30, 2021
48d4f50
Don't try for Linux
Jan 30, 2021
c03922b
Respect call for color and put extras at beginning.
Jan 31, 2021
628df5a
more test fixes
Jan 31, 2021
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
55 changes: 49 additions & 6 deletions Tests/SwiftDriverTests/Helpers/DriverExtensions.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@
//
//===----------------------------------------------------------------------===//

import SwiftDriver
@_spi(Testing) import SwiftDriver
import SwiftDriverExecution
import TSCBasic
import XCTest

extension Driver {
/// Initializer which creates an executor suitable for use in tests.
Expand All @@ -26,10 +27,52 @@ extension Driver {
processSet: ProcessSet(),
fileSystem: fileSystem,
env: env)
try self.init(args: args,
env: env,
diagnosticsEngine: diagnosticsEngine,
fileSystem: fileSystem,
executor: executor)
try self.init(
args: augment(args: args),
env: env,
diagnosticsEngine: diagnosticsEngine,
fileSystem: fileSystem,
executor: executor)
}
}

private func augment(args: [String]) throws -> [String] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Perhaps a more descriptive name than 'augment(args:)' could be used here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try to come up with something; it's a good point.

let extraArgsIndex = 1
var augmentedArgs = args

// Color codes in diagnostics cause mismatches
if !args.contains("-color-diagnostics") && !args.contains("-no-color-diagnostics") {
augmentedArgs.insert("-no-color-diagnostics", at: extraArgsIndex)
}

// The frontend fails to load the standard library because it cannot
// find it relative to the execution path used by the Swift Driver.
// So, pass in the sdk path explicitly.
if let sdkPath = try cachedSDKPath.get(), !args.contains("-sdk") {
Copy link
Contributor

@CodaFi CodaFi Feb 1, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't search for the SDK except in immediate mode. The expectation is that users and tests will provide SDKROOT in the environment or invoke swiftc via xcrun so the environment is configured for them. I recommend we provide SDKROOT in the testing environment until we land in a toolchain.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that might be better.

augmentedArgs.insert(
contentsOf: ["-sdk", sdkPath],
at: extraArgsIndex)
}
return augmentedArgs
}

private let cachedSDKPath = Result<String?, Error> {
if let pathFromEnv = ProcessEnv.vars["SDKROOT"] {
return pathFromEnv
}
#if !os(macOS)
return nil
#else
let process = Process(arguments: ["xcrun", "-sdk", "macosx", "--show-sdk-path"])
try process.launch()
let result = try process.waitUntilExit()
guard result.exitStatus == .terminated(code: EXIT_SUCCESS) else {
enum XCRunFailure: LocalizedError {
case xcrunFailure
}
throw XCRunFailure.xcrunFailure
}
return try XCTUnwrap(String(bytes: try result.output.get(), encoding: .utf8))
.spm_chomp()
#endif
}
7 changes: 5 additions & 2 deletions Tests/SwiftDriverTests/SwiftDriverTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,9 @@ final class SwiftDriverTests: XCTestCase {
XCTAssertNoThrow(try Driver(args: ["swift", ".foo"]))
XCTAssertNoThrow(try Driver(args: ["swift", "/foo"]))

XCTAssertThrowsError(try Driver(args: ["swift", "foo"]))
XCTAssertThrowsError(try Driver(args: ["swift", "foo",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It took me a while to figure out that the extra arguments were added to prevent the initializer from inserting them between "swift" and "foo", this seems kind of error-prone.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think you are right.

"-no-color-diagnostics",
"-sdk", "foo"]))
}

func testDriverKindParsing() throws {
Expand Down Expand Up @@ -3507,7 +3509,8 @@ final class SwiftDriverTests: XCTestCase {

func testIndexFilePathHandling() throws {
do {
var driver = try Driver(args: ["swiftc", "-index-file", "-index-file-path",
var driver = try Driver(args: ["swiftc", "-module-name", "mod",
"-index-file", "-index-file-path",
"bar.swift", "foo.swift", "bar.swift", "baz.swift"])
let plannedJobs = try driver.planBuild()
XCTAssertEqual(plannedJobs.count, 1)
Expand Down