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 `