[Fix] Subtask e2e suite can inherit a cancelled delayed mock stream from the previous test - #1074
Draft
zoomote[bot] wants to merge 1 commit into
Draft
[Fix] Subtask e2e suite can inherit a cancelled delayed mock stream from the previous test#1074zoomote[bot] wants to merge 1 commit into
zoomote[bot] wants to merge 1 commit into
Conversation
The API-hang subtask fixture used aimock's flat latency, which applies per SSE chunk and is never interrupted by client disconnects, so a cancelled delayed stream stayed pending server-side for chunks x latency and could flush into the next test's traffic. - delay only the first chunk via streamingProfile.ttft so the pending window is exactly the shared SUBTASK_API_HANG_RESPONSE_LATENCY_MS - anchor the post-test drain to the request's aimock journal timestamp and wait out only the remainder of that bounded window
Contributor
Author
|
No code issues found. See task Reviewed both changed files in context and verified the core claims against the installed aimock 1.35.0 internals:
Reviewed b3f3347 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
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.
Related GitHub Issue
Split out from #835 (test-stabilization portion only; no production telemetry changes). Independent from
main— does not depend on any of the other split PRs.Description
Stabilizes the delayed-stream subtask e2e tests in
apps/vscode-e2e, test-only.The race being prevented: the API-hang child fixture delayed its mocked response with aimock's flat
latency: 15_000. aimock applies flatlatencyto every SSE chunk and never observes client disconnects (writeSSEStreamonly honors the fixture's own interruption signal). When the test cancels the child's in-flight request, the mock keeps the stream pending server-side for chunks × latency (~75s for this 5-chunk response) and then flushes the remaining chunks to the dead socket. A streamed request opened by the next test can interleave with that late flush — so one test's unfinished delayed request leaked into the next test.The fix, in two parts:
fixtures/subtasks.ts— the fixture now delays only the first chunk viastreamingProfile: { ttft: SUBTASK_API_HANG_RESPONSE_LATENCY_MS }. With no flatlatencyset, chunks after the first get ~0 delay, so the server-side pending window is exactly one named constant instead of an unbounded chunks × latency. Observable pre-cancel behavior is identical (no bytes for 15s), so the test scenario is preserved.suite/subtasks.test.ts— the API-hang test captures the request's aimock journal timestamp (journal entries are stamped when the request is received, before the delayed write begins) and, infinally, drains viawaitForDelayedSubtaskStreamDrain: it waits only the remaining time untiltimestamp + SUBTASK_API_HANG_RESPONSE_LATENCY_MS + 500ms grace.Why this synchronization point is reliable: the deadline is anchored to the journal timestamp of the exact request this test created (scenario-unique marker + parent-marker exclusion, matched before cancellation, so it can only be the original hung request — never a stale or resumed one). The wait is bounded by one latency window plus grace, so it cannot hide a genuine hang; it no-ops if the delayed request never reached the mock. This replaces the arbitrary fixed
sleep(latency)/ blanket per-testsleep(1_500)teardown approach with an explicit, bounded drain.Production files deliberately untouched: all of
src/,webview-ui/,packages/— including every telemetry behavior from #835 (consent, circuit breaker, shutdown, task aggregation, tool attribution, model-cache).bedrock-mock-server.tswas also left untouched: the subtask suite routes through aimock, not the bedrock mock, and aimock's journal already exposes the needed synchronization point.Test Procedure
TEST_FILE=subtasks.test.js xvfb-run -a pnpm --filter @roo-code/vscode-e2e test:ci:mock— 11/11 passing, run twice consecutively; the API-hung test completes in a deterministic ~15.8s (one latency window).npx tsc -p apps/vscode-e2e/tsconfig.json --noEmit— clean.pnpm --dir apps/vscode-e2e exec eslint --prune-suppressions --max-warnings=0 src/fixtures/subtasks.ts src/suite/subtasks.test.ts— clean, no suppression-count increases.pnpm test:zoo-code(src) 7190 passed. One unrelated pre-existing flake inwebview-ui(SlashCommandsSettings.spec.tsx› "refreshes commands after creating new command") failed under the full parallel run but passes in isolation; that package is untouched by this diff.Pre-Submission Checklist
*.visual.tsxsnapshot inwebview-ui/. Seewebview-ui/AGENTS.md→ "When a UI change needs a snapshot".Visual Snapshots
N/A — test-only change, no UI surface.
Videos (interaction / animation only)
N/A.
Documentation Updates
Additional Notes
Success criterion: repeated runs of the subtask suite do not inherit delayed mock activity from the preceding test — verified with two consecutive clean mock-mode runs.
Get in Touch
edelauna