Add diagnostics for issues recorded after their associated test has finished

[wrdowney]

Adds a warning diagnostic message for issues that are recorded after their associated test has finished.

Resolves #1283

Motivation:

Currently, Issues can be recorded after a test has finished. This commonly occurs when asynchronous work completes after the test its associated test has ended. While responsibility falls on the user in this scenario, clear messaging can point them in the correct direction.

From the issue:

One common way this can occur is when a test uses an unstructured Task and does not await its value to ensure it finishes before the Test function returns. Here's an example:

@Test func example() async throws {
  Task {
    try await Task.sleep(for: .seconds(1))
    Issue.record("Late")
  }
  // Didn't 'await' that Task!
}

@Test func longRunning() async throws {
  try await Task.sleep(for: .seconds(5))
}

This outputs the following:

◇ Test longRunning() started.
◇ Test example() started.
✔ Test example() passed after 0.001 seconds.
✘ Test example() recorded an issue at Example.swift:6:17: Issue recorded
↳ Late
✔ Test longRunning() passed after 5.194 seconds.
✘ Test run with 2 tests in 0 suites failed after 5.194 seconds with 1 issue.

Modifications:

• Adds a boolean flag hasFinished to Test.Case along with a corresponding backing class that stores whether the test case has finished executing. This flag defaults to false
• When a test case finishes, the hasFinished flag is set to true
• Added a check when events are posted. If the event is has kind issueRecorded and the testCase has finished, we emit another issueRecorded event.
• Added a test case where a task records an issue after it's corresponding test case has finished, validating that both the original and late issue are recorded.

Checklist:

• Code and documentation should follow the style of the Style Guide.
• If public symbols are renamed or modified, DocC references should be updated.

[grynspan]

I'm not sure this is the right abstraction. A problem of this form should probably itself be surfaced as an issue (of kind .apiMisused maybe? Or a new kind if needed, although those are more of a pain to add.)

Just adding it to the console output means that, for instance, somebody using Xcode or VS Code won't see anything about the problem unless they go digging in the log file.

[wrdowney]

Thanks for the feedback! I agree about surfacing through issues, thanks for pointing that out. I'm working on implementing that now and looking for a little guidance on the following questions:

1. Are there existing APIs that I might be able to leverage to determine if a test case is current executing? I've thought about using Test.current but I believe that will incorrectly report issues emitted from detached tasks as "late". I want to avoid having to store a set of currently executing test cases.
2. Right now I'm looking at posting a second issueRecorded event from Event.post() for an apiMisuse Issue when an Issue is recorded after test case completion, are there concerns with posting two issues from one call to Event.post()?

[grynspan]

1. Are there existing APIs that I might be able to leverage to determine if a test case is current executing? I've thought about using Test.current but I believe that will incorrectly report issues emitted from detached tasks as "late". I want to avoid having to store a set of currently executing test cases.

If an issue is recorded from within a detached task that was originated by a specific test, we would expect that issue to be associated with that test (if we can figure out that it came from that test, which is hard.)

You shouldn't need to ever store a list of "current" tests anywhere.

2. Right now I'm looking at posting a second issueRecorded event from Event.post() for an apiMisuse Issue when an Issue is recorded after test case completion, are there concerns with posting two issues from one call to Event.post()?

The only immediate concern(s) I have are that:

a. We would need to guard against accidentally recursing, i.e. an issue of this specific flavour, not necessarily all .apiMisused issues, should not trigger its own "late" issue; and
b. We don't want to go off and try to recompute a bunch of state we computed for the first issue (e.g. source location info, current configuration) if we can avoid it.

For a., an internal-only isLate: Bool property would be sufficient, I think.

[wrdowney]

I have a working (albeit a little rough) solution for emitting a second recordIssue event when an issue is recorded after test completion. But I'm now running into the following issue in Xcode: recording an issue after test completion results in an internal inconsistency error .

@Suite struct ExampleTests {
  @Test func example() async throws {
    Task {
      try await Task.sleep(for: .seconds(1))
      Issue.record("Late")
    }
  }
  @Test func longRunning() async throws {
    try await Task.sleep(for: .seconds(5))
  }
}

Task 2: Fatal error: Internal inconsistency: No test reporter for test TestingTests.ExampleTests/example()/IssueTests.swift:1840:4 and test case Non-parameterized test case ID. Issue: Issue recorded (error): Late (at IssueTests.swift:1843:19)

This is run on the current main using Xcode 26.3

Unfortunately the error is thrown after the initial issue is recorded, thus any subsequent issues are neither logged to the console or recorded in editor.

I see #480 which fixes a similar issue for detached tasks. But those fixes don't seem to resolve this case. This seems to be an Xcode issue (I have confirmed running via CLI and VSCode extension does not throw an error). Although I do recognize this is quite the unique use case.

Assuming I'm correct here, do we want to wait on a potential Xcode update prior to this work or am I able to continue here and for now the second issue(regarding the late issue recording) will only be logged when running in environments outside of Xcode?

I wanted to validate I'm not missing anything here before filing a feedback.

[grynspan]

The crash you're seeing should be fixed in the Xcode 26.4 Beta. Our main branch de facto requires Xcode 26.4 (our release/6.3 branch works with earlier Xcode 26.x releases).

[wrdowney]

I ended up going with a slightly different approach by setting a flag on the test case. My main concern is that this approach might be too heavy but it's the simplest logic I could think of. I also call the private post method when emitting the second event so that should prevent any issues with accidental recursion or recomputing state.

[grynspan]

So far we've avoided adding mutable state to Test or Test.Case. They get copied around a lot and having each instance carry a heap-allocated mutex with it is a lot of potential overhead.

Something you might want to try instead… install an event handler somewhere under Runner.run() that listens for .testEnded and .testCaseEnded events and tracks the set of ended tests/cases. If it then gets .issueRecorded for a test/case that was already reported as ended, it can raise a secondary issue at that point. This then only requires a single lock-guarded Set.

Something like:

enum FinishedItem: Sendable, Equatable, Hashable {
  case test(Test.ID)
  case testCase(Test.Case.ID)
}

let finishedItems = Allocated(Mutex<Set<FinishedItem>>([]))
configuration.eventHandler = { [oldEventHandler = configuration.eventHandler] event, eventContext in
  switch event.kind {
  case .testEnded:
    finishedItems.value.withLock { finishedItems in
      _ = finishedItems.insert(.test(event.testID!))
    }
  case .testCaseEnded:
    finishedItems.value.withLock { finishedItems in
      _ = finishedItems.insert(.testCase(event.testCaseID!))
    }
  case .issueRecorded:
    let finishedItems = finishedItems.value.rawValue
    if let testCaseID = event.testCaseID, finishedItems.contains(.testCase(testCaseID)) {
      // Bad test case
    } else if let testID = event.testID, finishedItems.contains(.test(testID)) {
      // Bad test
    }
  default:
    break
  }
  oldEventHandler(event, eventContext)
}

Note I haven't tried to optimize this code or clean up its structure, and I'm not saying you must implement exactly what I've typed here. This is just an idea about how to shoehorn this functionality into the existing architecture.