If attach() is an instance member on Attachment I think it could much more easily be modified in the future to always use the "nearest" activity, falling back to the Test last.
If the plan to have this kind of inferred attachment scoping then would Test.attach not work because of the name, or a technical constraint?
Thanks for reminding us of the future direction here, @smontgomery! I agree that only having Test.attach will limit developers - at least conceptually - from adoption of some future Activity API.
I also agree with @plemarquand about the fact that Attachment is conceptually just a collection of data with some metadata. The only action is pinning it to something else, and I do agree that pinning the attachment to something else should be a method on that something else.
In addition, I like the semantics of developers being able to specifically pick which thing they attach their attachment to. For example, what if I want an attachment to go onto the overall Test and not a specific Activity? Or what if I want it to be attached to a previous Test/Activity? A common use case here is long-running asynchronous code that might not complete until after the current activity ends - which is also a common source of test pollution. Being able to associate that data with the correct test/activity helps massively to track down and fix that error.
To that end, I like the idea of Test.attach(myAttachment), Activity.attach(myAttachment), etc. Which is telling me that perhaps a protocol which specifies the attach method is a good approach?
It would be possible, yes. I would tend to want to give the shorthand option as few bells and whistles as possible, lest we find ourselves in the future wishing we'd added X but not Y. So something like:
extension Test {
/// Uses a default preferred name and, in the future, default lifetime etc.
public static func attach<A>(
_ attachableValue: consuming A,
sourceLocation: SourceLocation = #_sourceLocation
) where A: Attachable & ~Copyable
/// As above, but for attachments that can defer serialization.
public static func attach<A>(
_ attachableValue: consuming A,
sourceLocation: SourceLocation = #_sourceLocation
) where A: Attachable & Sendable & Copyable
}
If a custom preferred name or other property is desired, we would tell a developer to create an Attachment instance and mutate its properties, then pass it to Test.attach(_:sourceLocation:).
The reply that jumps to mind, "but being able to specify the preferred name inline is so useful!", goes back to my earlier comment about wanting to add other bells and whistles later. I don't want to find us in the position of having a bunch of overloads of this function over time as we add more properties.
I suspect we don't want to add a convenience function for attaching files because it would obscure the fact that the file needs to be read from disk and possibly compressed, either of which is an asynchronous operation. So I think for the URL extension in the Foundation cross-import overlay, we'd want to keep it as an Attachment initializer to enforce that something is being created first. Does that make sense?
Strictly speaking, the event handler is doing the work.
At this time, since we haven't designed an activity API yet, I don't know what it would be shaped like. We might want to make it ~Escapable, in which case the above interfaces would be sound. If it can escape the context that created it however, it's possible for somebody to try to attach to an activity that has ended, which is generally not good and can be difficult to handle correctly.
An additional future direction that I would love to see listed is support for adding attachments to Issues. That is something that I see people use frequently in XCTest for augmenting failures with extra diagnostics, particularly in integration tests, and I think we'll want to make that possible that in Swift Testing as well.
I've updated the pitch so that the attach function is now a static member on Attachment and has an overload that takes an Attachable, so:
Attachment.attach(someValue)
Or:
var attachment = Attachment(someValue)
attachment.hasGrommet = true
Attachment.attach(attachment)
My one concern is that it repeats the word "attach" twice, so I'm open to other verbs here. @smontgomery suggested record() similar to what we use with Issue.
My one concern is that it repeats the word "attach" twice, so I'm open to other verbs here. @smontgomery suggested record() similar to what we use with Issue.
I like record() here, having similar naming concepts for similar actions reduces the mental overhead when using these APIs.