Skip to content

Re-enable disabled tests #6422

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 6 commits into from
Apr 15, 2023
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
4 changes: 0 additions & 4 deletions Examples/package-info/Sources/package-info/example.swift
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
#if swift(>=5.7)
@preconcurrency import Basics
#else
import Basics
#endif
import TSCBasic
import Workspace

Expand Down
28 changes: 16 additions & 12 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -688,18 +688,6 @@ package.targets.append(contentsOf: [
]
),

// rdar://101868275 "error: cannot find 'XCTAssertEqual' in scope" can affect almost any functional test, so we flat out disable them all until we know what is going on
/*.testTarget(
name: "FunctionalTests",
dependencies: [
"swift-build",
"swift-package",
"swift-test",
"PackageModel",
"SPMTestSupport"
]
),*/

.testTarget(
name: "FunctionalPerformanceTests",
dependencies: [
Expand All @@ -710,6 +698,22 @@ package.targets.append(contentsOf: [
]
),
])

// rdar://101868275 "error: cannot find 'XCTAssertEqual' in scope" can affect almost any functional test, so we flat out disable them all until we know what is going on
if ProcessInfo.processInfo.environment["SWIFTCI_DISABLE_SDK_DEPENDENT_TESTS"] == nil {
package.targets.append(contentsOf: [
.testTarget(
name: "FunctionalTests",
dependencies: [
"swift-build",
"swift-package",
"swift-test",
"PackageModel",
"SPMTestSupport"
]
),
])
}
#endif

// Add package dependency on llbuild when not bootstrapping.
Expand Down
4 changes: 0 additions & 4 deletions Sources/Build/BuildPlan.swift
Original file line number Diff line number Diff line change
Expand Up @@ -770,10 +770,6 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
case.unknown:
throw InternalError("unknown binary target '\(target.name)' type")
}
case .macro:
if product.type == .macro {
staticTargets.append(target)
}
case .plugin:
continue
}
Expand Down
2 changes: 2 additions & 0 deletions Tests/FunctionalTests/DependencyResolutionTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ class DependencyResolutionTests: XCTestCase {
}

