Skip to content

Commit

Permalink
Make Configuration.TestFilter a pure-data type (#358)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@stmontgomery
stmontgomery committed May 6, 2024
1 parent 0a1ba7b commit c105aa1
Show file tree
Hide file tree
Showing 3 changed files with 134 additions and 61 deletions.
3 changes: 1 addition & 2 deletions Sources/Testing/EntryPoints/EntryPoint.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
179 changes: 121 additions & 58 deletions Sources/Testing/Running/Configuration.TestFilter.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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.
///
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -318,6 +380,7 @@ extension Configuration.TestFilter {
}
}
}

/// Combine this test filter with another one.
///
/// - Parameters:
Expand All @@ -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

Expand Down
13 changes: 12 additions & 1 deletion Sources/Testing/Running/Runner.Plan.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand Down

0 comments on commit c105aa1

Please sign in to comment.