Skip to content

Commit 5b2118b

Browse files
johnbuteJohn Bute
johnbute
and
John Bute
authored
Prevent non-targets from depending on test targets (#8513)
Prevent non-targets from depending on test targets ### Motivation: Fix for rdar://149007214 Currently, only test targets are allowed to have dependencies on other test targets. ### Modifications: Simply checked for each non-test target, if their dependency is of type test. If it is, throw an error. ### Result: Swift package manager will give an error explaining that only testTargets can depend on other testTargets Fixes #8478 --------- Co-authored-by: John Bute <[email protected]>
1 parent 2b659da commit 5b2118b

File tree

3 files changed

+194
-0
lines changed

3 files changed

+194
-0
lines changed

‎Sources/PackageLoading/ManifestLoader+Validation.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,12 @@ extension Basics.Diagnostic {
311311
return .error("\(messagePrefix) in dependencies of target '\(targetName)'; valid packages are: \(validPackages.map{ "\($0.descriptionForValidation)" }.joined(separator: ", "))")
312312
}
313313

314+
static func invalidDependencyOnTestTarget(dependency: Module.Dependency, targetName: String) -> Self {
315+
.error(
316+
"Invalid dependency: '\(targetName)' cannot depend on test target dependency '\(dependency.name)'. Only test targets can depend on other test targets"
317+
)
318+
}
319+
314320
static func invalidBinaryLocation(targetName: String) -> Self {
315321
.error("invalid location for binary target '\(targetName)'")
316322
}

‎Sources/PackageLoading/PackageBuilder.swift

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -951,6 +951,19 @@ public final class PackageBuilder {
951951
}
952952
}
953953

954+
// Ensure non-test targets do not depend on test targets.
955+
// Only test targets are allowed to have dependencies on other test targets.
956+
if !potentialModule.isTest {
957+
for dependency in dependencies {
958+
if let depTarget = dependency.module, depTarget.type == .test {
959+
self.observabilityScope.emit(.invalidDependencyOnTestTarget(
960+
dependency: dependency,
961+
targetName: potentialModule.name
962+
))
963+
}
964+
}
965+
}
966+
954967
// Create the build setting assignment table for this target.
955968
let buildSettings = try self.buildSettings(
956969
for: manifestTarget,

‎Tests/PackageGraphTests/ModulesGraphTests.swift

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,181 @@ final class ModulesGraphTests: XCTestCase {
412412
}
413413
}
414414

