Skip to content

Test webview messages and server lifecycle - #146

Open
rcosta358 wants to merge 2 commits into
codex/issue-132-passing-min-vscodefrom
codex/issue-133-webview-lifecycle
Open

rcosta358 wants to merge 2 commits into
codex/issue-132-passing-min-vscodefrom
codex/issue-133-webview-lifecycle

Conversation

@rcosta358

@rcosta358 rcosta358 commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

Adds real VS Code coverage for webview readiness, diagnostics/context messages, and Stop, Start, and Restart. Checks process termination and verification after Restart, and fixes a shutdown race that could clear the newly started server.

Validated both fixtures locally and in CI on stable and VS Code 1.82.0; lint, types, and installation passed.

Depends on #145. Closes #133.

Generated by Codex.

@rcosta358 rcosta358 added the testing Testing related label Oct 2, 2026

@CatarinaGamboa CatarinaGamboa left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Two assertions here can't catch the regression they're meant to catch.

Reviewed with Claude Code (reviewer + adversarial agents per PR, findings checked against the code before posting).

const diagnosticMessage = nextEvent(api.onWebviewMessage, event =>
event.direction === 'toWebview' && event.message.type === 'diagnostics' &&
isFixtureDiagnostic(event.message.diagnostics));
const contextMessage = nextEvent(api.onWebviewMessage, event =>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This can match the old context. handleLJDiagnostics sends a context message with the cached extension.context from the previous verification, so this listener can resolve on that message, not on the context Verify produces. The fixture doesn't change between runs, so the old and new contexts look the same, and the assertions on valid pass even if Verify stopped sending a context.

Suggest waiting for the context message that follows the liquidjava/context notification for this run, for example by clearing the cached context before Verify, or by matching on something that changes per run.

const [diagnostics, outboundDiagnostics, outboundContext] = await Promise.all([
manualDiagnostics, diagnosticMessage, contextMessage,
]);
assert.deepEqual(outboundDiagnostics.message.diagnostics, diagnostics);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This compares an array with itself. handleLJDiagnostics passes the same diagnostics array to sendMessage and to diagnosticsEmitter.fire, and sendMessage fires onWebviewMessage with that object before postMessage. So outboundDiagnostics.message.diagnostics and diagnostics are the same reference, and deepEqual always passes. To check what the webview receives, compare against a copy taken before sending, or check specific fields such as the error type and file.

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

Labels

testing Testing related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants