Skip to content

Commit 5bc8b84

Browse files
committed
Rethink how we capture expectation conditions and their subexpressions.
This PR completely rewrites how we capture expectation conditions. For example, given the following expectation: ```swift ``` We currently detect that there is a binary operation and emit code that calls the binary operator as a closure and passes the left-hand value and right-hand value, then checks that the result of the operation is `true`. This is sufficient for simpler expressions like that one, but more complex ones (including any that involve `try` or `await` keywords) cannot be expanded correctly. With this PR, such expressions _can_ generally be expanded correctly. The change involves rewriting the macro condition as a closure to which is passed a local, mutable "context" value. Subexpressions of the condition expression are then rewritten by walking the syntax tree of the expression (using typical swift-syntax API) and replacing them with calls into the context value that pass in the value and related state. If the expectation ultimately fails, the collected data is transformed into an instance of the SPI type `Expression` that contains the source code of the expression and interesting subexpressions as well as the runtime values of those subexpressions. Nodes in the syntax tree are identified by a unique ID which is composed of the swift-syntax ID for that node as well as all its parent nodes in a compact bitmask format. These IDs can be transformed into graph/trie key paths when expression/subexpression relationships need to be reconstructed on failure, meaning that a single rewritten node doesn't otherwise need to know its "place" in the overall expression. There remain a few caveats (that also generally affect the current implementation): - Mutating member functions are syntactically indistinguishable from non-mutating ones and miscompile when rewritten; - Expressions involving move-only types are also indistinguishable, but need lifetime management to be rewritten correctly; and - Expressions where the `try` or `await` keyword is _outside_ the `#expect` macro cannot be expanded correctly because the macro cannot see those keywords during expansion. The first issue might be resolvable in the future using pointer tricks, although I don't hold a lot of hope for it. The second issue is probably resolved by non-escaping types. The third issue is an area of active exploration for us and the macros/swift-syntax team.
1 parent 0b77119 commit 5bc8b84

34 files changed

+2183
-1837
lines changed

Package.swift

+7
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,13 @@ let package = Package(
5656
],
5757
swiftSettings: .packageSettings
5858
),
59+
.testTarget(
60+
name: "SubexpressionShowcase",
61+
dependencies: [
62+
"Testing",
63+
],
64+
swiftSettings: .packageSettings
65+
),
5966

