Merge pull request #494 from ddunbar/SR-1135

[Commands] Fix `swift-test` to always build using the correct manifest.

Author: ddunbar

CHANGELOG.md

@@ -0,0 +1,11 @@
+Note: This is in reverse chronological order, so newer entries are added to the top.
+
+Swift 3.0
+---------
+
+* It is no longer necessary to run `swift build` before running `swift test` (it
+  will always regenerates the build manifest when necessary). In addition, it
+  now accepts (and requires) the same `-Xcc`, etc. options as are used with
+  `swift build`.
+
+* The `Package` initializer now requires the `name:` parameter.

Sources/Build/Configuration.swift

@@ -11,7 +11,7 @@
 public enum Configuration {
     case debug, release
 
-    var dirname: String {
+    public var dirname: String {
         switch self {
             case .debug: return "debug"
             case .release: return "release"

Sources/Commands/SwiftBuildTool.swift

@@ -14,7 +14,6 @@ import Get
 import PackageLoading
 import PackageModel
 import Utility
-import Xcodeproj
 
 #if HasCustomVersionString
 import VersionInfo

Sources/Commands/SwiftTestTool.swift

@@ -11,6 +11,7 @@ See http://swift.org/CONTRIBUTORS.txt for Swift project authors
 import class Foundation.ProcessInfo
 
 import Basic
+import Build
 import Utility
 
 #if HasCustomVersionString
@@ -21,8 +22,9 @@ import func POSIX.chdir
 import func POSIX.exit
 
 private enum TestError: Swift.Error {
-    case testsExecutableNotFound
     case invalidListTestJSONData
+    case multipleTestProducts
+    case testsExecutableNotFound
 }
 
 extension TestError: CustomStringConvertible {
@@ -32,6 +34,8 @@ extension TestError: CustomStringConvertible {
             return "no tests found to execute, create a module in your `Tests' directory"
         case .invalidListTestJSONData:
             return "Invalid list test JSON structure."
+        case .multipleTestProducts:
+            return "cannot test packages with multiple test products defined"
         }
     }
 }
@@ -75,11 +79,16 @@ private func ==(lhs: Mode, rhs: Mode) -> Bool {
     return lhs.description == rhs.description
 }
 
+// FIXME: Merge this with the `swift-build` arguments.
 private enum TestToolFlag: Argument {
+    case xcc(String)
+    case xld(String)
+    case xswiftc(String)
     case chdir(AbsolutePath)
     case buildPath(AbsolutePath)
     case colorMode(ColorWrap.Mode)
     case skipBuild
+    case ignoreDependencies
     case verbose(Int)
 
     init?(argument: String, pop: () -> String?) throws {
@@ -95,6 +104,12 @@ private enum TestToolFlag: Argument {
             self = .verbose(1)
         case "--skip-build":
             self = .skipBuild
+        case "-Xcc":
+            self = try .xcc(forcePop())
+        case "-Xlinker":
+            self = try .xld(forcePop())
+        case "-Xswiftc":
+            self = try .xswiftc(forcePop())
         case "--build-path":
             self = try .buildPath(AbsolutePath(forcePop(), relativeTo: currentWorkingDirectory))
         case "--color":
@@ -103,6 +118,8 @@ private enum TestToolFlag: Argument {
                 throw OptionParserError.invalidUsage("invalid color mode: \(rawValue)")
             }
             self = .colorMode(mode)
+        case "--ignore-dependencies":
+            self = .ignoreDependencies
         default:
             return nil
         }
@@ -113,6 +130,8 @@ private class TestToolOptions: Options {
     var verbosity: Int = 0
     var buildTests: Bool = true
     var colorMode: ColorWrap.Mode = .Auto
+    var flags = BuildFlags()
+    var ignoreDependencies: Bool = false
 }
 
 /// swift-test tool namespace
@@ -133,7 +152,7 @@ public struct SwiftTestTool: SwiftTool {
             if let dir = opts.chdir {
                 try chdir(dir.asString)
             }
-        
+
             switch mode {
             case .usage:
                 usage()
@@ -146,7 +165,7 @@ public struct SwiftTestTool: SwiftTool {
                 #endif
 
             case .listTests:
-                let testPath = try determineTestPath(opts: opts)
+                let testPath = try buildTestsIfNeeded(opts)
                 let testSuites = try getTestSuites(path: testPath)
                 // Print the tests.
                 for testSuite in testSuites {
@@ -158,9 +177,7 @@ public struct SwiftTestTool: SwiftTool {
                 }
 
             case .run(let specifier):
-                try buildTestsIfNeeded(opts)
-                let testPath = try determineTestPath(opts: opts)
-
+                let testPath = try buildTestsIfNeeded(opts)
                 let success = test(path: testPath, xctestArg: specifier)
                 exit(success ? 0 : 1)
             }
@@ -172,57 +189,40 @@ public struct SwiftTestTool: SwiftTool {
         }
     }
 
-    private let configuration = "debug"  //FIXME should swift-test support configuration option?
-
     /// Builds the "test" target if enabled in options.
-    private func buildTestsIfNeeded(_ opts: TestToolOptions) throws {
-        let yamlPath = opts.path.build.appending(RelativePath("\(configuration).yaml"))
+    ///
+    /// - Returns: The path to the test binary.
+    private func buildTestsIfNeeded(_ opts: TestToolOptions) throws -> AbsolutePath {
+        let graph = try loadPackage(at: opts.path.root, ignoreDependencies: opts.ignoreDependencies)
         if opts.buildTests {
-            try build(yamlPath: yamlPath, target: "test")
+            let yaml = try describe(opts.path.build, configuration, graph, flags: opts.flags, toolchain: UserToolchain())
+            try build(yamlPath: yaml, target: "test")
         }
-    }
-
-    /// Locates the XCTest bundle on OSX and XCTest executable on Linux.
-    /// First check if <build_path>/debug/<PackageName>Tests.xctest is present, otherwise
-    /// walk the build folder and look for folder/file ending with `.xctest`.
-    ///
-    /// - Parameters:
-    ///     - opts: Options object created by parsing the commandline arguments.
-    ///
-    /// - Throws: TestError
-    ///
-    /// - Returns: Path to XCTest bundle (OSX) or executable (Linux).
-    private func determineTestPath(opts: Options) throws -> AbsolutePath {
-
-        //FIXME better, ideally without parsing manifest since
-        // that makes us depend on the whole Manifest system
-
-        let packageName = opts.path.root.basename  //FIXME probably not true
-        let maybePath = opts.path.build.appending(RelativePath(configuration)).appending(RelativePath("\(packageName)Tests.xctest"))
-
-        let possibleTestPath: AbsolutePath
-
-        if exists(maybePath) {
-            possibleTestPath = maybePath
-        } else {
-            let possiblePaths = try walk(opts.path.build).filter {
-                $0.basename != "Package.xctest" &&   // this was our hardcoded name, may still exist if no clean
-                $0.suffix == ".xctest"
+                
+        // See the logic in `PackageLoading`'s `PackageExtensions.swift`.
+        //
+        // FIXME: We should also check if the package has any test
+        // modules, which isn't trivial (yet).
+        let testProducts = graph.products.filter{
+            if case .Test = $0.type {
+                return true
+            } else {
+                return false
             }
-
-            guard let path = possiblePaths.first else {
-                throw TestError.testsExecutableNotFound
-            }
-
-            possibleTestPath = path
         }
-
-        guard isValidTestPath(possibleTestPath) else {
+        if testProducts.count == 0 {
             throw TestError.testsExecutableNotFound
+        } else if testProducts.count > 1 {
+            throw TestError.multipleTestProducts
+        } else {
+            return opts.path.build.appending(RelativePath(configuration.dirname)).appending(component: testProducts[0].name + ".xctest")
         }
-        return possibleTestPath
     }
 
+    // FIXME: We need to support testing in other build configurations, but need
+    // to solve the testability problem first.
+    private let configuration = Build.Configuration.debug
+
     private func usage(_ print: (String) -> Void = { print($0) }) {
         //     .........10.........20.........30.........40.........50.........60.........70..
         print("OVERVIEW: Build and run tests")
@@ -238,6 +238,9 @@ public struct SwiftTestTool: SwiftTool {
         print("  --color <mode>         Specify color mode (auto|always|never) [default: auto]")
         print("  -v, --verbose          Increase verbosity of informational output")
         print("  --skip-build           Skip building the test target")
+        print("  -Xcc <flag>              Pass flag through to all C compiler invocations")
+        print("  -Xlinker <flag>          Pass flag through to all linker invocations")
+        print("  -Xswiftc <flag>          Pass flag through to all Swift compiler invocations")
         print("")
         print("NOTE: Use `swift package` to perform other functions on packages")
     }
@@ -252,12 +255,20 @@ public struct SwiftTestTool: SwiftTool {
                 opts.chdir = path
             case .verbose(let amount):
                 opts.verbosity += amount
+            case .xcc(let value):
+                opts.flags.cCompilerFlags.append(value)
+            case .xld(let value):
+                opts.flags.linkerFlags.append(value)
+            case .xswiftc(let value):
+                opts.flags.swiftCompilerFlags.append(value)
             case .buildPath(let buildPath):
                 opts.path.build = buildPath
             case .colorMode(let mode):
                 opts.colorMode = mode
             case .skipBuild:
                 opts.buildTests = false
+            case .ignoreDependencies:
+                opts.ignoreDependencies = true
             }
         }
 
@@ -272,8 +283,6 @@ public struct SwiftTestTool: SwiftTool {
     ///
     /// - Returns: True if execution exited with return code 0.
     private func test(path: AbsolutePath, xctestArg: String? = nil) -> Bool {
-        precondition(isValidTestPath(path))
-
         var args: [String] = []
       #if os(macOS)
         args = ["xcrun", "xctest"]
@@ -330,9 +339,6 @@ public struct SwiftTestTool: SwiftTool {
     ///
     /// - Returns: Array of TestSuite
     private func getTestSuites(path: AbsolutePath) throws -> [TestSuite] {
-        // Make sure tests are present.
-        guard isValidTestPath(path) else { throw TestError.testsExecutableNotFound }
-
         // Run the correct tool.
       #if os(macOS)
         let tempFile = try TemporaryFile()
@@ -349,14 +355,6 @@ public struct SwiftTestTool: SwiftTool {
     }
 }
 
-private func isValidTestPath(_ path: AbsolutePath) -> Bool {
-  #if os(macOS)
-    return isDirectory(path)  // ${foo}.xctest is dir on OSX
-  #else
-    return isFile(path)       // otherwise ${foo}.xctest is executable file
-  #endif
-}
-
 /// A struct to hold the XCTestSuite data.
 struct TestSuite {
 

Utilities/bootstrap

@@ -775,53 +775,58 @@ def main():
     make_fake_toolchain()
 
     # Build the package manager with itself.
-               
-    env_cmd = ["env", "SWIFT_EXEC=" + os.path.join(bindir, "swiftc"),
-                      "SWIFT_BUILD_PATH=" + build_path]
-    if args.sysroot:
-        env_cmd.append("SYSROOT=" + args.sysroot)
 
-    cmd = [bootstrapped_product]
+    # Compute the build flags, needed for swift-build and swift-test.
+    build_flags = []
+    
     # We need to embed an RPATH so swift-{build,test} can find the core
     # libraries.
     if platform.system() == 'Linux':
         embed_rpath = "$ORIGIN/../lib/swift/linux"
     else:
         embed_rpath = "@executable_path/../lib/swift/macosx"
-    cmd.extend(["-Xlinker", "-rpath", "-Xlinker", embed_rpath])
+    build_flags.extend(["-Xlinker", "-rpath", "-Xlinker", embed_rpath])
     if args.verbose:
-        cmd.append("-v")
+        build_flags.append("-v")
 
     # If appropriate, add flags for a custom version string:
     if args.version_str:
         incdir = os.path.join(sandbox_path, "inc")
-        cmd.extend(["-Xswiftc", "-I{}".format(incdir)])
-        cmd.extend(["-Xswiftc", "-DHasCustomVersionString"])
+        build_flags.extend(["-Xswiftc", "-I{}".format(incdir)])
+        build_flags.extend(["-Xswiftc", "-DHasCustomVersionString"])
 
     if args.foundation_path:
         core_foundation_path = os.path.join(
             args.foundation_path, "usr", "lib", "swift")
         # Tell the linker where to look for XCTest, but autolinking
         # knows to pass -lXCTest.
-        cmd.extend(["-Xlinker", "-L", "-Xlinker", args.foundation_path])
+        build_flags.extend(["-Xlinker", "-L", "-Xlinker", args.foundation_path])
         # Add an RPATH, so that the tests can be run directly.
-        cmd.extend(["-Xlinker", "-rpath", "-Xlinker", args.foundation_path])
-        cmd.extend(["-Xswiftc", "-I{}".format(args.foundation_path)])
-        cmd.extend(["-Xswiftc", "-I{}".format(core_foundation_path)])
+        build_flags.extend(["-Xlinker", "-rpath", "-Xlinker",
+                            args.foundation_path])
+        build_flags.extend(["-Xswiftc", "-I{}".format(args.foundation_path)])
+        build_flags.extend(["-Xswiftc", "-I{}".format(core_foundation_path)])
 
     if args.xctest_path:
         # Tell the linker where to look for XCTest, but autolinking
         # knows to pass -lXCTest.
-        cmd.extend(["-Xlinker", "-L", "-Xlinker", args.xctest_path])
+        build_flags.extend(["-Xlinker", "-L", "-Xlinker", args.xctest_path])
         # Add an RPATH, so that the tests can be run directly.
-        cmd.extend(["-Xlinker", "-rpath", "-Xlinker", args.xctest_path])
-        cmd.extend(["-Xswiftc", "-I{}".format(args.xctest_path)])
+        build_flags.extend(["-Xlinker", "-rpath", "-Xlinker", args.xctest_path])
+        build_flags.extend(["-Xswiftc", "-I{}".format(args.xctest_path)])
 
     if args.libdispatch_build_dir:
-        cmd.extend(["-Xswiftc", "-I{}".format(os.path.join(args.libdispatch_build_dir,
-                                             "src"))])
-        cmd.extend(["-Xswiftc", "-I{}".format(args.libdispatch_source_dir)])
-        cmd.extend(["-Xcc", "-fblocks"])
+        build_flags.extend(["-Xswiftc", "-I{}".format(
+            os.path.join(args.libdispatch_build_dir, "src"))])
+        build_flags.extend(["-Xswiftc", "-I{}".format(
+            args.libdispatch_source_dir)])
+        build_flags.extend(["-Xcc", "-fblocks"])
+    
+    env_cmd = ["env", "SWIFT_EXEC=" + os.path.join(bindir, "swiftc"),
+                      "SWIFT_BUILD_PATH=" + build_path]
+    if args.sysroot:
+        env_cmd.append("SYSROOT=" + args.sysroot)
+    cmd = env_cmd + [bootstrapped_product] + build_flags
 
     if args.release:
         cmd.extend(["--configuration", "release"])
@@ -832,8 +837,6 @@ def main():
     if not args.release:
         cmd.extend(["--build-tests"])
 
-    cmd = env_cmd + cmd
-
     note("building self-hosted 'swift-build': %s" % (
         ' '.join(cmd),))
     result = subprocess.call(cmd, cwd=g_project_root)
@@ -848,14 +851,13 @@ def main():
     swift_package_path = os.path.join(build_path, conf, "swift-package")
     swift_build_path = os.path.join(build_path, conf, "swift-build")
     swift_test_path = os.path.join(build_path, conf, "swift-test")
-    swiftpm_xctest_helper_path = os.path.join(build_path, conf, "swiftpm-xctest-helper")
+    swiftpm_xctest_helper_path = os.path.join(
+        build_path, conf, "swiftpm-xctest-helper")
 
     # If testing, run each of the test bundles.
     if "test" in build_actions:
         # Construct the test environment.
-        env_cmd = ["env", "SWIFT_EXEC=" + os.path.join(bindir, "swiftc"),
-                          "SWIFT_BUILD_PATH=" + build_path]
-        cmd = env_cmd + [swift_test_path]
+        cmd = env_cmd + [swift_test_path] + build_flags
         result = subprocess.call(cmd, cwd=g_project_root)
         if result != 0:
             error("tests failed with exit status %d" % (result,))