test(integration): stop mcp-resources flaking on subprocess-spawn timeout#377
Merged
Conversation
…eout The beforeAll starts a Runtime, provisions two workspaces, and spawns two Node MCP subprocesses — all under the 5s default hook timeout, which is too tight under CI load. When it times out, afterAll then throws `TypeError: undefined is not an object (handle.stop)`, burying the real cause. Give the hook a 30s timeout and optional-chain every teardown step so a partial setup fails with its true error instead of a misleading teardown one. No production code change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
test/integration/mcp-resources.test.tsflaked in CI (seen on #374's run): thebeforeAllhit the 5s default hook timeout while spawning its MCP subprocesses ("killed 1 dangling process"), and theafterAllthen threwTypeError: undefined is not an object (evaluating 'handle.stop')becausehandlewas never assigned — burying the real cause.That
beforeAlldoes a lot under the 5s budget:Runtime.start, provisions two workspaces, spawns two Node MCP subprocesses (fixtureSource.start()+otherSource.start()), andstartServer. Under CI load the spawn occasionally exceeds 5s.Change (test-only, no production code)
beforeAll/afterAllhooks a 30s timeout — ample headroom for the subprocess spawns without masking a genuine hang.handle?.stop,fixtureSource?.stop,runtime?.shutdown, …) so a partial setup surfaces its true error instead of a misleading teardownTypeError.Verification
bunx tsc --noEmit✅Merge this first; it removes the flake that intermittently reds the integration job (including on #374).