From 260ed46c7500d4e8093b91dd798c9e231d7b7ba5 Mon Sep 17 00:00:00 2001 From: Valentin Dide Date: Thu, 10 Sep 2026 00:45:08 +0300 Subject: [PATCH] Microsoft.Testing.Platform support in the Test Explorer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #249. The Test Explorer was VSTest end to end, so a Microsoft.Testing.Platform (MTP) project showed an empty Testing view. On the .NET 10 SDK, MTP v2 removed the VSTest shim and `xunit.v3` 4.0.0 uses MTP v2 by default, so that is now the ordinary case: `--nologo` is rejected in MTP mode (exit 5, zero tests), `dotnet vstest` cannot load an MTP-only module at all, and neither `--filter` nor `--logger trx` exists there. ## Product changes * **`feat(vscode)`: discover and run MTP projects.** A second path beside VSTest that shares everything after the TRX file. The runner is chosen per target: the `global.json` `test.runner` opt-in selects MTP outright and skips the two doomed VSTest passes; without one, the VSTest passes run first and only a sweep that produced no fully-qualified name asks MSBuild for `IsTestingPlatformApplication` (`IsTestProject` MUST NOT be used — `xunit.v3` leaves it empty). A VSTest solution therefore pays nothing ([TEST-MTP-DETECT]). * **`feat(vscode)`: ask the test MODULE, not `dotnet test`.** MTP prints no `Test run for …` banner, so the modules come from `dotnet sln list` plus `-getProperty:TargetPath` — the only source that survives a custom `AssemblyName`, `OutputPath`, `ArtifactsPath` or `RuntimeIdentifier`. Each module then answers `dotnet exec --list-tests json` directly; `dotnet test` does not forward the `json` argument (dotnet/sdk#49754) ([TEST-MTP-MODULES], [TEST-MTP-DISCOVERY]). * **`feat(vscode)`: the id comes from the listing's `type` block.** `namespace` + `typeName` + `methodName` reproduces the [TEST-DISCOVERY-FQN] table exactly — F# backtick names carrying SPACES, the F# `[]` nested-type `+`, and no row data, so a data-driven test's rows collapse onto the one id they share. It must NOT come from `displayName`: MSTest renders that as the BARE method name, which is issue #180 in its MTP shape. The reconstructed id equals the `className` + `.` + `name` pair the TRX report holds, so [TEST-RUN-TRX] attributes MTP outcomes with no change at all. The listing also carries a source location, so MTP rows get a file and line the VSTest path never had. * **`feat(vscode)`: run by `--filter-uid`, report by `--report-trx`.** One invocation per module, uids batched under the Windows command-line ceiling. The uids are LITERAL, so the [TEST-FILTER-ESCAPE] grammar does not apply and must not be used — an NUnit uid is `Ns.Class.Adds_Case(2,2,4)`, and escaping its parentheses would match nothing ([TEST-MTP-RUN]). * **`fix(vscode)`: recover when a bridged framework REFUSES the selection.** NUnit translates `--filter-uid` back into a VSTest filter expression and then rejects its own translation for any uid carrying a SPACE — every idiomatic F# backtick binding. The whole module reported nothing and four runnable tests showed as phantom failures. That module is now re-run ONCE unfiltered and the outcomes read back by name, the rule [TEST-FILTER-ESCAPE] already sets for VSTest. A rejected OPTION earns no retry: it would be rejected again. * **`fix(vscode)`: name the package a module is missing.** `--report-trx` is an extension, not part of MTP; a module without `Microsoft.Testing.Extensions.TrxReport` exits 5. Silence would report every selected test as "No result reported" and hide the cause. * **`fix(vscode)`: find a waiting host behind a prefix.** An MTP module IS the test host and prints `Waiting for debugger to attach... Process Id: …` — the same text as VSTest behind a prefix. Anchored to the start of the line, every MTP debug run hung on a module nothing ever attached to ([TEST-MTP-DEBUG]). * **`fix(vscode)`: read Cobertura at both depths.** MTP collects with `--coverage --coverage-output-format cobertura` and writes `.cobertura.xml` directly into the results directory, where `coverlet.collector` writes one level down ([TEST-COVERAGE]). ## Tests New `testexplorer-mtp` chunk, its own job because it builds a SECOND six-project fixture solution — `xunit.v3`, MSTest and NUnit, each in C# and F#, F# first. Together with `testexplorer-frameworks` it would build twelve test projects in one job and blow the 15-minute ceiling ([DIST-CI-WIN-VSIX]). * **`test-explorer-mtp.test.ts`** — the opt-in and the MSBuild probe, module resolution, the tree for all six fixtures, the MSTest bare display name that must never become an id, and the source location that NUnit does not report and must not be invented. * **`test-explorer-mtp-outcomes.test.ts`** — pass/fail/skip across all six projects with each framework's OWN assertion text, a theory whose rows disagree, the class row, the NUnit refusal recovering and the C# selection that must NOT be retried, and the missing-TrxReport message. * **`test-explorer-mtp-parsers.test.ts`** — the readers at their boundary: the opt-in against every decoy that merely mentions MTP, a BOM, an unknown `schemaVersion`, a missing `type`, the uid batcher, and the pid line in both its bare and prefixed forms. The shared `assertFailed` hardcoded xUnit's `Assert.Equal() Failure`, which no MSTest or NUnit failure carries — it had only ever been fed xUnit fixtures. It now takes the framework's own text, defaulting to xUnit's, so every existing caller is unchanged. ## Refactoring `testing.ts` was 628 lines against a 500 ceiling; it is 480, with the tree builder, the run profiles, the result cache and the `dotnet` queue in their own modules. Deslop found `isRecord` written out identically five times — it now lives in `utils.ts`, re-exported from `dap-emulate` so the ten DAP importers are untouched — and one command-line batcher now serves the assembly list, the VSTest filter and the MTP uids. Duplication measures 15.69% against the 20.0% ceiling. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 --- docs/plans/TEST-EXPLORER-MTP-PLAN.md | 126 ++++++ docs/specs/TEST-EXPLORER-SPEC.md | 170 +++++++- src/editors/vscode/src/dap-emulate.ts | 13 +- src/editors/vscode/src/dap-hot-reload.ts | 6 +- src/editors/vscode/src/launch-profiles.ts | 6 +- src/editors/vscode/src/msbuild.ts | 17 +- src/editors/vscode/src/test-batching.ts | 47 +++ src/editors/vscode/src/test-coverage.ts | 27 +- src/editors/vscode/src/test-debug.ts | 82 +--- src/editors/vscode/src/test-discovery.ts | 159 ++----- src/editors/vscode/src/test-execution.ts | 85 +--- src/editors/vscode/src/test-filter.ts | 27 +- src/editors/vscode/src/test-host-announce.ts | 81 ++++ src/editors/vscode/src/test-items.ts | 153 +++++++ src/editors/vscode/src/test-listing-model.ts | 156 +++++++ src/editors/vscode/src/test-mtp-discovery.ts | 154 +++++++ src/editors/vscode/src/test-mtp-modules.ts | 175 ++++++++ src/editors/vscode/src/test-mtp-run.ts | 312 ++++++++++++++ src/editors/vscode/src/test-mtp.ts | 240 +++++++++++ src/editors/vscode/src/test-profiles.ts | 48 +++ src/editors/vscode/src/test-queue.ts | 37 ++ src/editors/vscode/src/test-result-cache.ts | 71 ++++ src/editors/vscode/src/test-run-output.ts | 45 ++ src/editors/vscode/src/test-targets.ts | 12 + src/editors/vscode/src/test-trx-collect.ts | 90 ++++ .../src/test/suite/dotnet-project-kit.ts | 85 ++++ .../src/test/suite/test-explorer-kit.ts | 22 + .../test/suite/test-explorer-mtp-fixtures.ts | 349 +++++++++++++++ .../suite/test-explorer-mtp-outcomes.test.ts | 320 ++++++++++++++ .../suite/test-explorer-mtp-parsers.test.ts | 398 ++++++++++++++++++ .../src/test/suite/test-explorer-mtp.test.ts | 305 ++++++++++++++ .../suite/test-explorer-outcome-assertions.ts | 17 +- src/editors/vscode/src/testing.ts | 328 ++++----------- src/editors/vscode/src/utils.ts | 12 + src/editors/vscode/test-chunks.json | 10 +- 35 files changed, 3626 insertions(+), 559 deletions(-) create mode 100644 docs/plans/TEST-EXPLORER-MTP-PLAN.md create mode 100644 src/editors/vscode/src/test-batching.ts create mode 100644 src/editors/vscode/src/test-host-announce.ts create mode 100644 src/editors/vscode/src/test-items.ts create mode 100644 src/editors/vscode/src/test-listing-model.ts create mode 100644 src/editors/vscode/src/test-mtp-discovery.ts create mode 100644 src/editors/vscode/src/test-mtp-modules.ts create mode 100644 src/editors/vscode/src/test-mtp-run.ts create mode 100644 src/editors/vscode/src/test-mtp.ts create mode 100644 src/editors/vscode/src/test-profiles.ts create mode 100644 src/editors/vscode/src/test-queue.ts create mode 100644 src/editors/vscode/src/test-result-cache.ts create mode 100644 src/editors/vscode/src/test-trx-collect.ts create mode 100644 src/editors/vscode/src/test/suite/test-explorer-mtp-fixtures.ts create mode 100644 src/editors/vscode/src/test/suite/test-explorer-mtp-outcomes.test.ts create mode 100644 src/editors/vscode/src/test/suite/test-explorer-mtp-parsers.test.ts create mode 100644 src/editors/vscode/src/test/suite/test-explorer-mtp.test.ts diff --git a/docs/plans/TEST-EXPLORER-MTP-PLAN.md b/docs/plans/TEST-EXPLORER-MTP-PLAN.md new file mode 100644 index 00000000..4b0c6922 --- /dev/null +++ b/docs/plans/TEST-EXPLORER-MTP-PLAN.md @@ -0,0 +1,126 @@ +# Test Explorer — Microsoft.Testing.Platform Plan + +Implements [TEST-MTP-DETECT], [TEST-MTP-MODULES], [TEST-MTP-DISCOVERY], [TEST-MTP-RUN] and +[TEST-MTP-DEBUG] in `docs/specs/TEST-EXPLORER-SPEC.md`. Source issue: Nimblesite/SharpLsp#249. + +## Why + +The Test Explorer was VSTest end to end. Microsoft.Testing.Platform (MTP) projects got an +empty Testing view, because every VSTest command fails against them. On the .NET 10 SDK, +MTP v2 removed the VSTest shim and `xunit.v3` 4.0.0 uses MTP v2 by default, so this is now +the usual case, not an edge case. + +## Measured behaviour + +Recorded on this repo's SDK (10.0.303) against real probe projects. These measurements, not +documentation, decide the design. + +| Package set | MTP version | `--list-tests json` | `--filter-uid` | `--report-trx` | +|---|---|---|---|---| +| `xunit.v3` 4.0.0 | 2.3.3 | yes | yes | needs `Microsoft.Testing.Extensions.TrxReport` | +| `MSTest` 4.4.0 | 2.x | yes | yes | built in | +| `NUnit` 4.4.0 + `NUnit3TestAdapter` 6.3.0 | 2.x | yes | yes | needs the TrxReport package | +| `MSTest` 3.11.0 | 1.9.0 | **no** — "expects no arguments" | yes | built in | + +Facts that shaped the code: + +* `dotnet vstest` loads an MSTest MTP module (it keeps a VSTest adapter) but CANNOT load an + `xunit.v3` module. The `xunit.v3` case is the reported defect. +* The JSON listing carries a `type` block (`namespace`, `typeName`, `methodName`). The test + id is built from it. MSTest's `displayName` is the bare method name, so a display name is + never an id. +* The id built from `type` equals the `className` + `.` + `name` pair in the TRX report, for + all three frameworks in both languages. The existing TRX reader needs no change. +* A data-driven test lists one node per row, each with its own uid, all sharing one `type`. + Rows collapse onto one id that owns several uids. +* NUnit sends no `location`. xUnit and MSTest do. +* An unrecognized option exits with code 5 and prints `Unknown option '--x'`. +* `--no-progress` is deprecated in MTP 2.3 and warns on every run. +* MTP coverage writes `.cobertura.xml` directly into the results directory. +* `--debug` prints `Waiting for debugger to attach... Process Id: , Name: `. + The module is the test host; there is no `testhost.dll` child. + +## Design + +`dotnet test` is used for neither MTP discovery nor MTP runs. The test module is asked +directly with `dotnet exec`, which mirrors the two-pass VSTest path: build first, then ask +the built artifact. + +| Step | VSTest path | MTP path | +|---|---|---| +| Build | `dotnet test --list-tests` | `dotnet build` | +| Modules | `Test run for …` banners | `dotnet sln list` + `-getProperty:TargetPath` | +| Names | `dotnet vstest --ListFullyQualifiedTests` | `dotnet exec … --list-tests json` | +| Selection | `--filter FullyQualifiedName=…` | `--filter-uid …` | +| Outcomes | `--logger trx` | `--report-trx` | + +Everything after the TRX file is shared: `test-trx.ts`, the worst-row merge, the reporting +and the status lens. + +## What the work found + +Two defects that only a real fixture could show: + +1. **NUnit refuses an F# selection.** The NUnit bridge translates `--filter-uid` back into a + VSTest filter EXPRESSION and then rejects its own translation for any uid carrying a + SPACE. The whole module then reported nothing, and four runnable F# tests showed as + phantom failures. The remedy is the rule [TEST-FILTER-ESCAPE] already sets: re-run that + module ONCE unfiltered and read the outcomes by name. A rejected OPTION earns no retry — + it would be rejected again. +2. **MSTest 3.11 has no JSON listing.** It carries MTP 1.9.0, whose `--list-tests` takes no + argument, and its text listing is bare method names. Current packages (MSTest 4.4.0, + NUnit3TestAdapter 6.3.0, `xunit.v3` 4.0.0) all carry MTP 2.3 or later, so the fixtures pin + those. A module on an older platform is reported with a warning naming the cause. + +## TODO + +- [x] Spec sections in `docs/specs/TEST-EXPLORER-SPEC.md` +- [x] This plan +- [x] `test-mtp.ts` — runner detection and the JSON listing reader (pure) +- [x] `test-mtp-modules.ts` — build, project list, MSBuild module resolution +- [x] `test-mtp-discovery.ts` — the `--list-tests json` sweep +- [x] `test-mtp-run.ts` — `--filter-uid` runs, per-module TRX, unfiltered retry +- [x] `test-trx-collect.ts` — the TRX collection both runners share +- [x] `test-batching.ts` — one command-line batcher for all three argument lists +- [x] `test-host-announce.ts` — the waiting-host pid reader, now pure and testable +- [x] `msbuild.ts` — `IsTestingPlatformApplication` property +- [x] `test-discovery.ts` — choose the runner, return the MTP plan +- [x] `test-execution.ts` / `testing.ts` — route a run to the MTP plan +- [x] `test-coverage.ts` — read Cobertura at both depths +- [x] `test-debug.ts` — MTP debug environment and prefixed pid line +- [x] MTP fixtures in `dotnet-project-kit.ts` and `test-explorer-mtp-fixtures.ts` +- [x] `test-explorer-mtp-parsers.test.ts` +- [x] `test-explorer-mtp.test.ts` +- [x] `test-explorer-mtp-outcomes.test.ts` +- [x] `test-chunks.json` — the new `testexplorer-mtp` chunk +- [x] `testing.ts` back under 500 lines (628 → 480) +- [ ] Deslop rescan — the CLI and the MCP server were not available in the session that did + this work. The three command-line batchers were unified by hand into + `test-batching.ts`; run `rescan` and `top-offenders` before merge. +- [x] Run the two new e2e suites in the real extension host. + +## Verification run + +`make _test-vsix-shard CHUNK=` on Linux, against the real LSP host and both sidecars: + +| Chunk | Result | Time | +|---|---|---| +| `testexplorer-mtp` (new) | 16 passing | 3 min | +| `testexplorer` | 113 passing | 8 min | +| `testexplorer-frameworks` | 51 passing | 5 min | +| `debug-tests` | 54 passing | 5 min | + +The new chunk is well inside the 15-minute ceiling [DIST-CI-WIN-VSIX] sets. The other three +are the chunks this work touched, and none regressed. + +The first host run found one more defect: the shared `assertFailed` helper hardcoded xUnit's +`Assert.Equal() Failure` text, which no MSTest or NUnit failure carries. It now takes the +framework's own text, defaulting to xUnit's so every existing caller is unchanged, and each +MTP fixture declares the text its framework writes. + +## Not done + +* **MTP server mode** (`--server jsonrpc`) is how Visual Studio and Rider talk to a module. + It would give streaming results, cancellation and locations with no extension packages at + all. The CLI path here is simpler and reuses the whole TRX pipeline. Revisit if the TRX + extension requirement proves a burden for users. diff --git a/docs/specs/TEST-EXPLORER-SPEC.md b/docs/specs/TEST-EXPLORER-SPEC.md index a6ee0e5b..7707731d 100644 --- a/docs/specs/TEST-EXPLORER-SPEC.md +++ b/docs/specs/TEST-EXPLORER-SPEC.md @@ -13,6 +13,11 @@ second-class case here: idiomatic F# backtick bindings produce fully-qualified n contain spaces, and an F# `[]` nested in a module produces a CLR nested-type name containing `+`. Both must survive discovery, filtering and result attribution verbatim. +There are TWO runners, and the Test Explorer supports both. VSTest is the older one. +Microsoft.Testing.Platform (MTP) is the newer one, and it is the default for `xunit.v3` +4.0.0 and later. The two paths share the tree, the TRX reader and every report step; they +differ only in the commands that discover and run the tests ([TEST-MTP-DETECT]). + ```mermaid flowchart LR VIEW["VS Code Testing view"] --> CONTROLLER["SharpLspTestController
testing.ts"] @@ -21,8 +26,11 @@ flowchart LR CONTROLLER --> COVERAGE["coverage — Cobertura
test-coverage.ts"] DISCOVERY --> LISTTESTS["dotnet test --list-tests
builds; announces assemblies"] DISCOVERY --> VSTEST["dotnet vstest
--ListFullyQualifiedTests"] + DISCOVERY --> MTPLIST["dotnet exec module.dll
--list-tests json"] EXECUTION --> RUN["dotnet test
--filter … --logger trx"] + EXECUTION --> MTPRUN["dotnet exec module.dll
--filter-uid … --report-trx"] RUN --> TRX["TRX report
→ per-test outcome
test-trx.ts"] + MTPRUN --> TRX ``` ## Discovery by Fully-Qualified Name `[TEST-DISCOVERY-FQN]` @@ -44,8 +52,9 @@ The listing from pass 1 prints each test's **DisplayName**, not its FullyQualifi xUnit's DisplayName happens to equal `Namespace.Class.Method`, so scraping the listing worked for xUnit by accident; NUnit and MSTest default their DisplayName to the BARE method name, so those tests were dropped outright and could never have been run by FQN filter -(issue #180). The DisplayName listing survives only as a fallback for projects VSTest cannot -load at all (for example a Microsoft.Testing.Platform project). +(issue #180). The DisplayName listing survives only as a last-resort fallback for a project +that neither runner could enumerate. A Microsoft.Testing.Platform project is NOT that case: +it has its own discovery path ([TEST-MTP-DISCOVERY]). Name shapes that MUST round-trip unchanged: @@ -148,6 +157,155 @@ Windows agent. Per-test outcomes come from the TRX report VSTest writes for that failure (a build error) or a note that the filter matched nothing. It is never silently reported as a pass. +## Microsoft.Testing.Platform: which runner `[TEST-MTP-DETECT]` + +Microsoft.Testing.Platform (MTP) is the second .NET test runner. A test project that uses it +builds to an EXECUTABLE test module, and that module — not `vstest.console` — discovers and +runs its own tests. On the .NET 10 SDK, MTP v2 removed the VSTest shim, and `xunit.v3` 4.0.0 +uses MTP v2 by default. Such a project is invisible to every VSTest command: + +* `dotnet test --list-tests --nologo` — `--nologo` is not a valid MTP option. The SDK exits + with code 5 and lists no test. +* `dotnet vstest ` — the module has no VSTest test host, so discovery dies with + "The application to execute does not exist: …testhost.dll". +* `dotnet test --filter … --logger trx` — neither option exists in MTP mode. + +The runner is chosen PER TARGET, not per project. The SDK makes MTP all-or-nothing: when +`global.json` opts in, a VSTest project in the same solution is an error. SharpLsp chooses +in two steps: + +1. Find the nearest `global.json` above the target. Read it with a JSON parser, never with a + regular expression or a string search. `test.runner` equal to `Microsoft.Testing.Platform` + (letter case ignored) selects MTP immediately, and the two doomed VSTest passes are not + run at all. +2. With no opt-in, run the VSTest passes first. Only if they produced no fully-qualified name + does SharpLsp ask MSBuild. A project whose `IsTestingPlatformApplication` property is + `true` is an MTP test module. `IsTestProject` MUST NOT be used for this: `xunit.v3` leaves + it empty. + +This order costs a VSTest solution nothing. It also keeps the MTP probe out of the hot path +for every sweep that already worked. + +## Microsoft.Testing.Platform: the test modules `[TEST-MTP-MODULES]` + +MTP prints no `Test run for ` banner, so the assemblies cannot be scraped out of a +listing. They come from MSBuild, which is the only source that survives a custom +`AssemblyName`, a custom `OutputPath`, an `ArtifactsPath` or a `RuntimeIdentifier`: + +1. `dotnet build ` once. It builds the same projects a VSTest sweep builds. +2. `dotnet sln list` for the projects. The first two lines are a header and are + dropped; the rest are project paths RELATIVE to the solution. With no solution loaded, + the workspace folder is searched for `*.csproj` and `*.fsproj` instead. +3. `dotnet msbuild -getProperty:IsTestingPlatformApplication -getProperty:TargetPath` + per project. `TargetPath` of an MTP project is its test module. + +A multi-targeted project reports one module per target framework. They are ONE project and +MUST collapse to one tree root, by the same union rule as [TEST-DISCOVERY-FQN]. + +## Microsoft.Testing.Platform: discovery `[TEST-MTP-DISCOVERY]` + +Each module is asked directly: `dotnet exec --list-tests json --no-banner`. +`dotnet test` cannot be used here — it does not forward the `json` argument (dotnet/sdk#49754) +— and `dotnet exec` runs the module on every platform without an apphost or an execute bit. + +The module answers with a JSON document on standard output: + +```json +{ "schemaVersion": 1, + "tests": [ { "uid": "9e472c8a…", "displayName": "Cs.Xunit.Mtp.CalculatorTests.Adds_TwoNumbers", + "type": { "namespace": "Cs.Xunit.Mtp", "typeName": "CalculatorTests", + "methodName": "Adds_TwoNumbers" }, + "location": { "file": "…/CalculatorTests.cs", "lineStart": 7, "lineEnd": 7 } } ] } +``` + +Three fields, three jobs, and they MUST NOT be confused: + +* **`type`** gives the test item's id, as `namespace` + `.` + `typeName` + `.` + `methodName`. + The id MUST come from here and never from `displayName`. MSTest reports the BARE method + name as its display name — `Adds_TwoNumbers`, with no namespace and no class — which is the + same defect as issue #180. `type` also carries no row data, so the rows of one data-driven + test collapse onto the one id they share, exactly as the VSTest path requires. The + reconstructed id is identical to the `className` + `.` + `name` pair the TRX report holds, + so [TEST-RUN-TRX] attributes MTP outcomes with no change at all. +* **`uid`** is the run key, and nothing else. Its shape is the framework's business: a SHA-256 + digest for `xunit.v3`, a GUID for MSTest, and the decorated name + `Cs.Nunit.Mtp.CalculatorTests.Adds_Case(2,2,4)` for NUnit. It is never shown and never + parsed. One id can own SEVERAL uids — one per row of a data-driven test — and running that + id runs all of them. +* **`location`** gives the test item its file and line. NUnit sends none, so the field is + optional and its absence is not an error. + +The reader tolerates a leading blank line and a byte-order mark, and it starts at the first +`{`. An unknown `schemaVersion` produces a warning, not an exception. A module that fails to +list leaves the other modules alone, the same contract as [TEST-DISCOVERY-FQN]. + +## Microsoft.Testing.Platform: runs `[TEST-MTP-RUN]` + +One invocation per MODULE for the whole selection, never one per test: + +``` +dotnet exec --filter-uid … \ + --report-trx --report-trx-filename .trx \ + --results-directory --no-banner --no-ansi +``` + +* `--filter-uid` takes LITERAL values, so the [TEST-FILTER-ESCAPE] grammar does not apply and + MUST NOT be used. An NUnit uid contains parentheses and commas; escaping them would make it + match nothing. The uids are still BATCHED against the Windows 32 767-character + command-line ceiling, for the same reason the VSTest filter is. +* An empty selection means "run everything", and then no `--filter-uid` is sent. +* A module the selection does not touch is not started at all. +* `--report-trx-filename` is set per module. Two modules writing one auto-named file in a + shared results directory would overwrite each other, which is the same defect + [TEST-RUN-TRX] avoids with auto-naming under VSTest. +* `--no-progress` MUST NOT be used. MTP 2.3 deprecated it and prints a warning on every run. + +The TRX report is read back by the reader [TEST-RUN-TRX] already specifies, and the worst-row +merge, the skip mapping and the assertion text all behave the same. + +A framework bridged onto MTP can translate `--filter-uid` back into a VSTest filter +EXPRESSION and then REFUSE its own translation. NUnit does exactly that for any uid carrying +a SPACE — which is every idiomatic F# backtick binding, and the same refusal +[TEST-FILTER-ESCAPE] records for the VSTest path: + +``` +Unhandled exception. NUnit.VisualStudio.TestAdapter.TestFilterConverter.TestFilterParserException: +Unexpected FQN 'case\(2,2,4\)' at position 46 in selection expression. +``` + +The whole module then reports nothing, and perfectly runnable tests show as phantom +failures. The remedy is the one [TEST-FILTER-ESCAPE] already sets for VSTest: when a module +FAILED and left a selected test unreported, that module is re-run ONCE without a filter and +the outcomes are picked out of its report by name. Slower, but correct — and only ever when +the module failed, never on a selection that legitimately matched nothing. A retry's counts +REPLACE the refused attempt's; counts are summed only ACROSS modules. + +`--report-trx` is an EXTENSION, not part of MTP. A module that does not register +`Microsoft.Testing.Extensions.TrxReport` rejects the option and exits with code 5, printing +`Unknown option '--report-trx'`. That exit code MUST be reported as itself: the message tells +the user to reference the package. A silent empty run would report every selected test as +"No result reported" and hide the cause. + +Coverage is also an extension. MTP has no `--collect:"XPlat Code Coverage"`; it takes +`--coverage --coverage-output-format cobertura`, and it writes `.cobertura.xml` +DIRECTLY into the results directory, not one level below it as `coverlet.collector` does. +[TEST-COVERAGE] therefore reads both depths. + +## Microsoft.Testing.Platform: debugging `[TEST-MTP-DEBUG]` + +An MTP module IS the test host: there is no `testhost.dll` grandchild. `--debug` makes the +module print + +``` +Waiting for debugger to attach... Process Id: 212243, Name: dotnet +``` + +and then wait. The announcement carries the same `Process Id: , Name: ` text as +VSTest, but a prefix comes before it, so the pid reader MUST find that text anywhere in the +line rather than only at its start. Everything else in [DEBUG-FEATURES-TESTS] is unchanged: +one invocation, attach to the announced pid, mirror the output into the terminal, and write +no result to the cache. + ## Reactivity `[TEST-REACTIVITY]` Discovery runs a full build, so it is NOT a side effect of merely loading a solution. Only @@ -187,7 +345,10 @@ The Run-with-Coverage profile adds `--collect:XPlat Code Coverage` and points directory would show the previous run's report. The collector writes one Cobertura report per test project, each in its own run-id folder one level down, and **every** one of them is parsed into `vscode.FileCoverage` entries and attached to the run; taking only the first drops every -other project's coverage, and which one is "first" is directory order. Per-file detail is +other project's coverage, and which one is "first" is directory order. An MTP run collects +with `--coverage --coverage-output-format cobertura` instead, and that extension writes +`.cobertura.xml` DIRECTLY into the results directory. Both depths are read, so one +rule covers both runners ([TEST-MTP-RUN]). Per-file detail is resolved lazily through `loadDetailedCoverage`. `coverlet.collector` leaves the TEST assembly out of its report by default @@ -218,6 +379,9 @@ the `dotnet` CLI built — never mocks and never a hand-authored `.sln`. The sui | `debug-test-debugging-e2e.test.ts` | the Debug run profile on ONE test: a real DAP session attached to the waiting test host, a breakpoint in the body and in a helper, a failing test, a skipped one, `[Theory]` rows, nothing armed, and disabled/conditional breakpoints | | `debug-test-groups-e2e.test.ts` | debugging a SELECTION: the class row, the namespace row, the assembly root, a multi-select across classes, and the unselected test that must not run | | `debug-test-fsharp-e2e.test.ts` | F# first: a backtick name carrying SPACES debugged, its module helper on the stack, `[]` rows, and Debug Test at the cursor | +| `test-explorer-mtp.test.ts` | Microsoft.Testing.Platform, end to end: the `global.json` opt-in and the `IsTestingPlatformApplication` probe, module resolution through MSBuild, and the tree for `xunit.v3`, MSTest and NUnit × C# and F# — including the MSTest bare display name that MUST NOT become an id, the F# backtick name carrying SPACES, and the source location the JSON listing carries ([TEST-MTP-DETECT], [TEST-MTP-MODULES], [TEST-MTP-DISCOVERY]) | +| `test-explorer-mtp-outcomes.test.ts` | MTP runs: pass, fail and skip attribution across all six projects, the assertion text, a data-driven test whose rows disagree collapsing onto one id, ▶ on one test and on a class row, ⏹, the unfiltered retry an F# NUnit refusal earns and the C# selection that must NOT be retried, and the exit-code-5 message a module without `Microsoft.Testing.Extensions.TrxReport` earns ([TEST-MTP-RUN]) | +| `test-explorer-mtp-parsers.test.ts` | the JSON listing reader at its boundary: a byte-order mark, a leading blank line, an unknown `schemaVersion`, an empty `tests` array, a missing `location`, a missing `type`, and two rows collapsing onto one id with two uids; the `global.json` opt-in against every decoy that merely mentions MTP; the uid batcher; and the waiting-host pid line in BOTH its bare and its prefixed form ([TEST-MTP-DETECT], [TEST-MTP-DISCOVERY], [TEST-MTP-RUN], [TEST-MTP-DEBUG]) | Every suite is declared in `src/editors/vscode/test-chunks.json` so it runs in the Windows matrix ([DIST-CI-WIN-VSIX]). diff --git a/src/editors/vscode/src/dap-emulate.ts b/src/editors/vscode/src/dap-emulate.ts index fc73bede..e5052457 100644 --- a/src/editors/vscode/src/dap-emulate.ts +++ b/src/editors/vscode/src/dap-emulate.ts @@ -9,6 +9,14 @@ /** One DAP message. The index signature keeps fields this file never names * surviving a spread when the router rewrites a message. */ +import { isRecord } from './utils'; + +// The DAP modules have always reached for `isRecord` here, and it is genuinely +// theirs: every wire message body is an untyped bag. The definition now lives in +// `utils.ts`, because five modules outside DAP had written it out identically. +// Re-exported so the ten DAP importers keep one obvious place to get it. +export { isRecord } from './utils'; + export interface DapMessage { type?: unknown; command?: unknown; @@ -18,11 +26,6 @@ export interface DapMessage { [field: string]: unknown; } -/** Narrow an unknown to a plain non-null object. */ -export function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value); -} - /** Narrow an unknown to a list of plain objects, dropping anything else. */ export function recordList(value: unknown): Record[] { return Array.isArray(value) ? value.filter(isRecord) : []; diff --git a/src/editors/vscode/src/dap-hot-reload.ts b/src/editors/vscode/src/dap-hot-reload.ts index 6ebfa83a..007be147 100644 --- a/src/editors/vscode/src/dap-hot-reload.ts +++ b/src/editors/vscode/src/dap-hot-reload.ts @@ -11,7 +11,7 @@ import * as vscode from 'vscode'; import type { DapMessage } from './dap-emulate'; import { error, traceInfo } from './log'; import * as state from './state'; -import { getErrorMessage } from './utils'; +import { getErrorMessage, isRecord } from './utils'; interface HotReloadHost { request(command: string, args: Record): Promise; @@ -545,7 +545,3 @@ function projectsAt(directory: string): string[] { return []; } } - -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value); -} diff --git a/src/editors/vscode/src/launch-profiles.ts b/src/editors/vscode/src/launch-profiles.ts index eac3526a..cab3669d 100644 --- a/src/editors/vscode/src/launch-profiles.ts +++ b/src/editors/vscode/src/launch-profiles.ts @@ -10,6 +10,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { isIgnoredDir } from './launch-target'; +import { isRecord } from './utils'; /** One entry of a `launchSettings.json` / `.run.json` profiles map. */ export interface LaunchProfile { @@ -26,11 +27,6 @@ const URLS_VARIABLE = 'ASPNETCORE_URLS'; /** Only `Project` profiles describe launching the project itself. */ const PROJECT_COMMAND = 'Project'; -/** A plain, non-null object. */ -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value); -} - /** * Sound type guard for a launch-settings document. * diff --git a/src/editors/vscode/src/msbuild.ts b/src/editors/vscode/src/msbuild.ts index 5c3fce12..934d1072 100644 --- a/src/editors/vscode/src/msbuild.ts +++ b/src/editors/vscode/src/msbuild.ts @@ -14,6 +14,7 @@ import * as path from 'node:path'; import { runDotnet } from './dotnet-process'; import { err, ok, type Result } from './result'; +import { isRecord } from './utils'; /** The properties a launch needs from MSBuild. */ export interface ProjectProperties { @@ -25,6 +26,14 @@ export interface ProjectProperties { readonly targetFrameworks: readonly string[]; /** `Exe`, `WinExe` or `Library`. */ readonly outputType: string; + /** + * True when the project builds a Microsoft.Testing.Platform test module. + * + * This is the property the .NET SDK itself uses to tell the two runners + * apart, so it is the one SharpLsp asks. `IsTestProject` MUST NOT be used + * instead: `xunit.v3` leaves it empty. Spec: [TEST-MTP-DETECT]. + */ + readonly isTestingPlatformApplication: boolean; } /** MSBuild evaluation is a full project load; a cold one can take a while. */ @@ -36,6 +45,7 @@ const REQUESTED = [ 'TargetFrameworks', 'OutputType', 'RunCommand', + 'IsTestingPlatformApplication', ] as const; /** `-getProperty:` arguments for a project, optionally pinned to one TFM. */ @@ -48,11 +58,6 @@ function evaluateArgs(projectFile: string, framework?: string): string[] { return args; } -/** A plain, non-null object — the only shape a properties bag can take. */ -function isRecord(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value); -} - /** Every string-valued entry of a bag, ignoring anything else MSBuild emitted. */ function stringEntries(bag: Record): Map { const strings = new Map(); @@ -106,6 +111,8 @@ export async function evaluateProject( targetFramework: bag.value.get('TargetFramework') ?? '', targetFrameworks: splitList(bag.value.get('TargetFrameworks')), outputType: bag.value.get('OutputType') ?? '', + isTestingPlatformApplication: + (bag.value.get('IsTestingPlatformApplication') ?? '').trim().toLowerCase() === 'true', }); } diff --git a/src/editors/vscode/src/test-batching.ts b/src/editors/vscode/src/test-batching.ts new file mode 100644 index 00000000..f5675321 --- /dev/null +++ b/src/editors/vscode/src/test-batching.ts @@ -0,0 +1,47 @@ +/** + * Splitting one argument list into command lines a process can actually take. + * + * Windows caps a process command line at 32 767 characters, and past it Node's + * `spawn` THROWS SYNCHRONOUSLY — `spawn ENAMETOOLONG`, before `dotnet` ever + * runs. Three argument lists reach that: the assemblies handed to one + * `dotnet vstest`, the `--filter` expression of one `dotnet test`, and the + * `--filter-uid` values of one MTP module. All three batch by the same rule, so + * they batch through the same function. + * + * Implements [TEST-DISCOVERY-FQN], [TEST-FILTER-ESCAPE] and [TEST-MTP-RUN]. + */ + +/** + * Ceiling on one argument list. The list is one part of a whole vector — the + * executable, the target, the results directory — so the budget leaves the + * whole vector well under the real limit. + */ +export const MAX_ARG_CHARS = 24_000; + +/** + * Split `items` into batches whose joined argument text stays under `maxChars`. + * + * A single over-budget item still gets its OWN batch: dropping it silently + * would lose a runnable test, and splitting it would corrupt the argument. + */ +export function batchByWidth( + items: readonly T[], + cost: (item: T) => number, + maxChars: number = MAX_ARG_CHARS, +): T[][] { + const batches: T[][] = []; + let current: T[] = []; + let width = 0; + for (const item of items) { + const size = cost(item); + if (current.length > 0 && width + size > maxChars) { + batches.push(current); + current = []; + width = 0; + } + current.push(item); + width += size; + } + if (current.length > 0) batches.push(current); + return batches; +} diff --git a/src/editors/vscode/src/test-coverage.ts b/src/editors/vscode/src/test-coverage.ts index e7ca07f7..d3546b18 100644 --- a/src/editors/vscode/src/test-coverage.ts +++ b/src/editors/vscode/src/test-coverage.ts @@ -46,20 +46,31 @@ const coberturaParser = new XMLParser({ isArray: (tagName) => tagName === 'package' || tagName === 'class' || tagName === 'line', }); +/** Cobertura reports end with this, whichever collector wrote them. */ +const COBERTURA_SUFFIX = '.cobertura.xml'; + /** - * EVERY `coverage.cobertura.xml` one directory below `resultsDir`. + * EVERY Cobertura report of one run, at either depth the collectors use. + * + * `coverlet.collector` — the VSTest path — writes one `coverage.cobertura.xml` + * per test project, each into its own run-id folder ONE LEVEL DOWN. Taking only + * the first silently dropped every other project's coverage from a + * solution-wide run, and which one "first" meant depended on directory order. * - * The collector writes one report per test project, each into its own - * run-id folder. Taking only the first — as this did — silently dropped every - * other project's coverage from a solution-wide run, and which one "first" meant - * depended on directory order. + * The Microsoft.Testing.Platform collector writes `.cobertura.xml` + * DIRECTLY into the results directory instead ([TEST-MTP-RUN]). Both depths are + * read, so one rule serves both runners and neither loses a report. */ export function findCoberturaFiles(resultsDir: string): string[] { if (!fs.existsSync(resultsDir)) return []; const reports: string[] = []; - for (const entry of fs.readdirSync(resultsDir)) { - const candidate = path.join(resultsDir, entry, 'coverage.cobertura.xml'); - if (fs.existsSync(candidate)) reports.push(candidate); + for (const entry of fs.readdirSync(resultsDir, { withFileTypes: true })) { + if (entry.isFile() && entry.name.toLowerCase().endsWith(COBERTURA_SUFFIX)) { + reports.push(path.join(resultsDir, entry.name)); + continue; + } + const nested = path.join(resultsDir, entry.name, 'coverage.cobertura.xml'); + if (entry.isDirectory() && fs.existsSync(nested)) reports.push(nested); } return reports.sort(); } diff --git a/src/editors/vscode/src/test-debug.ts b/src/editors/vscode/src/test-debug.ts index 28ce1985..089ce16a 100644 --- a/src/editors/vscode/src/test-debug.ts +++ b/src/editors/vscode/src/test-debug.ts @@ -24,7 +24,8 @@ import { DEBUG_TYPE } from './constants'; import { whenDebugSessionArmed } from './debug'; import { TEST_HOST_ATTACH_FLAG } from './dap-attach'; import { error, info, warn } from './log'; -import { runTests, type TestRunOptions, type TestRunOutcome } from './test-execution'; +import type { TestRunOptions, TestRunOutcome } from './test-execution'; +import { TestHostWatcher } from './test-host-announce'; import { filterBatches } from './test-filter'; import { runTarget } from './test-targets'; @@ -40,11 +41,19 @@ import { runTarget } from './test-targets'; export const TEST_HOST_DEBUG_ENV: Readonly> = { VSTEST_HOST_DEBUG: '1', VSTEST_RUNNER_DEBUG: '0', + // A Microsoft.Testing.Platform module IS the test host — there is no + // `testhost.dll` child — and it waits on its OWN variable. Both are set + // because each runner ignores the other's, so one environment serves both + // and no runner flag has to be threaded through this flow. + // Spec: [TEST-MTP-DEBUG]. + TESTINGPLATFORM_WAIT_ATTACH_DEBUGGER: '1', }; /** The terminal the Debug profile mirrors the run's output into. */ export const TEST_DEBUG_TERMINAL_NAME = 'SharpLsp Test Debug'; +export { announcedTestHostPid, TestHostWatcher } from './test-host-announce'; + /** * How long a debug run may live. A debuggee parked on a breakpoint is the * POINT of the exercise, so the ordinary `dotnet` ceiling (10 minutes) would @@ -53,73 +62,24 @@ export const TEST_DEBUG_TERMINAL_NAME = 'SharpLsp Test Debug'; */ const DEBUG_RUN_CEILING_MS = 24 * 60 * 60 * 1_000; -/** The stable prefix of VSTest's waiting-host announcement, en-US pinned. */ -const PROCESS_ID_PREFIX = 'Process Id:'; - /** What the debug flow needs from the owning test controller. */ export interface TestDebugHost { /** Serialise behind every other `dotnet` invocation the controller makes. */ enqueue(work: () => Promise): Promise; + /** + * Start the selection with whichever runner the last discovery sweep chose. + * The debug flow is identical for both; only the command differs. + * Spec: [TEST-MTP-DEBUG]. + */ + runSelection( + ids: readonly string[], + cwd: string, + options: TestRunOptions, + ): Promise; /** Report a finished invocation onto the RUN — never onto the result cache. */ finish(run: vscode.TestRun, tests: readonly vscode.TestItem[], outcome: TestRunOutcome): void; } -/** ASCII digits only, checked per UTF-16 unit — a pid is never a surrogate. */ -function isAllDigits(candidate: string): boolean { - for (let index = 0; index < candidate.length; index += 1) { - const code = candidate.charCodeAt(index); - if (code < 0x30 || code > 0x39) return false; - } - return true; -} - -/** - * The pid a waiting test host announced on `line`, or undefined. - * - * The contract is VSTest's own console line, `Process Id: {0}, Name: {1}`, - * printed by the HOST about itself — the parent never prints it with - * `VSTEST_RUNNER_DEBUG` pinned off. The digits are validated whole: a partial - * `parseInt` would accept a corrupted line and aim the debugger at noise. - */ -export function announcedTestHostPid(line: string): number | undefined { - const trimmed = line.trim(); - if (!trimmed.startsWith(PROCESS_ID_PREFIX)) return undefined; - const rest = trimmed.slice(PROCESS_ID_PREFIX.length); - const comma = rest.indexOf(','); - const digits = (comma === -1 ? rest : rest.slice(0, comma)).trim(); - if (digits.length === 0 || !isAllDigits(digits)) return undefined; - const pid = Number.parseInt(digits, 10); - return Number.isSafeInteger(pid) && pid > 0 ? pid : undefined; -} - -/** - * Watches a debug run's live output for waiting test hosts, once each. - * - * Chunk boundaries fall anywhere, so lines are reassembled before parsing; a - * solution with several test projects announces one host PER ASSEMBLY, and - * every one of them is waiting — each new pid is handed on exactly once. - */ -export class TestHostWatcher { - private tail = ''; - private readonly announced = new Set(); - - constructor(private readonly onHost: (pid: number) => void) {} - - /** Feed one raw output chunk; complete lines are scanned for announcements. */ - public absorb(chunk: string): void { - const lines = (this.tail + chunk).split('\n'); - this.tail = lines.pop() ?? ''; - for (const line of lines) this.offer(line); - } - - private offer(line: string): void { - const pid = announcedTestHostPid(line); - if (pid === undefined || this.announced.has(pid)) return; - this.announced.add(pid); - this.onHost(pid); - } -} - /** The attach configuration aimed at one waiting test host. */ export function testHostAttachConfig(pid: number, label: string): vscode.DebugConfiguration { return { @@ -310,7 +270,7 @@ class DebugRunFlow { options: TestRunOptions, ): Promise { const { started } = await this.host.enqueue(async () => { - const running = runTests(ids, this.cwd, options); + const running = this.host.runSelection(ids, this.cwd, options); await Promise.race([this.attached, running]); return { started: running }; }); diff --git a/src/editors/vscode/src/test-discovery.ts b/src/editors/vscode/src/test-discovery.ts index 270b0b62..a725623b 100644 --- a/src/editors/vscode/src/test-discovery.ts +++ b/src/editors/vscode/src/test-discovery.ts @@ -28,11 +28,21 @@ import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; import { DOTNET_TIMEOUT_MS, runDotnet, type DotnetRun } from './dotnet-process.js'; +import { batchByWidth, MAX_ARG_CHARS } from './test-batching.js'; import { parseTestList } from './test-listing.js'; import { HEX_DIGITS, parseFullyQualifiedTestList } from './test-names.js'; +import { + mergeMultiTargeted, + type TestAssemblyListing, + type TestListing, +} from './test-listing-model.js'; +import { listMtpTests } from './test-mtp-discovery.js'; +import { usesMtpRunner } from './test-mtp.js'; export { parseFullyQualifiedTestList, withoutAdapterUniqueId } from './test-names.js'; export { isDiscoveredTestLine, parseTestList } from './test-listing.js'; +export { mergeMultiTargeted } from './test-listing-model.js'; +export type { TestAssemblyListing, TestListing } from './test-listing-model.js'; /** VSTest prints one of these per test assembly it was handed. */ const ASSEMBLY_BANNER = 'Test run for '; @@ -48,37 +58,11 @@ const VSTEST_LISTING_OUTPUT = '-p:VsTestUseMSBuildOutput=false'; /** * Ceiling on the assembly arguments handed to a single `dotnet vstest`. - * Windows caps a process command line at 32 767 characters, and a solution with - * many test projects — each contributing a long `bin/Debug/netX/Name.dll` path — - * reaches that, at which point the spawn fails outright instead of enumerating. + * A solution with many test projects — each contributing a long + * `bin/Debug/netX/Name.dll` path — reaches the Windows command-line limit, at + * which point the spawn fails outright instead of enumerating. */ -const MAX_ASSEMBLY_ARG_CHARS = 24_000; - -/** The outcome of enumerating one target. Never an exception. */ -export interface TestListing { - /** Fully-qualified names, in discovery order, de-duplicated. */ - readonly names: readonly string[]; - /** True when the enumeration ran to completion (so an empty list is real). */ - readonly ok: boolean; - /** Diagnostics worth writing to the extension log. */ - readonly warnings: readonly string[]; - /** - * The names grouped by the assembly that contributed them — the grouping the - * Test Explorer renders as Assembly → Namespace → Class → Test. Empty for - * the weaker display-name fallback, which cannot attribute names. - */ - readonly byAssembly: readonly TestAssemblyListing[]; -} - -/** One built test assembly and the fully-qualified names it contributed. */ -export interface TestAssemblyListing { - /** Assembly file name without extension — the tree's root label. */ - readonly name: string; - /** Absolute path of the built assembly — the stable, unique group id. */ - readonly path: string; - /** Fully-qualified test names this assembly contributed, in listing order. */ - readonly names: readonly string[]; -} +const MAX_ASSEMBLY_ARG_CHARS = MAX_ARG_CHARS; /** * Extract the assembly path from a `Test run for ()` banner. @@ -167,21 +151,7 @@ export function batchAssemblies( assemblies: readonly string[], maxChars: number = MAX_ASSEMBLY_ARG_CHARS, ): string[][] { - const batches: string[][] = []; - let current: string[] = []; - let width = 0; - for (const assembly of assemblies) { - const cost = assembly.length + 3; - if (current.length > 0 && width + cost > maxChars) { - batches.push(current); - current = []; - width = 0; - } - current.push(assembly); - width += cost; - } - if (current.length > 0) batches.push(current); - return batches; + return batchByWidth(assemblies, (assembly) => assembly.length + 3, maxChars); } /** @@ -202,6 +172,29 @@ export async function listTests( byAssembly: [], }; } + // [TEST-MTP-DETECT]: a `global.json` opt-in makes the whole target MTP, and + // every VSTest command below would fail against it — `--nologo` alone exits + // with code 5 and lists nothing. Go straight to the runner that can answer. + if (usesMtpRunner(cwd)) return await listMtpTests(target, cwd, timeoutMs); + const vstest = await listWithVsTest(target, cwd, timeoutMs); + // No assembly attributed a name: either a genuinely empty solution, or an MTP + // project with no opt-in — which MTP v2 on the .NET 10 SDK makes ordinary, + // because it removed the VSTest shim. Ask MSBuild before settling for the + // display-name fallback, which cannot run anything it lists. + if (vstest.byAssembly.length > 0) return vstest; + const mtp = await listMtpTests(target, cwd, timeoutMs); + if (mtp.names.length === 0) { + return { ...vstest, warnings: [...vstest.warnings, ...mtp.warnings] }; + } + return { ...mtp, warnings: [...vstest.warnings, ...mtp.warnings] }; +} + +/** The two VSTest passes: build and announce, then ask for the real names. */ +async function listWithVsTest( + target: string, + cwd: string, + timeoutMs: number, +): Promise { const positional = cwd === target ? [] : [target]; const args = [ 'test', @@ -259,82 +252,6 @@ function listFailure(run: DotnetRun): string { : `dotnet test --list-tests failed: ${cause}${detail}`; } -/** - * Collapse the assemblies ONE multi-targeted project produced into one listing. - * - * `dotnet test --list-tests` announces a `Test run for …` banner per TARGET - * FRAMEWORK, so a project declaring `net8.0;net9.0` reports - * two assemblies carrying the same file name under different - * `bin///` directories. That is one project, and so one root of the - * Assembly → Namespace → Class → Test tree: keeping them apart rendered every - * namespace, class and test of that project TWICE, under two labels the user - * cannot tell apart. - * - * Names are UNIONED, never taken from whichever framework was announced first: a - * test compiled behind `#if NET8_0` exists in only one of the assemblies, and - * dropping it would trade a duplicated tree for a missing test. The surviving - * path is the identity the frameworks SHARE (see {@link sharedOutputPath}), so - * the group ids the tree builds from it stay put across sweeps however the - * build ordered its banners. - */ -export function mergeMultiTargeted( - listings: readonly TestAssemblyListing[], -): TestAssemblyListing[] { - const merged = new Map(); - for (const listing of listings) { - const existing = merged.get(listing.name); - if (existing === undefined) { - merged.set(listing.name, { paths: [listing.path], names: [...listing.names] }); - continue; - } - existing.paths.push(listing.path); - existing.names.push(...listing.names); - } - return [...merged].map(([name, entry]) => ({ - name, - path: sharedOutputPath(entry.paths), - names: [...new Set(entry.names)], - })); -} - -/** - * The identity several builds of ONE assembly share. - * - * Two target frameworks put the same assembly under `bin//net8.0/` and - * `bin//net9.0/`, so their paths agree everywhere except the segments - * that name a build. Keeping one of them as the merged group id keys the whole - * project's tree on a framework it merely happens to target: the id moves the - * moment the build announces its banners in another order, and a project - * targeting four frameworks gets a row identified by exactly one of them. - * - * Taking the common prefix back to its last separator and re-attaching the file - * name leaves what every build of the project agrees on. A project with one - * target framework has nothing to reconcile and keeps its real path. - */ -function sharedOutputPath(paths: readonly string[]): string { - const [first, ...rest] = paths; - if (first === undefined) return ''; - if (rest.length === 0) return first; - const shared = rest.reduce(commonPrefix, first); - return shared.slice(0, lastSeparator(shared) + 1) + first.slice(lastSeparator(first) + 1); -} - -/** The leading characters two paths agree on. */ -function commonPrefix(left: string, right: string): string { - let index = 0; - while (index < left.length && index < right.length && left[index] === right[index]) index += 1; - return left.slice(0, index); -} - -/** - * Index of the last `/` or `\`, or -1. Both are checked rather than `path.sep` - * because the separator comes from whichever host BUILT the listing, which is - * not necessarily the one reading it. - */ -function lastSeparator(value: string): number { - return Math.max(value.lastIndexOf('/'), value.lastIndexOf('\\')); -} - /** Prefer VSTest's fully-qualified names; fall back to the display listing. */ async function namesFrom(output: string, cwd: string, timeoutMs: number): Promise { const announced = parseAnnouncedAssemblies(output); diff --git a/src/editors/vscode/src/test-execution.ts b/src/editors/vscode/src/test-execution.ts index 67cdaa25..c2cd8cb6 100644 --- a/src/editors/vscode/src/test-execution.ts +++ b/src/editors/vscode/src/test-execution.ts @@ -24,13 +24,9 @@ import type * as vscode from 'vscode'; import { DOTNET_TIMEOUT_MS, runDotnet, type DotnetHooks } from './dotnet-process.js'; import { runTarget } from './test-targets.js'; import { filterBatches, filterExpression } from './test-filter.js'; -import { - parseFailureMessage, - parseRunSummary, - type TestOutcome, - type TestRunSummary, -} from './test-run-output.js'; -import { isRunError, parseTrxReport, type TrxRunInfo, type TrxTestResult } from './test-trx.js'; +import { parseFailureMessage, parseRunSummary, type TestRunSummary } from './test-run-output.js'; +import { isRunError, type TrxRunInfo, type TrxTestResult } from './test-trx.js'; +import { collectReport, trxFiles } from './test-trx-collect.js'; /** What one `dotnet test` invocation produced. */ export interface TestRunOutcome { @@ -295,78 +291,3 @@ function runFailure( if (!failed || resultCount > 0) return undefined; return parseFailureMessage(output) ?? errorMessage ?? 'dotnet test failed'; } - -/** Every `.trx` this run created, merged: results keyed by FQN, plus run info. */ -function collectReport( - dir: string, - before: ReadonlySet, -): { results: Map; runInfos: TrxRunInfo[] } { - const results = new Map(); - const runInfos: TrxRunInfo[] = []; - for (const file of trxFiles(dir)) { - if (before.has(file)) continue; - const report = readTrx(file); - runInfos.push(...report.runInfos); - for (const result of report.results) { - const existing = results.get(result.fullyQualifiedName); - results.set( - result.fullyQualifiedName, - existing === undefined ? result : worse(existing, result), - ); - } - } - return { results, runInfos }; -} - -/** Severity order, so a data-driven test is judged by its WORST row. */ -const OUTCOME_SEVERITY: Record = { - passed: 0, - skipped: 1, - notRun: 2, - failed: 3, -}; - -/** - * Merge two results reported under the SAME fully-qualified name. - * - * A theory or `[TestCase]` with several rows writes one TRX entry PER ROW, all - * carrying the same FQN. Keeping the last one seen would report a green tree for - * a theory whose second row failed, purely because of the order VSTest happened - * to write them. The worst outcome wins and the durations add up. - */ -function worse(left: TrxTestResult, right: TrxTestResult): TrxTestResult { - const durationMs = sumDurations(left.durationMs, right.durationMs); - const dominant = OUTCOME_SEVERITY[right.outcome] > OUTCOME_SEVERITY[left.outcome] ? right : left; - return { ...dominant, durationMs }; -} - -/** Add two optional durations, keeping `undefined` only when both are absent. */ -function sumDurations(left: number | undefined, right: number | undefined): number | undefined { - if (left === undefined) return right; - if (right === undefined) return left; - return left + right; -} - -/** Absolute paths of the `.trx` reports directly inside `dir`. */ -function trxFiles(dir: string): string[] { - try { - return fs - .readdirSync(dir) - .filter((entry) => entry.toLowerCase().endsWith('.trx')) - .map((entry) => path.join(dir, entry)); - } catch { - return []; - } -} - -/** Parse one TRX file, tolerating a truncated or unreadable report. */ -function readTrx(file: string): { - results: readonly TrxTestResult[]; - runInfos: readonly TrxRunInfo[]; -} { - try { - return parseTrxReport(fs.readFileSync(file, 'utf8')); - } catch { - return { results: [], runInfos: [] }; - } -} diff --git a/src/editors/vscode/src/test-filter.ts b/src/editors/vscode/src/test-filter.ts index 9aa93bd1..23a6db3d 100644 --- a/src/editors/vscode/src/test-filter.ts +++ b/src/editors/vscode/src/test-filter.ts @@ -11,6 +11,8 @@ * Implements [TEST-FILTER-ESCAPE]. */ +import { batchByWidth, MAX_ARG_CHARS } from './test-batching.js'; + /** Characters VSTest's filter grammar reserves; each is escaped with a backslash. */ const FILTER_METACHARACTERS = new Set(['\\', '(', ')', '&', '|', '=', '!', '~']); @@ -45,17 +47,12 @@ export function filterExpression(fullyQualifiedNames: readonly string[]): string * Windows caps a process command line at 32 767 characters, and past it * Node's `spawn` THROWS SYNCHRONOUSLY (issue: 816 discovered tests, ▶ on the * root of the Testing view, `spawn ENAMETOOLONG` rejected the run handler). - * The filter is one argv entry among several — exe, target, `--logger trx`, - * `--results-directory ` — so the budget keeps the WHOLE vector well - * under the ceiling. Mirrors `MAX_ASSEMBLY_ARG_CHARS` in discovery. */ -export const MAX_FILTER_ARG_CHARS = 24_000; +export const MAX_FILTER_ARG_CHARS = MAX_ARG_CHARS; /** * Split fully-qualified names into batches whose joined filter expression - * stays under the Windows command-line ceiling. A single over-budget name - * still gets its own batch: dropping it silently would lose a runnable test, - * and splitting a NAME would corrupt the filter. + * stays under the Windows command-line ceiling. * * The cost of a name is its escaped clause plus the joining `|` — escaping can * GROW the text (every `(` gains a backslash), so the clause is measured, not @@ -65,19 +62,5 @@ export function filterBatches( fullyQualifiedNames: readonly string[], maxChars: number = MAX_FILTER_ARG_CHARS, ): string[][] { - const batches: string[][] = []; - let current: string[] = []; - let width = 0; - for (const name of fullyQualifiedNames) { - const cost = filterClause(name).length + 1; - if (current.length > 0 && width + cost > maxChars) { - batches.push(current); - current = []; - width = 0; - } - current.push(name); - width += cost; - } - if (current.length > 0) batches.push(current); - return batches; + return batchByWidth(fullyQualifiedNames, (name) => filterClause(name).length + 1, maxChars); } diff --git a/src/editors/vscode/src/test-host-announce.ts b/src/editors/vscode/src/test-host-announce.ts new file mode 100644 index 00000000..77e69072 --- /dev/null +++ b/src/editors/vscode/src/test-host-announce.ts @@ -0,0 +1,81 @@ +/** + * Reading a waiting test host's pid out of a debug run's live output. + * + * Under a debug run every test host announces itself and then BLOCKS until a + * debugger attaches, so this text is the only signal saying which process to + * attach to. Both runners print it: VSTest from its `testhost.dll` child, and a + * Microsoft.Testing.Platform module about itself. + * + * Pure — no VS Code, no process — so it is asserted at its own boundary rather + * than only through a real debug session. + * + * Implements [DEBUG-FEATURES-TESTS] and [TEST-MTP-DEBUG]. + */ + +/** The stable text of a waiting host's announcement, en-US pinned. */ +const PROCESS_ID_PREFIX = 'Process Id:'; + +/** ASCII digits only, checked per UTF-16 unit — a pid is never a surrogate. */ +function isAllDigits(candidate: string): boolean { + for (let index = 0; index < candidate.length; index += 1) { + const code = candidate.charCodeAt(index); + if (code < 0x30 || code > 0x39) return false; + } + return true; +} + +/** + * The pid a waiting test host announced on `line`, or undefined. + * + * The contract is the console line `Process Id: {0}, Name: {1}`, printed by the + * HOST about itself — the parent never prints it with `VSTEST_RUNNER_DEBUG` + * pinned off. + * + * A Microsoft.Testing.Platform module prints the SAME text behind a prefix: + * `Waiting for debugger to attach... Process Id: 212243, Name: dotnet`. The + * text is therefore found ANYWHERE in the line rather than only at its start — + * anchored to the start, every MTP debug run hung on a module nothing ever + * attached to. Spec: [TEST-MTP-DEBUG]. + * + * The digits are validated whole: a partial `parseInt` would accept a corrupted + * line and aim the debugger at noise. + */ +export function announcedTestHostPid(line: string): number | undefined { + const trimmed = line.trim(); + const marker = trimmed.indexOf(PROCESS_ID_PREFIX); + if (marker < 0) return undefined; + const rest = trimmed.slice(marker + PROCESS_ID_PREFIX.length); + const comma = rest.indexOf(','); + const digits = (comma === -1 ? rest : rest.slice(0, comma)).trim(); + if (digits.length === 0 || !isAllDigits(digits)) return undefined; + const pid = Number.parseInt(digits, 10); + return Number.isSafeInteger(pid) && pid > 0 ? pid : undefined; +} + +/** + * Watches a debug run's live output for waiting test hosts, once each. + * + * Chunk boundaries fall anywhere, so lines are reassembled before parsing; a + * solution with several test projects announces one host PER ASSEMBLY, and + * every one of them is waiting — each new pid is handed on exactly once. + */ +export class TestHostWatcher { + private tail = ''; + private readonly announced = new Set(); + + constructor(private readonly onHost: (pid: number) => void) {} + + /** Feed one raw output chunk; complete lines are scanned for announcements. */ + public absorb(chunk: string): void { + const lines = (this.tail + chunk).split('\n'); + this.tail = lines.pop() ?? ''; + for (const line of lines) this.offer(line); + } + + private offer(line: string): void { + const pid = announcedTestHostPid(line); + if (pid === undefined || this.announced.has(pid)) return; + this.announced.add(pid); + this.onHost(pid); + } +} diff --git a/src/editors/vscode/src/test-items.ts b/src/editors/vscode/src/test-items.ts new file mode 100644 index 00000000..6ad47145 --- /dev/null +++ b/src/editors/vscode/src/test-items.ts @@ -0,0 +1,153 @@ +/** + * Building the rows the Testing view shows. + * + * The tree is **Assembly → Namespace → Class → Test**, and only the leaves are + * tests. It lives apart from `testing.ts` so the controller file stays about + * the VS Code Testing API wiring and nothing else. + * + * Both runners build the same tree from the same ids, because both discovery + * paths report `namespace.Type.Method` ([TEST-DISCOVERY-FQN], + * [TEST-MTP-DISCOVERY]). A Microsoft.Testing.Platform sweep also reports where + * each test is WRITTEN, so its leaves carry a file and a line; a VSTest sweep + * reports none, and its leaves carry the target folder as before. + * + * Implements [TEST-EXPLORER]. + */ + +import * as vscode from 'vscode'; +import type { TestAssemblyListing, TestLocation } from './test-listing-model.js'; +import { isExpectoTest, isFsCheckTest } from './test-targets.js'; + +/** + * Id prefix marking the row that explains WHY discovery failed. Error rows are + * leaves that never run; a successful sweep removes them. + */ +export const ERROR_ITEM_PREFIX = 'discovery-error:'; + +/** What every row of one sweep is built from. */ +export interface ItemContext { + readonly controller: vscode.TestController; + /** Fallback uri for a row whose test reported no source file. */ + readonly uri: vscode.Uri; + /** Source location per test id, when the runner reported one. */ + readonly locations: ReadonlyMap | undefined; +} + +/** The uri a test row points at: its own source file, else the target folder. */ +function uriFor(context: ItemContext, fullName: string): vscode.Uri { + const location = context.locations?.get(fullName); + return location === undefined ? context.uri : vscode.Uri.file(location.file); +} + +/** The one-line range a test row selects, when its line is known. */ +function rangeFor(context: ItemContext, fullName: string): vscode.Range | undefined { + const line = context.locations?.get(fullName)?.line; + if (line === undefined) return undefined; + const zeroBased = Math.max(0, line - 1); + return new vscode.Range(zeroBased, 0, zeroBased, 0); +} + +/** Build a TestItem for a fully-qualified name, tagging F# tests. */ +export function makeTestItem(context: ItemContext, fullName: string): vscode.TestItem { + const parts = fullName.split('.'); + const label = parts.at(-1) ?? fullName; + const item = context.controller.createTestItem(fullName, label, uriFor(context, fullName)); + item.description = fullName; + item.range = rangeFor(context, fullName); + if (isExpectoTest(fullName) || isFsCheckTest(fullName)) { + item.tags = [new vscode.TestTag('fsharp')]; + } + return item; +} + +/** A non-test group node: an assembly, a namespace or a class. */ +function makeGroupItem(context: ItemContext, id: string, label: string): vscode.TestItem { + const item = context.controller.createTestItem(id, label, context.uri); + item.canResolveChildren = true; + return item; +} + +/** The namespace and class labels one fully-qualified name sits under. */ +function levelsOf(fullName: string): { namespaceLabel: string; classLabel: string } { + const parts = fullName.split('.'); + return { + namespaceLabel: parts.length >= 3 ? parts.slice(0, -2).join('.') : '', + classLabel: parts.length >= 2 ? (parts.at(-2) ?? '') : '', + }; +} + +/** Find or create one level of the tree under `parent`. */ +function levelItem( + context: ItemContext, + cache: Map, + parent: vscode.TestItem, + level: { id: string; label: string }, +): vscode.TestItem { + const existing = cache.get(level.id); + if (existing !== undefined) return existing; + const item = makeGroupItem(context, level.id, level.label); + cache.set(level.id, item); + parent.children.add(item); + return item; +} + +/** + * Build one assembly's Assembly → Namespace → Class → Test subtree. + * + * The last dotted segment is the test, the one before it the class, the rest + * joined the namespace — deterministic for C# namespaces and dotted F# modules + * alike (`Fs.Xunit.Fixtures.adds two numbers` → `Fs.Xunit` / `Fixtures` / + * `adds two numbers`). Shorter names nest under whatever levels exist; nothing + * is ever dropped. + */ +export function makeAssemblyItem( + context: ItemContext, + assembly: TestAssemblyListing, +): vscode.TestItem { + const root = makeGroupItem(context, `assembly:${assembly.path}`, assembly.name); + const namespaces = new Map(); + const classes = new Map(); + for (const fqn of assembly.names) { + const { namespaceLabel, classLabel } = levelsOf(fqn); + let parent = root; + if (namespaceLabel !== '') { + parent = levelItem(context, namespaces, parent, { + id: `namespace:${assembly.path}|${namespaceLabel}`, + label: namespaceLabel, + }); + } + if (classLabel !== '') { + parent = levelItem(context, classes, parent, { + id: `class:${assembly.path}|${namespaceLabel}|${classLabel}`, + label: classLabel, + }); + } + parent.children.add(makeTestItem(context, fqn)); + } + return root; +} + +/** + * The row that explains WHY discovery failed: the real `dotnet` diagnostic plus + * a remedy, so the user acts instead of staring at an empty view. + */ +export function makeErrorItem( + context: ItemContext, + target: string, + warnings: readonly string[], +): vscode.TestItem { + const item = context.controller.createTestItem( + `${ERROR_ITEM_PREFIX}${target}`, + 'Test discovery failed', + context.uri, + ); + item.description = target; + const diagnostics = + warnings.length > 0 ? warnings.join('\n\n') : 'dotnet test produced no test listing.'; + item.error = new vscode.MarkdownString( + `SharpLsp could not enumerate tests for \`${target}\`.\n\n` + + `${diagnostics}\n\n` + + 'Load one solution with the **SharpLsp: Select Solution** command, fix the build errors above, then refresh the Testing view.', + ); + return item; +} diff --git a/src/editors/vscode/src/test-listing-model.ts b/src/editors/vscode/src/test-listing-model.ts new file mode 100644 index 00000000..60f5beb8 --- /dev/null +++ b/src/editors/vscode/src/test-listing-model.ts @@ -0,0 +1,156 @@ +/** + * The shape a discovery sweep reports, whichever runner produced it. + * + * Both runners fill the same model — VSTest through + * `dotnet vstest --ListFullyQualifiedTests`, Microsoft.Testing.Platform through + * a module's own `--list-tests json` — so the Test Explorer builds one tree and + * the two paths never diverge past this point. + * + * Implements [TEST-DISCOVERY-FQN] and [TEST-MTP-DISCOVERY]. + */ + +/** The outcome of enumerating one target. Never an exception. */ +export interface TestListing { + /** Fully-qualified names, in discovery order, de-duplicated. */ + readonly names: readonly string[]; + /** True when the enumeration ran to completion (so an empty list is real). */ + readonly ok: boolean; + /** Diagnostics worth writing to the extension log. */ + readonly warnings: readonly string[]; + /** + * The names grouped by the assembly that contributed them — the grouping the + * Test Explorer renders as Assembly → Namespace → Class → Test. Empty for + * the weaker display-name fallback, which cannot attribute names. + */ + readonly byAssembly: readonly TestAssemblyListing[]; + /** + * How to RUN what was discovered, when the runner was + * Microsoft.Testing.Platform. Absent for a VSTest sweep, whose ids are the + * filter values themselves. Spec: [TEST-MTP-RUN]. + */ + readonly mtp?: MtpRunPlan; + /** + * Source location per test id, when the runner reported one. The VSTest path + * reports none; MTP reports one for every framework except NUnit. + */ + readonly locations?: ReadonlyMap; +} + +/** Where a test is written, as the discovery pass reported it. */ +export interface TestLocation { + readonly file: string; + /** 1-based first line, when the framework reported one. */ + readonly line: number | undefined; +} + +/** Everything an MTP run needs that the test ids do not carry themselves. */ +export interface MtpRunPlan { + readonly modules: readonly MtpModuleRun[]; +} + +/** One test module, and the uids each of its test ids owns. */ +export interface MtpModuleRun { + /** Absolute path of the built test module. */ + readonly modulePath: string; + /** Test id to the `--filter-uid` values that run it. */ + readonly uidsById: ReadonlyMap; +} + +/** One built test assembly and the fully-qualified names it contributed. */ +export interface TestAssemblyListing { + /** Assembly file name without extension — the tree's root label. */ + readonly name: string; + /** Absolute path of the built assembly — the stable, unique group id. */ + readonly path: string; + /** Fully-qualified test names this assembly contributed, in listing order. */ + readonly names: readonly string[]; +} + +/** + * Collapse the assemblies ONE multi-targeted project produced into one listing. + * + * `dotnet test --list-tests` announces a `Test run for …` banner per TARGET + * FRAMEWORK, so a project declaring `net8.0;net9.0` reports + * two assemblies carrying the same file name under different + * `bin///` directories. That is one project, and so one root of the + * Assembly → Namespace → Class → Test tree: keeping them apart rendered every + * namespace, class and test of that project TWICE, under two labels the user + * cannot tell apart. + * + * Names are UNIONED, never taken from whichever framework was announced first: a + * test compiled behind `#if NET8_0` exists in only one of the assemblies, and + * dropping it would trade a duplicated tree for a missing test. The surviving + * path is the identity the frameworks SHARE (see {@link sharedOutputPath}), so + * the group ids the tree builds from it stay put across sweeps however the + * build ordered its banners. + */ +export function mergeMultiTargeted( + listings: readonly TestAssemblyListing[], +): TestAssemblyListing[] { + const merged = new Map(); + for (const listing of listings) { + const existing = merged.get(listing.name); + if (existing === undefined) { + merged.set(listing.name, { paths: [listing.path], names: [...listing.names] }); + continue; + } + existing.paths.push(listing.path); + existing.names.push(...listing.names); + } + return [...merged].map(([name, entry]) => ({ + name, + path: sharedOutputPath(entry.paths), + names: [...new Set(entry.names)], + })); +} + +/** + * The identity several builds of ONE assembly share. + * + * Two target frameworks put the same assembly under `bin//net8.0/` and + * `bin//net9.0/`, so their paths agree everywhere except the segments + * that name a build. Keeping one of them as the merged group id keys the whole + * project's tree on a framework it merely happens to target: the id moves the + * moment the build announces its banners in another order, and a project + * targeting four frameworks gets a row identified by exactly one of them. + * + * Taking the common prefix back to its last separator and re-attaching the file + * name leaves what every build of the project agrees on. A project with one + * target framework has nothing to reconcile and keeps its real path. + */ +function sharedOutputPath(paths: readonly string[]): string { + const [first, ...rest] = paths; + if (first === undefined) return ''; + if (rest.length === 0) return first; + const shared = rest.reduce(commonPrefix, first); + return shared.slice(0, lastSeparator(shared) + 1) + first.slice(lastSeparator(first) + 1); +} + +/** The leading characters two paths agree on. */ +function commonPrefix(left: string, right: string): string { + let index = 0; + while (index < left.length && index < right.length && left[index] === right[index]) index += 1; + return left.slice(0, index); +} + +/** + * Index of the last `/` or `\`, or -1. Both are checked rather than `path.sep` + * because the separator comes from whichever host BUILT the listing, which is + * not necessarily the one reading it. + */ +function lastSeparator(value: string): number { + return Math.max(value.lastIndexOf('/'), value.lastIndexOf('\\')); +} + +/** + * One run plan for every target of a sweep. + * + * A workspace with several folders enumerates each one, and a run selects tests + * across all of them. Concatenating the modules keeps that one invocation set; + * an empty result means the sweep found no MTP module at all, and the VSTest + * path stays in charge. + */ +export function mergePlans(plans: readonly MtpRunPlan[]): MtpRunPlan | undefined { + const modules = plans.flatMap((plan) => [...plan.modules]); + return modules.length === 0 ? undefined : { modules }; +} diff --git a/src/editors/vscode/src/test-mtp-discovery.ts b/src/editors/vscode/src/test-mtp-discovery.ts new file mode 100644 index 00000000..953a273a --- /dev/null +++ b/src/editors/vscode/src/test-mtp-discovery.ts @@ -0,0 +1,154 @@ +/** + * Discovering the tests of Microsoft.Testing.Platform modules. + * + * The VSTest path builds with `dotnet test --list-tests` and then asks + * `dotnet vstest` for the names. This path does the same two things with the + * MTP commands: `dotnet build` first, then the built MODULE is asked directly. + * + * `dotnet test` cannot be used here. It does not forward the `json` argument of + * `--list-tests` (dotnet/sdk#49754), and the text listing it does forward is + * display names — which MSTest renders as the BARE method name, the same defect + * as issue #180. `dotnet exec` runs a module on every platform, with no apphost + * and no execute bit. + * + * Nothing here throws. A module that cannot be listed adds a warning and leaves + * every other module alone. + * + * Implements [TEST-MTP-DISCOVERY]. + */ + +import * as path from 'node:path'; +import { DOTNET_TIMEOUT_MS, runDotnet } from './dotnet-process.js'; +import { + mergeMultiTargeted, + type MtpModuleRun, + type TestAssemblyListing, + type TestListing, + type TestLocation, +} from './test-listing-model.js'; +import { + mtpIds, + mtpLocationsById, + mtpUidsById, + parseMtpTestList, + rejectedMtpOption, + type MtpTest, +} from './test-mtp.js'; +import { scanMtpProjects, type MtpProject } from './test-mtp-modules.js'; + +/** Ask a module for its tests as JSON, and nothing else. */ +export function listArgs(modulePath: string): string[] { + return ['exec', modulePath, '--list-tests', 'json', '--no-banner', '--no-ansi']; +} + +/** One module's tests, plus whatever went wrong asking for them. */ +interface ModuleListing { + readonly tests: readonly MtpTest[]; + readonly warnings: readonly string[]; +} + +/** Run `--list-tests json` against one module. */ +async function listModule( + modulePath: string, + cwd: string, + timeoutMs: number, +): Promise { + const run = await runDotnet(listArgs(modulePath), cwd, timeoutMs); + const output = `${run.stdout}\n${run.stderr}`; + const rejected = rejectedMtpOption(output); + if (rejected !== undefined) { + return { + tests: [], + warnings: [ + `${path.basename(modulePath)} rejected ${rejected}. ` + + 'Its Microsoft.Testing.Platform version is older than 2.3, which is the first ' + + 'to list tests as JSON. Update the test framework package.', + ], + }; + } + const listing = parseMtpTestList(run.stdout); + const failure = + listing.tests.length === 0 && run.failed + ? [`${path.basename(modulePath)} listed no test: ${run.errorMessage ?? 'no detail'}`] + : []; + return { tests: listing.tests, warnings: [...listing.warnings, ...failure] }; +} + +/** The tree root and the run plan one module contributes. */ +interface ModuleResult { + readonly assembly: TestAssemblyListing; + readonly run: MtpModuleRun; + readonly locations: ReadonlyMap; + readonly warnings: readonly string[]; +} + +/** List one module and shape what it reported for both the tree and the run. */ +async function scanModule( + modulePath: string, + cwd: string, + timeoutMs: number, +): Promise { + const listed = await listModule(modulePath, cwd, timeoutMs); + return { + assembly: { + name: path.basename(modulePath, path.extname(modulePath)), + path: modulePath, + names: mtpIds(listed.tests), + }, + run: { modulePath, uidsById: mtpUidsById(listed.tests) }, + locations: mtpLocationsById(listed.tests), + warnings: listed.warnings, + }; +} + +/** Every module of every MTP project the scan found. */ +function modulesOf(projects: readonly MtpProject[]): string[] { + return projects.flatMap((project) => [...project.modules]); +} + +/** Merge one module's locations into the sweep-wide map, first report winning. */ +function collectLocations( + into: Map, + from: ReadonlyMap, +): void { + for (const [id, location] of from) { + if (!into.has(id)) into.set(id, location); + } +} + +/** + * Enumerate every MTP test of `target`. + * + * `ok` says whether an EMPTY result can be trusted, because that is what + * decides whether the caller blanks the Testing view. A target with no MTP + * project at all is a truthful empty answer; a target whose modules all failed + * to list is not. + */ +export async function listMtpTests( + target: string, + cwd: string, + timeoutMs: number = DOTNET_TIMEOUT_MS, +): Promise { + const scan = await scanMtpProjects(target, cwd, timeoutMs); + const warnings = [...scan.warnings]; + const modules = modulesOf(scan.projects); + const byAssembly: TestAssemblyListing[] = []; + const runs: MtpModuleRun[] = []; + const locations = new Map(); + for (const modulePath of modules) { + const scanned = await scanModule(modulePath, cwd, timeoutMs); + warnings.push(...scanned.warnings); + byAssembly.push(scanned.assembly); + runs.push(scanned.run); + collectLocations(locations, scanned.locations); + } + const names = [...new Set(byAssembly.flatMap((assembly) => [...assembly.names]))]; + return { + names, + ok: names.length > 0 || modules.length === 0, + warnings, + byAssembly: mergeMultiTargeted(byAssembly.filter((assembly) => assembly.names.length > 0)), + mtp: { modules: runs }, + locations, + }; +} diff --git a/src/editors/vscode/src/test-mtp-modules.ts b/src/editors/vscode/src/test-mtp-modules.ts new file mode 100644 index 00000000..52cb9caf --- /dev/null +++ b/src/editors/vscode/src/test-mtp-modules.ts @@ -0,0 +1,175 @@ +/** + * Finding the Microsoft.Testing.Platform test modules of a target. + * + * MTP prints no `Test run for ` banner, so the assemblies cannot be + * scraped out of a listing the way the VSTest path scrapes them. They come from + * MSBuild instead, which is the only source that survives a custom + * `AssemblyName`, a custom `OutputPath`, an `ArtifactsPath` or a + * `RuntimeIdentifier`. + * + * Nothing here throws: a project that cannot be evaluated adds a warning and + * leaves the other projects alone. + * + * Implements [TEST-MTP-MODULES]. + */ + +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { DOTNET_TIMEOUT_MS, runDotnet } from './dotnet-process.js'; +import { evaluateProject } from './msbuild.js'; + +/** Project file extensions the Test Explorer knows. */ +const PROJECT_EXTENSIONS = ['.csproj', '.fsproj']; + +/** Solution file extensions `dotnet sln list` accepts. */ +const SOLUTION_EXTENSIONS = ['.sln', '.slnx', '.slnf']; + +/** One MTP test project and the modules its target frameworks produced. */ +export interface MtpProject { + /** Absolute path of the project file. */ + readonly projectFile: string; + /** Absolute paths of the built test modules, one per target framework. */ + readonly modules: readonly string[]; +} + +/** What a module sweep found. Never an exception. */ +export interface MtpProjectScan { + readonly projects: readonly MtpProject[]; + readonly warnings: readonly string[]; +} + +/** True when `target` is a solution file `dotnet sln list` understands. */ +export function isSolutionFile(target: string): boolean { + return SOLUTION_EXTENSIONS.includes(path.extname(target).toLowerCase()); +} + +/** + * The project paths in `dotnet sln list` output. + * + * The command prints a two-line header (`Project(s)` and a rule) before the + * paths, and the paths are RELATIVE to the solution. Lines are classified one + * by one rather than sliced past a fixed header: a localized or a future header + * would otherwise turn into a project path that does not exist. + */ +export function parseSolutionProjects(output: string, solutionDir: string): string[] { + const projects: string[] = []; + for (const raw of output.split('\n')) { + const line = raw.trim(); + if (!PROJECT_EXTENSIONS.includes(path.extname(line).toLowerCase())) continue; + projects.push(path.resolve(solutionDir, line)); + } + return projects; +} + +/** Every project file directly under `dir` or below it, without `bin`/`obj`. */ +function projectsUnder(dir: string, depth = 6): string[] { + const found: string[] = []; + let entries: fs.Dirent[]; + try { + entries = fs.readdirSync(dir, { withFileTypes: true }); + } catch { + return found; + } + for (const entry of entries) { + const full = path.join(dir, entry.name); + if (entry.isFile() && PROJECT_EXTENSIONS.includes(path.extname(entry.name).toLowerCase())) { + found.push(full); + } + if (entry.isDirectory() && depth > 0 && !isIgnoredDirectory(entry.name)) { + found.push(...projectsUnder(full, depth - 1)); + } + } + return found; +} + +/** Build output and package directories never hold a source project. */ +function isIgnoredDirectory(name: string): boolean { + const lower = name.toLowerCase(); + return lower === 'bin' || lower === 'obj' || lower === 'node_modules' || name.startsWith('.'); +} + +/** The project files of a solution, or of a directory that has none. */ +export async function projectsOf(target: string, timeoutMs: number): Promise { + if (!isSolutionFile(target)) { + return projectsUnder(fs.statSync(target).isDirectory() ? target : path.dirname(target)); + } + const dir = path.dirname(target); + const run = await runDotnet(['sln', target, 'list'], dir, timeoutMs); + return parseSolutionProjects(run.stdout, dir); +} + +/** + * Every built module of one MTP project. + * + * A multi-targeted project reports an EMPTY `TargetPath` from the outer build — + * it has no single target — and one module per declared framework instead. Both + * shapes are handled here so the caller sees a plain list of modules. + */ +async function modulesOf(projectFile: string): Promise { + const evaluated = await evaluateProject(projectFile); + if (!evaluated.ok) return []; + if (evaluated.value.targetPath.length > 0) return [evaluated.value.targetPath]; + const modules: string[] = []; + for (const framework of evaluated.value.targetFrameworks) { + const pinned = await evaluateProject(projectFile, framework); + if (pinned.ok && pinned.value.targetPath.length > 0) modules.push(pinned.value.targetPath); + } + return modules; +} + +/** One project's MTP verdict, plus any diagnostic worth logging. */ +async function scanProject( + projectFile: string, +): Promise<{ project: MtpProject | undefined; warnings: string[] }> { + const evaluated = await evaluateProject(projectFile); + if (!evaluated.ok) { + return { + project: undefined, + warnings: [`Could not evaluate ${projectFile}: ${evaluated.error}`], + }; + } + if (!evaluated.value.isTestingPlatformApplication) return { project: undefined, warnings: [] }; + const modules = (await modulesOf(projectFile)).filter((module) => fs.existsSync(module)); + if (modules.length === 0) { + return { + project: undefined, + warnings: [`MTP test project built no module that exists on disk: ${projectFile}`], + }; + } + return { project: { projectFile, modules }, warnings: [] }; +} + +/** Build `target` so every test module exists before anything is asked of it. */ +export async function buildTarget( + target: string, + cwd: string, + timeoutMs: number, +): Promise { + const positional = cwd === target ? [] : [target]; + const run = await runDotnet(['build', ...positional, '--nologo'], cwd, timeoutMs); + if (!run.failed) return []; + const detail = `${run.stdout}\n${run.stderr}`.trim().slice(-2_000); + return [`dotnet build reported a failure: ${run.errorMessage ?? 'unknown'}; output: ${detail}`]; +} + +/** + * Build `target`, then report its MTP test projects and their modules. + * + * A build FAILURE is a warning, not a stop: a solution whose sibling project + * does not compile still has modules from the projects that did, and dropping + * them would blank the Testing view over an unrelated error. + */ +export async function scanMtpProjects( + target: string, + cwd: string, + timeoutMs: number = DOTNET_TIMEOUT_MS, +): Promise { + const warnings = await buildTarget(target, cwd, timeoutMs); + const projects: MtpProject[] = []; + for (const projectFile of await projectsOf(target, timeoutMs)) { + const scanned = await scanProject(projectFile); + warnings.push(...scanned.warnings); + if (scanned.project !== undefined) projects.push(scanned.project); + } + return { projects, warnings }; +} diff --git a/src/editors/vscode/src/test-mtp-run.ts b/src/editors/vscode/src/test-mtp-run.ts new file mode 100644 index 00000000..7783b46b --- /dev/null +++ b/src/editors/vscode/src/test-mtp-run.ts @@ -0,0 +1,312 @@ +/** + * Running Microsoft.Testing.Platform tests. + * + * One invocation per MODULE for the whole selection, never one per test — the + * same rule [TEST-RUN-TRX] sets for VSTest, and for the same reason: a class of + * twenty tests must not pay twenty starts. + * + * Selection is by `--filter-uid`, which takes the module's own run keys. Those + * keys are LITERAL values, so the [TEST-FILTER-ESCAPE] grammar does not apply + * and must not be used: an NUnit uid is + * `Cs.Nunit.Mtp.CalculatorTests.Adds_Case(2,2,4)`, and escaping its parentheses + * would make it match nothing. + * + * Outcomes come from the TRX report the module writes, read by the same reader + * the VSTest path uses. + * + * Implements [TEST-MTP-RUN]. + */ + +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { batchByWidth, MAX_ARG_CHARS } from './test-batching.js'; +import { DOTNET_TIMEOUT_MS, runDotnet } from './dotnet-process.js'; +import type { MtpModuleRun, MtpRunPlan } from './test-listing-model.js'; +import { MTP_INVALID_COMMAND_LINE, rejectedMtpOption } from './test-mtp.js'; +import { parseMtpSummary, type TestOutcome, type TestRunSummary } from './test-run-output.js'; +import { collectReport, trxFiles, worse } from './test-trx-collect.js'; +import type { TrxRunInfo, TrxTestResult } from './test-trx.js'; +import type { TestRunOptions, TestRunOutcome } from './test-execution.js'; + +/** Ceiling on the uid arguments handed to ONE invocation. */ +export const MAX_UID_ARG_CHARS = MAX_ARG_CHARS; + +/** + * Split uids into batches whose joined argument text stays under the ceiling. + * A uid is passed as its own argv entry, so its cost is its length plus the + * separator and the quoting a shell would add. + */ +export function uidBatches(uids: readonly string[], maxChars = MAX_UID_ARG_CHARS): string[][] { + return batchByWidth(uids, (uid) => uid.length + 3, maxChars); +} + +/** The uids one module must run for `testIds`; empty ids mean "run everything". */ +export function uidsFor(module: MtpModuleRun, testIds: readonly string[]): string[] { + if (testIds.length === 0) return []; + return testIds.flatMap((id) => [...(module.uidsById.get(id) ?? [])]); +} + +/** True when the module owns none of the selected ids, so it must not start. */ +function untouched(module: MtpModuleRun, testIds: readonly string[]): boolean { + return testIds.length > 0 && uidsFor(module, testIds).length === 0; +} + +/** + * A TRX name unique to this module. + * + * Two modules writing one auto-named report into a shared results directory + * would overwrite each other, which is the defect [TEST-RUN-TRX] avoids under + * VSTest by never pinning `LogFileName`. + */ +export function trxNameFor(modulePath: string, batchIndex: number): string { + const stem = path.basename(modulePath, path.extname(modulePath)); + return `${stem}.${String(batchIndex)}.trx`; +} + +/** The argument vector for one module invocation. */ +export function runArgs( + modulePath: string, + uids: readonly string[], + resultsDirectory: string, + options: TestRunOptions, + trxName: string, +): string[] { + return [ + 'exec', + modulePath, + ...(uids.length === 0 ? [] : ['--filter-uid', ...uids]), + '--report-trx', + '--report-trx-filename', + trxName, + '--results-directory', + resultsDirectory, + '--no-banner', + '--no-ansi', + // `--no-progress` is deprecated since MTP 2.3 and warns on every run. + ...(options.coverage === true ? ['--coverage', '--coverage-output-format', 'cobertura'] : []), + ]; +} + +/** An empty outcome, so a run that started nothing still has a shape. */ +function emptyOutcome(failure: string | undefined): TestRunOutcome { + return { + results: new Map(), + summary: undefined, + failure, + runInfos: [], + retriedUnfiltered: false, + durationMs: 0, + output: '', + }; +} + +/** + * The message an invocation earns when it produced no result. + * + * Exit code 5 is MTP's "I did not understand the command line". `--report-trx` + * and `--coverage` are EXTENSIONS, so a module that does not register them + * exits that way. Naming the missing package is the whole point: a silent empty + * run reports every selected test as "No result reported" and hides the cause. + */ +function invocationFailure( + modulePath: string, + output: string, + errorMessage: string | undefined, + resultCount: number, +): string | undefined { + const rejected = rejectedMtpOption(output); + if (rejected === '--report-trx') { + return ( + `${path.basename(modulePath)} does not support ${rejected} (MTP exit code ` + + `${String(MTP_INVALID_COMMAND_LINE)}). Add a PackageReference to ` + + 'Microsoft.Testing.Extensions.TrxReport so the Test Explorer can read per-test results.' + ); + } + if (rejected !== undefined) { + return `${path.basename(modulePath)} does not support ${rejected}.`; + } + return resultCount > 0 ? undefined : (errorMessage ?? undefined); +} + +/** One `dotnet exec ` invocation into `resultsDirectory`. */ +async function invoke( + module: MtpModuleRun, + uids: readonly string[], + batchIndex: number, + context: { resultsDirectory: string; cwd: string; options: TestRunOptions }, +): Promise { + const { resultsDirectory, cwd, options } = context; + fs.mkdirSync(resultsDirectory, { recursive: true }); + const before = new Set(trxFiles(resultsDirectory)); + const args = runArgs( + module.modulePath, + uids, + resultsDirectory, + options, + trxNameFor(module.modulePath, batchIndex), + ); + const started = Date.now(); + const run = await runDotnet( + args, + cwd, + options.timeoutMs ?? DOTNET_TIMEOUT_MS, + options.signal, + options.hooks, + ); + const output = `${run.stdout}\n${run.stderr}`; + const report = collectReport(resultsDirectory, before); + return { + results: report.results, + summary: parseMtpSummary(output), + failure: run.killed + ? `${path.basename(module.modulePath)} was killed: ${run.errorMessage ?? 'no detail'}` + : invocationFailure(module.modulePath, output, run.errorMessage, report.results.size), + runInfos: report.runInfos, + retriedUnfiltered: false, + durationMs: Date.now() - started, + output, + }; +} + +/** Sum two summaries; one module's counts are never the whole run's. */ +function mergeSummaries( + left: TestRunSummary | undefined, + right: TestRunSummary | undefined, +): TestRunSummary | undefined { + if (left === undefined) return right; + if (right === undefined) return left; + const totals = { + passed: left.passed + right.passed, + failed: left.failed + right.failed, + skipped: left.skipped + right.skipped, + total: left.total + right.total, + }; + return { ...totals, outcome: worstSummaryOutcome(left, right) }; +} + +/** A run of several modules is as bad as its worst module. */ +function worstSummaryOutcome(left: TestRunSummary, right: TestRunSummary): TestOutcome { + const ranked: readonly TestOutcome[] = ['passed', 'skipped', 'notRun', 'failed']; + return ranked.indexOf(right.outcome) > ranked.indexOf(left.outcome) + ? right.outcome + : left.outcome; +} + +/** Merge two invocations, keeping the WORST outcome reported for each id. */ +export function mergeOutcomes(left: TestRunOutcome, right: TestRunOutcome): TestRunOutcome { + const results = new Map(left.results); + for (const [name, result] of right.results) { + const existing = results.get(name); + results.set(name, existing === undefined ? result : worse(existing, result)); + } + const runInfos: TrxRunInfo[] = [...left.runInfos, ...right.runInfos]; + return { + results, + summary: mergeSummaries(left.summary, right.summary), + failure: results.size > 0 ? undefined : (left.failure ?? right.failure), + runInfos, + retriedUnfiltered: false, + durationMs: left.durationMs + right.durationMs, + output: `${left.output}\n${right.output}`, + }; +} + +/** Every batch of one module, merged, plus its one unfiltered recovery. */ +async function runModule( + module: MtpModuleRun, + testIds: readonly string[], + context: { resultsDirectory: string; cwd: string; options: TestRunOptions }, +): Promise { + const uids = uidsFor(module, testIds); + const batches = testIds.length === 0 ? [[]] : uidBatches(uids); + let merged: TestRunOutcome | undefined; + let index = 0; + for (const batch of batches) { + if (context.options.signal?.aborted === true) break; + const one = await invoke(module, batch, index, context); + merged = merged === undefined ? one : mergeOutcomes(merged, one); + index += 1; + } + if (merged === undefined || !needsUnfilteredRetry(module, merged, testIds, context)) { + return merged; + } + const unfiltered = await invoke(module, [], index, context); + return { + ...mergeOutcomes(merged, unfiltered), + // The retry re-ran the SAME module, so its counts REPLACE the refused + // attempt's rather than adding to them. Summing is right only ACROSS + // modules, which is what `mergeOutcomes` does everywhere else. + summary: unfiltered.summary ?? merged.summary, + retriedUnfiltered: true, + }; +} + +/** + * True when the module REFUSED the selection rather than merely matching none. + * + * A framework bridged onto MTP can translate `--filter-uid` back into a VSTest + * filter EXPRESSION and then reject its own translation. NUnit does exactly + * that for a uid carrying both a SPACE and PARENTHESES — which is every + * idiomatic F# `[]` binding + * (`Fs.Nunit.Fixtures.adds case(2,2,4)` → "Unexpected FQN 'case\(2,2,4\)' at + * position 46 in selection expression") — and the whole module then reports + * nothing, so perfectly runnable tests show as phantom failures. + * + * The remedy is the one [TEST-FILTER-ESCAPE] already sets for the VSTest path: + * re-run the module ONCE without a filter and pick the outcomes out of the + * report by name. Slower, but correct, and only ever when the module both + * failed AND left a selected test unreported — never on a selection that + * legitimately matched nothing. + */ +function needsUnfilteredRetry( + module: MtpModuleRun, + outcome: TestRunOutcome, + testIds: readonly string[], + context: { options: TestRunOptions }, +): boolean { + if (testIds.length === 0) return false; + if (context.options.signal?.aborted === true) return false; + if (outcome.failure === undefined) return false; + // A REJECTED option is rejected again without a filter, so retrying only + // doubles the wait before the user reads the same message. + if (rejectedMtpOption(outcome.output) !== undefined) return false; + const mine = testIds.filter((id) => module.uidsById.has(id)); + return mine.some((id) => !outcome.results.has(id)); +} + +/** Run `testIds` (all tests when empty) across the plan's modules. */ +export async function runMtpTests( + plan: MtpRunPlan, + testIds: readonly string[], + cwd: string, + options: TestRunOptions = {}, +): Promise { + const owned = options.resultsDirectory === undefined; + const resultsDirectory = options.resultsDirectory ?? freshTempDir(); + const context = { resultsDirectory, cwd, options }; + try { + let merged: TestRunOutcome | undefined; + for (const module of plan.modules) { + if (options.signal?.aborted === true) break; + if (untouched(module, testIds)) continue; + const one = await runModule(module, testIds, context); + if (one === undefined) continue; + merged = merged === undefined ? one : mergeOutcomes(merged, one); + } + return merged ?? emptyOutcome(noModuleRan(plan, options)); + } finally { + if (owned) fs.rmSync(resultsDirectory, { recursive: true, force: true }); + } +} + +/** Why a plan started nothing: cancelled first, or simply empty. */ +function noModuleRan(plan: MtpRunPlan, options: TestRunOptions): string | undefined { + if (options.signal?.aborted === true) return 'Run cancelled before any module started'; + return plan.modules.length === 0 ? 'No Microsoft.Testing.Platform module to run' : undefined; +} + +/** A private, empty directory for one run's TRX output. */ +function freshTempDir(): string { + return fs.mkdtempSync(path.join(os.tmpdir(), 'sharplsp-mtp-')); +} diff --git a/src/editors/vscode/src/test-mtp.ts b/src/editors/vscode/src/test-mtp.ts new file mode 100644 index 00000000..5ea0850d --- /dev/null +++ b/src/editors/vscode/src/test-mtp.ts @@ -0,0 +1,240 @@ +/** + * Microsoft.Testing.Platform: choosing the runner, and reading what a test + * module reports. + * + * An MTP test project builds to an EXECUTABLE test module, and that module — + * not `vstest.console` — discovers and runs its own tests. Every VSTest command + * fails against it: `--nologo` is not a valid MTP option, `dotnet vstest` + * cannot load the assembly, and neither `--filter` nor `--logger trx` exists. + * + * This module is PURE: no process, no VS Code. It decides which runner a target + * uses, and it turns the module's `--list-tests json` answer into test ids. + * + * Implements [TEST-MTP-DETECT] and [TEST-MTP-DISCOVERY]. + */ + +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { isRecord } from './utils.js'; + +/** The `global.json` value that selects MTP, lower-cased for comparison. */ +const MTP_RUNNER = 'microsoft.testing.platform'; + +/** The JSON listing schema this reader was written against. */ +const KNOWN_SCHEMA_VERSION = 1; + +/** One test node a module reported. */ +export interface MtpTest { + /** + * `namespace.Type.Method`, the Test Explorer's id. Built from the listing's + * `type` block and NEVER from its display name: MSTest reports the BARE + * method name as the display name, which is the issue-#180 defect again. + */ + readonly id: string; + /** The module's own run key, passed back verbatim to `--filter-uid`. */ + readonly uid: string; + /** The name the module shows a human. Carries row data; a label, not a key. */ + readonly label: string; + /** Source file, when the framework reports one. NUnit does not. */ + readonly file: string | undefined; + /** 1-based first line of the test, when the framework reports one. */ + readonly line: number | undefined; +} + +/** What one module's listing produced. Never an exception. */ +export interface MtpListing { + readonly tests: readonly MtpTest[]; + readonly warnings: readonly string[]; +} + +/** A non-empty string at `key`, or `undefined`. */ +function text(bag: Record, key: string): string | undefined { + const value = bag[key]; + return typeof value === 'string' && value !== '' ? value : undefined; +} + +/** + * True when `globalJson` opts the whole target into MTP. + * + * Parsed, never searched: `"Microsoft.Testing.Platform"` also appears in a + * comment, in a package name and in an unrelated property, and a string search + * would switch a VSTest solution onto a path that cannot run it. + */ +export function mtpRunnerSelected(globalJson: string): boolean { + try { + const parsed: unknown = JSON.parse(globalJson); + if (!isRecord(parsed)) return false; + const test: unknown = parsed.test; + if (!isRecord(test)) return false; + return (text(test, 'runner') ?? '').toLowerCase() === MTP_RUNNER; + } catch { + return false; + } +} + +/** + * The nearest `global.json` at or above `startDir`, or `undefined`. + * + * The SDK resolves the runner the same way, so a solution in a sub-directory of + * the repository that holds the opt-in must find it too. + */ +export function findGlobalJson(startDir: string): string | undefined { + let current = path.resolve(startDir); + for (;;) { + const candidate = path.join(current, 'global.json'); + if (fs.existsSync(candidate)) return candidate; + const parent = path.dirname(current); + if (parent === current) return undefined; + current = parent; + } +} + +/** True when the `global.json` above `startDir` selects the MTP runner. */ +export function usesMtpRunner(startDir: string): boolean { + const file = findGlobalJson(startDir); + if (file === undefined) return false; + try { + return mtpRunnerSelected(fs.readFileSync(file, 'utf8')); + } catch { + return false; + } +} + +/** + * The id of one listed test. + * + * The `type` block holds the namespace, the type and the method separately, and + * it carries NO row data — so the rows of a data-driven test collapse onto the + * one id they share, as [TEST-DISCOVERY-FQN] requires. The joined value is also + * exactly the `className` + `.` + `name` pair the TRX report holds, which is + * what lets [TEST-RUN-TRX] attribute an MTP outcome with no change. + * + * A module that sends no `type` block falls back to the display name. That is + * weaker — MSTest would give a bare method name — but it is never worse than + * dropping the test. + */ +function idOf(node: Record, label: string): string { + const type = node.type; + if (!isRecord(type)) return label; + const parts = [text(type, 'namespace'), text(type, 'typeName'), text(type, 'methodName')]; + const named = parts.filter((part): part is string => part !== undefined); + return named.length === 0 ? label : named.join('.'); +} + +/** The 1-based start line of a node's `location`, when it has one. */ +function lineOf(node: Record): number | undefined { + const location = node.location; + if (!isRecord(location)) return undefined; + const start: unknown = location.lineStart; + return typeof start === 'number' && Number.isInteger(start) && start > 0 ? start : undefined; +} + +/** The source file of a node's `location`, when it has one. */ +function fileOf(node: Record): string | undefined { + const location = node.location; + return isRecord(location) ? text(location, 'file') : undefined; +} + +/** One `tests[]` entry, or `undefined` when it carries no usable uid. */ +function toTest(entry: unknown): MtpTest | undefined { + if (!isRecord(entry)) return undefined; + const uid = text(entry, 'uid'); + if (uid === undefined) return undefined; + const label = text(entry, 'displayName') ?? uid; + return { id: idOf(entry, label), uid, label, file: fileOf(entry), line: lineOf(entry) }; +} + +/** The JSON document inside a module's stdout, or `undefined`. */ +function documentIn(stdout: string): Record | undefined { + // The listing is preceded by a blank line, and on Windows by a byte-order + // mark, so the scan starts at the first brace rather than at character zero. + const start = stdout.indexOf('{'); + if (start < 0) return undefined; + try { + const parsed: unknown = JSON.parse(stdout.slice(start)); + return isRecord(parsed) ? parsed : undefined; + } catch { + return undefined; + } +} + +/** A warning when the module answered in a schema this reader does not know. */ +function schemaWarning(document: Record): string[] { + const version: unknown = document.schemaVersion; + if (version === KNOWN_SCHEMA_VERSION) return []; + return [ + `Test listing reported schemaVersion ${String(version)}; ` + + `this reader was written for ${String(KNOWN_SCHEMA_VERSION)}`, + ]; +} + +/** Read one module's `--list-tests json` answer. Never throws. */ +export function parseMtpTestList(stdout: string): MtpListing { + const document = documentIn(stdout); + if (document === undefined) { + return { tests: [], warnings: ['Test listing was not a JSON document'] }; + } + const entries: unknown = document.tests; + if (!Array.isArray(entries)) { + return { tests: [], warnings: ['Test listing carried no "tests" array'] }; + } + const tests = entries + .map((entry) => toTest(entry)) + .filter((test): test is MtpTest => test !== undefined); + return { tests, warnings: schemaWarning(document) }; +} + +/** The ids one module reported, in listing order, without repeats. */ +export function mtpIds(tests: readonly MtpTest[]): string[] { + return [...new Set(tests.map((test) => test.id))]; +} + +/** + * Every uid each id owns, in listing order. + * + * A data-driven test lists one node PER ROW, each with its own uid and all + * sharing one id. Running that id must run every row, so the map holds a list. + */ +export function mtpUidsById(tests: readonly MtpTest[]): Map { + const uids = new Map(); + for (const test of tests) { + const existing = uids.get(test.id); + if (existing === undefined) uids.set(test.id, [test.uid]); + else existing.push(test.uid); + } + return uids; +} + +/** The first source location reported for each id, when there is one. */ +export function mtpLocationsById( + tests: readonly MtpTest[], +): Map { + const locations = new Map(); + for (const test of tests) { + if (test.file === undefined || locations.has(test.id)) continue; + locations.set(test.id, { file: test.file, line: test.line }); + } + return locations; +} + +/** The MTP exit code for a command line the module could not understand. */ +export const MTP_INVALID_COMMAND_LINE = 5; + +/** Prefix of the message a module prints when it rejects an option. */ +const UNKNOWN_OPTION = "Unknown option '"; + +/** + * The option a module rejected, or `undefined`. + * + * `--report-trx` and `--coverage` are EXTENSIONS, not part of MTP. A module + * that does not register the extension exits with code 5 and prints this line. + * Reporting it as itself is what tells the user to reference the package; + * swallowing it would report every selected test as "No result reported". + */ +export function rejectedMtpOption(output: string): string | undefined { + const start = output.indexOf(UNKNOWN_OPTION); + if (start < 0) return undefined; + const rest = output.slice(start + UNKNOWN_OPTION.length); + const end = rest.indexOf("'"); + return end <= 0 ? undefined : rest.slice(0, end); +} diff --git a/src/editors/vscode/src/test-profiles.ts b/src/editors/vscode/src/test-profiles.ts new file mode 100644 index 00000000..1ea6ce40 --- /dev/null +++ b/src/editors/vscode/src/test-profiles.ts @@ -0,0 +1,48 @@ +/** + * The three profiles the Testing view offers: Run, Debug, and Run with + * Coverage, in the order it shows them. + * + * Kept apart from `testing.ts` so the controller file stays about state and + * dispatch. The handlers themselves belong to the controller; this module only + * declares the profiles and wires the lazy coverage lookup. + * + * Implements [TEST-EXPLORER] and [TEST-COVERAGE]. + */ + +import * as vscode from 'vscode'; +import { loadDetailedCoverage } from './test-coverage.js'; + +/** One profile's handler, as VS Code invokes it. */ +export type RunProfileHandler = ( + request: vscode.TestRunRequest, + token: vscode.CancellationToken, +) => Promise; + +/** What the controller supplies for each profile. */ +export interface RunProfileHandlers { + readonly run: RunProfileHandler; + readonly debug: RunProfileHandler; + readonly coverage: RunProfileHandler; +} + +/** Register the three profiles and return them in display order. */ +export function registerRunProfiles( + controller: vscode.TestController, + handlers: RunProfileHandlers, +): vscode.TestRunProfile[] { + const run = controller.createRunProfile('Run', vscode.TestRunProfileKind.Run, handlers.run, true); + const debug = controller.createRunProfile( + 'Debug', + vscode.TestRunProfileKind.Debug, + handlers.debug, + ); + const coverage = controller.createRunProfile( + 'Run with Coverage', + vscode.TestRunProfileKind.Coverage, + handlers.coverage, + ); + coverage.loadDetailedCoverage = + // eslint-disable-next-line @typescript-eslint/require-await -- VS Code API requires Thenable return but lookup is synchronous + async (_run, fileCoverage, _token) => loadDetailedCoverage(fileCoverage); + return [run, debug, coverage]; +} diff --git a/src/editors/vscode/src/test-queue.ts b/src/editors/vscode/src/test-queue.ts new file mode 100644 index 00000000..70513ed8 --- /dev/null +++ b/src/editors/vscode/src/test-queue.ts @@ -0,0 +1,37 @@ +/** + * One `dotnet` invocation at a time, for the whole Test Explorer. + * + * Discovery BUILDS the solution and a run rebuilds the same projects, so two + * overlapping invocations race on the shared `bin/`/`obj/` output — VSTest then + * dies with "The application to execute does not exist: …testhost.dll", and a + * Microsoft.Testing.Platform module cannot even be started while it is being + * written. Reactive re-discovery is debounced, not cancelled, so that overlap + * is reachable whenever a user runs a test while a sweep is still building. + * + * Implements [TEST-REACTIVITY]. + */ + +/** A serial queue of `dotnet` invocations. */ +export class DotnetQueue { + private tail: Promise = Promise.resolve(); + + /** Queue `work` behind any invocation already in flight. */ + public async enqueue(work: () => Promise): Promise { + const next = this.tail.then(work, work); + this.tail = next.catch(() => undefined); + return await next; + } + + /** + * Resolve once no invocation is outstanding. Tests use this to settle + * reactive re-discovery before touching the fixture on disk; a `dotnet test` + * left pointing at a deleted directory hangs and poisons the next run. + */ + public async whenIdle(): Promise { + let seen: Promise | undefined; + while (seen !== this.tail) { + seen = this.tail; + await seen; + } + } +} diff --git a/src/editors/vscode/src/test-result-cache.ts b/src/editors/vscode/src/test-result-cache.ts new file mode 100644 index 00000000..cafc870b --- /dev/null +++ b/src/editors/vscode/src/test-result-cache.ts @@ -0,0 +1,71 @@ +/** + * The last known outcome of every discovered test. + * + * The status CodeLens ([TEST-STATUS-LENS]) paints from this cache, so it + * outlives the run that filled it — and therefore has to be pruned when the + * tree changes. A result must not outlive the test it belongs to: after the + * loaded solution changes, a stale entry would paint an outcome for a test that + * was never run here. + * + * Implements [TEST-REACTIVITY]. + */ + +import * as vscode from 'vscode'; +import type { CacheWriter, CachedTestResult } from './test-reporting.js'; +import { forEachLeafIn } from './test-tree.js'; + +/** Cached outcomes keyed by test id, with a change signal for the lens. */ +export class TestResultCache { + private readonly results = new Map(); + private readonly changed = new vscode.EventEmitter(); + + /** Fires after any test run completes and results are cached. */ + public readonly onChanged = this.changed.event; + + /** Look up the last known result for a test id. */ + public get(testId: string): CachedTestResult | undefined { + return this.results.get(testId); + } + + /** All cached results keyed by test id. */ + public get all(): ReadonlyMap { + return this.results; + } + + /** Announce that results changed, so the lens repaints. */ + public fire(): void { + this.changed.fire(); + } + + /** The writer a real RUN reports through; a debug run passes none. */ + public writer(): CacheWriter { + return (id, result) => { + this.results.set(id, result); + }; + } + + /** Record one result without announcing it. */ + public set(testId: string, result: CachedTestResult): void { + this.results.set(testId, result); + } + + /** + * Drop cached outcomes for tests no longer in the tree. Listeners hear about + * it only when something was actually dropped. + */ + public pruneTo(items: readonly vscode.TestItem[]): void { + const alive = new Set(); + forEachLeafIn(items, (item) => alive.add(item.id)); + let dropped = 0; + for (const id of [...this.results.keys()]) { + if (alive.has(id)) continue; + this.results.delete(id); + dropped += 1; + } + if (dropped > 0) this.changed.fire(); + } + + public dispose(): void { + this.changed.dispose(); + } +} diff --git a/src/editors/vscode/src/test-run-output.ts b/src/editors/vscode/src/test-run-output.ts index 873ed8c3..56c725f7 100644 --- a/src/editors/vscode/src/test-run-output.ts +++ b/src/editors/vscode/src/test-run-output.ts @@ -100,3 +100,48 @@ function isErrorMessageTerminator(line: string): boolean { const trimmed = line.trim(); return ERROR_MESSAGE_TERMINATORS.some((terminator) => trimmed.startsWith(terminator)); } + +/** + * The per-module summary a Microsoft.Testing.Platform run prints. + * + * MTP does not print VSTest's `Passed! - Failed: 0, …` line. It prints a block + * instead, one per module, and the counts are summed the same way: + * + * ``` + * Test run summary: Failed! - …/CsXunitMtp.dll (net10.0|x64) + * total: 5 + * failed: 2 + * succeeded: 2 + * skipped: 1 + * ``` + * + * Implements [TEST-MTP-RUN]. The text is English because [TEST-ENV-LOCALE] + * pins `DOTNET_CLI_UI_LANGUAGE`. + */ +export function parseMtpSummary(output: string): TestRunSummary | undefined { + const totals = { passed: 0, failed: 0, skipped: 0, total: 0 }; + let seen = false; + for (const raw of output.split('\n')) { + const match = MTP_COUNT_PATTERN.exec(raw.trim()); + if (match === null) continue; + seen = true; + addMtpCount(totals, match[1] ?? '', Number(match[2] ?? '0')); + } + return seen ? { ...totals, outcome: outcomeOf(totals) } : undefined; +} + +/** One `