Conversation
CatarinaGamboa
left a comment
CatarinaGamboa
left a comment
There was a problem hiding this comment.
Three things, mostly about what happens when something goes wrong.
Reviewed with Claude Code (reviewer + adversarial agents per PR, findings checked against the code before posting).
| const nextFixtureDiagnostics = () => new Promise<LJDiagnostic[]>((resolve) => { | ||
| const subscription = api.onDiagnostics((diagnostics) => { | ||
| if (diagnostics.some(d => d.type === 'refinement-error' && path.resolve(d.file) === uri.fsPath)) { | ||
| const matches = passing ? diagnostics.length === 0 |
There was a problem hiding this comment.
A regression shows up as a bare 120s timeout. This wait only resolves when the result is the expected one (empty for passing, a refinement error for failing). If the passing fixture starts getting an error, or the failing one gets none, the promise never resolves. The asserts below (including assert.deepEqual(diagnostics, []) on line 46, which can't fail) are never reached, and the failure is a timeout with no diagnostics in the output.
Suggest resolving on the first diagnostics notification for this run and asserting on it, so a failure prints what actually came back.
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| version: ${{ (github.event_name == 'pull_request' || github.ref == 'refs/heads/main') && fromJSON('["stable", "minimum"]') || fromJSON('["stable"]') }} |
There was a problem hiding this comment.
Two things with this job:
- This PR removes the fork-only
ifthat Add real VS Code integration smoke test #144 had onintegration. So for same-repo PRs every push runs it twice: the push run (stable), plus the PR run (stable + minimum). - Release tags only test
stable.publish.ymlcalls this workflow with eventpushand refrefs/tags/v*, so neither branch of this condition matches. Releases then never test the minimum supported VS Code version.
Suggest bringing the if back, but still letting minimum run for PRs and tags. For example, put the decision in the matrix and the if, so that pushes to branches run stable, while pushes to main, tags (startsWith(github.ref, 'refs/tags/')) and fork PRs run both.
2 participants
Add an isolated passing workspace and require an explicit empty diagnostic result after Verify. Test both fixtures on stable for every branch and on the minimum supported VS Code for PRs and main; pin typings to 1.82.x.
Validated both fixtures locally and in CI on stable and 1.82.0; lint, types, and extension installation passed.
Depends on #144. Closes #132.
Generated by Codex.