Merge branch 'test-target-deps' of https://github.com/aciidb0mb3r/swift-package-manager

Author: ddunbar

Package.swift

@@ -101,7 +101,19 @@ let package = Package(
             dependencies: ["Commands"]),
         Target(
             /** Shim tool to find test names on OS X */
-            name: "swiftpm-xctest-helper"),
+            name: "swiftpm-xctest-helper",
+            dependencies: []),
+        Target(
+            /** FIXME: The Basic tests currently have a layering violation and a dependency on Utility for infrastructure. */
+            name: "BasicTests",
+            dependencies: ["Basic", "Utility"]),
+        Target(
+            name: "FunctionalTests",
+            dependencies: ["Basic", "Utility", "PackageModel"]),
+        Target(
+            /** FIXME: Turns out PackageLoadingTests violate encapsulation :( */
+            name: "PackageLoadingTests",
+            dependencies: ["Get", "PackageLoading"]),
     ])
 
 

Sources/PackageLoading/PackageBuilder.swift

@@ -214,6 +214,7 @@ public struct PackageBuilder {
         } else {
             testModules = []
         }
+        try fillDependencies(modules: modules + testModules)
         let products = try constructProducts(modules, testModules: testModules)
         return Package(manifest: manifest, path: packagePath, modules: modules, testModules: testModules, products: products)
     }
@@ -344,6 +345,12 @@ public struct PackageBuilder {
             }
         }
 
+        return modules
+    }
+
+    /// Fills the module dependencies delcared via targets in manifest.
+    private func fillDependencies(modules: [Module]) throws {
+
         // Create a map of modules indexed by name.
         var modulesByName = [String: Module]()
         for module in modules {
@@ -382,7 +389,15 @@ public struct PackageBuilder {
             throw ModuleError.modulesNotFound(missingModuleNames)
         }
 
-        return modules
+        // Normally, test modules are only dependent upon modules with
+        // the same basename. For example, a test module in
+        // 'Root/Tests/FooTests' is dependent upon 'Root/Sources/Foo'.
+        // Only do this if there is no explict dependency declared in manifest.
+        for module in modules where module.isTest && module.dependencies.isEmpty {
+            if let baseModule = modulesByName[module.basename] {
+                module.dependencies = [baseModule]
+            }
+        }
     }
 
     /// Private function that checks whether a module name is valid.  This method doesn't return anything, but rather, if there's a problem, it throws an error describing what the problem is.
@@ -504,57 +519,8 @@ public struct PackageBuilder {
         }
 
         // Create the test modules
-        let testModules = try directoryContents(testsPath).filter(shouldConsiderDirectory).flatMap { dir in
+        return try directoryContents(testsPath).filter(shouldConsiderDirectory).flatMap { dir in
             return [try createModule(dir, name: dir.basename, isTest: true)]
         }
-
-        // Populate the test module dependencies.
-        for case let testModule as SwiftModule in testModules {
-            // FIXME: This is very inefficient, we want a map on the modules.
-            if testModule.basename == "Basic" {
-                // FIXME: The Basic tests currently have a layering
-                // violation and a dependency on Utility for infrastructure.
-                testModule.dependencies = modules.filter{
-                    switch $0.name {
-                    case "Basic", "Utility":
-                        return true
-                    default:
-                        return false
-                    }
-                }
-            } else if testModule.basename == "Functional" {
-                // FIXME: swiftpm's own Functional tests module does not
-                //        follow the normal rules--there is no corresponding
-                //        'Sources/Functional' module to depend upon. For the
-                //        time being, assume test modules named 'Functional'
-                //        depend upon 'Utility', and hope that no users define
-                //        test modules named 'Functional'.
-                testModule.dependencies = modules.filter{
-                    switch $0.name {
-                    case "Basic", "Utility", "PackageModel":
-                        return true
-                    default:
-                        return false
-                    }
-                }
-            } else if testModule.basename == "PackageLoading" {
-                // FIXME: Turns out PackageLoadingTests violate encapsulation :(
-                testModule.dependencies = modules.filter{
-                    switch $0.name {
-                    case "Get", "PackageLoading":
-                        return true
-                    default:
-                        return false
-                    }
-                }
-            } else {
-                // Normally, test modules are only dependent upon modules with
-                // the same basename. For example, a test module in
-                // 'Root/Tests/Foo' is dependent upon 'Root/Sources/Foo'.
-                testModule.dependencies = modules.filter{ $0.name == testModule.basename }
-            }
-        }
-
-        return testModules
     }
 }

