Skip to content

Commit 33a04c7

Browse files
harlanhaskinsharlanhaskins
authored
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. ### Motivation: (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. ### Modifications: 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. ### 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://182236497
1 parent 8737f7a commit 33a04c7

4 files changed

Lines changed: 187 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/EventRecorderTests.swift‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -594,7 +594,9 @@ struct EventRecorderTests {
594594
let encodedEvents = [
595595
Event(.runStarted, testID: nil, testCaseID: nil),
596596
Event(.testStarted, testID: test.id, testCaseID: nil),
597+
Event(.testCaseStarted, testID: test.id, testCaseID: nil),
597598
Event(.issueRecorded(.init(kind: .unconditional)), testID: test.id, testCaseID: nil),
599+
Event(.testCaseEnded, testID: test.id, testCaseID: nil),
598600
Event(.testEnded, testID: test.id, testCaseID: nil),
599601
Event(.runEnded, testID: nil, testCaseID: nil),
600602
].compactMap { event in

‎Tests/TestingTests/TestCaseIterationTests.swift‎

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

0 commit comments

Comments
 (0)