[Pitch] Issue Handling Traits

Hi all,

Swift Testing offers ways to customize test attributes and perform custom logic using traits, but there’s currently no way to customize how issues (such as #expect failures) are handled when they occur during testing. The ability to handle issues using custom logic would enable test authors to modify, supplement, or filter issues based on their specific requirements before the testing library processes them.

I propose introducing a built-in trait for handling issues in Swift Testing, enabling test authors to customize how issues recorded by tests are represented. Here's one contrived example:

@Test(.transformIssues { issue in
  var issue = issue
  issue.comments.append("Checking whether two literals are equal")
  return issue
})
func literalComparisons() {
  #expect(1 == 1)     // ✅
  #expect(2 == 3)     // ❌ Will invoke issue handler
  #expect("a" == "b") // ❌ Will invoke issue handler again
}

For details and more realistic use case examples, see my draft proposal: Issue Handling Traits.

I'd love to hear your feedback! I'm particularly interested in input on the names of the proposed APIs.

Thanks,
Stuart

2 Likes

Can the issue be passed to the closure as inout instead? That would allow the body to be a bit more concise, and possibly also remove unnecessary copies for CoW things like the comments array:

@Test(.transformIssues { issue in
  issue.comments.append("...")
})
5 Likes

I considered that, and we could still consider including an overload where the parameter is inout. However I didn't prioritize that because I feel it's important to support suppressing/ignoring an issue, and that means the type would need to be Optional. If it's optional, then each access of the inout parameter would need to first unwrap the optional, and that felt non-ideal.

2 Likes

Why transform instead of map and/or compactMap?

edit: Ah, it’s mentioned in the alternatives. I think it’s quite common for compactMap’s type to be constrained by context, and this is no different.

2 Likes

I think there are some constraints we'd want to put on the sort of changes you can make to an issue (in particular, transmuting its kind property is rife with peril).

Is this something that could be implemented as a simple function akin to withKnownIssue {}? Then it could be used within a test body and could be composed into a trait with TestScoping (which I suspect third-party libraries will want to do.)

1 Like

I have a need for a filtering solution for the library I'm working on, but I'm unsure about making it available as a trait (particularly with an attached function body, which will become part of the @Test macro).

For my use-case, I would need to use this code:

Trait.transformIssues({ issue in
    ... 
}).provideScope(for: Test.current!, testCase: Test.Case.current) {
    ...
}

So I agree with Grynspan that it might be better to make this logic only available as a function.

It seems neater to me if only specific handling implementations were available as Traits, to keep the Test declaration succinct. These additional traits could be written by other 3rd parties using scope providers.

1 Like

I think some modifications to the kind property are safe and reasonable; I gave one example use case in the proposal: changing .expectationFailed to .unconditional. That ought to be permitted.

I'm guessing your concern is around cases of the Issue.Kind enum which are intended for use by the testing library itself though, such as .system or .apiMisused. It'd be helpful if you could elaborate on what specific problems you think could arise and what restrictions should be placed on that property. If we do place restrictions on issue modifications, I'd like to have them discussed in the proposal and included in the documentation.

Thanks, both of you. I agree it'd be reasonable to offer this as a standalone function so it could be applied to small sections of a test. I do think it would still make sense to offer it as a trait as well, since I've observed this kind of workflow (applying to a whole test, or suite) being common in other testing libraries. I think both could be included, so I may expand and re-title the proposal to something broader like "Custom Issue Handling".

2 Likes

Re: the standalone function idea, it occurred to me that might be a generalizable trait feature. I started a distinct thread to discuss it here: Generic utility for invoking a TestScoping trait

1 Like

Those two cases are definitely ones I want to keep reserved for use by Swift Testing, at least for the moment. I care somewhat less about being able to transmute e.g. expectationFailed to unconditional. I'm more concerned about future issue kinds that we haven't designed yet, but which by their nature are coupled to the environment (Xcode, VS Code, etc.) or which carry important associated data.

Off the top of my head (and this is not something I'm saying we should or will implement, it's just an example), let's say we add a memoryLimitReached case on iOS/watchOS/tvOS/visionOS; if developer code is able to naïvely transmute that case to unconditional, they might end up missing a memory leak that jetsams the test process.

This is not to say that transmutation of issues is always a bad idea. But, as with many new/experimental features/ideas/whatnots, I'd rather our initial implementation be conservative in nature. We can always add functionality later, but removing functionality is often impossible.

1 Like

On that note, I wouldn't mind if some issue kinds weren't passed through this new handler. Expectation failures and thrown errors from user space are the most important for my case, and other library errors should probably be preserved and shown as-is.

2 Likes

Of the current set of issue "kinds" (enumerated in Issue.Kind), only one strikes me as something we might want to omit from this interface: .system. The others generally do stem from something which is under the users' control. In general, I think it's a reasonable suggestion that only issues which may somehow be caused by a user should be included, so I'll incorporate this suggestion into the proposal.

1 Like

If we decide that certain kinds of issue transmutations should be forbidden because of potential harmful effects they may cause, we'll need to decide how exactly to enforce that. It'll likely need to be done at runtime—for example either by ignoring the illegal modification and recording a secondary issue describing the violation, or perhaps by triggering a fatal error or something more severe.

Given that I plan to incorporate @x-sheep's suggestion and bypass sending .system issues through this interface, I think the practical concern may be less to begin with. In general, if there is a non-.system issue, it should be the result of some action the user took, so I'm not sure we should prohibit modifications of that class of issues.

Looking to the future, supposing we did implement something like a .memoryLimitReached issue kind (again, hypothetical for example purposes only), my sense is that any behavioral effects of such an issue should be limited to reporting only. An Issue isn't the same as an Event: if, in the future, we implement some new feature which has important observable downstream effects on the overall testing system, more than just recording that something happened for later analysis, then such a feature should probably be represented as a distinct Event kind, possibly in addition to an Issue kind. This proposal is only intended to allow reacting to Issues, not any arbitrary Event of which .issueRecorded is but one kind.

Thanks @beccadax! Since you brought it up, and I came in to this discussion with not very strong feelings about the spellings of these APIs, I think I'll probably go ahead and rename transformIssues(_:) to compactMapIssues(_:).

Others following along, if you support this or have alternate naming suggestions please chime in!

1 Like

And assert if an issue is transmuted to .system, yes? :grin:

Is assert (i.e. a process termination) your suggested way to deal with such a situation? We could, or I had an alternate idea above (emphasized):

I can be convinced otherwise (the secondary issue ought to be .apiMisused), but in this case it would have to be a deliberate attempt to create a .system issue by calling issue.kind = .system, whereas if it were something less clear-cut, an issue might make sense.

Of course, generating a secondary issue runs the risk of recursively generating issues that get turned into .system issues incorrectly.

1 Like

The proposal already prevents that by only passing any issues recorded by the closure of an issue handling trait to subsequent issue handling traits. See the Recording issues from handlers section in the proposal.

1 Like