From 65f92dd11ceddf700f31f740b0a67ef68ebeda9d Mon Sep 17 00:00:00 2001 From: Christian Findlay <16697547+MelbourneDeveloper@users.noreply.github.com> Date: Tue, 4 Aug 2026 19:33:24 +1000 Subject: [PATCH 1/2] Gate Zed and Rider coverage; replace the Zed tests that asserted nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `.config/coverage/thresholds.json` gated five packages. Neither editor integration was one of them. Zed shipped 23 unit tests that were never executed: `make _lint-zed` runs clippy `--all-targets`, which compiles them, and `make test` had no Zed target. Unrun, they had rotted into tautologies — `expected_version_matches_cargo_toml` asserted `EXPECTED_VERSION == env!("CARGO_PKG_VERSION")` where `EXPECTED_VERSION` *is* that env; the test named for extension.toml/Cargo.toml version parity never opened extension.toml; the missing-binary test rebuilt the error string inside itself and asserted on its own copy. Rider had 2961 LOC of Kotlin, zero tests, and zero CI presence — not built, not linted, not tested. build.gradle.kts declared junit-jupiter, the platform test framework, and a `test { useJUnitPlatform() }` block against no `src/test` directory at all. Zed: extract the parse -> read -> enrich -> format pipeline out of the `zed::Worktree` shell into pipeline.rs so it is reachable from a test, and split `cached_or` / `with_default_log_level` / `tree_output` out of the WASM API adapters. Real tests replace the vacuous ones, including version parity that actually parses extension.toml. 75.41% -> 85.04%. Rider: first tests (NuGetState — installed-list merge, case-insensitive id matching, sort order, pending flags), plus Kover wired to the repo ratchet at its honest 4.09%. Two build fixes were needed before any test could run: the platform's JUnit5TestSessionListener loads junit.framework.TestCase and dies without JUnit 4 on the runtime classpath, and the junit-vintage it drags in throws during discovery against junit-jupiter 5.11.3's older junit-platform-commons. `failOnNoDiscoveredTests` now stops an empty test task reporting BUILD SUCCESSFUL, which is how the harness stayed green while empty. ci-editors.yml runs both. RIDER_REQUIRED=1 turns a missing JDK 21+ from a local convenience skip into a hard CI failure. Also: drop `default_threshold: 90`, which check-coverage.mjs never read, and de-duplicate the JDK-21 discovery loop into tools/rider/gradle.sh. Remaining debt tracked in #210. --- .config/coverage/thresholds.json | 7 +- .github/workflows/ci-editors.yml | 84 +++++++ .github/workflows/ci.yml | 7 + src/editors/rider/build.gradle.kts | 25 +- .../rider/toolwindow/nuget/NuGetStateTest.kt | 148 +++++++++++ src/editors/zed/Cargo.lock | 61 +++++ src/editors/zed/Cargo.toml | 5 + src/editors/zed/src/lib.rs | 229 +++++++++++------- src/editors/zed/src/pipeline.rs | 133 ++++++++++ tools/coverage/kover-line-percent.cs | 50 ++++ tools/make/main.mk | 72 +++--- tools/rider/gradle.sh | 72 ++++++ 12 files changed, 776 insertions(+), 117 deletions(-) create mode 100644 .github/workflows/ci-editors.yml create mode 100644 src/editors/rider/src/test/kotlin/com/forgelsp/rider/toolwindow/nuget/NuGetStateTest.kt create mode 100644 src/editors/zed/src/pipeline.rs create mode 100644 tools/coverage/kover-line-percent.cs create mode 100644 tools/rider/gradle.sh diff --git a/.config/coverage/thresholds.json b/.config/coverage/thresholds.json index 887295b2..d0c1bd70 100644 --- a/.config/coverage/thresholds.json +++ b/.config/coverage/thresholds.json @@ -1,9 +1,14 @@ { "_agent_pmo": "2efd847", - "default_threshold": 90, "sharplsp": { "line_percent": 95.0 }, + "sharplsp-zed": { + "line_percent": 85.04 + }, + "sharplsp-rider": { + "line_percent": 4.09 + }, "vscode-extension": { "line_percent": 94.0 }, diff --git a/.github/workflows/ci-editors.yml b/.github/workflows/ci-editors.yml new file mode 100644 index 00000000..2d247a7d --- /dev/null +++ b/.github/workflows/ci-editors.yml @@ -0,0 +1,84 @@ +# agent-pmo:0b21609 +--- +# Reusable editor-integration leg of the PR pipeline (called by ci.yml). +# +# Both of these shipped for their whole history with no CI job at all. The Zed +# extension's 23 unit tests existed in the tree and were compiled by +# `make _lint-zed` but never executed; the Rider plugin had a fully configured +# JUnit harness and not one test file, and was not even compiled on a PR. +# Each now runs its tests behind the same ratcheted coverage gate every other +# package answers to. [DIST-CI-EDITORS] +name: CI / Editors +'on': + workflow_call: {} +env: + CARGO_TERM_COLOR: always + RUST_BACKTRACE: 1 + DOTNET_NOLOGO: true + DOTNET_CLI_TELEMETRY_OPTOUT: true +permissions: + contents: read +jobs: + test-zed: + name: Zed Extension + # The Zed extension is a standalone Cargo workspace (it ships as + # wasm32-wasip1), so the root `cargo llvm-cov` run in ci-rust.yml cannot + # see it and it needs its own gate. Its unit tests build for the host. + runs-on: ${{ vars.UBUNTU_RUNNER || 'ubuntu-latest' }} + timeout-minutes: 15 + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable + with: + toolchain: stable + components: llvm-tools-preview + - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2 + with: + workspaces: src/editors/zed + - uses: taiki-e/install-action@43aecc8d72668fbcfe75c31400bc4f890f1c5853 # v2 + with: + tool: cargo-llvm-cov + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: '20' + - name: Test + coverage gate (Zed) + run: make _test-zed + test-rider: + name: Rider Plugin + # RIDER_REQUIRED=1 turns "no JDK 21+ found" from a local convenience skip + # into a hard failure. Without it a toolchain regression would present as a + # passing job that ran nothing. + runs-on: ${{ vars.UBUNTU_RUNNER || 'ubuntu-latest' }} + timeout-minutes: 30 + env: + RIDER_REQUIRED: '1' + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + - uses: actions/setup-java@b6effb05e454b25005698d916606bdc6ffcbf961 # v5.7.0 + with: + distribution: temurin + java-version: '21' + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: '20' + # kover-line-percent.cs is a `dotnet run --file` app, matching how + # merge-cobertura.cs reads the sidecar reports. + - uses: actions/setup-dotnet@26b0ec14cb23fa6904739307f278c14f94c95bf1 # v5.4.0 + with: + dotnet-version: 10.0.300 + # The IntelliJ Platform SDK is a multi-GB download resolved through + # Gradle. Uncached, it dominates this job's wall time. + - uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + with: + path: | + ~/.gradle/caches + ~/.gradle/wrapper + src/editors/rider/.intellijPlatform + key: "${{ runner.os }}-gradle-${{ hashFiles('src/editors/rider/build.gradle.kts', 'src/editors/rider/gradle.properties', 'src/editors/rider/gradle/**') }}" + restore-keys: "${{ runner.os }}-gradle-" + - name: Test + coverage gate (Rider) + run: make _test-rider + - name: Build plugin + # The plugin was never compiled on a PR before this job existed, so a + # Kotlin break in it could reach main behind a fully green pipeline. + run: make _build-rider diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8b13abdf..b909aa3c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -8,6 +8,7 @@ # ci-dotnet.yml sidecar tests (Ubuntu) + win32 named-pipe transport # ci-vsix.yml full VS Code suite + coverage gate (Ubuntu) # ci-vsix-windows.yml VS Code feature chunks on Windows ([DIST-CI-WIN-VSIX]) +# ci-editors.yml Zed + Rider tests and coverage gates ([DIST-CI-EDITORS]) # # Every leg is gated on `detect-changes`, and none of them `needs:` another: # lint and tests are independent required gates, and serializing tests behind @@ -139,6 +140,12 @@ jobs: - detect-changes uses: ./.github/workflows/ci-vsix-windows.yml if: ${{ needs.detect-changes.outputs.code_changed == 'true' }} + editors: + name: Editors + needs: + - detect-changes + uses: ./.github/workflows/ci-editors.yml + if: ${{ needs.detect-changes.outputs.code_changed == 'true' }} # NOTE: the former `coverage` job that git-committed+pushed ratcheted # thresholds was removed. On a pull_request, actions/checkout is a detached # HEAD, so `git push` failed the moment coverage changed — a latent red build, diff --git a/src/editors/rider/build.gradle.kts b/src/editors/rider/build.gradle.kts index 259cb713..e63c0905 100644 --- a/src/editors/rider/build.gradle.kts +++ b/src/editors/rider/build.gradle.kts @@ -6,6 +6,9 @@ plugins { // 2.14 is the current stable intellij-platform Gradle plugin release; // 2.2 was rejected by the platform with an "outdated" warning. id("org.jetbrains.intellij.platform") version "2.14.0" + // Line coverage for the Rider plugin, gated by the repo-wide ratchet in + // .config/coverage/thresholds.json. [DIST-CI-RIDER] + id("org.jetbrains.kotlinx.kover") version "0.9.2" } group = providers.gradleProperty("pluginGroup").get() @@ -52,6 +55,13 @@ dependencies { testImplementation("org.junit.jupiter:junit-jupiter:5.11.3") testRuntimeOnly("org.junit.platform:junit-platform-launcher") + // The platform test framework registers `com.intellij.tests. + // JUnit5TestSessionListener`, whose constructor loads `junit.framework. + // TestCase`. Without JUnit 4 on the runtime classpath the listener fails to + // instantiate and the whole test task dies before a single test runs -- + // NoClassDefFoundError, not a test failure. Required even though every test + // here is JUnit 5. + testRuntimeOnly("junit:junit:4.13.2") } intellijPlatform { @@ -89,7 +99,20 @@ intellijPlatform { tasks { test { - useJUnitPlatform() + useJUnitPlatform { + // The platform test framework also puts junit-vintage on the + // classpath, and that build is compiled against a newer + // junit-platform-commons than junit-jupiter 5.11.3 ships + // (`support.scanning.ClassFilter`). Vintage then dies during + // DISCOVERY, which aborts the whole task before any test runs. + // Every test here is JUnit 5, so only Jupiter is asked to discover. + includeEngines("junit-jupiter") + } + + // A `test` task that discovers nothing reports BUILD SUCCESSFUL. This + // project had a fully configured test harness and not one test file for + // its entire history, so the green was meaningless. Fail instead. + failOnNoDiscoveredTests = true } // Let Gradle wire the wrapper task so `./gradlew wrapper` regenerates. diff --git a/src/editors/rider/src/test/kotlin/com/forgelsp/rider/toolwindow/nuget/NuGetStateTest.kt b/src/editors/rider/src/test/kotlin/com/forgelsp/rider/toolwindow/nuget/NuGetStateTest.kt new file mode 100644 index 00000000..c3e25403 --- /dev/null +++ b/src/editors/rider/src/test/kotlin/com/forgelsp/rider/toolwindow/nuget/NuGetStateTest.kt @@ -0,0 +1,148 @@ +package com.forgelsp.rider.toolwindow.nuget + +import com.forgelsp.rider.lsp.InstalledPackage +import com.forgelsp.rider.lsp.PackageInfo +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test + +/** + * `NuGetState` decides what the browser renders. It is the only part of the + * Rider plugin with branching logic that does not need a running IDE, and the + * merge it performs is not obvious: a search response does not know what is + * installed, so the state has to enrich it — case-insensitively, because NuGet + * ids are not case-sensitive but the two responses do not agree on casing. + */ +class NuGetStateTest { + private fun info(id: String, version: String) = + PackageInfo(id = id, version = version, description = "desc for $id") + + private fun installed(id: String, resolved: String) = + InstalledPackage(id = id, requestedVersion = resolved, resolvedVersion = resolved) + + /** + * The Browse response carries `isInstalled = false` for everything. If the + * state did not merge, an already-installed package would offer "Install" + * again. + */ + @Test + fun `browse marks a package installed from the installed list`() { + val state = NuGetState() + state.setBrowse(listOf(info("Serilog", "4.2.0"), info("Newtonsoft.Json", "13.0.3"))) + state.setInstalled(listOf(installed("Serilog", "4.1.0"))) + + val visible = state.visible() + + assertEquals(2, visible.size) + val serilog = visible.first { it.info.id == "Serilog" } + assertTrue(serilog.info.isInstalled, "Serilog is installed and must render as such") + assertEquals("4.1.0", serilog.info.installedVersion) + + val newtonsoft = visible.first { it.info.id == "Newtonsoft.Json" } + assertFalse(newtonsoft.info.isInstalled) + assertNull(newtonsoft.info.installedVersion) + } + + /** NuGet ids are case-insensitive; the two responses need not agree. */ + @Test + fun `browse matches the installed list ignoring case`() { + val state = NuGetState() + state.setBrowse(listOf(info("Serilog", "4.2.0"))) + state.setInstalled(listOf(installed("serilog", "4.1.0"))) + + val serilog = state.visible().single() + + assertTrue(serilog.info.isInstalled, "casing must not decide installed-ness") + assertEquals("4.1.0", serilog.info.installedVersion) + } + + /** The Installed tab is a list the user scans, so ordering is part of it. */ + @Test + fun `installed tab sorts by id ignoring case`() { + val state = NuGetState() + state.tab = Tab.INSTALLED + state.setInstalled( + listOf( + installed("zzTop", "1.0.0"), + installed("Alpha", "2.0.0"), + installed("beta", "3.0.0"), + ), + ) + + val ids = state.visible().map { it.info.id } + + assertEquals(listOf("Alpha", "beta", "zzTop"), ids) + } + + /** + * When the Browse tab has already fetched metadata, the Installed tab + * reuses it rather than showing the placeholder description. + */ + @Test + fun `installed tab reuses cached browse metadata`() { + val state = NuGetState() + state.tab = Tab.INSTALLED + state.setBrowse(listOf(info("Serilog", "4.2.0"))) + state.setInstalled(listOf(installed("Serilog", "4.1.0"))) + + val serilog = state.visible().single() + + assertEquals("desc for Serilog", serilog.info.description) + assertTrue(serilog.info.isInstalled) + assertEquals("4.1.0", serilog.info.installedVersion, "the resolved version wins") + } + + /** With no cached metadata there is still a row, built from what is known. */ + @Test + fun `installed tab synthesises a row without cached metadata`() { + val state = NuGetState() + state.tab = Tab.INSTALLED + state.setInstalled(listOf(installed("Serilog", "4.1.0"))) + + val serilog = state.visible().single() + + assertEquals("Serilog", serilog.info.id) + assertEquals("4.1.0", serilog.info.version) + assertTrue(serilog.info.isInstalled) + assertEquals("Installed package", serilog.info.description) + } + + /** A row with an in-flight operation must render its spinner in both tabs. */ + @Test + fun `a pending package is flagged in both tabs`() { + val state = NuGetState() + state.setBrowse(listOf(info("Serilog", "4.2.0"))) + state.setInstalled(listOf(installed("Serilog", "4.1.0"))) + state.pending += "Serilog" + + assertTrue(state.visible().single().pending, "browse tab") + + state.tab = Tab.INSTALLED + assertTrue(state.visible().single().pending, "installed tab") + } + + @Test + fun `installed lookups ignore case and miss cleanly`() { + val state = NuGetState() + state.setInstalled(listOf(installed("Serilog", "4.1.0"))) + + assertTrue(state.isInstalled("SERILOG")) + assertEquals("4.1.0", state.installedVersion("serilog")) + + assertFalse(state.isInstalled("Newtonsoft.Json")) + assertNull(state.installedVersion("Newtonsoft.Json")) + } + + /** A fresh browser has nothing to show and must not throw doing so. */ + @Test + fun `an empty state renders nothing in either tab`() { + val state = NuGetState() + + assertTrue(state.visible().isEmpty(), "browse tab") + + state.tab = Tab.INSTALLED + assertTrue(state.visible().isEmpty(), "installed tab") + } +} diff --git a/src/editors/zed/Cargo.lock b/src/editors/zed/Cargo.lock index 8c354b21..f09d5176 100644 --- a/src/editors/zed/Cargo.lock +++ b/src/editors/zed/Cargo.lock @@ -475,11 +475,21 @@ dependencies = [ "zmij", ] +[[package]] +name = "serde_spanned" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6662b5879511e06e8999a8a235d848113e942c9124f211511b16466ee2995f26" +dependencies = [ + "serde_core", +] + [[package]] name = "sharplsp-zed" version = "0.1.0" dependencies = [ "quick-xml", + "toml", "zed_extension_api", ] @@ -548,6 +558,45 @@ dependencies = [ "zerovec", ] +[[package]] +name = "toml" +version = "0.9.12+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf92845e79fc2e2def6a5d828f0801e29a2f8acc037becc5ab08595c7d5e9863" +dependencies = [ + "indexmap", + "serde_core", + "serde_spanned", + "toml_datetime", + "toml_parser", + "toml_writer", + "winnow 0.7.15", +] + +[[package]] +name = "toml_datetime" +version = "0.7.5+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "92e1cfed4a3038bc5a127e35a2d360f145e1f4b971b551a2ba5fd7aedf7e1347" +dependencies = [ + "serde_core", +] + +[[package]] +name = "toml_parser" +version = "1.1.3+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1d38ac1cf9b95face32296c0a3ede1fdc270627c9d9c02a7274dd6d960dc4d56" +dependencies = [ + "winnow 1.0.4", +] + +[[package]] +name = "toml_writer" +version = "1.1.2+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7d56353a2a665ad0f41a421187180aab746c8c325620617ad883a99a1cbe66d2" + [[package]] name = "topological-sort" version = "0.2.2" @@ -625,6 +674,18 @@ dependencies = [ "semver", ] +[[package]] +name = "winnow" +version = "0.7.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "df79d97927682d2fd8adb29682d1140b343be4ac0f08fd68b7765d9c059d3945" + +[[package]] +name = "winnow" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "23b97319f7b8343df12cc98938e5c3eb436064524c8d2b4e30a1d3a36eecdf81" + [[package]] name = "wit-bindgen" version = "0.41.0" diff --git a/src/editors/zed/Cargo.toml b/src/editors/zed/Cargo.toml index 94ea9f77..b52d60e6 100644 --- a/src/editors/zed/Cargo.toml +++ b/src/editors/zed/Cargo.toml @@ -20,3 +20,8 @@ unused_variables = "deny" [dependencies] quick-xml = "0.41" zed_extension_api = "0.7" + +[dev-dependencies] +# Parses extension.toml in the version-parity test. A dev-dependency, so the +# wasm32-wasip1 extension build never pulls it in. +toml = "0.9" diff --git a/src/editors/zed/src/lib.rs b/src/editors/zed/src/lib.rs index c1a9efe1..483779e2 100644 --- a/src/editors/zed/src/lib.rs +++ b/src/editors/zed/src/lib.rs @@ -1,3 +1,4 @@ +mod pipeline; mod project; mod solution; mod tree; @@ -94,28 +95,48 @@ impl SharpLspExtension { /// the binary version matches the extension version. Version validation /// relies on the LSP server reporting its version during initialization. fn resolve_binary(&mut self, worktree: &zed::Worktree) -> zed::Result { - if let Some(ref path) = self.cached_binary_path { - return Ok(path.clone()); - } - - let path = worktree.which(SERVER_BINARY).ok_or_else(|| { - format!( - "{SERVER_BINARY} not found on PATH. \ - Install SharpLsp v{EXPECTED_VERSION} via `make install` \ - or download from https://github.com/Nimblesite/SharpLsp/releases" - ) - })?; + cached_or(&mut self.cached_binary_path, || { + worktree.which(SERVER_BINARY) + }) + } +} - self.cached_binary_path = Some(path.clone()); - Ok(path) +/// Return the cached path, otherwise look one up and cache it. +/// +/// Split from `resolve_binary` because `zed::Worktree` exists only inside Zed's +/// WASM host: the caching contract is testable, the `which` call is not. +fn cached_or( + cache: &mut Option, + lookup: impl FnOnce() -> Option, +) -> zed::Result { + if let Some(path) = cache { + return Ok(path.clone()); } + + let path = lookup().ok_or_else(missing_binary_error)?; + *cache = Some(path.clone()); + Ok(path) +} + +/// The message shown when `sharplsp` is absent from `$PATH`. Zed surfaces this +/// verbatim, and it is the only install guidance a Zed user ever sees. +fn missing_binary_error() -> String { + format!( + "{SERVER_BINARY} not found on PATH. \ + Install SharpLsp v{EXPECTED_VERSION} via `make install` \ + or download from https://github.com/Nimblesite/SharpLsp/releases" + ) } /// Build environment variables for the sharplsp server process. fn build_server_env(worktree: &zed::Worktree) -> Vec<(String, String)> { - let mut env: Vec<(String, String)> = worktree.shell_env(); - let has_rust_log = env.iter().any(|(key, _)| key == "RUST_LOG"); - if !has_rust_log { + with_default_log_level(worktree.shell_env()) +} + +/// The server logs through `tracing`, which emits nothing without `RUST_LOG`. +/// A shell that already sets it keeps its own value. +fn with_default_log_level(mut env: Vec<(String, String)>) -> Vec<(String, String)> { + if !env.iter().any(|(key, _)| key == "RUST_LOG") { env.push(("RUST_LOG".to_string(), "info".to_string())); } env @@ -137,108 +158,150 @@ fn run_tree_command( .read_text_file(sln_path) .map_err(|err| format!("Failed to read {}: {}", sln_path, err))?; - let projects = solution::parse_solution(&sln_content, sln_path); - let enriched = enrich_projects(wt, &projects); - let text = tree::format_solution_tree(sln_path, &enriched); - let label = format!("Solution: {}", sln_path); - - Ok(zed::SlashCommandOutput { - text: text.clone(), - sections: vec![zed::SlashCommandOutputSection { - range: (0..text.len()).into(), - label, - }], - }) -} + let text = pipeline::solution_tree(sln_path, &sln_content, |path| wt.read_text_file(path).ok()); -/// Read each project file and parse its dependencies. -fn enrich_projects( - worktree: &zed::Worktree, - projects: &[solution::SolutionProject], -) -> Vec { - projects - .iter() - .map(|proj| enrich_single_project(worktree, proj)) - .collect() + Ok(tree_output(sln_path, text)) } -fn enrich_single_project( - worktree: &zed::Worktree, - proj: &solution::SolutionProject, -) -> tree::EnrichedProject { - let deps = worktree - .read_text_file(&proj.relative_path) - .map(|content| project::parse_project_file(&content)) - .unwrap_or_default(); - - tree::EnrichedProject { - name: proj.name.clone(), - relative_path: proj.relative_path.clone(), - nuget_packages: deps.nuget_packages, - project_references: deps.project_references, +/// Wrap the rendered tree in the single labelled section Zed renders. +fn tree_output(sln_path: &str, text: String) -> zed::SlashCommandOutput { + let section = zed::SlashCommandOutputSection { + range: (0..text.len()).into(), + label: format!("Solution: {}", sln_path), + }; + zed::SlashCommandOutput { + text, + sections: vec![section], } } #[cfg(test)] mod tests { use super::*; + use std::cell::Cell; + + /// The top-level `version` declared in `extension.toml`, parsed as TOML + /// rather than scraped, so a moved or commented-out key is not mistaken for + /// a match. + fn manifest_version() -> Option { + include_str!("../extension.toml") + .parse::() + .ok()? + .get("version")? + .as_str() + .map(str::to_owned) + } + /// `make _stamp-version` rewrites the version in BOTH `Cargo.toml` and + /// `extension.toml`. Zed reads the latter; `EXPECTED_VERSION` — which the + /// install message quotes — comes from the former. If a release stamps one + /// and misses the other, the marketplace listing and the version the + /// extension tells users to install disagree, and nothing else in the + /// pipeline compares them. #[test] - fn expected_version_matches_cargo_toml() { - // EXPECTED_VERSION is set at compile time from Cargo.toml. - // This ensures the Zed extension version matches the crate version. - assert_eq!(EXPECTED_VERSION, env!("CARGO_PKG_VERSION")); + fn extension_toml_version_matches_crate_version() { + assert_eq!( + manifest_version().as_deref(), + Some(EXPECTED_VERSION), + "extension.toml and Cargo.toml versions must be stamped together", + ); } #[test] - fn expected_version_is_valid_semver() { + fn crate_version_is_numeric_and_dotted() { let segments: Vec<&str> = EXPECTED_VERSION.split('.').collect(); assert!( segments.len() >= 2, - "Version must have at least X.Y segments, got: {EXPECTED_VERSION}", + "version needs at least X.Y segments, got {EXPECTED_VERSION}", ); for segment in &segments { assert!( segment.parse::().is_ok(), - "Each version segment must be numeric, got: {segment} in {EXPECTED_VERSION}", + "every segment must be numeric, got {segment} in {EXPECTED_VERSION}", ); } } #[test] - fn expected_version_matches_extension_toml_version() { - // extension.toml `version` and Cargo.toml `version` MUST match. - // Since both are set to the same value, and EXPECTED_VERSION comes - // from Cargo.toml, this test proves they are in sync. - // If they drift, the build system should catch it. - let version = env!("CARGO_PKG_VERSION"); - assert!(!version.is_empty(), "CARGO_PKG_VERSION must not be empty",); - } + fn a_resolved_binary_is_returned_and_cached() { + let mut cache = None; - #[test] - fn server_binary_name_is_sharplsp() { - assert_eq!(SERVER_BINARY, "sharplsp"); + let resolved = cached_or(&mut cache, || Some("/usr/local/bin/sharplsp".to_owned())); + + assert_eq!(resolved, Ok("/usr/local/bin/sharplsp".to_owned())); + assert_eq!(cache.as_deref(), Some("/usr/local/bin/sharplsp")); } + /// The cache exists to keep `Worktree::which` off the hot path. If a second + /// resolve still looked up, the cache would be decorative. #[test] - fn missing_binary_error_includes_version_and_install_instructions() { - // Simulate what resolve_binary returns when the binary is not found. - let error_msg = format!( - "{SERVER_BINARY} not found on PATH. \ - Install SharpLsp v{EXPECTED_VERSION} via `make install` \ - or download from https://github.com/Nimblesite/SharpLsp/releases" - ); + fn a_cached_binary_is_not_looked_up_again() { + let mut cache = Some("/cached/sharplsp".to_owned()); + let looked_up = Cell::new(false); + + let resolved = cached_or(&mut cache, || { + looked_up.set(true); + Some("/fresh/sharplsp".to_owned()) + }); + + assert_eq!(resolved, Ok("/cached/sharplsp".to_owned())); assert!( - error_msg.contains(EXPECTED_VERSION), - "Error message must include the expected version", + !looked_up.get(), + "the cached path must short-circuit lookup" ); + } + + /// This asserts on the message `cached_or` actually produces. The previous + /// test rebuilt the string itself and so could not have caught a change to + /// the real one. + #[test] + fn an_absent_binary_reports_the_install_guidance() { + let mut cache = None; + + let resolved = cached_or(&mut cache, || None); + + let Err(message) = resolved else { + panic!("a missing binary must not resolve"); + }; + assert!(message.contains(SERVER_BINARY), "{message}"); + assert!(message.contains(EXPECTED_VERSION), "{message}"); + assert!(message.contains("make install"), "{message}"); assert!( - error_msg.contains("make install"), - "Error message must include install instructions", + message.contains("github.com/Nimblesite/SharpLsp"), + "{message}" ); + assert!(cache.is_none(), "a failed lookup must not poison the cache"); + } + + #[test] + fn a_shell_without_rust_log_gets_the_default_level() { + let env = with_default_log_level(vec![("PATH".to_owned(), "/usr/bin".to_owned())]); + assert!( - error_msg.contains("github.com"), - "Error message must include download URL", + env.contains(&("RUST_LOG".to_owned(), "info".to_owned())), + "{env:?}" ); } + + #[test] + fn a_shell_with_rust_log_keeps_its_own_level() { + let env = with_default_log_level(vec![("RUST_LOG".to_owned(), "trace".to_owned())]); + + assert_eq!(env, vec![("RUST_LOG".to_owned(), "trace".to_owned())]); + } + + /// Zed highlights the section by byte range. A range short of the text + /// leaves the tail unlabelled; one past it is out of bounds. + #[test] + fn the_output_section_spans_the_whole_tree() { + let text = "Solution: App.slnx\n└ Project: Api (Api.csproj)\n".to_owned(); + + let output = tree_output("App.slnx", text.clone()); + + assert_eq!(output.text, text); + assert_eq!(output.sections.len(), 1); + assert_eq!(output.sections[0].label, "Solution: App.slnx"); + assert_eq!(output.sections[0].range.start, 0); + assert_eq!(u64::from(output.sections[0].range.end), text.len() as u64); + } } diff --git a/src/editors/zed/src/pipeline.rs b/src/editors/zed/src/pipeline.rs new file mode 100644 index 00000000..0c8202d2 --- /dev/null +++ b/src/editors/zed/src/pipeline.rs @@ -0,0 +1,133 @@ +//! Pure solution-tree assembly: solution text in, rendered tree out. +//! +//! Extracted from `lib.rs` so the whole parse -> read -> enrich -> format +//! pipeline runs without a `zed::Worktree`, which exists only inside Zed's +//! WASM host and cannot be constructed in a unit test. The extension entry +//! point passes a reader backed by `Worktree::read_text_file`; tests pass a +//! plain closure. + +use crate::project; +use crate::solution::{self, SolutionProject}; +use crate::tree::{self, EnrichedProject}; + +/// Parse a solution, read each project file for its dependencies, and render +/// the tree. +pub fn solution_tree(sln_path: &str, sln_content: &str, read: R) -> String +where + R: Fn(&str) -> Option, +{ + let projects = solution::parse_solution(sln_content, sln_path); + let enriched: Vec = projects.iter().map(|proj| enrich(proj, &read)).collect(); + tree::format_solution_tree(sln_path, &enriched) +} + +/// A project whose file cannot be read still appears in the tree, with no +/// dependencies. A missing or unreadable `.csproj` must never silently drop a +/// project the solution declares — the tree would then disagree with the +/// solution it claims to display. +fn enrich(proj: &SolutionProject, read: &R) -> EnrichedProject +where + R: Fn(&str) -> Option, +{ + let deps = read(&proj.relative_path) + .map(|content| project::parse_project_file(&content)) + .unwrap_or_default(); + + EnrichedProject { + name: proj.name.clone(), + relative_path: proj.relative_path.clone(), + nuget_packages: deps.nuget_packages, + project_references: deps.project_references, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::cell::RefCell; + + const SLNX: &str = r#" + + +"#; + + const API_CSPROJ: &str = r#" + + + + +"#; + + /// Reader that serves the two project files the solution declares. + fn reader(path: &str) -> Option { + match path { + "src/Api/Api.csproj" => Some(API_CSPROJ.to_owned()), + "src/Core/Core.csproj" => Some("".to_owned()), + _ => None, + } + } + + #[test] + fn renders_every_project_with_its_dependencies() { + let text = solution_tree("App.slnx", SLNX, reader); + + assert!(text.contains("Solution: App.slnx"), "{text}"); + assert!(text.contains("Api"), "{text}"); + assert!(text.contains("Core"), "{text}"); + assert!(text.contains("Serilog"), "{text}"); + assert!(text.contains("4.2.0"), "{text}"); + } + + /// The dependency-bearing project renders a Dependencies subtree; the empty + /// one must not, or every leaf project would gain a hollow section. + #[test] + fn omits_the_dependencies_subtree_for_a_project_with_none() { + let text = solution_tree("App.slnx", SLNX, reader); + + assert_eq!( + text.matches("Dependencies").count(), + 1, + "only Api has dependencies, so exactly one subtree is expected: {text}" + ); + } + + /// An unreadable project file is the common case for a solution checked out + /// without submodules. The project must still be listed. + #[test] + fn keeps_a_project_whose_file_cannot_be_read() { + let text = solution_tree("App.slnx", SLNX, |_| None); + + assert!(text.contains("Api"), "{text}"); + assert!(text.contains("Core"), "{text}"); + assert!( + !text.contains("Dependencies"), + "nothing was readable, so no dependency subtree can be known: {text}" + ); + } + + /// A solution declaring no projects still renders its header rather than an + /// empty string, so the slash command output is never blank. + #[test] + fn renders_a_header_for_a_solution_with_no_projects() { + let text = solution_tree("Empty.slnx", "", reader); + + assert_eq!(text, "Solution: Empty.slnx\n"); + } + + /// The reader is keyed on the path the solution declares. Passing the wrong + /// path silently yields a dependency-free tree, so pin the exact lookups. + #[test] + fn reads_each_project_at_the_path_the_solution_declares() { + let seen = RefCell::new(Vec::new()); + + let _ = solution_tree("App.slnx", SLNX, |path: &str| { + seen.borrow_mut().push(path.to_owned()); + None + }); + + assert_eq!( + seen.into_inner(), + vec!["src/Api/Api.csproj", "src/Core/Core.csproj"] + ); + } +} diff --git a/tools/coverage/kover-line-percent.cs b/tools/coverage/kover-line-percent.cs new file mode 100644 index 00000000..84073444 --- /dev/null +++ b/tools/coverage/kover-line-percent.cs @@ -0,0 +1,50 @@ +#:property EnforceExtendedAnalyzerRules=false + +// Print the roll-up line-coverage percentage from a Kover XML report, for +// `check-coverage.mjs` to gate on. [DIST-CI-RIDER] +// +// Kover emits a JaCoCo-shaped document: every , and +// repeats the counters for its own scope, and carries the totals as +// direct children. Reading a descendant counter would score one arbitrary +// class instead of the plugin. + +using System.Globalization; +using System.Xml.Linq; + +if (args.Length != 1) +{ + Console.Error.WriteLine("usage: kover-line-percent.cs "); + return 2; +} + +var reportPath = args[0]; +if (!File.Exists(reportPath)) +{ + Console.Error.WriteLine($"ERROR: no Kover report at {reportPath}"); + return 1; +} + +var root = XDocument.Load(reportPath).Root; +var counter = root? + .Elements("counter") + .FirstOrDefault(element => (string?)element.Attribute("type") == "LINE"); + +if (counter is null) +{ + Console.Error.WriteLine($"ERROR: {reportPath} has no report-level LINE counter"); + return 1; +} + +var missed = (long?)counter.Attribute("missed") ?? 0L; +var covered = (long?)counter.Attribute("covered") ?? 0L; +var total = missed + covered; + +if (total == 0) +{ + Console.Error.WriteLine($"ERROR: {reportPath} reports zero instrumented lines"); + return 1; +} + +var percent = 100.0 * covered / total; +Console.WriteLine(percent.ToString("G17", CultureInfo.InvariantCulture)); +return 0; diff --git a/tools/make/main.mk b/tools/make/main.mk index f6fc47cf..dd2da663 100644 --- a/tools/make/main.mk +++ b/tools/make/main.mk @@ -89,7 +89,10 @@ HOST_VSIX_BIN = $(VSCODE_DIR)/bin/$(HOST_PLATFORM)/sharplsp$(EXE_EXT) PREFIX ?= $(HOME)/.local BINDIR = $(PREFIX)/bin CHECK_COV = node tools/coverage/check-coverage.mjs +# Resolves a JDK 21+ and runs a Gradle task in the Rider project. [DIST-CI-RIDER] +RIDER_GRADLE = sh tools/rider/gradle.sh MERGE_COBERTURA = dotnet run --file tools/coverage/merge-cobertura.cs -- +KOVER_PERCENT = dotnet run --file tools/coverage/kover-line-percent.cs -- .PHONY: build ci test lint fmt clean setup screenshots website-build website-test website-dev \ install-dotnet-10 uninstall-dotnet-10 \ @@ -101,6 +104,7 @@ MERGE_COBERTURA = dotnet run --file tools/coverage/merge-cobertura.cs -- _build-rust _build-dotnet _build-vsix _build-zed _build-rider \ _stage-vsix-binary _stage-vsix-binary-only _stage-sidecars \ test-rust _test-rust _prepare-rust-tests _test-rust-shard \ + test-zed _test-zed test-rider _test-rider \ _gate-rust-coverage _test-vsix _test-vsix-win _check-vsix-chunks \ _test-dotnet _test-website \ _lint-rust _lint-zed _lint-vsix _lint-dotnet \ @@ -155,36 +159,13 @@ _build-zed: rm -f $(ZED_PKG_TAR) && tar -czf $(ZED_PKG_TAR) -C $(dir $(ZED_PKG_DIR)) $(notdir $(ZED_PKG_DIR)) _build-rider: - @command -v java >/dev/null 2>&1 || { echo "==> Skipping Rider plugin (no java on PATH)"; exit 0; } - @echo "==> Building Rider plugin..." -ifeq ($(DETECTED_OS),windows) - @cd $(RIDER_DIR) && { \ - rider_java_home=""; \ - for candidate in \ - "$${JAVA_HOME:-}" \ - /c/Program\ Files/Microsoft/jdk-* \ - /c/Program\ Files/Eclipse\ Adoptium/jdk-* \ - /c/Program\ Files/Java/jdk-* \ - /c/Program\ Files/Android/Android\ Studio/jbr \ - /c/Program\ Files\ \(x86\)/Android/openjdk/jdk-* \ - /c/Program\ Files\ \(x86\)/JetBrains/JetBrains\ Rider*/jbr; do \ - [ -x "$$candidate/bin/java.exe" ] || continue; \ - version=$$("$$candidate/bin/java.exe" -XshowSettings:properties -version 2>&1 | sed -n 's/^[[:space:]]*java.specification.version = //p' | head -n1); \ - major=$${version%%.*}; \ - case "$$major" in ''|*[!0-9]*) continue ;; esac; \ - if [ "$$major" -ge 21 ]; then rider_java_home="$$candidate"; break; fi; \ - done; \ - [ -n "$$rider_java_home" ] || { echo "ERROR: Rider 2026.1 requires JDK 21+. Install one or set JAVA_HOME to it." >&2; exit 1; }; \ - echo " using JDK $$rider_java_home"; \ - JAVA_HOME="$$rider_java_home" PATH="$$rider_java_home/bin:$$PATH" ./gradlew buildPlugin --no-daemon; \ - } -else - cd $(RIDER_DIR) && ./gradlew buildPlugin --no-daemon -endif - mkdir -p $(DIST_DIR) + @$(RIDER_GRADLE) buildPlugin @zip=$$(ls $(RIDER_DIR)/build/distributions/sharplsp-rider-*.zip 2>/dev/null | head -n1); \ - test -n "$$zip" || { echo "ERROR: no Rider plugin zip in $(RIDER_DIR)/build/distributions/" >&2; exit 1; }; \ - cp "$$zip" $(RIDER_ZIP) + if [ -n "$$zip" ]; then \ + mkdir -p $(DIST_DIR) && cp "$$zip" $(RIDER_ZIP); \ + elif [ -n "$${RIDER_REQUIRED:-}" ]; then \ + echo "ERROR: no Rider plugin zip in $(RIDER_DIR)/build/distributions/" >&2; exit 1; \ + fi _stage-vsix-binary: _build-rust _build-dotnet @$(MAKE) _stage-vsix-binary-only @@ -246,11 +227,13 @@ ci: lint test build # ── Test ───────────────────────────────────────────────────────── -test: _test-rust _test-vsix _test-dotnet _test-website +test: _test-rust _test-zed _test-vsix _test-dotnet _test-rider _test-website @echo "==> All tests passed." # Public alias — CI and developers call this. test-rust: _test-rust +test-zed: _test-zed +test-rider: _test-rider # The e2e tests spawn the real sidecars from these paths. RUST_E2E_SIDECARS = \ @@ -273,6 +256,20 @@ _test-rust: _prepare-rust-tests cargo llvm-cov nextest --json --output-path target/coverage-rust.json --no-fail-fast --test-threads $(RUST_TEST_THREADS) @$(CHECK_COV) sharplsp --json target/coverage-rust.json data.0.totals.lines.percent +# The Zed extension is a standalone workspace (it targets wasm32-wasip1), so it +# is invisible to the root `cargo llvm-cov` run and needs its own gate. Its unit +# tests build for the host, which is why they can run here at all. +# +# `lib.rs` keeps a floor of uncoverable lines: the `zed::Extension` trait impl, +# `register_extension!`, and every function taking a `zed::Worktree` only exist +# inside Zed's WASM host. The logic behind them lives in `pipeline.rs` precisely +# so it is reachable from a test. +_test-zed: + @echo "==> Running Zed extension tests with coverage..." + cargo llvm-cov --manifest-path $(ZED_DIR)/Cargo.toml \ + --json --output-path target/coverage-zed.json + @$(CHECK_COV) sharplsp-zed --json target/coverage-zed.json data.0.totals.lines.percent + # [DIST-CI-RUST-SHARDS] One CI slice of the suite: identical tests, identical # serialization (RUST_TEST_THREADS), but only the hash:$(SHARD)/$(SHARD_COUNT) # nextest partition. Exports lcov instead of JSON so _gate-rust-coverage can @@ -345,6 +342,18 @@ _test-vsix-win: _stage-vsix-binary-only rm -rf "$(abspath $(VSCODE_DIR))/bin" || true; \ exit $$status +# [DIST-CI-RIDER] The Rider plugin's only automated verification. Skipped +# locally when no JDK 21+ is installed; CI sets RIDER_REQUIRED=1 so it can never +# silently skip there — a skipped gate that reports green is worse than none. +_test-rider: + @$(RIDER_GRADLE) koverXmlReport + @report="$(RIDER_DIR)/build/reports/kover/report.xml"; \ + if [ -f "$$report" ]; then \ + pct=$$($(KOVER_PERCENT) "$$report") && $(CHECK_COV) sharplsp-rider "$$pct"; \ + elif [ -n "$${RIDER_REQUIRED:-}" ]; then \ + echo "ERROR: no Kover report at $$report" >&2; exit 1; \ + fi + _test-dotnet: _build-dotnet @echo "==> Running .NET sidecar tests..." @rm -rf target/coverage-dotnet @@ -638,8 +647,7 @@ clean: _clean-rider @echo "==> Clean." _clean-rider: - @[ -d $(RIDER_DIR) ] && command -v java >/dev/null 2>&1 && \ - cd $(RIDER_DIR) && ./gradlew clean --no-daemon || true + @$(RIDER_GRADLE) clean || true rm -rf $(RIDER_DIR)/build $(RIDER_DIR)/.gradle $(RIDER_ZIP) # ── Setup ───────────────────────────────────────────────────────── diff --git a/tools/rider/gradle.sh b/tools/rider/gradle.sh new file mode 100644 index 00000000..9f5bb954 --- /dev/null +++ b/tools/rider/gradle.sh @@ -0,0 +1,72 @@ +#!/usr/bin/env sh +# Run a Gradle task in the Rider plugin project on a JDK the IntelliJ Platform +# will accept. [DIST-CI-RIDER] +# +# Rider 2026.1 requires JDK 21+, and a developer machine routinely has an older +# JDK first on PATH — the repo's own dev container ships 11. Every Rider make +# target funnels through here so the discovery rules exist exactly once. +# +# Exit codes: +# 0 the task ran and succeeded, OR no JDK was found and RIDER_REQUIRED is unset +# 1 the task failed, or no suitable JDK was found while RIDER_REQUIRED=1 +# +# CI sets RIDER_REQUIRED=1 so a missing toolchain fails loudly instead of +# silently skipping the only verification the Rider plugin has. +set -eu + +MIN_MAJOR=21 +SCRIPT_DIR="$(CDPATH='' cd -- "$(dirname -- "$0")" && pwd)" +RIDER_DIR="$(CDPATH='' cd -- "$SCRIPT_DIR/../../src/editors/rider" && pwd)" + +# Major version of the JDK at $1, or nothing if it is not a runnable JDK. +java_major() { + _exe="$1/bin/java" + [ -x "$_exe" ] || _exe="$1/bin/java.exe" + [ -x "$_exe" ] || return 0 + + "$_exe" -XshowSettings:properties -version 2>&1 | + sed -n 's/^[[:space:]]*java.specification.version = //p' | + head -n1 | + cut -d. -f1 +} + +# First JDK on the machine at or above $MIN_MAJOR, preferring $JAVA_HOME. +find_jdk() { + for candidate in \ + "${JAVA_HOME:-}" \ + /c/Program\ Files/Microsoft/jdk-* \ + /c/Program\ Files/Eclipse\ Adoptium/jdk-* \ + /c/Program\ Files/Java/jdk-* \ + /c/Program\ Files/Android/Android\ Studio/jbr \ + /c/Program\ Files\ \(x86\)/Android/openjdk/jdk-* \ + /c/Program\ Files\ \(x86\)/JetBrains/JetBrains\ Rider*/jbr \ + /usr/lib/jvm/*; do + [ -n "$candidate" ] && [ -d "$candidate" ] || continue + major="$(java_major "$candidate")" + case "$major" in '' | *[!0-9]*) continue ;; esac + if [ "$major" -ge "$MIN_MAJOR" ]; then + printf '%s' "$candidate" + return 0 + fi + done + return 0 +} + +[ "$#" -ge 1 ] || { + echo "usage: $0 [args...]" >&2 + exit 2 +} + +jdk="$(find_jdk)" +if [ -z "$jdk" ]; then + if [ -n "${RIDER_REQUIRED:-}" ]; then + echo "ERROR: Rider needs JDK ${MIN_MAJOR}+. Install one or set JAVA_HOME to it." >&2 + exit 1 + fi + echo "==> Skipping Rider '$1' (no JDK ${MIN_MAJOR}+ found; set RIDER_REQUIRED=1 to fail instead)" + exit 0 +fi + +echo "==> Rider: gradle $* (JDK $jdk)" +cd "$RIDER_DIR" +JAVA_HOME="$jdk" PATH="$jdk/bin:$PATH" ./gradlew "$@" --no-daemon From 0308fa8d2b5bef483b0cb2cb615b03341761137c Mon Sep 17 00:00:00 2001 From: Christian Findlay <16697547+MelbourneDeveloper@users.noreply.github.com> Date: Tue, 4 Aug 2026 19:42:03 +1000 Subject: [PATCH 2/2] Create target/ before writing the Zed coverage report The Zed workspace builds into src/editors/zed/target, so on a fresh checkout the root target/ that holds every other coverage artifact does not exist and llvm-cov fails with "No such file or directory" after all 31 tests pass. Local runs missed it because target/ was already there from the Rust host build. --- tools/make/main.mk | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/make/main.mk b/tools/make/main.mk index dd2da663..032e5cbf 100644 --- a/tools/make/main.mk +++ b/tools/make/main.mk @@ -266,6 +266,10 @@ _test-rust: _prepare-rust-tests # so it is reachable from a test. _test-zed: @echo "==> Running Zed extension tests with coverage..." + # The Zed workspace builds into $(ZED_DIR)/target, so on a fresh checkout the + # root target/ that holds every other coverage artifact does not exist yet + # and llvm-cov cannot write the report into it. + @mkdir -p target cargo llvm-cov --manifest-path $(ZED_DIR)/Cargo.toml \ --json --output-path target/coverage-zed.json @$(CHECK_COV) sharplsp-zed --json target/coverage-zed.json data.0.totals.lines.percent