Skip to content

Commit 3c0a692

Browse files
authored
Avoid reassigning task-local runtime state in a Runner's event handler unless it's nested (#339)
This is a mitigation for a bug in which an expectation inside a `withTaskGroup` closure can cause a crash if it fails. ### Motivation: It can sometimes be useful to perform expectations (e.g. `#expect`) within the body of the closure passed to `withTaskGroup` or similar APIs. However, doing so currently causes a crash if the expectation fails, illustrated by this small example: ```swift @test func example() async { await withTaskGroup(of: Void.self) { _ in #expect("abc" == "123") // 💥 } } ``` ### Modifications: This change avoids wrapping the event handler of a runner's configuration with one that reassigns a TaskLocal (potentially triggering the crash) unless that runner is running during the run operation of another runner. In other words, only mutate the TaskLocal for nested runners. Nesting, or running one runner in the context of another, is something which is only done in practice when performing tests of the testing library itself. Ordinary usage of the testing library only has one “outer“ runner. ### Result: The above example code no longer crashes, verified by the 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. Resolves rdar://126132392
1 parent 51f97be commit 3c0a692

File tree

3 files changed

+37
-9
lines changed

3 files changed

+37
-9
lines changed

Sources/Testing/Running/Runner.RuntimeState.swift

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,10 @@ extension Runner {
2525
/// The test case that is running on the current task, if any.
2626
var testCase: Test.Case?
2727

28-
/// The runtime state related to the runner running on the current task.
28+
/// The runtime state related to the runner running on the current task,
29+
/// if any.
2930
@TaskLocal
30-
static var current = Self()
31+
static var current: Self?
3132
}
3233
}
3334

@@ -42,7 +43,10 @@ extension Runner {
4243
/// In practice, the primary scenario where this is important is when running
4344
/// the testing library's own tests.
4445
mutating func configureEventHandlerRuntimeState() {
45-
let existingRuntimeState = RuntimeState.current
46+
guard let existingRuntimeState = RuntimeState.current else {
47+
return
48+
}
49+
4650
configuration.eventHandler = { [eventHandler = configuration.eventHandler] event, context in
4751
RuntimeState.$current.withValue(existingRuntimeState) {
4852
eventHandler(event, context)
@@ -56,7 +60,7 @@ extension Runner {
5660
extension Configuration {
5761
/// The configuration for the current task, if any.
5862
public static var current: Self? {
59-
Runner.RuntimeState.current.configuration
63+
Runner.RuntimeState.current?.configuration
6064
}
6165

6266
/// Call a function while the value of ``Configuration/current`` is set.
@@ -74,7 +78,7 @@ extension Configuration {
7478
configuration._removeFromAll(identifiedBy: id)
7579
}
7680

77-
var runtimeState = Runner.RuntimeState.current
81+
var runtimeState = Runner.RuntimeState.current ?? .init()
7882
runtimeState.configuration = configuration
7983
return try await Runner.RuntimeState.$current.withValue(runtimeState, operation: body)
8084
}
@@ -144,7 +148,7 @@ extension Test {
144148
/// or [`DispatchQueue.async(execute:)`](https://developer.apple.com/documentation/dispatch/dispatchqueue/2016103-async)),
145149
/// the value of this property may be `nil`.
146150
public static var current: Self? {
147-
Runner.RuntimeState.current.test
151+
Runner.RuntimeState.current?.test
148152
}
149153

150154
/// Call a function while the value of ``Test/current`` is set.
@@ -157,7 +161,7 @@ extension Test {
157161
///
158162
/// - Throws: Whatever is thrown by `body`.
159163
static func withCurrent<R>(_ test: Self, perform body: () async throws -> R) async rethrows -> R {
160-
var runtimeState = Runner.RuntimeState.current
164+
var runtimeState = Runner.RuntimeState.current ?? .init()
161165
runtimeState.test = test
162166
return try await Runner.RuntimeState.$current.withValue(runtimeState, operation: body)
163167
}
@@ -177,7 +181,7 @@ extension Test.Case {
177181
/// or [`DispatchQueue.async(execute:)`](https://developer.apple.com/documentation/dispatch/dispatchqueue/2016103-async)),
178182
/// the value of this property may be `nil`.
179183
public static var current: Self? {
180-
Runner.RuntimeState.current.testCase
184+
Runner.RuntimeState.current?.testCase
181185
}
182186

183187
/// Call a function while the value of ``Test/Case/current`` is set.
@@ -190,7 +194,7 @@ extension Test.Case {
190194
///
191195
/// - Throws: Whatever is thrown by `body`.
192196
static func withCurrent<R>(_ testCase: Self, perform body: () async throws -> R) async rethrows -> R {
193-
var runtimeState = Runner.RuntimeState.current
197+
var runtimeState = Runner.RuntimeState.current ?? .init()
194198
runtimeState.testCase = testCase
195199
return try await Runner.RuntimeState.$current.withValue(runtimeState, operation: body)
196200
}

Tests/TestingTests/Runner.RuntimeStateTests.swift

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,21 @@ struct Runner_RuntimeStateTests {
2525

2626
await Test(name: "foo") {}.run(configuration: configuration)
2727
}
28+
29+
// This confirms that an event posted within the closure of `withTaskGroup`
30+
// (and similar APIs) in regular tests (those which are not testing the
31+
// testing library itself) are handled appropriately and do not crash.
32+
@Test func eventPostingInTaskGroup() async {
33+
// Enable delivery of "expectation checked" events, merely as a way to allow
34+
// an event to be posted during the test below without causing any real
35+
// issues to be recorded or otherwise confuse the testing harness.
36+
var configuration = Configuration.current ?? .init()
37+
configuration.deliverExpectationCheckedEvents = true
38+
39+
await Configuration.withCurrent(configuration) {
40+
await withTaskGroup(of: Void.self) { group in
41+
#expect(Bool(true))
42+
}
43+
}
44+
}
2845
}

Tests/TestingTests/RunnerTests.swift

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,13 @@ struct NeverRunTests {
4949
}
5050

5151
final class RunnerTests: XCTestCase {
52+
func testInitialTaskLocalState() {
53+
// These are expected to be `nil` since this is an XCTest.
54+
XCTAssertNil(Test.current)
55+
XCTAssertNil(Test.Case.current)
56+
XCTAssertNil(Configuration.current)
57+
}
58+
5259
func testDefaultInit() async throws {
5360
let runner = await Runner()
5461
XCTAssertFalse(runner.tests.contains(where: \.isHidden))

0 commit comments

Comments
 (0)