Skip to content

Fix APIDiff test suite #3593

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

Merged
merged 2 commits into from
Jul 12, 2021
Merged

Fix APIDiff test suite #3593

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8fb3dcf
Revert "Temporarily disable failing APIDiff tests to unblock builds (…
owenv Jul 3, 2021
0f80435
Use supported flags to determine if APIDiff tests should run with a g…
owenv Jul 3, 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
2 changes: 1 addition & 1 deletion Package.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ let package = Package(
dependencies: ["Build", "SPMTestSupport"]),
.testTarget(
name: "CommandsTests",
dependencies: ["swift-build", "swift-package", "swift-test", "swift-run", "Commands", "Workspace", "SPMTestSupport"]),
dependencies: ["swift-build", "swift-package", "swift-test", "swift-run", "Commands", "Workspace", "SPMTestSupport", "Build"]),
Copy link
Contributor

Choose a reason for hiding this comment

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

@neonichu @abertelrud wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like the same dependency we ran into trouble with before, where running the unit tests for even libSwiftPMDataModel will now need Build and everything it brings with it (llbuild, etc). That will make it not possible to be able to build any of the tests on iOS, for example.

But I think it's the exact same issue as the need to test for the -entry-point-function-name flag, so we should be able to use the same solution here.

We should make also add something to the Package to check that there is no transitive dependency of the tests on Build, to give an earlier warning.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CommandsTests already has a transitive dependency on Build via Commands, so I'm not sure this changes much - are these tests being run on iOS today?

The check the entry point name tests use:

#if swift(<5.5)
        try XCTSkipIf(true, "skipping because host compiler doesn't support '-entry-point-function-name'")
#endif

won't work for this because the version of Swift which has the new flag doesn't have a version number yet

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I think this might be fine then, what was problematic was adding the Build dependency in SPMTestSupport so it would be needed for any test suite.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh sorry, I missed that this was CommandsTests and not SPMTestSupport. This is fine, since only the libSwiftPMDataModel tests are run on iOS. Shelling out on iOS is prohibited anyway, so none of the command-related tests can work.

.testTarget(
name: "WorkspaceTests",
dependencies: ["Workspace", "SPMTestSupport"]),
Expand Down
53 changes: 11 additions & 42 deletions Tests/CommandsTests/APIDiffTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import XCTest
import Foundation
import TSCBasic
import Build
import Commands
import SPMTestSupport

Expand All @@ -28,20 +29,20 @@ final class APIDiffTests: XCTestCase {
}

func skipIfApiDigesterUnsupported() throws {
guard let tool = try? Resources.default.toolchain.getSwiftAPIDigester() else {
// swift-api-digester is required to run tests.
guard (try? Resources.default.toolchain.getSwiftAPIDigester()) != nil else {
throw XCTSkip("swift-api-digester unavailable")
}
guard localFileSystem.isSymlink(tool) else {
// The version of Swift with a supported swift-api-digester doesn't have
// a version number yet, so use whether or not the tool is a symlink to
// determine if it's from a recent snapshot.
// SwiftPM's swift-api-digester integration relies on post-5.5 bugfixes and features,
// not all of which can be tested for easily. Fortunately, we can test for the
// `-disable-fail-on-error` option, and any version which supports this flag
// will meet the other requirements.
guard SwiftTargetBuildDescription.checkSupportedFrontendFlags(flags: ["disable-fail-on-error"], fs: localFileSystem) else {
throw XCTSkip("swift-api-digester is too old")
}
}

func testSimpleAPIDiff() throws {
throw XCTSkip("Fix this test")

try skipIfApiDigesterUnsupported()
fixture(name: "Miscellaneous/APIDiff/") { prefix in
let packageRoot = prefix.appending(component: "Foo")
Expand All @@ -61,8 +62,6 @@ final class APIDiffTests: XCTestCase {
}

func testMultiTargetAPIDiff() throws {
throw XCTSkip("Fix this test")

try skipIfApiDigesterUnsupported()
fixture(name: "Miscellaneous/APIDiff/") { prefix in
let packageRoot = prefix.appending(component: "Bar")
Expand All @@ -87,12 +86,7 @@ final class APIDiffTests: XCTestCase {
}

func testBreakageAllowlist() throws {
throw XCTSkip("Fix this test")

#if os(macOS)
guard (try? Resources.default.toolchain.getSwiftAPIDigester()) != nil else {
throw XCTSkip("swift-api-digester not available")
}
try skipIfApiDigesterUnsupported()
fixture(name: "Miscellaneous/APIDiff/") { prefix in
let packageRoot = prefix.appending(component: "Bar")
try localFileSystem.writeFileContents(packageRoot.appending(components: "Sources", "Baz", "Baz.swift")) {
Expand Down Expand Up @@ -120,14 +114,9 @@ final class APIDiffTests: XCTestCase {
}

}
#else
throw XCTSkip("Test unsupported on current platform")
#endif
}

func testCheckVendedModulesOnly() throws {
throw XCTSkip("Fix this test")

try skipIfApiDigesterUnsupported()
fixture(name: "Miscellaneous/APIDiff/") { prefix in
let packageRoot = prefix.appending(component: "NonAPILibraryTargets")
Expand Down Expand Up @@ -164,8 +153,6 @@ final class APIDiffTests: XCTestCase {
}

func testFilters() throws {
throw XCTSkip("Fix this test")

try skipIfApiDigesterUnsupported()
fixture(name: "Miscellaneous/APIDiff/") { prefix in
let packageRoot = prefix.appending(component: "NonAPILibraryTargets")
Expand Down Expand Up @@ -238,8 +225,6 @@ final class APIDiffTests: XCTestCase {
}

func testAPIDiffOfModuleWithCDependency() throws {
throw XCTSkip("Fix this test")

try skipIfApiDigesterUnsupported()
fixture(name: "Miscellaneous/APIDiff/") { prefix in
let packageRoot = prefix.appending(component: "CTargetDep")
Expand Down Expand Up @@ -338,12 +323,7 @@ final class APIDiffTests: XCTestCase {
}

func testBaselineDirOverride() throws {
throw XCTSkip("Fix this test")

#if os(macOS)
guard (try? Resources.default.toolchain.getSwiftAPIDigester()) != nil else {
throw XCTSkip("swift-api-digester not available")
}
try skipIfApiDigesterUnsupported()
fixture(name: "Miscellaneous/APIDiff/") { prefix in
let packageRoot = prefix.appending(component: "Foo")
// Overwrite the existing decl.
Expand All @@ -365,18 +345,10 @@ final class APIDiffTests: XCTestCase {
XCTAssertTrue(localFileSystem.exists(baselineDir.appending(components: "1.2.3", "Foo.json")))
}
}
#else
throw XCTSkip("Test unsupported on current platform")
#endif
}

func testRegenerateBaseline() throws {
throw XCTSkip("Fix this test")

#if os(macOS)
guard (try? Resources.default.toolchain.getSwiftAPIDigester()) != nil else {
throw XCTSkip("swift-api-digester not available")
}
try skipIfApiDigesterUnsupported()
fixture(name: "Miscellaneous/APIDiff/") { prefix in
let packageRoot = prefix.appending(component: "Foo")
// Overwrite the existing decl.
Expand Down Expand Up @@ -406,8 +378,5 @@ final class APIDiffTests: XCTestCase {
XCTAssertNotEqual((try! localFileSystem.readFileContents(fooBaselinePath)).description, "Old Baseline")
}
}
#else
throw XCTSkip("Test unsupported on current platform")
#endif
}
}