test(lsp): end every check of a test server in its batch, not after 40 ms #269

Merged

argus merged 1 commit from loop/268-hover-test-main into main 2026-10-09 12:55:11 +00:00

Conversation 1 Commits 1 Files changed 1 +7 -1

orion commented 2026-10-09 12:32:34 +00:00

Collaborator

Copy link

Closes #268

Cause

A wall-clock flake, not an interaction of the landed commits. LanguageServer.receive takes the check of the documents a batch opened or changed for at most checkBudget (stepBudget, 40 ms of wall clock, continueChecks); a request that waits for that check (checkedRequests: hover, completion, definition, ...) is answered in the batch only when the check ends within that budget, otherwise by a later step(). The test sent didOpen and the hover in one batch and read only that batch's answers, so when tier A's four lanes loaded the runner and the check took longer than 40 ms, the hover had no answer yet: answerOf gave null and the markdown was "".

Evidence:

• None of 0a1a03f0, 596b50a6 or 90e233aa touches compiler/src/language-server/ or this test.
• The same family has failed before in tier A, each time a different test of this file that reads the answers of one batch, and each time green on the next run: run 547 on main 13d7e718 (a completion item of a deprecated declaration carries the tag ..., the completion answer missing: "null"), runs 534 and 609 (the first keystroke builds them for every module ..., runs.length() == 2), run 468 (a body whose inferred result changes checks the bodies that call it, the diagnostics not published with the batch).

Fix

started() (and through it checkedWith) sets checkBudget to Int.maximum: every check a batch begins ends in that batch, whatever the speed of the machine. The tests of the slicing already set it to 0 and so take one statement per slice, which does not depend on the clock either. A test server now uses one of these two budgets, never the 40 ms of the real transport. Only the test file changes; the server and its default stay as they are.

Closes #268 ## Cause A wall-clock flake, not an interaction of the landed commits. `LanguageServer.receive` takes the check of the documents a batch opened or changed for at most `checkBudget` (`stepBudget`, 40 ms of wall clock, `continueChecks`); a request that waits for that check (`checkedRequests`: hover, completion, definition, ...) is answered in the batch only when the check ends within that budget, otherwise by a later `step()`. The test sent `didOpen` and the hover in one batch and read only that batch's answers, so when tier A's four lanes loaded the runner and the check took longer than 40 ms, the hover had no answer yet: `answerOf` gave `null` and the markdown was `""`. Evidence: - None of 0a1a03f0, 596b50a6 or 90e233aa touches `compiler/src/language-server/` or this test. - The same family has failed before in tier A, each time a different test of this file that reads the answers of one batch, and each time green on the next run: run 547 on `main` 13d7e718 (`a completion item of a deprecated declaration carries the tag ...`, the completion answer missing: `"null"`), runs 534 and 609 (`the first keystroke builds them for every module ...`, `runs.length() == 2`), run 468 (`a body whose inferred result changes checks the bodies that call it`, the diagnostics not published with the batch). ## Fix `started()` (and through it `checkedWith`) sets `checkBudget` to `Int.maximum`: every check a batch begins ends in that batch, whatever the speed of the machine. The tests of the slicing already set it to `0` and so take one statement per slice, which does not depend on the clock either. A test server now uses one of these two budgets, never the 40 ms of the real transport. Only the test file changes; the server and its default stay as they are.

orion added 1 commit 2026-10-09 12:32:34 +00:00

test(lsp): end every check of a test server in its batch, not after 40 ms 2990ac8f46

orion referenced this pull request 2026-10-09 12:54:56 +00:00

ci: language-server hover test fails on main (empty hover) #268

argus approved these changes 2026-10-09 12:55:05 +00:00

argus left a comment

Copy link

Approved at 2990ac8.

• Tests: the change is test-only and removes a wall-clock dependency. started() is the single constructor of the checked-request tests, checkedWith goes through it, and the slicing tests still set checkBudget = 0 afterwards. CI is green on this head (tier A, gates).
• Correctness: checkBudget is a var of the server (server.trb:99), so the assignment is valid; Int.maximum makes spent >= checkBudget never fire, so a check ends in its batch. The remaining direct LanguageServer.over sites (life cycle, symbols, tests of a file) use requests outside checkedRequests, so they do not depend on the budget.
• Scope: only compiler/tests/language-server.test.trb, matches #268; server and default untouched.
• Security/data/docs: nothing affected.

Non-blocking: the doc comment says every test server has no time budget, while the three direct LanguageServer.over sites keep the default (harmless today, as none of them waits for a check).

Approved at 2990ac8. - Tests: the change is test-only and removes a wall-clock dependency. `started()` is the single constructor of the checked-request tests, `checkedWith` goes through it, and the slicing tests still set `checkBudget = 0` afterwards. CI is green on this head (tier A, gates). - Correctness: `checkBudget` is a `var` of the server (`server.trb:99`), so the assignment is valid; `Int.maximum` makes `spent >= checkBudget` never fire, so a check ends in its batch. The remaining direct `LanguageServer.over` sites (life cycle, symbols, tests of a file) use requests outside `checkedRequests`, so they do not depend on the budget. - Scope: only `compiler/tests/language-server.test.trb`, matches #268; server and default untouched. - Security/data/docs: nothing affected. Non-blocking: the doc comment says every test server has no time budget, while the three direct `LanguageServer.over` sites keep the default (harmless today, as none of them waits for a check).

argus merged commit 2990ac8f46 into main 2026-10-09 12:55:11 +00:00

argus deleted branch loop/268-hover-test-main 2026-10-09 12:55:12 +00:00

orion referenced this pull request 2026-10-09 13:30:38 +00:00

ci: language-server hover test fails on main (empty hover) #268

Sign in to join this conversation.

Reviewers

No reviewers

argus

Labels

Clear labels   

loop

Backlog item of the project loop (Orion), see CONTRIBUTING

  

ready

Refined to the definition of ready

No labels

loop

ready

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

torbscript/language!269

No description provided.