Skip to content

Interop: send events to fallback event handler if active configuration not present #1439

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
jerryjrchen wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Interop: send events to fallback event handler if active configuration not present #1439

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Sources/Testing/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ add_library(Testing
Attachments/Attachment.swift
Events/Clock.swift
Events/Event.swift
Events/Event+FallbackHandler.swift
Events/Recorder/Event.AdvancedConsoleOutputRecorder.swift
Events/Recorder/Event.ConsoleOutputRecorder.swift
Events/Recorder/Event.HumanReadableOutputRecorder.swift
Expand Down
44 changes: 44 additions & 0 deletions Sources/Testing/Events/Event+FallbackHandler.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2025 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
// See https://swift.org/CONTRIBUTORS.txt for Swift project authors
//

private import _TestingInternals

extension Event {
/// Post this event to the currently-installed fallback event handler.
///
/// - Parameters:
/// - context: The context associated with this event.
///
/// - Returns: Whether or not the fallback event handler was invoked. If the
/// currently-installed handler belongs to the testing library, returns
/// `false`.
borrowing func postToFallbackHandler(in context: borrowing Context) -> Bool {
#if canImport(_TestingInterop)
Copy link
Contributor

@stmontgomery stmontgomery Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style-wise, we usually outdent #if guards to column 0

guard let fallbackEventHandler = _swift_testing_getFallbackEventHandler() else {
return false
}

// Encode the event as JSON and pass it to the handler.
let encodeAndInvoke = ABI.CurrentVersion.eventHandler(encodeAsJSONLines: false) {
Copy link
Contributor

@grynspan grynspan Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might want to cache this event handler somewhere as there's a bit of runtime overhead to generating it. Remember the fallback event handler can't change after its initial assignment, and will be set by the time an event is generated, so we don't need to worry about it having changed on later calls (i.e. the closure we generate here can permanently capture it.)

recordJSON in
fallbackEventHandler(
String(describing: ABI.CurrentVersion.versionNumber),
recordJSON.baseAddress!,
recordJSON.count,
nil
)
}
encodeAndInvoke(self, context)
return true
#else
return false
#endif
}
}
11 changes: 11 additions & 0 deletions Sources/_TestingInternals/include/Stubs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,17 @@ static int swt_setfdflags(int fd, int flags) {
}
#endif

#if !SWT_NO_INTEROP
Copy link
Contributor

@grynspan grynspan Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anywhere you're doing this, I suggest changing it to:

Suggested change
#if !SWT_NO_INTEROP
#if compiler(>=6.3) && !SWT_NO_INTEROP

So that when we build against the 6.2 toolchain, we don't even try to link this function.


typedef void (*FallbackEventHandler)(const char *recordJSONSchemaVersionNumber,
Copy link
Contributor

@stmontgomery stmontgomery Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR's still a draft, but if you stick with this approach, please plan to add some documentation for these (even if it's a cursory explanation of how they shadow functions elsewhere).

Copy link
Contributor

@grynspan grynspan Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
typedef void (*FallbackEventHandler)(const char *recordJSONSchemaVersionNumber,
typedef void (* FallbackEventHandler)(const char *recordJSONSchemaVersionNumber,

The smallest possible nitpick. :)

const void *recordJSONBaseAddress,
long recordJSONByteCount,
const void *_Nullable reserved);

FallbackEventHandler _Nullable _swift_testing_getFallbackEventHandler();
Copy link
Contributor

@grynspan grynspan Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
FallbackEventHandler _Nullable _swift_testing_getFallbackEventHandler();
SWT_EXTERN FallbackEventHandler _Nullable _swift_testing_getFallbackEventHandler(void);

The argument lists () and (void) don't mean the same thing in C. 😱


#endif

SWT_ASSUME_NONNULL_END

#endif
Loading