test: handler-level LSP harness coverage for on_completion / on_goto_definition - #210
Merged
mikebronner merged 2 commits intoJun 18, 2026
Conversation
…etion Drive the real LSP request handlers Backend::resolve_inertia_file (goto) and Backend::get_all_inertia_pages (completion) through the tower_lsp LspService harness, mirroring flux_goto_def_handler.rs. Prior Inertia coverage stopped at the pure path-math seam (resolve_page_candidates, list_pages) in inertia.rs and never exercised the request->handler->seam wiring, so a regression in the handler arm (dropped containment guard, inverted existence check, phantom or missing target) would slip through. New tests/inertia_handler.rs covers, with real tempdir fixtures: - goto: .vue happy path, nested .tsx, unresolvable -> None, and the .vue-wins extension priority when no dominant extension is primed - completion: sorted extensionless listing, empty project, and the non-page-extension (.php/.blade.php) filter The goto tests prime cached_config (root only); the completion tests prime root_path -- matching exactly the state each handler reads. Fixes #207 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
mikebronner
marked this pull request as ready for review
June 18, 2026 12:48
There was a problem hiding this comment.
✅ Approved
Review Summary
- Pure test-only PR (237 additions, 0 deletions): one new harness file
inertia_handler.rs+ a one-linetests/mod.rswiring. No production code touched. CI green (LSP test/fmt/clippy pass). - All 9 acceptance criteria met — verified each behaviour against the real handler arms, not just the test text:
goto_backendprimescached_config— exactly whatresolve_inertia_filereads (main.rs:18661→get_cached_config().root);completion_backendprimesroot_path— exactly whatget_all_inertia_pagesreads (main.rs:12467→list_pages). Each fixture primes the state its handler actually consumes. Honest harness..vue-wins priority confirmed againstPAGE_EXTENSIONS = ["vue","tsx","jsx","svelte"](inertia.rs:25); withroot_pathunset,inertia_dominant_extensionreturnsNone, so the static order decides — the test's "no dominant primed" premise genuinely holds.- Empty-project test drives the real
list_pageswalk on a real empty tempdir (not theNoneshort-circuit) — meaningful coverage, not a trivial pass.
- Built through
LspService::new(LaravelLanguageServer::new), matching theflux_goto_def_handler.rs/blade_var_rename_handler.rsconvention. No mocks. - For the record, two test-honesty observations were considered and judged non-actionable: the
.blade.phpfixture is excluded via the sameext=="php"branch as.php(redundant-but-correct, faithful to the AC), and the extension-priority test doesn't isolate the static-order path from the dominant tie-break (the AC scoped it to "no dominant primed," which the test honors exactly). Neither is a defect; forcing changes there would be gold-plating beyond the contract.
What's Good
- Tests drive the real Backend handler arms (
resolve_inertia_file/get_all_inertia_pages), a genuine step up from the already-covered pure path-math seam ininertia.rs— precisely what #207 asked for. - Each fixture primes exactly the state its handler reads, and the doc comments explain why (Salsa actor stays inert, document cache left empty so
file_exists_cachedfalls through to live disk probes). - The unresolvable-page test seeds a real unrelated page first, so the
Noneproves "missing target," not "empty/unconfigured project." Careful work.
📋 Non-blocking follow-ups
- Handler-level coverage for the dominant-extension float branch (
main.rs:18673-18679) — when a dominant extension is primed (inertia_default_ext = Some(Some("tsx"))),resolve_inertia_filefloats that extension ahead of the staticPAGE_EXTENSIONSorder. This PR deliberately leaves the dominant unset (the AC scoped it that way), so that production branch has only seam-level coverage ininertia.rs, no handler-level test. A belt-and-suspenders add on #207's own theme. General observation about untouched production code, outside this PR's AC — tracked as a new follow-up issue, not a blocker.
Ready for @mikebronner to merge.
5 tasks
mikebronner
deleted the
chore/207-handler-level-lsp-harness-coverage-for-oncompletio
branch
June 18, 2026 13:15
mikebronner
added a commit
that referenced
this pull request
Jun 18, 2026
PR #210 established handler-harness coverage for the goto/completion arms but deliberately left the dominant-extension *float* branch unreached: its priority test ran with no dominant extension primed, so `.vue` won by the static `PAGE_EXTENSIONS` order. The complementary arm — a primed dominant extension floating a non-first candidate to the front (main.rs:18673–18679) — had no handler-level test. Add a `goto_backend_with_dominant` helper (same shape as `goto_backend` but with the `inertia_default_ext` cache slot primed to `Some(Some(ext))`) and two `#[tokio::test]`s on the real handler: - `inertia_dominant_extension_floats_tsx_over_vue`: both `.vue` and `.tsx` exist, dominant `"tsx"` primed → `.tsx` wins (the float fires). - `inertia_dominant_extension_absent_file_falls_back_to_static_priority`: dominant `"tsx"` primed but only `.vue` on disk → `.vue` resolves (the float reorders, the existence probe skips the absent dominant candidate). Expands the existing `inertia_handler.rs` anchor per its module doc; no new test module or file. Fixes #212 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
9 of 10 tasks
mikebronner
added a commit
that referenced
this pull request
Jun 18, 2026
… branch (#216) * chore: start work on #212 * test: 🧪 cover the Inertia dominant-extension float at handler level PR #210 established handler-harness coverage for the goto/completion arms but deliberately left the dominant-extension *float* branch unreached: its priority test ran with no dominant extension primed, so `.vue` won by the static `PAGE_EXTENSIONS` order. The complementary arm — a primed dominant extension floating a non-first candidate to the front (main.rs:18673–18679) — had no handler-level test. Add a `goto_backend_with_dominant` helper (same shape as `goto_backend` but with the `inertia_default_ext` cache slot primed to `Some(Some(ext))`) and two `#[tokio::test]`s on the real handler: - `inertia_dominant_extension_floats_tsx_over_vue`: both `.vue` and `.tsx` exist, dominant `"tsx"` primed → `.tsx` wins (the float fires). - `inertia_dominant_extension_absent_file_falls_back_to_static_priority`: dominant `"tsx"` primed but only `.vue` on disk → `.vue` resolves (the float reorders, the existence probe skips the absent dominant candidate). Expands the existing `inertia_handler.rs` anchor per its module doc; no new test module or file. Fixes #212 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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
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.
Summary
Implements #207 — handler-level LSP harness coverage for the Inertia goto-definition and completion request handlers, the follow-up Holmes flagged in his review of #10 / PR #204.
Prior Inertia coverage stopped at the pure path-math seam (
resolve_page_candidates,list_pagesininertia.rs) and never drove the real LSP request handlers that wire those seams to the wire protocol. This adds end-to-end tests that build a real backend through thetower_lsp::LspServiceharness — the same pattern asflux_goto_def_handler.rs/blade_var_rename_handler.rs— so a regression in the handler arm (a dropped containment guard, an inverted existence check, a phantom or missing target) is now caught.Changes
laravel-lsp/src/tests/inertia_handler.rs, wired intolaravel-lsp/src/tests/mod.rs.Backend::resolve_inertia_fileon a backend primed with a minimal root-onlycached_config.Backend::get_all_inertia_pageson a backend primed withroot_path— matching exactly the state each handler reads.tempfile::TempDir; the document cache is left empty sofile_exists_cachedfalls through to real disk probes, as the live server does.Acceptance Criteria
laravel-lsp/src/tests/inertia_handler.rsis created and wired intolaravel-lsp/src/tests/mod.rsLspService::new(LaravelLanguageServer::new)to build a real backend — no mock; same harness pattern asflux_goto_def_handler.rsandblade_var_rename_handler.rsresolve_inertia_file("Dashboard")on a backend primed with a minimalcached_config(root only) and a realresources/js/Pages/Dashboard.vuereturns the correctPathBufresolve_inertia_file("Auth/Login")withresources/js/Pages/Auth/Login.tsxreturns the nestedPathBufresolve_inertia_file("Ghost")with no matching page file returnsNoneDashboard.vueandDashboard.tsxexist with no dominant extension primed,.vuewins (first inPAGE_EXTENSIONSorder)get_all_inertia_pages()on a backend primed withroot_pathpointing at a tempdir containingDashboard.vueandAuth/Login.tsxreturns["Auth/Login", "Dashboard"](sorted, without extension)get_all_inertia_pages()with noresources/js/Pages/directory returns an empty list.php,.blade.php) underresources/js/Pages/are excluded from the completion listTest Plan
cargo test inertia_handler→ 7 test fns covering the 8 AC behaviours, all green)cargo testlib + bin: 1875 + 395 passed, 0 failed)cargo fmt --checkcleancargo clippy --testsclean (CI gates on-D warnings)integration_tests.rsfailures seen locally are pre-existing and environmental — they require the gitignoredtest-project/fixture (.env+ composer vendor) that CI bootstraps (ci.yml lines 30-59); unaffected by this diff.Fixes #207