Skip to content

Commit 939737a

Browse files
authored
Represent non-encodable test argument values in Test.Case.ID (#1000)
This expands `Test.Case.ID` to represent combinations of arguments which aren't fully encodable, and uniquely distinguishes test cases associated with the same parameterized test function which have otherwise identical IDs. ### Motivation: It is possible to declare a parameterized test function with a collection of arguments that includes the same element more than once. As a trivial example: ```swift @test(arguments: [1, 1]) func repeatedArg(value: Int) { ... } ``` This can happen in more realistic scenarios if you dynamically construct a collection of arguments that accidentally or intentionally includes the same value more than once. The testing library attempts to form a unique identifier for each argument passed to a parameterized test. It does so by checking whether the value conforms to `Encodable`, or one of the other protocols mentioned in the documentation (see [Run selected test cases](https://swiftpackageindex.com/swiftlang/swift-testing/main/documentation/testing/parameterizedtesting#Run-selected-test-cases)). One problem with the current implementation is that if the value _doesn't_ conform to one of those known protocols, the testing library gives up and doesn't produce a unique identifier for that test case at all. Specifically, in that situation the `argumentIDs` property of `Test.Case.ID` will have a value of `nil`. Another problem is that if an argument is passed more than once, the derived identifiers for each argument's test case will be the same and the result reporting will be ambiguous at best; or worse, the lack of a unique identifier could cause an integrated tool to misbehave or crash. (Current versions of Xcode 16 experience this issue non-deterministically for projects which have test parallelization enabled.) To solve this, there needs to be a way to deterministically distinguish test cases which, from the testing library's perspective, appear identical—either because they actually are the same value, or because they encode to the same representation. ### Modifications: - Add a new `isStable` boolean property to `Test.Case.Argument.ID` representing whether or not the testing library was able to encode a stable representation of the argument value it identifies. - Add a new SPI property named `discriminator` to `Test.Case` which distinguishes test cases associated with the same test function whose arguments are identical. - Add a corresponding property to `Test.Case.ID` with the same name, `discriminator`. - Change the `Test.Case.ID.argumentIDs` property to non-Optional, so that it always has a value even if one or more argument IDs is non-stable. - Add a derived boolean property `isStable` to `Test.Case.ID` whose value is `true` iff all of its argument IDs are stable. - Add `Hashable` conformance to `Test.Case`, since the reason for avoiding such conformance has been resolved. - Add new tests. ### 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 #995 Resolves rdar://119522099
1 parent efc39a5 commit 939737a

13 files changed

+576
-101
lines changed

Sources/Testing/ABI/Encoded/ABI.EncodedTest.swift

+5-1
Original file line numberDiff line numberDiff line change
@@ -124,9 +124,13 @@ extension ABI {
124124
var displayName: String
125125

126126
init(encoding testCase: borrowing Test.Case) {
127+
guard let arguments = testCase.arguments else {
128+
preconditionFailure("Attempted to initialize an EncodedTestCase encoding a test case which is not parameterized: \(testCase). Please file a bug report at https://github.com/swiftlang/swift-testing/issues/new")
129+
}
130+
127131
// TODO: define an encodable form of Test.Case.ID
128132
id = String(describing: testCase.id)
129-
displayName = testCase.arguments.lazy
133+
displayName = arguments.lazy
130134
.map(\.value)
131135
.map(String.init(describingForTest:))
132136
.joined(separator: ", ")

Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift

+9-3
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,14 @@ extension Test.Case {
171171
/// - Parameters:
172172
/// - includeTypeNames: Whether the qualified type name of each argument's
173173
/// runtime type should be included. Defaults to `false`.
174+
///
175+
/// - Returns: A string containing the arguments of this test case formatted
176+
/// for presentation, or an empty string if this test cases is
177+
/// non-parameterized.
174178
fileprivate func labeledArguments(includingQualifiedTypeNames includeTypeNames: Bool = false) -> String {
175-
arguments.lazy
179+
guard let arguments else { return "" }
180+
181+
return arguments.lazy
176182
.map { argument in
177183
let valueDescription = String(describingForTest: argument.value)
178184

@@ -494,14 +500,14 @@ extension Event.HumanReadableOutputRecorder {
494500
return result
495501

496502
case .testCaseStarted:
497-
guard let testCase = eventContext.testCase, testCase.isParameterized else {
503+
guard let testCase = eventContext.testCase, testCase.isParameterized, let arguments = testCase.arguments else {
498504
break
499505
}
500506

501507
return [
502508
Message(
503509
symbol: .default,
504-
stringValue: "Passing \(testCase.arguments.count.counting("argument")) \(testCase.labeledArguments(includingQualifiedTypeNames: verbosity > 0)) to \(testName)"
510+
stringValue: "Passing \(arguments.count.counting("argument")) \(testCase.labeledArguments(includingQualifiedTypeNames: verbosity > 0)) to \(testName)"
505511
)
506512
]
507513

Sources/Testing/Parameterization/CustomTestArgumentEncodable.swift

+10-3
Original file line numberDiff line numberDiff line change
@@ -44,18 +44,25 @@ public protocol CustomTestArgumentEncodable: Sendable {
4444
}
4545

4646
extension Test.Case.Argument.ID {
47-
/// Initialize this instance with an ID for the specified test argument.
47+
/// Initialize an ID instance with the specified test argument value.
4848
///
4949
/// - Parameters:
5050
/// - value: The value of a test argument for which to get an ID.
5151
/// - parameter: The parameter of the test function to which this argument
5252
/// value was passed.
5353
///
54-
/// - Returns: `nil` if an ID cannot be formed from the specified test
54+
/// - Returns: `nil` if a stable ID cannot be formed from the specified test
5555
/// argument value.
5656
///
5757
/// - Throws: Any error encountered while attempting to encode `value`.
5858
///
59+
/// If a stable representation of `value` can be encoded successfully, the
60+
/// value of this instance's `bytes` property will be the the bytes of that
61+
/// encoded JSON representation and this instance may be considered stable. If
62+
/// no stable representation of `value` can be obtained, `nil` is returned. If
63+
/// a stable representation was obtained but failed to encode, the error
64+
/// resulting from the encoding attempt is thrown.
65+
///
5966
/// This function is not part of the public interface of the testing library.
6067
///
6168
/// ## See Also
@@ -83,7 +90,7 @@ extension Test.Case.Argument.ID {
8390
return nil
8491
}
8592

86-
self = .init(bytes: try Self._encode(encodableValue, parameter: parameter))
93+
self.init(bytes: try Self._encode(encodableValue, parameter: parameter))
8794
#else
8895
nil
8996
#endif

Sources/Testing/Parameterization/Test.Case.Generator.swift

+27-2
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ extension Test.Case {
6262
// A beautiful hack to give us the right number of cases: iterate over a
6363
// collection containing a single Void value.
6464
self.init(sequence: CollectionOfOne(())) { _ in
65-
Test.Case(arguments: [], body: testFunction)
65+
Test.Case(body: testFunction)
6666
}
6767
}
6868

@@ -257,7 +257,32 @@ extension Test.Case {
257257

258258
extension Test.Case.Generator: Sequence {
259259
func makeIterator() -> some IteratorProtocol<Test.Case> {
260-
_sequence.lazy.map(_mapElement).makeIterator()
260+
let state = (
261+
iterator: _sequence.makeIterator(),
262+
testCaseIDs: [Test.Case.ID: Int](minimumCapacity: underestimatedCount)
263+
)
264+
265+
return sequence(state: state) { state in
266+
guard let element = state.iterator.next() else {
267+
return nil
268+
}
269+
270+
var testCase = _mapElement(element)
271+
272+
if testCase.isParameterized {
273+
// Store the original, unmodified test case ID. We're about to modify a
274+
// property which affects it, and we want to update state based on the
275+
// original one.
276+
let testCaseID = testCase.id
277+
278+
// Ensure test cases with identical IDs each have a unique discriminator.
279+
let discriminator = state.testCaseIDs[testCaseID, default: 0]
280+
testCase.discriminator = discriminator
281+
state.testCaseIDs[testCaseID] = discriminator + 1
282+
}
283+
284+
return testCase
285+
}
261286
}
262287

263288
var underestimatedCount: Int {

Sources/Testing/Parameterization/Test.Case.ID.swift

+97-22
Original file line numberDiff line numberDiff line change
@@ -15,50 +15,125 @@ extension Test.Case {
1515
/// parameterized test function. They are not necessarily unique across two
1616
/// different ``Test`` instances.
1717
@_spi(ForToolsIntegrationOnly)
18-
public struct ID: Sendable, Equatable, Hashable {
18+
public struct ID: Sendable {
1919
/// The IDs of the arguments of this instance's associated ``Test/Case``, in
2020
/// the order they appear in ``Test/Case/arguments``.
2121
///
22-
/// The value of this property is `nil` if _any_ of the associated test
23-
/// case's arguments has a `nil` ID.
22+
/// The value of this property is `nil` for the ID of the single test case
23+
/// associated with a non-parameterized test function.
2424
public var argumentIDs: [Argument.ID]?
2525

26-
public init(argumentIDs: [Argument.ID]?) {
26+
/// A number used to distinguish this test case from others associated with
27+
/// the same parameterized test function whose arguments have the same ID.
28+
///
29+
/// The value of this property is `nil` for the ID of the single test case
30+
/// associated with a non-parameterized test function.
31+
///
32+
/// ## See Also
33+
///
34+
/// - ``Test/Case/discriminator``
35+
public var discriminator: Int?
36+
37+
/// Whether or not this test case ID is considered stable across successive
38+
/// runs.
39+
public var isStable: Bool
40+
41+
init(argumentIDs: [Argument.ID]?, discriminator: Int?, isStable: Bool) {
42+
precondition((argumentIDs == nil) == (discriminator == nil))
43+
2744
self.argumentIDs = argumentIDs
45+
self.discriminator = discriminator
46+
self.isStable = isStable
2847
}
2948
}
3049

3150
@_spi(ForToolsIntegrationOnly)
3251
public var id: ID {
33-
let argumentIDs = arguments.compactMap(\.id)
34-
guard argumentIDs.count == arguments.count else {
35-
return ID(argumentIDs: nil)
36-
}
37-
38-
return ID(argumentIDs: argumentIDs)
52+
ID(argumentIDs: arguments.map { $0.map(\.id) }, discriminator: discriminator, isStable: isStable)
3953
}
4054
}
4155

4256
// MARK: - CustomStringConvertible
4357

4458
extension Test.Case.ID: CustomStringConvertible {
4559
public var description: String {
46-
"argumentIDs: \(String(describing: argumentIDs))"
60+
if let argumentIDs, let discriminator {
61+
"Parameterized test case ID: argumentIDs: \(argumentIDs), discriminator: \(discriminator), isStable: \(isStable)"
62+
} else {
63+
"Non-parameterized test case ID"
64+
}
4765
}
4866
}
4967

5068
// MARK: - Codable
5169

52-
extension Test.Case.ID: Codable {}
70+
extension Test.Case.ID: Codable {
71+
private enum CodingKeys: String, CodingKey {
72+
/// A coding key for ``Test/Case/ID/argumentIDs``.
73+
///
74+
/// This case's string value is non-standard because ``legacyArgumentIDs``
75+
/// already used "argumentIDs" and this needs to be different.
76+
case argumentIDs = "argIDs"
5377

54-
// MARK: - Equatable
78+
/// A coding key for ``Test/Case/ID/discriminator``.
79+
case discriminator
5580

56-
// We cannot safely implement Equatable for Test.Case because its values are
57-
// type-erased. It does conform to `Identifiable`, but its ID type is composed
58-
// of the IDs of its arguments, and those IDs are not always available (for
59-
// example, if the type of an argument is not Codable). Thus, we cannot check
60-
// for equality of test cases based on this, because if two test cases had
61-
// different arguments, but the type of those arguments is not Codable, they
62-
// both will have a `nil` ID and would incorrectly be considered equal.
63-
//
64-
// `Test.Case.ID` is Equatable, however.
81+
/// A coding key for ``Test/Case/ID/isStable``.
82+
case isStable
83+
84+
/// A coding key for the legacy representation of ``Test/Case/ID/argumentIDs``.
85+
///
86+
/// This case's string value is non-standard in order to maintain
87+
/// legacy compatibility with its original value.
88+
case legacyArgumentIDs = "argumentIDs"
89+
}
90+
91+
public init(from decoder: any Decoder) throws {
92+
let container = try decoder.container(keyedBy: CodingKeys.self)
93+
94+
if container.contains(.isStable) {
95+
// `isStable` is present, so we're decoding an instance encoded using the
96+
// newest style: every property can be decoded straightforwardly.
97+
try self.init(
98+
argumentIDs: container.decodeIfPresent([Test.Case.Argument.ID].self, forKey: .argumentIDs),
99+
discriminator: container.decodeIfPresent(Int.self, forKey: .discriminator),
100+
isStable: container.decode(Bool.self, forKey: .isStable)
101+
)
102+
} else if container.contains(.legacyArgumentIDs) {
103+
// `isStable` is absent, so we're decoding using the old style. Since the
104+
// legacy `argumentIDs` is present, the representation should be
105+
// considered stable.
106+
let decodedArgumentIDs = try container.decode([Test.Case.Argument.ID].self, forKey: .legacyArgumentIDs)
107+
let argumentIDs = decodedArgumentIDs.isEmpty ? nil : decodedArgumentIDs
108+
109+
// Discriminator should be `nil` for the ID of a non-parameterized test
110+
// case, but can default to 0 for the ID of a parameterized test case.
111+
let discriminator = argumentIDs == nil ? nil : 0
112+
113+
self.init(argumentIDs: argumentIDs, discriminator: discriminator, isStable: true)
114+
} else {
115+
// This is the old style, and since `argumentIDs` is absent, we know this
116+
// ID represents a parameterized test case which is non-stable.
117+
self.init(argumentIDs: [.init(bytes: [])], discriminator: 0, isStable: false)
118+
}
119+
}
120+
121+
public func encode(to encoder: any Encoder) throws {
122+
var container = encoder.container(keyedBy: CodingKeys.self)
123+
124+
try container.encode(isStable, forKey: .isStable)
125+
try container.encodeIfPresent(discriminator, forKey: .discriminator)
126+
try container.encodeIfPresent(argumentIDs, forKey: .argumentIDs)
127+
128+
// Encode the legacy representation of `argumentIDs`.
129+
if argumentIDs == nil {
130+
try container.encode([Test.Case.Argument.ID](), forKey: .legacyArgumentIDs)
131+
} else if isStable, let argumentIDs = argumentIDs {
132+
try container.encode(argumentIDs, forKey: .legacyArgumentIDs)
133+
}
134+
}
135+
}
136+
137+
// MARK: - Equatable, Hashable
138+
139+
extension Test.Case.ID: Equatable, Hashable {}

0 commit comments

Comments
 (0)