Implement SE-0129 to update the test naming conventions. Instead of

automatically and silently appending a `TestSuite` suffix, tests are
now expected to have a `Tests` suffix in the module name.  Right now
we make sure there are no tests under `Sources` and no non-tests
under `Tests`, but we expect to lift that restriction in the future
to allow test support libraries etc.

Author: abertelrud

Fixtures/Miscellaneous/Spaces Fixture/Tests/Test Module Name/Foo.swift renamed to Fixtures/Miscellaneous/Spaces Fixture/Tests/Moudule Tests/Foo.swift

@@ -1,9 +1,9 @@
 @import XCTest;
 
-@interface Objc: XCTestCase
+@interface ObjCTests: XCTestCase
 @end
 
-@implementation Objc
+@implementation ObjCTests
 
 - (void)testThisThing {
 }

Fixtures/Miscellaneous/SwiftPMXCTestHelper/Tests/ObjcTest/Objc.m renamed to Fixtures/Miscellaneous/SwiftPMXCTestHelper/Tests/ObjCTests/ObjCTests.m

@@ -114,7 +114,7 @@ final class InitPackage {
         guard exists(sources) == false else {
             return
         }
-        print("Creating Sources/")
+        print("Creating \(sources.relative(to: rootd).asString)/")
         try makeDirectories(sources)
 
         let sourceFileName = (mode == .executable) ? "main.swift" : "\(typeName).swift"
@@ -160,7 +160,7 @@ final class InitPackage {
         guard exists(tests) == false else {
             return
         }
-        print("Creating Tests/")
+        print("Creating \(tests.relative(to: rootd).asString)/")
         try makeDirectories(tests)
 
         // Only libraries are testable for now.
@@ -173,16 +173,16 @@ final class InitPackage {
     private func writeLinuxMain(testsPath: AbsolutePath) throws {
         try writePackageFile(testsPath.appending(component: "LinuxMain.swift")) { stream in
             stream <<< "import XCTest\n"
-            stream <<< "@testable import \(moduleName)TestSuite\n\n"
+            stream <<< "@testable import \(moduleName)Tests\n\n"
             stream <<< "XCTMain([\n"
             stream <<< "     testCase(\(typeName)Tests.allTests),\n"
             stream <<< "])\n"
         }
     }
 
     private func writeTestFileStubs(testsPath: AbsolutePath) throws {
-        let testModule = testsPath.appending(RelativePath(pkgname))
-        print("Creating Tests/\(pkgname)/")
+        let testModule = testsPath.appending(RelativePath(pkgname + Module.testModuleNameSuffix))
+        print("Creating \(testModule.relative(to: rootd).asString)/")
         try makeDirectories(testModule)
 
         try writePackageFile(testModule.appending(RelativePath("\(moduleName)Tests.swift"))) { stream in

Fixtures/Miscellaneous/SwiftPMXCTestHelper/Tests/SwiftPMXCTestHelper/SwiftPMXCTestHelperTests.swift renamed to Fixtures/Miscellaneous/SwiftPMXCTestHelper/Tests/SwiftPMXCTestHelperTests/SwiftPMXCTestHelperTests.swift

@@ -78,25 +78,45 @@ extension ModuleError.InvalidLayoutType: FixableError {
 }
 
 extension Module {
-    /// An error in the organization of an individual module.
+    
+    /// An error in the organization or configuration of an individual module.
     enum Error: Swift.Error {
+        
+        /// The module's name is invalid.
+        case invalidName(path: String, name: String, problem: ModuleNameProblem)
+        enum ModuleNameProblem {
+            /// Empty module name.
+            case emptyName
+            /// Test module doesn't have a "Tests" suffix.
+            case noTestSuffix
+            /// Non-test module does have a "Tests" suffix.
+            case hasTestSuffix
+        }
+        
+        /// The module contains no source code at all.
         case noSources(String)
+        
+        /// The module contains an invalid mix of languages (e.g. both Swift and C).
         case mixedSources(String)
     }
 }
 
 extension Module.Error: FixableError {
     var error: String {
         switch self {
-        case .noSources(let path):
+          case .invalidName(let path, let name, let problem):
+            return "the module at \(path) has an invalid name ('\(name)'): \(problem.error)"
+          case .noSources(let path):
             return "the module at \(path) does not contain any source files"
-        case .mixedSources(let path):
+          case .mixedSources(let path):
             return "the module at \(path) contains mixed language source files"
         }
     }
 
     var fix: String? {
         switch self {
+        case .invalidName(_, _, let problem):
+            return "rename the module (\(problem.fix))"
         case .noSources(_):
             return "either remove the module folder, or add a source file to the module"
         case .mixedSources(_):
@@ -105,6 +125,30 @@ extension Module.Error: FixableError {
     }
 }
 
+extension Module.Error.ModuleNameProblem : FixableError {
+    var error: String {
+        switch self {
+          case .emptyName:
+            return "the module name is empty"
+          case .noTestSuffix:
+            return "the name of a test module has no ‘Tests’ suffix"
+          case .hasTestSuffix:
+            return "the name of a non-test module has a ‘Tests’ suffix"
+        }
+    }
+    var fix: String? {
+        switch self {
+          case .emptyName:
+            return "give the module a name"
+          case .noTestSuffix:
+            return "add a ‘Tests’ suffix"
+          case .hasTestSuffix:
+            return "remove the ‘Tests’ suffix"
+        }
+    }
+}
+
+
 extension Product {
     /// An error in a product definition.
     enum Error: Swift.Error {
@@ -349,8 +393,26 @@ public struct PackageBuilder {
         return modules
     }
 
+    /// 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.
+    // FIXME: We will eventually be loosening this restriction to allow test-only libraries etc
+    private func validateModuleName(_ path: AbsolutePath, _ name: String, isTest: Bool) throws {
+        if name.isEmpty {
+            throw Module.Error.invalidName(path: path.asString, name: name, problem: .emptyName)
+        }
+        if name.hasSuffix(Module.testModuleNameSuffix) && !isTest {
+            throw Module.Error.invalidName(path: path.asString, name: name, problem: .hasTestSuffix)
+        }
+        if !name.hasSuffix(Module.testModuleNameSuffix) && isTest {
+            throw Module.Error.invalidName(path: path.asString, name: name, problem: .noTestSuffix)
+        }
+    }
+    
     /// Private function that constructs a single Module object for the moduel at `path`, having the name `name`.  If `isTest` is true, the module is constructed as a test module; if false, it is a regular module.
     private func createModule(_ path: AbsolutePath, name: String, isTest: Bool) throws -> Module {
+        
+        // Validate the module name.  This function will throw an error if it detects a problem.
+        try validateModuleName(path, name, isTest: isTest)
+        
         // Find all the files under the module path.
         let walked = try walk(path, fileSystem: fileSystem, recursing: shouldConsiderDirectory).map{ $0 }
 

Sources/Commands/init.swift

@@ -49,7 +49,8 @@ public class Module: ModuleProtocol {
     // FIXME: This should probably be rolled into the type.
     public let isTest: Bool
 
-    private let testModuleNameSuffix = "TestSuite"
+    /// Suffix that's expected for test modules.
+    public static let testModuleNameSuffix = "Tests"
 
     /// The "type" of module.
     public let type: ModuleType
@@ -58,8 +59,7 @@ public class Module: ModuleProtocol {
     public let sources: Sources
 
     public init(name: String, type: ModuleType, sources: Sources, isTest: Bool = false) throws {
-        // Append TestSuite to name if its a test module.
-        self.name = name + (isTest ? testModuleNameSuffix : "")
+        self.name = name
         self.type = type
         self.sources = sources
         self.dependencies = []
@@ -79,8 +79,8 @@ public class Module: ModuleProtocol {
         guard isTest else {
             fatalError("\(self.dynamicType) should be a test module to access basename.")
         }
-        precondition(name.hasSuffix(testModuleNameSuffix))
-        return name[name.startIndex..<name.index(name.endIndex, offsetBy: -testModuleNameSuffix.characters.count)]
+        precondition(name.hasSuffix(Module.testModuleNameSuffix))
+        return name[name.startIndex..<name.index(name.endIndex, offsetBy: -Module.testModuleNameSuffix.characters.count)]
     }
 }
 

Sources/PackageLoading/PackageBuilder.swift

@@ -26,11 +26,11 @@ class SwiftPMXCTestHelperTests: XCTestCase {
             let testCases = ["name": "All Tests", "tests": [["name" : "SwiftPMXCTestHelperTests.xctest",
                 "tests": [
                 [
-                "name": "Objc",
+                "name": "ObjCTests",
                 "tests": [["name": "test_example"], ["name": "testThisThing"]]
                 ],
                 [
-                "name": "SwiftPMXCTestHelperTestSuite.SwiftPMXCTestHelperTests1",
+                "name": "SwiftPMXCTestHelperTests.SwiftPMXCTestHelperTests1",
                 "tests": [["name": "test_Example2"], ["name": "testExample1"]]
                 ]
               ]]]

Sources/PackageModel/Module.swift

@@ -99,17 +99,17 @@ class ConventionTests: XCTestCase {
     func testCInTests() throws {
         var fs = InMemoryFileSystem()
         try fs.createEmptyFiles("/Sources/main.swift",
-                                "/Tests/MyPackage/abc.c")
+                                "/Tests/MyPackageTests/abc.c")
 
         PackageBuilderTester("MyPackage", in: fs) { result in
             result.checkModule("MyPackage") { moduleResult in
                 moduleResult.check(type: .executable, isTest: false)
                 moduleResult.checkSources(root: "/Sources", paths: "main.swift")
             }
 
-            result.checkModule("MyPackageTestSuite") { moduleResult in
+            result.checkModule("MyPackageTests") { moduleResult in
                 moduleResult.check(type: .library, isTest: true)
-                moduleResult.checkSources(root: "/Tests/MyPackage", paths: "abc.c")
+                moduleResult.checkSources(root: "/Tests/MyPackageTests", paths: "abc.c")
             }
 
           #if os(Linux)