Tests/PackageLoadingTests/ConventionTests.swift

@@ -509,6 +509,51 @@ class ConventionTests: XCTestCase {
         }
     }
 
+    func testTestTargetDependencies() throws {
+        var fs = InMemoryFileSystem()
+        try fs.createEmptyFiles("/Sources/Foo/source.swift",
+                                "/Sources/Bar/source.swift",
+                                "/Tests/FooTests/source.swift"
+                                )
+
+        let package = PackageDescription.Package(name: "pkg", targets: [Target(name: "FooTests", dependencies: ["Bar"])])
+        PackageBuilderTester(package, in: fs) { result in
+            result.checkModule("Foo") { moduleResult in
+                moduleResult.check(c99name: "Foo", type: .library, isTest: false)
+                moduleResult.checkSources(root: "/Sources/Foo", paths: "source.swift")
+            }
+
+            result.checkModule("Bar") { moduleResult in
+                moduleResult.check(c99name: "Bar", type: .library, isTest: false)
+                moduleResult.checkSources(root: "/Sources/Bar", paths: "source.swift")
+            }
+
+            result.checkModule("FooTests") { moduleResult in
+                moduleResult.check(c99name: "FooTests", type: .library, isTest: true)
+                moduleResult.checkSources(root: "/Tests/FooTests", paths: "source.swift")
+                moduleResult.check(dependencies: ["Bar"])
+                moduleResult.check(recursiveDependencies: ["Bar"])
+            }
+        }
+    }
+
+    func testInvalidTestTargets() throws {
+        // Test module in Sources/
+        var fs = InMemoryFileSystem()
+        try fs.createEmptyFiles("/Sources/FooTests/source.swift")
+        PackageBuilderTester("TestsInSources", in: fs) { result in
+            result.checkDiagnostic("the module at Sources/FooTests has an invalid name (\'FooTests\'): the name of a non-test module has a ‘Tests’ suffix fix: rename the module at ‘Sources/FooTests’ to not have a ‘Tests’ suffix")
+        }
+
+        // Normal module in Tests/
+        fs = InMemoryFileSystem()
+        try fs.createEmptyFiles("/Sources/main.swift",
+                                "/Tests/Foo/source.swift")
+        PackageBuilderTester("TestsInSources", in: fs) { result in
+            result.checkDiagnostic("the module at Tests/Foo has an invalid name (\'Foo\'): the name of a test module has no ‘Tests’ suffix fix: rename the module at ‘Tests/Foo’ to have a ‘Tests’ suffix")
+        }
+    }
+
     func testManifestTargetDeclErrors() throws {
         // Reference a target which doesn't exist.
         var fs = InMemoryFileSystem()

Utilities/bootstrap

@@ -306,6 +306,9 @@ def parse_manifest():
         return Target(name, deps)
     targets = list(map(convert, pattern.finditer(manifest_data)))
 
+    # Remove the targets which should be ignored in stage 1.
+    targets = [target for target in targets if target.name not in g_ignored_targets]
+
     # substitute strings for Target objects
     for target in targets:
         def convert(targetName):
@@ -337,6 +340,7 @@ def parse_manifest():
 # Hard-coded target definition.
 g_project_root = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
 g_source_root = os.path.join(g_project_root, "Sources")
+g_ignored_targets = ["swiftpm-xctest-helper", "BasicTests", "FunctionalTests", "PackageLoadingTests"]
 targets = parse_manifest()
 target_map = dict((t.name, t) for t in targets)