Allow skipping tests with a flag, instead of the environment variable

Author: finagolfin

Documentation/Development.md

@@ -47,6 +47,9 @@ $ swift test --parallel
 
 # Run a single test.
 $ swift test --filter PackageGraphTests.DependencyResolverTests/testBasics
+
+# Run tests for the test targets BuildTests and WorkspaceTests, but skip some test cases.
+$ swift test --filter BuildTests --skip BuildPlanTests --filter WorkspaceTests --skip InitTests
 ```
 
 Note: PackageDescription v4 is not available when developing using this method.

Sources/Commands/SwiftTestTool.swift

@@ -83,50 +83,25 @@ public class TestToolOptions: ToolOptions {
     /// If the path of the exported code coverage JSON should be printed.
     var shouldPrintCodeCovPath = false
 
-    var testCaseSpecifier: TestCaseSpecifier {
-        testCaseSpecifierOverride() ?? _testCaseSpecifier
-    }
+    var testCaseSpecifier: TestCaseSpecifier = .none
 
-    var _testCaseSpecifier: TestCaseSpecifier = .none
+    var testCaseSkip: TestCaseSpecifier = .none
 
     /// Path where the xUnit xml file should be generated.
     var xUnitOutput: AbsolutePath?
 
     /// The test product to use. This is useful when there are multiple test products
     /// to choose from (usually in multiroot packages).
     public var testProduct: String?
-
-    /// Returns the test case specifier if overridden in the env.
-    private func testCaseSpecifierOverride() -> TestCaseSpecifier? {
-        guard let override = ProcessEnv.vars["_SWIFTPM_SKIP_TESTS_LIST"] else {
-            return nil
-        }
-
-        do {
-            let skipTests: [String.SubSequence]
-            // Read from the file if it exists.
-            if let path = try? AbsolutePath(validating: override), localFileSystem.exists(path) {
-                let contents = try localFileSystem.readFileContents(path).cString
-                skipTests = contents.split(separator: "\n", omittingEmptySubsequences: true)
-            } else {
-                // Otherwise, read the env variable.
-                skipTests = override.split(separator: ":", omittingEmptySubsequences: true)
-            }
-
-            return .skip(skipTests.map(String.init))
-        } catch {
-            // FIXME: We should surface errors from here.
-        }
-        return nil
-    }
 }
 
 /// Tests filtering specifier
 ///
 /// This is used to filter tests to run
 ///   .none     => No filtering
 ///   .specific => Specify test with fully quantified name
-///   .regex    => RegEx patterns
+///   .regex    => RegEx patterns for tests to run
+///   .skip     => RegEx patterns for tests to skip
 public enum TestCaseSpecifier {
     case none
     case specific(String)
@@ -168,7 +143,9 @@ public class SwiftTestTool: SwiftTool<TestToolOptions> {
         case .listTests:
             let testProducts = try buildTestsIfNeeded()
             let testSuites = try getTestSuites(in: testProducts)
-            let tests = testSuites.filteredTests(specifier: options.testCaseSpecifier)
+            let tests = testSuites
+                .filteredTests(specifier: options.testCaseSpecifier)
+                .skippedTests(specifier: options.testCaseSkip)
 
             // Print the tests.
             for test in tests {
@@ -218,7 +195,9 @@ public class SwiftTestTool: SwiftTool<TestToolOptions> {
 
                 // Find the tests we need to run.
                 let testSuites = try getTestSuites(in: testProducts)
-                let tests = testSuites.filteredTests(specifier: options.testCaseSpecifier)
+                let tests = testSuites
+                    .filteredTests(specifier: options.testCaseSpecifier)
+                    .skippedTests(specifier: options.testCaseSkip)
 
                 // If there were no matches, emit a warning.
                 if tests.isEmpty {
@@ -252,7 +231,9 @@ public class SwiftTestTool: SwiftTool<TestToolOptions> {
             let toolchain = try getToolchain()
             let testProducts = try buildTestsIfNeeded()
             let testSuites = try getTestSuites(in: testProducts)
-            let tests = testSuites.filteredTests(specifier: options.testCaseSpecifier)
+            let tests = testSuites
+                .filteredTests(specifier: options.testCaseSpecifier)
+                .skippedTests(specifier: options.testCaseSkip)
             let buildParameters = try self.buildParameters()
 
             // If there were no matches, emit a warning and exit.
@@ -408,7 +389,7 @@ public class SwiftTestTool: SwiftTool<TestToolOptions> {
 
         binder.bind(
             option: parser.add(option: "--specifier", shortName: "-s", kind: String.self),
-            to: { $0._testCaseSpecifier = .specific($1) })
+            to: { $0.testCaseSpecifier = .specific($1) })
 
         binder.bind(
             option: parser.add(option: "--xunit-output", kind: PathArgument.self),
@@ -418,7 +399,12 @@ public class SwiftTestTool: SwiftTool<TestToolOptions> {
             option: parser.add(option: "--filter", kind: [String].self,
                 usage: "Run test cases matching regular expression, Format: <test-target>.<test-case> or " +
                     "<test-target>.<test-case>/<test>"),
-            to: { $0._testCaseSpecifier = .regex($1) })
+            to: { $0.testCaseSpecifier = .regex($1) })
+
+        binder.bind(
+            option: parser.add(option: "--skip", kind: [String].self,
+                usage: "Skip test cases matching regular expression, Example: --skip PerformanceTests"),
+            to: { $0.testCaseSkip = .skip($1) })
 
         binder.bind(
             option: parser.add(option: "--enable-code-coverage", kind: Bool.self,
@@ -967,14 +953,28 @@ fileprivate extension Dictionary where Key == AbsolutePath, Value == [TestSuite]
             })
         case .specific(let name):
             return allTests.filter{ $0.specifier == name }
+        case .skip:
+            fatalError("Tests to skip should never have been passed here.")
+        }
+    }
+}
+
+fileprivate extension Array where Element == UnitTest {
+    /// Skip tests matching the provided specifier
+    func skippedTests(specifier: TestCaseSpecifier) -> [UnitTest] {
+        switch specifier {
+        case .none:
+            return self
         case .skip(let skippedTests):
-            var result = allTests
+            var result = self
             for skippedTest in skippedTests {
                 result = result.filter{
                     $0.specifier.range(of: skippedTest, options: .regularExpression) == nil
                 }
             }
             return result
+        case .regex, .specific:
+            fatalError("Tests to filter should never have been passed here.")
         }
     }
 }
@@ -1086,6 +1086,6 @@ final class XUnitGenerator {
 
 private extension Diagnostic.Message {
     static var noMatchingTests: Diagnostic.Message {
-        .warning("'--filter' predicate did not match any test case")
+        .warning("No matching test cases were run")
     }
 }