Skip to content

Commit bfd57a6

Browse files
authored
Store a collection of patterns to match against in Configuration.TestFilter instead of chaining multiple pattern-matcher filters (#494)
This fixes a crash which can occur if there are many filter or skip arguments passed to the testing library's entry point. In this situation, before this fix, one `Configuration.TestFilter` was created for each filter or skip argument and they were chained together to form a tree using `combining(with:using:)`. Then, they were applied to a test graph recursively. If this tree was deep due to having many filter or skip arguments chained together, the recursion could exceed the stack and cause a crash. The fix is to change `Configuration.TestFilter.Kind.pattern` so that it can store multiple patterns to match against instead of just one. This makes it more aligned with the other cases in its enum, which also store collections, and prevents the crash since it typically results in much shallower filter trees. ### Result: Crash is resolved, validated by a new unit test. ### 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.
1 parent bd9249e commit bfd57a6

File tree

3 files changed

+44
-26
lines changed

3 files changed

+44
-26
lines changed

Sources/Testing/EntryPoints/EntryPoint.swift

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -429,9 +429,7 @@ public func configurationForEntryPoint(from args: __CommandLineArguments_v0) thr
429429
guard #available(_regexAPI, *) else {
430430
throw _EntryPointError.featureUnavailable("The `\(label)' option is not supported on this OS version.")
431431
}
432-
return try regexes.lazy
433-
.map { try Configuration.TestFilter(membership: membership, matching: $0) }
434-
.reduce(into: .unfiltered) { $0.combine(with: $1, using: .or) }
432+
return try Configuration.TestFilter(membership: membership, matchingAnyOf: regexes)
435433
}
436434
filters.append(try testFilter(forRegularExpressions: args.filter, label: "--filter", membership: .including))
437435
filters.append(try testFilter(forRegularExpressions: args.skip, label: "--skip", membership: .excluding))

Sources/Testing/Running/Configuration.TestFilter.swift

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,9 @@ extension Configuration {
5454
/// The test filter contains a pattern to predicate test IDs against.
5555
///
5656
/// - Parameters:
57-
/// - pattern: The pattern to predicate test IDs against.
57+
/// - patterns: The patterns to predicate test IDs against.
5858
/// - membership: How to interpret the result when predicating tests.
59-
case pattern(_ pattern: String, membership: Membership)
59+
case patterns(_ patterns: [String], membership: Membership)
6060

6161
/// The test filter is a combination of other test filter kinds.
6262
///
@@ -126,21 +126,23 @@ extension Configuration.TestFilter {
126126
///
127127
/// - Parameters:
128128
/// - membership: How to interpret the result when predicating tests.
129-
/// - pattern: The pattern, expressed as a `Regex`-compatible regular
130-
/// expression, to match test IDs against.
129+
/// - patterns: The patterns, expressed as a `Regex`-compatible regular
130+
/// expressions, to match test IDs against.
131131
@available(_regexAPI, *)
132-
init(membership: Membership, matching pattern: String) throws {
133-
// Validate the regular expression by attempting to initialize a `Regex`
132+
init(membership: Membership, matchingAnyOf patterns: some Sequence<String>) throws {
133+
// Validate each regular expression by attempting to initialize a `Regex`
134134
// representing it, but do not preserve it. This type only represents
135135
// the pattern in the abstract, and is not responsible for actually
136136
// applying it to a test graph — that happens later during planning.
137137
//
138138
// Performing this validation here currently makes such errors easier to
139139
// surface when using the SwiftPM entry point. But longer-term, we should
140140
// make the planning phase throwing and propagate errors from there instead.
141-
_ = try Regex(pattern)
141+
for pattern in patterns {
142+
_ = try Regex(pattern)
143+
}
142144

143-
self.init(_kind: .pattern(pattern, membership: membership))
145+
self.init(_kind: .patterns(Array(patterns), membership: membership))
144146
}
145147

146148
/// Initialize this instance to include tests with a given set of tags.
@@ -248,15 +250,15 @@ extension Configuration.TestFilter.Kind {
248250
test.tags.isSuperset(of: tags)
249251
}
250252
}, membership: membership)
251-
case let .pattern(pattern, membership):
253+
case let .patterns(patterns, membership):
252254
guard #available(_regexAPI, *) else {
253255
throw SystemError(description: "Filtering by regular expression matching is unavailable")
254256
}
255257

