[Experimental] Capturing values in exit tests #1040
+809
−137
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
grynspan
added
enhancement
|
@swift-ci test |
grynspan
force-pushed
the
jgrynspan/exit-test-value-capture
branch
from
c2a2534 to
74101e0
Compare
|
@swift-ci test |
grynspan
force-pushed
the
jgrynspan/exit-test-value-capture
branch
from
ec3ff3d to
514ed01
Compare
|
@swift-ci test |
1 similar comment
|
@swift-ci test |
grynspan
changed the title
grynspan
force-pushed
the
jgrynspan/exit-test-value-capture
branch
from
9f06c93 to
6db82cf
Compare
grynspan
changed the title
This PR implements an experimental form of state capture in exit tests. If you
specify a capture list on the test's body closure and explicitly write the type
of each captured value, _and_ each value conforms to `Sendable`, `Codable`, and
(implicitly) `Copyable`, we'll encode them and send them "over the wire" to the
child process:
```swift
let a = Int.random(in: 100 ..< 200)
await #expect(exitsWith: .failure) { [a = a as Int] in
assert(a > 500)
}
```
This PR is incomplete. Among other details:
- [] Need to properly transmit the data, not stuff it in an environment variable
- [] Need to capture source location information correctly for error handling
during value decoding
- [] Need to implement diagnostics correctly
We are ultimately constrained by the language here as we don't have real type
information for the captured values, nor can we infer captures by inspecting the
syntax of the exit test body (hence the need for an explicit capture list with
types.) If we had something like `decltype()` we could apply during macro
expansion, you wouldn't need to write `x = x as T` and could just write `x`.
grynspan
force-pushed
the
jgrynspan/exit-test-value-capture
branch
from
6db82cf to
6f87c49
Compare
|
@swift-ci test |
grynspan
requested review from
stmontgomery,
briancroom and
SeanROlszewski
as code owners
|
@swift-ci test |
|
@swift-ci test |
|
@swift-ci test |
|
@swift-ci test |
|
@swift-ci test |
|
@swift-ci test |
briancroom
reviewed
Apr 14, 2025
|
@swift-ci test |
| extension Array where Element == ExitTest.CapturedValue { | ||
| init<each T>(_ wrappedValues: repeat each T) where repeat each T: Codable & Sendable { | ||
| self.init() | ||
| repeat self.append(ExitTest.CapturedValue(wrappedValue: each wrappedValues)) |
There was a problem hiding this comment.
Any reason not to use parameter pack iteration here and below?
| // Make sure the value is of the correct type. If it's not, that's also | ||
| // probably a problem with the exit test handler or entry point. | ||
| guard let wrappedValue = wrappedValue as? U else { | ||
| fatalError("Expected captured value at index \(capturedValues.startIndex) with type '\(String(describingForTest: U.self))', but found an instance of '\(String(describingForTest: Swift.type(of: wrappedValue)))' instead.") |
There was a problem hiding this comment.
Include bug report link in these fatalErrors?
Comment on lines
-585
to
+668
| ?? parentArguments.dropFirst().last | ||
| ?? parentArguments.dropFirst().last |
There was a problem hiding this comment.
nit: Intentional indentation change?
Comment on lines
+765
to
+766
| // Create another pipe to send captured values (and possibly other state | ||
| // in the future) to the child process. |
There was a problem hiding this comment.
Is this the only "from parent to child" pipe, or was there another one previously and this is a second? (I know there's a pipe in the opposite direction, just asking whether this is the only one which goes from parent to "exit test" child.)
Comment on lines
775
to
+778
| childEnvironment["SWT_EXPERIMENTAL_BACKCHANNEL"] = backChannelEnvironmentVariable | ||
| } | ||
| if let capturedValuesEnvironmentVariable = _makeEnvironmentVariable(for: capturedValuesReadEnd) { | ||
| childEnvironment["SWT_EXPERIMENTAL_CAPTURED_VALUES"] = capturedValuesEnvironmentVariable |
There was a problem hiding this comment.
I'd find it nice if the two env vars used a similar naming convention as each other
| } | ||
| let capturedValuesJSON = try fileHandle.readToEnd() | ||
| let capturedValuesJSONLines = capturedValuesJSON.split(whereSeparator: \.isASCIINewline) | ||
| assert(capturedValues.count == capturedValuesJSONLines.count, "Expected to decode \(capturedValues.count) captured value(s) for the current exit test, but received \(capturedValuesJSONLines.count). Please file a bug report at https://github.com/swiftlang/swift-testing/issues/new") |
There was a problem hiding this comment.
Given this is an assert (and thus disabled in optimized/release builds), the bug report link seems less important here. Or should it be precondition, or something stronger?
Comment on lines
+16
to
+19
| #if !SWT_NO_LIBRARY_MACRO_PLUGINS | ||
| private import Foundation | ||
| private import SwiftParser | ||
| #endif |
There was a problem hiding this comment.
Curious about these import changes: what is Foundation being used for, and why are they guarded by !SWT_NO_LIBRARY_MACRO_PLUGINS even though that guard doesn't appear anywhere in this file?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements an experimental form of state capture in exit tests. If you specify a capture list on the test's body closure and explicitly write the type of each captured value, and each value conforms to
Sendable,Codable, and (implicitly)Copyable, we'll encode them and send them "over the wire" to the child process:This PR is incomplete. Among other details:
ExitTest.CapturedValueand__Expression.Valuehave any synergy. (They do, but it's beyond the scope of this initial/experimental PR.)We are ultimately constrained by the language here as we don't have real type information for the captured values, nor can we infer captures by inspecting the syntax of the exit test body (hence the need for an explicit capture list with types.) If we had something like
decltype()we could apply during macro expansion, you wouldn't need to writex = x as Tand could just writex. The macro would usedecltype()to produce a thunk function of the form:Checklist: