From ea28dc30fbd9ab056c5f43b0962ae76e44601022 Mon Sep 17 00:00:00 2001 From: Christian Findlay <16697547+MelbourneDeveloper@users.noreply.github.com> Date: Mon, 28 Sep 2026 22:25:30 +1000 Subject: [PATCH] Share the hierarchy handlers, sidecar request plumbing and VS Code test kits Deslop pass over the host and the extension. Nothing changes behaviour; every clone below collapses onto the one occurrence that already existed. Host: `hierarchy::handle_prepare` and `hierarchy::handle_related` answer all six hierarchy requests, so `call_hierarchy` and `type_hierarchy` keep only their item mappers. Code actions, inlay hints, hover and completion resolve go through `request_sidecar` instead of hand-rolled MessagePack round-trips. Extension: the profiler's dump picker, address prompt and plain-text report live once in `profiler-prompts.ts`; the profiler tree builds its headed groups through one helper. The debug suites share `debug-inspect-kit.ts` (variables, watch, console, hover and run-to-end checks), the Run/Debug suites share `run-debug-case.ts` (scratch dir and observers per test), and the Test Explorer suites share the xUnit C#/F# pair in `test-explorer-xunit-pair.ts`. Duplication measured at 9.7% with Deslop 0.34.0; the gate ratchets from 11% to 10%. --- .deslop.toml | 2 +- src/editors/vscode/src/profiler-diff.ts | 14 +- src/editors/vscode/src/profiler-graph.ts | 22 +- src/editors/vscode/src/profiler-prompts.ts | 36 ++ src/editors/vscode/src/profiler.ts | 434 +++++++----------- .../test/suite/completions-visible.test.ts | 34 +- .../test/suite/csharp-refactor-test-kit.ts | 12 +- .../test/suite/debug-adapter-startup.test.ts | 83 ++-- .../debug-breakpoint-conditions-e2e.test.ts | 108 ++--- .../test/suite/debug-breakpoints-e2e.test.ts | 21 +- .../test/suite/debug-callstack-e2e.test.ts | 17 +- .../src/test/suite/debug-evaluate-e2e.test.ts | 40 +- .../suite/debug-exception-filters-e2e.test.ts | 31 +- .../test/suite/debug-exceptions-e2e.test.ts | 33 +- .../suite/debug-fsharp-inspection-e2e.test.ts | 58 +-- .../suite/debug-fsharp-stepping-e2e.test.ts | 47 +- .../src/test/suite/debug-inspect-kit.ts | 205 +++++++++ .../debug-protocol-capabilities-e2e.test.ts | 50 +- .../debug-stepping-boundaries-e2e.test.ts | 8 +- .../src/test/suite/debug-stepping-e2e.test.ts | 35 +- .../vscode/src/test/suite/debug-suite-kit.ts | 79 ++-- .../suite/debug-test-debugging-e2e.test.ts | 137 +++--- .../suite/debug-test-decorated-e2e.test.ts | 9 +- .../test/suite/debug-test-fsharp-e2e.test.ts | 41 +- .../test/suite/debug-test-groups-e2e.test.ts | 17 +- .../src/test/suite/debug-test-harness.ts | 60 ++- .../src/test/suite/debug-test-mtp-e2e.test.ts | 55 +-- .../suite/debug-test-mtp-fsharp-e2e.test.ts | 36 +- .../test/suite/debug-variables-e2e.test.ts | 37 +- .../vscode/src/test/suite/explorer-kit.ts | 11 +- .../suite/fsharp-lsp-codefix-basics.test.ts | 74 +-- .../fsharp-lsp-codefix-conversions.test.ts | 64 +-- .../fsharp-lsp-codefix-generation.test.ts | 59 +-- .../test/suite/fsharp-refactor-test-kit.ts | 75 +++ .../src/test/suite/lsp-refactor-core.test.ts | 147 +----- .../suite/lsp-refactor-quickfixes.test.ts | 98 +--- .../vscode/src/test/suite/run-debug-case.ts | 41 ++ .../src/test/suite/run-debug-profiles.test.ts | 22 +- .../src/test/suite/run-debug-target.test.ts | 31 +- .../src/test/suite/solution-explorer.test.ts | 125 ++--- .../suite/test-explorer-cancellation.test.ts | 17 +- .../test/suite/test-explorer-coverage.test.ts | 33 +- .../src/test/suite/test-explorer-e2e.test.ts | 172 ++----- .../src/test/suite/test-explorer-fixtures.ts | 239 ++++++---- .../suite/test-explorer-frameworks.test.ts | 28 +- .../suite/test-explorer-goto-test.test.ts | 6 +- .../src/test/suite/test-explorer-harness.ts | 56 +++ .../src/test/suite/test-explorer-kit.ts | 16 + .../suite/test-explorer-mtp-batches.test.ts | 27 +- .../test/suite/test-explorer-mtp-fixtures.ts | 98 +--- .../suite/test-explorer-mtp-modules.test.ts | 25 +- .../suite/test-explorer-mtp-outcomes.test.ts | 15 +- .../suite/test-explorer-mtp-sweeps.test.ts | 17 +- .../src/test/suite/test-explorer-mtp.test.ts | 30 +- .../suite/test-explorer-multitarget.test.ts | 5 +- .../test/suite/test-explorer-names.test.ts | 57 +-- .../suite/test-explorer-outcome-assertions.ts | 11 + .../test/suite/test-explorer-outcomes.test.ts | 3 +- .../test/suite/test-explorer-parsers.test.ts | 212 +++------ .../test/suite/test-explorer-reactive.test.ts | 11 +- .../test/suite/test-explorer-refusals.test.ts | 28 +- .../test/suite/test-explorer-windows.test.ts | 158 +++---- .../test/suite/test-explorer-xunit-pair.ts | 27 ++ .../test/suite/testing-lens-status.test.ts | 3 +- src/sharplsp/src/call_hierarchy.rs | 123 +---- src/sharplsp/src/code_actions.rs | 110 ++--- src/sharplsp/src/hierarchy.rs | 96 +++- src/sharplsp/src/inlay_hints.rs | 25 +- src/sharplsp/src/main.rs | 26 +- src/sharplsp/src/semantic.rs | 31 +- src/sharplsp/src/type_hierarchy.rs | 90 +--- 71 files changed, 1764 insertions(+), 2539 deletions(-) create mode 100644 src/editors/vscode/src/profiler-prompts.ts create mode 100644 src/editors/vscode/src/test/suite/debug-inspect-kit.ts create mode 100644 src/editors/vscode/src/test/suite/run-debug-case.ts create mode 100644 src/editors/vscode/src/test/suite/test-explorer-xunit-pair.ts diff --git a/.deslop.toml b/.deslop.toml index dc04e04e..883b42b1 100644 --- a/.deslop.toml +++ b/.deslop.toml @@ -1,6 +1,6 @@ # agent-pmo:a72c926 [threshold] -max_duplication_percent = 11 +max_duplication_percent = 10 [defaults] exclude = [ diff --git a/src/editors/vscode/src/profiler-diff.ts b/src/editors/vscode/src/profiler-diff.ts index a71814e0..cd4adfc2 100644 --- a/src/editors/vscode/src/profiler-diff.ts +++ b/src/editors/vscode/src/profiler-diff.ts @@ -9,6 +9,7 @@ import * as vscode from 'vscode'; import { type LanguageClient } from 'vscode-languageclient/node'; import { escapeHtml, getErrorMessage } from './utils.js'; import { ObjectGraphPanel } from './profiler-graph.js'; +import { pickDumpFile } from './profiler-prompts.js'; // ── LSP types ───────────────────────────────────────────────────── @@ -449,23 +450,18 @@ export async function detectLeaksWorkflow( } /** The one `.dmp` file the user picks for `which` side, or undefined on cancel. */ -async function pickDump(which: 'BASELINE' | 'COMPARISON'): Promise { - const picked = await vscode.window.showOpenDialog({ - canSelectMany: false, - filters: { 'Dump files': ['dmp'] }, - title: `Select ${which} dump file`, - }); - return picked?.[0]; +async function pickDump(which: 'BASELINE' | 'COMPARISON'): Promise { + return await pickDumpFile(`Select ${which} dump file`); } /** Pick the comparison dump, then open the diff of `baseline` against it. */ async function diffAgainst( - baseline: vscode.Uri, + baseline: string, context: vscode.ExtensionContext, client: LanguageClient, ): Promise { const comparison = await pickDump('COMPARISON'); if (comparison !== undefined) { - await HeapDiffPanel.open(baseline.fsPath, comparison.fsPath, context, client); + await HeapDiffPanel.open(baseline, comparison, context, client); } } diff --git a/src/editors/vscode/src/profiler-graph.ts b/src/editors/vscode/src/profiler-graph.ts index b06b486d..41539eba 100644 --- a/src/editors/vscode/src/profiler-graph.ts +++ b/src/editors/vscode/src/profiler-graph.ts @@ -5,6 +5,7 @@ import * as vscode from 'vscode'; import { type LanguageClient } from 'vscode-languageclient/node'; import { getErrorMessage } from './utils.js'; +import { askObjectAddress, pickDumpFile } from './profiler-prompts.js'; interface ObjectGraphNode { readonly id: string; @@ -112,20 +113,11 @@ export async function promptAndOpenGraph( context: vscode.ExtensionContext, client: LanguageClient, ): Promise { - const dumpFiles = await vscode.window.showOpenDialog({ - canSelectMany: false, - filters: { 'Dump files': ['dmp'] }, - title: 'Select memory dump file for object graph', - }); - const selectedFile = dumpFiles?.[0]; - if (selectedFile === undefined) return; - - const rootAddress = await vscode.window.showInputBox({ - prompt: 'Enter the root object address (hex, e.g. 00007ff812345678)', - placeHolder: '00007ff812345678', - validateInput: (v) => (v.trim().length > 0 ? undefined : 'Address is required'), - }); + const dumpPath = await pickDumpFile('Select memory dump file for object graph'); + if (dumpPath === undefined) return; + const rootAddress = await askObjectAddress( + 'Enter the root object address (hex, e.g. 00007ff812345678)', + ); if (rootAddress === undefined) return; - - await ObjectGraphPanel.open(selectedFile.fsPath, rootAddress.trim(), context, client); + await ObjectGraphPanel.open(dumpPath, rootAddress.trim(), context, client); } diff --git a/src/editors/vscode/src/profiler-prompts.ts b/src/editors/vscode/src/profiler-prompts.ts new file mode 100644 index 00000000..d59c1693 --- /dev/null +++ b/src/editors/vscode/src/profiler-prompts.ts @@ -0,0 +1,36 @@ +// The dialogs the profiler commands share: pick a file, ask for an object +// address, and show a plain-text report. Each resolves `undefined` when the +// user cancels, so a command can simply stop. +import * as vscode from 'vscode'; + +/** The one file the user picks in an Open dialog, or `undefined` on cancel. */ +export async function pickOneFile( + filters: Record, + title: string, +): Promise { + const picked = await vscode.window.showOpenDialog({ canSelectMany: false, filters, title }); + return picked?.[0]?.fsPath; +} + +/** A memory dump (`.dmp`) the user picks, or `undefined` on cancel. */ +export async function pickDumpFile(title = 'Select memory dump file'): Promise { + return await pickOneFile({ 'Dump files': ['dmp'] }, title); +} + +/** A non-empty object address (hex) the user types, or `undefined` on cancel. */ +export async function askObjectAddress(prompt: string): Promise { + return await vscode.window.showInputBox({ + prompt, + placeHolder: '00007ff812345678', + validateInput: (v) => (v.trim().length > 0 ? undefined : 'Address is required'), + }); +} + +/** Show `lines` as a plain-text preview document. */ +export async function showPlainText(lines: readonly string[]): Promise { + const doc = await vscode.workspace.openTextDocument({ + content: lines.join('\n'), + language: 'plaintext', + }); + await vscode.window.showTextDocument(doc, { preview: true }); +} diff --git a/src/editors/vscode/src/profiler.ts b/src/editors/vscode/src/profiler.ts index f8c7a3ee..dc29bf2f 100644 --- a/src/editors/vscode/src/profiler.ts +++ b/src/editors/vscode/src/profiler.ts @@ -28,7 +28,10 @@ import { CMD_PROFILER_KILL_PROCESS, } from './constants.js'; import { promptAndOpenGraph } from './profiler-graph.js'; -import { promptAndOpenDiff, detectLeaksWorkflow } from './profiler-diff.js'; +import { promptAndOpenDiff, detectLeaksWorkflow, formatBytes } from './profiler-diff.js'; +import { askObjectAddress, pickDumpFile, pickOneFile, showPlainText } from './profiler-prompts.js'; + +export { formatBytes } from './profiler-diff.js'; // ── LSP Types ───────────────────────────────────────────────────── @@ -203,37 +206,20 @@ export class ProfilerTreeProvider implements vscode.TreeDataProvider 0) { - const header = new ProfilerTreeItem( - `Active Sessions (${String(this.activeSessions.length)})`, - 'header', - vscode.TreeItemCollapsibleState.None, - { contextValue: 'profiler-header-sessions' }, - ); - header.iconPath = new vscode.ThemeIcon('pulse'); - nodes.push(header); - - for (const session of this.activeSessions) { - nodes.push(buildSessionNode(session)); - } - } - - if (this.processes.length > 0) { - const header = new ProfilerTreeItem( - `.NET Processes (${String(this.processes.length)})`, - 'header', - vscode.TreeItemCollapsibleState.None, - { contextValue: 'profiler-header-processes' }, - ); - header.iconPath = new vscode.ThemeIcon('server-process'); - nodes.push(header); - - for (const proc of this.processes) { - nodes.push(buildProcessNode(proc)); - } - } + const nodes: ProfilerTreeItem[] = [ + ...headedGroup( + 'Active Sessions', + 'sessions', + 'pulse', + this.activeSessions.map(buildSessionNode), + ), + ...headedGroup( + '.NET Processes', + 'processes', + 'server-process', + this.processes.map(buildProcessNode), + ), + ]; if (nodes.length === 0) { const empty = new ProfilerTreeItem( @@ -261,6 +247,27 @@ export class ProfilerTreeProvider implements vscode.TreeDataProvider { - const lsp = getClient(); - if (lsp === undefined) return; - const proc = await pickProcess(lsp); - if (proc === undefined) return; - await startTraceOn(proc.pid, proc.name); - }), - ); + /** + * Register the toolbar command `pickId` (the user picks a process) and the + * per-process row command `rowId`, both running `run` on the chosen PID. + * `dialogGated` skips the picker in test mode, where it would hang. + */ + function registerProcessPair( + pickId: string, + rowId: string, + run: (pid: number, processName?: string) => Promise, + dialogGated = false, + ): void { + context.subscriptions.push( + vscode.commands.registerCommand(pickId, async () => { + const lsp = getClient(); + if (lsp === undefined || (dialogGated && isTestMode)) return; + const proc = await pickProcess(lsp); + if (proc !== undefined) await run(proc.pid, proc.name); + }), + vscode.commands.registerCommand(rowId, async (item?: ProfilerTreeItem) => { + const pid = item?.processPid; + if (pid !== undefined) await run(pid, item?.processName); + }), + ); + } - context.subscriptions.push( - vscode.commands.registerCommand(CMD_PROFILER_TRACE_PROCESS, async (item?: ProfilerTreeItem) => { - const pid = item?.processPid; - if (pid === undefined) return; - await startTraceOn(pid, item?.processName); - }), - ); + /** Register stop command `id`: the row's session, else the one the user picks of `kind`. */ + function registerStop( + id: string, + kind: string, + stop: (sessionId: string) => Promise, + ): void { + context.subscriptions.push( + vscode.commands.registerCommand(id, async (item?: ProfilerTreeItem) => { + const sessionId = item?.sessionId ?? (await pickActiveSession(provider, kind)); + if (sessionId !== undefined) await stop(sessionId); + }), + ); + } - context.subscriptions.push( - vscode.commands.registerCommand(CMD_PROFILER_STOP_TRACE, async (item?: ProfilerTreeItem) => { - let sessionId = item?.sessionId; - sessionId ??= await pickActiveSession(provider, 'Trace'); - if (sessionId === undefined) return; - const outputPath = await stopTraceById(sessionId); - if (outputPath !== undefined) { - // Default Stop action also opens the resulting trace (converted if needed). - await openTraceFile(getClient(), outputPath); - } - }), - ); + registerProcessPair(CMD_PROFILER_START_TRACE, CMD_PROFILER_TRACE_PROCESS, startTraceOn); + registerStop(CMD_PROFILER_STOP_TRACE, 'Trace', async (sessionId) => { + const outputPath = await stopTraceById(sessionId); + // Default Stop action also opens the resulting trace (converted if needed). + if (outputPath !== undefined) await openTraceFile(getClient(), outputPath); + }); async function startCountersOn(pid: number, processName?: string): Promise { const lsp = getClient(); @@ -694,35 +716,8 @@ export function registerCommands( } } - context.subscriptions.push( - vscode.commands.registerCommand(CMD_PROFILER_START_COUNTERS, async () => { - const lsp = getClient(); - if (lsp === undefined) return; - const proc = await pickProcess(lsp); - if (proc === undefined) return; - await startCountersOn(proc.pid, proc.name); - }), - ); - - context.subscriptions.push( - vscode.commands.registerCommand( - CMD_PROFILER_COUNTERS_PROCESS, - async (item?: ProfilerTreeItem) => { - const pid = item?.processPid; - if (pid === undefined) return; - await startCountersOn(pid, item?.processName); - }, - ), - ); - - context.subscriptions.push( - vscode.commands.registerCommand(CMD_PROFILER_STOP_COUNTERS, async (item?: ProfilerTreeItem) => { - let sessionId = item?.sessionId; - sessionId ??= await pickActiveSession(provider, 'Counters'); - if (sessionId === undefined) return; - await stopCountersById(sessionId); - }), - ); + registerProcessPair(CMD_PROFILER_START_COUNTERS, CMD_PROFILER_COUNTERS_PROCESS, startCountersOn); + registerStop(CMD_PROFILER_STOP_COUNTERS, 'Counters', stopCountersById); /** The live session a tree item stands for, with its id. */ const sessionOf = (item?: ProfilerTreeItem): { id: string; session: SessionInfo } | undefined => { @@ -832,14 +827,11 @@ export function registerCommands( context.subscriptions.push( vscode.commands.registerCommand(CMD_PROFILER_OPEN_TRACE, async () => { - const picked = await vscode.window.showOpenDialog({ - canSelectMany: false, - filters: { Traces: ['nettrace', 'speedscope.json', 'json'] }, - title: 'Open Trace File', - }); - const file = picked?.[0]; - if (file === undefined) return; - await openTraceFile(getClient(), file.fsPath); + const file = await pickOneFile( + { Traces: ['nettrace', 'speedscope.json', 'json'] }, + 'Open Trace File', + ); + if (file !== undefined) await openTraceFile(getClient(), file); }), ); @@ -847,16 +839,11 @@ export function registerCommands( vscode.commands.registerCommand(CMD_PROFILER_CONVERT_TRACE, async () => { const lsp = getClient(); if (lsp === undefined) return; - const picked = await vscode.window.showOpenDialog({ - canSelectMany: false, - filters: { 'nettrace files': ['nettrace'] }, - title: 'Convert .nettrace File', - }); - const file = picked?.[0]; + const file = await pickOneFile({ 'nettrace files': ['nettrace'] }, 'Convert .nettrace File'); if (file === undefined) return; try { const result = await lsp.sendRequest('sharplsp/profiler/convertTrace', { - input_path: file.fsPath, + input_path: file, format: 'speedscope', }); const size = formatBytes(result.file_size_bytes); @@ -887,153 +874,97 @@ export function registerCommands( } } - context.subscriptions.push( - vscode.commands.registerCommand(CMD_PROFILER_COLLECT_DUMP, async () => { - const lsp = getClient(); - if (lsp === undefined || isTestMode) return; - const proc = await pickProcess(lsp); - if (proc === undefined) return; - await collectDumpOn(proc.pid); - }), - ); - - context.subscriptions.push( - vscode.commands.registerCommand(CMD_PROFILER_DUMP_PROCESS, async (item?: ProfilerTreeItem) => { - const pid = item?.processPid; - if (pid === undefined) return; + registerProcessPair( + CMD_PROFILER_COLLECT_DUMP, + CMD_PROFILER_DUMP_PROCESS, + async (pid) => { await collectDumpOn(pid); - }), - ); - - context.subscriptions.push( - vscode.commands.registerCommand(CMD_PROFILER_ANALYZE_HEAP, async () => { - const lsp = getClient(); - if (lsp === undefined || isTestMode) return; - - try { - const dumpFiles = await vscode.window.showOpenDialog({ - canSelectMany: false, - filters: { 'Dump files': ['dmp'] }, - title: 'Select memory dump file', - }); - const selectedFile = dumpFiles?.[0]; - if (selectedFile === undefined) return; - - const dumpPath = selectedFile.fsPath; - const result = await lsp.sendRequest('sharplsp/profiler/analyzeHeap', { - dump_path: dumpPath, - }); - - await showHeapStats(result); - } catch (err: unknown) { - const msg = getErrorMessage(err); - void vscode.window.showErrorMessage(`Heap analysis failed: ${msg}`); - } - }), + }, + true, ); - context.subscriptions.push( - vscode.commands.registerCommand(CMD_PROFILER_DIFF_SNAPSHOTS, async () => { - const lsp = getClient(); - if (lsp === undefined || isTestMode) return; - try { - await promptAndOpenDiff(context, lsp); - } catch (err: unknown) { - void vscode.window.showErrorMessage(`Heap diff failed: ${getErrorMessage(err)}`); - } - }), - ); + /** + * Register dialog-gated command `id`: it runs once a client exists (never in + * test mode, where its dialogs would hang) and reports a throw as `${what} failed`. + */ + function registerDialogCommand( + id: string, + what: string, + run: (lsp: LanguageClient) => Promise, + ): void { + context.subscriptions.push( + vscode.commands.registerCommand(id, async () => { + const lsp = getClient(); + if (lsp === undefined || isTestMode) return; + try { + await run(lsp); + } catch (err: unknown) { + void vscode.window.showErrorMessage(`${what} failed: ${getErrorMessage(err)}`); + } + }), + ); + } - context.subscriptions.push( - vscode.commands.registerCommand(CMD_PROFILER_DETECT_LEAKS, async () => { - const lsp = getClient(); - if (lsp === undefined || isTestMode) return; - try { - await detectLeaksWorkflow(context, lsp); - } catch (err: unknown) { - void vscode.window.showErrorMessage(`Leak detection failed: ${getErrorMessage(err)}`); - } - }), - ); + registerDialogCommand(CMD_PROFILER_ANALYZE_HEAP, 'Heap analysis', async (lsp) => { + const dumpPath = await pickDumpFile(); + if (dumpPath === undefined) return; + const result = await lsp.sendRequest('sharplsp/profiler/analyzeHeap', { + dump_path: dumpPath, + }); + await showHeapStats(result); + }); + registerDialogCommand(CMD_PROFILER_DIFF_SNAPSHOTS, 'Heap diff', async (lsp) => { + await promptAndOpenDiff(context, lsp); + }); + registerDialogCommand(CMD_PROFILER_DETECT_LEAKS, 'Leak detection', async (lsp) => { + await detectLeaksWorkflow(context, lsp); + }); + registerDialogCommand(CMD_PROFILER_SHOW_OBJECT_GRAPH, 'Object graph', async (lsp) => { + await promptAndOpenGraph(context, lsp); + }); + registerDialogCommand(CMD_PROFILER_INSPECT_OBJECT, 'Object inspection', inspectObject); +} - context.subscriptions.push( - vscode.commands.registerCommand(CMD_PROFILER_SHOW_OBJECT_GRAPH, async () => { - const lsp = getClient(); - if (lsp === undefined || isTestMode) return; - try { - await promptAndOpenGraph(context, lsp); - } catch (err: unknown) { - void vscode.window.showErrorMessage(`Object graph failed: ${getErrorMessage(err)}`); - } +/** Ask for a dump and an object address in it, and show that object's fields. */ +async function inspectObject(lsp: LanguageClient): Promise { + const dumpPath = await pickDumpFile(); + if (dumpPath === undefined) return; + const address = await askObjectAddress('Enter the object address (hex)'); + if (address === undefined) return; + const result = await lsp.sendRequest('sharplsp/profiler/inspectObject', { + dump_path: dumpPath, + object_address: address.trim(), + }); + await showPlainText([ + `Object Inspection: ${result.type_name}`, + `Address: ${result.address}`, + `Size: ${String(result.size_bytes)} bytes`, + `Generation: ${result.generation}`, + `Pinned: ${String(result.is_pinned)}`, + '', + 'Fields:', + '─'.repeat(60), + ...result.fields.map((f) => { + const ref = f.reference_address !== undefined ? ` → ${f.reference_address}` : ''; + return ` ${f.name}: ${f.type_name} = ${f.value}${ref}`; }), - ); - - context.subscriptions.push( - vscode.commands.registerCommand(CMD_PROFILER_INSPECT_OBJECT, async () => { - const lsp = getClient(); - if (lsp === undefined || isTestMode) return; - - const dumpFiles = await vscode.window.showOpenDialog({ - canSelectMany: false, - filters: { 'Dump files': ['dmp'] }, - title: 'Select memory dump file', - }); - const dumpFile = dumpFiles?.[0]; - if (dumpFile === undefined) return; - - const address = await vscode.window.showInputBox({ - prompt: 'Enter the object address (hex)', - placeHolder: '00007ff812345678', - validateInput: (v) => (v.trim().length > 0 ? undefined : 'Address is required'), - }); - if (address === undefined) return; - - try { - const result = await lsp.sendRequest<{ - address: string; - type_name: string; - size_bytes: number; - generation: string; - is_pinned: boolean; - fields: { - name: string; - type_name: string; - value: string; - is_reference: boolean; - reference_address?: string; - }[]; - }>('sharplsp/profiler/inspectObject', { - dump_path: dumpFile.fsPath, - object_address: address.trim(), - }); - - const lines = [ - `Object Inspection: ${result.type_name}`, - `Address: ${result.address}`, - `Size: ${String(result.size_bytes)} bytes`, - `Generation: ${result.generation}`, - `Pinned: ${String(result.is_pinned)}`, - '', - 'Fields:', - '─'.repeat(60), - ]; - for (const f of result.fields) { - const ref = f.reference_address !== undefined ? ` → ${f.reference_address}` : ''; - lines.push(` ${f.name}: ${f.type_name} = ${f.value}${ref}`); - } + ]); +} - const doc = await vscode.workspace.openTextDocument({ - content: lines.join('\n'), - language: 'plaintext', - }); - await vscode.window.showTextDocument(doc, { - preview: true, - }); - } catch (err: unknown) { - void vscode.window.showErrorMessage(`Object inspection failed: ${getErrorMessage(err)}`); - } - }), - ); +/** One object read out of a dump by `sharplsp/profiler/inspectObject`. */ +interface ObjectInspection { + address: string; + type_name: string; + size_bytes: number; + generation: string; + is_pinned: boolean; + fields: { + name: string; + type_name: string; + value: string; + is_reference: boolean; + reference_address?: string; + }[]; } // ── SpeedScope Integration ──────────────────────────────────────── @@ -1129,20 +1060,7 @@ async function showHeapStats(stats: HeapStats): Promise { const size = formatBytes(t.total_size_bytes).padStart(12); lines.push(`${name.padEnd(53)} ${count} ${size}`); } - - const doc = await vscode.workspace.openTextDocument({ - content: lines.join('\n'), - language: 'plaintext', - }); - await vscode.window.showTextDocument(doc, { preview: true }); -} - -export function formatBytes(bytes: number): string { - if (bytes < 1024) return `${String(bytes)} B`; - const kb = bytes / 1024; - if (kb < 1024) return `${kb.toFixed(1)} KB`; - const mb = kb / 1024; - return `${mb.toFixed(1)} MB`; + await showPlainText(lines); } export function formatDuration(ms: number): string { diff --git a/src/editors/vscode/src/test/suite/completions-visible.test.ts b/src/editors/vscode/src/test/suite/completions-visible.test.ts index 924a1c83..53c1c845 100644 --- a/src/editors/vscode/src/test/suite/completions-visible.test.ts +++ b/src/editors/vscode/src/test/suite/completions-visible.test.ts @@ -2,42 +2,18 @@ import * as assert from 'node:assert/strict'; import * as fs from 'node:fs'; import * as path from 'node:path'; import * as vscode from 'vscode'; -import { - closeAllEditors, - setupLspTestSuite, - teardownLspTestSuite, - waitForDocumentSymbols, - assertContainsAll, -} from './test-helpers'; -import { ACTIVATION_MS, LSP_RESPONSE_MS } from './test-timeouts'; +import { waitForDocumentSymbols, assertContainsAll, requireWorkspaceRoot } from './test-helpers'; +import { LSP_RESPONSE_MS } from './test-timeouts'; import { assertShotMembers, MEMBER_CARET } from './completion-shot-kit'; +import { useLspTestSuite } from './lsp-suite-kit'; suite('Visible Completions', () => { - let tmpDir: string; - let workspaceRoot: string; - - suiteSetup(async function () { - this.timeout(ACTIVATION_MS); - const result = await setupLspTestSuite('visible-completions-'); - tmpDir = result.tmpDir; - const ws = vscode.workspace.workspaceFolders?.[0]?.uri.fsPath; - assert.ok(ws, 'Workspace folder must be available'); - workspaceRoot = ws; - }); - - suiteTeardown(async () => { - await closeAllEditors(); - teardownLspTestSuite(tmpDir); - }); - - teardown(async () => { - await closeAllEditors(); - }); + useLspTestSuite('visible-completions-'); test('screenshot completion site offers real instance members', async function () { this.timeout(LSP_RESPONSE_MS + 5_000); - const filePath = path.join(workspaceRoot, 'CompletionShot.cs'); + const filePath = path.join(requireWorkspaceRoot(), 'CompletionShot.cs'); assert.ok(fs.existsSync(filePath), 'CompletionShot.cs fixture must exist'); const uri = vscode.Uri.file(filePath); const document = await vscode.workspace.openTextDocument(uri); diff --git a/src/editors/vscode/src/test/suite/csharp-refactor-test-kit.ts b/src/editors/vscode/src/test/suite/csharp-refactor-test-kit.ts index 79f08c42..fcb813e4 100644 --- a/src/editors/vscode/src/test/suite/csharp-refactor-test-kit.ts +++ b/src/editors/vscode/src/test/suite/csharp-refactor-test-kit.ts @@ -214,7 +214,7 @@ function vscodeKind(value: string): vscode.CodeActionKind { } } -async function assertRequiredDiagnostic( +export async function assertRequiredDiagnostic( fixture: OpenFixture, actionCase: ActionLifecycleCase, ): Promise { @@ -292,7 +292,7 @@ async function assertBoundaryRanges( await assertCaretProviderProbe(fixture, actionCase, range.end); } -async function assertOutsideActionRange( +export async function assertOutsideActionRange( fixture: OpenFixture, actionCase: ActionLifecycleCase, ): Promise { @@ -317,7 +317,7 @@ function discoveryRange( return actionCase.caretOnly ? new vscode.Range(range.start, range.start) : range; } -async function discoverAction( +export async function discoverAction( fixture: OpenFixture, actionCase: ActionLifecycleCase, ): Promise<{ readonly range: vscode.Range; readonly raw: RawCodeAction[] }> { @@ -335,7 +335,7 @@ async function discoverAction( return { range, raw }; } -async function resolveAction( +export async function resolveAction( fixture: OpenFixture, actionCase: ActionLifecycleCase, range: vscode.Range, @@ -353,7 +353,7 @@ async function resolveAction( return action.edit; } -async function applyAction( +export async function applyAction( fixture: OpenFixture, actionCase: ActionLifecycleCase, edit: vscode.WorkspaceEdit, @@ -372,7 +372,7 @@ async function applyAction( return source; } -async function assertActionRequery( +export async function assertActionRequery( fixture: OpenFixture, actionCase: ActionLifecycleCase, originalRange: vscode.Range, diff --git a/src/editors/vscode/src/test/suite/debug-adapter-startup.test.ts b/src/editors/vscode/src/test/suite/debug-adapter-startup.test.ts index e0c22a29..df796039 100644 --- a/src/editors/vscode/src/test/suite/debug-adapter-startup.test.ts +++ b/src/editors/vscode/src/test/suite/debug-adapter-startup.test.ts @@ -170,16 +170,18 @@ suite('Debug adapter startup is total', () => { ); }); - // The router's teardown must signal the ADAPTER, never the process group the - // extension host itself sits in. Adapter resolution builds a router purely to - // read which binary it would spawn and throws it away in the SAME TICK, so - // `dispose()` routinely runs while the asynchronous spawn failure is still in - // flight and the router still believes its child is open. - // Implements [DEBUG-ARCHITECTURE-ROUTER]. - test('disposing a router whose adapter never started signals only that adapter', async () => { - const missing = path.join(tmpDir, 'never-started', EXE); + /** + * Start a router on an adapter that was never there, `act` on it, and prove + * the teardown signalled ONLY that adapter: the extension host never signals + * itself, and a canary sibling in our own process group survives. + */ + async function assertSignalsOnlyTheAdapter( + dir: string, + what: string, + act: (router: DapRouter) => Promise, + ): Promise { + const missing = path.join(tmpDir, dir, EXE); assert.ok(!fs.existsSync(missing), 'the premise: nothing is at that path'); - const canary = processGroupCanary(); assert.ok( typeof canary.pid === 'number' && canary.pid > 0, @@ -190,55 +192,50 @@ suite('Debug adapter startup is total', () => { const outcome = DapRouter.start(missing); assert.ok(outcome.ok, 'ENOENT fails asynchronously, not at construction'); if (!outcome.ok) return; - outcome.value.dispose(); - await new Promise((resolve) => setTimeout(resolve, 750)); - + await act(outcome.value); assert.strictEqual(host.count(), 0, 'the extension host must never signal ITSELF'); assert.ok( stillAlive(canary), - `disposing an unstarted adapter killed an unrelated sibling (signal ${String( - canary.signalCode, - )}): the kill reached our whole process group instead of the adapter`, + `${what} killed an unrelated sibling (signal ${String(canary.signalCode)}): ` + + 'the kill reached our whole process group instead of the adapter', ); } finally { host.stop(); canary.kill('SIGKILL'); } + } + + // The router's teardown must signal the ADAPTER, never the process group the + // extension host itself sits in. Adapter resolution builds a router purely to + // read which binary it would spawn and throws it away in the SAME TICK, so + // `dispose()` routinely runs while the asynchronous spawn failure is still in + // flight and the router still believes its child is open. + // Implements [DEBUG-ARCHITECTURE-ROUTER]. + test('disposing a router whose adapter never started signals only that adapter', async () => { + await assertSignalsOnlyTheAdapter( + 'never-started', + 'disposing an unstarted adapter', + async (router) => { + router.dispose(); + await new Promise((resolve) => setTimeout(resolve, 750)); + }, + ); }); // The same hazard on the restart path, where it is worse: `respawn` escalates // to SIGKILL after a grace second, and a SIGKILL aimed at our own process // group cannot be caught, logged or survived by anything in it. test('respawning a router whose adapter never started signals only that adapter', async () => { - const missing = path.join(tmpDir, 'never-respawned', EXE); - assert.ok(!fs.existsSync(missing), 'the premise: nothing is at that path'); - - const canary = processGroupCanary(); - assert.ok( - typeof canary.pid === 'number' && canary.pid > 0, - 'the premise: the canary must really be running, in our own process group', + await assertSignalsOnlyTheAdapter( + 'never-respawned', + 'respawning past an unstarted adapter', + async (router) => { + router.respawn([]); + // Past the SIGKILL escalation, so BOTH signals have had their chance. + await new Promise((resolve) => setTimeout(resolve, 1_750)); + router.dispose(); + }, ); - const host = catchHostSigterm(); - try { - const outcome = DapRouter.start(missing); - assert.ok(outcome.ok, 'ENOENT fails asynchronously, not at construction'); - if (!outcome.ok) return; - outcome.value.respawn([]); - // Past the SIGKILL escalation, so BOTH signals have had their chance. - await new Promise((resolve) => setTimeout(resolve, 1_750)); - outcome.value.dispose(); - - assert.strictEqual(host.count(), 0, 'the extension host must never signal ITSELF'); - assert.ok( - stillAlive(canary), - `respawning past an unstarted adapter killed an unrelated sibling (signal ${String( - canary.signalCode, - )}): the kill reached our whole process group instead of the adapter`, - ); - } finally { - host.stop(); - canary.kill('SIGKILL'); - } }); test('a resolvable adapter still starts, so the guard did not disable debugging', () => { diff --git a/src/editors/vscode/src/test/suite/debug-breakpoint-conditions-e2e.test.ts b/src/editors/vscode/src/test/suite/debug-breakpoint-conditions-e2e.test.ts index 5c076935..8edf9805 100644 --- a/src/editors/vscode/src/test/suite/debug-breakpoint-conditions-e2e.test.ts +++ b/src/editors/vscode/src/test/suite/debug-breakpoint-conditions-e2e.test.ts @@ -31,43 +31,14 @@ import { useDebuggee, debugTest, } from './debug-suite-kit'; -import type { DapRecorder } from './debug-dap-kit'; import { deepEq, eq, requireAt } from './test-helpers'; - -/** The `breakpoints` entries of one `setBreakpoints` request. */ -function entriesOf(args: Record): Record[] { - const list: unknown = args['breakpoints']; - return Array.isArray(list) ? (list as Record[]) : []; -} - -/** - * Wait until a `setBreakpoints` request carries exactly `want` under `field`. - * - * Reading the LAST request the wire holds asks what is there RIGHT NOW, which - * is the final state only if nothing further is in flight. One gesture is - * routinely several requests - the workbench syncs a breakpoint as a remove - * then an add - so the last entry at an arbitrary moment can be an - * INTERMEDIATE one carrying the state before the gesture completed, and which - * one a test observes is then decided by how fast the machine is. - */ -async function waitForSentField( - recorder: DapRecorder, - field: string, - want: readonly (string | undefined)[], - why: string, -): Promise<(string | undefined)[]> { - const read = (args: Record): (string | undefined)[] => - entriesOf(args).map((entry) => entry[field] as string | undefined); - const carried = await recorder.waitForRequestArgs( - 'setBreakpoints', - (args) => { - const got = read(args); - return got.length === want.length && want.every((value, at) => got[at] === value); - }, - why, - ); - return read(carried); -} +import { + assertValues, + onlySourceBreakpoint, + entriesOf, + waitForSentField, + assertSentField, +} from './debug-inspect-kit'; /** How many times the fixture's `Add` call site is reached in one run. */ const LOOP_ITERATIONS = 3; @@ -99,13 +70,9 @@ suite('Debug breakpoints — conditions, hit counts and logpoints', () => { '[DEBUG-PROTOCOL-CAPABILITIES] lists supportsConditionalBreakpoints as Yes for Phase 4; ' + 'without it VS Code never forwards the condition and the breakpoint stops every time', ); - deepEq( - await waitForSentField( - recorder, - 'condition', - ['index == 3'], - 'the C# expression must be forwarded verbatim to the adapter', - ), + await assertSentField( + recorder, + 'condition', ['index == 3'], 'the C# expression must be forwarded verbatim to the adapter', ); @@ -123,16 +90,14 @@ suite('Debug breakpoints — conditions, hit counts and logpoints', () => { 'the conditional breakpoint', ); const locals = await localsOf(session, frame.id); - eq( - variableNamed(locals, 'index').value, - '3', - 'the stop must happen on the iteration the CONDITION names, not on the first one', - ); - eq( - variableNamed(locals, 'running').value, - '5', - 'two iterations must already have run: 2 -> 3 -> 5, with 8 still to come', - ); + assertValues(locals, [ + [ + 'index', + '3', + 'the stop must happen on the iteration the CONDITION names, not on the first one', + ], + ['running', '5', 'two iterations must already have run: 2 -> 3 -> 5, with 8 still to come'], + ]); await recorder.assertNoFurtherStop(1, 'a condition true on ONE iteration stops exactly once'); // Interaction 4 — replace it with a condition that is never true. @@ -175,13 +140,9 @@ suite('Debug breakpoints — conditions, hit counts and logpoints', () => { true, '[DEBUG-PROTOCOL-CAPABILITIES] lists supportsHitConditionalBreakpoints as Yes for Phase 4', ); - deepEq( - await waitForSentField( - recorder, - 'hitCondition', - ['3'], - 'the hit condition must be forwarded to the adapter, not evaluated in the editor', - ), + await assertSentField( + recorder, + 'hitCondition', ['3'], 'the hit condition must be forwarded to the adapter, not evaluated in the editor', ); @@ -269,13 +230,9 @@ suite('Debug breakpoints — conditions, hit counts and logpoints', () => { 'makes the DapRouter responsible for rewriting them — an unadvertised capability ' + 'means VS Code silently downgrades the logpoint to a normal breakpoint', ); - deepEq( - await waitForSentField( - recorder, - 'logMessage', - [message], - 'the log message must reach the adapter layer that emulates it', - ), + await assertSentField( + recorder, + 'logMessage', [message], 'the log message must reach the adapter layer that emulates it', ); @@ -326,9 +283,7 @@ suite('Debug breakpoints — conditions, hit counts and logpoints', () => { vscode.debug.addBreakpoints([ breakpointAt(fixture, 'accumulate-call', { condition: 'index == 2 && running > 1' }), ]); - eq(vscode.debug.breakpoints.length, 1, 'one conditional breakpoint is armed'); - const armed = requireAt(vscode.debug.breakpoints, 0, 'the conditional breakpoint'); - assert.ok(armed instanceof vscode.SourceBreakpoint, 'armed as a source breakpoint'); + const armed = onlySourceBreakpoint('the conditional breakpoint'); eq(armed.condition, 'index == 2 && running > 1', 'carrying the expression the user typed'); eq(armed.hitCondition, undefined, 'and no hit count - this is an expression condition'); @@ -362,12 +317,10 @@ suite('Debug breakpoints — conditions, hit counts and logpoints', () => { const frame = await topFrame(session, stop.threadId); assertStoppedAt(frame, fixture, 'accumulate-call', 'Accumulate', 'the selected pass'); const locals = await localsOf(session, frame.id); - eq(variableNamed(locals, 'index').value, '2', 'stopped on the pass the condition selects'); - eq( - variableNamed(locals, 'running').value, - '3', - 'with the accumulator the condition required', - ); + assertValues(locals, [ + ['index', '2', 'stopped on the pass the condition selects'], + ['running', '3', 'with the accumulator the condition required'], + ]); await vscode.commands.executeCommand(CMD_CONTINUE); await assertRanToCompletion(recorder, 0, 'a multi-local conditional breakpoint'); eq( @@ -456,8 +409,7 @@ suite('Debug breakpoints — conditions, hit counts and logpoints', () => { vscode.debug.addBreakpoints([ breakpointAt(fixture, 'accumulate-call', { hitCondition: '>= 2' }), ]); - const armed = requireAt(vscode.debug.breakpoints, 0, 'the hit-count breakpoint'); - assert.ok(armed instanceof vscode.SourceBreakpoint, 'armed as a source breakpoint'); + const armed = onlySourceBreakpoint('the hit-count breakpoint'); eq(armed.hitCondition, '>= 2', 'carrying the relational hit condition the user typed'); eq(armed.condition, undefined, 'a hit count is not an expression condition'); const session = await startDebuggee(debuggee(), { mode: MODE.plain }); diff --git a/src/editors/vscode/src/test/suite/debug-breakpoints-e2e.test.ts b/src/editors/vscode/src/test/suite/debug-breakpoints-e2e.test.ts index efc3c549..4ac23dba 100644 --- a/src/editors/vscode/src/test/suite/debug-breakpoints-e2e.test.ts +++ b/src/editors/vscode/src/test/suite/debug-breakpoints-e2e.test.ts @@ -34,6 +34,7 @@ import { debugTest, } from './debug-suite-kit'; import { deepEq, eq, requireAt } from './test-helpers'; +import { assertSentField } from './debug-inspect-kit'; /** The 0-based lines the workbench currently holds source breakpoints on. */ function armedLines(): number[] { @@ -283,24 +284,12 @@ suite('Debug breakpoints — F9, the Breakpoints view, and function breakpoints' // Waited for by NAME, not read off the end of the wire: `requested` was // sampled above to prove the request exists at all, and the entry last on // the wire at that instant need not be the one carrying this name. - const sentFunction = await recorder.waitForRequestArgs( - 'setFunctionBreakpoints', - (args) => { - const list: unknown = args['breakpoints']; - return ( - Array.isArray(list) && - list.length === 1 && - String((list[0] as Record)['name']) === functionName - ); - }, - 'the fully-qualified method name must be forwarded verbatim', - ); - const names: unknown = sentFunction['breakpoints']; - assert.ok(Array.isArray(names), '`setFunctionBreakpoints` carries a breakpoints array'); - deepEq( - names.map((entry) => String((entry as Record)['name'])), + await assertSentField( + recorder, + 'name', [functionName], 'the fully-qualified method name must be forwarded verbatim', + 'setFunctionBreakpoints', ); // Interaction 3 — the debuggee must stop on entry to Add, three frames deep. diff --git a/src/editors/vscode/src/test/suite/debug-callstack-e2e.test.ts b/src/editors/vscode/src/test/suite/debug-callstack-e2e.test.ts index 79d6feae..9760ec00 100644 --- a/src/editors/vscode/src/test/suite/debug-callstack-e2e.test.ts +++ b/src/editors/vscode/src/test/suite/debug-callstack-e2e.test.ts @@ -36,6 +36,7 @@ import { import { comparablePath, deepEq, eq, neq, pollUntilResult, requireAt } from './test-helpers'; import { LSP_RESPONSE_MS } from './test-timeouts'; import { assertAnswered, assertSessionEnded, assertAdapterAlive } from './debug-dap-kit'; +import { assertValues } from './debug-inspect-kit'; /** The logical await chain the sidecar must reconstruct, innermost first. */ const ASYNC_CHAIN = ['LeafAsync', 'MiddleAsync', 'RootAsync'] as const; @@ -148,13 +149,15 @@ suite('Debug call stack — frames, per-frame state, threads and async chains', // Interaction 3 — click the CALLER: its locals are Accumulate's. const caller = requireAt(frames, 1, 'the Accumulate frame'); const callerLocals = await localsOf(session, caller.id); - eq( - variableNamed(callerLocals, 'running').value, - '2', - 'selecting a caller frame must read THAT frame’s state: `running` is still the seed ' + - 'because the assignment on this line has not completed', - ); - eq(variableNamed(callerLocals, 'index').value, '1', 'and the loop is on its first iteration'); + assertValues(callerLocals, [ + [ + 'running', + '2', + 'selecting a caller frame must read THAT frame’s state: `running` is still the seed ' + + 'because the assignment on this line has not completed', + ], + ['index', '1', 'and the loop is on its first iteration'], + ]); deepEq( callerLocals.map((local) => local.name).includes('sum'), false, diff --git a/src/editors/vscode/src/test/suite/debug-evaluate-e2e.test.ts b/src/editors/vscode/src/test/suite/debug-evaluate-e2e.test.ts index e6d31024..c09c84ae 100644 --- a/src/editors/vscode/src/test/suite/debug-evaluate-e2e.test.ts +++ b/src/editors/vscode/src/test/suite/debug-evaluate-e2e.test.ts @@ -39,6 +39,7 @@ import { debugTest, } from './debug-suite-kit'; import { deepEq, eq, neq, requireAt } from './test-helpers'; +import { assertEvaluatesEverywhere } from './debug-inspect-kit'; /** T1 expressions: field access, arithmetic, casts, null checks. All "Works". */ const TIER_ONE: readonly { expression: string; expected: string; kind: string }[] = [ @@ -300,35 +301,16 @@ suite('Debug evaluation — hover, watch, REPL, setVariable and DebuggerDisplay' // Interaction 2 — a method call on a local, in each of the three contexts // the specification names. Answering in one and not another is worse than // failing everywhere: the user cannot tell which panel to trust. - const calls: readonly { expression: string; expected: string }[] = [ - { expression: 'box.Describe()', expected: 'boxed=8' }, - { expression: 'numbers.Contains(20)', expected: 'true' }, - { expression: 'text.Length', expected: '7' }, - ]; - for (const { expression, expected } of calls) { - const watch = await evaluate(session, expression, frame.id, 'watch'); - assert.ok( - watch.value.includes(expected), - expression + - ' is a T2 "method calls on locals" expression, marked Works for Phase 4; ' + - 'the Watch panel answered ' + - JSON.stringify(watch.value), - ); - const repl = await evaluate(session, expression, frame.id, 'repl'); - eq( - repl.value, - watch.value, - expression + ': the Debug Console must agree with the Watch panel over one frame', - ); - const hover = await evaluate(session, expression, frame.id, 'hover'); - eq( - hover.value, - watch.value, - expression + - ': and so must a hover - three answers for one expression is a bug the ' + - 'user reads as their own code misbehaving', - ); - } + await assertEvaluatesEverywhere( + session, + frame.id, + [ + { expression: 'box.Describe()', expected: 'boxed=8' }, + { expression: 'numbers.Contains(20)', expected: 'true' }, + { expression: 'text.Length', expected: '7' }, + ], + 'a T2 "method calls on locals" expression, marked Works for Phase 4', + ); // Interaction 3 — a call with an argument computed from another local, and // a chained call. Both are still T2, and both must survive the round trip. diff --git a/src/editors/vscode/src/test/suite/debug-exception-filters-e2e.test.ts b/src/editors/vscode/src/test/suite/debug-exception-filters-e2e.test.ts index 1431d11c..dbe1b465 100644 --- a/src/editors/vscode/src/test/suite/debug-exception-filters-e2e.test.ts +++ b/src/editors/vscode/src/test/suite/debug-exception-filters-e2e.test.ts @@ -37,7 +37,8 @@ import { runToFirstStop, debugTest, } from './debug-suite-kit'; -import { deepEq, eq, neq, assertContainsAll } from './test-helpers'; +import { deepEq, eq, neq } from './test-helpers'; +import { assertRunsPastHandledThrow } from './debug-inspect-kit'; /** A type the fixture never throws — the exclude half of every filter case. */ const NEVER_THROWN_TYPE = 'System.DivideByZeroException'; @@ -115,34 +116,18 @@ suite('Debug exceptions — per-type include and exclude filters', () => { const { session } = await runToFirstStop(debuggee(), 'main-mode', { mode: MODE.caught }); const beforeFilter = recorder.requests('setExceptionBreakpoints').length; await dap(session, 'setExceptionBreakpoints', onlyType(NEVER_THROWN_TYPE)); - const baseline = recorder.stops().length; // Interaction 2 — continue. The InvalidOperationException the program DOES - // throw must pass straight through the filter. - await vscode.commands.executeCommand(CMD_CONTINUE); - await assertRanToCompletion(recorder, 0, 'an exception the filter excludes'); - await recorder.waitForOutput(`handled ${CAUGHT_MESSAGE}`); - deepEq( - recorder - .stops() - .slice(baseline) - .map((stop) => `${stop.reason}:${stop.text}`), - [], + // throw must pass straight through the filter, and the program really RUN: + // a filter that silences a stop by killing the session would satisfy "no + // stops" while proving the opposite of what this test is about. + await assertRunsPastHandledThrow( + recorder, + 'an exception the filter excludes', `a filter naming only ${NEVER_THROWN_TYPE} must ignore ${CAUGHT_TYPE} entirely. ` + 'Breaking anyway means the type list is decorative and the filter is really "break ' + 'on all", which [DEBUG-FEATURES-EXCEPTIONS] lists as a SEPARATE row', ); - await recorder.waitForOutput('done caught 45'); - - // Interaction 3 — the program really RAN. A filter that silences a stop by - // killing the session would satisfy "no stops" while proving the opposite - // of what this test is about. - eq(recorder.stops().length, baseline, 'no stop was added by the excluded type'); - assertContainsAll( - recorder.outputText(), - [`handled ${CAUGHT_MESSAGE}`, 'done caught 45'], - 'recorder.outputText()', - ); // Interaction 4 — the filter that WAS set is the one that was asked for. // A `setExceptionBreakpoints` whose type list is dropped on the way to the diff --git a/src/editors/vscode/src/test/suite/debug-exceptions-e2e.test.ts b/src/editors/vscode/src/test/suite/debug-exceptions-e2e.test.ts index a8dee836..9fdd63f4 100644 --- a/src/editors/vscode/src/test/suite/debug-exceptions-e2e.test.ts +++ b/src/editors/vscode/src/test/suite/debug-exceptions-e2e.test.ts @@ -44,7 +44,8 @@ import { stopInFrame, debugTest, } from './debug-suite-kit'; -import { deepEq, eq, neq, requireAt, assertContainsAll } from './test-helpers'; +import { deepEq, eq, neq, requireAt } from './test-helpers'; +import { assertRunsPastHandledThrow } from './debug-inspect-kit'; /** The filter id every DAP adapter uses for "break on every throw". */ const FILTER_ALL = 'all'; @@ -237,35 +238,15 @@ suite('Debug exceptions — breaking on them, and ignoring them', () => { 'everything and the user would have no way to get the behaviour they asked for', ); - // Interaction 2 — continue. The handled throw must NOT stop the debuggee. - const baseline = recorder.stops().length; - await vscode.commands.executeCommand(CMD_CONTINUE); - await assertRanToCompletion(recorder, 0, 'an ignored, handled exception'); - await recorder.waitForOutput(`handled ${CAUGHT_MESSAGE}`); - - // Interaction 3 — prove the negative, precisely. - deepEq( - recorder - .stops() - .slice(baseline) - .map((stop) => `${stop.reason}:${stop.text}`), - [], + // Interaction 2 — continue. The handled throw must NOT stop the debuggee, + // and the program RAN, to its own end, through the catch block and past it. + await assertRunsPastHandledThrow( + recorder, + 'an ignored, handled exception', 'a throw the program CATCHES must be invisible when only the unhandled filter is ' + 'selected. Breaking here is the defect that makes a debugger useless on any codebase ' + 'that uses exceptions for control flow', ); - await recorder.waitForOutput('done caught 45'); - - // Interaction 4 - the program RAN, and ran to its own end. "No stops" is - // also what a session that died on launch produces, so the negative above - // only means something beside the positive evidence that the debuggee got - // all the way through the catch block and past it. - assertContainsAll( - recorder.outputText(), - [`handled ${CAUGHT_MESSAGE}`, 'done caught 45'], - 'recorder.outputText()', - ); - eq(recorder.stops().length, baseline, 'with no stop added after the gate'); // Interaction 5 - the SELECTION is still the one that was asked for. A // later `setExceptionBreakpoints` that quietly re-adds `all` would produce diff --git a/src/editors/vscode/src/test/suite/debug-fsharp-inspection-e2e.test.ts b/src/editors/vscode/src/test/suite/debug-fsharp-inspection-e2e.test.ts index 0d10a8aa..c52f33e3 100644 --- a/src/editors/vscode/src/test/suite/debug-fsharp-inspection-e2e.test.ts +++ b/src/editors/vscode/src/test/suite/debug-fsharp-inspection-e2e.test.ts @@ -37,6 +37,7 @@ import { } from './debug-suite-kit'; import { deepEq, eq, neq, assertContainsAll } from './test-helpers'; import { assertAnswered, assertSessionEnded, assertAdapterAlive } from './debug-dap-kit'; +import { assertValues, assertEvaluatesEverywhere } from './debug-inspect-kit'; /** CLR spellings [DEBUG-FSHARP-UNIONS] names as the wrong answer. */ const RAW_CLR_FORMS: readonly string[] = ['Tag =', 'Tag=', 'FSharpOption`1', 'FSharpList`1']; @@ -135,16 +136,10 @@ suite('Debug F# — unions, records, tuples and task {} stacks', () => { const point = variableNamed(locals, 'point'); assert.ok(point.reference > 0, 'an F# record must be expandable'); const fields = await variablesOf(session, point.reference); - eq( - variableNamed(fields, 'X').value, - '8', - '`accumulate 2` produces 8, so the record field X must read 8', - ); - eq( - variableNamed(fields, 'Y').value, - '12', - '`area (Rect(3, 4))` is 12, so the record field Y must read 12', - ); + assertValues(fields, [ + ['X', '8', '`accumulate 2` produces 8, so the record field X must read 8'], + ['Y', '12', '`area (Rect(3, 4))` is 12, so the record field Y must read 12'], + ]); // Interaction 3 — the tuple's elements. const pair = variableNamed(locals, 'pair'); @@ -337,8 +332,10 @@ suite('Debug F# — unions, records, tuples and task {} stacks', () => { const fields = await variablesOf(session, point.reference); const fieldNames = fields.map((field) => field.name); assertContainsAll(fieldNames, ['X', 'Y'], 'fieldNames'); - eq(variableNamed(fields, 'X').value, '8', 'X carries the value the program bound'); - eq(variableNamed(fields, 'Y').value, '12', 'and Y the value the match computed'); + assertValues(fields, [ + ['X', '8', 'X carries the value the program bound'], + ['Y', '12', 'and Y the value the match computed'], + ]); for (const field of fields) { assert.ok( !RAW_CLR_FORMS.some((raw) => field.value.includes(raw)), @@ -403,32 +400,17 @@ suite('Debug F# — unions, records, tuples and task {} stacks', () => { ); // Interaction 2 — T1 expressions over F# bindings, in all three contexts. - const expressions: readonly { expression: string; expected: string }[] = [ - { expression: 'total', expected: '8' }, - { expression: 'point.X', expected: '8' }, - { expression: 'point.Y', expected: '12' }, - { expression: 'total + 1', expected: '9' }, - ]; - for (const { expression, expected } of expressions) { - const watch = await evaluate(session, expression, frame.id, 'watch'); - assert.ok( - watch.value.includes(expected), - expression + - ' is a T1 expression over an F# binding and must evaluate; the Watch ' + - 'panel answered ' + - JSON.stringify(watch.value), - ); - eq( - (await evaluate(session, expression, frame.id, 'hover')).value, - watch.value, - expression + ': a hover must agree with the Watch panel', - ); - eq( - (await evaluate(session, expression, frame.id, 'repl')).value, - watch.value, - expression + ': and so must the Debug Console', - ); - } + await assertEvaluatesEverywhere( + session, + frame.id, + [ + { expression: 'total', expected: '8' }, + { expression: 'point.X', expected: '8' }, + { expression: 'point.Y', expected: '12' }, + { expression: 'total + 1', expected: '9' }, + ], + 'a T1 expression over an F# binding', + ); // Interaction 3 — the evaluated values must agree with the PANEL, and // evaluating must not disturb the session. diff --git a/src/editors/vscode/src/test/suite/debug-fsharp-stepping-e2e.test.ts b/src/editors/vscode/src/test/suite/debug-fsharp-stepping-e2e.test.ts index 25ff10d5..5f7703a3 100644 --- a/src/editors/vscode/src/test/suite/debug-fsharp-stepping-e2e.test.ts +++ b/src/editors/vscode/src/test/suite/debug-fsharp-stepping-e2e.test.ts @@ -18,7 +18,6 @@ import { CMD_CONTINUE, CMD_STEP_INTO, CMD_STEP_OUT, - CMD_STEP_OVER, CMD_TOGGLE_BREAKPOINT, assertExceptionIs, assertStopReason, @@ -34,7 +33,6 @@ import { topFrame, trace, variableNamed, - walk, } from './debug-drive-kit'; import { assertBreakpointsBound, @@ -48,6 +46,7 @@ import { debugTest, } from './debug-suite-kit'; import { deepEq, eq, requireAt, assertContainsAll } from './test-helpers'; +import { assertValues, onlySourceBreakpoint, stepIntoAddThroughLoop } from './debug-inspect-kit'; suite('Debug F# — breakpoints, stepping and exceptions', () => { const debuggee = useDebuggee('debug-step-fs-', 'fsharp'); @@ -129,30 +128,18 @@ suite('Debug F# — breakpoints, stepping and exceptions', () => { 'stepping into an F# function must push exactly one frame', ); - // Interaction 3 — F10 twice down to the call inside the loop. - const toCall = await walk(recorder, [CMD_STEP_OVER, CMD_STEP_OVER]); - deepEq( - trace(toCall.frames), - [ - at(fixture, 'accumulate', 'accumulate-loop'), - at(fixture, 'accumulate', 'accumulate-call'), - ], - 'F10 in an F# `for` loop must visit the loop header, then the body — landing inside ' + - '`add` means `next` was serviced as `stepIn`', - ); - - // Interaction 4 — F11 into `add`; three F# frames, innermost first. - const intoAdd = await stepToFrame(recorder, CMD_STEP_INTO); - assertStoppedAt(intoAdd.frame, fixture, 'add-body', 'add', 'F11 into the innermost F# call'); - const deep = await stackFrames(session, intoAdd.stop.threadId); - deepEq( - deep.slice(0, 3).map((frame) => methodOf(frame)), - ['add', 'accumulate', 'main'], - 'an F# call stack must name F# functions, in DAP’s innermost-first order', - ); + // Interactions 3 and 4 — F10 twice down to the call inside the F# `for` + // loop, then F11 into `add`: three F# frames naming F# functions. + const { deep } = await stepIntoAddThroughLoop(recorder, session, fixture, [ + 'add', + 'accumulate', + 'main', + ]); const addLocals = await localsOf(session, requireAt(deep, 0, 'the `add` frame').id); - eq(variableNamed(addLocals, 'left').value, '2', 'the F# function’s first parameter is bound'); - eq(variableNamed(addLocals, 'right').value, '1', 'and its second'); + assertValues(addLocals, [ + ['left', '2', 'the F# function’s first parameter is bound'], + ['right', '1', 'and its second'], + ]); // Interaction 5 — Shift+F11 back out, one frame per press. const outOnce = await stepToFrame(recorder, CMD_STEP_OUT); @@ -293,9 +280,7 @@ suite('Debug F# — breakpoints, stepping and exceptions', () => { vscode.debug.addBreakpoints([ breakpointAt(fixture, 'accumulate-call', { condition: 'index = 2' }), ]); - eq(vscode.debug.breakpoints.length, 1, 'one conditional breakpoint is armed in F# source'); - const armed = requireAt(vscode.debug.breakpoints, 0, 'the F# conditional breakpoint'); - assert.ok(armed instanceof vscode.SourceBreakpoint, 'armed as a source breakpoint'); + const armed = onlySourceBreakpoint('the F# conditional breakpoint'); eq(armed.condition, 'index = 2', 'carrying the F# expression the user typed'); assert.ok( armed.location.uri.fsPath.endsWith('.fs'), @@ -363,8 +348,10 @@ suite('Debug F# — breakpoints, stepping and exceptions', () => { const locals = await localsOf(session, frame.id); const names = locals.map((local) => local.name); assertContainsAll(names, ['left', 'right'], 'names'); - eq(variableNamed(locals, 'left').value, '2', 'carrying the value the loop passed it'); - eq(variableNamed(locals, 'right').value, '1', 'and the first loop index'); + assertValues(locals, [ + ['left', '2', 'carrying the value the loop passed it'], + ['right', '1', 'and the first loop index'], + ]); eq( (await evaluate(session, 'left + right', frame.id, 'watch')).value, '3', diff --git a/src/editors/vscode/src/test/suite/debug-inspect-kit.ts b/src/editors/vscode/src/test/suite/debug-inspect-kit.ts new file mode 100644 index 00000000..130092b6 --- /dev/null +++ b/src/editors/vscode/src/test/suite/debug-inspect-kit.ts @@ -0,0 +1,205 @@ +// The checks debug suites make once the debuggee has stopped: what the +// Variables panel, the Watch panel, the Debug Console and a hover read, the one +// breakpoint the workbench holds, and how a session runs on to its end. +// +// Spec: [DEBUG-FEATURES-VARIABLES], [DEBUG-FEATURES-BREAKPOINTS], +// [DEBUG-FEATURES-EXCEPTIONS]. +import * as assert from 'node:assert/strict'; +import * as vscode from 'vscode'; +import { DapRecorder, type StopRecord } from './debug-dap-kit'; +import { + CMD_CONTINUE, + evaluate, + gesture, + variableNamed, + type Variable, + CMD_STEP_INTO, + CMD_STEP_OVER, + assertStoppedAt, + at, + methodOf, + stackFrames, + stepToFrame, + trace, + walk, + type Frame, +} from './debug-drive-kit'; +import { CAUGHT_MESSAGE, type DebugFixture } from './debug-fixture-programs'; +import { assertRanToCompletion } from './debug-suite-kit'; +import { assertContainsAll, requireAt } from './test-helpers'; +import { DEBUG_SESSION_MS } from './test-timeouts'; + +/** Each named variable reads exactly its expected value, each row failing with its own reason. */ +export function assertValues( + variables: readonly Variable[], + rows: readonly (readonly [name: string, value: string, why: string])[], +): void { + for (const [name, value, why] of rows) + assert.strictEqual(variableNamed(variables, name).value, value, why); +} + +/** F5 from the stop, then the session runs to its end with no adapter transport error. */ +export async function continueToEnd(recorder: DapRecorder, why: string): Promise { + await gesture(CMD_CONTINUE); + await recorder.waitForEvents('terminated', 1, DEBUG_SESSION_MS); + assert.deepStrictEqual(recorder.errors, [], why); +} + +/** + * Each `expression` evaluates in `frameId` to a Watch value containing its + * `expected` text, and the Debug Console and a hover agree with that value + * exactly: three answers for one expression is a bug the user reads as their + * own code misbehaving. `tier` says what kind of expression the rows are. + */ +export async function assertEvaluatesEverywhere( + session: vscode.DebugSession, + frameId: number, + rows: readonly { readonly expression: string; readonly expected: string }[], + tier: string, +): Promise { + for (const { expression, expected } of rows) { + const watch = await evaluate(session, expression, frameId, 'watch'); + assert.ok( + watch.value.includes(expected), + `${expression} is ${tier} and must evaluate; the Watch panel answered ${JSON.stringify(watch.value)}`, + ); + for (const context of ['repl', 'hover'] as const) { + const other = await evaluate(session, expression, frameId, context); + assert.strictEqual( + other.value, + watch.value, + `${expression}: ${context} must agree with Watch`, + ); + } + } +} + +/** The ONE breakpoint armed in the workbench, asserted a source (line) breakpoint. */ +export function onlySourceBreakpoint(why: string): vscode.SourceBreakpoint { + assert.strictEqual(vscode.debug.breakpoints.length, 1, `${why}: exactly one is armed`); + const armed = requireAt(vscode.debug.breakpoints, 0, why); + assert.ok(armed instanceof vscode.SourceBreakpoint, `${why}: armed as a SOURCE breakpoint`); + return armed; +} + +/** + * F5 past the program's HANDLED throw with nothing set to catch it: the run + * completes, prints both its `handled` and `done` lines, and gains no stop. + * "No stops" alone is also what a session that died on launch produces, so the + * negative only counts beside that positive evidence. `noStopWhy` says why a + * stop there would be the defect. + */ +export async function assertRunsPastHandledThrow( + recorder: DapRecorder, + why: string, + noStopWhy: string, +): Promise { + const baseline = recorder.stops().length; + await vscode.commands.executeCommand(CMD_CONTINUE); + await assertRanToCompletion(recorder, 0, why); + await recorder.waitForOutput(`handled ${CAUGHT_MESSAGE}`); + const added = recorder.stops().slice(baseline); + assert.deepStrictEqual( + added.map((stop) => `${stop.reason}:${stop.text}`), + [], + noStopWhy, + ); + await recorder.waitForOutput('done caught 45'); + assert.strictEqual(recorder.stops().length, baseline, `${why}: no stop was added`); + assertContainsAll( + recorder.outputText(), + [`handled ${CAUGHT_MESSAGE}`, 'done caught 45'], + 'recorder.outputText()', + ); +} + +/** + * From a stop inside `accumulate`: F10 twice visits the loop header and then + * the body's call — landing inside `add` instead means `next` was serviced as + * `stepIn` — and F11 enters `add`, whose stack names `add`, `accumulate` and + * `main` innermost-first. `names` spells those three in the fixture's language. + */ +export async function stepIntoAddThroughLoop( + recorder: DapRecorder, + session: vscode.DebugSession, + fixture: DebugFixture, + names: readonly [add: string, accumulate: string, main: string], +): Promise<{ intoAdd: { stop: StopRecord; frame: Frame }; deep: Frame[] }> { + const [add, accumulate] = names; + const toCall = await walk(recorder, [CMD_STEP_OVER, CMD_STEP_OVER]); + assert.deepStrictEqual( + trace(toCall.frames), + [at(fixture, accumulate, 'accumulate-loop'), at(fixture, accumulate, 'accumulate-call')], + 'F10 in a for-loop visits the loop header, then the body statement', + ); + const intoAdd = await stepToFrame(recorder, CMD_STEP_INTO); + assertStoppedAt(intoAdd.frame, fixture, 'add-body', add, 'F11 into the innermost call'); + const deep = await stackFrames(session, intoAdd.stop.threadId); + assert.deepStrictEqual( + deep.slice(0, 3).map((frame) => methodOf(frame)), + [...names], + '[DEBUG-FEATURES-STACK]: `stackTrace` names the frames innermost-first', + ); + return { intoAdd, deep }; +} + +/** The `breakpoints` entries of one `setBreakpoints`-family request. */ +export function entriesOf(args: Record): Record[] { + const list: unknown = args['breakpoints']; + return Array.isArray(list) ? (list as Record[]) : []; +} + +/** + * Wait until a `command` request (`setBreakpoints` unless named) carries exactly + * `want` under `field`, and return what it carried. + * + * Reading the LAST request the wire holds asks what is there RIGHT NOW, which + * is the final state only if nothing further is in flight. One gesture is + * routinely several requests - the workbench syncs a breakpoint as a remove + * then an add - so the last entry at an arbitrary moment can be an + * INTERMEDIATE one carrying the state before the gesture completed, and which + * one a test observes is then decided by how fast the machine is. + */ +export async function waitForSentField( + recorder: DapRecorder, + field: string, + want: readonly (string | undefined)[], + why: string, + command = 'setBreakpoints', +): Promise<(string | undefined)[]> { + const read = (args: Record): (string | undefined)[] => + entriesOf(args).map((entry) => entry[field] as string | undefined); + const carried = await recorder.waitForRequestArgs( + command, + (args) => { + const got = read(args); + return got.length === want.length && want.every((value, at) => got[at] === value); + }, + why, + ); + return read(carried); +} + +/** {@link waitForSentField}, then assert the request carried exactly `want`. */ +export async function assertSentField( + recorder: DapRecorder, + field: string, + want: readonly (string | undefined)[], + why: string, + command = 'setBreakpoints', +): Promise { + assert.deepStrictEqual(await waitForSentField(recorder, field, want, why, command), want, why); +} + +/** Every response to each named DAP command succeeded, each row failing with its own reason. */ +export function assertEverySucceeded( + recorder: DapRecorder, + rows: readonly (readonly [command: string, why: string])[], +): void { + for (const [command, why] of rows) { + assert.ok( + recorder.responses(command).every((response) => response.success), + why, + ); + } +} diff --git a/src/editors/vscode/src/test/suite/debug-protocol-capabilities-e2e.test.ts b/src/editors/vscode/src/test/suite/debug-protocol-capabilities-e2e.test.ts index 787e008c..5653e3ef 100644 --- a/src/editors/vscode/src/test/suite/debug-protocol-capabilities-e2e.test.ts +++ b/src/editors/vscode/src/test/suite/debug-protocol-capabilities-e2e.test.ts @@ -34,6 +34,7 @@ import { import { DEBUG_TYPE_ID } from './run-debug-kit'; import { comparablePath, deepEq, eq, neq, requireAt } from './test-helpers'; import { DEBUG_SESSION_MS } from './test-timeouts'; +import { assertEverySucceeded } from './debug-inspect-kit'; /** The Phase Four column of [DEBUG-PROTOCOL-CAPABILITIES], "Yes" rows. */ const PHASE_FOUR_YES: readonly { flag: string; note: string }[] = [ @@ -257,22 +258,12 @@ suite('Debug protocol — the DAP 1.71.0 handshake and the capability table', () assertCleanSession(debuggee(), 'the DAP handshake'); // Interaction 5 - the handshake is a SEQUENCE, and every step of it was // answered. An unanswered step leaves the session half-configured. - assert.ok( - recorder.responses('initialize').every((response) => response.success), - 'initialize was answered successfully', - ); - assert.ok( - recorder.responses('launch').every((response) => response.success), - 'and launch', - ); - assert.ok( - recorder.responses('setBreakpoints').every((response) => response.success), - 'and every breakpoint sync', - ); - assert.ok( - recorder.responses('configurationDone').every((response) => response.success), - 'and configurationDone', - ); + assertEverySucceeded(recorder, [ + ['initialize', 'initialize was answered successfully'], + ['launch', 'and launch'], + ['setBreakpoints', 'and every breakpoint sync'], + ['configurationDone', 'and configurationDone'], + ]); deepEq(recorder.exits, [], 'with the adapter process alive throughout'); }, ); @@ -408,26 +399,13 @@ suite('Debug protocol — the DAP 1.71.0 handshake and the capability table', () assertCleanSession(debuggee(), 'the five panel requests'); // Interaction 4 - the five panel requests were each answered, which is what // makes the panels render at all. - assert.ok( - recorder.responses('threads').every((response) => response.success), - 'threads was answered', - ); - assert.ok( - recorder.responses('stackTrace').every((response) => response.success), - 'and stackTrace', - ); - assert.ok( - recorder.responses('scopes').every((response) => response.success), - 'and scopes', - ); - assert.ok( - recorder.responses('variables').every((response) => response.success), - 'and variables', - ); - assert.ok( - recorder.responses('evaluate').every((response) => response.success), - 'and evaluate', - ); + assertEverySucceeded(recorder, [ + ['threads', 'threads was answered'], + ['stackTrace', 'and stackTrace'], + ['scopes', 'and scopes'], + ['variables', 'and variables'], + ['evaluate', 'and evaluate'], + ]); }, ); diff --git a/src/editors/vscode/src/test/suite/debug-stepping-boundaries-e2e.test.ts b/src/editors/vscode/src/test/suite/debug-stepping-boundaries-e2e.test.ts index f9935292..d2fc17ac 100644 --- a/src/editors/vscode/src/test/suite/debug-stepping-boundaries-e2e.test.ts +++ b/src/editors/vscode/src/test/suite/debug-stepping-boundaries-e2e.test.ts @@ -27,7 +27,6 @@ import { stackFrames, stepToFrame, trace, - variableNamed, walk, } from './debug-drive-kit'; import { @@ -40,6 +39,7 @@ import { debugTest, } from './debug-suite-kit'; import { comparablePath, deepEq, eq, neq, requireAt } from './test-helpers'; +import { assertValues } from './debug-inspect-kit'; suite('Debug stepping — breakpoints inside steps, and stepping off the end', () => { const debuggee = useDebuggee('debug-stepedge-cs-', 'csharp'); @@ -218,8 +218,10 @@ suite('Debug stepping — breakpoints inside steps, and stepping off the end', ( 'and the caller sits directly beneath it', ); const locals = await localsOf(session, into.frame.id); - eq(variableNamed(locals, 'left').value, '2', 'called with the seed the loop is carrying'); - eq(variableNamed(locals, 'right').value, '1', 'and the first loop index'); + assertValues(locals, [ + ['left', '2', 'called with the seed the loop is carrying'], + ['right', '1', 'and the first loop index'], + ]); // Interaction 3 - Shift+F11 OUT. Back in the caller, exactly one frame // shallower, on or past the statement that made the call. diff --git a/src/editors/vscode/src/test/suite/debug-stepping-e2e.test.ts b/src/editors/vscode/src/test/suite/debug-stepping-e2e.test.ts index 17fd732c..9571f982 100644 --- a/src/editors/vscode/src/test/suite/debug-stepping-e2e.test.ts +++ b/src/editors/vscode/src/test/suite/debug-stepping-e2e.test.ts @@ -45,6 +45,7 @@ import { debugTest, } from './debug-suite-kit'; import { deepEq, eq, neq, requireAt } from './test-helpers'; +import { onlySourceBreakpoint, stepIntoAddThroughLoop } from './debug-inspect-kit'; /** The framework directory a Just-My-Code step must never surface. */ const FRAMEWORK_HINTS: readonly string[] = ['Microsoft.NETCore.App', 'System.Private.CoreLib']; @@ -67,12 +68,7 @@ suite('Debug stepping — F10 / F11 / Shift+F11 over a live session', () => { // Interaction 1 — arm one breakpoint on the call statement and open the file. armBreakpoints(fixture, 'main-accumulate'); const editor = await openFixture(fixture); - eq(vscode.debug.breakpoints.length, 1, 'exactly one breakpoint is armed'); - const armed = requireAt(vscode.debug.breakpoints, 0, 'the armed breakpoint'); - assert.ok( - armed instanceof vscode.SourceBreakpoint, - 'a line breakpoint is a SourceBreakpoint', - ); + const armed = onlySourceBreakpoint('the armed breakpoint'); assert.ok(armed.enabled, 'an armed breakpoint is enabled'); eq(armed.condition, undefined, 'an unconditional breakpoint carries no condition'); eq(armed.hitCondition, undefined, 'and no hit condition'); @@ -165,26 +161,13 @@ suite('Debug stepping — F10 / F11 / Shift+F11 over a live session', () => { 'the caller frame must still be parked on the call statement', ); - // Interaction 3 — F10 twice to reach the call inside the loop. - const toCall = await walk(recorder, [CMD_STEP_OVER, CMD_STEP_OVER]); - deepEq( - trace(toCall.frames), - [ - at(fixture, 'Accumulate', 'accumulate-loop'), - at(fixture, 'Accumulate', 'accumulate-call'), - ], - 'F10 inside a for-loop visits the loop header, then the body statement', - ); - - // Interaction 4 — F11 into Add: three user frames, innermost first. - const intoAdd = await stepToFrame(recorder, CMD_STEP_INTO); - assertStoppedAt(intoAdd.frame, fixture, 'add-body', 'Add', 'F11 into the innermost callee'); - const deep = await stackFrames(session, intoAdd.stop.threadId); - deepEq( - deep.slice(0, 3).map((frame) => methodOf(frame)), - ['Add', 'Accumulate', 'Main'], - '[DEBUG-FEATURES-STACK]: `stackTrace` reports physical frames innermost-first', - ); + // Interactions 3 and 4 — F10 twice to reach the call inside the loop, + // then F11 into Add: three user frames, innermost first. + const { deep } = await stepIntoAddThroughLoop(recorder, session, fixture, [ + 'Add', + 'Accumulate', + 'Main', + ]); deepEq( deep.slice(0, 3).map((frame) => frame.line), [ diff --git a/src/editors/vscode/src/test/suite/debug-suite-kit.ts b/src/editors/vscode/src/test/suite/debug-suite-kit.ts index 0747b715..3483cf4f 100644 --- a/src/editors/vscode/src/test/suite/debug-suite-kit.ts +++ b/src/editors/vscode/src/test/suite/debug-suite-kit.ts @@ -298,7 +298,6 @@ async function materialise(scratchDir: string, language: Language): Promise Debuggee { let fixture: DebugFixture; let scratchDir: string; - let current: Debuggee | undefined; suiteSetup(async function () { this.timeout(FIXTURE_BUILD_MS); @@ -310,36 +309,10 @@ export function useDebuggee(prefix: string, language: Language): () => Debuggee removeDirRecursive(scratchDir); }); - setup(() => { - clearAllBreakpoints(); - current = { - fixture, - folder: fakeFolder(requireWorkspaceRoot()), - recorder: new DapRecorder(), - stubs: installUiStubs(), - sessions: new DebugSessionRecorder(), - }; - }); - - teardown(async function () { - // Awaits stopAnyDebugSession's DEBUG_SESSION_MS poll; the ceiling must sit - // above it so the poll's own message wins ([DIST-CI-VSIX-SHARDS-TIMEOUTS]). - this.timeout(DEBUG_SESSION_MS + 5_000); - const active = current; - current = undefined; - if (active === undefined) return; - await stopDebuggee(); - clearAllBreakpoints(); - active.sessions.dispose(); - active.recorder.dispose(); - active.stubs.restore(); - await closeAllEditors(); - }); - - return () => { - assert.ok(current, 'the debuggee harness must be created in setup'); - return current; - }; + return useArmedRecorders( + () => ({ fixture, folder: fakeFolder(requireWorkspaceRoot()) }), + 'debuggee', + ); } /** @@ -459,3 +432,47 @@ export function assertCleanSession(debuggee: Debuggee, why: string): void { ); assert.deepStrictEqual(debuggee.stubs.log.infoMessages, [], `${why}: and shows no info toast`); } + +/** The recorders every debug suite arms fresh for each test. */ +export interface ArmedRecorders { + readonly recorder: DapRecorder; + readonly sessions: DebugSessionRecorder; + readonly stubs: UiStubs; +} + +/** + * Arm a fresh DapRecorder, DebugSessionRecorder and UI stubs around every + * test, beside what `extra` adds, and hand back an accessor for them. The + * teardown order is the contract: stop the debuggee BEFORE the recorders are + * disposed, or the stop's own traffic lands on a disposed listener and the next + * test starts with a live session still attached. + */ +export function useArmedRecorders( + extra: () => T, + what: string, +): () => T & ArmedRecorders { + let current: (T & ArmedRecorders) | undefined; + setup(() => { + clearAllBreakpoints(); + const armed = { recorder: new DapRecorder(), stubs: installUiStubs() }; + current = { ...extra(), ...armed, sessions: new DebugSessionRecorder() }; + }); + teardown(async function () { + // Awaits stopAnyDebugSession's DEBUG_SESSION_MS poll; the ceiling must sit + // above it so the poll's own message wins ([DIST-CI-VSIX-SHARDS-TIMEOUTS]). + this.timeout(DEBUG_SESSION_MS + 5_000); + const active = current; + current = undefined; + if (active === undefined) return; + await stopDebuggee(); + clearAllBreakpoints(); + active.sessions.dispose(); + active.recorder.dispose(); + active.stubs.restore(); + await closeAllEditors(); + }); + return () => { + assert.ok(current, `the ${what} harness must be created in setup`); + return current; + }; +} diff --git a/src/editors/vscode/src/test/suite/debug-test-debugging-e2e.test.ts b/src/editors/vscode/src/test/suite/debug-test-debugging-e2e.test.ts index b045bad4..5e5cd2e3 100644 --- a/src/editors/vscode/src/test/suite/debug-test-debugging-e2e.test.ts +++ b/src/editors/vscode/src/test/suite/debug-test-debugging-e2e.test.ts @@ -53,14 +53,15 @@ import { requireDebugSession, type TestDebugFixture, } from './debug-test-kit'; -import { debugRun, useDebugTestFixture, firstBreakpointStop } from './debug-test-harness'; -import { DEBUG_TYPE_ID, DebugSessionRecorder, fakeFolder } from './run-debug-kit'; import { - activateTestExplorer, - discoverSolution, - findItem, - profileOfKind, -} from './test-explorer-kit'; + debugRun, + useDebugTestFixture, + firstBreakpointStop, + debuggableRow, + stopInTestSource, +} from './debug-test-harness'; +import { DEBUG_TYPE_ID, DebugSessionRecorder, fakeFolder } from './run-debug-kit'; +import { activateTestExplorer, findItem, profileOfKind } from './test-explorer-kit'; import { comparablePath, deepEq, @@ -72,6 +73,12 @@ import { } from './test-helpers'; import { DEBUG_SESSION_MS, DEBUG_TEST_MS } from './test-timeouts'; import { type UiStubs } from './ui-stubs'; +import { + assertValues, + continueToEnd, + onlySourceBreakpoint, + assertSentField, +} from './debug-inspect-kit'; /** The `hitCondition` of every entry in one `setBreakpoints` request. */ function hitConditionsOf(args: Record): string[] { @@ -92,20 +99,6 @@ suite('Debug ONE test — the Test Explorer Debug profile and test breakpoints', ({ fixture, recorder, sessions, stubs } = harness()); }); - /** Discover the fixture and return the tree row for `fqn`. */ - async function rowFor(fqn: string): Promise { - const api = await activateTestExplorer(); - const discovered = await discoverSolution(api, fixture.solutionPath, CS_ALL); - assert.ok( - discovered.includes(fqn), - `${fqn} must be discovered before it can be debugged; found: ${discovered.join(', ')}`, - ); - const item = findItem(api.testController.items, fqn); - assert.ok(item, `the TestItem for ${fqn} must exist`); - eq(item.children.size, 0, `${fqn} is a test, so it is a LEAF the Debug button applies to`); - return item; - } - // Implements "Debug individual test" and "Breakpoints inside test methods". test('the Debug profile starts a session and stops inside the test body', async function () { this.timeout(DEBUG_TEST_MS); @@ -113,7 +106,7 @@ suite('Debug ONE test — the Test Explorer Debug profile and test breakpoints', // Interaction 1 — find the row the user is about to press the Debug button // on, and the profile that button maps to. const api = await activateTestExplorer(); - const item = await rowFor(CS_ADDS); + const item = await debuggableRow(fixture, CS_ALL, CS_ADDS); eq(item.label, 'Adds_Two_Numbers', 'a test row is labelled with its method name'); eq(item.id, CS_ADDS, 'and identified by the FQN the debug filter substitutes'); const profile = profileOfKind(api.testController, vscode.TestRunProfileKind.Debug); @@ -133,9 +126,7 @@ suite('Debug ONE test — the Test Explorer Debug profile and test breakpoints', // Interaction 2 — arm a breakpoint INSIDE the test method, then debug it. vscode.debug.addBreakpoints([breakpointAt(CS_SOURCE, fixture.sourceUri, 'adds-call')]); - eq(vscode.debug.breakpoints.length, 1, 'one breakpoint is armed inside the test body'); - const armed = vscode.debug.breakpoints[0]; - assert.ok(armed instanceof vscode.SourceBreakpoint, 'armed as a SOURCE breakpoint'); + const armed = onlySourceBreakpoint('one breakpoint is armed inside the test body'); eq( comparablePath(armed.location.uri.fsPath), comparablePath(fixture.sourceFile), @@ -152,18 +143,15 @@ suite('Debug ONE test — the Test Explorer Debug profile and test breakpoints', assertHandshakeOrder(recorder, 'debugging one test'); // Interaction 4 — the session stopped ON that breakpoint, in the TEST, with // the test's own state readable. - const stop = await firstBreakpointStop(recorder, CS_SOURCE, 'adds-call'); + const { stop, active, frame } = await stopInTestSource( + recorder, + fixture, + CS_SOURCE, + 'adds-call', + ); neq(stop.hitBreakpointIds.length, 0, 'the stop names the breakpoint it hit'); neq(stop.threadId, 0, 'and the thread the test is running on'); - const active = requireActive('a breakpoint stop'); - const frame = await topFrame(active, stop.threadId); eq(methodOf(frame), 'Adds_Two_Numbers', `stopped in '${frame.name}', not in the test method`); - eq(frame.line, CS_SOURCE.dapLine('adds-call'), 'on the armed line, not on the method entry'); - eq( - comparablePath(frame.sourcePath), - comparablePath(fixture.sourceFile), - 'and in the user’s OWN file — a frame with no source is a debugger with no symbols', - ); eq( variableNamed(await localsOf(active, frame.id), 'seed').value, '20', @@ -177,10 +165,8 @@ suite('Debug ONE test — the Test Explorer Debug profile and test breakpoints', // Interaction 5 — continuing runs the test to the end and ends the session, // rather than leaving the test host wedged on a breakpoint forever. - await gesture(CMD_CONTINUE); - await recorder.waitForEvents('terminated', 1); + await continueToEnd(recorder, 'and no adapter transport error'); deepEq(stubs.log.errorMessages, [], 'a working test debug run reports no error'); - deepEq(recorder.errors, [], 'and no adapter transport error'); }); // Implements [DEBUG-FEATURES-TESTS]'s closing rule: attach to the test HOST. @@ -189,7 +175,7 @@ suite('Debug ONE test — the Test Explorer Debug profile and test breakpoints', // Interaction 1 — arm a breakpoint one frame deeper, in the helper the test // calls, so the whole stack can be inspected, and debug the single test. - const item = await rowFor(CS_ADDS); + const item = await debuggableRow(fixture, CS_ALL, CS_ADDS); vscode.debug.addBreakpoints([breakpointAt(CS_SOURCE, fixture.sourceUri, 'add-body')]); eq(vscode.debug.breakpoints.length, 1, 'exactly one breakpoint is armed, in the helper'); await debugRun([item]); @@ -236,8 +222,10 @@ suite('Debug ONE test — the Test Explorer Debug profile and test breakpoints', const helperFrame = requireAt(frames, 0, 'the helper frame'); eq(helperFrame.line, CS_SOURCE.dapLine('add-body'), 'the helper stopped on the armed line'); const helperLocals = await localsOf(active, helperFrame.id); - eq(variableNamed(helperLocals, 'left').value, '20', 'carrying the values the test passed'); - eq(variableNamed(helperLocals, 'right').value, '22', 'both of them, not just the first'); + assertValues(helperLocals, [ + ['left', '20', 'carrying the values the test passed'], + ['right', '22', 'both of them, not just the first'], + ]); // Interaction 4 — stepping OUT lands back in the test, in the user's own // code: that landing is what "Just My Code in test context" buys. @@ -265,7 +253,7 @@ suite('Debug ONE test — the Test Explorer Debug profile and test breakpoints', // Interaction 1 — the commonest accident: the user presses Debug having // forgotten to arm anything. That must still be a debug SESSION, not a // silent no-op, and not a hang. - const item = await rowFor(CS_ADDS); + const item = await debuggableRow(fixture, CS_ALL, CS_ADDS); deepEq(vscode.debug.breakpoints, [], 'nothing is armed anywhere in the workbench'); await debugRun([item]); const session = assertOneTestSession(sessions, 'debugging with nothing armed'); @@ -291,7 +279,7 @@ suite('Debug ONE test — the Test Explorer Debug profile and test breakpoints', // Interaction 1 — a red test is the one a user actually debugs. Arm the // line before the failing assertion. - const item = await rowFor(CS_FAILS); + const item = await debuggableRow(fixture, CS_ALL, CS_FAILS); eq(item.label, 'Fails_On_Purpose', 'the red test is the row being debugged'); vscode.debug.addBreakpoints([breakpointAt(CS_SOURCE, fixture.sourceUri, 'fails-seed')]); await debugRun([item]); @@ -312,9 +300,7 @@ suite('Debug ONE test — the Test Explorer Debug profile and test breakpoints', // Interaction 3 — continuing lets xUnit's assertion throw and the session // end. A failing test must not leave the adapter in an error state, or the // NEXT debug press starts from a poisoned host. - await gesture(CMD_CONTINUE); - await recorder.waitForEvents('terminated', 1, DEBUG_SESSION_MS); - deepEq(recorder.errors, [], 'an assertion failure is not an adapter transport error'); + await continueToEnd(recorder, 'an assertion failure is not an adapter transport error'); deepEq(stubs.log.errorMessages, [], 'nor a SharpLsp error the user has to read'); eq( recorder.stops().length, @@ -329,7 +315,7 @@ suite('Debug ONE test — the Test Explorer Debug profile and test breakpoints', // Interaction 1 — a `[Fact(Skip=…)]` row is still a row the user can press // Debug on. Arm its body. - const item = await rowFor(CS_SKIPPED); + const item = await debuggableRow(fixture, CS_ALL, CS_SKIPPED); eq(item.label, 'Skipped_Test', 'the skipped test is a row like any other'); vscode.debug.addBreakpoints([breakpointAt(CS_SOURCE, fixture.sourceUri, 'skipped-body')]); eq(vscode.debug.breakpoints.length, 1, 'armed inside a body that will never run'); @@ -358,7 +344,7 @@ suite('Debug ONE test — the Test Explorer Debug profile and test breakpoints', // Interaction 1 — a theory is ONE row in the tree ([TEST-DISCOVERY-FQN]: // "no row data") but TWO executions of the same body. - const item = await rowFor(CS_ROWS); + const item = await debuggableRow(fixture, CS_ALL, CS_ROWS); eq(item.id, CS_ROWS, 'the theory is addressed by one fully-qualified name'); assert.ok(!item.id.includes('('), 'carrying no row data, so no filter metacharacter'); vscode.debug.addBreakpoints([breakpointAt(CS_SOURCE, fixture.sourceUri, 'rows-body')]); @@ -397,7 +383,7 @@ suite('Debug ONE test — the Test Explorer Debug profile and test breakpoints', // Interaction 1 — the user unticks the breakpoint in the Breakpoints view // rather than deleting it. It must reach the adapter as disabled, or not at // all — never as a live breakpoint. - const item = await rowFor(CS_ADDS); + const item = await debuggableRow(fixture, CS_ALL, CS_ADDS); vscode.debug.addBreakpoints([disabledBreakpointAt(CS_SOURCE, fixture.sourceUri, 'adds-call')]); eq(vscode.debug.breakpoints.length, 1, 'the breakpoint is still in the workbench'); eq( @@ -427,7 +413,7 @@ suite('Debug ONE test — the Test Explorer Debug profile and test breakpoints', // Interaction 1 — the theory runs its body twice; the condition picks the // second row. This is the only way a user debugs "the row that fails". - const item = await rowFor(CS_ROWS); + const item = await debuggableRow(fixture, CS_ALL, CS_ROWS); vscode.debug.addBreakpoints([ conditionalBreakpointAt(CS_SOURCE, fixture.sourceUri, 'rows-body', 'expected == 30'), ]); @@ -442,24 +428,11 @@ suite('Debug ONE test — the Test Explorer Debug profile and test breakpoints', // the session starts asks what the wire holds right now, and the sync // carrying the condition may not have landed yet. A machine fast enough to // have sent it passes; a slower one reads an earlier request, or none. - const armed = await recorder.waitForRequestArgs( - 'setBreakpoints', - (args) => { - const list: unknown = args['breakpoints']; - return ( - Array.isArray(list) && - list.length === 1 && - String((list[0] as Record)['condition']) === 'expected == 30' - ); - }, - 'the condition the user typed must reach the adapter unaltered', - ); - const sent = armed['breakpoints']; - assert.ok(Array.isArray(sent), '`setBreakpoints` must carry a breakpoints array'); - deepEq( - (sent as Record[]).map((entry) => entry['condition']), + await assertSentField( + recorder, + 'condition', ['expected == 30'], - 'the condition the user typed is sent to the adapter unaltered', + 'the condition the user typed must reach the adapter unaltered', ); // Interaction 3 — exactly ONE row stops, and it is the row the condition @@ -470,12 +443,14 @@ suite('Debug ONE test — the Test Explorer Debug profile and test breakpoints', const frame = await topFrame(active, stop.threadId); const locals = await localsOf(active, frame.id); eq(methodOf(frame), 'Adds_Rows', 'stopped in the theory body'); - eq( - variableNamed(locals, 'expected').value, - '30', - 'on the row the condition selected, not on the first row that reached the line', - ); - eq(variableNamed(locals, 'left').value, '10', 'carrying that row’s own arguments'); + assertValues(locals, [ + [ + 'expected', + '30', + 'on the row the condition selected, not on the first row that reached the line', + ], + ['left', '10', 'carrying that row’s own arguments'], + ]); await gesture(CMD_CONTINUE); await recorder.waitForEvents('terminated', 1, DEBUG_SESSION_MS); eq( @@ -490,7 +465,7 @@ suite('Debug ONE test — the Test Explorer Debug profile and test breakpoints', // Interaction 1 — record what the Testing view holds before the gesture. const api = await activateTestExplorer(); - const item = await rowFor(CS_ADDS); + const item = await debuggableRow(fixture, CS_ALL, CS_ADDS); const before = api.testController.getResult(CS_ADDS); const cacheSize = api.testController.cachedResults.size; const treeBefore = api.testController.items.size; @@ -528,7 +503,7 @@ suite('Debug ONE test — the Test Explorer Debug profile and test breakpoints', this.timeout(DEBUG_TEST_MS); // Interaction 1 — stop on the first statement of the test body. - const item = await rowFor(CS_ADDS); + const item = await debuggableRow(fixture, CS_ALL, CS_ADDS); vscode.debug.addBreakpoints([breakpointAt(CS_SOURCE, fixture.sourceUri, 'adds-seed')]); eq(vscode.debug.breakpoints.length, 1, 'one breakpoint, on the test body first line'); await debugRun([item]); @@ -611,7 +586,7 @@ suite('Debug ONE test — the Test Explorer Debug profile and test breakpoints', this.timeout(DEBUG_TEST_MS); // Interaction 1 — stop deep: inside the helper, called from the test. - const item = await rowFor(CS_ADDS); + const item = await debuggableRow(fixture, CS_ALL, CS_ADDS); vscode.debug.addBreakpoints([breakpointAt(CS_SOURCE, fixture.sourceUri, 'add-body')]); await debugRun([item]); assertOneTestSession(sessions, 'inspecting a test call stack'); @@ -665,7 +640,7 @@ suite('Debug ONE test — the Test Explorer Debug profile and test breakpoints', // Interaction 1 — stop on the theory body, where the frame carries the row // ARGUMENTS as well as the locals. - const item = await rowFor(CS_ROWS); + const item = await debuggableRow(fixture, CS_ALL, CS_ROWS); vscode.debug.addBreakpoints([breakpointAt(CS_SOURCE, fixture.sourceUri, 'rows-assert')]); await debugRun([item]); assertOneTestSession(sessions, 'inspecting theory row variables'); @@ -740,13 +715,11 @@ suite('Debug ONE test — the Test Explorer Debug profile and test breakpoints', this.timeout(DEBUG_TEST_MS); // Interaction 1 — arm the theory body to stop only on its second hit. - const item = await rowFor(CS_ROWS); + const item = await debuggableRow(fixture, CS_ALL, CS_ROWS); vscode.debug.addBreakpoints([ hitCountBreakpointAt(CS_SOURCE, fixture.sourceUri, 'rows-body', '2'), ]); - eq(vscode.debug.breakpoints.length, 1, 'one breakpoint is armed'); - const armed = requireAt(vscode.debug.breakpoints, 0, 'the hit-count breakpoint'); - assert.ok(armed instanceof vscode.SourceBreakpoint, 'armed as a source breakpoint'); + const armed = onlySourceBreakpoint('the hit-count breakpoint'); eq(armed.hitCondition, '2', 'carrying the hit condition the user typed'); assert.ok(armed.enabled, 'and enabled'); eq(armed.condition, undefined, 'a hit count is not an expression condition'); @@ -826,7 +799,7 @@ suite('Debug ONE test — the Test Explorer Debug profile and test breakpoints', // Interaction 1 — arm two breakpoints in two different methods, so more // than one must bind before the gesture may settle. - const item = await rowFor(CS_ADDS); + const item = await debuggableRow(fixture, CS_ALL, CS_ADDS); vscode.debug.addBreakpoints([ breakpointAt(CS_SOURCE, fixture.sourceUri, 'adds-seed'), breakpointAt(CS_SOURCE, fixture.sourceUri, 'add-body'), @@ -884,7 +857,7 @@ suite('Debug ONE test — the Test Explorer Debug profile and test breakpoints', this.timeout(DEBUG_TEST_MS); // Interaction 1 — the first session, armed and stopped. - const item = await rowFor(CS_ADDS); + const item = await debuggableRow(fixture, CS_ALL, CS_ADDS); vscode.debug.addBreakpoints([breakpointAt(CS_SOURCE, fixture.sourceUri, 'adds-call')]); await debugRun([item]); assertOneTestSession(sessions, 'the first debug of a test'); diff --git a/src/editors/vscode/src/test/suite/debug-test-decorated-e2e.test.ts b/src/editors/vscode/src/test/suite/debug-test-decorated-e2e.test.ts index 1ee86dd7..1ebd73e4 100644 --- a/src/editors/vscode/src/test/suite/debug-test-decorated-e2e.test.ts +++ b/src/editors/vscode/src/test/suite/debug-test-decorated-e2e.test.ts @@ -11,7 +11,7 @@ // This suite presses Debug on that adapter and requires the stop, the test's // own frame, and a clean finish. Implements [DEBUG-FEATURES-TESTS]. import * as vscode from 'vscode'; -import { CMD_CONTINUE, gesture, methodOf, topFrame } from './debug-drive-kit'; +import { methodOf, topFrame } from './debug-drive-kit'; import { CS_ADDS, CS_ALL, @@ -27,7 +27,8 @@ import { useDebugTestFixture, } from './debug-test-harness'; import { comparablePath, deepEq, eq } from './test-helpers'; -import { DEBUG_SESSION_MS, DEBUG_TEST_MS } from './test-timeouts'; +import { DEBUG_TEST_MS } from './test-timeouts'; +import { continueToEnd } from './debug-inspect-kit'; suite('Debug Test — an adapter that decorates test names still stops on the breakpoint', () => { const harness = useDebugTestFixture('debug-decorated-', 'csharp', 'vstest-decorating'); @@ -63,10 +64,8 @@ suite('Debug Test — an adapter that decorates test names still stops on the br // Interaction 4 — continuing runs the test to the end and the session ends // on its own, with nothing reported to the user as a failure. - await gesture(CMD_CONTINUE); - await recorder.waitForEvents('terminated', 1, DEBUG_SESSION_MS); + await continueToEnd(recorder, 'no adapter transport error'); eq(recorder.stops().length, 1, 'one stop, the armed one: nothing else halted the host'); - deepEq(recorder.errors, [], 'no adapter transport error'); deepEq(stubs.log.errorMessages, [], 'and no SharpLsp error the user has to read'); }); }); diff --git a/src/editors/vscode/src/test/suite/debug-test-fsharp-e2e.test.ts b/src/editors/vscode/src/test/suite/debug-test-fsharp-e2e.test.ts index e5c309cf..99007c2d 100644 --- a/src/editors/vscode/src/test/suite/debug-test-fsharp-e2e.test.ts +++ b/src/editors/vscode/src/test/suite/debug-test-fsharp-e2e.test.ts @@ -45,6 +45,8 @@ import { useDebugTestFixture, firstBreakpointStop, caretInSource, + debuggableRow, + stopInTestSource, } from './debug-test-harness'; import { DebugSessionRecorder } from './run-debug-kit'; import { activateTestExplorer, discoverSolution, findItem } from './test-explorer-kit'; @@ -63,19 +65,6 @@ suite('Debug an F# test — backtick names, modules and the at-cursor gesture', ({ fixture, recorder, sessions, stubs } = harness()); }); - /** Discover the F# fixture and return the tree row for `fqn`. */ - async function rowFor(fqn: string): Promise { - const api = await activateTestExplorer(); - const discovered = await discoverSolution(api, fixture.solutionPath, FS_ALL); - assert.ok( - discovered.includes(fqn), - `${fqn} must be discovered before it can be debugged; found: ${discovered.join(', ')}`, - ); - const item = findItem(api.testController.items, fqn); - assert.ok(item, `the TestItem for ${fqn} must exist`); - return item; - } - test('an F# backtick test whose FQN contains SPACES debugs and breaks in its body', async function () { this.timeout(DEBUG_TEST_MS); @@ -83,7 +72,7 @@ suite('Debug an F# test — backtick names, modules and the at-cursor gesture', // grammar, so it must be substituted verbatim ([TEST-FILTER-ESCAPE]); an // escaped or truncated name selects no test and the Debug press ends in // silence. - const item = await rowFor(FS_SPACED); + const item = await debuggableRow(fixture, FS_ALL, FS_SPACED); assert.ok(FS_SPACED.includes(' '), 'the fixture name really does contain spaces'); eq(item.id, FS_SPACED, 'and the tree carries it verbatim as the id'); eq(item.label, 'adds two numbers with spaces', 'labelled with the backtick binding'); @@ -98,16 +87,8 @@ suite('Debug an F# test — backtick names, modules and the at-cursor gesture', // Interaction 3 — it stops IN the F# source, with F# locals readable. The // F# compiler's PDB gaps ([DEBUG-FSHARP-PDB]) are about state machines, not // about plain `let` bindings: these must be inspectable. - const stop = await firstBreakpointStop(recorder, FS_SOURCE, 'fs-call'); + const { stop, active, frame } = await stopInTestSource(recorder, fixture, FS_SOURCE, 'fs-call'); neq(stop.hitBreakpointIds.length, 0, 'naming the breakpoint it hit'); - const active = requireActive('an F# breakpoint stop'); - const frame = await topFrame(active, stop.threadId); - eq(frame.line, FS_SOURCE.dapLine('fs-call'), 'on the armed line of the .fs file'); - eq( - comparablePath(frame.sourcePath), - comparablePath(fixture.sourceFile), - 'attributed to the F# source the user wrote, not to a generated file', - ); eq( variableNamed(await localsOf(active, frame.id), 'seed').value, '20', @@ -148,7 +129,7 @@ suite('Debug an F# test — backtick names, modules and the at-cursor gesture', // Interaction 1 — arm the private module-level helper, one frame deeper // than the test. - const item = await rowFor(FS_SPACED); + const item = await debuggableRow(fixture, FS_ALL, FS_SPACED); vscode.debug.addBreakpoints([breakpointAt(FS_SOURCE, fixture.sourceUri, 'fs-add-body')]); await debugRun([item]); assertOneTestSession(sessions, 'debugging into an F# helper'); @@ -228,7 +209,7 @@ suite('Debug an F# test — backtick names, modules and the at-cursor gesture', this.timeout(DEBUG_TEST_MS); // Interaction 1 — the F# theory is ONE row in the tree, under one name. - const item = await rowFor(FS_ROWS); + const item = await debuggableRow(fixture, FS_ALL, FS_ROWS); eq(item.id, FS_ROWS, 'one fully-qualified name for both rows'); assert.ok(item.id.startsWith(`${FS_MODULE}.`), 'qualified by the F# MODULE, not by a class'); assert.ok(!item.id.includes('('), 'and carrying no row data into the filter grammar'); @@ -266,7 +247,7 @@ suite('Debug an F# test — backtick names, modules and the at-cursor gesture', eq(sessions.ours.length, 1, 'both rows ran in the ONE session the selection started'); // Interaction 4 - a theory is ONE test in the tree however many rows it // runs, and one session however many times it stops ([TEST-RUN-TRX]). - const theoryRow = await rowFor(FS_ROWS); + const theoryRow = await debuggableRow(fixture, FS_ALL, FS_ROWS); eq(theoryRow.id, FS_ROWS, 'the theory is addressed by the single name its rows share'); eq(theoryRow.children.size, 0, 'and is a LEAF - one row per [] is two tests'); eq(recorder.stops().length, 2, 'two rows, two stops, and no third'); @@ -291,7 +272,7 @@ suite('Debug an F# test — backtick names, modules and the at-cursor gesture', // Testing view. It must reach the same debugger the Testing view does — // a command that resolves nothing is exactly how "Debug Test does nothing" // presents (issue #233). - await rowFor(FS_SPACED); + await debuggableRow(fixture, FS_ALL, FS_SPACED); await caretInSource(fixture, FS_SOURCE, 'fs-call', 'fsharp'); // Interaction 2 — arm a breakpoint and fire the at-cursor command. @@ -349,7 +330,7 @@ suite('Debug an F# test — backtick names, modules and the at-cursor gesture', // // Interaction 1 — reach the module row through a leaf, and check it holds // every binding the fixture declares. - const leaf = await rowFor(FS_SPACED); + const leaf = await debuggableRow(fixture, FS_ALL, FS_SPACED); const moduleRow = leaf.parent; assert.ok(moduleRow, 'an F# binding hangs off the module it is declared in'); eq(moduleRow.label, FS_MODULE_TYPE, 'and that group is the module, by its TYPE name'); @@ -419,7 +400,7 @@ suite('Debug an F# test — backtick names, modules and the at-cursor gesture', // // Interaction 1 — select every binding the fixture exposes. const rows = [] as vscode.TestItem[]; - for (const fqn of FS_ALL) rows.push(await rowFor(fqn)); + for (const fqn of FS_ALL) rows.push(await debuggableRow(fixture, FS_ALL, fqn)); eq(rows.length, FS_ALL.length, 'every F# binding resolved to a row'); deepEq( rows.map((row) => row.id), @@ -498,7 +479,7 @@ suite('Debug an F# test — backtick names, modules and the at-cursor gesture', // Interaction 2 — debug one binding with nothing armed, so the run goes // straight through to termination. - const row = await rowFor(FS_SPACED); + const row = await debuggableRow(fixture, FS_ALL, FS_SPACED); eq(vscode.debug.breakpoints.length, 0, 'the user has armed nothing'); await debugRun([row]); assertOneTestSession(sessions, 'debugging an F# binding with nothing armed'); diff --git a/src/editors/vscode/src/test/suite/debug-test-groups-e2e.test.ts b/src/editors/vscode/src/test/suite/debug-test-groups-e2e.test.ts index 4b890248..37fe18cf 100644 --- a/src/editors/vscode/src/test/suite/debug-test-groups-e2e.test.ts +++ b/src/editors/vscode/src/test/suite/debug-test-groups-e2e.test.ts @@ -41,6 +41,7 @@ import { import { deepEq, eq, neq, requireAt, assertContainsAll } from './test-helpers'; import { DEBUG_SESSION_MS, DEBUG_TEST_MS } from './test-timeouts'; import { type UiStubs } from './ui-stubs'; +import { continueToEnd } from './debug-inspect-kit'; /** How many tests the fixture declares in its first class. */ const MATH_CLASS_TESTS = 5; @@ -186,15 +187,13 @@ suite('Debug a SELECTION — class, namespace, assembly and multi-select', () => // …and the OTHER namespace never runs. [TEST-RUN-TRX] makes a run one // invocation for THE SELECTION; a debug that widened to the whole assembly // stops here too and would look identical from the first stop alone. - await gesture(CMD_CONTINUE); - await recorder.waitForEvents('terminated', 1, DEBUG_SESSION_MS); + await continueToEnd(recorder, 'and no adapter transport error'); eq( recorder.stops().length, 1, `debugging ${CS_MATH_NAMESPACE} must not execute a test in ${CS_TEXT_NAMESPACE}: the ` + 'control breakpoint there BOUND, so a second stop is proof the selection widened', ); - deepEq(recorder.errors, [], 'and no adapter transport error'); // Interaction 4 - the namespace row is a group under the assembly, and the // OTHER namespace must be untouched in the tree as well as at runtime. eq(sessions.ours.length, 1, 'one namespace, one session'); @@ -337,14 +336,12 @@ suite('Debug a SELECTION — class, namespace, assembly and multi-select', () => ['Adds_Two_Numbers', 'Joins_Two_Words'], 'both selected tests break, one stop each', ); - await gesture(CMD_CONTINUE); - await recorder.waitForEvents('terminated', 1, DEBUG_SESSION_MS); + await continueToEnd(recorder, 'with no adapter transport error'); eq( recorder.stops().length, 2, 'and exactly two stops in total: the unselected test must never have executed', ); - deepEq(recorder.errors, [], 'with no adapter transport error'); // Interaction 4 - a multi-select is still ONE invocation, and the rows the // user did NOT select must be untouched in the tree. eq(sessions.ours.length, 1, 'two selected classes are ONE session, not two'); @@ -513,15 +510,13 @@ suite('Debug a SELECTION — class, namespace, assembly and multi-select', () => // Interaction 3 — running on, the session ends with no further stop, even // though four other tests of the class ran to completion inside it. - await gesture(CMD_CONTINUE); - await recorder.waitForEvents('terminated', 1, DEBUG_SESSION_MS); + await continueToEnd(recorder, 'with no adapter transport error'); eq( recorder.stops().length, 1, 'the other four tests of the class execute but carry no breakpoint, so they must not stop', ); eq(sessions.ours.length, 1, 'and a class is ONE session throughout'); - deepEq(recorder.errors, [], 'with no adapter transport error'); deepEq(stubs.log.errorMessages, [], 'and nothing reported to the user as a failure'); // Interaction 4 - the class row itself is unchanged, and the four unarmed // tests really did execute inside the one session. @@ -577,10 +572,8 @@ suite('Debug a SELECTION — class, namespace, assembly and multi-select', () => 'Adds_Two_Numbers', 'in the test carrying the ENABLED breakpoint', ); - await gesture(CMD_CONTINUE); - await recorder.waitForEvents('terminated', 1, DEBUG_SESSION_MS); + await continueToEnd(recorder, 'and no adapter transport error'); eq(recorder.stops().length, 1, 'the disabled line must never produce a stop'); - deepEq(recorder.errors, [], 'and no adapter transport error'); // Interaction 4 - the Breakpoints view still holds BOTH, exactly as the // user left them. A debugger that silently deletes a breakpoint it declined // to honour is worse than one that ignores it. diff --git a/src/editors/vscode/src/test/suite/debug-test-harness.ts b/src/editors/vscode/src/test/suite/debug-test-harness.ts index 52e8398f..91a56fda 100644 --- a/src/editors/vscode/src/test/suite/debug-test-harness.ts +++ b/src/editors/vscode/src/test/suite/debug-test-harness.ts @@ -19,10 +19,11 @@ import * as assert from 'node:assert/strict'; import * as vscode from 'vscode'; import type { AnchoredSource } from './debug-anchors'; import { DapRecorder, type StopRecord } from './debug-dap-kit'; -import { assertStopReason } from './debug-drive-kit'; -import { assertBoundAtLines, clearAllBreakpoints, stopDebuggee } from './debug-suite-kit'; +import { assertStopReason, topFrame, type Frame } from './debug-drive-kit'; +import { assertBoundAtLines, useArmedRecorders } from './debug-suite-kit'; import { disposeDebugTestFixture, + requireActive, writeDebugTestFixture, type FixtureLanguage, type FixtureRunner, @@ -35,9 +36,9 @@ import { findItem, runViaProfile, } from './test-explorer-kit'; -import { closeAllEditors, comparablePath, eq, requireAt } from './test-helpers'; +import { comparablePath, eq, requireAt } from './test-helpers'; import { FIXTURE_BUILD_MS } from './test-timeouts'; -import { installUiStubs, type UiStubs } from './ui-stubs'; +import { type UiStubs } from './ui-stubs'; /** The suite's built fixture, plus the recorders armed for the current test. */ export interface DebugTestHarness { @@ -59,7 +60,6 @@ export function useDebugTestFixture( runner: FixtureRunner = 'vstest', ): () => DebugTestHarness { let fixture: TestDebugFixture; - let current: DebugTestHarness | undefined; suiteSetup(async function () { this.timeout(FIXTURE_BUILD_MS); @@ -71,32 +71,7 @@ export function useDebugTestFixture( await disposeDebugTestFixture(fixture); }); - setup(() => { - clearAllBreakpoints(); - current = { - fixture, - recorder: new DapRecorder(), - sessions: new DebugSessionRecorder(), - stubs: installUiStubs(), - }; - }); - - teardown(async () => { - const active = current; - current = undefined; - if (active === undefined) return; - await stopDebuggee(); - clearAllBreakpoints(); - active.sessions.dispose(); - active.recorder.dispose(); - active.stubs.restore(); - await closeAllEditors(); - }); - - return () => { - assert.ok(current, 'the debug-test harness must be created in setup'); - return current; - }; + return useArmedRecorders(() => ({ fixture }), 'debug-test'); } /** @@ -161,3 +136,26 @@ export async function caretInSource( eq(document.languageId, languageId, `and the editor knows it is ${languageId}`); eq(comparablePath(document.uri.fsPath), comparablePath(fixture.sourceFile), 'in the fixture'); } + +/** + * The first stop of a test debug run, read back through the ACTIVE session: + * {@link firstBreakpointStop}, and then a top frame ON `anchor`'s line of the + * fixture's own source — a frame with no source is a debugger with no symbols. + */ +export async function stopInTestSource( + recorder: DapRecorder, + fixture: { readonly sourceFile: string }, + source: AnchoredSource, + anchor: string, +): Promise<{ stop: StopRecord; active: vscode.DebugSession; frame: Frame }> { + const stop = await firstBreakpointStop(recorder, source, anchor); + const active = requireActive(`the stop at '${anchor}'`); + const frame = await topFrame(active, stop.threadId); + eq(frame.line, source.dapLine(anchor), `the top frame is on the line armed at '${anchor}'`); + eq( + comparablePath(frame.sourcePath), + comparablePath(fixture.sourceFile), + `and in the fixture's own source file, not a generated or decompiled one`, + ); + return { stop, active, frame }; +} diff --git a/src/editors/vscode/src/test/suite/debug-test-mtp-e2e.test.ts b/src/editors/vscode/src/test/suite/debug-test-mtp-e2e.test.ts index 3c192fe1..ae9ca277 100644 --- a/src/editors/vscode/src/test/suite/debug-test-mtp-e2e.test.ts +++ b/src/editors/vscode/src/test/suite/debug-test-mtp-e2e.test.ts @@ -14,14 +14,7 @@ import * as assert from 'node:assert/strict'; import * as vscode from 'vscode'; import { DapRecorder } from './debug-dap-kit'; -import { - CMD_CONTINUE, - gesture, - localsOf, - methodOf, - topFrame, - variableNamed, -} from './debug-drive-kit'; +import { localsOf, methodOf, variableNamed } from './debug-drive-kit'; import { CS_ADDS, CS_ALL, @@ -30,22 +23,22 @@ import { assertHandshakeOrder, assertOneTestSession, breakpointAt, - requireActive, type TestDebugFixture, } from './debug-test-kit'; -import { debugRun, useDebugTestFixture, firstBreakpointStop } from './debug-test-harness'; +import { + debugRun, + useDebugTestFixture, + debuggableRow, + stopInTestSource, +} from './debug-test-harness'; import { TEST_HOST_DEBUG_ENV } from '../../test-debug.js'; import { DEBUG_TYPE_ID, DebugSessionRecorder } from './run-debug-kit'; -import { - activateTestExplorer, - discoverSolution, - findItem, - runViaProfile, -} from './test-explorer-kit'; +import { activateTestExplorer, runViaProfile } from './test-explorer-kit'; import { assertFailed, cachedFor } from './test-explorer-outcome-assertions'; -import { comparablePath, deepEq, eq, neq } from './test-helpers'; +import { deepEq, eq, neq } from './test-helpers'; import { DEBUG_TEST_MS } from './test-timeouts'; import { type UiStubs } from './ui-stubs'; +import { continueToEnd } from './debug-inspect-kit'; suite( 'Debug an MTP test — the Test Explorer Debug profile on a Microsoft.Testing.Platform module', @@ -60,21 +53,11 @@ suite( ({ fixture, recorder, sessions, stubs } = harness()); }); - /** Discover the MTP fixture and return the tree row for `fqn`. */ - async function rowFor(fqn: string): Promise { - const api = await activateTestExplorer(); - const discovered = await discoverSolution(api, fixture.solutionPath, CS_ALL); - assert.ok(discovered.includes(fqn), `${fqn} must be discovered: ${discovered.join(', ')}`); - const item = findItem(api.testController.items, fqn); - assert.ok(item, `the TestItem for ${fqn} must exist`); - return item; - } - test('the Debug profile attaches to the waiting MODULE and stops inside the test body', async function () { this.timeout(DEBUG_TEST_MS); // 1. Arm a breakpoint inside the test method and press Debug on its row. - const item = await rowFor(CS_ADDS); + const item = await debuggableRow(fixture, CS_ALL, CS_ADDS); const api = await activateTestExplorer(); const cachedBefore = api.testController.getResult(CS_ADDS); vscode.debug.addBreakpoints([breakpointAt(CS_SOURCE, fixture.sourceUri, 'adds-call')]); @@ -98,16 +81,8 @@ suite( // 3. The breakpoint BOUND and the module stopped on it, in the test, with // the test's own state readable. - const stop = await firstBreakpointStop(recorder, CS_SOURCE, 'adds-call'); - const active = requireActive('a breakpoint stop in an MTP module'); - const frame = await topFrame(active, stop.threadId); + const { active, frame } = await stopInTestSource(recorder, fixture, CS_SOURCE, 'adds-call'); eq(methodOf(frame), 'Adds_Two_Numbers', `stopped in '${frame.name}', not in the test`); - eq(frame.line, CS_SOURCE.dapLine('adds-call'), 'on the armed line'); - eq( - comparablePath(frame.sourcePath), - comparablePath(fixture.sourceFile), - 'in the user’s file', - ); eq( variableNamed(await localsOf(active, frame.id), 'seed').value, '20', @@ -115,10 +90,8 @@ suite( ); // 4. Continue: the module runs the test to the end and the session ends. - await gesture(CMD_CONTINUE); - await recorder.waitForEvents('terminated', 1); + await continueToEnd(recorder, 'and no adapter transport error'); deepEq(stubs.log.errorMessages, [], 'a working MTP debug run reports no error'); - deepEq(recorder.errors, [], 'and no adapter transport error'); // 5. The user's breakpoint was the first stop AND the only one: no wait loop // or runner frame stopped the module on its own. @@ -132,7 +105,7 @@ suite( test('an MTP debug run writes no result, and ▶ afterwards still reports the real one', async function () { this.timeout(DEBUG_TEST_MS); const api = await activateTestExplorer(); - const item = await rowFor(CS_FAILS); + const item = await debuggableRow(fixture, CS_ALL, CS_FAILS); const before = api.testController.getResult(CS_FAILS); // 1. Debug a FAILING test with nothing armed: a session starts, the module diff --git a/src/editors/vscode/src/test/suite/debug-test-mtp-fsharp-e2e.test.ts b/src/editors/vscode/src/test/suite/debug-test-mtp-fsharp-e2e.test.ts index 79bee21c..9596c832 100644 --- a/src/editors/vscode/src/test/suite/debug-test-mtp-fsharp-e2e.test.ts +++ b/src/editors/vscode/src/test/suite/debug-test-mtp-fsharp-e2e.test.ts @@ -43,18 +43,16 @@ import { useDebugTestFixture, firstBreakpointStop, caretInSource, + debuggableRow, + stopInTestSource, } from './debug-test-harness'; import { DEBUG_TYPE_ID, DebugSessionRecorder } from './run-debug-kit'; -import { - activateTestExplorer, - discoverSolution, - findItem, - runViaProfile, -} from './test-explorer-kit'; +import { activateTestExplorer, runViaProfile } from './test-explorer-kit'; import { assertPassed, cachedFor } from './test-explorer-outcome-assertions'; import { comparablePath, deepEq, eq, neq, requireAt } from './test-helpers'; import { DEBUG_SESSION_MS, DEBUG_TEST_MS } from './test-timeouts'; import { type UiStubs } from './ui-stubs'; +import { continueToEnd } from './debug-inspect-kit'; suite('Debug an F# MTP test — backtick names, theory rows and the at-cursor gesture', () => { let fixture: TestDebugFixture; @@ -67,22 +65,12 @@ suite('Debug an F# MTP test — backtick names, theory rows and the at-cursor ge ({ fixture, recorder, sessions, stubs } = harness()); }); - /** Discover the F# MTP fixture and return the tree row for `fqn`. */ - async function rowFor(fqn: string): Promise { - const api = await activateTestExplorer(); - const discovered = await discoverSolution(api, fixture.solutionPath, FS_ALL); - assert.ok(discovered.includes(fqn), `${fqn} must be discovered: ${discovered.join(', ')}`); - const item = findItem(api.testController.items, fqn); - assert.ok(item, `the TestItem for ${fqn} must exist`); - return item; - } - test('an F# backtick test on MTP attaches to the module and breaks in its body', async function () { this.timeout(DEBUG_TEST_MS); // 1. The spaced id reaches the tree verbatim, from the module's own listing // — which, unlike VSTest's, carries the binding's location. - const item = await rowFor(FS_SPACED); + const item = await debuggableRow(fixture, FS_ALL, FS_SPACED); eq(item.id, FS_SPACED, 'the tree carries the spaced id verbatim'); eq(item.label, 'adds two numbers with spaces', 'labelled with the backtick binding'); eq(item.children.size, 0, 'a module-level binding is a leaf'); @@ -102,20 +90,14 @@ suite('Debug an F# MTP test — backtick names, theory rows and the at-cursor ge assertHandshakeOrder(recorder, 'debugging an F# MTP test'); // 3. It stops IN the F# binding, with its `let` bindings readable and the // watch window evaluating in the F# frame. - const stop = await firstBreakpointStop(recorder, FS_SOURCE, 'fs-call'); - const active = requireActive('an F# MTP breakpoint stop'); - const frame = await topFrame(active, stop.threadId); - eq(frame.line, FS_SOURCE.dapLine('fs-call'), 'on the armed line of the .fs file'); - eq(comparablePath(frame.sourcePath), comparablePath(fixture.sourceFile), 'in Tests.fs'); + const { active, frame } = await stopInTestSource(recorder, fixture, FS_SOURCE, 'fs-call'); eq(variableNamed(await localsOf(active, frame.id), 'seed').value, '20', 'an F# local'); eq((await evaluate(active, 'seed', frame.id, 'watch')).value, '20', 'and a watch on it'); // 4. Continue: the module runs to the end, ONE stop, no error, and a debug // run writes no result ([TEST-MTP-DEBUG]). - await gesture(CMD_CONTINUE); - await recorder.waitForEvents('terminated', 1, DEBUG_SESSION_MS); + await continueToEnd(recorder, 'no adapter transport error'); eq(recorder.stops().length, 1, 'one stop, on the armed line'); - deepEq(recorder.errors, [], 'no adapter transport error'); deepEq(stubs.log.errorMessages, [], 'a working F# MTP debug run reports no error'); await api.testController.whenIdle(); deepEq(api.testController.getResult(FS_SPACED), cachedBefore, 'the cache is untouched'); @@ -125,7 +107,7 @@ suite('Debug an F# MTP test — backtick names, theory rows and the at-cursor ge this.timeout(DEBUG_TEST_MS); // 1. ONE row in the tree for both uids, qualified by the F# module. - const item = await rowFor(FS_ROWS); + const item = await debuggableRow(fixture, FS_ALL, FS_ROWS); eq(item.id, FS_ROWS, 'one id for both rows'); assert.ok(item.id.startsWith(`${FS_MODULE}.`), 'qualified by the F# module'); eq(item.children.size, 0, 'a theory is ONE leaf, however many rows it runs'); @@ -168,7 +150,7 @@ suite('Debug an F# MTP test — backtick names, theory rows and the at-cursor ge this.timeout(DEBUG_TEST_MS); // 1. The caret sits in the spaced binding of the MTP module's source. - await rowFor(FS_SPACED); + await debuggableRow(fixture, FS_ALL, FS_SPACED); await caretInSource(fixture, FS_SOURCE, 'fs-call', 'fsharp'); // 2. Arm and fire the at-cursor command: the same attach the tree makes. diff --git a/src/editors/vscode/src/test/suite/debug-variables-e2e.test.ts b/src/editors/vscode/src/test/suite/debug-variables-e2e.test.ts index 5a264a07..217ff018 100644 --- a/src/editors/vscode/src/test/suite/debug-variables-e2e.test.ts +++ b/src/editors/vscode/src/test/suite/debug-variables-e2e.test.ts @@ -30,6 +30,7 @@ import { debugTest, } from './debug-suite-kit'; import { deepEq, eq, neq, requireAt, assertContainsAll } from './test-helpers'; +import { assertValues } from './debug-inspect-kit'; /** Assert a variable's rendered value CONTAINS `needle`, naming what it was. */ function assertValueHas(variable: Variable, needle: string, why: string): void { @@ -91,9 +92,11 @@ suite('Debug variables — locals, arguments, this, statics and expansion', () = '"Function arguments" and "Local variables" are separate P1 rows: `left`, `right` and ' + `\`sum\` must all be inspectable. The frame offered: ${names.join(', ')}`, ); - eq(variableNamed(locals, 'left').value, '2', 'the first call passes the seed, 2'); - eq(variableNamed(locals, 'right').value, '1', 'and the loop index, 1'); - eq(variableNamed(locals, 'sum').value, '3', 'the local has already been assigned 2 + 1'); + assertValues(locals, [ + ['left', '2', 'the first call passes the seed, 2'], + ['right', '1', 'and the loop index, 1'], + ['sum', '3', 'the local has already been assigned 2 + 1'], + ]); assertTyped(variableNamed(locals, 'left'), 'int', 'the first argument'); assertTyped(variableNamed(locals, 'sum'), 'int', 'the local'); @@ -410,9 +413,11 @@ suite('Debug variables — locals, arguments, this, statics and expansion', () = // Interaction 1 — stop inside the loop on its first pass. const { session, frame: firstFrame } = await runToFirstFrame(debuggee(), 'accumulate-call'); const firstLocals = await localsOf(session, firstFrame.id); - eq(variableNamed(firstLocals, 'running').value, '2', 'the accumulator starts at the seed'); - eq(variableNamed(firstLocals, 'index').value, '1', 'and the loop is on its first pass'); - eq(variableNamed(firstLocals, 'seed').value, '2', 'with the argument it was called with'); + assertValues(firstLocals, [ + ['running', '2', 'the accumulator starts at the seed'], + ['index', '1', 'and the loop is on its first pass'], + ['seed', '2', 'with the argument it was called with'], + ]); // Interaction 2 — continue to the SECOND pass. Both the accumulator and // the loop variable must have moved on. @@ -421,12 +426,14 @@ suite('Debug variables — locals, arguments, this, statics and expansion', () = const second = requireAt(stops, 1, 'the second loop stop'); const secondFrame = await topFrame(session, second.threadId); const secondLocals = await localsOf(session, secondFrame.id); - eq( - variableNamed(secondLocals, 'index').value, - '2', - 'the loop variable must report its NEW value, not the value of the first read', - ); - eq(variableNamed(secondLocals, 'running').value, '3', 'and the accumulator its new total'); + assertValues(secondLocals, [ + [ + 'index', + '2', + 'the loop variable must report its NEW value, not the value of the first read', + ], + ['running', '3', 'and the accumulator its new total'], + ]); neq( variableNamed(secondLocals, 'running').value, variableNamed(firstLocals, 'running').value, @@ -440,8 +447,10 @@ suite('Debug variables — locals, arguments, this, statics and expansion', () = const third = requireAt(await recorder.waitForStops(3), 2, 'the third loop stop'); const thirdFrame = await topFrame(session, third.threadId); const thirdLocals = await localsOf(session, thirdFrame.id); - eq(variableNamed(thirdLocals, 'index').value, '3', 'the third pass reports the third index'); - eq(variableNamed(thirdLocals, 'running').value, '5', 'and the running total to date'); + assertValues(thirdLocals, [ + ['index', '3', 'the third pass reports the third index'], + ['running', '5', 'and the running total to date'], + ]); eq( (await evaluate(session, 'running + index', thirdFrame.id, 'watch')).value, '8', diff --git a/src/editors/vscode/src/test/suite/explorer-kit.ts b/src/editors/vscode/src/test/suite/explorer-kit.ts index cd09977e..8017ee68 100644 --- a/src/editors/vscode/src/test/suite/explorer-kit.ts +++ b/src/editors/vscode/src/test/suite/explorer-kit.ts @@ -8,7 +8,7 @@ import * as assert from 'node:assert/strict'; import * as fs from 'node:fs'; import * as path from 'node:path'; import * as vscode from 'vscode'; -import { EXTENSION_ID } from './test-helpers'; +import { EXTENSION_ID, pollUntilResult } from './test-helpers'; /** One node of the tree, viewed only through the fields the searches read. */ export interface ExplorerNode { @@ -73,3 +73,12 @@ export function writeOneProjectSolution( fs.writeFileSync(slnPath, [header, entry, 'EndProject', 'Global', 'EndGlobal'].join('\n')); return { slnPath, projDir, csprojPath }; } + +/** Poll `provider`'s tree for up to 5 s until a node's label contains `target`. */ +export async function pollTreeFor(provider: ExplorerProvider, target: string): Promise { + return await pollUntilResult( + async () => treeContains(provider.getChildren(), target), + (found) => found, + 5_000, + ); +} diff --git a/src/editors/vscode/src/test/suite/fsharp-lsp-codefix-basics.test.ts b/src/editors/vscode/src/test/suite/fsharp-lsp-codefix-basics.test.ts index abd6fb23..43c215a5 100644 --- a/src/editors/vscode/src/test/suite/fsharp-lsp-codefix-basics.test.ts +++ b/src/editors/vscode/src/test/suite/fsharp-lsp-codefix-basics.test.ts @@ -12,19 +12,17 @@ import { activateWarmFSharp, assertInsertion, assertNoAction, - assertQuickFix, assertReplacement, diagnosticWithCode, openOverlay, quickFixes, - resolvedQuickFixes, singleEdit, tokenRange, - undoAction, - uniqueAction, assertFixApplied, + inspectQuickFix, + undoAndRequery, + assertSidecarDiagnostics, } from './fsharp-refactor-test-kit'; -import { diagnosticCode } from './document-anchors'; import { revertDocument } from './refactor-test-helpers'; import { closeAllEditors } from './test-helpers'; import { LSP_RESPONSE_MS, SIDECAR_COLD_MS } from './test-timeouts'; @@ -103,7 +101,7 @@ async function assertFalseOpen(name: string, title: string): Promise { try { const range = tokenRange(fixture.document, name); const diagnostics = await diagnosticWithCode(fixture.uri, 'FS0039'); - assertDiagnostic(diagnostics, range, 'FS0039'); + assertSidecarDiagnostics(diagnostics, range, 'FS0039'); assertNoAction(await quickFixes(fixture.uri, range), title); assert.strictEqual(fixture.document.getText(), source); assert.ok(fixture.document.isDirty); @@ -194,64 +192,15 @@ async function runBasicFix(spec: BasicFixSpec): Promise { const fixture = await openOverlay(TARGET_FILE, spec.source); try { const range = tokenRange(fixture.document, spec.target, spec.occurrence); - const action = await inspectAction(fixture, range, spec); + const action = await inspectQuickFix(fixture, range, spec, spec.preferred); inspectEdit(fixture.document, fixture.uri, action, spec); await applyAndRecheck(fixture, action, spec); - await undoAndRequery(fixture, spec); + await undoAndRequery(fixture, spec, spec.preferred); } finally { await revertDocument(fixture.document); } } -async function inspectAction( - fixture: Awaited>, - range: vscode.Range, - spec: BasicFixSpec, -): Promise { - const diagnostics = await diagnosticWithCode(fixture.uri, spec.diagnostic); - assertDiagnostic(diagnostics, range, spec.diagnostic); - const listed = await quickFixes(fixture.uri, range); - const raw = uniqueAction(listed, spec.title); - assertListedAction(raw, spec); - await assertOutsideRange(fixture, spec.title); - const resolved = await resolvedQuickFixes(fixture.uri, range, spec.title); - const action = uniqueAction(resolved, spec.title); - assertQuickFix(action, spec.title, spec.preferred); - return action; -} - -function assertDiagnostic( - diagnostics: readonly vscode.Diagnostic[], - range: vscode.Range, - code: string, -): void { - const matches = diagnostics.filter((item) => diagnosticCode(item) === code); - assert.ok(matches.length >= 1, `${code} must be published`); - assert.ok(matches.some((item) => item.range.intersection(range) !== undefined)); - assert.ok(matches.every((item) => item.message.trim().length > 0)); - assert.ok(matches.every((item) => item.source === 'sharplsp-fsharp')); -} - -function assertListedAction(action: vscode.CodeAction, spec: BasicFixSpec): void { - assert.strictEqual(action.title, spec.title); - assert.strictEqual(action.kind?.value, vscode.CodeActionKind.QuickFix.value); - assert.strictEqual(action.isPreferred, spec.preferred); - assert.strictEqual(action.edit, undefined, 'listed action must remain unresolved'); - assert.strictEqual(action.command, undefined, 'quick fix must use an edit, not a command'); -} - -async function assertOutsideRange( - fixture: Awaited>, - title: string, -): Promise { - const outside = tokenRange(fixture.document, 'sentinel'); - const actions = await quickFixes(fixture.uri, outside); - assertNoAction(actions, title); - assert.ok( - actions.every((action) => action.kind?.contains(vscode.CodeActionKind.QuickFix) ?? true), - ); -} - function inspectEdit( document: vscode.TextDocument, uri: vscode.Uri, @@ -274,14 +223,3 @@ async function applyAndRecheck( await assertFixApplied(fixture, action, spec.expected, spec.diagnostic, target); assert.ok(fixture.document.getText().includes('sentinel')); } - -async function undoAndRequery( - fixture: Awaited>, - spec: BasicFixSpec, -): Promise { - await undoAction(fixture.document, spec.source); - await diagnosticWithCode(fixture.uri, spec.diagnostic); - const range = tokenRange(fixture.document, spec.target, spec.occurrence); - const actions = await resolvedQuickFixes(fixture.uri, range, spec.title); - assertQuickFix(uniqueAction(actions, spec.title), spec.title, spec.preferred); -} diff --git a/src/editors/vscode/src/test/suite/fsharp-lsp-codefix-conversions.test.ts b/src/editors/vscode/src/test/suite/fsharp-lsp-codefix-conversions.test.ts index f2162e29..0bd6c0c6 100644 --- a/src/editors/vscode/src/test/suite/fsharp-lsp-codefix-conversions.test.ts +++ b/src/editors/vscode/src/test/suite/fsharp-lsp-codefix-conversions.test.ts @@ -9,18 +9,16 @@ import { import { applyAction, assertNoAction, - assertQuickFix, assertReplacement, diagnosticsSettled, diagnosticWithCode, openOverlay, quickFixes, assertPrepareAcrossToken, - resolvedQuickFixes, singleEdit, tokenRange, - undoAction, - uniqueAction, + inspectQuickFix, + undoAndRequery, } from './fsharp-refactor-test-kit'; import { diagnosticCode } from './document-anchors'; import { activateRealSharpLsp, revertDocument } from './refactor-test-helpers'; @@ -118,52 +116,15 @@ async function runConversion(scenario: CodeFixScenario): Promise { const fixture = await openOverlay(TARGET_FILE, scenario.source); try { const range = tokenRange(fixture.document, scenario.target, scenario.occurrence); - const action = await inspectConversion(fixture, range, scenario); + const action = await inspectQuickFix(fixture, range, scenario, false, assertTypeMismatch); assertConversionEdit(fixture.document, fixture.uri, action, scenario); await applyConversion(fixture, action, scenario); - await undoConversion(fixture, scenario); + await undoAndRequery(fixture, scenario, false); } finally { await revertDocument(fixture.document); } } -async function inspectConversion( - fixture: Awaited>, - range: vscode.Range, - scenario: CodeFixScenario, -): Promise { - const diagnostics = await diagnosticWithCode(fixture.uri, scenario.diagnostic); - assertConversionDiagnostic(diagnostics, range); - const raw = uniqueAction(await quickFixes(fixture.uri, range), scenario.title); - assertRawConversion(raw, scenario.title); - const outside = await quickFixes(fixture.uri, tokenRange(fixture.document, 'sentinel')); - assertNoAction(outside, scenario.title); - const resolved = await resolvedQuickFixes(fixture.uri, range, scenario.title); - const action = uniqueAction(resolved, scenario.title); - assertQuickFix(action, scenario.title, false); - return action; -} - -function assertConversionDiagnostic( - diagnostics: readonly vscode.Diagnostic[], - range: vscode.Range, -): void { - const mismatches = diagnostics.filter((item) => diagnosticCode(item) === 'FS0001'); - assert.ok(mismatches.length >= 1); - assert.ok(mismatches.some((item) => item.range.intersection(range) !== undefined)); - assert.ok(mismatches.every((item) => item.severity === vscode.DiagnosticSeverity.Error)); - assert.ok(mismatches.every((item) => item.source === 'sharplsp-fsharp')); - assert.ok(mismatches.every((item) => /type/i.test(item.message))); -} - -function assertRawConversion(action: vscode.CodeAction, title: string): void { - assert.strictEqual(action.title, title); - assert.strictEqual(action.kind?.value, vscode.CodeActionKind.QuickFix.value); - assert.strictEqual(action.isPreferred, false); - assert.strictEqual(action.edit, undefined); - assert.strictEqual(action.command, undefined); -} - function assertConversionEdit( document: vscode.TextDocument, uri: vscode.Uri, @@ -204,17 +165,12 @@ async function assertConversionClean(uri: vscode.Uri, diagnostic: string): Promi ); } -async function undoConversion( - fixture: Awaited>, - scenario: CodeFixScenario, -): Promise { - await undoAction(fixture.document, scenario.source); - await diagnosticWithCode(fixture.uri, scenario.diagnostic); - const range = tokenRange(fixture.document, scenario.target, scenario.occurrence); - const actions = await resolvedQuickFixes(fixture.uri, range, scenario.title); - assertQuickFix(uniqueAction(actions, scenario.title), scenario.title, false); -} - function expectedSource(scenario: CodeFixScenario): string { return scenario.source.replace(`accept ${scenario.target}\n`, `accept ${scenario.replacement}\n`); } + +/** FS0001 is a TYPE mismatch the compiler reports as an error. */ +function assertTypeMismatch(mismatches: readonly vscode.Diagnostic[]): void { + assert.ok(mismatches.every((item) => item.severity === vscode.DiagnosticSeverity.Error)); + assert.ok(mismatches.every((item) => /type/i.test(item.message))); +} diff --git a/src/editors/vscode/src/test/suite/fsharp-lsp-codefix-generation.test.ts b/src/editors/vscode/src/test/suite/fsharp-lsp-codefix-generation.test.ts index 64e47763..036edf8c 100644 --- a/src/editors/vscode/src/test/suite/fsharp-lsp-codefix-generation.test.ts +++ b/src/editors/vscode/src/test/suite/fsharp-lsp-codefix-generation.test.ts @@ -20,16 +20,13 @@ import { applyAction, assertInsertion, assertNoAction, - assertQuickFix, diagnosticsSettled, - diagnosticWithCode, openOverlay, quickFixes, - resolvedQuickFixes, singleEdit, tokenRange, - undoAction, - uniqueAction, + inspectQuickFix, + undoAndRequery, } from './fsharp-refactor-test-kit'; import { diagnosticCode } from './document-anchors'; import { activateRealSharpLsp, revertDocument } from './refactor-test-helpers'; @@ -329,52 +326,15 @@ async function runGeneration(spec: GenerationSpec): Promise { const fixture = await openOverlay(TARGET_FILE, spec.source); try { const range = tokenRange(fixture.document, spec.target, spec.occurrence); - const action = await inspectGeneration(fixture, range, spec); + const action = await inspectQuickFix(fixture, range, spec, true); inspectGenerationEdit(fixture.uri, action, spec); await applyGeneration(fixture, action, spec); - await undoGeneration(fixture, spec); + await undoAndRequery(fixture, spec, true); } finally { await revertDocument(fixture.document); } } -async function inspectGeneration( - fixture: Awaited>, - range: vscode.Range, - spec: GenerationSpec, -): Promise { - const diagnostics = await diagnosticWithCode(fixture.uri, spec.diagnostic, range); - assertGenerationDiagnostic(diagnostics, range, spec.diagnostic); - const raw = uniqueAction(await quickFixes(fixture.uri, range), spec.title); - assertRawGeneration(raw, spec.title); - const outside = await quickFixes(fixture.uri, tokenRange(fixture.document, 'sentinel')); - assertNoAction(outside, spec.title); - const resolved = await resolvedQuickFixes(fixture.uri, range, spec.title); - const action = uniqueAction(resolved, spec.title); - assertQuickFix(action, spec.title, true); - return action; -} - -function assertGenerationDiagnostic( - diagnostics: readonly vscode.Diagnostic[], - range: vscode.Range, - code: string, -): void { - const matches = diagnostics.filter((item) => diagnosticCode(item) === code); - assert.ok(matches.length >= 1, `${code} must drive the generator`); - assert.ok(matches.some((item) => item.range.intersection(range) !== undefined)); - assert.ok(matches.every((item) => item.message.trim().length > 0)); - assert.ok(matches.every((item) => item.source === 'sharplsp-fsharp')); -} - -function assertRawGeneration(action: vscode.CodeAction, title: string): void { - assert.strictEqual(action.title, title); - assert.strictEqual(action.kind?.value, vscode.CodeActionKind.QuickFix.value); - assert.strictEqual(action.isPreferred, true); - assert.strictEqual(action.edit, undefined); - assert.strictEqual(action.command, undefined); -} - function inspectGenerationEdit( uri: vscode.Uri, action: vscode.CodeAction, @@ -448,17 +408,6 @@ function assertGeneratedDocument(document: vscode.TextDocument, spec: Generation assert.ok(text.includes('sentinel')); } -async function undoGeneration( - fixture: Awaited>, - spec: GenerationSpec, -): Promise { - await undoAction(fixture.document, spec.source); - await diagnosticWithCode(fixture.uri, spec.diagnostic); - const range = tokenRange(fixture.document, spec.target, spec.occurrence); - const actions = await resolvedQuickFixes(fixture.uri, range, spec.title); - assertQuickFix(uniqueAction(actions, spec.title), spec.title, true); -} - async function assertComplete(spec: GenerationSpec): Promise { const fixture = await openOverlay(TARGET_FILE, spec.source); try { diff --git a/src/editors/vscode/src/test/suite/fsharp-refactor-test-kit.ts b/src/editors/vscode/src/test/suite/fsharp-refactor-test-kit.ts index 23ae8f58..a222c5c6 100644 --- a/src/editors/vscode/src/test/suite/fsharp-refactor-test-kit.ts +++ b/src/editors/vscode/src/test/suite/fsharp-refactor-test-kit.ts @@ -347,3 +347,78 @@ export function changedFileNames(edit: vscode.WorkspaceEdit): string[] { export function countOccurrences(text: string, needle: string): number { return text.split(needle).length - 1; } + +/** Where a quick fix is exercised: the overlay, the token it fixes, and what it answers. */ +export interface QuickFixSite { + readonly source: string; + readonly target: string; + readonly occurrence?: number; + readonly title: string; + readonly diagnostic: string; +} + +/** The `code` diagnostics: published, one ON `range`, each worded, each from the F# sidecar. */ +export function assertSidecarDiagnostics( + diagnostics: readonly vscode.Diagnostic[], + range: vscode.Range, + code: string, +): vscode.Diagnostic[] { + const matches = diagnostics.filter((item) => diagnosticCode(item) === code); + assert.ok(matches.length >= 1, `${code} must be published`); + assert.ok(matches.some((item) => item.range.intersection(range) !== undefined)); + assert.ok(matches.every((item) => item.message.trim().length > 0)); + assert.ok(matches.every((item) => item.source === 'sharplsp-fsharp')); + return matches; +} + +/** A LISTED quick fix: still unresolved, and an edit rather than a command. */ +function assertListedQuickFix(action: vscode.CodeAction, title: string, preferred: boolean): void { + assert.strictEqual(action.title, title); + assert.strictEqual(action.kind?.value, vscode.CodeActionKind.QuickFix.value); + assert.strictEqual(action.isPreferred, preferred); + assert.strictEqual(action.edit, undefined, 'listed action must remain unresolved'); + assert.strictEqual(action.command, undefined, 'quick fix must use an edit, not a command'); +} + +/** + * List then resolve the quick fix `site` names on `range`: its diagnostic sits + * there, the listing offers it once and unresolved, the `sentinel` binding + * outside it never does, and it resolves to an edit. `refine` adds the + * caller's own checks on the diagnostics that drove it. [ANALYZERS-FSAC-PARITY] + */ +export async function inspectQuickFix( + fixture: OpenFixture, + range: vscode.Range, + site: QuickFixSite, + preferred: boolean, + refine: (matches: readonly vscode.Diagnostic[]) => void = () => undefined, +): Promise { + const diagnostics = await diagnosticWithCode(fixture.uri, site.diagnostic, range); + refine(assertSidecarDiagnostics(diagnostics, range, site.diagnostic)); + assertListedQuickFix( + uniqueAction(await quickFixes(fixture.uri, range), site.title), + site.title, + preferred, + ); + const outside = await quickFixes(fixture.uri, tokenRange(fixture.document, 'sentinel')); + assertNoAction(outside, site.title); + assert.ok( + outside.every((action) => action.kind?.contains(vscode.CodeActionKind.QuickFix) ?? true), + ); + const action = uniqueAction(await resolvedQuickFixes(fixture.uri, range, site.title), site.title); + assertQuickFix(action, site.title, preferred); + return action; +} + +/** Undo restores `site.source`, its diagnostic returns, and the fix is offered again. */ +export async function undoAndRequery( + fixture: OpenFixture, + site: QuickFixSite, + preferred: boolean, +): Promise { + await undoAction(fixture.document, site.source); + await diagnosticWithCode(fixture.uri, site.diagnostic); + const range = tokenRange(fixture.document, site.target, site.occurrence); + const actions = await resolvedQuickFixes(fixture.uri, range, site.title); + assertQuickFix(uniqueAction(actions, site.title), site.title, preferred); +} diff --git a/src/editors/vscode/src/test/suite/lsp-refactor-core.test.ts b/src/editors/vscode/src/test/suite/lsp-refactor-core.test.ts index 4d086933..0214323a 100644 --- a/src/editors/vscode/src/test/suite/lsp-refactor-core.test.ts +++ b/src/editors/vscode/src/test/suite/lsp-refactor-core.test.ts @@ -1,22 +1,14 @@ // Exhaustive real-LSP Roslyn refactor matrix for [SHARPLSP-FEATURES-REFACTORING]. -import * as assert from 'node:assert/strict'; -import * as vscode from 'vscode'; import { - assertFragments, - assertFreshActionDataIds, - assertRawActionData, - assertRawTitles, - assertSingleDocumentEdit, - onlyAction, - rawCodeActions, - type RawCodeAction, + applyAction, + assertActionRequery, + assertOutsideActionRange, + discoverAction, + resolveAction, + type ActionLifecycleCase, } from './csharp-refactor-test-kit'; -import { rangeAfterAction, rangeOf } from './document-anchors'; import { - applyWorkspaceEdit, replaceDocumentText, - waitForCodeActions, - waitForResolvedCodeActions, type OpenFixture, useRefactorFixture, restoreCommitted, @@ -39,35 +31,8 @@ import { LSP_RESPONSE_MS } from './test-timeouts'; const FILE = 'RefactorCore.cs'; -interface RefactorScenario { - readonly label: string; - readonly source: string; - readonly snippet: string; - readonly focus: string; - readonly title: string; - readonly kind: string; - readonly options: readonly string[]; - readonly presentAfter: readonly string[]; - readonly absentAfter: readonly string[]; - readonly patternsAfter?: readonly RegExp[]; - readonly mustDisappear?: boolean; - readonly postApplySnippet?: string; - readonly postApplyFocus?: string; - readonly requeryTitleCount?: number; -} - -function vscodeKind(value: string): vscode.CodeActionKind { - switch (value) { - case 'refactor.extract': - return vscode.CodeActionKind.RefactorExtract; - case 'refactor.inline': - return vscode.CodeActionKind.RefactorInline; - case 'refactor.rewrite': - return vscode.CodeActionKind.RefactorRewrite; - default: - return vscode.CodeActionKind.Refactor; - } -} +/** A lifecycle case that lists every title its range offers. */ +type RefactorScenario = ActionLifecycleCase & { readonly options: readonly string[] }; const EXPRESSION_SITE = { source: EXPRESSION_SOURCE, @@ -277,104 +242,16 @@ const SCENARIOS: readonly RefactorScenario[] = [ }, ]; -async function assertOutsideRange(fixture: OpenFixture, scenario: RefactorScenario): Promise { - const range = rangeOf(fixture.document, 'namespace'); - const raw = await rawCodeActions(fixture.uri, range); - assert.ok(!raw.some((action) => action.title === scenario.title)); - const actions = await waitForCodeActions({ - uri: fixture.uri, - range, - kind: vscodeKind(scenario.kind), - predicate: () => true, - }); - assert.ok(!actions.some((action) => action.title === scenario.title)); -} - -async function discover( - fixture: OpenFixture, - scenario: RefactorScenario, -): Promise<{ readonly range: vscode.Range; readonly raw: RawCodeAction[] }> { - const range = rangeOf(fixture.document, scenario.snippet, scenario.focus); - const actions = await waitForCodeActions({ - uri: fixture.uri, - range, - kind: vscodeKind(scenario.kind), - predicate: (items) => items.some((item) => item.title === scenario.title), - }); - onlyAction(actions, scenario.title); - const raw = await rawCodeActions(fixture.uri, range); - assertRawTitles(raw, scenario.options, scenario.kind); - assertRawActionData(raw, fixture.uri); - return { range, raw }; -} - -async function resolve( - fixture: OpenFixture, - scenario: RefactorScenario, - range: vscode.Range, -): Promise { - const actions = await waitForResolvedCodeActions({ - uri: fixture.uri, - range, - kind: vscodeKind(scenario.kind), - predicate: (items) => items.some((item) => item.title === scenario.title && item.edit), - }); - for (const title of scenario.options) onlyAction(actions, title); - const selected = onlyAction(actions, scenario.title); - assert.strictEqual(selected.kind?.value, scenario.kind); - assert.ok(selected.edit, `${scenario.title} must resolve to an edit`); - return selected.edit; -} - -function assertMutation( - fixture: OpenFixture, - scenario: RefactorScenario, - previousVersion: number, -): void { - const source = fixture.document.getText(); - assertFragments(source, scenario.presentAfter, scenario.absentAfter); - for (const pattern of scenario.patternsAfter ?? []) assert.match(source, pattern); - assert.ok(fixture.document.version > previousVersion); - assert.ok(fixture.document.isDirty); -} - -async function assertRequery( - fixture: OpenFixture, - scenario: RefactorScenario, - range: vscode.Range, - before: readonly RawCodeAction[], -): Promise { - const requeryRange = rangeAfterAction( - fixture, - range, - scenario.postApplySnippet, - scenario.postApplyFocus, - ); - const after = await rawCodeActions(fixture.uri, requeryRange); - assertRawActionData(after, fixture.uri); - assertFreshActionDataIds(after, before); - if (scenario.mustDisappear) assert.ok(!after.some((action) => action.title === scenario.title)); - if (scenario.requeryTitleCount !== undefined) { - assert.strictEqual( - after.filter((action) => action.title === scenario.title).length, - scenario.requeryTitleCount, - ); - } -} - async function runScenario( fixture: OpenFixture, committedText: string, scenario: RefactorScenario, ): Promise { await replaceDocumentText(fixture.document, scenario.source); - await assertOutsideRange(fixture, scenario); - const discovered = await discover(fixture, scenario); - const edit = await resolve(fixture, scenario, discovered.range); - const version = fixture.document.version; - assertSingleDocumentEdit(await applyWorkspaceEdit(edit), fixture); - assertMutation(fixture, scenario, version); - await assertRequery(fixture, scenario, discovered.range, discovered.raw); + await assertOutsideActionRange(fixture, scenario); + const discovered = await discoverAction(fixture, scenario); + await applyAction(fixture, scenario, await resolveAction(fixture, scenario, discovered.range)); + await assertActionRequery(fixture, scenario, discovered.range, discovered.raw); await restoreCommitted(fixture, committedText); } diff --git a/src/editors/vscode/src/test/suite/lsp-refactor-quickfixes.test.ts b/src/editors/vscode/src/test/suite/lsp-refactor-quickfixes.test.ts index d59c401a..a0fd613c 100644 --- a/src/editors/vscode/src/test/suite/lsp-refactor-quickfixes.test.ts +++ b/src/editors/vscode/src/test/suite/lsp-refactor-quickfixes.test.ts @@ -3,18 +3,18 @@ import * as assert from 'node:assert/strict'; import * as vscode from 'vscode'; import { assertFragments, - assertRawActionData, - assertRawTitles, - onlyAction, rawCodeActions, + assertOutsideActionRange, + assertRequiredDiagnostic, + discoverAction, + resolveAction, + type ActionLifecycleCase, } from './csharp-refactor-test-kit'; -import { diagnosticCode, rangeOf } from './document-anchors'; +import { diagnosticCode } from './document-anchors'; import { applyWorkspaceEdit, replaceDocumentText, - waitForCodeActions, waitForMatchingDiagnostics, - waitForResolvedCodeActions, type OpenFixture, type WorkspaceEditSnapshot, useRefactorFixture, @@ -24,17 +24,11 @@ import { LSP_RESPONSE_MS } from './test-timeouts'; const FILE = 'RefactorQuickFixes.cs'; -interface QuickFixScenario { - readonly label: string; - readonly source: string; - readonly snippet: string; - readonly focus: string; +/** A compiler quick fix: the lifecycle case plus the diagnostic it answers. */ +type QuickFixScenario = ActionLifecycleCase & { readonly diagnosticCode: string; - readonly title: string; readonly options: readonly string[]; - readonly presentAfter: readonly string[]; - readonly absentAfter: readonly string[]; -} +}; const UNUSED_LOCAL = `namespace SharpLsp.TestFixtures.Refactors; @@ -96,6 +90,7 @@ const SCENARIOS: readonly QuickFixScenario[] = [ focus: 'unusedValue', diagnosticCode: 'CS0219', title: 'Remove unused variable', + kind: 'quickfix', options: ['Remove unused variable'], presentAfter: ['return input + 1;', 'unused-local-sentinel'], absentAfter: ['unusedValue'], @@ -107,6 +102,7 @@ const SCENARIOS: readonly QuickFixScenario[] = [ focus: 'StringBuilder', diagnosticCode: 'CS0246', title: 'using System.Text;', + kind: 'quickfix', options: ['System.Text.StringBuilder', 'using System.Text;'], presentAfter: ['using System.Text;', 'new StringBuilder()', 'add-using-sentinel'], absentAfter: [], @@ -118,6 +114,7 @@ const SCENARIOS: readonly QuickFixScenario[] = [ focus: 'MissingOperation', diagnosticCode: 'CS0103', title: "Generate method 'MissingOperation'", + kind: 'quickfix', options: ["Generate method 'MissingOperation'"], presentAfter: ['MissingOperation(int input)', 'throw new', 'generate-method-sentinel'], absentAfter: [], @@ -129,6 +126,7 @@ const SCENARIOS: readonly QuickFixScenario[] = [ focus: 'IQuickContract', diagnosticCode: 'CS0535', title: 'Implement interface', + kind: 'quickfix', options: ['Implement interface', 'Implement all members explicitly'], presentAfter: [ 'public int Compute(int input)', @@ -139,68 +137,6 @@ const SCENARIOS: readonly QuickFixScenario[] = [ }, ]; -async function assertNegativeRange( - fixture: OpenFixture, - scenario: QuickFixScenario, -): Promise { - const outside = rangeOf(fixture.document, 'namespace', 'namespace'); - const raw = await rawCodeActions(fixture.uri, outside); - assert.ok(!raw.some((action) => action.title === scenario.title)); - const actions = await waitForCodeActions({ - uri: fixture.uri, - range: outside, - kind: vscode.CodeActionKind.QuickFix, - predicate: () => true, - }); - assert.ok(!actions.some((action) => action.title === scenario.title)); -} - -async function assertDiagnostic(fixture: OpenFixture, scenario: QuickFixScenario): Promise { - const diagnostics = await waitForMatchingDiagnostics(fixture.uri, (items) => - items.some((item) => diagnosticCode(item) === scenario.diagnosticCode), - ); - const matches = diagnostics.filter((item) => diagnosticCode(item) === scenario.diagnosticCode); - assert.ok(matches.length >= 1, `missing ${scenario.diagnosticCode}`); - assert.ok(matches.every((item) => item.message.length > 0)); - assert.ok(matches.every((item) => !item.range.isEmpty)); -} - -async function discoverInside( - fixture: OpenFixture, - scenario: QuickFixScenario, -): Promise { - const range = rangeOf(fixture.document, scenario.snippet, scenario.focus); - const actions = await waitForCodeActions({ - uri: fixture.uri, - range, - kind: vscode.CodeActionKind.QuickFix, - predicate: (items) => items.some((item) => item.title === scenario.title), - }); - onlyAction(actions, scenario.title); - const raw = await rawCodeActions(fixture.uri, range); - assertRawTitles(raw, scenario.options, 'quickfix'); - assertRawActionData(raw, fixture.uri); - return range; -} - -async function resolveEdit( - fixture: OpenFixture, - scenario: QuickFixScenario, - range: vscode.Range, -): Promise { - const actions = await waitForResolvedCodeActions({ - uri: fixture.uri, - range, - kind: vscode.CodeActionKind.QuickFix, - predicate: (items) => items.some((item) => item.title === scenario.title && item.edit), - }); - for (const title of scenario.options) onlyAction(actions, title); - const selected = onlyAction(actions, scenario.title); - assert.strictEqual(selected.kind?.value, vscode.CodeActionKind.QuickFix.value); - assert.ok(selected.edit, `${scenario.title} must resolve to a WorkspaceEdit`); - return selected.edit; -} - function assertSnapshots(snapshots: readonly WorkspaceEditSnapshot[], fixture: OpenFixture): void { assert.strictEqual(snapshots.length, 1, 'selected quick fix must edit one document'); assert.strictEqual(snapshots[0]?.uri.toString(), fixture.uri.toString()); @@ -238,10 +174,10 @@ async function runScenario( ): Promise { await replaceDocumentText(fixture.document, scenario.source); assert.ok(fixture.document.isDirty); - await assertDiagnostic(fixture, scenario); - await assertNegativeRange(fixture, scenario); - const range = await discoverInside(fixture, scenario); - const edit = await resolveEdit(fixture, scenario, range); + await assertRequiredDiagnostic(fixture, scenario); + await assertOutsideActionRange(fixture, scenario); + const { range } = await discoverAction(fixture, scenario); + const edit = await resolveAction(fixture, scenario, range); const version = fixture.document.version; assertSnapshots(await applyWorkspaceEdit(edit), fixture); assertMutation(fixture, scenario, version); diff --git a/src/editors/vscode/src/test/suite/run-debug-case.ts b/src/editors/vscode/src/test/suite/run-debug-case.ts new file mode 100644 index 00000000..a54b1ba0 --- /dev/null +++ b/src/editors/vscode/src/test/suite/run-debug-case.ts @@ -0,0 +1,41 @@ +// The per-test lifecycle of the Run/Debug suites: a fresh scratch directory and +// the observers every negative assertion needs — UI stubs, the debug sessions +// and the tasks a gesture starts — torn down in one safe order afterwards. +import * as assert from 'node:assert/strict'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { DebugSessionRecorder, TaskRecorder, stopAnyDebugSession } from './run-debug-kit'; +import type { Quiet } from './run-debug-target-kit'; +import { closeAllEditors, removeDirRecursive } from './test-helpers'; +import { installUiStubs } from './ui-stubs'; + +/** One test's scratch directory, beside the observers armed around it. */ +export interface RunDebugCase extends Quiet { + readonly tmpDir: string; +} + +/** Arm a fresh {@link RunDebugCase} (temp dir named `prefix…`) around every test. */ +export function useRunDebugCase(prefix: string): () => RunDebugCase { + let current: RunDebugCase | undefined; + setup(() => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + const stubs = installUiStubs(); + current = { tmpDir, stubs, sessions: new DebugSessionRecorder(), tasks: new TaskRecorder() }; + }); + teardown(async () => { + const active = current; + current = undefined; + if (active === undefined) return; + active.stubs.restore(); + await stopAnyDebugSession(); + active.sessions.dispose(); + active.tasks.dispose(); + await closeAllEditors(); + removeDirRecursive(active.tmpDir); + }); + return () => { + assert.ok(current, 'the run/debug case must be armed in setup'); + return current; + }; +} diff --git a/src/editors/vscode/src/test/suite/run-debug-profiles.test.ts b/src/editors/vscode/src/test/suite/run-debug-profiles.test.ts index 78b76040..28debcdf 100644 --- a/src/editors/vscode/src/test/suite/run-debug-profiles.test.ts +++ b/src/editors/vscode/src/test/suite/run-debug-profiles.test.ts @@ -10,7 +10,6 @@ // provider at activation and a second registration corrupts the host. import * as assert from 'node:assert/strict'; import * as fs from 'node:fs'; -import * as os from 'node:os'; import * as path from 'node:path'; import * as vscode from 'vscode'; import { @@ -27,7 +26,6 @@ import { fakeFolder, focusDocument, bareF5Config, - stopAnyDebugSession, undefinedF5Config, } from './run-debug-kit'; import { @@ -37,9 +35,10 @@ import { writeRawLaunchSettings, writeRunJson, } from './run-debug-fixtures'; -import { installUiStubs, type UiStubs } from './ui-stubs'; -import { closeAllEditors, comparablePath, removeDirRecursive } from './test-helpers'; +import { type UiStubs } from './ui-stubs'; +import { comparablePath } from './test-helpers'; import { DOTNET_CLI_MS } from './test-timeouts'; +import { useRunDebugCase } from './run-debug-case'; /** Shorthand so every assertion helper fits one signature line. */ type Config = vscode.DebugConfiguration; @@ -166,20 +165,9 @@ suite('Run and Debug: launch profiles', () => { let sessions: DebugSessionRecorder; let tasks: TaskRecorder; + const runCase = useRunDebugCase('sharplsp-run-debug-profiles-'); setup(() => { - tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sharplsp-run-debug-profiles-')); - stubs = installUiStubs(); - sessions = new DebugSessionRecorder(); - tasks = new TaskRecorder(); - }); - - teardown(async () => { - stubs.restore(); - await stopAnyDebugSession(); - sessions.dispose(); - tasks.dispose(); - await closeAllEditors(); - removeDirRecursive(tmpDir); + ({ tmpDir, stubs, sessions, tasks } = runCase()); }); /** A fresh case directory under the suite's temp root. */ diff --git a/src/editors/vscode/src/test/suite/run-debug-target.test.ts b/src/editors/vscode/src/test/suite/run-debug-target.test.ts index 261e7d34..92e2e60d 100644 --- a/src/editors/vscode/src/test/suite/run-debug-target.test.ts +++ b/src/editors/vscode/src/test/suite/run-debug-target.test.ts @@ -24,7 +24,6 @@ // activation and a second registration corrupts the host. import * as assert from 'node:assert/strict'; import * as fs from 'node:fs'; -import * as os from 'node:os'; import * as path from 'node:path'; import { chosenFramework } from '../../launch-framework-run'; import type { ProjectTarget } from '../../launch-resolver'; @@ -51,17 +50,11 @@ import { TaskRecorder, assertCommandRegistered, invokeCommand, - stopAnyDebugSession, } from './run-debug-kit'; -import { - closeAllEditors, - comparablePath, - openCSharpFile, - openFSharpFile, - removeDirRecursive, -} from './test-helpers'; +import { comparablePath, openCSharpFile, openFSharpFile } from './test-helpers'; import { DOTNET_CLI_MS, QUIET_MS } from './test-timeouts'; -import { installUiStubs, type UiStubs } from './ui-stubs'; +import { type UiStubs } from './ui-stubs'; +import { useRunDebugCase } from './run-debug-case'; suite('Run/Debug launch target — [DEBUG-FEATURES-LAUNCH-TARGET] + [SCRIPT-CONE]', () => { let tmpDir: string; @@ -70,21 +63,11 @@ suite('Run/Debug launch target — [DEBUG-FEATURES-LAUNCH-TARGET] + [SCRIPT-CONE let tasks: TaskRecorder; let q: Quiet; + const runCase = useRunDebugCase('sharplsp-run-target-'); setup(() => { - tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sharplsp-run-target-')); - stubs = installUiStubs(); - sessions = new DebugSessionRecorder(); - tasks = new TaskRecorder(); - q = { stubs, sessions, tasks }; - }); - - teardown(async () => { - stubs.restore(); - await stopAnyDebugSession(); - sessions.dispose(); - tasks.dispose(); - await closeAllEditors(); - removeDirRecursive(tmpDir); + const armed = runCase(); + q = armed; + ({ tmpDir, stubs, sessions, tasks } = armed); }); test('an explicit project does not borrow the focused project framework when paths differ by case', async function () { diff --git a/src/editors/vscode/src/test/suite/solution-explorer.test.ts b/src/editors/vscode/src/test/suite/solution-explorer.test.ts index f928afc7..7d8bcc63 100644 --- a/src/editors/vscode/src/test/suite/solution-explorer.test.ts +++ b/src/editors/vscode/src/test/suite/solution-explorer.test.ts @@ -22,6 +22,7 @@ import { treeContains, writeOneProjectSolution, csprojText, + pollTreeFor, } from './explorer-kit'; import { assertReachableCommand, commandEntries } from './extension-manifest-kit'; import { assertSymbolShape, assertSymbolTree, childNamed } from './lsp-invariants-kit'; @@ -161,65 +162,41 @@ suite('Solution Explorer & Workspace Symbols', () => { // ── Command Registration ───────────────────────────────────── - test('sharplsp.selectSolution command is registered', async () => { - // Interaction 1 - reachable: registered, declared once, titled, categorised. - const allCommands = await vscode.commands.getCommands(true); - assert.ok( - allCommands.includes('sharplsp.selectSolution'), - 'sharplsp.selectSolution should be registered', - ); - const entry = assertReachableCommand('sharplsp.selectSolution', allCommands); - - // Interaction 2 - [SE-COMMANDS] gives it a title and a toolbar icon. A - // command with no icon cannot appear in the view's title bar at all, which - // is the only place a user goes looking for "open a different solution". - assert.strictEqual(entry.title, 'Select Solution', 'the spec fixes the title'); - const icon: unknown = (entry as { icon?: unknown }).icon; - assert.strictEqual(icon, '$(folder-opened)', 'and the folder-opened codicon'); - assert.strictEqual(entry.category, 'SharpLsp', 'under the SharpLsp category'); - - // Interaction 3 - it is placed in the Solution Explorer's title bar, and - // unconditionally: [SE-COMMANDS] marks it "Always". - const placements = viewTitleMenu().filter((item) => item.command === 'sharplsp.selectSolution'); - assert.strictEqual(placements.length, 1, 'placed in view/title exactly once'); - assert.ok( - (placements[0]?.when ?? '').includes('sharplsp.solutionExplorer'), - `scoped to the Solution Explorer view; got: ${String(placements[0]?.when)}`, - ); - assert.ok( - !(placements[0]?.when ?? '').includes('sortOrder'), - 'and never gated on the sort mode - it is always available', - ); - }); - - test('sharplsp.refreshExplorer command is registered', async () => { - // Interaction 1 - reachable from the palette and the manifest alike. - const allCommands = await vscode.commands.getCommands(true); - assert.ok( - allCommands.includes('sharplsp.refreshExplorer'), - 'sharplsp.refreshExplorer should be registered', - ); - const entry = assertReachableCommand('sharplsp.refreshExplorer', allCommands); - - // Interaction 2 - [SE-COMMANDS]: "Refresh Explorer", with the refresh - // codicon. Refresh is the user's manual escape hatch when reactivity has - // not caught up, so it has to be visible without opening the palette. - assert.strictEqual(entry.title, 'Refresh Explorer', 'the spec fixes the title'); - assert.strictEqual((entry as { icon?: unknown }).icon, '$(refresh)', 'and the refresh codicon'); - assert.strictEqual(entry.category, 'SharpLsp', 'under the SharpLsp category'); - - // Interaction 3 - it sits in the view title bar, always, next to Select - // Solution rather than behind a sort-mode condition. - const placements = viewTitleMenu().filter( - (item) => item.command === 'sharplsp.refreshExplorer', - ); - assert.strictEqual(placements.length, 1, 'placed in view/title exactly once'); - assert.ok( - (placements[0]?.when ?? '').includes('sharplsp.solutionExplorer'), - 'scoped to the Solution Explorer view', - ); - assert.ok(!(placements[0]?.when ?? '').includes('sortOrder'), 'and is always available'); - }); + // [SE-COMMANDS] marks both "Always" and gives each a title and a toolbar + // codicon. A command with no icon cannot appear in the view's title bar at + // all — the only place a user goes looking for "open a different solution", + // and Refresh is the manual escape hatch when reactivity has not caught up. + const ALWAYS_IN_TITLE_BAR = [ + { id: 'sharplsp.selectSolution', title: 'Select Solution', icon: '$(folder-opened)' }, + { id: 'sharplsp.refreshExplorer', title: 'Refresh Explorer', icon: '$(refresh)' }, + ] as const; + + for (const { id, title, icon } of ALWAYS_IN_TITLE_BAR) { + test(`${id} command is registered`, async () => { + // Interaction 1 - reachable: registered, declared once, titled, categorised. + const allCommands = await vscode.commands.getCommands(true); + assert.ok(allCommands.includes(id), `${id} should be registered`); + const entry = assertReachableCommand(id, allCommands); + + // Interaction 2 - the spec's title and codicon, under the SharpLsp category. + assert.strictEqual(entry.title, title, 'the spec fixes the title'); + assert.strictEqual((entry as { icon?: unknown }).icon, icon, `and the ${icon} codicon`); + assert.strictEqual(entry.category, 'SharpLsp', 'under the SharpLsp category'); + + // Interaction 3 - placed in the Solution Explorer's title bar, and + // unconditionally: never gated on the sort mode. + const placements = viewTitleMenu().filter((item) => item.command === id); + assert.strictEqual(placements.length, 1, 'placed in view/title exactly once'); + assert.ok( + (placements[0]?.when ?? '').includes('sharplsp.solutionExplorer'), + `scoped to the Solution Explorer view; got: ${String(placements[0]?.when)}`, + ); + assert.ok( + !(placements[0]?.when ?? '').includes('sortOrder'), + 'and never gated on the sort mode - it is always available', + ); + }); + } for (const cmd of SORT_COMMANDS) { test(`${cmd} command is registered`, async function () { @@ -1211,11 +1188,7 @@ EndGlobal`, // Load solution — tree should show "DiskVersion". await provider.loadSolution(slnPath); - const hasDisk = await pollUntilResult( - async () => treeContains(provider.getChildren(), 'DiskVersion'), - (found) => found, - 5_000, - ); + const hasDisk = await pollTreeFor(provider, 'DiskVersion'); assert.ok(hasDisk, "Tree must show 'DiskVersion' initially"); // Edit the buffer WITHOUT saving — rename to "BufferVersion". @@ -1229,11 +1202,7 @@ EndGlobal`, await provider.refresh(); // Give a moment for the tree to rebuild from the signal. - const hasBuffer = await pollUntilResult( - async () => treeContains(provider.getChildren(), 'BufferVersion'), - (found) => found, - 5_000, - ); + const hasBuffer = await pollTreeFor(provider, 'BufferVersion'); provider.clear(); @@ -1273,11 +1242,7 @@ EndGlobal`, // After all edits, explicitly refresh and check the FINAL state. await provider.refresh(); - const hasFinal = await pollUntilResult( - async () => treeContains(provider.getChildren(), 'Step3'), - (found) => found, - 5_000, - ); + const hasFinal = await pollTreeFor(provider, 'Step3'); provider.clear(); @@ -1310,11 +1275,7 @@ EndGlobal`, // Load solution into tree and verify "Alpha" appears. await provider.loadSolution(slnPath); - const hasAlpha = await pollUntilResult( - async () => treeContains(provider.getChildren(), 'Alpha'), - (found) => found, - 5_000, - ); + const hasAlpha = await pollTreeFor(provider, 'Alpha'); assert.ok(hasAlpha, "Tree must show 'Alpha' before rename"); assert.ok( @@ -1333,11 +1294,7 @@ EndGlobal`, await replaceDocumentContent(doc, renamed); // Wait for debounced auto-refresh — tree must show "Bravo". - const hasBravo = await pollUntilResult( - async () => treeContains(provider.getChildren(), 'Bravo'), - (found) => found, - 5_000, - ); + const hasBravo = await pollTreeFor(provider, 'Bravo'); // Clean up tree state for other tests. provider.clear(); diff --git a/src/editors/vscode/src/test/suite/test-explorer-cancellation.test.ts b/src/editors/vscode/src/test/suite/test-explorer-cancellation.test.ts index 74405f95..9dbe54f0 100644 --- a/src/editors/vscode/src/test/suite/test-explorer-cancellation.test.ts +++ b/src/editors/vscode/src/test/suite/test-explorer-cancellation.test.ts @@ -51,6 +51,7 @@ import { activateWithScratch, teardownFixtureSolution, assertLeavesAre, + assertOneProfile, } from './test-explorer-kit'; import { cachedFor, itemsFor, sorted, runIds } from './test-explorer-outcome-assertions'; import { pollUntilResult, removeDirRecursive, sleep } from './test-helpers.js'; @@ -572,11 +573,9 @@ suite('Test Explorer e2e — pressing Stop kills the run', () => { vscode.TestRunProfileKind.Coverage, 'the Coverage profile is still registered after being cancelled', ); - assert.strictEqual( - api.testController.profiles.filter( - (profile) => profile.kind === vscode.TestRunProfileKind.Coverage, - ).length, - 1, + assertOneProfile( + api.testController, + vscode.TestRunProfileKind.Coverage, 'and there is still exactly one of it', ); assertTreeIntact('with the tree intact'); @@ -589,11 +588,9 @@ suite('Test Explorer e2e — pressing Stop kills the run', () => { 'no report is readable after the kill', ); assert.deepStrictEqual(reportDirsOf(coverageDir), [], 'and no run-id folder holds one'); - assert.strictEqual( - api.testController.profiles.filter( - (profile) => profile.kind === vscode.TestRunProfileKind.Coverage, - ).length, - 1, + assertOneProfile( + api.testController, + vscode.TestRunProfileKind.Coverage, 'the Coverage profile survived being cancelled', ); assert.ok( diff --git a/src/editors/vscode/src/test/suite/test-explorer-coverage.test.ts b/src/editors/vscode/src/test/suite/test-explorer-coverage.test.ts index b2640a0c..54d08555 100644 --- a/src/editors/vscode/src/test/suite/test-explorer-coverage.test.ts +++ b/src/editors/vscode/src/test/suite/test-explorer-coverage.test.ts @@ -66,6 +66,7 @@ import { runViaProfile, assertLeavesAre, profilesOf, + assertOneProfile, } from './test-explorer-kit'; import { assertFailed, @@ -1038,11 +1039,9 @@ suite('Test Explorer e2e — the Coverage profile [TEST-COVERAGE]', () => { profileOfKind(api.testController, vscode.TestRunProfileKind.Coverage), 'Run must not be the Coverage profile wearing another label', ); - assert.strictEqual( - api.testController.profiles.filter( - (profile) => profile.kind === vscode.TestRunProfileKind.Coverage, - ).length, - 1, + assertOneProfile( + api.testController, + vscode.TestRunProfileKind.Coverage, 'and there is exactly ONE Coverage profile, or the menu gesture is ambiguous', ); for (const id of PASSING) { @@ -1068,11 +1067,9 @@ suite('Test Explorer e2e — the Coverage profile [TEST-COVERAGE]', () => { for (const id of PASSING) { assertPassed(cachedFor(api, id), id); } - assert.strictEqual( - api.testController.profiles.filter( - (profile) => profile.kind === vscode.TestRunProfileKind.Run, - ).length, - 1, + assertOneProfile( + api.testController, + vscode.TestRunProfileKind.Run, 'with exactly ONE plain Run profile behind the gesture', ); assert.ok( @@ -1591,11 +1588,9 @@ suite('Test Explorer e2e — the Coverage profile [TEST-COVERAGE]', () => { } // Interaction 4 - the Debug profile is a diagnostic, not a measurement. It // must leave the results directory exactly as the previous run left it. - assert.strictEqual( - api.testController.profiles.filter( - (profile) => profile.kind === vscode.TestRunProfileKind.Debug, - ).length, - 1, + assertOneProfile( + api.testController, + vscode.TestRunProfileKind.Debug, 'exactly one Debug profile, or the gesture is ambiguous in the menu', ); assert.notStrictEqual( @@ -1626,11 +1621,9 @@ suite('Test Explorer e2e — the Coverage profile [TEST-COVERAGE]', () => { TEST_PROJECTS, 'and no run-id folder beyond those', ); - assert.strictEqual( - api.testController.profiles.filter( - (profile) => profile.kind === vscode.TestRunProfileKind.Debug, - ).length, - 1, + assertOneProfile( + api.testController, + vscode.TestRunProfileKind.Debug, 'exactly one Debug profile is registered', ); assert.ok( diff --git a/src/editors/vscode/src/test/suite/test-explorer-e2e.test.ts b/src/editors/vscode/src/test/suite/test-explorer-e2e.test.ts index 9297540a..f53d2a9b 100644 --- a/src/editors/vscode/src/test/suite/test-explorer-e2e.test.ts +++ b/src/editors/vscode/src/test/suite/test-explorer-e2e.test.ts @@ -39,12 +39,10 @@ import { warmDiscovery, writeProject, } from './dotnet-project-kit'; -import { fixtureFor } from './test-explorer-fixtures'; import { assertDeclaredInside, assertLeafItem, collectItemIds, - drainDiscovery, errorTextOf, findItem, pollForIds, @@ -58,6 +56,9 @@ import { assertLeavesAre, assertPlainLeaf, announcedPair, + rootsOf, + discoverSolution, + unloadSolution, } from './test-explorer-kit'; import { findTestByMethodName } from '../../test-lens.js'; import { @@ -69,30 +70,14 @@ import { } from './test-helpers.js'; import { DOTNET_CLI_MS, FAST_MS, FIXTURE_BUILD_MS } from './test-timeouts'; import { sorted } from './test-explorer-outcome-assertions'; - -const CS = fixtureFor('xunit-csharp'); -const FS_FIXTURE = fixtureFor('xunit-fsharp'); - -/** The idiomatic F# backtick binding whose xUnit FQN literally contains spaces. */ -const FS_FACT_SPACED = 'Fs.Xunit.Fixtures.adds two numbers with spaces'; -/** The theories whose two rows DISAGREE — both report under this one FQN. */ -const FS_MIXED_THEORY = 'Fs.Xunit.Fixtures.mixed theory'; -const CS_MIXED_THEORY = 'Cs.Xunit.Fixtures.CalculatorTests.Mixed_Theory'; - -/** EXHAUSTIVELY every FQN the two xUnit fixtures expose: six F# (first), five C#. */ -const EXPECTED = [ - FS_FIXTURE.passing, +import { + CS, + FS_FIXTURE, FS_FACT_SPACED, - FS_FIXTURE.failing, - FS_FIXTURE.skipped, - FS_FIXTURE.parameterized, FS_MIXED_THEORY, - CS.passing, - CS.failing, - CS.skipped, - CS.parameterized, CS_MIXED_THEORY, -] as const; + XUNIT_PAIR_IDS as EXPECTED, +} from './test-explorer-xunit-pair'; /** The ONLY names `--list-tests` prints — it prints DISPLAY names, which for an * xUnit [Fact] equal the FQN, while a [Theory]'s carries row arguments whose @@ -505,9 +490,7 @@ suite('Test Explorer e2e — real C#/F# discovery', () => { teardown(async () => { // Reset the loaded solution so each test's load is a real transition that // re-fires reactive discovery, and let the debounced sweep settle first. - await drainDiscovery(() => { - api.explorerProvider.clear(); - }, api.testController); + await unloadSolution(api); }); suiteTeardown(async function () { @@ -980,22 +963,7 @@ suite('Test Explorer e2e — real C#/F# discovery', () => { // The bug this guards: a failed sweep used to leave the view BLANK — no // tests, no reason, nothing to act on. It must instead surface an error // item carrying the real diagnostic AND a remedy. - const failure = await awaitSingleErrorRow(); - const failureRoots: vscode.TestItem[] = []; - api.testController.items.forEach((item) => failureRoots.push(item)); - assert.strictEqual( - failureRoots.length, - 1, - `exactly one row stands after the failed sweep, got: ${failureRoots - .map((item) => item.label) - .join(' | ')}`, - ); - assert.notStrictEqual( - failure.error, - undefined, - 'the failure row carries an error — that is what the Testing view renders', - ); - const message = errorTextOf(failure); + const { failure, message } = await assertOnlyErrorRow('a missing target'); assert.ok( message.includes(ghost), `the error must NAME the missing target so the user knows what to fix: ${message}`, @@ -1009,38 +977,18 @@ suite('Test Explorer e2e — real C#/F# discovery', () => { `the error must offer a remedy the user can act on: ${message}`, ); assert.ok(!EXPECTED_SET.has(failure.id), 'the failure row is not a test'); - assert.strictEqual(failure.children.size, 0, 'the failure row is a leaf'); - assert.deepStrictEqual( - collectLeafIds(api.testController.items).filter((id) => EXPECTED_SET.has(id)), - [], - 'a missing target contributes NO test items — none are invented', - ); assert.strictEqual( findItem(api.testController.items, CS.passing), undefined, 'no stale item survives a failed sweep over an empty tree', ); // And the extension recovers: pointing back at the real solution refills it. - await api.explorerProvider.loadSolution(slnPath); - await api.testController.activateAndDiscover(); - await pollUntilDiscovered(api.testController, EXPECTED); - const recoveredLeaves = assertHierarchyTree(api.testController.items); - assertExactTree(recoveredLeaves, 'recovery after a missing target'); + const recoveredLeaves = await recoverTree('recovery after a missing target'); assert.strictEqual( api.testController.items.size, 2, 'recovery rebuilds the two assembly roots and their whole hierarchy', ); - const errorsAfter: string[] = []; - api.testController.items.forEach(function collect(item) { - if (item.error !== undefined) errorsAfter.push(item.id); - item.children.forEach(collect); - }); - assert.deepStrictEqual( - errorsAfter, - [], - 'the stale error row is gone once discovery succeeds again', - ); assertContainsAll(recoveredLeaves, [FS_FIXTURE.passing, CS.passing], 'recoveredLeaves'); for (const snapshot of snapshotItems(api.testController.items).filter( (s) => s.childCount === 0, @@ -1187,8 +1135,7 @@ suite('Test Explorer e2e — real C#/F# discovery', () => { async function awaitSingleErrorRow(): Promise { const deadline = Date.now() + FIXTURE_BUILD_MS; for (;;) { - const roots: vscode.TestItem[] = []; - api.testController.items.forEach((item) => roots.push(item)); + const roots = rootsOf(api.testController.items); const failure = roots.find((item) => item.error !== undefined); if (roots.length === 1 && failure !== undefined) return failure; if (Date.now() > deadline) { @@ -1202,6 +1149,40 @@ suite('Test Explorer e2e — real C#/F# discovery', () => { } } + /** + * The failed sweep's ONE error row, re-read from the tree: the only root, + * carrying an error, a leaf, and never a test — no test is invented for a + * target that could not be enumerated. Returns the row and its error text. + */ + async function assertOnlyErrorRow( + why: string, + ): Promise<{ failure: vscode.TestItem; message: string }> { + const failure = await awaitSingleErrorRow(); + const roots = rootsOf(api.testController.items); + const labels = roots.map((item) => item.label).join(' | '); + assert.strictEqual(roots.length, 1, `${why}: exactly one row stands, got: ${labels}`); + assert.notStrictEqual(failure.error, undefined, `${why}: the row carries an error to render`); + assert.strictEqual(failure.children.size, 0, `${why}: the error row is a leaf`); + assert.deepStrictEqual( + collectLeafIds(api.testController.items).filter((id) => EXPECTED_SET.has(id)), + [], + `${why}: no test is invented for a target that could not be enumerated`, + ); + return { failure, message: errorTextOf(failure) }; + } + + /** Point discovery back at the real solution: the whole tree returns, no error row left. */ + async function recoverTree(why: string): Promise { + await discoverSolution(api, slnPath, EXPECTED); + const leaves = assertHierarchyTree(api.testController.items); + assertExactTree(leaves, why); + const errorsAfter = collectItemIds(api.testController.items).filter( + (id) => findItem(api.testController.items, id)?.error !== undefined, + ); + assert.deepStrictEqual(errorsAfter, [], `${why}: the stale error row is gone`); + return leaves; + } + /** The FQNs whose dotted split places them in `namespaceLabel`. */ function fqnsOfNamespace(namespaceLabel: string): string[] { return EXPECTED.filter((fqn) => fqn.split('.').slice(0, -2).join('.') === namespaceLabel); @@ -1835,29 +1816,12 @@ suite('Test Explorer e2e — real C#/F# discovery', () => { // silent empty view. The tree must carry a USEFUL error instead. const folder = vscode.workspace.workspaceFolders?.[0]?.uri.fsPath; assert.ok(folder, 'the test host must open a workspace folder for this case'); - await drainDiscovery(() => { - api.explorerProvider.clear(); - }, api.testController); + await unloadSolution(api); api.testController.items.replace([]); await assert.doesNotReject(async () => { await api.testController.activateAndDiscover(); }, 'ambiguous-folder discovery must not reject'); - const failure = await awaitSingleErrorRow(); - const failureRoots: vscode.TestItem[] = []; - api.testController.items.forEach((item) => failureRoots.push(item)); - assert.strictEqual( - failureRoots.length, - 1, - `one error row for the ambiguous folder, got: ${failureRoots - .map((item) => item.label) - .join(' | ')}`, - ); - assert.notStrictEqual( - failure.error, - undefined, - 'the row carries an error for the view to render', - ); - const message = errorTextOf(failure); + const { message } = await assertOnlyErrorRow('the ambiguous folder'); assert.ok( message.includes('MSB1011'), `the error must carry the REAL dotnet diagnostic, not a shrug: ${message.slice(0, 400)}`, @@ -1870,25 +1834,9 @@ suite('Test Explorer e2e — real C#/F# discovery', () => { /select solution/i.test(message), `and offer the remedy — load one solution: ${message.slice(0, 400)}`, ); - assert.strictEqual(failure.children.size, 0, 'the error row is a leaf'); - assert.deepStrictEqual( - collectLeafIds(api.testController.items).filter((id) => EXPECTED_SET.has(id)), - [], - 'no tests are invented for a folder that could not be enumerated', - ); // Recovery: loading a solution makes the error row vanish and the full // hierarchy appear — the user's exact escape route from the message. - await api.explorerProvider.loadSolution(slnPath); - await api.testController.activateAndDiscover(); - await pollUntilDiscovered(api.testController, EXPECTED); - const leaves = assertHierarchyTree(api.testController.items); - assertExactTree(leaves, 'recovery from the ambiguous folder'); - const errorsAfter: string[] = []; - api.testController.items.forEach(function collect(item) { - if (item.error !== undefined) errorsAfter.push(item.id); - item.children.forEach(collect); - }); - assert.deepStrictEqual(errorsAfter, [], 'the MSB1011 error row is gone after recovery'); + await recoverTree('recovery from the ambiguous MSB1011 folder'); }); test('a project that fails to BUILD surfaces the compiler error in the tree, and ▶ fabricates nothing', async function () { @@ -1910,21 +1858,7 @@ suite('Test Explorer e2e — real C#/F# discovery', () => { api.testController.items.replace([]); await api.explorerProvider.loadSolution(brokenSln); await api.testController.activateAndDiscover(); - const failure = await awaitSingleErrorRow(); - const failureRoots: vscode.TestItem[] = []; - api.testController.items.forEach((item) => failureRoots.push(item)); - assert.strictEqual( - failureRoots.length, - 1, - `one error row for the broken build, got: ${failureRoots - .map((item) => item.label) - .join(' | ')}`, - ); - assert.notStrictEqual(failure.error, undefined, 'the row carries an error'); - const message = - failure.error instanceof vscode.MarkdownString - ? failure.error.value - : String(failure.error); + const { failure, message } = await assertOnlyErrorRow('the broken build'); assert.ok( /error CS\d+/.test(message), `the error must carry the COMPILER diagnostic (error CS…), not a generic failure: ${message.slice( @@ -1932,12 +1866,6 @@ suite('Test Explorer e2e — real C#/F# discovery', () => { 400, )}`, ); - assert.strictEqual(failure.children.size, 0, 'the error row is a leaf'); - assert.deepStrictEqual( - collectLeafIds(api.testController.items).filter((id) => EXPECTED_SET.has(id)), - [], - 'no tests are invented for a project that cannot build', - ); // ▶ gestures on an error-only tree must neither crash nor fabricate // results: there is nothing runnable to report. await assert.doesNotReject(async () => { diff --git a/src/editors/vscode/src/test/suite/test-explorer-fixtures.ts b/src/editors/vscode/src/test/suite/test-explorer-fixtures.ts index f9e0a44f..84385a38 100644 --- a/src/editors/vscode/src/test/suite/test-explorer-fixtures.ts +++ b/src/editors/vscode/src/test/suite/test-explorer-fixtures.ts @@ -56,65 +56,74 @@ export interface FrameworkFixture { readonly mixedParameterized?: string; } -const CS_XUNIT_SOURCE = [ - 'using Xunit;', - '', - 'namespace Cs.Xunit.Fixtures', - '{', - ' public class CalculatorTests', - ' {', - ' [Fact] public void Adds_TwoNumbers() => Assert.Equal(3, 1 + 2);', - ' [Fact] public void Fails_OnPurpose() => Assert.Equal(4, 1 + 2);', - ' [Fact(Skip = "fixture: deliberately skipped")] public void Skipped_OnPurpose() { }', - ' [Theory]', - ' [InlineData(2, 2, 4)]', - ' [InlineData(1, 1, 2)]', - ' public void Adds_Theory(int a, int b, int expected) => Assert.Equal(expected, a + b);', - ' [Theory]', - ' [InlineData(2, 2, 4)]', - ' [InlineData(1, 1, 99)]', - ' public void Mixed_Theory(int a, int b, int expected) => Assert.Equal(expected, a + b);', - ' }', - '}', - '', -].join('\n'); +/** The C# xUnit fixture: a pass, a failure, a skip, and two theories. */ +export function csXunitSource(namespace: string): string { + return [ + 'using Xunit;', + '', + namespace, + '{', + ' public class CalculatorTests', + ' {', + ' [Fact] public void Adds_TwoNumbers() => Assert.Equal(3, 1 + 2);', + ' [Fact] public void Fails_OnPurpose() => Assert.Equal(4, 1 + 2);', + ' [Fact(Skip = "fixture: deliberately skipped")] public void Skipped_OnPurpose() { }', + ' [Theory]', + ' [InlineData(2, 2, 4)]', + ' [InlineData(1, 1, 2)]', + ' public void Adds_Theory(int a, int b, int expected) => Assert.Equal(expected, a + b);', + ' [Theory]', + ' [InlineData(2, 2, 4)]', + ' [InlineData(1, 1, 99)]', + ' public void Mixed_Theory(int a, int b, int expected) => Assert.Equal(expected, a + b);', + ' }', + '}', + '', + ].join('\n'); +} -const CS_NUNIT_SOURCE = [ - 'using NUnit.Framework;', - '', - 'namespace Cs.Nunit.Fixtures', - '{', - ' public class CalculatorTests', - ' {', - ' [Test] public void Adds_TwoNumbers() => Assert.That(1 + 2, Is.EqualTo(3));', - ' [Test] public void Fails_OnPurpose() => Assert.That(1 + 2, Is.EqualTo(4));', - ' [Test, Ignore("fixture: deliberately skipped")] public void Skipped_OnPurpose() { }', - ' [TestCase(2, 2, 4)]', - ' public void Adds_Case(int a, int b, int expected) =>', - ' Assert.That(a + b, Is.EqualTo(expected));', - ' }', - '}', - '', -].join('\n'); +/** The C# NUnit fixture: a pass, a failure, an ignore, and a test case. */ +export function csNunitSource(namespace: string): string { + return [ + 'using NUnit.Framework;', + '', + namespace, + '{', + ' public class CalculatorTests', + ' {', + ' [Test] public void Adds_TwoNumbers() => Assert.That(1 + 2, Is.EqualTo(3));', + ' [Test] public void Fails_OnPurpose() => Assert.That(1 + 2, Is.EqualTo(4));', + ' [Test, Ignore("fixture: deliberately skipped")] public void Skipped_OnPurpose() { }', + ' [TestCase(2, 2, 4)]', + ' public void Adds_Case(int a, int b, int expected) =>', + ' Assert.That(a + b, Is.EqualTo(expected));', + ' }', + '}', + '', + ].join('\n'); +} -const CS_MSTEST_SOURCE = [ - 'using Microsoft.VisualStudio.TestTools.UnitTesting;', - '', - 'namespace Cs.Mstest.Fixtures', - '{', - ' [TestClass]', - ' public class CalculatorTests', - ' {', - ' [TestMethod] public void Adds_TwoNumbers() => Assert.AreEqual(3, 1 + 2);', - ' [TestMethod] public void Fails_OnPurpose() => Assert.AreEqual(4, 1 + 2);', - ' [TestMethod, Ignore] public void Skipped_OnPurpose() { }', - ' [DataTestMethod]', - ' [DataRow(2, 2, 4)]', - ' public void Adds_Row(int a, int b, int expected) => Assert.AreEqual(expected, a + b);', - ' }', - '}', - '', -].join('\n'); +/** The C# MSTest fixture; `rowMethod` is the attribute line above its data row. */ +export function csMstestSource(namespace: string, rowMethod: string): string { + return [ + 'using Microsoft.VisualStudio.TestTools.UnitTesting;', + '', + namespace, + '{', + ' [TestClass]', + ' public class CalculatorTests', + ' {', + ' [TestMethod] public void Adds_TwoNumbers() => Assert.AreEqual(3, 1 + 2);', + ' [TestMethod] public void Fails_OnPurpose() => Assert.AreEqual(4, 1 + 2);', + ' [TestMethod, Ignore] public void Skipped_OnPurpose() { }', + rowMethod, + ' [DataRow(2, 2, 4)]', + ' public void Adds_Row(int a, int b, int expected) => Assert.AreEqual(expected, a + b);', + ' }', + '}', + '', + ].join('\n'); +} const FS_XUNIT_SOURCE = [ 'module Fs.Xunit.Fixtures', @@ -144,24 +153,27 @@ const FS_XUNIT_SOURCE = [ '', ].join('\n'); -const FS_NUNIT_SOURCE = [ - 'module Fs.Nunit.Fixtures', - '', - 'open NUnit.Framework', - '', - '[]', - 'let addsTwoNumbers () = Assert.That(1 + 2, Is.EqualTo(3))', - '', - '[]', - 'let ``fails on purpose`` () = Assert.That(1 + 2, Is.EqualTo(4))', - '', - '[]', - 'let ``skipped on purpose`` () = ()', - '', - '[]', - 'let ``adds case`` (a: int) (b: int) (expected: int) = Assert.That(a + b, Is.EqualTo(expected))', - '', -].join('\n'); +/** The F# NUnit fixture; `passing` is the line declaring its passing test. */ +export function fsNunitSource(module: string, passing: string): string { + return [ + module, + '', + 'open NUnit.Framework', + '', + '[]', + passing, + '', + '[]', + 'let ``fails on purpose`` () = Assert.That(1 + 2, Is.EqualTo(4))', + '', + '[]', + 'let ``skipped on purpose`` () = ()', + '', + '[]', + 'let ``adds case`` (a: int) (b: int) (expected: int) = Assert.That(a + b, Is.EqualTo(expected))', + '', + ].join('\n'); +} const FS_MSTEST_SOURCE = [ 'module Fs.Mstest.Fixtures', @@ -232,37 +244,64 @@ export const FRAMEWORK_FIXTURES: readonly FrameworkFixture[] = [ parameterized: 'Fs.Xunit.Fixtures.adds theory', mixedParameterized: 'Fs.Xunit.Fixtures.mixed theory', }), - frameworkFixture('nunit', 'fsharp', NUNIT_PACKAGES, FS_NUNIT_SOURCE, { - passing: 'Fs.Nunit.Fixtures.addsTwoNumbers', - failing: 'Fs.Nunit.Fixtures.fails on purpose', - skipped: 'Fs.Nunit.Fixtures.skipped on purpose', - parameterized: 'Fs.Nunit.Fixtures.adds case(2,2,4)', - }), + frameworkFixture( + 'nunit', + 'fsharp', + NUNIT_PACKAGES, + fsNunitSource( + 'module Fs.Nunit.Fixtures', + 'let addsTwoNumbers () = Assert.That(1 + 2, Is.EqualTo(3))', + ), + { + passing: 'Fs.Nunit.Fixtures.addsTwoNumbers', + failing: 'Fs.Nunit.Fixtures.fails on purpose', + skipped: 'Fs.Nunit.Fixtures.skipped on purpose', + parameterized: 'Fs.Nunit.Fixtures.adds case(2,2,4)', + }, + ), frameworkFixture('mstest', 'fsharp', MSTEST_PACKAGES, FS_MSTEST_SOURCE, { passing: 'Fs.Mstest.Fixtures+CalculatorTests.AddsTwoNumbers', failing: 'Fs.Mstest.Fixtures+CalculatorTests.FailsOnPurpose', skipped: 'Fs.Mstest.Fixtures+CalculatorTests.SkippedOnPurpose', parameterized: 'Fs.Mstest.Fixtures+CalculatorTests.AddsRow', }), - frameworkFixture('xunit', 'csharp', XUNIT_PACKAGES, CS_XUNIT_SOURCE, { - passing: 'Cs.Xunit.Fixtures.CalculatorTests.Adds_TwoNumbers', - failing: 'Cs.Xunit.Fixtures.CalculatorTests.Fails_OnPurpose', - skipped: 'Cs.Xunit.Fixtures.CalculatorTests.Skipped_OnPurpose', - parameterized: 'Cs.Xunit.Fixtures.CalculatorTests.Adds_Theory', - mixedParameterized: 'Cs.Xunit.Fixtures.CalculatorTests.Mixed_Theory', - }), - frameworkFixture('nunit', 'csharp', NUNIT_PACKAGES, CS_NUNIT_SOURCE, { - passing: 'Cs.Nunit.Fixtures.CalculatorTests.Adds_TwoNumbers', - failing: 'Cs.Nunit.Fixtures.CalculatorTests.Fails_OnPurpose', - skipped: 'Cs.Nunit.Fixtures.CalculatorTests.Skipped_OnPurpose', - parameterized: 'Cs.Nunit.Fixtures.CalculatorTests.Adds_Case(2,2,4)', - }), - frameworkFixture('mstest', 'csharp', MSTEST_PACKAGES, CS_MSTEST_SOURCE, { - passing: 'Cs.Mstest.Fixtures.CalculatorTests.Adds_TwoNumbers', - failing: 'Cs.Mstest.Fixtures.CalculatorTests.Fails_OnPurpose', - skipped: 'Cs.Mstest.Fixtures.CalculatorTests.Skipped_OnPurpose', - parameterized: 'Cs.Mstest.Fixtures.CalculatorTests.Adds_Row', - }), + frameworkFixture( + 'xunit', + 'csharp', + XUNIT_PACKAGES, + csXunitSource('namespace Cs.Xunit.Fixtures'), + { + passing: 'Cs.Xunit.Fixtures.CalculatorTests.Adds_TwoNumbers', + failing: 'Cs.Xunit.Fixtures.CalculatorTests.Fails_OnPurpose', + skipped: 'Cs.Xunit.Fixtures.CalculatorTests.Skipped_OnPurpose', + parameterized: 'Cs.Xunit.Fixtures.CalculatorTests.Adds_Theory', + mixedParameterized: 'Cs.Xunit.Fixtures.CalculatorTests.Mixed_Theory', + }, + ), + frameworkFixture( + 'nunit', + 'csharp', + NUNIT_PACKAGES, + csNunitSource('namespace Cs.Nunit.Fixtures'), + { + passing: 'Cs.Nunit.Fixtures.CalculatorTests.Adds_TwoNumbers', + failing: 'Cs.Nunit.Fixtures.CalculatorTests.Fails_OnPurpose', + skipped: 'Cs.Nunit.Fixtures.CalculatorTests.Skipped_OnPurpose', + parameterized: 'Cs.Nunit.Fixtures.CalculatorTests.Adds_Case(2,2,4)', + }, + ), + frameworkFixture( + 'mstest', + 'csharp', + MSTEST_PACKAGES, + csMstestSource('namespace Cs.Mstest.Fixtures', ' [DataTestMethod]'), + { + passing: 'Cs.Mstest.Fixtures.CalculatorTests.Adds_TwoNumbers', + failing: 'Cs.Mstest.Fixtures.CalculatorTests.Fails_OnPurpose', + skipped: 'Cs.Mstest.Fixtures.CalculatorTests.Skipped_OnPurpose', + parameterized: 'Cs.Mstest.Fixtures.CalculatorTests.Adds_Row', + }, + ), ]; /** diff --git a/src/editors/vscode/src/test/suite/test-explorer-frameworks.test.ts b/src/editors/vscode/src/test/suite/test-explorer-frameworks.test.ts index 360a3360..bbcf0f95 100644 --- a/src/editors/vscode/src/test/suite/test-explorer-frameworks.test.ts +++ b/src/editors/vscode/src/test/suite/test-explorer-frameworks.test.ts @@ -43,18 +43,12 @@ import { nextResultsChange, snapshotItems, type TestItemSnapshot, - activateWithScratch, - teardownFixtureSolution, assertPlainLeaf, } from './test-explorer-kit'; -import { - comparablePath, - removeDirRecursive, - assertContainsAll, - assertContainsNone, -} from './test-helpers'; -import { DOTNET_CLI_MS, FIXTURE_BUILD_MS } from './test-timeouts'; +import { comparablePath, assertContainsAll, assertContainsNone } from './test-helpers'; +import { DOTNET_CLI_MS } from './test-timeouts'; import { sorted } from './test-explorer-outcome-assertions'; +import { useScratchSuite } from './test-explorer-harness'; /** The idiomatic F# backtick fact whose xUnit FQN literally contains spaces. */ const FS_SPACED_FACT = 'Fs.Xunit.Fixtures.adds two numbers with spaces'; @@ -484,9 +478,8 @@ suite('Test Explorer e2e — xUnit, NUnit and MSTest across C# and F#', () => { return ids; } - suiteSetup(async function () { - this.timeout(FIXTURE_BUILD_MS); - ({ api, root } = await activateWithScratch('sharplsp-frameworks-')); + useScratchSuite('sharplsp-frameworks-', async (scratch) => { + ({ api, root } = scratch); projectDirs = writeAllProjects(root); slnPath = await createSolution(root, 'Frameworks', [...projectDirs.values()]); // Warm the FULL discovery path once — six restores, six builds, three VSTest @@ -495,17 +488,6 @@ suite('Test Explorer e2e — xUnit, NUnit and MSTest across C# and F#', () => { listing = await warmDiscovery(slnPath, root); }); - teardown(async () => { - // Never leave a `dotnet` invocation in flight across tests: discovery builds - // the same `bin/`/`obj/` a run rebuilds, and the overlap kills VSTest. - await api.testController.whenIdle(); - }); - - suiteTeardown(async function () { - this.timeout(DOTNET_CLI_MS); - await teardownFixtureSolution(api, root, removeDirRecursive); - }); - test('discovery finds every test in all six framework × language fixtures', async function () { this.timeout(DOTNET_CLI_MS); api.testController.items.replace([]); diff --git a/src/editors/vscode/src/test/suite/test-explorer-goto-test.test.ts b/src/editors/vscode/src/test/suite/test-explorer-goto-test.test.ts index 099a6960..6ea5f716 100644 --- a/src/editors/vscode/src/test/suite/test-explorer-goto-test.test.ts +++ b/src/editors/vscode/src/test/suite/test-explorer-goto-test.test.ts @@ -32,10 +32,10 @@ import { } from './dotnet-project-kit'; import { discoverSolution, - drainDiscovery, findItem, activateWithScratch, teardownFixtureSolution, + unloadSolution, } from './test-explorer-kit'; import { closeAllEditors, @@ -293,9 +293,7 @@ suite('Test Explorer — Go to Test reveals the declaring source', () => { teardown(async () => { await closeAllEditors(); - await drainDiscovery(() => { - api.explorerProvider.clear(); - }, api.testController); + await unloadSolution(api); }); suiteTeardown(async function () { diff --git a/src/editors/vscode/src/test/suite/test-explorer-harness.ts b/src/editors/vscode/src/test/suite/test-explorer-harness.ts index 3d22363f..d821807a 100644 --- a/src/editors/vscode/src/test/suite/test-explorer-harness.ts +++ b/src/editors/vscode/src/test/suite/test-explorer-harness.ts @@ -16,6 +16,7 @@ import { drainDiscovery, pollUntilDiscovered, teardownFixtureSolution, + clearTestTree, } from './test-explorer-kit'; import { removeDirRecursive } from './test-helpers'; import { DOTNET_CLI_MS, FIXTURE_BUILD_MS } from './test-timeouts'; @@ -83,3 +84,58 @@ export function useWarmFixture( return fixture; }; } + +/** A suite's activated Test Explorer and the scratch root its fixture lives under. */ +export interface ScratchSuite { + readonly api: SharpLspExtensionApi; + readonly root: string; +} + +/** + * Activate the Test Explorer into a fresh scratch root (`prefix`) and hand it + * to `build` once for the suite. After each test no `dotnet` invocation may be + * left in flight — discovery builds the same `bin/`/`obj/` a run rebuilds, and + * the overlap kills VSTest — and the suite ends with `teardownFixtureSolution`. + */ +export function useScratchSuite( + prefix: string, + build: (scratch: ScratchSuite) => Promise, +): void { + let scratch: ScratchSuite | undefined; + suiteSetup(async function () { + this.timeout(FIXTURE_BUILD_MS); + scratch = await activateWithScratch(prefix); + await build(scratch); + }); + teardown(async () => { + await scratch?.api.testController.whenIdle(); + }); + suiteTeardown(async function () { + this.timeout(DOTNET_CLI_MS); + if (scratch !== undefined) + await teardownFixtureSolution(scratch.api, scratch.root, removeDirRecursive); + }); +} + +/** + * A suite whose tests each build their own solution under ONE scratch parent + * (`prefix`): the tree is cleared after every test, and the parent removed + * once the suite is done. + */ +export function useScratchParent(prefix: string): () => ScratchSuite { + let scratch: ScratchSuite | undefined; + suiteSetup(async function () { + this.timeout(FIXTURE_BUILD_MS); + scratch = await activateWithScratch(prefix); + }); + teardown(async () => { + if (scratch !== undefined) await clearTestTree(scratch.api); + }); + suiteTeardown(() => { + if (scratch !== undefined) removeDirRecursive(scratch.root); + }); + return () => { + assert.ok(scratch, 'the scratch parent must be created in suiteSetup'); + return scratch; + }; +} diff --git a/src/editors/vscode/src/test/suite/test-explorer-kit.ts b/src/editors/vscode/src/test/suite/test-explorer-kit.ts index 7897d2ee..195ca31d 100644 --- a/src/editors/vscode/src/test/suite/test-explorer-kit.ts +++ b/src/editors/vscode/src/test/suite/test-explorer-kit.ts @@ -258,6 +258,13 @@ export async function drainDiscovery( await controller.whenIdle(); } +/** Unload every solution, then let re-discovery settle. */ +export async function unloadSolution(api: SharpLspExtensionApi): Promise { + await drainDiscovery(() => { + api.explorerProvider.clear(); + }, api.testController); +} + /** Unload every solution and empty the tree, then let re-discovery settle. */ export async function clearTestTree(api: SharpLspExtensionApi): Promise { await drainDiscovery(() => { @@ -307,6 +314,15 @@ export function profileOfKind( return profile; } +/** Exactly ONE profile of `kind` is registered: two make the gesture ambiguous. */ +export function assertOneProfile( + controller: SharpLspTestController, + kind: vscode.TestRunProfileKind, + why: string, +): void { + assert.strictEqual(controller.profiles.filter((profile) => profile.kind === kind).length, 1, why); +} + /** The Run, Debug and Coverage profiles, each asserted registered. */ export function profilesOf(controller: SharpLspTestController): { runProfile: vscode.TestRunProfile; diff --git a/src/editors/vscode/src/test/suite/test-explorer-mtp-batches.test.ts b/src/editors/vscode/src/test/suite/test-explorer-mtp-batches.test.ts index 10b26169..3d25799a 100644 --- a/src/editors/vscode/src/test/suite/test-explorer-mtp-batches.test.ts +++ b/src/editors/vscode/src/test/suite/test-explorer-mtp-batches.test.ts @@ -30,16 +30,11 @@ import { writeMtpGlobalJson, writeProject, } from './dotnet-project-kit'; -import { - discoverSolution, - rootsOf, - runViaProfile, - teardownFixtureSolution, - activateWithScratch, -} from './test-explorer-kit'; +import { discoverSolution, rootsOf, runViaProfile } from './test-explorer-kit'; import { assertPassed, cachedFor, itemsFor } from './test-explorer-outcome-assertions'; -import { removeDirRecursive, assertContainsAll } from './test-helpers'; -import { DOTNET_CLI_MS, FIXTURE_BUILD_MS } from './test-timeouts'; +import { assertContainsAll } from './test-helpers'; +import { FIXTURE_BUILD_MS } from './test-timeouts'; +import { useScratchSuite } from './test-explorer-harness'; const PROJECT = 'BatchMtpFs'; const MODULE = 'Fs.BatchMtp.Fixtures'; @@ -97,9 +92,8 @@ suite('Test Explorer e2e — Microsoft.Testing.Platform selections past one invo let root: string; let slnPath: string; - suiteSetup(async function () { - this.timeout(FIXTURE_BUILD_MS); - ({ api, root } = await activateWithScratch('sharplsp-mtp-batches-')); + useScratchSuite('sharplsp-mtp-batches-', async (scratch) => { + ({ api, root } = scratch); writeMtpGlobalJson(root); const dir = writeProject( path.join(root, PROJECT), @@ -112,15 +106,6 @@ suite('Test Explorer e2e — Microsoft.Testing.Platform selections past one invo await discoverSolution(api, slnPath, EXPECTED); }); - teardown(async () => { - await api.testController.whenIdle(); - }); - - suiteTeardown(async function () { - this.timeout(DOTNET_CLI_MS); - await teardownFixtureSolution(api, root, removeDirRecursive); - }); - test('a refusal in a LATER batch is still retried, whatever an earlier batch reported', async function () { this.timeout(FIXTURE_BUILD_MS); const module = await batchModule(slnPath, root); diff --git a/src/editors/vscode/src/test/suite/test-explorer-mtp-fixtures.ts b/src/editors/vscode/src/test/suite/test-explorer-mtp-fixtures.ts index be7025f2..6b903358 100644 --- a/src/editors/vscode/src/test/suite/test-explorer-mtp-fixtures.ts +++ b/src/editors/vscode/src/test/suite/test-explorer-mtp-fixtures.ts @@ -33,7 +33,13 @@ import { MTP_XUNIT_PACKAGES, type PackageRef, } from './dotnet-project-kit'; -import { fixtureNames } from './test-explorer-fixtures'; +import { + fixtureNames, + csXunitSource, + csNunitSource, + csMstestSource, + fsNunitSource, +} from './test-explorer-fixtures'; /** One buildable MTP fixture project plus the ids it is expected to expose. */ export interface MtpFixture { @@ -95,29 +101,6 @@ const FS_XUNIT_SOURCE = [ '', ].join('\n'); -const CS_XUNIT_SOURCE = [ - 'using Xunit;', - '', - 'namespace Cs.XunitMtp.Fixtures', - '{', - ' public class CalculatorTests', - ' {', - ' [Fact] public void Adds_TwoNumbers() => Assert.Equal(3, 1 + 2);', - ' [Fact] public void Fails_OnPurpose() => Assert.Equal(4, 1 + 2);', - ' [Fact(Skip = "fixture: deliberately skipped")] public void Skipped_OnPurpose() { }', - ' [Theory]', - ' [InlineData(2, 2, 4)]', - ' [InlineData(1, 1, 2)]', - ' public void Adds_Theory(int a, int b, int expected) => Assert.Equal(expected, a + b);', - ' [Theory]', - ' [InlineData(2, 2, 4)]', - ' [InlineData(1, 1, 99)]', - ' public void Mixed_Theory(int a, int b, int expected) => Assert.Equal(expected, a + b);', - ' }', - '}', - '', -].join('\n'); - const FS_MSTEST_SOURCE = [ 'module Fs.MstestMtp.Fixtures', '', @@ -139,62 +122,6 @@ const FS_MSTEST_SOURCE = [ '', ].join('\n'); -const CS_MSTEST_SOURCE = [ - 'using Microsoft.VisualStudio.TestTools.UnitTesting;', - '', - 'namespace Cs.MstestMtp.Fixtures', - '{', - ' [TestClass]', - ' public class CalculatorTests', - ' {', - ' [TestMethod] public void Adds_TwoNumbers() => Assert.AreEqual(3, 1 + 2);', - ' [TestMethod] public void Fails_OnPurpose() => Assert.AreEqual(4, 1 + 2);', - ' [TestMethod, Ignore] public void Skipped_OnPurpose() { }', - ' [TestMethod]', - ' [DataRow(2, 2, 4)]', - ' public void Adds_Row(int a, int b, int expected) => Assert.AreEqual(expected, a + b);', - ' }', - '}', - '', -].join('\n'); - -const FS_NUNIT_SOURCE = [ - 'module Fs.NunitMtp.Fixtures', - '', - 'open NUnit.Framework', - '', - '[]', - 'let ``adds two numbers with spaces`` () = Assert.That(1 + 2, Is.EqualTo(3))', - '', - '[]', - 'let ``fails on purpose`` () = Assert.That(1 + 2, Is.EqualTo(4))', - '', - '[]', - 'let ``skipped on purpose`` () = ()', - '', - '[]', - 'let ``adds case`` (a: int) (b: int) (expected: int) = Assert.That(a + b, Is.EqualTo(expected))', - '', -].join('\n'); - -const CS_NUNIT_SOURCE = [ - 'using NUnit.Framework;', - '', - 'namespace Cs.NunitMtp.Fixtures', - '{', - ' public class CalculatorTests', - ' {', - ' [Test] public void Adds_TwoNumbers() => Assert.That(1 + 2, Is.EqualTo(3));', - ' [Test] public void Fails_OnPurpose() => Assert.That(1 + 2, Is.EqualTo(4));', - ' [Test, Ignore("fixture: deliberately skipped")] public void Skipped_OnPurpose() { }', - ' [TestCase(2, 2, 4)]', - ' public void Adds_Case(int a, int b, int expected) =>', - ' Assert.That(a + b, Is.EqualTo(expected));', - ' }', - '}', - '', -].join('\n'); - /** * Every MTP fixture, F# FIRST within each framework. * @@ -222,7 +149,7 @@ export const MTP_FIXTURES: readonly MtpFixture[] = [ framework: 'xunit', language: 'csharp', packages: MTP_XUNIT_PACKAGES, - source: CS_XUNIT_SOURCE, + source: csXunitSource('namespace Cs.XunitMtp.Fixtures'), passing: 'Cs.XunitMtp.Fixtures.CalculatorTests.Adds_TwoNumbers', failing: 'Cs.XunitMtp.Fixtures.CalculatorTests.Fails_OnPurpose', skipped: 'Cs.XunitMtp.Fixtures.CalculatorTests.Skipped_OnPurpose', @@ -249,7 +176,7 @@ export const MTP_FIXTURES: readonly MtpFixture[] = [ framework: 'mstest', language: 'csharp', packages: MTP_MSTEST_PACKAGES, - source: CS_MSTEST_SOURCE, + source: csMstestSource('namespace Cs.MstestMtp.Fixtures', ' [TestMethod]'), passing: 'Cs.MstestMtp.Fixtures.CalculatorTests.Adds_TwoNumbers', failing: 'Cs.MstestMtp.Fixtures.CalculatorTests.Fails_OnPurpose', skipped: 'Cs.MstestMtp.Fixtures.CalculatorTests.Skipped_OnPurpose', @@ -262,7 +189,10 @@ export const MTP_FIXTURES: readonly MtpFixture[] = [ framework: 'nunit', language: 'fsharp', packages: MTP_NUNIT_PACKAGES, - source: FS_NUNIT_SOURCE, + source: fsNunitSource( + 'module Fs.NunitMtp.Fixtures', + 'let ``adds two numbers with spaces`` () = Assert.That(1 + 2, Is.EqualTo(3))', + ), passing: 'Fs.NunitMtp.Fixtures.adds two numbers with spaces', failing: 'Fs.NunitMtp.Fixtures.fails on purpose', skipped: 'Fs.NunitMtp.Fixtures.skipped on purpose', @@ -275,7 +205,7 @@ export const MTP_FIXTURES: readonly MtpFixture[] = [ framework: 'nunit', language: 'csharp', packages: MTP_NUNIT_PACKAGES, - source: CS_NUNIT_SOURCE, + source: csNunitSource('namespace Cs.NunitMtp.Fixtures'), passing: 'Cs.NunitMtp.Fixtures.CalculatorTests.Adds_TwoNumbers', failing: 'Cs.NunitMtp.Fixtures.CalculatorTests.Fails_OnPurpose', skipped: 'Cs.NunitMtp.Fixtures.CalculatorTests.Skipped_OnPurpose', diff --git a/src/editors/vscode/src/test/suite/test-explorer-mtp-modules.test.ts b/src/editors/vscode/src/test/suite/test-explorer-mtp-modules.test.ts index a2d48fec..725bcdd1 100644 --- a/src/editors/vscode/src/test/suite/test-explorer-mtp-modules.test.ts +++ b/src/editors/vscode/src/test/suite/test-explorer-mtp-modules.test.ts @@ -40,13 +40,7 @@ import { writeProject, } from './dotnet-project-kit'; import { idsOf, mtpFixtureFor, writeMtpProject } from './test-explorer-mtp-fixtures'; -import { - discoverSolution, - rootsOf, - runViaProfile, - teardownFixtureSolution, - activateWithScratch, -} from './test-explorer-kit'; +import { discoverSolution, rootsOf, runViaProfile } from './test-explorer-kit'; import { assertFailed, assertPassed, @@ -55,7 +49,8 @@ import { sorted, } from './test-explorer-outcome-assertions'; import { removeDirRecursive } from './test-helpers'; -import { DOTNET_CLI_MS, FIXTURE_BUILD_MS } from './test-timeouts'; +import { FIXTURE_BUILD_MS } from './test-timeouts'; +import { useScratchSuite } from './test-explorer-harness'; /** The multi-targeted project: TWO modules sharing one file name. */ const PROJECT = 'ModulesMtpCs'; @@ -159,24 +154,14 @@ suite('Test Explorer e2e — Microsoft.Testing.Platform across several modules', /** Every id the fixture solution must expose. */ let expected: string[]; - suiteSetup(async function () { - this.timeout(FIXTURE_BUILD_MS); - ({ api, root } = await activateWithScratch('sharplsp-mtp-modules-')); + useScratchSuite('sharplsp-mtp-modules-', async (scratch) => { + ({ api, root } = scratch); frameworks = await installedFrameworkPair(root); slnPath = await createFixture(root, frameworks); expected = [EDITED, ...frameworks.map(frameworkId), ...NO_TRX_IDS]; await discoverSolution(api, slnPath, expected); }); - teardown(async () => { - await api.testController.whenIdle(); - }); - - suiteTeardown(async function () { - this.timeout(DOTNET_CLI_MS); - await teardownFixtureSolution(api, root, removeDirRecursive); - }); - test('a multi-targeted project reports EVERY framework, and no report overwrites another', async function () { this.timeout(FIXTURE_BUILD_MS); diff --git a/src/editors/vscode/src/test/suite/test-explorer-mtp-outcomes.test.ts b/src/editors/vscode/src/test/suite/test-explorer-mtp-outcomes.test.ts index e4c80764..4e8ba5c7 100644 --- a/src/editors/vscode/src/test/suite/test-explorer-mtp-outcomes.test.ts +++ b/src/editors/vscode/src/test/suite/test-explorer-mtp-outcomes.test.ts @@ -47,6 +47,7 @@ import { } from './test-explorer-outcome-assertions'; import { removeDirRecursive } from './test-helpers'; import { DOTNET_CLI_MS, FIXTURE_BUILD_MS } from './test-timeouts'; +import { useScratchSuite } from './test-explorer-harness'; /** Every id that must come back green. */ const PASSING: readonly string[] = MTP_FIXTURES.flatMap((fixture) => [ @@ -66,22 +67,12 @@ suite('Test Explorer e2e — Microsoft.Testing.Platform runs', () => { let root: string; let slnPath: string; - suiteSetup(async function () { - this.timeout(FIXTURE_BUILD_MS); - ({ api, root } = await activateWithScratch('sharplsp-mtp-run-')); + useScratchSuite('sharplsp-mtp-run-', async (scratch) => { + ({ api, root } = scratch); slnPath = await createMtpSolution(root); await discoverSolution(api, slnPath, ALL_MTP_IDS); }); - teardown(async () => { - await api.testController.whenIdle(); - }); - - suiteTeardown(async function () { - this.timeout(DOTNET_CLI_MS); - await teardownFixtureSolution(api, root, removeDirRecursive); - }); - test('▶ on the whole tree attributes a pass, a fail and a skip to each test', async function () { this.timeout(FIXTURE_BUILD_MS); const items = itemsFor(api, [...PASSING, ...FAILING, ...SKIPPED]); diff --git a/src/editors/vscode/src/test/suite/test-explorer-mtp-sweeps.test.ts b/src/editors/vscode/src/test/suite/test-explorer-mtp-sweeps.test.ts index 58d50c2e..1821505c 100644 --- a/src/editors/vscode/src/test/suite/test-explorer-mtp-sweeps.test.ts +++ b/src/editors/vscode/src/test/suite/test-explorer-mtp-sweeps.test.ts @@ -51,12 +51,11 @@ import { errorTextOf, findItem, rootsOf, - activateWithScratch, - clearTestTree, } from './test-explorer-kit'; import { assertPassed, cachedFor, runIds } from './test-explorer-outcome-assertions'; -import { removeDirRecursive, assertContainsAll } from './test-helpers'; +import { assertContainsAll } from './test-helpers'; import { FIXTURE_BUILD_MS } from './test-timeouts'; +import { useScratchParent } from './test-explorer-harness'; /** An F# library: something to build, and no test anywhere. */ const LIBRARY_SOURCE = 'module Sweep.Library\n\nlet add a b = a + b\n'; @@ -185,15 +184,9 @@ suite('Test Explorer e2e — what an MTP probe sweep costs, reaches and keeps', let api: SharpLspExtensionApi; let parent: string; - suiteSetup(async function () { - this.timeout(FIXTURE_BUILD_MS); - ({ api, root: parent } = await activateWithScratch('sharplsp-mtp-sweeps-')); - }); - - teardown(() => clearTestTree(api)); - - suiteTeardown(() => { - removeDirRecursive(parent); + const scratch = useScratchParent('sharplsp-mtp-sweeps-'); + setup(() => { + ({ api, root: parent } = scratch()); }); test('a solution with no MTP project is built no more often than VSTest builds it', async function () { diff --git a/src/editors/vscode/src/test/suite/test-explorer-mtp.test.ts b/src/editors/vscode/src/test/suite/test-explorer-mtp.test.ts index 1c62424d..684d29b6 100644 --- a/src/editors/vscode/src/test/suite/test-explorer-mtp.test.ts +++ b/src/editors/vscode/src/test/suite/test-explorer-mtp.test.ts @@ -32,17 +32,11 @@ import { mtpFixtureFor, type MtpFixture, } from './test-explorer-mtp-fixtures'; -import { - collectLeafIds, - discoverSolution, - findItem, - rootsOf, - teardownFixtureSolution, - activateWithScratch, -} from './test-explorer-kit'; +import { collectLeafIds, discoverSolution, findItem, rootsOf } from './test-explorer-kit'; import { sorted } from './test-explorer-outcome-assertions'; -import { removeDirRecursive, assertContainsAll } from './test-helpers'; -import { DOTNET_CLI_MS, FIXTURE_BUILD_MS } from './test-timeouts'; +import { assertContainsAll } from './test-helpers'; +import { FIXTURE_BUILD_MS } from './test-timeouts'; +import { useScratchSuite } from './test-explorer-harness'; /** The awkward id shapes, the label each renders as, and why it is hard. */ const AWKWARD_SHAPES: readonly (readonly [string, string, string])[] = [ @@ -86,23 +80,11 @@ suite('Test Explorer e2e — Microsoft.Testing.Platform discovery', () => { let root: string; let slnPath: string; - suiteSetup(async function () { - this.timeout(FIXTURE_BUILD_MS); - ({ api, root } = await activateWithScratch('sharplsp-mtp-')); + useScratchSuite('sharplsp-mtp-', async (scratch) => { + ({ api, root } = scratch); slnPath = await createMtpSolution(root); }); - teardown(async () => { - // Never leave a `dotnet` invocation in flight across tests: discovery builds - // the same `bin/`/`obj/` a run rebuilds, and the overlap breaks both. - await api.testController.whenIdle(); - }); - - suiteTeardown(async function () { - this.timeout(DOTNET_CLI_MS); - await teardownFixtureSolution(api, root, removeDirRecursive); - }); - test('the global.json opt-in selects MTP, and the modules come from MSBuild', async function () { this.timeout(FIXTURE_BUILD_MS); diff --git a/src/editors/vscode/src/test/suite/test-explorer-multitarget.test.ts b/src/editors/vscode/src/test/suite/test-explorer-multitarget.test.ts index f9656e61..b3e828a8 100644 --- a/src/editors/vscode/src/test/suite/test-explorer-multitarget.test.ts +++ b/src/editors/vscode/src/test/suite/test-explorer-multitarget.test.ts @@ -49,7 +49,6 @@ import { warmDiscovery, writeProject, } from './dotnet-project-kit'; -import { fixtureFor } from './test-explorer-fixtures'; import { collectItemIds, collectLeafIds, @@ -66,9 +65,7 @@ import { import { removeDirRecursive } from './test-helpers.js'; import { cachedFor, itemsFor, sorted, runIds } from './test-explorer-outcome-assertions'; import { DOTNET_CLI_MS, FAST_MS, FIXTURE_BUILD_MS } from './test-timeouts'; - -/** The C# xUnit fixture, rebuilt here for TWO target frameworks. */ -const CS = fixtureFor('xunit-csharp'); +import { CS } from './test-explorer-xunit-pair'; /** The namespace both classes are declared in. */ const NAMESPACE = 'Cs.Xunit.Fixtures'; diff --git a/src/editors/vscode/src/test/suite/test-explorer-names.test.ts b/src/editors/vscode/src/test/suite/test-explorer-names.test.ts index 21998d2b..28ebbdcf 100644 --- a/src/editors/vscode/src/test/suite/test-explorer-names.test.ts +++ b/src/editors/vscode/src/test/suite/test-explorer-names.test.ts @@ -35,8 +35,8 @@ import { HEX_DIGITS, dedupeLines } from '../../test-names.js'; import { fixtureFor } from './test-explorer-fixtures'; import { eq, deepEq, assertContainsAll, assertContainsNone } from './test-helpers'; import { FAST_MS } from './test-timeouts'; - -const CS = fixtureFor('xunit-csharp'); +import { assertMapsEach } from './test-explorer-outcome-assertions'; +import { CS, FS_FACT_SPACED } from './test-explorer-xunit-pair'; /** * Every framework fixture in the suite, so the name reader is driven against @@ -63,9 +63,6 @@ function everyFixtureName(): string[] { return [...new Set(names)]; } -/** The idiomatic F# backtick binding whose xUnit FQN literally contains spaces. */ -const FS_FACT_SPACED = 'Fs.Xunit.Fixtures.adds two numbers with spaces'; - /** The real NUnit `[TestCase]` FQN — parentheses and commas, verbatim. */ const NUNIT_CASE = 'Cs.Nunit.Fixtures.CalculatorTests.Adds_Case(2,2,4)'; @@ -148,21 +145,23 @@ suite('Test Explorer — adapter decoration comes off, real names stay on', () = // Interaction 3 — the two rules stated as rules, not as a table: no space // before the bracket, and no hex inside it. Both are what // [TEST-DISCOVERY-FQN] names as the distinguishing conditions. - assert.strictEqual( - withoutAdapterUniqueId(`Ns.C.M(${UNIQUE_ID})`), - `Ns.C.M(${UNIQUE_ID})`, - 'no SPACE before the bracket means it is part of the name, however hex-like it looks', - ); - assert.strictEqual( - withoutAdapterUniqueId('Ns.C.M (2,2,4)'), - 'Ns.C.M (2,2,4)', - 'a space before a NON-hex bracket is a name that happens to contain a space', - ); - assert.strictEqual( - withoutAdapterUniqueId(withoutAdapterUniqueId(`${CS.passing} (${UNIQUE_ID})`)), - CS.passing, - 'stripping is idempotent — an already-bare id survives a second pass unchanged', - ); + assertMapsEach(withoutAdapterUniqueId, [ + [ + `Ns.C.M(${UNIQUE_ID})`, + `Ns.C.M(${UNIQUE_ID})`, + 'no SPACE before the bracket means it is part of the name, however hex-like it looks', + ], + [ + 'Ns.C.M (2,2,4)', + 'Ns.C.M (2,2,4)', + 'a space before a NON-hex bracket is a name that happens to contain a space', + ], + [ + withoutAdapterUniqueId(`${CS.passing} (${UNIQUE_ID})`), + CS.passing, + 'stripping is idempotent — an already-bare id survives a second pass unchanged', + ], + ]); // Interaction 4 - stripping is a TOTAL function: it answers for every // string, and answering twice never changes the answer. A stripper that is // not idempotent corrupts a name the second time discovery sweeps. @@ -173,14 +172,16 @@ suite('Test Explorer — adapter decoration comes off, real names stay on', () = `'${name}' must survive a second pass unchanged`, ); } - assert.strictEqual(withoutAdapterUniqueId(' '), ' ', 'a lone space is not a decoration'); - assert.strictEqual(withoutAdapterUniqueId('()'), '()', 'nor a bare pair of brackets'); - assert.strictEqual(withoutAdapterUniqueId(' ()'), ' ()', 'nor a space and a bare pair'); - assert.strictEqual( - withoutAdapterUniqueId(`(${UNIQUE_ID})`), - `(${UNIQUE_ID})`, - 'a decoration with NO name in front of it is not a decorated name', - ); + assertMapsEach(withoutAdapterUniqueId, [ + [' ', ' ', 'a lone space is not a decoration'], + ['()', '()', 'nor a bare pair of brackets'], + [' ()', ' ()', 'nor a space and a bare pair'], + [ + `(${UNIQUE_ID})`, + `(${UNIQUE_ID})`, + 'a decoration with NO name in front of it is not a decorated name', + ], + ]); }); test('the REAL listing file collapses theory rows onto one id, whatever decorated it', function () { diff --git a/src/editors/vscode/src/test/suite/test-explorer-outcome-assertions.ts b/src/editors/vscode/src/test/suite/test-explorer-outcome-assertions.ts index 05348c72..bac9da69 100644 --- a/src/editors/vscode/src/test/suite/test-explorer-outcome-assertions.ts +++ b/src/editors/vscode/src/test/suite/test-explorer-outcome-assertions.ts @@ -194,3 +194,14 @@ export function assertEveryOutcome(api: SharpLspExtensionApi, groups: OutcomeGro for (const id of groups.failing) assertFailed(cachedFor(api, id), id); for (const id of groups.skipped) assertSkipped(cachedFor(api, id), id); } + +/** + * `map(input)` is exactly `expected` on every row, each row failing with its + * own reason — the table form of a run of `assert.strictEqual(map(x), …)`. + */ +export function assertMapsEach( + map: (input: I) => O, + rows: readonly (readonly [input: I, expected: O, why: string])[], +): void { + for (const [input, expected, why] of rows) assert.strictEqual(map(input), expected, why); +} diff --git a/src/editors/vscode/src/test/suite/test-explorer-outcomes.test.ts b/src/editors/vscode/src/test/suite/test-explorer-outcomes.test.ts index ae1605d7..dda80376 100644 --- a/src/editors/vscode/src/test/suite/test-explorer-outcomes.test.ts +++ b/src/editors/vscode/src/test/suite/test-explorer-outcomes.test.ts @@ -56,8 +56,7 @@ import { import { DEBUG_SESSION_MS, DOTNET_CLI_MS } from './test-timeouts'; import { useWarmFixture } from './test-explorer-harness'; import { COVERAGE_DIR_NAME } from './test-coverage-fixtures'; - -const CS = fixtureFor('xunit-csharp'); +import { CS } from './test-explorer-xunit-pair'; const FSX = fixtureFor('xunit-fsharp'); /** The idiomatic F# backtick fact whose xUnit FQN literally contains spaces. */ diff --git a/src/editors/vscode/src/test/suite/test-explorer-parsers.test.ts b/src/editors/vscode/src/test/suite/test-explorer-parsers.test.ts index 03ef6426..6a4f6a48 100644 --- a/src/editors/vscode/src/test/suite/test-explorer-parsers.test.ts +++ b/src/editors/vscode/src/test/suite/test-explorer-parsers.test.ts @@ -39,14 +39,10 @@ import { } from '../../test-discovery.js'; import { parseFailureMessage, parseRunSummary } from '../../test-run-output.js'; import { isRunError, parseTrx, parseTrxDuration, parseTrxReport } from '../../test-trx.js'; -import { fixtureFor } from './test-explorer-fixtures'; import { FAST_MS } from './test-timeouts'; import { assertContainsAll, assertContainsNone } from './test-helpers'; - -const CS = fixtureFor('xunit-csharp'); - -/** The idiomatic F# backtick binding whose xUnit FQN literally contains spaces. */ -const FS_FACT_SPACED = 'Fs.Xunit.Fixtures.adds two numbers with spaces'; +import { assertMapsEach } from './test-explorer-outcome-assertions'; +import { CS, FS_FACT_SPACED } from './test-explorer-xunit-pair'; /** The real NUnit `[TestCase]` FQN — parentheses and commas, verbatim. */ const NUNIT_CASE = 'Cs.Nunit.Fixtures.CalculatorTests.Adds_Case(2,2,4)'; @@ -252,24 +248,18 @@ suite('Test Explorer e2e — TRX and console readers on Windows shapes', () => { ); // Outcome spellings, and the ones that must NOT become a silent pass. - assert.strictEqual(mappedOutcome('Passed'), 'passed', 'Passed'); - assert.strictEqual(mappedOutcome('Failed'), 'failed', 'Failed'); - assert.strictEqual(mappedOutcome('Error'), 'failed', 'an Error is a failure'); - assert.strictEqual(mappedOutcome('Timeout'), 'failed', 'a Timeout is a failure'); - assert.strictEqual(mappedOutcome('Aborted'), 'failed', 'an Aborted run is a failure'); - assert.strictEqual(mappedOutcome('NotExecuted'), 'skipped', 'NotExecuted is a skip'); - assert.strictEqual(mappedOutcome('Inconclusive'), 'skipped', 'Inconclusive is a skip'); - assert.strictEqual( - mappedOutcome('nOtExEcUtEd'), - 'skipped', - 'the outcome match is case-insensitive', - ); - assert.strictEqual( - mappedOutcome('Nonsense'), - 'notRun', - 'an unknown spelling is never silently a pass', - ); - assert.strictEqual(mappedOutcome(''), 'notRun', 'nor is a missing outcome'); + assertMapsEach(mappedOutcome, [ + ['Passed', 'passed', 'Passed'], + ['Failed', 'failed', 'Failed'], + ['Error', 'failed', 'an Error is a failure'], + ['Timeout', 'failed', 'a Timeout is a failure'], + ['Aborted', 'failed', 'an Aborted run is a failure'], + ['NotExecuted', 'skipped', 'NotExecuted is a skip'], + ['Inconclusive', 'skipped', 'Inconclusive is a skip'], + ['nOtExEcUtEd', 'skipped', 'the outcome match is case-insensitive'], + ['Nonsense', 'notRun', 'an unknown spelling is never silently a pass'], + ['', 'notRun', 'nor is a missing outcome'], + ]); // A result with no definition falls back to its display name rather than // vanishing — losing a result entirely would show the user a phantom. @@ -293,22 +283,16 @@ suite('Test Explorer e2e — TRX and console readers on Windows shapes', () => { ); // Durations, including the shapes that must NOT be guessed at. - assert.strictEqual(parseTrxDuration('00:00:00.0010748'), 1, 'sub-millisecond rounds to 1ms'); - assert.strictEqual(parseTrxDuration('00:00:01.5000000'), 1500, 'one and a half seconds'); - assert.strictEqual(parseTrxDuration('00:01:02.0000000'), 62_000, 'minutes and seconds add up'); - assert.strictEqual(parseTrxDuration('01:00:00.0000000'), 3_600_000, 'a whole hour'); - assert.strictEqual( - parseTrxDuration('00:00:00.0000000'), - 0, - 'a zero duration is zero, not undefined', - ); - assert.strictEqual( - parseTrxDuration('garbage'), - undefined, - 'an unparseable duration is undefined, never NaN', - ); - assert.strictEqual(parseTrxDuration(undefined), undefined, 'a missing duration is undefined'); - assert.strictEqual(parseTrxDuration('1:2'), undefined, 'a malformed clock is undefined'); + assertMapsEach(parseTrxDuration, [ + ['00:00:00.0010748', 1, 'sub-millisecond rounds to 1ms'], + ['00:00:01.5000000', 1500, 'one and a half seconds'], + ['00:01:02.0000000', 62_000, 'minutes and seconds add up'], + ['01:00:00.0000000', 3_600_000, 'a whole hour'], + ['00:00:00.0000000', 0, 'a zero duration is zero, not undefined'], + ['garbage', undefined, 'an unparseable duration is undefined, never NaN'], + [undefined, undefined, 'a missing duration is undefined'], + ['1:2', undefined, 'a malformed clock is undefined'], + ]); }); test('a REFUSED filter and an unmatched filter are told apart by the run info', function () { @@ -429,19 +413,15 @@ suite('Test Explorer e2e — TRX and console readers on Windows shapes', () => { // No summary at all is what a filter matching nothing prints, and it is NOT // a failure — it is "nothing ran". - assert.strictEqual(parseRunSummary(''), undefined, 'no output means no summary'); - assert.strictEqual( - parseRunSummary( + assertMapsEach(parseRunSummary, [ + ['', undefined, 'no output means no summary'], + [ 'No test matches the given testcase filter `FullyQualifiedName=Ns.C.Nope` in X.dll', - ), - undefined, - 'an unmatched filter prints no summary at all', - ); - assert.strictEqual( - parseRunSummary('Determining projects to restore...'), - undefined, - 'build chatter is not a summary', - ); + undefined, + 'an unmatched filter prints no summary at all', + ], + ['Determining projects to restore...', undefined, 'build chatter is not a summary'], + ]); const zero = parseRunSummary(summaryLine('Passed', 0, 0, 0, 'Empty.dll')); assert.ok(zero, 'an assembly with no tests still prints a summary'); assert.strictEqual(zero.outcome, 'notRun', 'zero total tests is "nothing ran", not a pass'); @@ -464,24 +444,20 @@ suite('Test Explorer e2e — TRX and console readers on Windows shapes', () => { ); assertContainsAll(message, ['Expected: 4', 'Actual: 3'], 'message'); assertContainsNone(message, ['Stack Trace', 'at Cs.Xunit'], 'message'); - assert.strictEqual( - parseFailureMessage('Passed! - Failed: 0'), - undefined, - 'output with no Error Message block yields undefined', - ); - assert.strictEqual(parseFailureMessage(''), undefined, 'empty output yields undefined'); - assert.strictEqual( - parseFailureMessage( + assertMapsEach(parseFailureMessage, [ + ['Passed! - Failed: 0', undefined, 'output with no Error Message block yields undefined'], + ['', undefined, 'empty output yields undefined'], + [ [ ' Error Message:', ' first failure', ' Failed Ns.C.Second [1 ms]', ' second failure', ].join('\n'), - ), - 'first failure', - "the block ends at the next test's Failed header", - ); + 'first failure', + "the block ends at the next test's Failed header", + ], + ]); }); test('the dotnet child process is pinned to English and sized for a cold Windows restore', function () { @@ -536,87 +512,33 @@ suite('Test Explorer e2e — TRX and console readers on Windows shapes', () => { // that does not exist. Dropping it skipped the fully-qualified listing and // degraded discovery to DISPLAY names, losing every NUnit test, every MSTest // test and every theory, silently. - assert.strictEqual( - unescapeMsBuildPath('C:\\Program Files %28x86%29\\bin\\A.dll'), - 'C:\\Program Files (x86)\\bin\\A.dll', - 'parentheses come back as ( and )', - ); - assert.strictEqual( - unescapeMsBuildPath('/tmp/a %3B b/c.dll'), - '/tmp/a ; b/c.dll', - 'a semicolon comes back from %3B', - ); - assert.strictEqual( - unescapeMsBuildPath('/tmp/it%27s/c.dll'), - "/tmp/it's/c.dll", - 'an apostrophe from %27', - ); - assert.strictEqual( - unescapeMsBuildPath('/tmp/a%2Cb/c.dll'), - '/tmp/a,b/c.dll', - 'a comma from %2C', - ); - assert.strictEqual( - unescapeMsBuildPath('/tmp/a%40b/c.dll'), - '/tmp/a@b/c.dll', - 'an at sign from %40', - ); - assert.strictEqual( - unescapeMsBuildPath('/tmp/a%24b/c.dll'), - '/tmp/a$b/c.dll', - 'a dollar from %24', - ); - assert.strictEqual( - unescapeMsBuildPath('/tmp/a%2Ab/c.dll'), - '/tmp/a*b/c.dll', - 'an asterisk from %2A', - ); - assert.strictEqual( - unescapeMsBuildPath('/tmp/a%3Fb/c.dll'), - '/tmp/a?b/c.dll', - 'a question mark from %3F', - ); - assert.strictEqual( - unescapeMsBuildPath('/tmp/100%25/c.dll'), - '/tmp/100%/c.dll', - 'and the percent sign itself from %25', - ); - assert.strictEqual( - unescapeMsBuildPath('/tmp/%28%29%28%29/c.dll'), - '/tmp/()()/c.dll', - 'several escapes in a row all decode', - ); - assert.strictEqual( - unescapeMsBuildPath('/tmp/%2f/c.dll'), - '/tmp///c.dll', - 'lower-case hex decodes too', - ); - assert.strictEqual( - unescapeMsBuildPath('/tmp/plain/c.dll'), - '/tmp/plain/c.dll', - 'a path with nothing to decode is returned unchanged', - ); - assert.strictEqual( - unescapeMsBuildPath('/tmp/50%/c.dll'), - '/tmp/50%/c.dll', - 'a bare percent is left alone, never dropped', - ); - assert.strictEqual( - unescapeMsBuildPath('/tmp/c.dll%'), - '/tmp/c.dll%', - 'a trailing percent with no hex survives', - ); - assert.strictEqual( - unescapeMsBuildPath('/tmp/%zz/c.dll'), - '/tmp/%zz/c.dll', - 'a non-hex escape is not a decode', - ); - assert.strictEqual( - unescapeMsBuildPath('/tmp/%2/c.dll'), - '/tmp/%2/c.dll', - 'a one-digit escape is not a decode', - ); - assert.strictEqual(unescapeMsBuildPath(''), '', 'the empty path decodes to itself'); + assertMapsEach(unescapeMsBuildPath, [ + [ + 'C:\\Program Files %28x86%29\\bin\\A.dll', + 'C:\\Program Files (x86)\\bin\\A.dll', + 'parentheses come back as ( and )', + ], + ['/tmp/a %3B b/c.dll', '/tmp/a ; b/c.dll', 'a semicolon comes back from %3B'], + ['/tmp/it%27s/c.dll', "/tmp/it's/c.dll", 'an apostrophe from %27'], + ['/tmp/a%2Cb/c.dll', '/tmp/a,b/c.dll', 'a comma from %2C'], + ['/tmp/a%40b/c.dll', '/tmp/a@b/c.dll', 'an at sign from %40'], + ['/tmp/a%24b/c.dll', '/tmp/a$b/c.dll', 'a dollar from %24'], + ['/tmp/a%2Ab/c.dll', '/tmp/a*b/c.dll', 'an asterisk from %2A'], + ['/tmp/a%3Fb/c.dll', '/tmp/a?b/c.dll', 'a question mark from %3F'], + ['/tmp/100%25/c.dll', '/tmp/100%/c.dll', 'and the percent sign itself from %25'], + ['/tmp/%28%29%28%29/c.dll', '/tmp/()()/c.dll', 'several escapes in a row all decode'], + ['/tmp/%2f/c.dll', '/tmp///c.dll', 'lower-case hex decodes too'], + [ + '/tmp/plain/c.dll', + '/tmp/plain/c.dll', + 'a path with nothing to decode is returned unchanged', + ], + ['/tmp/50%/c.dll', '/tmp/50%/c.dll', 'a bare percent is left alone, never dropped'], + ['/tmp/c.dll%', '/tmp/c.dll%', 'a trailing percent with no hex survives'], + ['/tmp/%zz/c.dll', '/tmp/%zz/c.dll', 'a non-hex escape is not a decode'], + ['/tmp/%2/c.dll', '/tmp/%2/c.dll', 'a one-digit escape is not a decode'], + ['', '', 'the empty path decodes to itself'], + ]); // And the banner parser keeps the raw, still-escaped spelling: resolving it // to a real file is a separate, filesystem-aware step. diff --git a/src/editors/vscode/src/test/suite/test-explorer-reactive.test.ts b/src/editors/vscode/src/test/suite/test-explorer-reactive.test.ts index 642c2b34..9c1645cc 100644 --- a/src/editors/vscode/src/test/suite/test-explorer-reactive.test.ts +++ b/src/editors/vscode/src/test/suite/test-explorer-reactive.test.ts @@ -20,7 +20,6 @@ import { warmDiscovery, writeProject, } from './dotnet-project-kit'; -import { fixtureFor } from './test-explorer-fixtures'; import { assertDeclaredInside, activateTestExplorer, @@ -33,13 +32,11 @@ import { snapshotItems, teardownFixtureSolution, assertLeavesAre, + unloadSolution, } from './test-explorer-kit'; import { comparablePath, removeDirRecursive, assertContainsAll } from './test-helpers.js'; import { DOTNET_CLI_MS, FIXTURE_BUILD_MS } from './test-timeouts'; - -const CS = fixtureFor('xunit-csharp'); -/** The xUnit C# mixed theory, literal so `suiteSetup` can prove the fixture agrees. */ -const CS_MIXED_THEORY = 'Cs.Xunit.Fixtures.CalculatorTests.Mixed_Theory'; +import { CS, CS_MIXED_THEORY } from './test-explorer-xunit-pair'; /** Exactly the FQNs the xUnit C# fixture exposes — theory rows collapse to one. */ const EXPECTED = [CS.passing, CS.failing, CS.skipped, CS.parameterized, CS_MIXED_THEORY] as const; @@ -274,9 +271,7 @@ suite('Test Explorer e2e — reactive discovery, refresh and tree lifecycle', () teardown(async function () { this.timeout(DOTNET_CLI_MS); // Reset the signal so the next load is a REAL transition, and let it settle. - await drainDiscovery(() => { - api.explorerProvider.clear(); - }, api.testController); + await unloadSolution(api); }); suiteTeardown(async function () { diff --git a/src/editors/vscode/src/test/suite/test-explorer-refusals.test.ts b/src/editors/vscode/src/test/suite/test-explorer-refusals.test.ts index 3f3cda1b..18539fd8 100644 --- a/src/editors/vscode/src/test/suite/test-explorer-refusals.test.ts +++ b/src/editors/vscode/src/test/suite/test-explorer-refusals.test.ts @@ -41,11 +41,9 @@ import { drainDiscovery, errorTextOf, rootsOf, - activateWithScratch, - clearTestTree, } from './test-explorer-kit'; -import { removeDirRecursive } from './test-helpers'; import { FIXTURE_BUILD_MS } from './test-timeouts'; +import { useScratchParent } from './test-explorer-harness'; /** One real xUnit test — the test the refusal loses. */ const TEST_SOURCE = [ @@ -99,15 +97,9 @@ suite('Test Explorer e2e — a target dotnet refuses without failing', () => { let api: SharpLspExtensionApi; let parent: string; - suiteSetup(async function () { - this.timeout(FIXTURE_BUILD_MS); - ({ api, root: parent } = await activateWithScratch('sharplsp-refusals-')); - }); - - teardown(() => clearTestTree(api)); - - suiteTeardown(() => { - removeDirRecursive(parent); + const scratch = useScratchParent('sharplsp-refusals-'); + setup(() => { + ({ api, root: parent } = scratch()); }); test('a solution dotnet restores nothing from explains itself instead of going blank', async function () { @@ -208,15 +200,9 @@ suite('Test Explorer e2e — a refusal is told apart from an ordinary diagnostic let api: SharpLspExtensionApi; let parent: string; - suiteSetup(async function () { - this.timeout(FIXTURE_BUILD_MS); - ({ api, root: parent } = await activateWithScratch('sharplsp-refusal-kinds-')); - }); - - teardown(() => clearTestTree(api)); - - suiteTeardown(() => { - removeDirRecursive(parent); + const scratch = useScratchParent('sharplsp-refusal-kinds-'); + setup(() => { + ({ api, root: parent } = scratch()); }); test('a healthy solution carrying a package advisory keeps its truthful empty tree', async function () { diff --git a/src/editors/vscode/src/test/suite/test-explorer-windows.test.ts b/src/editors/vscode/src/test/suite/test-explorer-windows.test.ts index 2e2763fb..530561e6 100644 --- a/src/editors/vscode/src/test/suite/test-explorer-windows.test.ts +++ b/src/editors/vscode/src/test/suite/test-explorer-windows.test.ts @@ -21,7 +21,6 @@ import { import { buildFilterArgs, runTests } from '../../test-execution.js'; import { escapeFilterValue, filterClause, filterExpression } from '../../test-filter.js'; import { createSolution, projectXml, warmDiscovery, writeProject } from './dotnet-project-kit'; -import { fixtureFor } from './test-explorer-fixtures'; import { assertDeclaredInside, collectItemIds, @@ -41,29 +40,14 @@ import { assertContainsNone, } from './test-helpers.js'; import { DOTNET_CLI_MS, FAST_MS, FIXTURE_BUILD_MS } from './test-timeouts'; - -const CS = fixtureFor('xunit-csharp'); -const FS_FIXTURE = fixtureFor('xunit-fsharp'); - -/** The idiomatic F# backtick binding whose xUnit FQN literally contains spaces. */ -const FS_FACT_SPACED = 'Fs.Xunit.Fixtures.adds two numbers with spaces'; -/** The C# theory whose two rows DISAGREE — both report under this one name. */ -const CS_MIXED_THEORY = 'Cs.Xunit.Fixtures.CalculatorTests.Mixed_Theory'; - -/** EXHAUSTIVELY every FQN the two xUnit fixtures expose: six F# (first), five C#. */ -const EXPECTED = [ - FS_FIXTURE.passing, +import { assertMapsEach } from './test-explorer-outcome-assertions'; +import { + CS, + FS_FIXTURE, FS_FACT_SPACED, - FS_FIXTURE.failing, - FS_FIXTURE.skipped, - FS_FIXTURE.parameterized, - 'Fs.Xunit.Fixtures.mixed theory', - CS.passing, - CS.failing, - CS.skipped, - CS.parameterized, CS_MIXED_THEORY, -] as const; + XUNIT_PAIR_IDS as EXPECTED, +} from './test-explorer-xunit-pair'; /** The real NUnit `[TestCase]` FQNs — the ones that kill the adapter unescaped. */ const NUNIT_CASE = 'Cs.Nunit.Fixtures.CalculatorTests.Adds_Case(2,2,4)'; @@ -553,70 +537,46 @@ suite('Test Explorer e2e — Windows-hostile paths, encodings and filter grammar test('the filter grammar escapes every VSTest metacharacter and nothing else', function () { this.timeout(FAST_MS); // Each of these is grammar to VSTest, so each must be backslash-escaped. - assert.strictEqual( - escapeFilterValue('\\'), - '\\\\', - 'the backslash itself must be escaped, or the escape is forgeable', - ); - assert.strictEqual(escapeFilterValue('('), '\\(', 'an open paren opens a sub-expression'); - assert.strictEqual(escapeFilterValue(')'), '\\)', 'a close paren closes a sub-expression'); - assert.strictEqual(escapeFilterValue('&'), '\\&', 'ampersand is the AND operator'); - assert.strictEqual(escapeFilterValue('|'), '\\|', 'pipe is the OR operator'); - assert.strictEqual(escapeFilterValue('='), '\\=', 'equals separates property from value'); - assert.strictEqual(escapeFilterValue('!'), '\\!', 'bang is negation'); - assert.strictEqual(escapeFilterValue('~'), '\\~', 'tilde is the contains operator'); - assert.strictEqual( - escapeFilterValue('a(b)c&d|e=f!g~h\\i'), - 'a\\(b\\)c\\&d\\|e\\=f\\!g\\~h\\\\i', - 'every metacharacter in one value is escaped, not just the first', - ); - assert.strictEqual( - escapeFilterValue('((('), - '\\(\\(\\(', - 'EVERY occurrence is escaped — a non-global replace is the classic incomplete-sanitization bug', - ); - assert.strictEqual( - escapeFilterValue('\\('), - '\\\\\\(', - 'escaping is deliberately NOT idempotent: a value is escaped exactly once, at the clause', - ); + assertMapsEach(escapeFilterValue, [ + ['\\', '\\\\', 'the backslash itself must be escaped, or the escape is forgeable'], + ['(', '\\(', 'an open paren opens a sub-expression'], + [')', '\\)', 'a close paren closes a sub-expression'], + ['&', '\\&', 'ampersand is the AND operator'], + ['|', '\\|', 'pipe is the OR operator'], + ['=', '\\=', 'equals separates property from value'], + ['!', '\\!', 'bang is negation'], + ['~', '\\~', 'tilde is the contains operator'], + [ + 'a(b)c&d|e=f!g~h\\i', + 'a\\(b\\)c\\&d\\|e\\=f\\!g\\~h\\\\i', + 'every metacharacter in one value is escaped, not just the first', + ], + [ + '(((', + '\\(\\(\\(', + 'EVERY occurrence is escaped — a non-global replace is the classic incomplete-sanitization bug', + ], + [ + '\\(', + '\\\\\\(', + 'escaping is deliberately NOT idempotent: a value is escaped exactly once, at the clause', + ], + ]); // These are NOT grammar. Escaping them would change the value and stop it matching. - assert.strictEqual( - escapeFilterValue(','), - ',', - 'a comma is data — NUnit case arguments are comma-separated', - ); - assert.strictEqual( - escapeFilterValue(' '), - ' ', - 'a space is data — F# backtick names are full of them', - ); - assert.strictEqual( - escapeFilterValue('.'), - '.', - 'a dot is the namespace separator, not grammar', - ); - assert.strictEqual( - escapeFilterValue('+'), - '+', - 'plus is the CLR nested-type separator, not grammar', - ); - assert.strictEqual( - escapeFilterValue('a-b_c'), - 'a-b_c', - 'hyphen and underscore are ordinary identifier characters', - ); - assert.strictEqual( - escapeFilterValue('0123456789'), - '0123456789', - 'digits pass through untouched', - ); - assert.strictEqual( - escapeFilterValue('adds_二つ_числа'), - 'adds_二つ_числа', - 'non-ASCII identifiers are legal C#/F# and must pass through', - ); - assert.strictEqual(escapeFilterValue(''), '', 'an empty value escapes to an empty value'); + assertMapsEach(escapeFilterValue, [ + [',', ',', 'a comma is data — NUnit case arguments are comma-separated'], + [' ', ' ', 'a space is data — F# backtick names are full of them'], + ['.', '.', 'a dot is the namespace separator, not grammar'], + ['+', '+', 'plus is the CLR nested-type separator, not grammar'], + ['a-b_c', 'a-b_c', 'hyphen and underscore are ordinary identifier characters'], + ['0123456789', '0123456789', 'digits pass through untouched'], + [ + 'adds_二つ_числа', + 'adds_二つ_числа', + 'non-ASCII identifiers are legal C#/F# and must pass through', + ], + ['', '', 'an empty value escapes to an empty value'], + ]); // The real fixture names, in the shapes VSTest actually produces. assert.strictEqual( escapeFilterValue(NUNIT_CASE), @@ -651,21 +611,19 @@ suite('Test Explorer e2e — Windows-hostile paths, encodings and filter grammar ); } // A clause, and the OR expression built from clauses. - assert.strictEqual( - filterClause(NUNIT_CASE), - 'FullyQualifiedName=Cs.Nunit.Fixtures.CalculatorTests.Adds_Case\\(2,2,4\\)', - 'the clause escapes its value but never its own = separator', - ); - assert.strictEqual( - filterClause(FS_MSTEST_NESTED), - `FullyQualifiedName=${FS_MSTEST_NESTED}`, - "the CLR nested-type '+' reaches VSTest verbatim", - ); - assert.strictEqual( - filterClause(''), - 'FullyQualifiedName=', - 'an empty name still produces a well-formed clause', - ); + assertMapsEach(filterClause, [ + [ + NUNIT_CASE, + 'FullyQualifiedName=Cs.Nunit.Fixtures.CalculatorTests.Adds_Case\\(2,2,4\\)', + 'the clause escapes its value but never its own = separator', + ], + [ + FS_MSTEST_NESTED, + `FullyQualifiedName=${FS_MSTEST_NESTED}`, + "the CLR nested-type '+' reaches VSTest verbatim", + ], + ['', 'FullyQualifiedName=', 'an empty name still produces a well-formed clause'], + ]); assert.strictEqual(filterExpression([]), '', 'no names produce an empty expression'); assert.strictEqual( filterExpression([FS_FACT_SPACED]), diff --git a/src/editors/vscode/src/test/suite/test-explorer-xunit-pair.ts b/src/editors/vscode/src/test/suite/test-explorer-xunit-pair.ts new file mode 100644 index 00000000..04d26ab5 --- /dev/null +++ b/src/editors/vscode/src/test/suite/test-explorer-xunit-pair.ts @@ -0,0 +1,27 @@ +// The two xUnit fixtures — C# and F# — that the Test Explorer suites discover +// side by side, and every fully-qualified name the pair exposes. +import { fixtureFor } from './test-explorer-fixtures'; + +export const CS = fixtureFor('xunit-csharp'); +export const FS_FIXTURE = fixtureFor('xunit-fsharp'); + +/** The idiomatic F# backtick binding whose xUnit FQN literally contains spaces. */ +export const FS_FACT_SPACED = 'Fs.Xunit.Fixtures.adds two numbers with spaces'; +/** The theories whose two rows DISAGREE — both report under this one FQN. */ +export const FS_MIXED_THEORY = 'Fs.Xunit.Fixtures.mixed theory'; +export const CS_MIXED_THEORY = 'Cs.Xunit.Fixtures.CalculatorTests.Mixed_Theory'; + +/** EXHAUSTIVELY every FQN the two xUnit fixtures expose: six F# (first), five C#. */ +export const XUNIT_PAIR_IDS = [ + FS_FIXTURE.passing, + FS_FACT_SPACED, + FS_FIXTURE.failing, + FS_FIXTURE.skipped, + FS_FIXTURE.parameterized, + FS_MIXED_THEORY, + CS.passing, + CS.failing, + CS.skipped, + CS.parameterized, + CS_MIXED_THEORY, +] as const; diff --git a/src/editors/vscode/src/test/suite/testing-lens-status.test.ts b/src/editors/vscode/src/test/suite/testing-lens-status.test.ts index c19c8d74..48e85fc1 100644 --- a/src/editors/vscode/src/test/suite/testing-lens-status.test.ts +++ b/src/editors/vscode/src/test/suite/testing-lens-status.test.ts @@ -73,8 +73,7 @@ import { SIDECAR_COLD_MS, } from './test-timeouts'; import { warmAndDiscover } from './test-explorer-harness'; - -const CS = fixtureFor('xunit-csharp'); +import { CS } from './test-explorer-xunit-pair'; const FSX = fixtureFor('xunit-fsharp'); /** The F# binding whose fully-qualified name carries SPACES. */ diff --git a/src/sharplsp/src/call_hierarchy.rs b/src/sharplsp/src/call_hierarchy.rs index 83c23028..e5ff5d9c 100644 --- a/src/sharplsp/src/call_hierarchy.rs +++ b/src/sharplsp/src/call_hierarchy.rs @@ -1,111 +1,30 @@ -//! Call hierarchy handlers (`textDocument/prepareCallHierarchy`, -//! `callHierarchy/incomingCalls`, `callHierarchy/outgoingCalls`). +//! Call hierarchy mapping (`textDocument/prepareCallHierarchy`, +//! `callHierarchy/incomingCalls`, `callHierarchy/outgoingCalls`): the requests +//! themselves are answered by [`crate::hierarchy`]. -use std::sync::Arc; - -use anyhow::Result; -use lsp_server::Request; use lsp_types::{ - CallHierarchyIncomingCall, CallHierarchyIncomingCallsParams, CallHierarchyItem, - CallHierarchyOutgoingCall, CallHierarchyOutgoingCallsParams, CallHierarchyPrepareParams, - Position, Range, SymbolKind, -}; -use tracing::debug; - -use crate::hierarchy::{ - hierarchy_item_location, prepare_hierarchy, SidecarCallHierarchyCall, SidecarHierarchyItem, + CallHierarchyIncomingCall, CallHierarchyItem, CallHierarchyOutgoingCall, Position, Range, + SymbolKind, }; -use crate::sidecar::manager::SidecarManager; -use crate::utils::{request_sidecar, with_sidecar, SidecarPositionReq}; -/// Handle `textDocument/prepareCallHierarchy`. -pub fn handle_prepare( - req: Request, - runtime: &tokio::runtime::Runtime, - sidecar: Option<&Arc>, -) -> Result { - with_sidecar( - req, - sidecar, - |sidecar, params: CallHierarchyPrepareParams| { - prepare_hierarchy( - runtime, - sidecar, - "textDocument/prepareCallHierarchy", - ¶ms.text_document_position_params, - map_hierarchy_item, - ) - }, - ) -} - -/// Handle `callHierarchy/incomingCalls`. -pub fn handle_incoming( - req: Request, - runtime: &tokio::runtime::Runtime, - sidecar: Option<&Arc>, -) -> Result { - with_sidecar( - req, - sidecar, - |sidecar, params: CallHierarchyIncomingCallsParams| { - hierarchy_calls( - runtime, - sidecar, - "callHierarchy/incomingCalls", - ¶ms.item, - |from, from_ranges| CallHierarchyIncomingCall { from, from_ranges }, - ) - }, - ) -} +use crate::hierarchy::{hierarchy_item_location, SidecarCallHierarchyCall, SidecarHierarchyItem}; -/// Handle `callHierarchy/outgoingCalls`. -pub fn handle_outgoing( - req: Request, - runtime: &tokio::runtime::Runtime, - sidecar: Option<&Arc>, -) -> Result { - with_sidecar( - req, - sidecar, - |sidecar, params: CallHierarchyOutgoingCallsParams| { - hierarchy_calls( - runtime, - sidecar, - "callHierarchy/outgoingCalls", - ¶ms.item, - |to, from_ranges| CallHierarchyOutgoingCall { to, from_ranges }, - ) - }, - ) +/// An incoming call (`callHierarchy/incomingCalls`): the caller, and where it calls. +pub fn incoming_call(call: &SidecarCallHierarchyCall) -> Option { + let from = map_hierarchy_item(&call.item())?; + Some(CallHierarchyIncomingCall { + from, + from_ranges: call_site_ranges(call), + }) } -/// The calls the sidecar reports for `item` via `method`, each built by `make`. -/// -/// An unreachable sidecar answers an empty list, the same as a symbol with no -/// callers: the tree shows nothing rather than an error. -fn hierarchy_calls( - runtime: &tokio::runtime::Runtime, - sidecar: &SidecarManager, - method: &str, - item: &CallHierarchyItem, - make: impl Fn(CallHierarchyItem, Vec) -> T, -) -> Result { - let request = SidecarPositionReq::at(&item.uri, item.selection_range.start)?; - let calls: Vec = - request_sidecar(runtime, sidecar, method, &request)?.unwrap_or_default(); - debug!(method, count = calls.len(), "hierarchy calls from sidecar"); - let result: Vec = calls - .iter() - .filter_map(|call| { - Some(make( - map_hierarchy_item(&call.item())?, - call_site_ranges(call), - )) - }) - .collect(); - Ok(serde_json::to_value(result)?) +/// An outgoing call (`callHierarchy/outgoingCalls`): the callee, and where it is called. +pub fn outgoing_call(call: &SidecarCallHierarchyCall) -> Option { + let to = map_hierarchy_item(&call.item())?; + Some(CallHierarchyOutgoingCall { + to, + from_ranges: call_site_ranges(call), + }) } /// The ranges at which the calls appear, per LSP 3.17. @@ -134,7 +53,7 @@ fn call_site_ranges(call: &SidecarCallHierarchyCall) -> Vec { // ── Helpers ──────────────────────────────────────────────────────── /// Convert a sidecar hierarchy item into an LSP `CallHierarchyItem`. -fn map_hierarchy_item(item: &SidecarHierarchyItem) -> Option { +pub fn map_hierarchy_item(item: &SidecarHierarchyItem) -> Option { let (uri, range, selection_range) = hierarchy_item_location(item)?; Some(CallHierarchyItem { name: item.name.clone(), diff --git a/src/sharplsp/src/code_actions.rs b/src/sharplsp/src/code_actions.rs index 70723587..6dea6e3f 100644 --- a/src/sharplsp/src/code_actions.rs +++ b/src/sharplsp/src/code_actions.rs @@ -10,10 +10,10 @@ use lsp_server::Request; use lsp_types::{ CodeAction, CodeActionKind, CodeActionOrCommand, CodeActionParams, TextEdit, Uri, WorkspaceEdit, }; -use tracing::{debug, warn}; +use tracing::debug; use crate::sidecar::manager::SidecarManager; -use crate::utils::{map_text_edit, SidecarTextEdit}; +use crate::utils::{map_text_edit, request_sidecar, with_sidecar, SidecarTextEdit}; /// Handle `textDocument/codeAction` — returns available code fixes and refactorings. pub fn handle_code_action( @@ -21,39 +21,31 @@ pub fn handle_code_action( runtime: &tokio::runtime::Runtime, sidecar: Option<&Arc>, ) -> Result { - let Some(sidecar) = sidecar else { - return Ok(serde_json::Value::Null); - }; - let params: CodeActionParams = serde_json::from_value(req.params)?; - let file_path = crate::paths::uri_to_path(params.text_document.uri.as_str())?; - - let request = SidecarCodeActionReq { - file_path, - start_line: params.range.start.line, - start_character: params.range.start.character, - end_line: params.range.end.line, - end_character: params.range.end.character, - }; - let payload = rmp_serde::to_vec(&request)?; - let response_bytes = match runtime.block_on(sidecar.request("textDocument/codeAction", payload)) - { - Ok(bytes) => bytes, - Err(err) => { - warn!("Sidecar codeAction unavailable: {err:#}"); + with_sidecar(req, sidecar, |sidecar, params: CodeActionParams| { + let request = SidecarCodeActionReq { + file_path: crate::paths::uri_to_path(params.text_document.uri.as_str())?, + start_line: params.range.start.line, + start_character: params.range.start.character, + end_line: params.range.end.line, + end_character: params.range.end.character, + }; + let Some(items) = request_sidecar::, _>( + runtime, + sidecar, + "textDocument/codeAction", + &request, + )? + else { return Ok(serde_json::Value::Null); - } - }; - - let items: Vec = rmp_serde::from_slice(&response_bytes)?; - debug!("Got {} code actions from sidecar", items.len()); - - let doc_uri = params.text_document.uri; - let actions: Vec = items - .into_iter() - .map(|item| CodeActionOrCommand::CodeAction(map_code_action(&item, &doc_uri))) - .collect(); - - Ok(serde_json::to_value(actions)?) + }; + debug!("Got {} code actions from sidecar", items.len()); + let doc_uri = params.text_document.uri; + let actions: Vec = items + .iter() + .map(|item| CodeActionOrCommand::CodeAction(map_code_action(item, &doc_uri))) + .collect(); + Ok(serde_json::to_value(actions)?) + }) } /// Handle `codeAction/resolve` — resolves a code action to a full workspace edit. @@ -62,34 +54,30 @@ pub fn handle_code_action_resolve( runtime: &tokio::runtime::Runtime, sidecar: Option<&Arc>, ) -> Result { - let Some(sidecar) = sidecar else { - return Ok(serde_json::Value::Null); - }; - let action: CodeAction = serde_json::from_value(req.params)?; - let data = action - .data - .as_ref() - .ok_or_else(|| anyhow::anyhow!("codeAction/resolve: missing data field"))?; - - // Data format: {"id": N, "uri": "file://..."} — extract just the ID for the sidecar. - let action_id: i32 = data - .get("id") - .and_then(|v| serde_json::from_value(v.clone()).ok()) - .ok_or_else(|| anyhow::anyhow!("codeAction/resolve: missing id in data"))?; - let resolve_req = SidecarCodeActionResolveReq { id: action_id }; - let payload = rmp_serde::to_vec(&resolve_req)?; - let response_bytes = match runtime.block_on(sidecar.request("codeAction/resolve", payload)) { - Ok(bytes) => bytes, - Err(err) => { - warn!("Sidecar codeAction/resolve unavailable: {err:#}"); + with_sidecar(req, sidecar, |sidecar, action: CodeAction| { + let data = action + .data + .as_ref() + .ok_or_else(|| anyhow::anyhow!("codeAction/resolve: missing data field"))?; + // Data format: {"id": N, "uri": "file://..."} — extract just the ID for the sidecar. + let action_id: i32 = data + .get("id") + .and_then(|v| serde_json::from_value(v.clone()).ok()) + .ok_or_else(|| anyhow::anyhow!("codeAction/resolve: missing id in data"))?; + let resolve_req = SidecarCodeActionResolveReq { id: action_id }; + let Some(edit) = request_sidecar::( + runtime, + sidecar, + "codeAction/resolve", + &resolve_req, + )? + else { return Ok(serde_json::Value::Null); - } - }; - - let edit: SidecarWorkspaceEdit = rmp_serde::from_slice(&response_bytes)?; - let mut resolved = action; - resolved.edit = Some(map_workspace_edit(&edit)); - Ok(serde_json::to_value(resolved)?) + }; + let mut resolved = action; + resolved.edit = Some(map_workspace_edit(&edit)); + Ok(serde_json::to_value(resolved)?) + }) } /// Convert a sidecar code action item into an LSP `CodeAction`. diff --git a/src/sharplsp/src/hierarchy.rs b/src/sharplsp/src/hierarchy.rs index 82454495..4e21cbce 100644 --- a/src/sharplsp/src/hierarchy.rs +++ b/src/sharplsp/src/hierarchy.rs @@ -1,14 +1,17 @@ //! What the call- and type-hierarchy handlers share: the sidecar's wire shapes //! for a hierarchy item and a call, the LSP location they map to, and the -//! `prepare` request both hierarchies answer the same way. +//! `prepare` and follow-up requests both hierarchies answer the same way. + +use std::sync::Arc; use anyhow::Result; +use lsp_server::Request; use lsp_types::{Position, Range, TextDocumentPositionParams, Uri}; use tracing::debug; use crate::paths::path_to_lsp_uri; use crate::sidecar::manager::SidecarManager; -use crate::utils::{request_sidecar, SidecarPositionReq}; +use crate::utils::{request_sidecar, with_sidecar, SidecarPositionReq}; /// A hierarchy item returned by the sidecar for call- and type-hierarchy /// requests. Shared by `call_hierarchy` and `type_hierarchy`, which map it @@ -105,28 +108,77 @@ pub fn hierarchy_item_location(item: &SidecarHierarchyItem) -> Option<(Uri, Rang Some((parsed_uri, range, range)) } -/// `prepare` for either hierarchy: the item at the cursor, mapped by `map`. +/// Handle `prepare` for either hierarchy: the item at the cursor, mapped by +/// `map`. The sidecar method is the LSP method, so one handler serves both. /// -/// JSON `null` when the sidecar is unreachable; an empty list when it answered -/// that nothing hierarchical sits at the cursor. -pub fn prepare_hierarchy( +/// JSON `null` when no sidecar is attached or it is unreachable; an empty list +/// when it answered that nothing hierarchical sits at the cursor. +pub fn handle_prepare( + req: Request, runtime: &tokio::runtime::Runtime, - sidecar: &SidecarManager, - method: &str, - at: &TextDocumentPositionParams, + sidecar: Option<&Arc>, map: impl Fn(&SidecarHierarchyItem) -> Option, ) -> Result { - let request = SidecarPositionReq::at(&at.text_document.uri, at.position)?; - let Some(item) = - request_sidecar::, _>(runtime, sidecar, method, &request)? - else { - return Ok(serde_json::Value::Null); - }; - debug!( - method, - found = item.is_some(), - "hierarchy item from sidecar" - ); - let result: Vec = item.as_ref().and_then(map).into_iter().collect(); - Ok(serde_json::to_value(result)?) + let method = req.method.clone(); + with_sidecar(req, sidecar, |sidecar, at: TextDocumentPositionParams| { + let request = SidecarPositionReq::at(&at.text_document.uri, at.position)?; + let Some(item) = request_sidecar::, _>( + runtime, sidecar, &method, &request, + )? + else { + return Ok(serde_json::Value::Null); + }; + debug!( + method, + found = item.is_some(), + "hierarchy item from sidecar" + ); + let result: Vec = item.as_ref().and_then(map).into_iter().collect(); + Ok(serde_json::to_value(result)?) + }) +} + +/// Where the item a follow-up hierarchy request (`incomingCalls`, +/// `supertypes`, …) expands sits: all the sidecar needs of either LSP item. +#[derive(serde::Deserialize)] +#[serde(rename_all = "camelCase")] +struct ItemAnchor { + /// The document declaring the item. + uri: Uri, + /// The item's name, whose start the sidecar resolves the symbol at. + selection_range: Range, +} + +/// The params of every follow-up hierarchy request. +#[derive(serde::Deserialize)] +struct ItemParams { + /// The item being expanded, as a previous `prepare` answered it. + item: ItemAnchor, +} + +/// Handle a follow-up hierarchy request: ask the sidecar (whose method is the +/// LSP method) for the `W` entries related to the item at its selection, and +/// answer each one `map` accepts. +/// +/// An unreachable sidecar answers an empty list, the same as an item with no +/// relatives: the tree shows nothing rather than an error. +pub fn handle_related( + req: Request, + runtime: &tokio::runtime::Runtime, + sidecar: Option<&Arc>, + map: impl Fn(&W) -> Option, +) -> Result { + let method = req.method.clone(); + with_sidecar(req, sidecar, |sidecar, params: ItemParams| { + let request = SidecarPositionReq::at(¶ms.item.uri, params.item.selection_range.start)?; + let related: Vec = + request_sidecar(runtime, sidecar, &method, &request)?.unwrap_or_default(); + debug!( + method, + count = related.len(), + "related hierarchy entries from sidecar" + ); + let result: Vec = related.iter().filter_map(map).collect(); + Ok(serde_json::to_value(result)?) + }) } diff --git a/src/sharplsp/src/inlay_hints.rs b/src/sharplsp/src/inlay_hints.rs index 979330d1..7aefe242 100644 --- a/src/sharplsp/src/inlay_hints.rs +++ b/src/sharplsp/src/inlay_hints.rs @@ -5,9 +5,10 @@ use std::sync::Arc; use anyhow::{bail, Result}; use lsp_server::Request; use lsp_types::{InlayHint, InlayHintKind, InlayHintLabel, InlayHintParams, Position}; -use tracing::{debug, warn}; +use tracing::debug; use crate::sidecar::manager::SidecarManager; +use crate::utils::request_sidecar; use crate::vfs::Vfs; /// Handle `textDocument/inlayHint`. @@ -25,24 +26,20 @@ pub fn handle_inlay_hint( let Some(sidecar) = sidecar else { return Ok(serde_json::Value::Null); }; - let file_path = crate::paths::uri_to_path(params.text_document.uri.as_str())?; - let request = SidecarInlayHintReq { - file_path, + file_path: crate::paths::uri_to_path(params.text_document.uri.as_str())?, start_line: params.range.start.line, end_line: params.range.end.line, }; - let payload = rmp_serde::to_vec(&request)?; - let response_bytes = match runtime.block_on(sidecar.request("textDocument/inlayHint", payload)) - { - Ok(bytes) => bytes, - Err(err) => { - warn!("Sidecar inlayHint unavailable: {err:#}"); - return Ok(serde_json::Value::Null); - } + let Some(items) = request_sidecar::, _>( + runtime, + sidecar, + "textDocument/inlayHint", + &request, + )? + else { + return Ok(serde_json::Value::Null); }; - - let items: Vec = rmp_serde::from_slice(&response_bytes)?; debug!("Got {} inlay hints from sidecar", items.len()); let hints: Vec = items.iter().map(map_inlay_hint).collect(); diff --git a/src/sharplsp/src/main.rs b/src/sharplsp/src/main.rs index b5ee8fae..c847c187 100644 --- a/src/sharplsp/src/main.rs +++ b/src/sharplsp/src/main.rs @@ -867,28 +867,34 @@ fn handle_request( // Call hierarchy CallHierarchyPrepare::METHOD => { let sidecar = pick_sidecar(&req, csharp_sidecar, fsharp_sidecar); - call_hierarchy::handle_prepare(req, runtime, sidecar) + hierarchy::handle_prepare(req, runtime, sidecar, call_hierarchy::map_hierarchy_item) } CallHierarchyIncomingCalls::METHOD => { let sidecar = pick_sidecar(&req, csharp_sidecar, fsharp_sidecar); - call_hierarchy::handle_incoming(req, runtime, sidecar) + hierarchy::handle_related(req, runtime, sidecar, call_hierarchy::incoming_call) } CallHierarchyOutgoingCalls::METHOD => { let sidecar = pick_sidecar(&req, csharp_sidecar, fsharp_sidecar); - call_hierarchy::handle_outgoing(req, runtime, sidecar) + hierarchy::handle_related(req, runtime, sidecar, call_hierarchy::outgoing_call) } // Type hierarchy TypeHierarchyPrepare::METHOD => { let sidecar = pick_sidecar(&req, csharp_sidecar, fsharp_sidecar); - type_hierarchy::handle_prepare(req, runtime, sidecar) - } - TypeHierarchySupertypes::METHOD => { - let sidecar = pick_sidecar(&req, csharp_sidecar, fsharp_sidecar); - type_hierarchy::handle_supertypes(req, runtime, sidecar) + hierarchy::handle_prepare( + req, + runtime, + sidecar, + type_hierarchy::map_type_hierarchy_item, + ) } - TypeHierarchySubtypes::METHOD => { + TypeHierarchySupertypes::METHOD | TypeHierarchySubtypes::METHOD => { let sidecar = pick_sidecar(&req, csharp_sidecar, fsharp_sidecar); - type_hierarchy::handle_subtypes(req, runtime, sidecar) + hierarchy::handle_related( + req, + runtime, + sidecar, + type_hierarchy::map_type_hierarchy_item, + ) } // Code actions CodeActionRequest::METHOD => { diff --git a/src/sharplsp/src/semantic.rs b/src/sharplsp/src/semantic.rs index f82fb84d..4070e504 100644 --- a/src/sharplsp/src/semantic.rs +++ b/src/sharplsp/src/semantic.rs @@ -125,17 +125,15 @@ pub fn handle_completion_resolve( file_path, index: i32::try_from(index).unwrap_or(-1), }; - let payload = rmp_serde::to_vec(&request)?; - let response_bytes = match runtime.block_on(sidecar.request("completionItem/resolve", payload)) - { - Ok(bytes) => bytes, - Err(err) => { - warn!("Sidecar completion resolve unavailable: {err:#}"); - return Ok(serde_json::to_value(item)?); - } + let Some(result) = request_sidecar::( + runtime, + sidecar, + "completionItem/resolve", + &request, + )? + else { + return Ok(serde_json::to_value(item)?); }; - - let result: SidecarCompletionResolveResult = rmp_serde::from_slice(&response_bytes)?; if !result.additional_edits.is_empty() { item.additional_text_edits = Some(map_text_edits(&result.additional_edits)); } @@ -185,16 +183,11 @@ pub fn handle_hover( line: position.line, character: position.character, }; - let payload = rmp_serde::to_vec(&request)?; - let response_bytes = match runtime.block_on(sidecar.request(method, payload)) { - Ok(bytes) => bytes, - Err(err) => { - warn!("Sidecar hover unavailable: {err:#}"); - return Ok(serde_json::Value::Null); - } + let Some(result) = + request_sidecar::, _>(runtime, sidecar, method, &request)? + else { + return Ok(serde_json::Value::Null); }; - - let result: Option = rmp_serde::from_slice(&response_bytes)?; let has_content = result.is_some(); let hover = result.map(|r| { let range = build_hover_range(&r); diff --git a/src/sharplsp/src/type_hierarchy.rs b/src/sharplsp/src/type_hierarchy.rs index cf36b69b..c886d8c0 100644 --- a/src/sharplsp/src/type_hierarchy.rs +++ b/src/sharplsp/src/type_hierarchy.rs @@ -1,93 +1,15 @@ -//! Type hierarchy handlers (`textDocument/prepareTypeHierarchy`, -//! `typeHierarchy/supertypes`, `typeHierarchy/subtypes`). +//! Type hierarchy mapping (`textDocument/prepareTypeHierarchy`, +//! `typeHierarchy/supertypes`, `typeHierarchy/subtypes`): the requests +//! themselves are answered by [`crate::hierarchy`]. -use std::sync::Arc; +use lsp_types::{SymbolKind, TypeHierarchyItem}; -use anyhow::Result; -use lsp_server::Request; -use lsp_types::{ - SymbolKind, TypeHierarchyItem, TypeHierarchyPrepareParams, TypeHierarchySubtypesParams, - TypeHierarchySupertypesParams, -}; -use tracing::debug; - -use crate::hierarchy::{hierarchy_item_location, prepare_hierarchy, SidecarHierarchyItem}; -use crate::sidecar::manager::SidecarManager; -use crate::utils::{request_sidecar, with_sidecar, SidecarPositionReq}; - -/// Handle `textDocument/prepareTypeHierarchy`. -pub fn handle_prepare( - req: Request, - runtime: &tokio::runtime::Runtime, - sidecar: Option<&Arc>, -) -> Result { - with_sidecar( - req, - sidecar, - |sidecar, params: TypeHierarchyPrepareParams| { - prepare_hierarchy( - runtime, - sidecar, - "textDocument/prepareTypeHierarchy", - ¶ms.text_document_position_params, - map_type_hierarchy_item, - ) - }, - ) -} - -/// Handle `typeHierarchy/supertypes`. -pub fn handle_supertypes( - req: Request, - runtime: &tokio::runtime::Runtime, - sidecar: Option<&Arc>, -) -> Result { - with_sidecar( - req, - sidecar, - |sidecar, params: TypeHierarchySupertypesParams| { - related_types(runtime, sidecar, "typeHierarchy/supertypes", ¶ms.item) - }, - ) -} - -/// Handle `typeHierarchy/subtypes`. -pub fn handle_subtypes( - req: Request, - runtime: &tokio::runtime::Runtime, - sidecar: Option<&Arc>, -) -> Result { - with_sidecar( - req, - sidecar, - |sidecar, params: TypeHierarchySubtypesParams| { - related_types(runtime, sidecar, "typeHierarchy/subtypes", ¶ms.item) - }, - ) -} - -/// The types the sidecar relates to `item` via `method` (super- or subtypes). -/// -/// An unreachable sidecar answers an empty list, the same as a type with no -/// relatives: the tree shows nothing rather than an error. -fn related_types( - runtime: &tokio::runtime::Runtime, - sidecar: &SidecarManager, - method: &str, - item: &TypeHierarchyItem, -) -> Result { - let request = SidecarPositionReq::at(&item.uri, item.selection_range.start)?; - let items: Vec = - request_sidecar(runtime, sidecar, method, &request)?.unwrap_or_default(); - debug!(method, count = items.len(), "related types from sidecar"); - let result: Vec = items.iter().filter_map(map_type_hierarchy_item).collect(); - Ok(serde_json::to_value(result)?) -} +use crate::hierarchy::{hierarchy_item_location, SidecarHierarchyItem}; // ── Helpers ──────────────────────────────────────────────────────── /// Convert a sidecar hierarchy item into an LSP `TypeHierarchyItem`. -fn map_type_hierarchy_item(item: &SidecarHierarchyItem) -> Option { +pub fn map_type_hierarchy_item(item: &SidecarHierarchyItem) -> Option { let (uri, range, selection_range) = hierarchy_item_location(item)?; Some(TypeHierarchyItem { name: item.name.clone(),