256-
nonisolated(unsafe) let regex = try Regex(pattern)
258+
nonisolated(unsafe) let regexes = try patterns.map(Regex.init)
257259
return .function({ test in
258260
let id = String(describing: test.id)
259-
return id.contains(regex)
261+
return regexes.contains { id.contains($0) }
260262
}, membership: membership)
261263
case let .combination(lhs, rhs, op):
262264
return try .combination(lhs.operation, rhs.operation, op)

Tests/TestingTests/ABIEntryPointTests.swift

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,35 @@ private import _TestingInternals
2020
@Suite("ABI entry point tests")
2121
struct ABIEntryPointTests {
2222
@Test func v0() async throws {
23+
var arguments = __CommandLineArguments_v0()
24+
arguments.filter = ["NonExistentTestThatMatchesNothingHopefully"]
25+
arguments.experimentalEventStreamVersion = 0
26+
arguments.verbosity = .min
27+
28+
let result = try await _invokeEntryPointV0(passing: arguments) { recordJSON in
29+
let record = try! JSON.decode(ABIv0.Record.self, from: recordJSON)
30+
_ = record.version
31+
}
32+
33+
#expect(result == EXIT_SUCCESS)
34+
}
35+
36+
@Test("v0 entry point with a large number of filter arguments")
37+
func v0_manyFilters() async throws {
38+
var arguments = __CommandLineArguments_v0()
39+
arguments.filter = (1...100).map { "NonExistentTestThatMatchesNothingHopefully_\($0)" }
40+
arguments.experimentalEventStreamVersion = 0
41+
arguments.verbosity = .min
42+
43+
let result = try await _invokeEntryPointV0(passing: arguments)
44+
45+
#expect(result == EXIT_SUCCESS)
46+
}
47+
48+
private func _invokeEntryPointV0(
49+
passing arguments: __CommandLineArguments_v0,
50+
recordHandler: @escaping @Sendable (_ recordJSON: UnsafeRawBufferPointer) -> Void = { _ in }
51+
) async throws -> CInt {
2352
#if !os(Linux) && !SWT_NO_DYNAMIC_LINKING
2453
// Get the ABI entry point by dynamically looking it up at runtime.
2554
//
@@ -38,11 +67,6 @@ struct ABIEntryPointTests {
3867
abiEntryPoint.deallocate()
3968
}
4069

41-
// Construct arguments and convert them to JSON.
42-
var arguments = __CommandLineArguments_v0()
43-
arguments.filter = ["NonExistentTestThatMatchesNothingHopefully"]
44-
arguments.experimentalEventStreamVersion = 0
45-
arguments.verbosity = .min
4670
let argumentsJSON = try JSON.withEncoding(of: arguments) { argumentsJSON in
4771
let result = UnsafeMutableRawBufferPointer.allocate(byteCount: argumentsJSON.count, alignment: 1)
4872
result.copyMemory(from: argumentsJSON)
@@ -53,13 +77,7 @@ struct ABIEntryPointTests {
5377
}
5478

5579
// Call the entry point function.
56-
let result = try await abiEntryPoint.pointee(.init(argumentsJSON)) { recordJSON in
57-
let record = try! JSON.decode(ABIv0.Record.self, from: recordJSON)
58-
_ = record.version
59-
}
60-
61-
// Validate expectations.
62-
#expect(result == EXIT_SUCCESS)
80+
return try await abiEntryPoint.pointee(.init(argumentsJSON), recordHandler)
6381
}
6482
}
6583
#endif

0 commit comments

Comments
 (0)