Skip to content

Commit ef3e4af

Browse files
authored
Fix a regression that prevents saving attachments. (#1400)
This PR ensures that the attachment-saving logic in `Runner` is applied consistently. I made the mistake of moving that logic to `configureEventHandlerRuntimeState()` because I'd forgotten that that function only has an effect when we're running our own tests. And, as a result, all our tests continued to pass because the code was in place for our tests (and for nobody else.) I've moved the code to a different location that we will always call when running a `Runner` instance, so the issue is resolved now. I don't have a great way to set up a unit test for this; tested manually at desk. ### 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 d3b0af5 commit ef3e4af

File tree

3 files changed

+22
-11
lines changed

3 files changed

+22
-11
lines changed

Sources/Testing/Attachments/Attachment.swift

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,26 @@ extension Attachment where AttachableValue: ~Copyable {
501501
}
502502
}
503503

504+
extension Runner {
505+
/// Modify this runner's configured event handler so that it handles "value
506+
/// attached" events and saves attachments where necessary.
507+
mutating func configureAttachmentHandling() {
508+
configuration.eventHandler = { [oldEventHandler = configuration.eventHandler] event, context in
509+
#if !SWT_NO_FILE_IO
510+
var event = copy event
511+
if case .valueAttached = event.kind {
512+
guard let configuration = context.configuration,
513+
configuration.handleValueAttachedEvent(&event, in: context) else {
514+
// The attachment could not be handled, so suppress this event.
515+
return
516+
}
517+
}
518+
oldEventHandler(event, context)
519+
#endif
520+
}
521+
}
522+
}
523+
504524
extension Configuration {
505525
/// Handle the given "value attached" event.
506526
///
@@ -517,7 +537,7 @@ extension Configuration {
517537
/// not need to call it elsewhere. It automatically saves the attachment
518538
/// associated with `event` and modifies `event` to include the path where the
519539
/// attachment was saved.
520-
func handleValueAttachedEvent(_ event: inout Event, in eventContext: borrowing Event.Context) -> Bool {
540+
fileprivate func handleValueAttachedEvent(_ event: inout Event, in eventContext: borrowing Event.Context) -> Bool {
521541
guard let attachmentsPath else {
522542
// If there is no path to which attachments should be written, there's
523543
// nothing to do here. The event handler may still want to handle it.

Sources/Testing/Running/Runner.RuntimeState.swift

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -48,16 +48,6 @@ extension Runner {
4848
}
4949

5050
configuration.eventHandler = { [oldEventHandler = configuration.eventHandler] event, context in
51-
#if !SWT_NO_FILE_IO
52-
var event = copy event
53-
if case .valueAttached = event.kind {
54-
guard let configuration = context.configuration,
55-
configuration.handleValueAttachedEvent(&event, in: context) else {
56-
// The attachment could not be handled, so suppress this event.
57-
return
58-
}
59-
}
60-
#endif
6151
RuntimeState.$current.withValue(existingRuntimeState) {
6252
oldEventHandler(event, context)
6353
}

Sources/Testing/Running/Runner.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,7 @@ extension Runner {
416416
private static func _run(_ runner: Self) async {
417417
var runner = runner
418418
runner.configureEventHandlerRuntimeState()
419+
runner.configureAttachmentHandling()
419420

420421
// Track whether or not any issues were recorded across the entire run.
421422
let issueRecorded = Locked(rawValue: false)

0 commit comments

Comments
 (0)