[Build] BuildPlan: Always discover test modules through their aggrega… (#7879)

…te products

### Motivation:

Fixes a situation when tests were build for both host and target even
though they should have been built only for the host (i.e. when one of
the modules depends on a macro).

### Modifications:

- Adjusted `BuildPlan.computeDestinations` to never add `test` modules
to successor list of a package and allow `test` products to always
iterate their module dependencies instead.

### Result:

If one of the test targets depends on a macro we'd never attempt to
build others for "target".

Author: xedin

Sources/Build/BuildPlan/BuildPlan.swift

@@ -986,9 +986,9 @@ extension BuildPlan {
             }
 
             for module in package.modules {
-                if case .test = module.underlying.type,
-                   !graph.rootPackages.contains(id: package.id)
-                {
+                // Tests are discovered through an aggregate product which also
+                // informs their destination.
+                if case .test = module.underlying.type {
                     continue
                 }
 
@@ -1002,7 +1002,7 @@ extension BuildPlan {
             for product: ResolvedProduct,
             destination: Destination
         ) -> [TraversalNode] {
-            guard destination == .host else {
+            guard destination == .host || product.underlying.type == .test else {
                 return []
             }
 

Sources/_InternalTestSupport/MockPackageGraphs.swift

@@ -137,6 +137,7 @@ package func macrosTestsPackageGraph() throws -> MockPackageGraph {
         "/swift-mmio/Sources/MMIOMacros/source.swift",
         "/swift-mmio/Sources/MMIOMacrosTests/source.swift",
         "/swift-mmio/Sources/MMIOMacro+PluginTests/source.swift",
+        "/swift-mmio/Sources/NOOPTests/source.swift",
         "/swift-syntax/Sources/SwiftSyntax/source.swift",
         "/swift-syntax/Sources/SwiftSyntaxMacrosTestSupport/source.swift",
         "/swift-syntax/Sources/SwiftSyntaxMacros/source.swift",
@@ -203,6 +204,11 @@ package func macrosTestsPackageGraph() throws -> MockPackageGraph {
                             .target(name: "MMIOMacros")
                         ],
                         type: .test
+                    ),
+                    TargetDescription(
+                        name: "NOOPTests",
+                        dependencies: [],
+                        type: .test
                     )
                 ]
             ),

Tests/BuildTests/CrossCompilationBuildPlanTests.swift

@@ -295,9 +295,15 @@ final class CrossCompilationBuildPlanTests: XCTestCase {
             fileSystem: fs,
             observabilityScope: scope
         )
+
+        // Make sure that build plan doesn't have any "target" tests.
+        for (_, description) in plan.targetMap where description.module.underlying.type == .test {
+            XCTAssertEqual(description.buildParameters.destination, .host)
+        }
+
         let result = try BuildPlanResult(plan: plan)
         result.checkProductsCount(2)
-        result.checkTargetsCount(16)
+        result.checkTargetsCount(17)
 
         XCTAssertTrue(try result.allTargets(named: "SwiftSyntax")
             .map { try $0.swift() }

Tests/PackageGraphTests/CrossCompilationPackageGraphTests.swift

@@ -116,7 +116,11 @@ final class CrossCompilationPackageGraphTests: XCTestCase {
                 "SwiftSyntaxMacrosTestSupport",
                 "SwiftSyntaxMacrosTestSupport"
             )
-            result.check(testModules: "MMIOMacrosTests", "MMIOMacro+PluginTests")
+            // TODO: NOOPTests are mentioned twice because in the graph they appear
+            // as if they target both "tools" and "destination", see the test below.
+            // Once the `buildTriple` is gone, there is going to be only one mention
+            // left.
+            result.check(testModules: "MMIOMacrosTests", "MMIOMacro+PluginTests", "NOOPTests", "NOOPTests")
             result.checkTarget("MMIO") { result in
                 result.check(buildTriple: .destination)
                 result.check(dependencies: "MMIOMacros")
@@ -180,6 +184,13 @@ final class CrossCompilationPackageGraphTests: XCTestCase {
                     XCTAssertEqual(graph.package(for: result.target)?.identity, .plain("swift-syntax"))
                 }
             }
+
+            result.checkTargets("NOOPTests") { results in
+                XCTAssertEqual(results.count, 2)
+
+                XCTAssertEqual(results.filter({ $0.target.buildTriple == .tools }).count, 1)
+                XCTAssertEqual(results.filter({ $0.target.buildTriple == .destination }).count, 1)
+            }
         }
     }