Skip to content

Add withTimeout() to surface hangs as diagnosable failures#267

Open
broken-circle wants to merge 1 commit into
swiftlang:mainfrom
broken-circle:withtimeout-157
Open

Add withTimeout() to surface hangs as diagnosable failures#267
broken-circle wants to merge 1 commit into
swiftlang:mainfrom
broken-circle:withtimeout-157

Conversation

@broken-circle
Copy link
Copy Markdown
Contributor

@broken-circle broken-circle commented May 17, 2026

This PR adds withTimeout() as an internal test helper and applies it to testDoesNotReapUnrelatedChildProcess(), so a hang surfaces as a TimeoutError with the test name and timeout duration instead of consuming the full CI job timeout with no actionable diagnostic.

withTimeout() is adapted from the swift-build implementation as suggested by @jakepetroules. I made description non-optional since every call site in this repo is likely to want a useful description, added defer-based cancelation, and threw directly from group.next(). I can use the exact swift-build version if you'd prefer it.

Addresses #157, which should remain open until the reason for the underlying hang is determined.

Some tests such as `testDoesNotReapUnrelatedChildProcess()` non-
deterministically hang, consuming the full CI job timeout with no
actionable signal. Surface the hang as a `TimeoutError` with the
test name and timeout duration instead.
@broken-circle broken-circle requested a review from iCharlesHu as a code owner May 17, 2026 20:28
@jakepetroules
Copy link
Copy Markdown
Contributor

I actually don't remember why I suggested using the Swift Build code as Swift Testing has had the testTimeout trait for well over a year. @iCharlesHu do you remember what the issue was here?

@broken-circle
Copy link
Copy Markdown
Contributor Author

Yeah, .timeLimit(.minutes(1)) is simpler. I can update the PR to use that and drop the helper. One thing worth noting is the coarse granularity:

Test timeouts do not support high-precision, arbitrarily short durations due to variability in testing environments. You express the duration in minutes, with a minimum duration of one minute.

The 1 minute timeout is fine for this test, but if finer-grained timeouts come up later, the helper would be more useful. I'd say we can drop it now and come back to it later if we need it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants