-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Implement --enable-parseable-module-interfaces for swift-build buildsystem #8421
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
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,7 +38,7 @@ class BuildCommandTestCases: CommandsBuildProviderTestCase { | |
} | ||
|
||
@discardableResult | ||
private func execute( | ||
func execute( | ||
_ args: [String] = [], | ||
environment: Environment? = nil, | ||
packagePath: AbsolutePath? = nil | ||
|
@@ -70,7 +70,19 @@ class BuildCommandTestCases: CommandsBuildProviderTestCase { | |
// is what `binContents` is meant to represent. | ||
return contents != ["output-file-map.json"] | ||
} | ||
let moduleContents = (try? localFileSystem.getDirectoryContents(binPath.appending(component: "Modules"))) ?? [] | ||
var moduleContents: [String] = [] | ||
if buildSystemProvider == .native { | ||
moduleContents = (try? localFileSystem.getDirectoryContents(binPath.appending(component: "Modules"))) ?? [] | ||
} else { | ||
let moduleDirs = (try? localFileSystem.getDirectoryContents(binPath).filter { | ||
$0.hasSuffix(".swiftmodule") | ||
}) ?? [] | ||
for dir: String in moduleDirs { | ||
moduleContents += | ||
(try? localFileSystem.getDirectoryContents(binPath.appending(component: dir)).map { "\(dir)/\($0)" }) ?? [] | ||
} | ||
} | ||
|
||
|
||
if cleanAfterward { | ||
try! await executeSwiftPackage( | ||
|
@@ -103,6 +115,10 @@ class BuildCommandTestCases: CommandsBuildProviderTestCase { | |
XCTAssertMatch(stdout, .contains("USAGE: swift build")) | ||
} | ||
|
||
func testBinSymlink() async throws { | ||
XCTAssertTrue(false, "Must be implemented at build system test class.") | ||
} | ||
|
||
func testSeeAlso() async throws { | ||
let stdout = try await execute(["--help"]).stdout | ||
XCTAssertMatch(stdout, .contains("SEE ALSO: swift run, swift package, swift test")) | ||
|
@@ -190,48 +206,6 @@ class BuildCommandTestCases: CommandsBuildProviderTestCase { | |
} | ||
} | ||
|
||
func testBinSymlink() async throws { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: In order to ensure the "concrete" class implement this this, can we keep an implementation that will fail, forcing the subclasses to implement the test? This way, we are guaranteed that each sub-class will implement this test. |
||
try await fixture(name: "ValidLayouts/SingleModule/ExecutableNew") { fixturePath in | ||
let fullPath = try resolveSymlinks(fixturePath) | ||
let targetPath = try fullPath.appending( | ||
components: ".build", | ||
UserToolchain.default.targetTriple.platformBuildPathComponent | ||
) | ||
let xcbuildTargetPath = fullPath.appending(components: ".build", "apple") | ||
try await XCTAssertAsyncEqual( | ||
try await self.execute(["--show-bin-path"], packagePath: fullPath).stdout, | ||
"\(targetPath.appending("debug").pathString)\n" | ||
) | ||
try await XCTAssertAsyncEqual( | ||
try await self.execute(["-c", "release", "--show-bin-path"], packagePath: fullPath).stdout, | ||
"\(targetPath.appending("release").pathString)\n" | ||
) | ||
|
||
guard buildSystemProvider == .xcode || buildSystemProvider == .swiftbuild else { | ||
// The remainder of this test only applies to XCBuild or Swift Build | ||
return | ||
} | ||
|
||
// Print correct path when building with XCBuild or Swift Build | ||
let xcodeDebugOutput = try await execute( | ||
["-c", "debug", "--show-bin-path"], | ||
packagePath: fullPath) | ||
.stdout | ||
let xcodeReleaseOutput = try await execute( | ||
["-c", "release", "--show-bin-path"], | ||
packagePath: fullPath | ||
).stdout | ||
XCTAssertEqual( | ||
xcodeDebugOutput, | ||
"\(xcbuildTargetPath.appending(components: "Products", "Debug").pathString)\n" | ||
) | ||
XCTAssertEqual( | ||
xcodeReleaseOutput, | ||
"\(xcbuildTargetPath.appending(components: "Products", "Release").pathString)\n" | ||
) | ||
} | ||
} | ||
|
||
func testSymlink() async throws { | ||
try await fixture(name: "ValidLayouts/SingleModule/ExecutableNew") { fixturePath in | ||
let fullPath = try resolveSymlinks(fixturePath) | ||
|
@@ -792,6 +766,25 @@ class BuildCommandNativeTests: BuildCommandTestCases { | |
override func testUsage() async throws { | ||
try await super.testUsage() | ||
} | ||
|
||
override func testBinSymlink() async throws { | ||
try await fixture(name: "ValidLayouts/SingleModule/ExecutableNew") { fixturePath in | ||
let fullPath = try resolveSymlinks(fixturePath) | ||
let targetPath = try fullPath.appending( | ||
components: ".build", | ||
UserToolchain.default.targetTriple.platformBuildPathComponent | ||
) | ||
try await XCTAssertAsyncEqual( | ||
try await self.execute(["--show-bin-path"], packagePath: fullPath).stdout, | ||
"\(targetPath.appending("debug").pathString)\n" | ||
) | ||
try await XCTAssertAsyncEqual( | ||
try await self.execute(["-c", "release", "--show-bin-path"], packagePath: fullPath) | ||
.stdout, | ||
"\(targetPath.appending("release").pathString)\n" | ||
) | ||
} | ||
} | ||
} | ||
|
||
#if os(macOS) | ||
|
@@ -806,51 +799,51 @@ class BuildCommandXcodeTests: BuildCommandTestCases { | |
} | ||
|
||
override func testAutomaticParseableInterfacesWithLibraryEvolution() async throws { | ||
try XCTSkip("Test not implemented for xcode build system.") | ||
throw XCTSkip("Test not implemented for xcode build system.") | ||
} | ||
|
||
override func testNonReachableProductsAndTargetsFunctional() async throws { | ||
try XCTSkip("Test not implemented for xcode build system.") | ||
throw XCTSkip("Test not implemented for xcode build system.") | ||
} | ||
|
||
override func testCodeCoverage() async throws { | ||
try XCTSkip("Test not implemented for xcode build system.") | ||
throw XCTSkip("Test not implemented for xcode build system.") | ||
} | ||
|
||
override func testBuildStartMessage() async throws { | ||
try XCTSkip("Test not implemented for xcode build system.") | ||
throw XCTSkip("Test not implemented for xcode build system.") | ||
} | ||
|
||
override func testBinSymlink() async throws { | ||
try XCTSkip("Test not implemented for xcode build system.") | ||
throw XCTSkip("Test not implemented for xcode build system.") | ||
} | ||
|
||
override func testSymlink() async throws { | ||
try XCTSkip("Test not implemented for xcode build system.") | ||
throw XCTSkip("Test not implemented for xcode build system.") | ||
} | ||
|
||
override func testSwiftGetVersion() async throws { | ||
try XCTSkip("Test not implemented for xcode build system.") | ||
throw XCTSkip("Test not implemented for xcode build system.") | ||
} | ||
|
||
override func testParseableInterfaces() async throws { | ||
try XCTSkip("Test not implemented for xcode build system.") | ||
throw XCTSkip("Test not implemented for xcode build system.") | ||
} | ||
|
||
override func testProductAndTarget() async throws { | ||
try XCTSkip("Test not implemented for xcode build system.") | ||
throw XCTSkip("Test not implemented for xcode build system.") | ||
} | ||
|
||
override func testImportOfMissedDepWarning() async throws { | ||
try XCTSkip("Test not implemented for xcode build system.") | ||
throw XCTSkip("Test not implemented for xcode build system.") | ||
} | ||
|
||
override func testGetTaskAllowEntitlement() async throws { | ||
try XCTSkip("Test not implemented for xcode build system.") | ||
throw XCTSkip("Test not implemented for xcode build system.") | ||
} | ||
|
||
override func testBuildCompleteMessage() async throws { | ||
try XCTSkip("Test not implemented for xcode build system.") | ||
throw XCTSkip("Test not implemented for xcode build system.") | ||
} | ||
} | ||
#endif | ||
|
@@ -866,9 +859,31 @@ class BuildCommandSwiftBuildTests: BuildCommandTestCases { | |
} | ||
|
||
override func testParseableInterfaces() async throws { | ||
throw XCTSkip("SWBINTTODO: Test failed with swiftbuild engine because the --enable-parseable-module-interfaces flag doesn't yet produce .swiftinterface files. This needs to be investigated") | ||
try await fixture(name: "Miscellaneous/ParseableInterfaces") { fixturePath in | ||
do { | ||
let result = try await build(["--enable-parseable-module-interfaces"], packagePath: fixturePath) | ||
XCTAssertMatch(result.moduleContents, [.regex(#"A\.swiftmodule\/.*\.swiftinterface"#)]) | ||
XCTAssertMatch(result.moduleContents, [.regex(#"B\.swiftmodule\/.*\.swiftmodule"#)]) | ||
} catch SwiftPMError.executionFailure(_, _, let stderr) { | ||
XCTFail(stderr) | ||
} | ||
} | ||
} | ||
|
||
override func testBinSymlink() async throws { | ||
try await fixture(name: "ValidLayouts/SingleModule/ExecutableNew") { fixturePath in | ||
let fullPath = try resolveSymlinks(fixturePath) | ||
let targetPath = try fullPath.appending( | ||
components: ".build", | ||
UserToolchain.default.targetTriple.platformBuildPathComponent | ||
) | ||
let debugPath = try await self.execute(["--show-bin-path"], packagePath: fullPath).stdout | ||
XCTAssertMatch(debugPath, .regex(targetPath.appending(components: "Products", "Debug").pathString + "(\\-linux|\\-Windows)?\\n")) | ||
let releasePath = try await self.execute(["-c", "release", "--show-bin-path"], packagePath: fullPath).stdout | ||
XCTAssertMatch(releasePath, .regex(targetPath.appending(components: "Products", "Release").pathString + "(\\-linux|\\-Windows)?\\n")) | ||
} | ||
} | ||
|
||
override func testGetTaskAllowEntitlement() async throws { | ||
throw XCTSkip("SWBINTTODO: Test failed because swiftbuild doesn't output precis codesign commands. Once swift run works with swiftbuild the test can be investigated.") | ||
} | ||
|
@@ -880,13 +895,20 @@ class BuildCommandSwiftBuildTests: BuildCommandTestCases { | |
override func testAtMainSupport() async throws { | ||
#if !os(macOS) | ||
throw XCTSkip("SWBINTTODO: File not found or missing libclang errors on non-macOS platforms. This needs to be investigated") | ||
#endif | ||
|
||
#else | ||
try await super.testAtMainSupport() | ||
#endif | ||
} | ||
|
||
override func testAutomaticParseableInterfacesWithLibraryEvolution() async throws { | ||
throw XCTSkip("SWBINTTODO: The test fails because when the unsafe flag for a target is set to '-enable-library-evolution' it is not producing the correct .swiftinterface files. This needs to be investigated") | ||
try await fixture(name: "Miscellaneous/LibraryEvolution") { fixturePath in | ||
daveinglis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
do { | ||
let result = try await build([], packagePath: fixturePath) | ||
XCTAssertMatch( | ||
result.moduleContents, [.regex(#"A\.swiftmodule\/.*\.swiftinterface"#)]) | ||
XCTAssertMatch(result.moduleContents, [.regex(#"B\.swiftmodule\/.*\.swiftmodule"#)]) | ||
} | ||
} | ||
} | ||
|
||
override func testImportOfMissedDepWarning() async throws { | ||
|
@@ -901,10 +923,6 @@ class BuildCommandSwiftBuildTests: BuildCommandTestCases { | |
throw XCTSkip("SWBINTTODO: Test fails because the dummy-swiftc used in the test isn't accepted by swift-build. This needs to be investigated") | ||
} | ||
|
||
override func testBinSymlink() async throws { | ||
throw XCTSkip("SWBINTTODO: Test fails because of a difference in the build layout. This needs to be updated to the expected path") | ||
} | ||
|
||
override func testSymlink() async throws { | ||
throw XCTSkip("SWBINTTODO: Test fails because of a difference in the build layout. This needs to be updated to the expected path") | ||
} | ||
|
@@ -927,7 +945,7 @@ class BuildCommandSwiftBuildTests: BuildCommandTestCases { | |
|
||
override func testBuildSystemDefaultSettings() async throws { | ||
#if os(Linux) | ||
if FileManager.default.contents(atPath: "/etc/system-release").map { String(decoding: $0, as: UTF8.self) == "Amazon Linux release 2 (Karoo)\n" } ?? false { | ||
if FileManager.default.contents(atPath: "/etc/system-release").map( { String(decoding: $0, as: UTF8.self) == "Amazon Linux release 2 (Karoo)\n" } ) ?? false { | ||
throw XCTSkip("Skipping SwiftBuild testing on Amazon Linux because of platform issues.") | ||
} | ||
#endif | ||
|
@@ -941,9 +959,10 @@ class BuildCommandSwiftBuildTests: BuildCommandTestCases { | |
|
||
override func testBuildCompleteMessage() async throws { | ||
#if os(Linux) | ||
try XCTSkip("SWBINTTODO: Need to properly set LD_LIBRARY_PATH on linux") | ||
#endif | ||
throw XCTSkip("SWBINTTODO: Need to properly set LD_LIBRARY_PATH on linux") | ||
#else | ||
try await super.testBuildCompleteMessage() | ||
#endif | ||
} | ||
|
||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: instead of creating an
if
statement to control the behaviour of thebuild
function, can we instead call a function that is overridden in the sub classes?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should wait since we will be moving to swift testing, we can refactor these tests better.