6067
.macro(
6168
name: "TestingMacros",

Sources/Testing/ABI/v0/Encoded/ABIv0.EncodedMessage.swift

+8
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,19 @@ extension ABIv0 {
6161
/// The symbol associated with this message.
6262
var symbol: Symbol
6363

64+
/// How much to indent this message when presenting it.
65+
///
66+
/// - Warning: This property is not yet part of the JSON schema.
67+
var _indentation: Int?
68+
6469
/// The human-readable, unformatted text associated with this message.
6570
var text: String
6671

6772
init(encoding message: borrowing Event.HumanReadableOutputRecorder.Message) {
6873
symbol = Symbol(encoding: message.symbol ?? .default)
74+
if message.indentation > 0 {
75+
_indentation = message.indentation
76+
}
6977
text = message.conciseStringValue ?? message.stringValue
7078
}
7179
}

Sources/Testing/CMakeLists.txt

+2-1
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ add_library(Testing
3939
Expectations/Expectation.swift
4040
Expectations/Expectation+Macro.swift
4141
Expectations/ExpectationChecking+Macro.swift
42+
Expectations/ExpectationContext.swift
4243
Issues/Confirmation.swift
4344
Issues/ErrorSnapshot.swift
4445
Issues/Issue.swift
@@ -61,7 +62,7 @@ add_library(Testing
6162
SourceAttribution/Backtrace+Symbolication.swift
6263
SourceAttribution/CustomTestStringConvertible.swift
6364
SourceAttribution/Expression.swift
64-
SourceAttribution/Expression+Macro.swift
65+
SourceAttribution/ExpressionID.swift
6566
SourceAttribution/SourceContext.swift
6667
SourceAttribution/SourceLocation.swift
6768
SourceAttribution/SourceLocation+Macro.swift

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

+25-15
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,22 @@ private let _ansiEscapeCodePrefix = "\u{001B}["
129129
private let _resetANSIEscapeCode = "\(_ansiEscapeCodePrefix)0m"
130130

131131
extension Event.Symbol {
132+
/// Get the string value to use for a message with no associated symbol.
133+
///
134+
/// - Parameters:
135+
/// - options: Options to use when writing the symbol.
136+
///
137+
/// - Returns: A string representation of "no symbol" appropriate for writing
138+
/// to a stream.
139+
fileprivate static func placeholderStringValue(options: Event.ConsoleOutputRecorder.Options) -> String {
140+
#if os(macOS) || (os(iOS) && targetEnvironment(macCatalyst))
141+
if options.useSFSymbols {
142+
return " "
143+
}
144+
#endif
145+
return " "
146+
}
147+
132148
/// Get the string value for this symbol with the given write options.
133149
///
134150
/// - Parameters:
@@ -169,7 +185,7 @@ extension Event.Symbol {
169185
case .attachment:
170186
return "\(_ansiEscapeCodePrefix)94m\(symbolCharacter)\(_resetANSIEscapeCode)"
171187
case .details:
172-
return symbolCharacter
188+
return "\(symbolCharacter)"
173189
}
174190
}
175191
return "\(symbolCharacter)"
@@ -303,18 +319,12 @@ extension Event.ConsoleOutputRecorder {
303319
/// - Returns: Whether any output was produced and written to this instance's
304320
/// destination.
305321
@discardableResult public func record(_ event: borrowing Event, in context: borrowing Event.Context) -> Bool {
306-
let messages = _humanReadableOutputRecorder.record(event, in: context)
307-
308-
// Padding to use in place of a symbol for messages that don't have one.
309-
var padding = " "
310-
#if os(macOS) || (os(iOS) && targetEnvironment(macCatalyst))
311-
if options.useSFSymbols {
312-
padding = " "
313-
}
314-
#endif
322+
let symbolPlaceholder = Event.Symbol.placeholderStringValue(options: options)
315323

324+
let messages = _humanReadableOutputRecorder.record(event, in: context)
316325
let lines = messages.lazy.map { [test = context.test] message in
317-
let symbol = message.symbol?.stringValue(options: options) ?? padding
326+
let symbol = message.symbol?.stringValue(options: options) ?? symbolPlaceholder
327+
let indentation = String(repeating: " ", count: message.indentation)
318328

319329
if case .details = message.symbol {
320330
// Special-case the detail symbol to apply grey to the entire line of
@@ -323,17 +333,17 @@ extension Event.ConsoleOutputRecorder {
323333
// to the indentation provided by the symbol.
324334
var lines = message.stringValue.split(whereSeparator: \.isNewline)
325335
lines = CollectionOfOne(lines[0]) + lines.dropFirst().map { line in
326-
"\(padding) \(line)"
336+
"\(indentation)\(symbolPlaceholder) \(line)"
327337
}
328338
let stringValue = lines.joined(separator: "\n")
329339
if options.useANSIEscapeCodes, options.ansiColorBitDepth > 1 {
330-
return "\(_ansiEscapeCodePrefix)90m\(symbol) \(stringValue)\(_resetANSIEscapeCode)\n"
340+
return "\(_ansiEscapeCodePrefix)90m\(symbol) \(indentation)\(stringValue)\(_resetANSIEscapeCode)\n"
331341
} else {
332-
return "\(symbol) \(stringValue)\n"
342+
return "\(symbol) \(indentation)\(stringValue)\n"
333343
}
334344
} else {
335345
let colorDots = test.map { self.colorDots(for: $0.tags) } ?? ""
336-
return "\(symbol) \(colorDots)\(message.stringValue)\n"
346+
return "\(symbol) \(indentation)\(colorDots)\(message.stringValue)\n"
337347
}
338348
}
339349

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

+22-11
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,15 @@ extension Event {
2525
/// The symbol associated with this message, if any.
2626
var symbol: Symbol?
2727

28+
/// How much to indent this message when presenting it.
29+
///
30+
/// The way in which this additional indentation is rendered is
31+
/// implementation-defined. Typically, the greater the value of this
32+
/// property, the more whitespace characters are inserted.
33+
///
34+
/// Rendering of indentation is optional.
35+
var indentation = 0
36+
2837
/// The human-readable message.
2938
var stringValue: String
3039

@@ -415,20 +424,22 @@ extension Event.HumanReadableOutputRecorder {
415424
}
416425
additionalMessages += _formattedComments(issue.comments)
417426

418-
if verbosity > 0, case let .expectationFailed(expectation) = issue.kind {
427+
if verbosity >= 0, case let .expectationFailed(expectation) = issue.kind {
419428
let expression = expectation.evaluatedExpression
420-
func addMessage(about expression: __Expression) {
421-
let description = expression.expandedDebugDescription()
422-
additionalMessages.append(Message(symbol: .details, stringValue: description))
423-
}
424-
let subexpressions = expression.subexpressions
425-
if subexpressions.isEmpty {
426-
addMessage(about: expression)
427-
} else {
428-
for subexpression in subexpressions {
429-
addMessage(about: subexpression)
429+
func addMessage(about expression: __Expression, depth: Int) {
430+
let description = if verbosity <= 0 {
431+
expression.expandedDescription()
432+
} else {
433+
expression.expandedDebugDescription()
434+
}
435+
if description != expression.sourceCode {
436+
additionalMessages.append(Message(symbol: .details, indentation: depth, stringValue: description))
437+
}
438+
for subexpression in expression.subexpressions {
439+
addMessage(about: subexpression, depth: depth + 1)
430440
}
431441
}
442+
addMessage(about: expression, depth: 0)
432443
}
433444

434445
let atSourceLocation = issue.sourceLocation.map { " at \($0)" } ?? ""

Sources/Testing/ExitTests/ExitTest.swift

+7-4
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ extension ExitTest {
241241
func callExitTest(
242242
exitsWith expectedExitCondition: ExitCondition,
243243
observing observedValues: [any PartialKeyPath<ExitTestArtifacts> & Sendable],
244-
expression: __Expression,
244+
sourceCode: String,
245245
comments: @autoclosure () -> [Comment],
246246
isRequired: Bool,
247247
isolation: isolated (any Actor)? = #isolation,
@@ -293,10 +293,13 @@ func callExitTest(
293293
let actualExitCondition = result.exitCondition
294294

295295
// Plumb the exit test's result through the general expectation machinery.
296-
return __checkValue(
296+
var expectationContext = __ExpectationContext()
297+
expectationContext.sourceCode[.root] = sourceCode
298+
expectationContext.runtimeValues[.root] = { Expression.Value(reflecting: actualExitCondition) }
299+
return check(
297300
expectedExitCondition == actualExitCondition,
298-
expression: expression,
299-
expressionWithCapturedRuntimeValues: expression.capturingRuntimeValues(actualExitCondition),
301+
expectationContext: expectationContext,
302+
mismatchedErrorDescription: nil,
300303
mismatchedExitConditionDescription: String(describingForTest: expectedExitCondition),
301304
comments: comments(),
302305
isRequired: isRequired,

Sources/Testing/Expectations/Expectation+Macro.swift

+4-4
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,10 @@
6666
/// running in the current task and an instance of ``ExpectationFailedError`` is
6767
/// thrown.
6868
@freestanding(expression) public macro require<T>(
69-
_ optionalValue: T?,
69+
_ optionalValue: consuming T?,
7070
_ comment: @autoclosure () -> Comment? = nil,
7171
sourceLocation: SourceLocation = #_sourceLocation
72-
) -> T = #externalMacro(module: "TestingMacros", type: "RequireMacro")
72+
) -> T = #externalMacro(module: "TestingMacros", type: "RequireMacro") where T: ~Copyable
7373

7474
/// Unwrap an optional boolean value or, if it is `nil`, fail and throw an
7575
/// error.
@@ -124,10 +124,10 @@ public macro require(
124124
@freestanding(expression)
125125
@_documentation(visibility: private)
126126
public macro require<T>(
127-
_ optionalValue: T,
127+
_ optionalValue: consuming T,
128128
_ comment: @autoclosure () -> Comment? = nil,
129129
sourceLocation: SourceLocation = #_sourceLocation
130-
) -> T = #externalMacro(module: "TestingMacros", type: "NonOptionalRequireMacro")
130+
) -> T = #externalMacro(module: "TestingMacros", type: "NonOptionalRequireMacro") where T: ~Copyable
131131

132132
// MARK: - Matching errors by type
133133

Sources/Testing/Expectations/Expectation.swift

+5-3
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,14 @@
1212
public struct Expectation: Sendable {
1313
/// The expression evaluated by this expectation.
1414
@_spi(ForToolsIntegrationOnly)
15-
public var evaluatedExpression: Expression
15+
public internal(set) var evaluatedExpression: Expression
1616

1717
/// A description of the error mismatch that occurred, if any.
1818
///
1919
/// If this expectation passed, the value of this property is `nil` because no
2020
/// error mismatch occurred.
2121
@_spi(Experimental) @_spi(ForToolsIntegrationOnly)
22-
public var mismatchedErrorDescription: String?
22+
public internal(set) var mismatchedErrorDescription: String?
2323

2424
/// A description of the difference between the operands in the expression
2525
/// evaluated by this expectation, if the difference could be determined.
@@ -28,7 +28,9 @@ public struct Expectation: Sendable {
2828
/// the difference is only computed when necessary to assist with diagnosing
2929
/// test failures.
3030
@_spi(Experimental) @_spi(ForToolsIntegrationOnly)
31-
public var differenceDescription: String?
31+
public var differenceDescription: String? {
32+
evaluatedExpression.differenceDescription
33+
}
3234

3335
/// A description of the exit condition that was expected to be matched.
3436
///

0 commit comments

Comments
 (0)