SE-0129 test naming conventions

Fixtures/Miscellaneous/PackageWithInvalidTargets/Tests/LinuxMain.swift

@@ -1,5 +1,5 @@
 import XCTest
-@testable import MyAppTestSuite
+@testable import MyAppTests
 
 XCTMain([
      testCase(MyAppTests.allTests),

Fixtures/Miscellaneous/SwiftPMXCTestHelper/Tests/ObjcTest/Objc.m → Fixtures/Miscellaneous/SwiftPMXCTestHelper/Tests/ObjCTests/ObjCTests.m

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

Sources/Commands/init.swift

@@ -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

Sources/PackageLoading/PackageBuilder.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 {
@@ -341,8 +385,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/PackageModel/Module.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)]
     }
 }
 

Tests/BuildTests/Utilities.swift

@@ -0,0 +1 @@
+../FunctionalTests/Utilities.swift

Tests/CommandsTests/Utilities.swift

@@ -0,0 +1 @@
+../FunctionalTests/Utilities.swift

Tests/Functional/ModuleMapTests.swift → Tests/FunctionalTests/ModuleMapTests.swift

@@ -24,7 +24,7 @@ private let dylib = "so"
 class ModuleMapsTestCase: XCTestCase {
 
     private func fixture(name: String, cModuleName: String, rootpkg: String, body: (AbsolutePath, [String]) throws -> Void) {
-        FunctionalTestSuite.fixture(name: name) { prefix in
+        FunctionalTests.fixture(name: name) { prefix in
             let input = prefix.appending(components: cModuleName, "C", "foo.c")
             let outdir = prefix.appending(components: rootpkg, ".build", "debug")
             try makeDirectories(outdir)

Tests/Functional/SwiftPMXCTestHelperTests.swift → Tests/FunctionalTests/SwiftPMXCTestHelperTests.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"]]
                 ]
               ]]]

Tests/GetTests/Utilities.swift

@@ -0,0 +1 @@
+../FunctionalTests/Utilities.swift

Tests/LinuxMain.swift

@@ -10,32 +10,32 @@ See http://swift.org/CONTRIBUTORS.txt for Swift project authors
 
 import XCTest
 
-import BasicTestSuite
-import BuildTestSuite
-import CommandsTestSuite
-import FunctionalTestSuite
-import GetTestSuite
-import POSIXTestSuite
-import PackageDescriptionTestSuite
-import PackageGraphTestSuite
-import PackageLoadingTestSuite
-import PackageModelTestSuite
-import SourceControlTestSuite
-import UtilityTestSuite
-import XcodeprojTestSuite
+import BasicTests
+import BuildTests
+import CommandsTests
+import FunctionalTests
+import GetTests
+import POSIXTests
+import PackageDescriptionTests
+import PackageGraphTests
+import PackageLoadingTests
+import PackageModelTests
+import SourceControlTests
+import UtilityTests
+import XcodeprojTests
 
 var tests = [XCTestCaseEntry]()
-tests += BasicTestSuite.allTests()
-tests += BuildTestSuite.allTests()
-tests += CommandsTestSuite.allTests()
-tests += FunctionalTestSuite.allTests()
-tests += GetTestSuite.allTests()
-tests += POSIXTestSuite.allTests()
-tests += PackageDescriptionTestSuite.allTests()
-tests += PackageGraphTestSuite.allTests()
-tests += PackageLoadingTestSuite.allTests()
-tests += PackageModelTestSuite.allTests()
-tests += SourceControlTestSuite.allTests()
-tests += UtilityTestSuite.allTests()
-tests += XcodeprojTestSuite.allTests()
+tests += BasicTests.allTests()
+tests += BuildTests.allTests()
+tests += CommandsTests.allTests()
+tests += FunctionalTests.allTests()
+tests += GetTests.allTests()
+tests += POSIXTests.allTests()
+tests += PackageDescriptionTests.allTests()
+tests += PackageGraphTests.allTests()
+tests += PackageLoadingTests.allTests()
+tests += PackageModelTests.allTests()
+tests += SourceControlTests.allTests()
+tests += UtilityTests.allTests()
+tests += XcodeprojTests.allTests()
 XCTMain(tests)