From 859fadd897ed7335c65edc008a91d27887b147c4 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Thu, 6 Nov 2025 16:02:13 -0500 Subject: [PATCH] Fix a regression that prevents saving attachments. 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. --- Sources/Testing/Attachments/Attachment.swift | 22 ++++++++++++++++++- .../Testing/Running/Runner.RuntimeState.swift | 10 --------- Sources/Testing/Running/Runner.swift | 1 + 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/Sources/Testing/Attachments/Attachment.swift b/Sources/Testing/Attachments/Attachment.swift index b22911919..40d498433 100644 --- a/Sources/Testing/Attachments/Attachment.swift +++ b/Sources/Testing/Attachments/Attachment.swift @@ -501,6 +501,26 @@ extension Attachment where AttachableValue: ~Copyable { } } +extension Runner { + /// Modify this runner's configured event handler so that it handles "value + /// attached" events and saves attachments where necessary. + mutating func configureAttachmentHandling() { + configuration.eventHandler = { [oldEventHandler = configuration.eventHandler] event, context in +#if !SWT_NO_FILE_IO + var event = copy event + if case .valueAttached = event.kind { + guard let configuration = context.configuration, + configuration.handleValueAttachedEvent(&event, in: context) else { + // The attachment could not be handled, so suppress this event. + return + } + } + oldEventHandler(event, context) +#endif + } + } +} + extension Configuration { /// Handle the given "value attached" event. /// @@ -517,7 +537,7 @@ extension Configuration { /// not need to call it elsewhere. It automatically saves the attachment /// associated with `event` and modifies `event` to include the path where the /// attachment was saved. - func handleValueAttachedEvent(_ event: inout Event, in eventContext: borrowing Event.Context) -> Bool { + fileprivate func handleValueAttachedEvent(_ event: inout Event, in eventContext: borrowing Event.Context) -> Bool { guard let attachmentsPath else { // If there is no path to which attachments should be written, there's // nothing to do here. The event handler may still want to handle it. diff --git a/Sources/Testing/Running/Runner.RuntimeState.swift b/Sources/Testing/Running/Runner.RuntimeState.swift index 2d22002c5..eb892da62 100644 --- a/Sources/Testing/Running/Runner.RuntimeState.swift +++ b/Sources/Testing/Running/Runner.RuntimeState.swift @@ -48,16 +48,6 @@ extension Runner { } configuration.eventHandler = { [oldEventHandler = configuration.eventHandler] event, context in -#if !SWT_NO_FILE_IO - var event = copy event - if case .valueAttached = event.kind { - guard let configuration = context.configuration, - configuration.handleValueAttachedEvent(&event, in: context) else { - // The attachment could not be handled, so suppress this event. - return - } - } -#endif RuntimeState.$current.withValue(existingRuntimeState) { oldEventHandler(event, context) } diff --git a/Sources/Testing/Running/Runner.swift b/Sources/Testing/Running/Runner.swift index 1cedf6182..e67b7cd8b 100644 --- a/Sources/Testing/Running/Runner.swift +++ b/Sources/Testing/Running/Runner.swift @@ -416,6 +416,7 @@ extension Runner { private static func _run(_ runner: Self) async { var runner = runner runner.configureEventHandlerRuntimeState() + runner.configureAttachmentHandling() // Track whether or not any issues were recorded across the entire run. let issueRecorded = Locked(rawValue: false)