415+
func testLibraryInvalidDependencyOnTestTarget() throws {
416+
let fs = InMemoryFileSystem(
417+
emptyFiles:
418+
"/Foo/Sources/Foo/Foo.swift",
419+
"/Foo/Tests/FooTest/FooTest.swift"
420+
)
421+
422+
let observability = ObservabilitySystem.makeForTesting()
423+
424+
let _ = try loadModulesGraph(
425+
fileSystem: fs,
426+
manifests: [
427+
Manifest.createRootManifest(
428+
displayName: "Foo",
429+
path: "/Foo",
430+
toolsVersion: .v6_0,
431+
products: [
432+
ProductDescription(name: "Foo", type: .library(.automatic), targets: ["FooTest"]),
433+
],
434+
targets: [
435+
TargetDescription(name: "Foo", dependencies: ["FooTest"]),
436+
TargetDescription(name: "FooTest", type: .test),
437+
]
438+
),
439+
],
440+
observabilityScope: observability.topScope
441+
)
442+
443+
testDiagnostics(observability.diagnostics) { result in
444+
result.check(
445+
diagnostic: "Invalid dependency: 'Foo' cannot depend on test target dependency 'FooTest'. Only test targets can depend on other test targets",
446+
severity: .error
447+
)
448+
}
449+
}
450+
451+
func testExecutableInvalidDependencyOnTestTarget() throws {
452+
let fs = InMemoryFileSystem(
453+
emptyFiles:
454+
"/Foo/Sources/Foo/main.swift",
455+
"/Foo/Tests/FooTest/FooTest.swift"
456+
)
457+
458+
let observability = ObservabilitySystem.makeForTesting()
459+
460+
let _ = try loadModulesGraph(
461+
fileSystem: fs,
462+
manifests: [
463+
Manifest.createRootManifest(
464+
displayName: "Foo",
465+
path: "/Foo",
466+
toolsVersion: .v6_0,
467+
targets: [
468+
TargetDescription(name: "Foo", dependencies: ["FooTest"], type: .executable),
469+
TargetDescription(name: "FooTest", type: .test),
470+
]
471+
),
472+
],
473+
observabilityScope: observability.topScope
474+
)
475+
476+
testDiagnostics(observability.diagnostics) { result in
477+
result.check(
478+
diagnostic: "Invalid dependency: 'Foo' cannot depend on test target dependency 'FooTest'. Only test targets can depend on other test targets",
479+
severity: .error
480+
)
481+
}
482+
}
483+
484+
func testPluginInvalidDependencyOnTestTarget() throws {
485+
let fs = InMemoryFileSystem(
486+
emptyFiles:
487+
"/Foo/Plugins/Foo/main.swift",
488+
"/Foo/Tests/FooTest/FooTest.swift"
489+
)
490+
491+
let observability = ObservabilitySystem.makeForTesting()
492+
493+
let _ = try loadModulesGraph(
494+
fileSystem: fs,
495+
manifests: [
496+
Manifest.createRootManifest(
497+
displayName: "Foo",
498+
path: "/Foo",
499+
toolsVersion: .v6_0,
500+
targets: [
501+
TargetDescription(
502+
name: "Foo",
503+
dependencies: ["FooTest"],
504+
type: .plugin,
505+
pluginCapability: .buildTool
506+
),
507+
TargetDescription(name: "FooTest", type: .test),
508+
]
509+
),
510+
],
511+
observabilityScope: observability.topScope
512+
)
513+
514+
testDiagnostics(observability.diagnostics) { result in
515+
result.check(
516+
diagnostic: "Invalid dependency: 'Foo' cannot depend on test target dependency 'FooTest'. Only test targets can depend on other test targets",
517+
severity: .error
518+
)
519+
}
520+
}
521+
522+
func testMacroInvalidDependencyOnTestTarget() throws {
523+
let fs = InMemoryFileSystem(
524+
emptyFiles:
525+
"/Foo/Sources/Foo/main.swift",
526+
"/Foo/Tests/FooTest/FooTest.swift"
527+
)
528+
529+
let observability = ObservabilitySystem.makeForTesting()
530+
531+
let _ = try loadModulesGraph(
532+
fileSystem: fs,
533+
manifests: [
534+
Manifest.createRootManifest(
535+
displayName: "Foo",
536+
path: "/Foo",
537+
toolsVersion: .v6_0,
538+
targets: [
539+
TargetDescription(
540+
name: "Foo",
541+
dependencies: ["FooTest"],
542+
type: .macro
543+
),
544+
TargetDescription(name: "FooTest", type: .test),
545+
]
546+
),
547+
],
548+
observabilityScope: observability.topScope
549+
)
550+
551+
testDiagnostics(observability.diagnostics) { result in
552+
result.check(
553+
diagnostic: "Invalid dependency: 'Foo' cannot depend on test target dependency 'FooTest'. Only test targets can depend on other test targets",
554+
severity: .error
555+
)
556+
}
557+
}
558+
559+
560+
func testValidDependencyOnTestTarget() throws {
561+
let fs = InMemoryFileSystem(
562+
emptyFiles:
563+
"/Foo/Tests/Foo/Foo.swift",
564+
"/Foo/Tests/FooTest/FooTest.swift"
565+
)
566+
567+
let observability = ObservabilitySystem.makeForTesting()
568+
569+
let _ = try loadModulesGraph(
570+
fileSystem: fs,
571+
manifests: [
572+
Manifest.createRootManifest(
573+
displayName: "Foo",
574+
path: "/Foo",
575+
toolsVersion: .v6_0,
576+
products: [
577+
],
578+
targets: [
579+
TargetDescription(name: "Foo", dependencies: ["FooTest"], type: .test),
580+
TargetDescription(name: "FooTest", type: .test),
581+
]
582+
),
583+
],
584+
observabilityScope: observability.topScope
585+
)
586+
587+
XCTAssertNoDiagnostics(observability.diagnostics)
588+
}
589+
415590
// Make sure there is no error when we reference Test targets in a package and then
416591
// use it as a dependency to another package. SR-2353
417592
func testTestTargetDeclInExternalPackage() throws {

0 commit comments

Comments
 (0)