Skip to content

Commit 0e1e450

Browse files
harlanhaskinsharlanhaskins
committed
Simplify and fix encoded event ordering issues with test repetitions (#1793)
This fixes an issue introduced with #1528 where we now accidentally report `testStarted`/`testEnded` for non-parameterized repeated tests in an inconsistent way. (this is copy/pasted from a comment I left in-source, and normally I don't like leaving verbose comments in source but this one's confusing enough.) For all test cases of a given test, we emit the following set of events internally. ``` testStarted testCaseStarted (for each case) ... testCaseEnded (for each case) ... testEnded ``` For parameterized tests, this is what clients expect; the test itself has a distinct start/end from all the cases. For non-parameterized tests, however, clients don't need a redundant `testCaseStarted`/`testCaseEnded` for a single case, so we elide it. However, we don't know which `iteration` we're on until we've started running test cases, and subsequent iterations will post additional `testCaseStarted`/`testCaseEnded` events. To provide a coherent façade to our clients: - For non-parameterized tests, elide the outer `testStarted`/`testEnded` events, and replace `testCaseStarted`/`testCaseEnded` with `testStarted`/`testEnded`. - For parameterized tests, emit all events. This extracts the mapping from `Event.Kind` to `EncodedEvent.Kind` into a failable initializer on `EncodedEvent.Kind`, which splits the kind-mapping from the field-population logic, and then simplifies the logic for determining when to substitute/elide `test`/`testCase` events. - [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://182236497
1 parent daeb69e commit 0e1e450

3 files changed

Lines changed: 185 additions & 40 deletions

File tree

‎Sources/Testing/ABI/Encoded/ABI.EncodedEvent.swift‎

Lines changed: 76 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,76 @@ extension ABI {
3535
case testSkipped
3636
case testCancelled
3737
case runEnded
38+
39+
/// Encodes an ``Event/Kind`` into an ``EncodedEvent/Kind``.
40+
///
41+
/// Not all ``Event/Kind`` values map to an encoded kind value.
42+
init?(encoding kind: Event.Kind, in eventContext: borrowing Event.Context) {
43+
/// For all test cases of a given test, we emit the following set of
44+
/// events internally.
45+
///
46+
/// ```
47+
/// testStarted
48+
/// testCaseStarted (for each case)
49+
/// ...
50+
/// testCaseEnded (for each case)
51+
/// ...
52+
/// testEnded
53+
/// ```
54+
///
55+
/// For parameterized tests, this is what clients expect; the test
56+
/// itself has a distinct start/end from all the cases.
57+
///
58+
/// For non-parameterized tests, however, clients don't need a redundant
59+
/// `testCaseStarted`/`testCaseEnded` for a single case, so we elide it.
60+
///
61+
/// However, we don't know which `iteration` we're on until we've
62+
/// started running test cases, and subsequent iterations will post
63+
/// additional `testCaseStarted`/`testCaseEnded` events.
64+
///
65+
/// To provide a coherent façade to our clients:
66+
/// - For non-parameterized tests, elide the outer
67+
/// `testStarted`/`testEnded` events, and replace `testCaseStarted`/
68+
/// `testCaseEnded` with `testStarted`/`testEnded`.
69+
/// - For parameterized tests, emit all events.
70+
let isNonParameterizedTest = eventContext.test?.isParameterized == false
71+
72+
switch kind {
73+
case .runStarted:
74+
self = .runStarted
75+
case .testStarted:
76+
if isNonParameterizedTest {
77+
return nil
78+
}
79+
self = .testStarted
80+
case .testCaseStarted:
81+
self = isNonParameterizedTest ? .testStarted : .testCaseStarted
82+
case .issueRecorded:
83+
self = .issueRecorded
84+
case .valueAttached:
85+
self = .valueAttached
86+
case .testCaseEnded:
87+
self = isNonParameterizedTest ? .testEnded : .testCaseEnded
88+
case .testCaseCancelled:
89+
self = isNonParameterizedTest ? .testCancelled : .testCaseCancelled
90+
case .testEnded:
91+
if isNonParameterizedTest {
92+
return nil
93+
}
94+
self = .testEnded
95+
case .testSkipped:
96+
self = .testSkipped
97+
case .testCancelled:
98+
if isNonParameterizedTest {
99+
return nil
100+
}
101+
self = .testCancelled
102+
case .runEnded:
103+
self = .runEnded
104+
default:
105+
return nil
106+
}
107+
}
38108
}
39109

40110
/// The kind of event.
@@ -110,52 +180,18 @@ extension ABI {
110180
public var _sourceLocation: EncodedSourceLocation<V>?
111181

112182
init?(encoding event: borrowing Event, in eventContext: borrowing Event.Context, messages: borrowing [Event.HumanReadableOutputRecorder.Message]) {
183+
guard let encodedKind = Kind(encoding: event.kind, in: eventContext) else {
184+
return nil
185+
}
186+
kind = encodedKind
187+
113188
switch event.kind {
114-
case .runStarted:
115-
kind = .runStarted
116-
case .testStarted:
117-
kind = .testStarted
118-
case .testCaseStarted:
119-
// For non-parameterized tests, we elide `testCaseStarted` calls because it would be
120-
// redundant. However, for multiple iterations of a test case within a non-parameterized
121-
// function, we need to emit another `testStarted` event.
122-
if eventContext.test?.isParameterized == false {
123-
if let iteration = eventContext.iteration, iteration > 1 {
124-
kind = .testStarted
125-
} else {
126-
return nil
127-
}
128-
} else {
129-
kind = .testCaseStarted
130-
}
131189
case let .issueRecorded(recordedIssue):
132-
kind = .issueRecorded
133190
issue = EncodedIssue(encoding: recordedIssue, in: eventContext)
134191
case let .valueAttached(attachment):
135-
kind = .valueAttached
136192
self.attachment = EncodedAttachment(encoding: attachment)
137-
case .testCaseEnded:
138-
if eventContext.test?.isParameterized == false {
139-
if let iteration = eventContext.iteration, iteration > 1 {
140-
kind = .testEnded
141-
} else {
142-
return nil
143-
}
144-
} else {
145-
kind = .testCaseEnded
146-
}
147-
case .testCaseCancelled:
148-
kind = .testCaseCancelled
149-
case .testEnded:
150-
kind = .testEnded
151-
case .testSkipped:
152-
kind = .testSkipped
153-
case .testCancelled:
154-
kind = .testCancelled
155-
case .runEnded:
156-
kind = .runEnded
157193
default:
158-
return nil
194+
break
159195
}
160196
instant = EncodedInstant(encoding: event.instant)
161197
self.messages = messages.map(EncodedMessage.init)

‎Tests/TestingTests/ABI.EncodedEventTests.swift‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,5 +166,24 @@
166166
""")
167167
#expect(event.iteration == nil)
168168
}
169+
170+
@Test func `Encoded event for non-parameterized test doesn't add testCase`() async {
171+
var configuration = Configuration()
172+
configuration.eventHandler = { event, context in
173+
guard let encoded = ABI.EncodedEvent<ABI.CurrentVersion>(encoding: event, in: context, messages: []) else {
174+
return
175+
}
176+
switch encoded.kind {
177+
case .testStarted, .testEnded, .testCancelled:
178+
#expect(encoded._testCase == nil)
179+
case .testCaseStarted, .testCaseEnded, .testCaseCancelled:
180+
Issue.record("Should not encode test case events for non-parameterized test")
181+
default:
182+
return
183+
}
184+
}
185+
let test = Test() {}
186+
await test.run(configuration: configuration)
187+
}
169188
}
170189
#endif

‎Tests/TestingTests/TestCaseIterationTests.swift‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,4 +116,94 @@ struct TestCaseIterationTests {
116116
}
117117
}
118118
}
119+
120+
// MARK: Encoded event ordering
121+
122+
private func assertEncodedEventKinds(
123+
_ test: Test,
124+
equals expected: [ABI.EncodedEvent<ABI.v6_4>.Kind],
125+
sourceLocation: SourceLocation = #_sourceLocation
126+
) async {
127+
let events = Mutex<[ABI.EncodedEvent<ABI.CurrentVersion>]>([])
128+
var configuration = Configuration()
129+
configuration.eventHandler = { event, context in
130+
guard let encoded = ABI.EncodedEvent<ABI.CurrentVersion>(encoding: event, in: context, messages: []) else {
131+
return
132+
}
133+
events.withLock {
134+
$0.append(encoded)
135+
}
136+
}
137+
configuration.repetitionPolicy = .repeating(maximumIterationCount: 2)
138+
139+
await test.run(configuration: configuration)
140+
let kinds = events.rawValue.map(\.kind)
141+
#expect(kinds == expected, sourceLocation: sourceLocation)
142+
}
143+
144+
@Test
145+
func `Non-parameterized test repetitions don't nest`() async throws {
146+
let test = Test(name: "Test Name") {}
147+
await assertEncodedEventKinds(test, equals: [
148+
.runStarted,
149+
.testStarted,
150+
.testEnded,
151+
.testStarted,
152+
.testEnded,
153+
.runEnded
154+
])
155+
}
156+
157+
@Test
158+
func `Parameterized test repetitions are bookended with testStarted/Ended events`() async throws {
159+
let test = Test(arguments: [0], name: "Test Name") { _ in }
160+
await assertEncodedEventKinds(test, equals: [
161+
.runStarted,
162+
.testStarted,
163+
.testCaseStarted,
164+
.testCaseEnded,
165+
.testCaseStarted,
166+
.testCaseEnded,
167+
.testEnded,
168+
.runEnded
169+
])
170+
}
171+
172+
@Test
173+
func `Non-parameterized test cancellation reports a testCancelled event`() async throws {
174+
let test = Test(name: "Test Name") {
175+
try Test.cancel()
176+
}
177+
178+
await assertEncodedEventKinds(test, equals: [
179+
.runStarted,
180+
.testStarted,
181+
.testCancelled,
182+
.testEnded,
183+
.testStarted,
184+
.testCancelled,
185+
.testEnded,
186+
.runEnded
187+
])
188+
}
189+
190+
@Test
191+
func `Parameterized test cancellation reports a testCaseCancelled event`() async throws {
192+
let test = Test(arguments: [0], name: "Test Name") { _ in
193+
try Test.cancel()
194+
}
195+
196+
await assertEncodedEventKinds(test, equals: [
197+
.runStarted,
198+
.testStarted,
199+
.testCaseStarted,
200+
.testCaseCancelled,
201+
.testCaseEnded,
202+
.testCaseStarted,
203+
.testCaseCancelled,
204+
.testCaseEnded,
205+
.testEnded,
206+
.runEnded
207+
])
208+
}
119209
}

0 commit comments

Comments
 (0)