Skip to content

fix: stabilize Windows session and terminal tests - #1844

Merged
eitsupi merged 5 commits into
REditorSupport:mainfrom
eitsupi:fix/windows-ci-flaky-tests
Oct 7, 2026
Merged

eitsupi merged 5 commits into
REditorSupport:mainfrom
eitsupi:fix/windows-ci-flaky-tests

Conversation

@eitsupi

@eitsupi eitsupi commented Oct 7, 2026 •

Copy link
Copy Markdown
Member

Windows session tests could resend plot_latest after a local RPC timeout while the R handler was still running, and the standard viewer could send additional rendering requests for updates and resize events. Coalesce viewer requests into one in-flight request and one replaceable pending render, retaining the latest dimensions and binding each render to its requesting session and Webview generation. Discard stale queued renders, responses, and resize events instead of retargeting them to a newly active session. A failed request suppresses retries for its own session while preserving a pending render for a different session that is still active.

The plot integration test now waits for the real viewer's Webview update and delivery result instead of issuing its own RPC retries or sleeping. Move PNG/SVG/fallback checks into sess tinytests. Synchronize the out-of-order terminal PID test with completion of the actual discovery-file write. Add deterministic process command/parsing coverage and make only the Windows real-CIM smoke test opt-in with VSCODE_R_TEST_WINDOWS_PROCESS_ANCESTRY=1; Linux/macOS real-process coverage remains enabled. Production timeouts and CI workflows are unchanged.

@eitsupi
eitsupi requested a review from renkun-ken October 7, 2026 02:09
@eitsupi
eitsupi marked this pull request as ready for review October 7, 2026 02:09

@renkun-ken renkun-ken left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed b281d83. I found one request-coordination regression when the active session changes during a failed render; details are inline.

Validation: TypeScript compilation/type checking and lint pass; all 14 new R rendering assertions pass. The 7 viewer-coordination tests and 6 process-ancestry tests pass in a focused Node harness (including the real macOS process test). An additional deterministic session-switch test fails: session B receives no plot_latest request after session A's in-flight request resolves undefined. GitHub CI is green on Windows, macOS, and Linux. I did not rerun the full VS Code extension integration suite locally.

Comment thread src/plotViewer/standardViewer.ts Outdated
@eitsupi
eitsupi marked this pull request as draft October 7, 2026 02:27
@eitsupi
eitsupi marked this pull request as ready for review October 7, 2026 02:37
@eitsupi
eitsupi requested a review from renkun-ken October 7, 2026 02:37

@renkun-ken renkun-ken left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the latest commit, 59354fd. The previous P2 finding is addressed: a queued render retains its captured session and panel generation, and a failed old-session request preserves a queued render for a different session that remains active. Stale queued renders are discarded without retargeting, and same-session failures still suppress automatic retries. I found no remaining actionable issues.

Validation: TypeScript compilation/type checking, lint of the changed files, and diff whitespace checks pass. All 12 viewer-coordination tests and 6 process-ancestry tests pass in the focused Node harness, including the real macOS process test. My original failing reproduction now passes; three additional checks covering synchronous requester reentrancy, events during drain completion, and stale-session handoff also pass (22 checks total). GitHub CI is green on Windows, macOS, and Linux. I did not rerun the full VS Code extension integration suite locally.

@eitsupi
eitsupi merged commit 5ddebde into REditorSupport:main Oct 7, 2026
5 checks passed
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