Test speedup - #411
Merged
Merged
Conversation
razzmatazz
force-pushed
the
speed-up-tests
branch
from
August 20, 2026 11:54
1fbb699 to
b2c4d1e
Compare
…ures 🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
…ePool 🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
…ePool 🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
…ePool 🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
…Pool 🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
…Pool 🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
…ePool 🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
…Pool 🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
…rePool 🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
…hangs CI (github.com/razzmatazz/csharp-language-server/actions/runs/32340155559/job/96337460610) hung consistently on a 4-core runner with NUnit.NumberOfTestWorkers=2. Two compounding bugs: 1. maxPoolSize's `max 2` floor forced the pool to permanently reserve 2 of the 4 activeClientsSemaphore slots (50%) once booted, starving the rest of the suite's ad-hoc activateFixture calls far more than intended on a low-core machine. Lowered the floor to 1. 2. Booting a fresh pooled instance ran synchronously inside poolManager's single-threaded message loop, blocking on activeClientsSemaphore. If that wait took a while (because unrelated ad-hoc tests held the remaining slots), the *entire* pool actor froze — unable to process even CheckIn messages returning already-idle instances of the same fixture. Split booting out into a second, dedicated bootWorker actor so a slow/blocked boot only stalls bootWorker, never poolManager. Verified locally with DOTNET_PROCESSOR_COUNT=4 + NUnit.NumberOfTestWorkers=2 (simulating the CI environment) and --blame-hang: 295/295 pass, no hang. 🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
…t/formatting Both handlers only return an unapplied edit description (CodeAction[]/TextEdit[]) without mutating the document or workspace, and pooled fixtures have no way to apply the result back (no Change/Save, no workspace/applyEdit, no workspace/executeCommand) — same reasoning as the existing rename entries. 🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
…turePool 🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
Only the tests using a plain activateFixture "genericProject" are converted; the TFM-parameterized test (activateFixtureExt with a fixture patch) stays ad-hoc since it needs a dedicated, per-variant solution. 🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
…xturePool Only testEditorConfigFormatting is converted; the other test needs a custom server profile (applyFormattingOptions) and repeatedly mutates the open document via Change, neither of which pooled fixtures support. 🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
…here Fixtures.fs (module CSharpLanguageServer.Tests.Fixtures) now owns the whole fixture-activation surface that used to live in Tooling.fs: emptyFixturePatch, patchFixtureWithTfm, activateFixtureExt, activateFixture, and activateFixtureWithLoggingEnabled, alongside the existing FixturePool/rentFixture machinery that already depended on activateFixture. Tooling.fs keeps only the lower-level harness (LspTestClient, LspDocumentHandle, LspClientProfile, etc.). Updated every test file's `open` accordingly, the .fsproj compile entry, and the architecture doc. 🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
razzmatazz
force-pushed
the
speed-up-tests
branch
from
August 21, 2026 05:34
b2c4d1e to
7a87467
Compare
…g-calls tests The six testCallHierarchyOutgoingCalls* tests were added alongside the callHierarchy/outgoingCalls feature after the rest of this file had already been migrated to FixturePool, so they were still booting a dedicated server per test via activateFixture. Switch them to rentFixture; they only issue allow-listed read-only requests (prepareCallHierarchy, outgoingCalls) and never mutate documents. 🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
…ticsForUri LspTestClient and PooledLspTestClient both expose an identical Request<'Req,'Res> member but aren't otherwise unified, which forced getWorkspaceDiagnosticsForUri to either take a concrete LspTestClient or be duplicated for the pooled client. Extract the minimal shared bit into an ILspRequestClient interface (Request only — Open, GetState etc. deliberately stay unshared since their availability/return types are part of the pooled client's read-only safety boundary) and have getWorkspaceDiagnosticsForUri accept that instead, so both client types can use it. 🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
Migrate the remaining read-only tests to rentFixture/FixturePool: - DiagnosticTests: the seven workspace/textDocument-diagnostic tests that only poll (no Change/Save, no progress/streaming, no custom profile). - DocumentSyncTests: the two open-nonexisting-file tests (now using the shared getWorkspaceDiagnosticsForUri via ILspRequestClient). - WorkspaceFolderRegressionTests: the loose-document-attaches-to-project test. 🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
…deadlock pooled instances hold a permanent activeClientsSemaphore permit for the whole run once booted (they're only ever checked back into Idle, never disposed until final teardown). With enough distinct fixture names now going through FixturePool, the pool alone can claim every permit on a low-core CI runner, permanently starving any later ad-hoc activateFixture/activateFixtureExt call (e.g. SourceGeneratorTests, which mutates a generator DLL on disk and so can never be pooled) — a genuine deadlock, since nothing releases a permit until PoolTeardown, which can't run until the stuck test finishes. Move activeClientsSemaphore ownership into Fixtures.fs and route every LspTestClient construction — pooled and ad-hoc alike — through poolManager, so the pool has visibility into (and can act on) every boot, not just its own. LspTestClient itself no longer touches a semaphore directly; its constructor takes a releaseSlot callback instead. Idle instances are now evicted rather than held forever: reactively, right before any new boot is committed to, if the semaphore is fully saturated; and proactively, via a periodic TTL sweep. Ad-hoc boots are "spoiled" leases — routed through the same admission control but never added to any fixture's Idle list, torn down by the caller's own Dispose same as before. Also drops the separate bootWorker actor in favor of a self-posting Async fired directly from poolManager, so all pool state lives in one place instead of being split across two actors. Verified locally with DOTNET_PROCESSOR_COUNT=4 + NUnit.NumberOfTestWorkers=2 (simulating CI) and --blame-hang: 307/307 pass, no hang. 🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
…olManager Replace the freestanding Async.Sleep-based sweep loop (started via a `do` block right after `poolManager`) with a `System.Threading.Timer` created and disposed as part of poolManager's own state machine, via two new messages: - `Initialize`, posted once right after the actor starts, creates the timer (idempotent — a no-op if already set). - `Shutdown` (renamed from `DisposeAll`, since it now does more than that) disposes the timer alongside the existing idle-instance cleanup. PoolState now bundles the per-fixture map together with the timer handle (`SweepTimer: Timer option`), so all of poolManager's state — including the sweep timer's lifecycle — lives in one record instead of being split between actor state and a loose background loop with no explicit shutdown. Verified: dotnet test (307/307) at full concurrency and again under DOTNET_PROCESSOR_COUNT=4 + NUnit.NumberOfTestWorkers=2 with --blame-hang (simulating CI) — no hangs, no regressions. 🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
maxPoolSize: now that idle instances are evicted on demand (evictIfSaturated) and via a TTL sweep, a small, artificially conservative per-fixture cap is no longer load-bearing for starvation/deadlock safety — activeClientsSemaphore (Environment. ProcessorCount) is the real, global ceiling on concurrently-alive instances across every fixture combined regardless of what this returns. Bump it to 2 * ProcessorCount so a single fixture's pool is never artificially capped below what the semaphore would allow anyway; idle instances cost memory/handles but essentially no CPU, so there's little reason to keep it small. Verified: dotnet test (307/307) at full concurrency and again under DOTNET_PROCESSOR_COUNT=4 + NUnit.NumberOfTestWorkers=2 with --blame-hang (simulating CI). pre-commit hook: adds .githooks/pre-commit, running `dotnet fantomas --check` on staged .fs/.fsi/.fsx files before allowing a commit. Excludes src/Ionide.LanguageServerProtocol to match .github/workflows/test.yaml's Fantomas check steps, which only check `tests` and `src/CSharpLanguageServer` (that directory is a vendored LSP protocol library, not held to this project's style). Not active by default — opt in per clone with `git config core.hooksPath .githooks`, documented in CONTRIBUTING.md; bypassable per-commit with `git commit --no-verify`. Also reformats Fixtures.fs per Fantomas, caught by the new hook itself. 🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
Retry attributes were masking real flakiness instead of fixing it. Dropped them from all 9 annotated tests and re-ran the suite repeatedly under full parallel load to surface what actually breaks without the safety net. Two genuine bugs were found: - ServerStateLoop.RequestQueueDrained tore down and replaced workspace folders on reconfiguration (e.g. solutionPathOverride change via workspace/didChangeConfiguration) but never proactively kicked off loading for the newly-Uninitialized folder — it relied entirely on some future request happening to call ctx.LoadWorkspaceFolder again. Under heavy scheduling contention, a request already in flight when draining began could resolve against the *stale* pre-reload workspace, after which nothing was left to trigger the new load, leaving the workspace stuck in Configured/Uninitialized forever. Fixed by posting ProcessSolutionAwaiters right after applying reconfigurations, so the reload starts deterministically instead of by chance. - testActorRemainsHealthyAfterTimeout asserted a 200ms JSON-RPC timeout fired within an outer 500ms wall-clock wait — too tight a margin under a busy, fully parallel test run. Widened the outer wait to 5s; the test still verifies the call times out, just without racing the actor's own timer under load. The remaining 8 previously-Retry-annotated tests (Razor 250ms-sleep races, push-diagnostics polling, --diagnose subprocess) did not reproduce as flaky across multiple full-suite runs and needed no changes. 🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
…FiresAtCorrectTime CI failed on ubuntu-24.04 with the short-deadline call taking exactly 500 ms, right at the assertion's wait boundary. With tests now running across multiple parallel NUnit workers, thread-pool/scheduler contention can push a 200 ms deadline's actual firing past a tight 500 ms wall-clock budget. Widen the wait window to 2 s; the long call's 5 s deadline still bounds the correctness check.
razzmatazz
force-pushed
the
speed-up-tests
branch
2 times, most recently
from
August 21, 2026 07:35
31e3d29 to
d4510a2
Compare
Ports the first test file off NUnit as a pilot for a gradual, file-by-file migration. YoloDev.Expecto.TestSdk is a VSTest adapter just like NUnit3TestAdapter (still used by every other file in this project), so `dotnet test` discovers and runs both frameworks' tests together from the same project/assembly, with no separate project or custom entry point needed. InternalTests.fs keeps the same function bodies, nesting, and comments as before, only swapping NUnit's [<TestCase>]/[<Test>]/ClassicAssert for a plain case list plus Expecto's testList/testCase/Expect.equal. 🤖 Generated with [ECA](https://eca.dev) (anthropic/claude-sonnet-5) Co-Authored-By: eca-agent <git@eca.dev>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.