func testMirrors() throws {
try XCTSkipIf(true, "test is broken and needs investigation rdar://107970938")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test has broken while FunctionalTests were disabled and it wasn't clear to me what to do, so it remains disabled for now.


try fixture(name: "DependencyResolution/External/Mirror") { fixturePath in
let prefix = try resolveSymlinks(fixturePath)
let appPath = prefix.appending("App")
Expand Down
4 changes: 3 additions & 1 deletion Tests/FunctionalTests/PluginTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,7 @@ class PluginTests: XCTestCase {
accessibleTools: [:],
writableDirectories: [pluginDir.appending("output")],
readOnlyDirectories: [package.path],
allowNetworkConnections: [],
pkgConfigDirectories: [],
fileSystem: localFileSystem,
observabilityScope: observability.topScope,
Expand Down Expand Up @@ -729,6 +730,7 @@ class PluginTests: XCTestCase {
accessibleTools: [:],
writableDirectories: [pluginDir.appending("output")],
readOnlyDirectories: [package.path],
allowNetworkConnections: [],
pkgConfigDirectories: [],
fileSystem: localFileSystem,
observabilityScope: observability.topScope,
Expand Down Expand Up @@ -1022,7 +1024,7 @@ class PluginTests: XCTestCase {
let (stdout, _) = try executeSwiftBuild(
fixturePath.appending(component: "MySourceGenPlugin"),
configuration: .Debug,
extraArgs: ["--product", "MyLocalTool", "-Xbuild-tools-swiftc", "-DUSE_CREATING"]
extraArgs: ["--product", "MyLocalTool", "-Xbuild-tools-swiftc", "-DUSE_CREATE"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test was not working, the source code uses USE_CREATE.

)
XCTAssert(stdout.contains("Linking MySourceGenBuildTool"), "stdout:\n\(stdout)")
XCTAssert(stdout.contains("Creating foo.swift from foo.dat"), "stdout:\n\(stdout)")
Expand Down
4 changes: 3 additions & 1 deletion Tests/FunctionalTests/ResourcesTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
import XCTest
import SPMTestSupport

import TSCBasic
import struct TSCBasic.AbsolutePath
import var TSCBasic.localFileSystem
import func TSCBasic.withTemporaryDirectory

class ResourcesTests: XCTestCase {
func testSimpleResources() throws {
Expand Down
8 changes: 1 addition & 7 deletions Tests/WorkspaceTests/InitTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,6 @@ class InitTests: XCTestCase {
}

func testNonC99NameExecutablePackage() throws {
throw XCTSkip("This test fails to find XCTAssertEqual; rdar://101868275")

try withTemporaryDirectory(removeTreeOnDeinit: true) { tempDirPath in
XCTAssertDirectoryExists(tempDirPath)

Expand All @@ -256,11 +254,7 @@ class InitTests: XCTestCase {
)
try initPackage.writePackageStructure()

#if os(macOS)
XCTAssertSwiftTest(packageRoot)
#else
XCTAssertBuilds(packageRoot)
#endif
XCTAssertBuilds(packageRoot)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Executable templates no longer contain tests.

}
}

Expand Down
7 changes: 4 additions & 3 deletions Tests/WorkspaceTests/WorkspaceTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4948,8 +4948,6 @@ final class WorkspaceTests: XCTestCase {

// This verifies that the simplest possible loading APIs are available for package clients.
func testSimpleAPI() throws {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This uses an executable template which doesn't contain tests anymore, so the original flaky failure should not be possible anymore regardless of whether the newer superior Xcode fixed it or not.

throw XCTSkip("This test fails to find XCTAssertEqual; rdar://101868275")

try testWithTemporaryDirectory { path in
// Create a temporary package as a test case.
let packagePath = path.appending("MyPkg")
Expand All @@ -4963,7 +4961,10 @@ final class WorkspaceTests: XCTestCase {

// Load the workspace.
let observability = ObservabilitySystem.makeForTesting()
let workspace = try Workspace(forRootPackage: packagePath)
let workspace = try Workspace(
forRootPackage: packagePath,
customHostToolchain: UserToolchain.default
)

// From here the API should be simple and straightforward:
let manifest = try tsc_await {
Expand Down
36 changes: 5 additions & 31 deletions Utilities/bootstrap
Original file line number Diff line number Diff line change
Expand Up @@ -583,14 +583,7 @@ def add_rpath_for_cmake_build(args, rpath):

def get_swift_backdeploy_library_paths(args):
if platform.system() == 'Darwin':
# Need to include backwards compatibility libraries for Concurrency
if args.release:
# For release builds, we need a relative path that finds the compatibility libs in the current toolchain.
backdeploy_path = '@executable_path/../lib/swift-5.5/macosx'
else:
# FIXME: Would be nice if we could get this from `swiftc -print-target-info`
backdeploy_path = args.target_info["paths"]["runtimeLibraryPaths"][0] + '/../../swift-5.5/macosx'
return ['/usr/lib/swift', backdeploy_path]
return ['/usr/lib/swift']
else:
return []

Expand Down Expand Up @@ -750,7 +743,7 @@ def get_swiftpm_env_cmd(args):
env_cmd.append("SWIFTCI_INSTALL_RPATH_OS=%s" % args.platform_path.group(2))

if args.bootstrap:
libs_joined = ":".join([
libs = [
os.path.join(args.bootstrap_dir, "lib"),
os.path.join(args.build_dirs["tsc"], "lib"),
os.path.join(args.build_dirs["llbuild"], "lib"),
Expand All @@ -762,11 +755,12 @@ def get_swiftpm_env_cmd(args):
os.path.join(args.build_dirs["swift-collections"], "lib"),
os.path.join(args.build_dirs["swift-asn1"], "lib"),
os.path.join(args.build_dirs["swift-certificates"], "lib"),
] + args.target_info["paths"]["runtimeLibraryPaths"])
]

if platform.system() == 'Darwin':
env_cmd.append("DYLD_LIBRARY_PATH=%s" % libs_joined)
env_cmd.append("DYLD_LIBRARY_PATH=%s" % ":".join(libs))
else:
libs_joined = ":".join(libs + args.target_info["paths"]["runtimeLibraryPaths"])
env_cmd.append("LD_LIBRARY_PATH=%s" % libs_joined)

return env_cmd
Expand Down Expand Up @@ -799,26 +793,6 @@ def get_swiftpm_flags(args):
"-Xlinker", "@executable_path/../lib/swift/pm/llbuild",
])

# Add a relative rpath to find Swift libraries in toolchains.
# On non-Darwin platforms, a relative rpath is necessary because Swift
# libraries are not part of the OS.
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 think this comment should actually stay since we're only changing this for Darwin.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I had automerge enabled, going to restore this comment in a follow up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, we moved the non-Darwin part out of bootstrap and put it behind SWIFTCI_INSTALL_RPATH_OS, so this it is correct for the comment to be gone.

# On Darwin platforms, a relative rpath is necessary for experimental
# toolchains that include libraries not part of the OS (e.g. PythonKit or
# TensorFlow).
if '-macosx' in args.build_target:
# rpaths for compatibility libraries
for lib_path in get_swift_backdeploy_library_paths(args):
build_flags.extend(["-Xlinker", "-rpath", "-Xlinker", lib_path])

build_flags.extend(
[
"-Xlinker",
"-rpath",
"-Xlinker",
"@executable_path/../" + args.platform_path.group(1),
]
)

if '-openbsd' in args.build_target:
build_flags.extend(["-Xlinker", "-z", "-Xlinker", "origin"])
build_flags.extend(["-Xcc", "-I/usr/local/include"])
Expand Down