From 616dd879db5205720596a9da611e17d80f4ba689 Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Thu, 6 Mar 2025 14:28:13 -0600 Subject: [PATCH] Represent non-encodable test argument values in Test.Case.ID Resolves #995 Resolves rdar://119522099 --- .../CustomTestArgumentEncodable.swift | 80 ++++++--- .../Test.Case.Generator.swift | 24 ++- .../Parameterization/Test.Case.ID.swift | 65 +++++--- .../Testing/Parameterization/Test.Case.swift | 156 ++++++++++++++---- Tests/TestingTests/EventTests.swift | 2 +- .../Test.Case.GeneratorTests.swift | 31 ++++ Tests/TestingTests/Test.CaseTests.swift | 61 +++++++ 7 files changed, 337 insertions(+), 82 deletions(-) create mode 100644 Tests/TestingTests/Test.Case.GeneratorTests.swift create mode 100644 Tests/TestingTests/Test.CaseTests.swift diff --git a/Sources/Testing/Parameterization/CustomTestArgumentEncodable.swift b/Sources/Testing/Parameterization/CustomTestArgumentEncodable.swift index 58d738f11..1201badc5 100644 --- a/Sources/Testing/Parameterization/CustomTestArgumentEncodable.swift +++ b/Sources/Testing/Parameterization/CustomTestArgumentEncodable.swift @@ -43,50 +43,76 @@ public protocol CustomTestArgumentEncodable: Sendable { func encodeTestArgument(to encoder: some Encoder) throws } +/// Get the best encodable representation of a test argument value, if any. +/// +/// - Parameters: +/// - value: The value for which an encodable representation is requested. +/// +/// - Returns: The best encodable representation of `value`, if one is available, +/// otherwise `nil`. +/// +/// For a description of the heuristics used to obtain an encodable +/// representation of an argument value, see . +func encodableArgumentValue(for value: some Sendable) -> (any Encodable)? { +#if canImport(Foundation) + // Helper for opening an existential. + func customArgumentWrapper(for value: some CustomTestArgumentEncodable) -> some Encodable { + _CustomArgumentWrapper(rawValue: value) + } + + return if let customEncodable = value as? any CustomTestArgumentEncodable { + customArgumentWrapper(for: customEncodable) + } else if let rawRepresentable = value as? any RawRepresentable, let encodableRawValue = rawRepresentable.rawValue as? any Encodable { + encodableRawValue + } else if let encodable = value as? any Encodable { + encodable + } else if let identifiable = value as? any Identifiable, let encodableID = identifiable.id as? any Encodable { + encodableID + } else { + nil + } +#else + return nil +#endif +} + extension Test.Case.Argument.ID { /// Initialize this instance with an ID for the specified test argument. /// /// - Parameters: /// - value: The value of a test argument for which to get an ID. + /// - encodableValue: An encodable representation of `value`, if any, with + /// which to attempt to encode a stable representation. /// - parameter: The parameter of the test function to which this argument /// value was passed. /// - /// - Returns: `nil` if an ID cannot be formed from the specified test - /// argument value. - /// - /// - Throws: Any error encountered while attempting to encode `value`. + /// If a representation of `value` can be successfully encoded, the value of + /// this instance's `bytes` property will be the the bytes of that encoded + /// JSON representation and the value of its `isStable` property will be + /// `true`. Otherwise, the value of its `bytes` property will be the bytes of + /// a textual description of `value` and the value of `isStable` will be + /// `false` to reflect that the representation is not considered stable. /// /// This function is not part of the public interface of the testing library. /// /// ## See Also /// /// - ``CustomTestArgumentEncodable`` - init?(identifying value: some Sendable, parameter: Test.Parameter) throws { + init(identifying value: some Sendable, encodableValue: (any Encodable)?, parameter: Test.Parameter) { #if canImport(Foundation) - func customArgumentWrapper(for value: some CustomTestArgumentEncodable) -> some Encodable { - _CustomArgumentWrapper(rawValue: value) - } - - let encodableValue: (any Encodable)? = if let customEncodable = value as? any CustomTestArgumentEncodable { - customArgumentWrapper(for: customEncodable) - } else if let rawRepresentable = value as? any RawRepresentable, let encodableRawValue = rawRepresentable.rawValue as? any Encodable { - encodableRawValue - } else if let encodable = value as? any Encodable { - encodable - } else if let identifiable = value as? any Identifiable, let encodableID = identifiable.id as? any Encodable { - encodableID - } else { - nil - } - - guard let encodableValue else { - return nil + if let encodableValue { + do { + self = .init(bytes: try Self._encode(encodableValue, parameter: parameter), isStable: true) + return + } catch { + // FIXME: Capture the error and propagate to the user, not as a test + // failure but as an advisory warning. A missing argument ID will + // prevent re-running the test case, but is not a blocking issue. + } } - - self = .init(bytes: try Self._encode(encodableValue, parameter: parameter)) -#else - nil #endif + + self = .init(bytes: String(describingForTest: value).utf8, isStable: false) } #if canImport(Foundation) diff --git a/Sources/Testing/Parameterization/Test.Case.Generator.swift b/Sources/Testing/Parameterization/Test.Case.Generator.swift index d4d583e48..1adf08d95 100644 --- a/Sources/Testing/Parameterization/Test.Case.Generator.swift +++ b/Sources/Testing/Parameterization/Test.Case.Generator.swift @@ -62,7 +62,7 @@ extension Test.Case { // A beautiful hack to give us the right number of cases: iterate over a // collection containing a single Void value. self.init(sequence: CollectionOfOne(())) { _ in - Test.Case(arguments: [], body: testFunction) + Test.Case(body: testFunction) } } @@ -257,7 +257,27 @@ extension Test.Case { extension Test.Case.Generator: Sequence { func makeIterator() -> some IteratorProtocol { - _sequence.lazy.map(_mapElement).makeIterator() + sequence(state: ( + iterator: _sequence.makeIterator(), + testCaseIDs: [Test.Case.ID: Int]() + )) { state in + guard let element = state.iterator.next() else { + return nil + } + + var testCase = _mapElement(element) + + // Store the original, unmodified test case ID. We're about to modify a + // property which affects it, and we want to update state based on the + // original one. + let testCaseID = testCase.id + + // Ensure test cases with identical IDs each have a unique discriminator. + testCase.discriminator = state.testCaseIDs[testCaseID, default: 0] + state.testCaseIDs[testCaseID] = testCase.discriminator + 1 + + return testCase + } } var underestimatedCount: Int { diff --git a/Sources/Testing/Parameterization/Test.Case.ID.swift b/Sources/Testing/Parameterization/Test.Case.ID.swift index 26b57fdf8..08637b968 100644 --- a/Sources/Testing/Parameterization/Test.Case.ID.swift +++ b/Sources/Testing/Parameterization/Test.Case.ID.swift @@ -15,27 +15,37 @@ extension Test.Case { /// parameterized test function. They are not necessarily unique across two /// different ``Test`` instances. @_spi(ForToolsIntegrationOnly) - public struct ID: Sendable, Equatable, Hashable { + public struct ID: Sendable { /// The IDs of the arguments of this instance's associated ``Test/Case``, in /// the order they appear in ``Test/Case/arguments``. + public var argumentIDs: [Argument.ID] + + /// A number used to distinguish this test case from others associated with + /// the same test function whose arguments have the same ID. + /// + /// ## See Also /// - /// The value of this property is `nil` if _any_ of the associated test - /// case's arguments has a `nil` ID. - public var argumentIDs: [Argument.ID]? + /// - ``Test/Case/discriminator`` + public var discriminator: Int - public init(argumentIDs: [Argument.ID]?) { + public init(argumentIDs: [Argument.ID], discriminator: Int) { self.argumentIDs = argumentIDs + self.discriminator = discriminator + } + + /// Whether or not this test case ID is considered stable across successive + /// runs. + /// + /// The value of this property is `true` if all of the argument IDs for this + /// instance are stable, otherwise it is `false`. + public var isStable: Bool { + argumentIDs.allSatisfy(\.isStable) } } @_spi(ForToolsIntegrationOnly) public var id: ID { - let argumentIDs = arguments.compactMap(\.id) - guard argumentIDs.count == arguments.count else { - return ID(argumentIDs: nil) - } - - return ID(argumentIDs: argumentIDs) + ID(argumentIDs: arguments.map(\.id), discriminator: discriminator) } } @@ -43,22 +53,31 @@ extension Test.Case { extension Test.Case.ID: CustomStringConvertible { public var description: String { - "argumentIDs: \(String(describing: argumentIDs))" + "argumentIDs: \(argumentIDs), discriminator: \(discriminator)" } } // MARK: - Codable -extension Test.Case.ID: Codable {} +extension Test.Case.ID: Codable { + public init(from decoder: some Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) -// MARK: - Equatable + // The `argumentIDs` property was optional when this type was first + // introduced, and a `nil` value represented a non-stable test case ID. + // To maintain previous behavior, if this value is absent when decoding, + // default to a single argument ID marked as non-stable. + let argumentIDs = try container.decodeIfPresent([Test.Case.Argument.ID].self, forKey: .argumentIDs) + ?? [Test.Case.Argument.ID(bytes: [], isStable: false)] -// We cannot safely implement Equatable for Test.Case because its values are -// type-erased. It does conform to `Identifiable`, but its ID type is composed -// of the IDs of its arguments, and those IDs are not always available (for -// example, if the type of an argument is not Codable). Thus, we cannot check -// for equality of test cases based on this, because if two test cases had -// different arguments, but the type of those arguments is not Codable, they -// both will have a `nil` ID and would incorrectly be considered equal. -// -// `Test.Case.ID` is Equatable, however. + // The `discriminator` property was added after this type was first + // introduced. It can safely default to zero when absent. + let discriminator = try container.decodeIfPresent(type(of: discriminator), forKey: .discriminator) ?? 0 + + self.init(argumentIDs: argumentIDs, discriminator: discriminator) + } +} + +// MARK: - Equatable, Hashable + +extension Test.Case.ID: Hashable {} diff --git a/Sources/Testing/Parameterization/Test.Case.swift b/Sources/Testing/Parameterization/Test.Case.swift index 80ff101da..b4e55c7c8 100644 --- a/Sources/Testing/Parameterization/Test.Case.swift +++ b/Sources/Testing/Parameterization/Test.Case.swift @@ -26,38 +26,56 @@ extension Test { /// The raw bytes of this instance's identifier. public var bytes: [UInt8] - public init(bytes: [UInt8]) { - self.bytes = bytes + /// Whether or not this argument ID is considered stable across + /// successive runs. + /// + /// If the value of this property is `true`, the testing library can use + /// this ID to deterministically match against the original argument + /// it represents, and a user can selectively (re-)run that argument + /// of the associated parameterized test. If it is `false`, that + /// functionality is not supported for the argument this ID represents. + public var isStable: Bool + + public init(bytes: some Sequence, isStable: Bool) { + self.bytes = Array(bytes) + self.isStable = isStable } } - /// The ID of this parameterized test argument, if any. + /// The value of this parameterized test argument. + public var value: any Sendable + + /// The ID of this parameterized test argument. /// /// The uniqueness of this value is narrow: it is considered unique only /// within the scope of the parameter of the test function this argument /// was passed to. /// - /// The value of this property is `nil` when an ID cannot be formed. This - /// may occur if the type of ``value`` does not conform to one of the - /// protocols used for encoding a stable and unique representation of the - /// value. - /// /// ## See Also /// /// - ``CustomTestArgumentEncodable`` - @_spi(ForToolsIntegrationOnly) - public var id: ID? { - // FIXME: Capture the error and propagate to the user, not as a test - // failure but as an advisory warning. A missing argument ID will - // prevent re-running the test case, but is not a blocking issue. - try? Argument.ID(identifying: value, parameter: parameter) - } - - /// The value of this parameterized test argument. - public var value: any Sendable + public var id: ID /// The parameter of the test function to which this argument was passed. public var parameter: Parameter + + /// Initialize an instance of this type representing the specified + /// argument value. + /// + /// - Parameters: + /// - value: The value of this parameterized test argument. + /// - encodableValue: An encodable representation of `value`, if one is + /// available. When non-`nil`, this is used to attempt to form a + /// stable identifier. + /// - parameter: The parameter of the test function to which this + /// argument was passed. + /// + /// This forms an ``ID`` identifying `value` using `encodableValue`. + init(value: any Sendable, encodableValue: (any Encodable)?, parameter: Parameter) { + self.value = value + self.id = .init(identifying: value, encodableValue: encodableValue, parameter: parameter) + self.parameter = parameter + } } /// The arguments passed to this test case. @@ -75,11 +93,36 @@ extension Test { @_spi(Experimental) @_spi(ForToolsIntegrationOnly) public var arguments: [Argument] - init( - arguments: [Argument], - body: @escaping @Sendable () async throws -> Void - ) { - self.arguments = arguments + /// A number used to distinguish this test case from others associated with + /// the same test function whose arguments have the same ID. + /// + /// As an example, imagine the same argument is passed more than once to a + /// parameterized test: + /// + /// ```swift + /// @Test(arguments: [1, 1]) + /// func example(x: Int) { ... } + /// ``` + /// + /// There will be two ``Test/Case`` instances associated with this test + /// function. Each will represent one instance of the repeated argument `1`, + /// and each will have a different value for this property. + /// + /// The value of this property for successive runs of the same test are not + /// guaranteed to be the same. The value of this property may be equal for + /// two test cases associated with the same test if the IDs of their + /// arguments are different. + @_spi(Experimental) @_spi(ForToolsIntegrationOnly) + public var discriminator: Int = 0 + + /// Initialize a test case for a non-parameterized test function. + /// + /// - Parameters: + /// - body: The body closure of this test case. + /// + /// The resulting test case will have zero arguments. + init(body: @escaping @Sendable () async throws -> Void) { + self.arguments = [] self.body = body } @@ -95,10 +138,29 @@ extension Test { parameters: [Parameter], body: @escaping @Sendable () async throws -> Void ) { - let arguments = zip(values, parameters).map { value, parameter in - Argument(value: value, parameter: parameter) + // Attempt to obtain an encodable representation of each value in order + // to construct a stable ID. + let encodingResult = values.reduce(into: ([any Encodable](), hasFailure: false)) { result, value in + // If we couldn't get an encodable representation of one of the values, + // give up and mark the overall attempt as a failure. This allows + // skipping unnecessary encoding work later: if any individual argument + // doesn't have a stable ID, the Test.Case.ID can't be considered stable, + // so there's no point encoding the values which _are_ encodable. + guard !result.hasFailure, let encodableValue = encodableArgumentValue(for: value) else { + return result.hasFailure = true + } + result.0.append(encodableValue) + } + let encodableValues: [any Encodable]? = if !encodingResult.hasFailure { + encodingResult.0 + } else { + nil } - self.init(arguments: arguments, body: body) + + self.arguments = zip(values.enumerated(), parameters).map { value, parameter in + Argument(value: value.1, encodableValue: encodableValues?[value.0], parameter: parameter) + } + self.body = body } /// Whether or not this test case is from a parameterized test. @@ -156,10 +218,32 @@ extension Test { // MARK: - Codable extension Test.Parameter: Codable {} -extension Test.Case.Argument.ID: Codable {} +extension Test.Case.Argument.ID: Codable { + public init(from decoder: some Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + + // The `isStable` property was added after this type was introduced. + // Previously, only stable argument IDs were ever encoded, so if we're + // attempting to decode one, we can safely assume it is stable. + let isStable = try container.decodeIfPresent(type(of: isStable), forKey: .isStable) ?? true + + let bytes = try container.decode(type(of: bytes), forKey: .bytes) + self.init(bytes: bytes, isStable: isStable) + } +} // MARK: - Equatable, Hashable +extension Test.Case: Hashable { + public static func ==(lhs: Test.Case, rhs: Test.Case) -> Bool { + lhs.id == rhs.id + } + + public func hash(into hasher: inout Hasher) { + hasher.combine(id) + } +} + extension Test.Parameter: Hashable {} extension Test.Case.Argument.ID: Hashable {} @@ -197,8 +281,8 @@ extension Test.Case.Argument { /// A serializable snapshot of a ``Test/Case/Argument`` instance. @_spi(ForToolsIntegrationOnly) public struct Snapshot: Sendable, Codable { - /// The ID of this parameterized test argument, if any. - public var id: Test.Case.Argument.ID? + /// The ID of this parameterized test argument. + public var id: Test.Case.Argument.ID /// A representation of this parameterized test argument's /// ``Test/Case/Argument/value`` property. @@ -217,6 +301,20 @@ extension Test.Case.Argument { value = Expression.Value(reflecting: argument.value) ?? .init(describing: argument.value) parameter = argument.parameter } + + public init(from decoder: some Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + + // The `id` property was optional when this type was first introduced, + // and a `nil` value represented an argument whose ID was non-stable. + // To maintain previous behavior, if this value is absent when decoding, + // default to an argument ID marked as non-stable. + id = try container.decodeIfPresent(Test.Case.Argument.ID.self, forKey: .id) + ?? ID(bytes: [], isStable: false) + + value = try container.decode(type(of: value), forKey: .value) + parameter = try container.decode(type(of: parameter), forKey: .parameter) + } } } #endif diff --git a/Tests/TestingTests/EventTests.swift b/Tests/TestingTests/EventTests.swift index 941dcadb9..d6dd48971 100644 --- a/Tests/TestingTests/EventTests.swift +++ b/Tests/TestingTests/EventTests.swift @@ -57,7 +57,7 @@ struct EventTests { let testID = Test.ID(moduleName: "ModuleName", nameComponents: ["NameComponent1", "NameComponent2"], sourceLocation: #_sourceLocation) - let testCaseID = Test.Case.ID(argumentIDs: nil) + let testCaseID = Test.Case.ID(argumentIDs: [], discriminator: 0) let event = Event(kind, testID: testID, testCaseID: testCaseID, instant: .now) let eventSnapshot = Event.Snapshot(snapshotting: event) let decoded = try JSON.encodeAndDecode(eventSnapshot) diff --git a/Tests/TestingTests/Test.Case.GeneratorTests.swift b/Tests/TestingTests/Test.Case.GeneratorTests.swift new file mode 100644 index 000000000..0c2afd0ef --- /dev/null +++ b/Tests/TestingTests/Test.Case.GeneratorTests.swift @@ -0,0 +1,31 @@ +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2025 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for Swift project authors +// + +@testable @_spi(Experimental) @_spi(ForToolsIntegrationOnly) import Testing + +@Suite("Test.Case.Generator Tests") +struct Test_Case_GeneratorTests { + @Test func uniqueDiscriminators() throws { + let generator = Test.Case.Generator( + arguments: [1, 1, 1], + parameters: [Test.Parameter(index: 0, firstName: "x", type: Int.self)], + testFunction: { _ in } + ) + + let testCases = Array(generator) + #expect(testCases.count == 3) + + let firstCase = try #require(testCases.first) + #expect(firstCase.id.discriminator == 0) + + let discriminators = Set(testCases.map(\.id.discriminator)) + #expect(discriminators.count == 3) + } +} diff --git a/Tests/TestingTests/Test.CaseTests.swift b/Tests/TestingTests/Test.CaseTests.swift new file mode 100644 index 000000000..c757ab808 --- /dev/null +++ b/Tests/TestingTests/Test.CaseTests.swift @@ -0,0 +1,61 @@ +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2025 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for Swift project authors +// + +@testable @_spi(Experimental) @_spi(ForToolsIntegrationOnly) import Testing + +@Suite("Test.Case Tests") +struct Test_CaseTests { + @Test func singleStableArgument() throws { + let testCase = Test.Case( + values: [1], + parameters: [Test.Parameter(index: 0, firstName: "x", type: Int.self)], + body: {} + ) + #expect(testCase.id.isStable) + #expect(testCase.arguments.allSatisfy { $0.id.isStable }) + } + + @Test func twoStableArguments() throws { + let testCase = Test.Case( + values: [1, "a"], + parameters: [ + Test.Parameter(index: 0, firstName: "x", type: Int.self), + Test.Parameter(index: 1, firstName: "y", type: String.self), + ], + body: {} + ) + #expect(testCase.id.isStable) + #expect(testCase.arguments.allSatisfy { $0.id.isStable }) + } + + @Test("Two arguments: one non-stable, followed by one stable") + func nonStableAndStableArgument() throws { + let testCase = Test.Case( + values: [NonCodable(), IssueRecordingEncodable()], + parameters: [ + Test.Parameter(index: 0, firstName: "x", type: NonCodable.self), + Test.Parameter(index: 1, firstName: "y", type: IssueRecordingEncodable.self), + ], + body: {} + ) + #expect(!testCase.id.isStable) + #expect(testCase.arguments.allSatisfy { !$0.id.isStable }) + } +} + +// MARK: - Fixtures, helpers + +private struct NonCodable {} + +private struct IssueRecordingEncodable: Encodable { + func encode(to encoder: any Encoder) throws { + Issue.record("Unexpected attempt to encode an instance of \(Self.self)") + } +}