Conversation
CatarinaGamboa
left a comment
There was a problem hiding this comment.
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 => |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Fixed in 6ded2e6: changed the fixture on disk between runs and require the renamed variable in the outbound context, so cached context cannot satisfy the manual Verify assertion. Lifecycle tests passed on stable and minimum VS Code.
| const [diagnostics, outboundDiagnostics, outboundContext] = await Promise.all([ | ||
| manualDiagnostics, diagnosticMessage, contextMessage, | ||
| ]); | ||
| assert.deepEqual(outboundDiagnostics.message.diagnostics, diagnostics); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Replaced the self-comparison with independent assertions on outbound diagnostic type, category, title, file, and position in 6ded2e6. The waits also capture the first diagnostics result and fail immediately on verifier crashes.
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
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.