Fix flaky //compiler/lsp-tests:lsp-tests script interrupt tests - #23281
Fix flaky //compiler/lsp-tests:lsp-tests script interrupt tests#23281roger-bosman-da wants to merge 2 commits into
Conversation
The "scenario service does not interrupt on non-script messages" test uses an interpreted `foldl (+) 0 [1..N]` to create a long-running script, then sends a hover request during execution to verify the hover doesn't cancel the running script. The previous fold size of 10,000,000 took ~30s on a fast machine but could exceed the 60s timeout on loaded m1 CI runners — the only platform where this test flaked (see #20724). The fold just needs to run long enough for the hover to land during execution; it doesn't need to be maximally expensive. Changes: - Reduce fold size from 10M to 1M (~3s locally, ~6-10s under CI load) - Reduce timeout from 60s to 30s (1M should finish well within this) - Add timing instrumentation: measure time from hover-sent to script-finished, and assert it took at least 0.5s — proving the script was still running when the hover was processed, not that it finished before the hover arrived - Log elapsed time in test steps for future debugging run-all-tests: true Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Roger Bosman <roger.bosman@digitalasset.com>
Both "interrupts outdated script runs" and "does not interrupt on non-script messages" used a 10M-element fold. The "interrupts" test also flaked on linux arm and linux intel CI (not just m1) — the 30s timeout had to cover compilation + code lenses + script start + fold evaluation, and the 10M fold alone took 25-43s on loaded runners. Changes: - Reduce the "interrupts" test fold from 10M to 1M (same as "does not interrupt") - Add upper-bound timing guard to both tests: fail deterministically if total test time exceeds 50% of the timeout, with a message explaining the test is at risk of flaking and suggesting to reduce fold size or investigate slowness - The "does not interrupt" test keeps its existing lower-bound guard (afterHover >= 0.5s) proving the script was still running when the hover was processed On this machine, 1M fold gives total times of ~3s and ~5s against a 30s timeout — well within the 50% safety margin even under CI load. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Roger Bosman <roger.bosman@digitalasset.com>
samuel-williams-da
left a comment
There was a problem hiding this comment.
Looks reasonable, is the plan to run this a few times on a PR branch before merging?
Since in its current state, its likely even more flaky than before until we tune that value better
9bf4286 to
93174e3
Compare
Modified the CI to run the test 20x for each target (including M1), all test pass (CI as a whole failed tough, since the artifact failed (as it should)), results here: https://dev.azure.com/digitalasset/daml/_build/results?buildId=220514&view=logs&jobId=61ac79bb-734c-5c5d-b939-064aaa72fe3b&j=5f7337b7-8c41-55ac-da29-0870b12c9d5d I will now revert my CI test changes & merge |
93174e3 to
4faa062
Compare
Fixes #20724
Problem
Both script interrupt tests ("scenario service interrupts outdated script runs" and "scenario service does not interrupt on non-script messages") used an interpreted foldl (+) 0 [1..10000000] to simulate a long-running script. This 10M-element fold took ~3s on a fast local machine but 25-43s on loaded CI runners (linux arm, linux intel, m1 mac), regularly exceeding the 30-60s timeouts.
The flake was observed across multiple platforms and dates:
Fix
Test results (local)