[Commands] Allow skipping tests with a flag (swiftlang#2847)

* [Tests] Enable testSwiftTestParallel() and testSwiftTestFilter() on non-mac platforms by adding '--enable-test-discovery' flag

* [Commands] Allow skipping tests with a flag

Multiple '--skip' invocations can be used to skip multiple tests. Keep the environment
variable around till its users switch over.

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.
@@ -184,14 +187,6 @@ absolute search paths. SwiftPM will choose the first
 path which exists on disk. If none of the paths are present on disk, it will fall
 back to built-in computation.
 
-## Skipping SwiftPM tests
-
-SwiftPM has a hidden env variable `_SWIFTPM_SKIP_TESTS_LIST` that can be used
-to skip a list of tests. This value of the variable is either a file path that contains a
-newline separated list of tests to skip, or a colon-separated list of tests.
-
-This is only a development feature and should be considered _unsupported_.
-
 ## Making changes in TSC targets
 
 All targets with the prefix TSC define the interface for the tools support core. Those APIs might be used in other projects as well and need to be updated in this repository by copying their sources directories to the TSC repository. The repository can be found [here](https://github.com/apple/swift-tools-support-core).

IntegrationTests/Tests/IntegrationTests/BasicTests.swift

@@ -222,13 +222,13 @@ final class BasicTests: XCTestCase {
                         func testBaz() { }
                     }
                     """))
-            let testOutput = try sh(swiftTest, "--package-path", toolDir, "--filter", "MyTests.*").stderr
+            let testOutput = try sh(swiftTest, "--package-path", toolDir, "--filter", "MyTests.*", "--skip", "testBaz").stderr
 
             // Check the test log.
             XCTAssertContents(testOutput) { checker in
                 checker.check(.contains("Test Suite 'MyTests' started"))
                 checker.check(.contains("Test Suite 'MyTests' passed"))
-                checker.check(.contains("Executed 3 tests, with 0 failures"))
+                checker.check(.contains("Executed 2 tests, with 0 failures"))
             }
         }
     }

Sources/Commands/SwiftTestTool.swift

@@ -83,11 +83,14 @@ 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 testCaseSkip: TestCaseSpecifier {
+        // TODO: Remove this once the environment variable is no longer used.
+        testCaseSkipOverride() ?? _testCaseSkip
     }
 
-    var _testCaseSpecifier: TestCaseSpecifier = .none
+    var _testCaseSkip: TestCaseSpecifier = .none
 
     /// Path where the xUnit xml file should be generated.
     var xUnitOutput: AbsolutePath?
@@ -97,7 +100,7 @@ public class TestToolOptions: ToolOptions {
     public var testProduct: String?
 
     /// Returns the test case specifier if overridden in the env.
-    private func testCaseSpecifierOverride() -> TestCaseSpecifier? {
+    private func testCaseSkipOverride() -> TestCaseSpecifier? {
         guard let override = ProcessEnv.vars["_SWIFTPM_SKIP_TESTS_LIST"] else {
             return nil
         }
@@ -126,7 +129,8 @@ public class TestToolOptions: ToolOptions {
 /// 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 +172,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 {
@@ -208,7 +214,11 @@ public class SwiftTestTool: SwiftTool<TestToolOptions> {
 
             switch options.testCaseSpecifier {
             case .none:
-                xctestArg = nil
+                if case .skip = options.testCaseSkip {
+                    fallthrough
+                } else {
+                    xctestArg = nil
+                }
 
             case .regex, .specific, .skip:
                 // If old specifier `-s` option was used, emit deprecation notice.
@@ -218,7 +228,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 +264,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 +422,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 +432,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 +986,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 +1119,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")
     }
 }

Tests/FunctionalTests/MiscellaneousTests.swift

@@ -213,19 +213,21 @@ class MiscellaneousTestCase: XCTestCase {
     }
 
     func testSwiftTestParallel() throws {
-        // Running swift-test fixtures on linux is not yet possible.
-      #if os(macOS)
         fixture(name: "Miscellaneous/ParallelTestsPkg") { prefix in
           // First try normal serial testing.
           do {
-            _ = try SwiftPMProduct.SwiftTest.execute([], packagePath: prefix)
-          } catch SwiftPMProductError.executionFailure(_, _, let stderr) {
-            XCTAssertTrue(stderr.contains("Executed 2 tests"))
+            _ = try SwiftPMProduct.SwiftTest.execute(["--enable-test-discovery"], packagePath: prefix)
+          } catch SwiftPMProductError.executionFailure(_, let output, let stderr) {
+            #if os(macOS)
+              XCTAssertTrue(stderr.contains("Executed 2 tests"))
+            #else
+              XCTAssertTrue(output.contains("Executed 2 tests"))
+            #endif
           }
 
           do {
             // Run tests in parallel.
-            _ = try SwiftPMProduct.SwiftTest.execute(["--parallel"], packagePath: prefix)
+            _ = try SwiftPMProduct.SwiftTest.execute(["--parallel", "--enable-test-discovery"], packagePath: prefix)
           } catch SwiftPMProductError.executionFailure(_, let output, _) {
             XCTAssert(output.contains("testExample1"))
             XCTAssert(output.contains("testExample2"))
@@ -238,7 +240,7 @@ class MiscellaneousTestCase: XCTestCase {
           do {
             // Run tests in parallel with verbose output.
             _ = try SwiftPMProduct.SwiftTest.execute(
-                ["--parallel", "--verbose", "--xunit-output", xUnitOutput.pathString],
+                ["--parallel", "--verbose", "--xunit-output", xUnitOutput.pathString, "--enable-test-discovery"],
                 packagePath: prefix)
           } catch SwiftPMProductError.executionFailure(_, let output, _) {
             XCTAssert(output.contains("testExample1"))
@@ -253,25 +255,46 @@ class MiscellaneousTestCase: XCTestCase {
           let contents = try localFileSystem.readFileContents(xUnitOutput).description
           XCTAssertTrue(contents.contains("tests=\"3\" failures=\"1\""))
         }
-      #endif
     }
 
     func testSwiftTestFilter() throws {
-        #if os(macOS)
-            fixture(name: "Miscellaneous/ParallelTestsPkg") { prefix in
-                let (stdout, _) = try SwiftPMProduct.SwiftTest.execute(["--filter", ".*1", "-l"], packagePath: prefix)
-                XCTAssertMatch(stdout, .contains("testExample1"))
-                XCTAssertNoMatch(stdout, .contains("testExample2"))
-                XCTAssertNoMatch(stdout, .contains("testSureFailure"))
-            }
+        fixture(name: "Miscellaneous/ParallelTestsPkg") { prefix in
+            let (stdout, _) = try SwiftPMProduct.SwiftTest.execute(["--filter", ".*1", "-l", "--enable-test-discovery"], packagePath: prefix)
+            XCTAssertMatch(stdout, .contains("testExample1"))
+            XCTAssertNoMatch(stdout, .contains("testExample2"))
+            XCTAssertNoMatch(stdout, .contains("testSureFailure"))
+        }
 
-            fixture(name: "Miscellaneous/ParallelTestsPkg") { prefix in
-                let (stdout, _) = try SwiftPMProduct.SwiftTest.execute(["--filter", ".*1", "--filter", "testSureFailure", "-l"], packagePath: prefix)
-                XCTAssertMatch(stdout, .contains("testExample1"))
-                XCTAssertNoMatch(stdout, .contains("testExample2"))
-                XCTAssertMatch(stdout, .contains("testSureFailure"))
-            }
-        #endif
+        fixture(name: "Miscellaneous/ParallelTestsPkg") { prefix in
+            let (stdout, _) = try SwiftPMProduct.SwiftTest.execute(["--filter", "ParallelTestsTests", "--skip", ".*1", "--filter", "testSureFailure", "-l", "--enable-test-discovery"], packagePath: prefix)
+            XCTAssertNoMatch(stdout, .contains("testExample1"))
+            XCTAssertMatch(stdout, .contains("testExample2"))
+            XCTAssertMatch(stdout, .contains("testSureFailure"))
+        }
+    }
+
+    func testSwiftTestSkip() throws {
+        fixture(name: "Miscellaneous/ParallelTestsPkg") { prefix in
+            let (stdout, _) = try SwiftPMProduct.SwiftTest.execute(["--skip", "ParallelTestsTests", "-l", "--enable-test-discovery"], packagePath: prefix)
+            XCTAssertNoMatch(stdout, .contains("testExample1"))
+            XCTAssertNoMatch(stdout, .contains("testExample2"))
+            XCTAssertMatch(stdout, .contains("testSureFailure"))
+        }
+
+        fixture(name: "Miscellaneous/ParallelTestsPkg") { prefix in
+            let (stdout, _) = try SwiftPMProduct.SwiftTest.execute(["--filter", "ParallelTestsTests", "--skip", ".*2", "--filter", "TestsFailure", "--skip", "testSureFailure", "-l", "--enable-test-discovery"], packagePath: prefix)
+            XCTAssertMatch(stdout, .contains("testExample1"))
+            XCTAssertNoMatch(stdout, .contains("testExample2"))
+            XCTAssertNoMatch(stdout, .contains("testSureFailure"))
+        }
+
+        fixture(name: "Miscellaneous/ParallelTestsPkg") { prefix in
+            let (stdout, stderr) = try SwiftPMProduct.SwiftTest.execute(["--skip", "Tests", "--enable-test-discovery"], packagePath: prefix)
+            XCTAssertNoMatch(stdout, .contains("testExample1"))
+            XCTAssertNoMatch(stdout, .contains("testExample2"))
+            XCTAssertNoMatch(stdout, .contains("testSureFailure"))
+            XCTAssertMatch(stderr, .contains("No matching test cases were run"))
+        }
     }
 
     func testOverridingSwiftcArguments() throws {