test: language-server tests are timing dependent and fail tier A at random #244

Closed
opened 2026-10-09 01:52:14 +00:00 by orion · 8 comments
Collaborator

Problem

The language-server tests in compiler/tests/language-server.test.trb failed intermittently in tier A (linux-x64), and the failing test differed from run to run on the same code. Observed on 2026-10-08/09 (run numbers of the forge CI):

  • run 395 (PR #232 head fef9149): Diagnostics > the findings of the lint rules are hints that name their rule (diagnostics.length() == 2)
  • run 429 (another PR): the same Diagnostics > ... test
  • run 443 (another PR): Quick fixes > the fix of a lint finding is a code action on its range
  • run 432 (PR #232 head 05564c7, only runs-on in images.yml changed): A keystroke in a body keeps the sites and the index of every other module > the first keystroke builds them for every module, ... (runs.length() == 2) and Hover > a function shows its signature and its doc comment (answers.length() == 1; the log shows expected one message, got [publishDiagnostics, hover result])

Cause: the check of an open document runs in slices of 40 ms (stepBudget in compiler/src/language-server/run.trb, docs/design/LANGUAGE-SERVER.md: "the diagnostics are published after the last slice", "a request about a document whose check runs waits for its last slice"). On a loaded runner a slice ran out in the middle of a batch, so the publishDiagnostics of a document and the answer to a request landed in different batches than on an idle machine. A test that counts the messages of a batch (only, length() == 1) then saw two messages or none.

State on main

The time-budget half is fixed: started() in the test file sets server.checkBudget = Int.maximum (commit 2990ac8f, 2026-10-09), so a check a batch begins ends in that batch. Tests that need several steps set checkBudget = 0 on purpose. Since then no failed tier A job has a failing language-server test (details in the decision comment).

What is left of the issue's "Wanted": the helper only still counts the messages of a batch, so any later change that lets a notification share a batch with a response (a new notification, a changed publish order) turns into a random red again, with the same misleading message.

Goal

A test that asks for the answer to a request gets exactly that answer, found by its request id, whatever notifications the same batch carries.

Acceptance criteria

  1. compiler/tests/language-server.test.trb has a helper responseTo(var server, messages, id) that returns the response with that id from the batch, ignoring notifications (publishDiagnostics, logMessage, ...), and fails with the batch printed (as only does) when there is not exactly one response with that id.
  2. Every test that sends a request and reads its answer through only (the hover, definition, completion, rename, code action, resolve, semantic tokens and torbscript/tests tests; request ids 5 to 85 in the file) uses responseTo with its id instead. only stays for the tests whose point is that the batch is exactly one message: initialize, malformed input, a refused request, $/cancelRequest, an unknown method.
  3. One new test shows the difference: a batch that carries a publishDiagnostics of an opened document and the answer to a hover request in the same batch is answered by responseTo with the hover result.
  4. started() keeps checkBudget = Int.maximum and its comment; no test in the file reads an answer by position (answers[0]) or by answers.length() == 1 when its batch holds a request.
  5. sh tools/gates.sh a is green in the forge CI of the PR (the changed file has no other behavioural change; the test count rises by one).

Design record, seed impact, size

  • Design record: docs/design/LANGUAGE-SERVER.md (the check in slices between the messages; behaviour under test, not changed). ROADMAP: Milestone 8, language server; no milestone slice changes.
  • Seed impact: no. Only a test file changes: no syntax, no native, no driver flag, no std name looked up by string.
  • Size: one PR, one Conventional Commit (test(lsp): ...).
## Problem The `language-server` tests in `compiler/tests/language-server.test.trb` failed intermittently in tier A (linux-x64), and the failing test differed from run to run on the same code. Observed on 2026-10-08/09 (run numbers of the forge CI): - run 395 (PR #232 head fef9149): `Diagnostics > the findings of the lint rules are hints that name their rule` (`diagnostics.length() == 2`) - run 429 (another PR): the same `Diagnostics > ...` test - run 443 (another PR): `Quick fixes > the fix of a lint finding is a code action on its range` - run 432 (PR #232 head 05564c7, only `runs-on` in `images.yml` changed): `A keystroke in a body keeps the sites and the index of every other module > the first keystroke builds them for every module, ...` (`runs.length() == 2`) and `Hover > a function shows its signature and its doc comment` (`answers.length() == 1`; the log shows `expected one message, got [publishDiagnostics, hover result]`) Cause: the check of an open document runs in slices of 40 ms (`stepBudget` in `compiler/src/language-server/run.trb`, `docs/design/LANGUAGE-SERVER.md`: "the diagnostics are published after the last slice", "a request about a document whose check runs waits for its last slice"). On a loaded runner a slice ran out in the middle of a batch, so the `publishDiagnostics` of a document and the answer to a request landed in different batches than on an idle machine. A test that counts the messages of a batch (`only`, `length() == 1`) then saw two messages or none. ## State on `main` The time-budget half is fixed: `started()` in the test file sets `server.checkBudget = Int.maximum` (commit 2990ac8f, 2026-10-09), so a check a batch begins ends in that batch. Tests that need several steps set `checkBudget = 0` on purpose. Since then no failed tier A job has a failing language-server test (details in the decision comment). What is left of the issue's "Wanted": the helper `only` still counts the messages of a batch, so any later change that lets a notification share a batch with a response (a new notification, a changed publish order) turns into a random red again, with the same misleading message. ## Goal A test that asks for the answer to a request gets exactly that answer, found by its request id, whatever notifications the same batch carries. ## Acceptance criteria 1. `compiler/tests/language-server.test.trb` has a helper `responseTo(var server, messages, id)` that returns the response with that `id` from the batch, ignoring notifications (`publishDiagnostics`, `logMessage`, ...), and fails with the batch printed (as `only` does) when there is not exactly one response with that id. 2. Every test that sends a request and reads its answer through `only` (the hover, definition, completion, rename, code action, resolve, semantic tokens and `torbscript/tests` tests; request ids 5 to 85 in the file) uses `responseTo` with its id instead. `only` stays for the tests whose point is that the batch is exactly one message: `initialize`, malformed input, a refused request, `$/cancelRequest`, an unknown method. 3. One new test shows the difference: a batch that carries a `publishDiagnostics` of an opened document and the answer to a hover request in the same batch is answered by `responseTo` with the hover result. 4. `started()` keeps `checkBudget = Int.maximum` and its comment; no test in the file reads an answer by position (`answers[0]`) or by `answers.length() == 1` when its batch holds a request. 5. `sh tools/gates.sh a` is green in the forge CI of the PR (the changed file has no other behavioural change; the test count rises by one). ## Design record, seed impact, size - Design record: `docs/design/LANGUAGE-SERVER.md` (the check in slices between the messages; behaviour under test, not changed). ROADMAP: Milestone 8, language server; no milestone slice changes. - Seed impact: no. Only a test file changes: no syntax, no native, no driver flag, no std name looked up by string. - Size: one PR, one Conventional Commit (`test(lsp): ...`).
Author
Collaborator

Label loop added by Orion (orion/infra#1391): the torbscript loop only reads issues with that label, so this open issue was invisible to it (107 other items were picked up). No scope change intended; intake will triage it.

Label `loop` added by Orion (orion/infra#1391): the torbscript loop only reads issues with that label, so this open issue was invisible to it (107 other items were picked up). No scope change intended; intake will triage it.
Author
Collaborator

[loop:torbscript] claim refine by session:loop-torbscript-20261013 until 2026-10-13T00:57Z

[loop:torbscript] claim refine by session:loop-torbscript-20261013 until 2026-10-13T00:57Z
Author
Collaborator

[loop:torbscript] decided by the loop: narrow the issue to hardening the request helper in the test file (responseTo by request id), keep the existing checkBudget = Int.maximum fix, and do not add a clock or a retry. Why: the two causes were separated by measurement. (1) The flake came from the 40 ms slice budget (stepBudget): on a loaded runner a check straddled two batches, so publishDiagnostics and the response of a request shared or split a batch differently than on an idle machine. Commit 2990ac8f (2026-10-09) removed the budget in started(). (2) Evidence from forge CI since that fix reached main (runs from 2026-10-09 13:30Z to 2026-10-12 22:55Z, 396 runs): 178 tier A jobs, 105 success, 33 failure, 33 cancelled, 7 skipped; the logs of all failed jobs (run window from 12:56Z) show 11 distinct failing tests, none of them in language-server.test.trb except one completion-item test in run 602 (2026-10-09 12:56Z), which ran before the fix reached main. Before the fix the issue lists 5 language-server failures in 4 runs within one day. (3) The slices stay (they are the design in docs/design/LANGUAGE-SERVER.md); a fake clock would change the server for a test only, and server.now / checkBudget = 0 already give the deterministic stepping the slice tests need. (4) What remains is cheap and mechanical: 30 uses of only in the file, about 20 of them read the answer of a request; selecting it by id removes the whole class (a stray notification in the batch) rather than the one cause already fixed. Size: one test file, one commit, no seed impact.

[loop:torbscript] decided by the loop: narrow the issue to hardening the request helper in the test file (`responseTo` by request id), keep the existing `checkBudget = Int.maximum` fix, and do not add a clock or a retry. Why: the two causes were separated by measurement. (1) The flake came from the 40 ms slice budget (`stepBudget`): on a loaded runner a check straddled two batches, so `publishDiagnostics` and the response of a request shared or split a batch differently than on an idle machine. Commit 2990ac8f (2026-10-09) removed the budget in `started()`. (2) Evidence from forge CI since that fix reached `main` (runs from 2026-10-09 13:30Z to 2026-10-12 22:55Z, 396 runs): 178 tier A jobs, 105 success, 33 failure, 33 cancelled, 7 skipped; the logs of all failed jobs (run window from 12:56Z) show 11 distinct failing tests, none of them in `language-server.test.trb` except one completion-item test in run 602 (2026-10-09 12:56Z), which ran before the fix reached `main`. Before the fix the issue lists 5 language-server failures in 4 runs within one day. (3) The slices stay (they are the design in `docs/design/LANGUAGE-SERVER.md`); a fake clock would change the server for a test only, and `server.now` / `checkBudget = 0` already give the deterministic stepping the slice tests need. (4) What remains is cheap and mechanical: 30 uses of `only` in the file, about 20 of them read the answer of a request; selecting it by id removes the whole class (a stray notification in the batch) rather than the one cause already fixed. Size: one test file, one commit, no seed impact.
Author
Collaborator

[loop:torbscript] refine: triaged in scope (author orion's issue under label loop, no duplicate, no design-record collision), located the cause in the 40 ms slice budget of the check (already fixed on main by 2990ac8f, confirmed by tier A history), narrowed the remaining work to a responseTo helper by request id in the language-server test file, rewrote the body with goal, five acceptance criteria, design record, seed impact (no) and size (one PR, one commit); about 96k tokens, next: implement

[loop:torbscript] refine: triaged in scope (author orion's issue under label `loop`, no duplicate, no design-record collision), located the cause in the 40 ms slice budget of the check (already fixed on `main` by 2990ac8f, confirmed by tier A history), narrowed the remaining work to a `responseTo` helper by request id in the language-server test file, rewrote the body with goal, five acceptance criteria, design record, seed impact (no) and size (one PR, one commit); about 96k tokens, next: implement
Author
Collaborator

[loop:torbscript] claim implement by session:loop-torbscript-20261013 until 2026-10-13T05:03Z

[loop:torbscript] claim implement by session:loop-torbscript-20261013 until 2026-10-13T05:03Z
Author
Collaborator

[loop:torbscript] release implement: the worker ended on a session limit before opening a PR, next: implement (respawn follows with a fresh claim)

[loop:torbscript] release implement: the worker ended on a session limit before opening a PR, next: implement (respawn follows with a fresh claim)
Author
Collaborator

[loop:torbscript] claim implement by session:loop-torbscript-20261013 until 2026-10-13T09:42Z

[loop:torbscript] claim implement by session:loop-torbscript-20261013 until 2026-10-13T09:42Z
Author
Collaborator

[loop:torbscript] release implement: the worker ended on a session limit after opening PR #356 (head b0410e55); all required forge CI jobs are green and the merge gate's review is requested, next: waiting (merge gate review of PR #356)

[loop:torbscript] release implement: the worker ended on a session limit after opening PR https://git.torb.dev/torbscript/language/pulls/356 (head b0410e55); all required forge CI jobs are green and the merge gate's review is requested, next: waiting (merge gate review of PR #356)
argus closed this issue 2026-10-13 07:45:40 +00:00
Sign in to join this conversation.
No labels
loop
ready
No milestone
No project
No assignees
1 participant
Notifications
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#244
No description provided.