Skip to content

Commit 9d1bb5f

Browse files
authored
Record issues generated within exit tests. (#697)
This PR introduces a "back channel" file handle to exit tests, allowing us to record issues that occur within exit test bodies. For example: ```swift await #expect(exitsWith: .failure) { let context = try #require(possiblyMissingContext) ... } ``` In this example, if the call to `try #require()` finds `nil`, it will record an issue, but that issue today will be lost because there's no mechanism to forward the issue back to the parent process hosting the exit test. This PR fixes that! Issues are converted to JSON using the same schema we use for event handling, then written over a pipe back to the parent process where they are decoded. This decoding is lossy, so there will be further refinement needed here to try to preserve more information about the recorded issues. That said, "it's got good bones" right? On Darwin, Linux, and FreeBSD, the pipe's write end is allowed to survive into the child process (i.e. no `FD_CLOEXEC`). On Windows, the equivalent is to tell `CreateProcessW()` to explicitly inherit a `HANDLE`. The identity of this file descriptor or handle is passed to the child process via environment variable. The child process then parses the file descriptor or handle out of the environment and converts it back to a `FileHandle` that is then connected to an instance of `Configuration` with an event handler set, and off we go. Because we can now report these issues back to the parent process, I've removed the compile-time diagnostic in the `#expect(exitsWith:)` macro implementation that we emit when we see a nested `#expect()` or `#require()` call. ### 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.
1 parent e20774a commit 9d1bb5f

File tree

9 files changed

+455
-121
lines changed

9 files changed

+455
-121
lines changed

Sources/Testing/ExitTests/ExitTest.swift

+173-8
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,33 @@ extension ExitTest {
230230
/// recording any issues that occur.
231231
public typealias Handler = @Sendable (_ exitTest: borrowing ExitTest) async throws -> ExitCondition
232232

233+
/// The back channel file handle set up by the parent process.
234+
///
235+
/// The value of this property is a file handle open for writing to which
236+
/// events should be written, or `nil` if the file handle could not be
237+
/// resolved.
238+
private static let _backChannelForEntryPoint: FileHandle? = {
239+
guard let backChannelEnvironmentVariable = Environment.variable(named: "SWT_EXPERIMENTAL_BACKCHANNEL") else {
240+
return nil
241+
}
242+
243+
var fd: CInt?
244+
#if SWT_TARGET_OS_APPLE || os(Linux) || os(FreeBSD)
245+
fd = CInt(backChannelEnvironmentVariable)
246+
#elseif os(Windows)
247+
if let handle = UInt(backChannelEnvironmentVariable).flatMap(HANDLE.init(bitPattern:)) {
248+
fd = _open_osfhandle(Int(bitPattern: handle), _O_WRONLY | _O_BINARY)
249+
}
250+
#else
251+
#warning("Platform-specific implementation missing: back-channel pipe unavailable")
252+
#endif
253+
guard let fd, fd >= 0 else {
254+
return nil
255+
}
256+
257+
return try? FileHandle(unsafePOSIXFileDescriptor: fd, mode: "wb")
258+
}()
259+
233260
/// Find the exit test function specified in the environment of the current
234261
/// process, if any.
235262
///
@@ -240,16 +267,50 @@ extension ExitTest {
240267
/// `__swiftPMEntryPoint()` function. The effect of using it under other
241268
/// configurations is undefined.
242269
static func findInEnvironmentForEntryPoint() -> Self? {
270+
// Find the source location of the exit test to run, if any, in the
271+
// environment block.
272+
var sourceLocation: SourceLocation?
243273
if var sourceLocationString = Environment.variable(named: "SWT_EXPERIMENTAL_EXIT_TEST_SOURCE_LOCATION") {
244-
let sourceLocation = try? sourceLocationString.withUTF8 { sourceLocationBuffer in
274+
sourceLocation = try? sourceLocationString.withUTF8 { sourceLocationBuffer in
245275
let sourceLocationBuffer = UnsafeRawBufferPointer(sourceLocationBuffer)
246276
return try JSON.decode(SourceLocation.self, from: sourceLocationBuffer)
247277
}
248-
if let sourceLocation {
249-
return find(at: sourceLocation)
278+
}
279+
guard let sourceLocation else {
280+
return nil
281+
}
282+
283+
// If an exit test was found, inject back channel handling into its body.
284+
// External tools authors should set up their own back channel mechanisms
285+
// and ensure they're installed before calling ExitTest.callAsFunction().
286+
guard var result = find(at: sourceLocation) else {
287+
return nil
288+
}
289+
290+
// We can't say guard let here because it counts as a consume.
291+
guard _backChannelForEntryPoint != nil else {
292+
return result
293+
}
294+
295+
// Set up the configuration for this process.
296+
var configuration = Configuration()
297+
298+
// Encode events as JSON and write them to the back channel file handle.
299+
// Only forward issue-recorded events. (If we start handling other kinds of
300+
// events in the future, we can forward them too.)
301+
let eventHandler = ABIv0.Record.eventHandler(encodeAsJSONLines: true) { json in
302+
try? _backChannelForEntryPoint?.write(json)
303+
}
304+
configuration.eventHandler = { event, eventContext in
305+
if case .issueRecorded = event.kind {
306+
eventHandler(event, eventContext)
250307
}
251308
}
252-
return nil
309+
310+
result.body = { [configuration, body = result.body] in
311+
try await Configuration.withCurrent(configuration, perform: body)
312+
}
313+
return result
253314
}
254315

255316
/// The exit test handler used when integrating with Swift Package Manager via
@@ -343,11 +404,115 @@ extension ExitTest {
343404
childEnvironment["SWT_EXPERIMENTAL_EXIT_TEST_SOURCE_LOCATION"] = String(decoding: json, as: UTF8.self)
344405
}
345406

346-
return try await spawnAndWait(
347-
forExecutableAtPath: childProcessExecutablePath,
348-
arguments: childArguments,
349-
environment: childEnvironment
407+
return try await withThrowingTaskGroup(of: ExitCondition?.self) { taskGroup in
408+
// Create a "back channel" pipe to handle events from the child process.
409+
let backChannel = try FileHandle.Pipe()
410+
411+
// Let the child process know how to find the back channel by setting a
412+
// known environment variable to the corresponding file descriptor
413+
// (HANDLE on Windows.)
414+
var backChannelEnvironmentVariable: String?
415+
#if SWT_TARGET_OS_APPLE || os(Linux) || os(FreeBSD)
416+
backChannelEnvironmentVariable = backChannel.writeEnd.withUnsafePOSIXFileDescriptor { fd in
417+
fd.map(String.init(describing:))
418+
}
419+
#elseif os(Windows)
420+
backChannelEnvironmentVariable = backChannel.writeEnd.withUnsafeWindowsHANDLE { handle in
421+
handle.flatMap { String(describing: UInt(bitPattern: $0)) }
422+
}
423+
#else
424+
#warning("Platform-specific implementation missing: back-channel pipe unavailable")
425+
#endif
426+
if let backChannelEnvironmentVariable {
427+
childEnvironment["SWT_EXPERIMENTAL_BACKCHANNEL"] = backChannelEnvironmentVariable
428+
}
429+
430+
// Spawn the child process.
431+
let processID = try withUnsafePointer(to: backChannel.writeEnd) { writeEnd in
432+
try spawnExecutable(
433+
atPath: childProcessExecutablePath,
434+
arguments: childArguments,
435+
environment: childEnvironment,
436+
additionalFileHandles: .init(start: writeEnd, count: 1)
437+
)
438+
}
439+
440+
// Await termination of the child process.
441+
taskGroup.addTask {
442+
try await wait(for: processID)
443+
}
444+
445+
// Read back all data written to the back channel by the child process
446+
// and process it as a (minimal) event stream.
447+
let readEnd = backChannel.closeWriteEnd()
448+
taskGroup.addTask {
449+
Self._processRecords(fromBackChannel: readEnd)
450+
return nil
451+
}
452+
453+
// This is a roundabout way of saying "and return the exit condition
454+
// yielded by wait(for:)".
455+
return try await taskGroup.compactMap { $0 }.first { _ in true }!
456+
}
457+
}
458+
}
459+
460+
/// Read lines from the given back channel file handle and process them as
461+
/// event records.
462+
///
463+
/// - Parameters:
464+
/// - backChannel: The file handle to read from. Reading continues until an
465+
/// error is encountered or the end of the file is reached.
466+
private static func _processRecords(fromBackChannel backChannel: borrowing FileHandle) {
467+
let bytes: [UInt8]
468+
do {
469+
bytes = try backChannel.readToEnd()
470+
} catch {
471+
// NOTE: an error caught here indicates an I/O problem.
472+
// TODO: should we record these issues as systemic instead?
473+
Issue.record(error)
474+
return
475+
}
476+
477+
for recordJSON in bytes.split(whereSeparator: \.isASCIINewline) where !recordJSON.isEmpty {
478+
do {
479+
try recordJSON.withUnsafeBufferPointer { recordJSON in
480+
try Self._processRecord(.init(recordJSON), fromBackChannel: backChannel)
481+
}
482+
} catch {
483+
// NOTE: an error caught here indicates a decoding problem.
484+
// TODO: should we record these issues as systemic instead?
485+
Issue.record(error)
486+
}
487+
}
488+
}
489+
490+
/// Decode a line of JSON read from a back channel file handle and handle it
491+
/// as if the corresponding event occurred locally.
492+
///
493+
/// - Parameters:
494+
/// - recordJSON: The JSON to decode and process.
495+
/// - backChannel: The file handle that `recordJSON` was read from.
496+
///
497+
/// - Throws: Any error encountered attempting to decode or process the JSON.
498+
private static func _processRecord(_ recordJSON: UnsafeRawBufferPointer, fromBackChannel backChannel: borrowing FileHandle) throws {
499+
let record = try JSON.decode(ABIv0.Record.self, from: recordJSON)
500+
501+
if case let .event(event) = record.kind, let issue = event.issue {
502+
// Translate the issue back into a "real" issue and record it
503+
// in the parent process. This translation is, of course, lossy
504+
// due to the process boundary, but we make a best effort.
505+
let comments: [Comment] = event.messages.compactMap { message in
506+
message.symbol == .details ? Comment(rawValue: message.text) : nil
507+
}
508+
let sourceContext = SourceContext(
509+
backtrace: nil, // `issue._backtrace` will have the wrong address space.
510+
sourceLocation: issue.sourceLocation
350511
)
512+
// TODO: improve fidelity of issue kind reporting (especially those without associated values)
513+
var issueCopy = Issue(kind: .unconditional, comments: comments, sourceContext: sourceContext)
514+
issueCopy.isKnown = issue.isKnown
515+
issueCopy.record()
351516
}
352517
}
353518
}

0 commit comments

Comments
 (0)