Make Configuration.TestFilter a pure-data type (#358)

This modifies the structure of `Configuration.TestFilter` so that it's a
pure data type and is easier to become serializable.

### Motivation:

For easier integration with tools it would be helpful for the stored
properties of `Configuration` to be serializable, or be trivially
convertible to/from serializable types at least.

`Configuration`'s nested `TestFilter` type currently includes a closure
for filtering tests and is therefore cannot be serialized in its current
form. This type doesn't really need to _store_ a closure, however; it
only needs to store information about how to perform filtering (e.g.
data about criteria to match against), and later that can be interpreted
when applying filtering.

### Modifications:

- Change the `Configuration.TestFilter.Kind` enum so that its cases only
store data, and no closures.
- Move the filtering logic derived from the pure data `Kind` type above
to a new `Configuration.TestFilter.Operation` enum.

### Checklist:

- [x] Code and documentation should follow the style of the [Style
Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md).
- [x] If public symbols are renamed or modified, DocC references should
be updated.

Resolves rdar://126627664

Author: stmontgomery

Sources/Testing/EntryPoints/EntryPoint.swift

@@ -298,8 +298,7 @@ public func configurationForEntryPoint(from args: __CommandLineArguments_v0) thr
       throw _EntryPointError.featureUnavailable("The `\(label)' option is not supported on this OS version.")
     }
     return try regexes.lazy
-      .map { try Regex($0) }
-      .map { Configuration.TestFilter(membership: membership, matching: $0) }
+      .map { try Configuration.TestFilter(membership: membership, matching: $0) }
       .reduce(into: .unfiltered) { $0.combine(with: $1, using: .or) }
   }
   filters.append(try testFilter(forRegularExpressions: args.filter, label: "--filter", membership: .including))

Sources/Testing/Running/Configuration.TestFilter.swift

@@ -40,16 +40,25 @@ extension Configuration {
       /// - Parameters:
       ///   - testIDs: The set of test IDs to predicate tests against.
       ///   - membership: How to interpret the result when predicating tests.
-      case precomputed(_ testIDs: Test.ID.Selection, membership: Membership)
+      case testIDs(_ testIDs: Set<Test.ID>, membership: Membership)
 
-      /// The test filter is an arbitrary predicate function.
+      /// The test filter contains a set of tags to predicate tests against.
       ///
       /// - Parameters:
-      ///   - predicate: The function to predicate tests against.
+      ///   - tags: The set of test tags to predicate tests against.
+      ///   - anyOf: Whether to require that tests have any (`true`) or all
+      ///     (`false`) of the specified tags.
       ///   - membership: How to interpret the result when predicating tests.
-      case function(_ predicate: @Sendable (borrowing Test) -> Bool, membership: Membership)
+      case tags(_ tags: Set<Tag>, anyOf: Bool, membership: Membership)
 
-      /// The test filter is a combination of other test filters.
+      /// The test filter contains a pattern to predicate test IDs against.
+      ///
+      /// - Parameters:
+      ///   - pattern: The pattern to predicate test IDs against.
+      ///   - membership: How to interpret the result when predicating tests.
+      case pattern(_ pattern: String, membership: Membership)
+
+      /// The test filter is a combination of other test filter kinds.
       ///
       /// - Parameters:
       ///   - lhs: The first test filter's kind.
@@ -59,7 +68,7 @@ extension Configuration {
       ///
       /// The result of a test filter with this kind is the combination of the
       /// results of its subfilters using `op`.
-      indirect case combined(_ lhs: Kind, _ rhs: Kind, _ op: CombinationOperator)
+      indirect case combination(_ lhs: Self, _ rhs: Self, _ op: CombinationOperator)
     }
 
     /// The kind of test filter.
@@ -100,8 +109,7 @@ extension Configuration.TestFilter {
   /// - Parameters:
   ///   - testIDs: A set of test IDs to be filtered.
   public init(including testIDs: some Collection<Test.ID>) {
-    let selection = Test.ID.Selection(testIDs: testIDs)
-    self.init(_kind: .precomputed(selection, membership: .including))
+    self.init(_kind: .testIDs(Set(testIDs), membership: .including))
   }
 
   /// Initialize this instance to filter tests to those _not_ specified by a set
@@ -110,35 +118,29 @@ extension Configuration.TestFilter {
   /// - Parameters:
   ///   - selection: A set of test IDs to be excluded.
   public init(excluding testIDs: some Collection<Test.ID>) {
-    let selection = Test.ID.Selection(testIDs: testIDs)
-    self.init(_kind: .precomputed(selection, membership: .excluding))
+    self.init(_kind: .testIDs(Set(testIDs), membership: .excluding))
   }
 
-  /// Initialize this instance from an arbitrary function.
+  /// Initialize this instance to represent a pattern expression matched against
+  /// a test's ID.
   ///
   /// - Parameters:
   ///   - membership: How to interpret the result when predicating tests.
-  ///   - predicate: The function to predicate tests against.
-  init(membership: Membership, matching predicate: @escaping @Sendable (borrowing Test) -> Bool) {
-    self.init(_kind: .function(predicate, membership: membership))
-  }
+  ///   - pattern: The pattern, expressed as a `Regex`-compatible regular
+  ///     expression, to match test IDs against.
+  @available(_regexAPI, *)
+  init(membership: Membership, matching pattern: String) throws {
+    // Validate the regular expression by attempting to initialize a `Regex`
+    // representing it, but do not preserve it. This type only represents
+    // the pattern in the abstract, and is not responsible for actually
+    // applying it to a test graph — that happens later during planning.
+    //
+    // Performing this validation here currently makes such errors easier to
+    // surface when using the SwiftPM entry point. But longer-term, we should
+    // make the planning phase throwing and propagate errors from there instead.
+    _ = try Regex(pattern)
 
-  /// Initialize this instance to operate based on a set of tags.
-  ///
-  /// - Parameters:
-  ///   - tags: The set of tags to either include or exclude.
-  ///   - anyOf: Whether tests must have _any_ of the tags in `tags` (as opposed
-  ///     to all of them.)
-  ///   - membership: How to interpret the result when predicating tests.
-  init(tags: some Collection<Tag>, anyOf: Bool, membership: Membership) {
-    let tags = Set(tags)
-    self.init(membership: membership) { test in
-      if anyOf {
-        !test.tags.isDisjoint(with: tags) // .intersects()
-      } else {
-        test.tags.isSuperset(of: tags)
-      }
-    }
+    self.init(_kind: .pattern(pattern, membership: membership))
   }
 
   /// Initialize this instance to include tests with a given set of tags.
@@ -148,7 +150,7 @@ extension Configuration.TestFilter {
   ///
   /// Matching tests have had _any_ of the tags in `tags` added to them.
   public init(includingAnyOf tags: some Collection<Tag>) {
-    self.init(tags: tags, anyOf: true, membership: .including)
+    self.init(_kind: .tags(Set(tags), anyOf: true, membership: .including))
   }
 
   /// Initialize this instance to exclude tests with a given set of tags.
@@ -158,7 +160,7 @@ extension Configuration.TestFilter {
   ///
   /// Matching tests have had _any_ of the tags in `tags` added to them.
   public init(excludingAnyOf tags: some Collection<Tag>) {
-    self.init(tags: tags, anyOf: true, membership: .excluding)
+    self.init(_kind: .tags(Set(tags), anyOf: true, membership: .excluding))
   }
 
   /// Initialize this instance to include tests with a given set of tags.
@@ -168,7 +170,7 @@ extension Configuration.TestFilter {
   ///
   /// Matching tests have had _all_ of the tags in `tags` added to them.
   public init(includingAllOf tags: some Collection<Tag>) {
-    self.init(tags: tags, anyOf: false, membership: .including)
+    self.init(_kind: .tags(Set(tags), anyOf: false, membership: .including))
   }
 
   /// Initialize this instance to exclude tests with a given set of tags.
@@ -178,32 +180,92 @@ extension Configuration.TestFilter {
   ///
   /// Matching tests have had _all_ of the tags in `tags` added to them.
   public init(excludingAllOf tags: some Collection<Tag>) {
-    self.init(tags: tags, anyOf: false, membership: .excluding)
+    self.init(_kind: .tags(Set(tags), anyOf: false, membership: .excluding))
   }
+}
 
-  /// Initialize this instance to represent a regular expression matched against
-  /// a test's ID.
-  ///
-  /// - Parameters:
-  ///   - membership: How to interpret the result when predicating tests.
-  ///   - regex: The regular expression to predicate test IDs against.
+// MARK: - Operations
+
+extension Configuration.TestFilter {
+  /// An enumeration which represents filtering logic to be applied to a test
+  /// graph.
+  fileprivate enum Operation: Sendable {
+    /// A filter operation which has no effect.
+    ///
+    /// All tests are allowed when this operation is applied.
+    case unfiltered
+
+    /// A filter operation which accepts tests included in a precomputed
+    /// selection of test IDs.
+    ///
+    /// - Parameters:
+    ///   - testIDs: The set of test IDs to predicate tests against.
+    ///   - membership: How to interpret the result when predicating tests.
+    case precomputed(_ testIDs: Test.ID.Selection, membership: Membership)
+
+    /// A filter operation which accepts tests which satisfy an arbitrary
+    /// predicate function.
+    ///
+    /// - Parameters:
+    ///   - predicate: The function to predicate tests against.
+    ///   - membership: How to interpret the result when predicating tests.
+    case function(_ predicate: @Sendable (borrowing Test) -> Bool, membership: Membership)
+
+    /// A filter operation which is a combination of other operations.
+    ///
+    /// - Parameters:
+    ///   - lhs: The first test filter operation.
+    ///   - rhs: The second test filter operation.
+    ///   - op: The operator to apply when combining the results of the two
+    ///     filter operations.
+    ///
+    /// The result of applying this filter operation is the combination of
+    /// applying the results of its sub-operations using `op`.
+    indirect case combination(_ lhs: Self, _ rhs: Self, _ op: CombinationOperator)
+  }
+}
+
+extension Configuration.TestFilter.Kind {
+  /// An operation which implements the filtering logic for this test filter
+  /// kind.
   ///
-  /// The caller is responsible for ensuring that `regex` is safe to send across
-  /// isolation boundaries. Regular expressions parsed from strings are
-  /// generally sendable.
-  @available(_regexAPI, *)
-  init(membership: Membership, matching regex: Regex<AnyRegexOutput>) {
-    let regex = UncheckedSendable(rawValue: regex)
-    self.init(membership: membership) { test in
-      let id = String(describing: test.id)
-      return id.contains(regex.rawValue)
+  /// - Throws: Any error encountered while generating an operation for this
+  ///   test filter kind. One example is the creation of a `Regex` from a
+  ///   `.pattern` kind: if the pattern is not a valid regular expression, an
+  ///   error will be thrown.
+  var operation: Configuration.TestFilter.Operation {
+    get throws {
+      switch self {
+      case .unfiltered:
+        return .unfiltered
+      case let .testIDs(testIDs, membership):
+        return .precomputed(Test.ID.Selection(testIDs: testIDs), membership: membership)
+      case let .tags(tags, anyOf, membership):
+        return .function({ test in
+          if anyOf {
+            !test.tags.isDisjoint(with: tags) // .intersects()
+          } else {
+            test.tags.isSuperset(of: tags)
+          }
+        }, membership: membership)
+      case let .pattern(pattern, membership):
+        guard #available(_regexAPI, *) else {
+          throw SystemError(description: "Filtering by regular expression matching is unavailable")
+        }
+
+        let regex = UncheckedSendable(rawValue: try Regex(pattern))
+        return .function({ test in
+          let id = String(describing: test.id)
+          return id.contains(regex.rawValue)
+        }, membership: membership)
+      case let .combination(lhs, rhs, op):
+        return try .combination(lhs.operation, rhs.operation, op)
+      }
     }
   }
 }
 
-// MARK: - Operations
-
-extension Configuration.TestFilter.Kind {
+extension Configuration.TestFilter.Operation {
   /// Apply this test filter to a test graph and remove tests that should not be
   /// included.
   ///
@@ -246,7 +308,7 @@ extension Configuration.TestFilter.Kind {
         .map(\.id)
       let selection = Test.ID.Selection(testIDs: testIDs)
       return Self.precomputed(selection, membership: membership).apply(to: testGraph)
-    case let .combined(lhs, rhs, op):
+    case let .combination(lhs, rhs, op):
       return zip(
         lhs.apply(to: testGraph),
         rhs.apply(to: testGraph)
@@ -265,13 +327,13 @@ extension Configuration.TestFilter {
   ///   - testGraph: The test graph to filter.
   ///
   /// - Returns: A copy of `testGraph` with filtered tests replaced with `nil`.
-  func apply(to testGraph: Graph<String, Test?>) -> Graph<String, Test?> {
-    var result = _kind.apply(to: testGraph)
+  func apply(to testGraph: Graph<String, Test?>) throws -> Graph<String, Test?> {
+    var result = try _kind.operation.apply(to: testGraph)
 
     // After performing the test function, run through one more time and remove
     // hidden tests. (Note that this property's value is not recursively set on
     // combined test filters. It is only consulted on the outermost call to
-    // apply(to:), not in _apply(to:).
+    // apply(to:), not in _apply(to:).)
     if !includeHiddenTests {
       result = result.mapValues { _, test in
         (test?.isHidden == true) ? nil : test
@@ -318,6 +380,7 @@ extension Configuration.TestFilter {
       }
     }
   }
+
   /// Combine this test filter with another one.
   ///
   /// - Parameters:
@@ -336,7 +399,7 @@ extension Configuration.TestFilter {
     case (_, .unfiltered):
       self
     default:
-      Self(_kind: .combined(_kind, other._kind, op))
+      Self(_kind: .combination(_kind, other._kind, op))
     }
     result.includeHiddenTests = includeHiddenTests
 

Sources/Testing/Running/Runner.Plan.swift

@@ -190,7 +190,18 @@ extension Runner.Plan {
     // configuration. The action graph is not modified here: actions that lose
     // their corresponding tests are effectively filtered out by the call to
     // zip() near the end of the function.
-    testGraph = configuration.testFilter.apply(to: testGraph)
+    do {
+      testGraph = try configuration.testFilter.apply(to: testGraph)
+    } catch {
+      // FIXME: Handle this more gracefully, either by propagating the error
+      // (which will ultimately require `Runner.init(...)` to be throwing:
+      // rdar://126631222) or by recording a single `Issue` representing the
+      // planning failure.
+      //
+      // For now, ignore the error and include all tests. As of this writing,
+      // the only scenario where this will throw is when using regex filtering,
+      // and that is already guarded earlier in the SwiftPM entry point.
+    }
 
     // For each test value, determine the appropriate action for it.
     //