From 658e28e5fca7a6eece43756700cbf1be30b81b35 Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Thu, 12 Mar 2026 14:31:05 -0700 Subject: [PATCH 1/4] testing: extract helper functions from RunTestTool and add comprehensive tests - Extracts 6 pure helper functions (buildTestRunSummary, getCoverageSummary, getOverallCoverageSummary, getFileCoverageDetails, mergeLineRanges, getFailureDetails) as module-level exports for better testability - Makes RunTestTool class public to allow external instantiation in tests - Adds comprehensive 44-test suite covering all helper functions and tool behavior - Tests include coverage details formatting, failure message handling, test result summarization, and line range merging logic - Uses upcastPartial() for type-safe mock creation without any type casts (Commit message generated by Copilot) --- .../testing/common/testingChatAgentTool.ts | 335 +++++---- .../test/common/testingChatAgentTool.test.ts | 693 ++++++++++++++++++ 2 files changed, 897 insertions(+), 131 deletions(-) create mode 100644 src/vs/workbench/contrib/testing/test/common/testingChatAgentTool.test.ts diff --git a/src/vs/workbench/contrib/testing/common/testingChatAgentTool.ts b/src/vs/workbench/contrib/testing/common/testingChatAgentTool.ts index 94cb8a0f182..5d3aae6010b 100644 --- a/src/vs/workbench/contrib/testing/common/testingChatAgentTool.ts +++ b/src/vs/workbench/contrib/testing/common/testingChatAgentTool.ts @@ -31,14 +31,14 @@ import { ToolProgress, } from '../../chat/common/tools/languageModelToolsService.js'; import { TestId } from './testId.js'; -import { FileCoverage, getTotalCoveragePercent } from './testCoverage.js'; +import { FileCoverage, TestCoverage, getTotalCoveragePercent } from './testCoverage.js'; import { TestingContextKeys } from './testingContextKeys.js'; import { collectTestStateCounts, getTestProgressText } from './testingProgressMessages.js'; import { isFailedState } from './testingStates.js'; import { LiveTestResult } from './testResult.js'; import { ITestResultService } from './testResultService.js'; import { ITestService, testsInFile, waitForTestToBeIdle } from './testService.js'; -import { IncrementalTestCollectionItem, TestItemExpandState, TestMessageType, TestResultState, TestRunProfileBitset } from './testTypes.js'; +import { DetailType, IncrementalTestCollectionItem, TestItemExpandState, TestMessageType, TestResultState, TestRunProfileBitset } from './testTypes.js'; import { Position } from '../../../../editor/common/core/position.js'; import { ITestProfileService } from './testProfileService.js'; @@ -70,7 +70,7 @@ interface IRunTestToolParams { mode?: Mode; } -class RunTestTool implements IToolImpl { +export class RunTestTool implements IToolImpl { public static readonly ID = 'runTests'; public static readonly DEFINITION: IToolData = { id: this.ID, @@ -101,7 +101,7 @@ class RunTestTool implements IToolImpl { coverageFiles: { type: 'array', items: { type: 'string' }, - description: 'When mode="coverage": absolute file paths to include detailed coverage info for. Only the first matching file will be summarized.' + description: 'When mode="coverage": absolute file paths to include detailed coverage info for. If not provided, a file-level summary of all files with incomplete coverage is shown.' } }, }, @@ -168,7 +168,7 @@ class RunTestTool implements IToolImpl { }; } - const summary = await this._buildSummary(result, mode, coverageFiles); + const summary = await buildTestRunSummary(result, mode, coverageFiles); const content = [{ kind: 'text', value: summary } as const]; return { @@ -177,132 +177,6 @@ class RunTestTool implements IToolImpl { }; } - private async _buildSummary(result: LiveTestResult, mode: Mode, coverageFiles: string[] | undefined): Promise { - const failures = result.counts[TestResultState.Errored] + result.counts[TestResultState.Failed]; - let str = `\n`; - if (failures !== 0) { - str += await this._getFailureDetails(result); - } - if (mode === 'coverage') { - str += await this._getCoverageSummary(result, coverageFiles); - } - return str; - } - - private async _getCoverageSummary(result: LiveTestResult, coverageFiles: string[] | undefined): Promise { - if (!coverageFiles || !coverageFiles.length) { - return ''; - } - for (const task of result.tasks) { - const coverage = task.coverage.get(); - if (!coverage) { - continue; - } - const normalized = coverageFiles.map(file => URI.file(file).fsPath); - const coveredFilesMap = new Map(); - for (const file of coverage.getAllFiles().values()) { - coveredFilesMap.set(file.uri.fsPath, file); - } - for (const path of normalized) { - const file = coveredFilesMap.get(path); - if (!file) { - continue; - } - let summary = `\n`; - const pct = getTotalCoveragePercent(file.statement, file.branch, file.declaration) * 100; - summary += `\n`; - summary += `\n`; - return summary; - } - } - return ''; - } - - private async _getFailureDetails(result: LiveTestResult): Promise { - let str = ''; - let hadMessages = false; - for (const failure of result.tests) { - if (!isFailedState(failure.ownComputedState)) { - continue; - } - - const [, ...testPath] = TestId.split(failure.item.extId); - const testName = testPath.pop(); - str += ` '))}>\n`; - // Extract detailed failure information from error messages - for (const task of failure.tasks) { - for (const message of task.messages.filter(m => m.type === TestMessageType.Error)) { - hadMessages = true; - - // Add expected/actual outputs if available - if (message.expected !== undefined && message.actual !== undefined) { - str += `\n${message.expected}\n\n`; - str += `\n${message.actual}\n\n`; - } else { - // Fallback to the message content - const messageText = typeof message.message === 'string' ? message.message : message.message.value; - str += `\n${messageText}\n\n`; - } - - // Add stack trace information if available (limit to first 10 frames) - if (message.stackTrace && message.stackTrace.length > 0) { - for (const frame of message.stackTrace.slice(0, 10)) { - if (frame.uri && frame.position) { - str += `\n`; - } else if (frame.uri) { - str += `${frame.label}\n`; - } else { - str += `${frame.label}\n`; - } - } - } - - // Add location information if available - if (message.location) { - str += `\n`; - } - } - } - - str += `\n`; - } - - if (!hadMessages) { // some adapters don't have any per-test messages and just output - const output = result.tasks.map(t => t.output.getRange(0, t.output.length).toString().trim()).join('\n'); - if (output) { - str += `\n${output}\n\n`; - } - } - - return str; - } - /** Updates the UI progress as the test runs, resolving when the run is finished. */ private async _monitorRunProgress(result: LiveTestResult, progress: ToolProgress, token: CancellationToken): Promise { const store = new DisposableStore(); @@ -451,3 +325,202 @@ class RunTestTool implements IToolImpl { }); } } + +/** Builds the full summary string for a completed test run. */ +export async function buildTestRunSummary(result: LiveTestResult, mode: Mode, coverageFiles: string[] | undefined): Promise { + const failures = result.counts[TestResultState.Errored] + result.counts[TestResultState.Failed]; + let str = `\n`; + if (failures !== 0) { + str += await getFailureDetails(result); + } + if (mode === 'coverage') { + str += await getCoverageSummary(result, coverageFiles); + } + return str; +} + +/** Gets a coverage summary from a test result, either overall or per-file. */ +export async function getCoverageSummary(result: LiveTestResult, coverageFiles: string[] | undefined): Promise { + for (const task of result.tasks) { + const coverage = task.coverage.get(); + if (!coverage) { + continue; + } + + if (!coverageFiles || !coverageFiles.length) { + return getOverallCoverageSummary(coverage); + } + + const normalized = coverageFiles.map(file => URI.file(file).fsPath); + const coveredFilesMap = new Map(); + for (const file of coverage.getAllFiles().values()) { + coveredFilesMap.set(file.uri.fsPath, file); + } + + let str = ''; + for (const path of normalized) { + const file = coveredFilesMap.get(path); + if (!file) { + continue; + } + str += await getFileCoverageDetails(file, path); + } + return str; + } + return ''; +} + +/** Gets a file-level coverage overview sorted by lowest coverage first. */ +export function getOverallCoverageSummary(coverage: TestCoverage): string { + const files = [...coverage.getAllFiles().values()] + .map(f => ({ path: f.uri.fsPath, pct: getTotalCoveragePercent(f.statement, f.branch, f.declaration) * 100 })) + .filter(f => f.pct < 100) + .sort((a, b) => a.pct - b.pct); + + if (!files.length) { + return 'All files have 100% coverage.\n'; + } + + let str = '\n'; + for (const f of files) { + str += `\n`; + } + str += '\n'; + return str; +} + +/** Gets detailed coverage information for a single file including uncovered items. */ +export async function getFileCoverageDetails(file: FileCoverage, path: string): Promise { + const pct = getTotalCoveragePercent(file.statement, file.branch, file.declaration) * 100; + let str = ` `${d.name}(L${d.line})`).join(', ') + '\n'; + } + if (uncoveredBranches.length) { + str += 'uncovered branches: ' + uncoveredBranches.map(b => b.label ? `L${b.line}(${b.label})` : `L${b.line}`).join(', ') + '\n'; + } + if (uncoveredLines.length) { + str += 'uncovered lines: ' + mergeLineRanges(uncoveredLines) + '\n'; + } + } catch { /* ignore - details not available */ } + + str += '\n'; + return str; +} + +/** Merges overlapping/contiguous line ranges and formats them compactly. */ +export function mergeLineRanges(ranges: [number, number][]): string { + if (!ranges.length) { + return ''; + } + ranges.sort((a, b) => a[0] - b[0]); + const merged: [number, number][] = [ranges[0]]; + for (let i = 1; i < ranges.length; i++) { + const last = merged[merged.length - 1]; + const [start, end] = ranges[i]; + if (start <= last[1] + 1) { + last[1] = Math.max(last[1], end); + } else { + merged.push([start, end]); + } + } + return merged.map(([s, e]) => s === e ? `${s}` : `${s}-${e}`).join(', '); +} + +/** Formats failure details from a test result into an XML-like string. */ +export async function getFailureDetails(result: LiveTestResult): Promise { + let str = ''; + let hadMessages = false; + for (const failure of result.tests) { + if (!isFailedState(failure.ownComputedState)) { + continue; + } + + const [, ...testPath] = TestId.split(failure.item.extId); + const testName = testPath.pop(); + str += ` '))}>\n`; + for (const task of failure.tasks) { + for (const message of task.messages.filter(m => m.type === TestMessageType.Error)) { + hadMessages = true; + + if (message.expected !== undefined && message.actual !== undefined) { + str += `\n${message.expected}\n\n`; + str += `\n${message.actual}\n\n`; + } else { + const messageText = typeof message.message === 'string' ? message.message : message.message.value; + str += `\n${messageText}\n\n`; + } + + if (message.stackTrace && message.stackTrace.length > 0) { + for (const frame of message.stackTrace.slice(0, 10)) { + if (frame.uri && frame.position) { + str += `\n`; + } else if (frame.uri) { + str += `${frame.label}\n`; + } else { + str += `${frame.label}\n`; + } + } + } + + if (message.location) { + str += `\n`; + } + } + } + + str += `\n`; + } + + if (!hadMessages) { + const output = result.tasks.map(t => t.output.getRange(0, t.output.length).toString().trim()).join('\n'); + if (output) { + str += `\n${output}\n\n`; + } + } + + return str; +} diff --git a/src/vs/workbench/contrib/testing/test/common/testingChatAgentTool.test.ts b/src/vs/workbench/contrib/testing/test/common/testingChatAgentTool.test.ts new file mode 100644 index 00000000000..914433c13bb --- /dev/null +++ b/src/vs/workbench/contrib/testing/test/common/testingChatAgentTool.test.ts @@ -0,0 +1,693 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import { CancellationToken } from '../../../../../base/common/cancellation.js'; +import { Event } from '../../../../../base/common/event.js'; +import { URI } from '../../../../../base/common/uri.js'; +import { upcastPartial } from '../../../../../base/test/common/mock.js'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; +import { Range } from '../../../../../editor/common/core/range.js'; +import { Position } from '../../../../../editor/common/core/position.js'; +import { NullTelemetryService } from '../../../../../platform/telemetry/common/telemetryUtils.js'; +import { IExtUri } from '../../../../../base/common/resources.js'; +import { IUriIdentityService } from '../../../../../platform/uriIdentity/common/uriIdentity.js'; +import { IWorkspace, IWorkspaceContextService, IWorkspaceFolder } from '../../../../../platform/workspace/common/workspace.js'; +import { IToolInvocation, IToolInvocationPreparationContext, IToolProgressStep } from '../../../chat/common/tools/languageModelToolsService.js'; +import { FileCoverage, ICoverageAccessor, TestCoverage } from '../../common/testCoverage.js'; +import { LiveTestResult } from '../../common/testResult.js'; +import { ITestResultService } from '../../common/testResultService.js'; +import { IMainThreadTestCollection, ITestService } from '../../common/testService.js'; +import { CoverageDetails, DetailType, IBranchCoverage, IDeclarationCoverage, IFileCoverage, IStatementCoverage, ResolvedTestRunRequest, TestMessageType, TestResultState, TestRunProfileBitset } from '../../common/testTypes.js'; +import { ITestProfileService } from '../../common/testProfileService.js'; +import { observableValue } from '../../../../../base/common/observable.js'; +import { VSBuffer } from '../../../../../base/common/buffer.js'; +import { TestId } from '../../common/testId.js'; +import { RunTestTool, buildTestRunSummary, getCoverageSummary, getOverallCoverageSummary, getFileCoverageDetails, mergeLineRanges, getFailureDetails } from '../../common/testingChatAgentTool.js'; + +suite('Workbench - RunTestTool', () => { + const ds = ensureNoDisposablesAreLeakedInTestSuite(); + + let insertCounter = 0; + let tool: RunTestTool; + + const noopProgress = { + report: (_update: IToolProgressStep) => { }, + }; + const noopCountTokens = () => Promise.resolve(0); + + function createLiveTestResult(request?: ResolvedTestRunRequest): LiveTestResult { + const req = request ?? { + group: TestRunProfileBitset.Run, + targets: [{ profileId: 0, controllerId: 'ctrlId', testIds: ['id-a'] }], + }; + return ds.add(new LiveTestResult( + `result-${insertCounter++}`, + false, + req, + insertCounter, + NullTelemetryService, + )); + } + + function createTestCoverage(files: { uri: URI; statement: { covered: number; total: number }; branch?: { covered: number; total: number }; declaration?: { covered: number; total: number }; details?: CoverageDetails[] }[]): TestCoverage { + const result = createLiveTestResult(); + const accessor: ICoverageAccessor = { + getCoverageDetails: (id, _testId, _token) => { + const entry = files.find(f => f.uri.toString() === id); + return Promise.resolve(entry?.details ?? []); + }, + }; + const uriIdentity = upcastPartial({ + asCanonicalUri: (uri: URI) => uri, + extUri: upcastPartial({ + isEqual: (a: URI, b: URI) => a.toString() === b.toString(), + ignorePathCasing: () => false, + }), + }); + const coverage = new TestCoverage(result, 'task-1', uriIdentity, accessor); + for (const f of files) { + const fileCoverage: IFileCoverage = { + id: f.uri.toString(), + uri: f.uri, + statement: f.statement, + branch: f.branch, + declaration: f.declaration, + }; + coverage.append(fileCoverage, undefined); + } + return coverage; + } + + function makeStatement(line: number, count: number, endLine?: number, branches?: IBranchCoverage[]): IStatementCoverage { + return { + type: DetailType.Statement, + count, + location: new Range(line, 1, endLine ?? line, 1), + branches, + }; + } + + function makeDeclaration(name: string, line: number, count: number): IDeclarationCoverage { + return { + type: DetailType.Declaration, + name, + count, + location: new Range(line, 1, line, 1), + }; + } + + function makeBranch(line: number, count: number, label?: string): IBranchCoverage { + return { + count, + label, + location: new Range(line, 1, line, 1), + }; + } + + function createResultWithCoverage(coverageData: TestCoverage): LiveTestResult { + const result = createLiveTestResult(); + result.addTask({ id: 'task-1', name: 'Test Task', running: true, ctrlId: 'ctrlId' }); + const taskCov = result.tasks[0].coverage as ReturnType>; + taskCov.set(coverageData, undefined); + return result; + } + + function createResultWithTests(tests: { extId: string; label: string; state: TestResultState; messages?: { type: TestMessageType; message: string; expected?: string; actual?: string; location?: { uri: URI; range: Range }; stackTrace?: { uri?: URI; position?: { lineNumber: number; column: number }; label: string }[] }[] }[]): LiveTestResult { + const result = createLiveTestResult(); + result.addTask({ id: 't', name: 'Test Task', running: true, ctrlId: 'ctrlId' }); + + for (const t of tests) { + const chain = TestId.split(t.extId); + const items = chain.map((segment, i) => ({ + extId: new TestId(chain.slice(0, i + 1)).toString(), + label: i === chain.length - 1 ? t.label : segment, + busy: false, + description: null, + error: null, + range: null, + sortText: null, + tags: [], + uri: undefined, + })); + result.addTestChainToRun('ctrlId', items); + } + + for (const t of tests) { + result.updateState(t.extId, 't', t.state); + if (t.messages) { + for (const msg of t.messages) { + result.appendMessage(t.extId, 't', { + type: msg.type as TestMessageType.Error, + message: msg.message, + expected: msg.expected, + actual: msg.actual, + contextValue: undefined, + location: msg.location ? { uri: msg.location.uri, range: msg.location.range } : undefined, + stackTrace: msg.stackTrace?.map(f => ({ + uri: f.uri, + position: f.position ? new Position(f.position.lineNumber, f.position.column) : undefined, + label: f.label, + })), + }); + } + } + } + + return result; + } + + function createFileCov(uri: URI, statement: { covered: number; total: number }, details: CoverageDetails[], opts?: { branch?: { covered: number; total: number }; declaration?: { covered: number; total: number } }): FileCoverage { + const result = createLiveTestResult(); + const accessor: ICoverageAccessor = { + getCoverageDetails: () => Promise.resolve(details), + }; + return new FileCoverage({ id: 'file-1', uri, statement, branch: opts?.branch, declaration: opts?.declaration }, result, accessor); + } + + setup(() => { + insertCounter = 0; + + const mockTestService = upcastPartial({ + collection: upcastPartial({ + rootItems: [], + rootIds: [], + expand: () => Promise.resolve(), + getNodeById: () => undefined, + getNodeByUrl: () => [], + }), + runTests: () => Promise.resolve(upcastPartial({})), + cancelTestRun: () => { }, + }); + + const mockResultService = upcastPartial({ + onResultsChanged: Event.None, + }); + + const mockProfileService = upcastPartial({ + capabilitiesForTest: () => TestRunProfileBitset.Run | TestRunProfileBitset.Coverage, + }); + + const mockUriIdentity = upcastPartial({ + asCanonicalUri: (uri: URI) => uri, + extUri: upcastPartial({ isEqual: (a: URI, b: URI) => a.toString() === b.toString() }), + }); + + const mockWorkspaceContext = upcastPartial({ + getWorkspace: () => upcastPartial({ id: 'test', folders: [upcastPartial({ uri: URI.file('/workspace') })] }), + }); + + tool = new RunTestTool( + mockTestService, + mockUriIdentity, + mockWorkspaceContext, + mockResultService, + mockProfileService, + ); + }); + + suite('invoke', () => { + test('returns error when no tests found', async () => { + const result = await tool.invoke( + upcastPartial({ parameters: { files: ['/nonexistent/test.ts'] } }), + noopCountTokens, noopProgress, CancellationToken.None, + ); + assert.ok(result.toolResultError); + assert.ok(result.content[0].kind === 'text' && result.content[0].value.includes('No tests found')); + }); + }); + + suite('_buildSummary', () => { + test('includes pass/fail counts', async () => { + const result = createResultWithTests([ + { extId: new TestId(['ctrlId', 'a']).toString(), label: 'a', state: TestResultState.Passed }, + { extId: new TestId(['ctrlId', 'b']).toString(), label: 'b', state: TestResultState.Failed, messages: [{ type: TestMessageType.Error, message: 'boom' }] }, + ]); + result.markComplete(); + + const summary = await buildTestRunSummary(result, 'run', undefined); + assert.ok(summary.includes('')); + }); + + test('combines errored and failed in failure count', async () => { + const result = createResultWithTests([ + { extId: new TestId(['ctrlId', 'a']).toString(), label: 'a', state: TestResultState.Failed, messages: [{ type: TestMessageType.Error, message: 'fail' }] }, + { extId: new TestId(['ctrlId', 'b']).toString(), label: 'b', state: TestResultState.Errored, messages: [{ type: TestMessageType.Error, message: 'error' }] }, + { extId: new TestId(['ctrlId', 'c']).toString(), label: 'c', state: TestResultState.Passed }, + ]); + result.markComplete(); + + const summary = await buildTestRunSummary(result, 'run', undefined); + assert.ok(summary.includes('failed=2')); + }); + + test('includes coverage when mode is coverage', async () => { + const coverageData = createTestCoverage([ + { uri: URI.file('/src/a.ts'), statement: { covered: 8, total: 10 } }, + ]); + const result = createResultWithCoverage(coverageData); + result.markComplete(); + + const summary = await buildTestRunSummary(result, 'coverage', undefined); + assert.ok(summary.includes('')); + }); + + test('omits coverage when mode is run', async () => { + const result = createLiveTestResult(); + result.addTask({ id: 't', name: 'n', running: true, ctrlId: 'ctrl' }); + result.markComplete(); + + const summary = await buildTestRunSummary(result, 'run', undefined); + assert.ok(!summary.includes(' { + test('returns overall summary when no coverageFiles specified', async () => { + const coverageData = createTestCoverage([ + { uri: URI.file('/src/a.ts'), statement: { covered: 5, total: 10 } }, + { uri: URI.file('/src/b.ts'), statement: { covered: 10, total: 10 } }, + ]); + const result = createResultWithCoverage(coverageData); + + const summary = await getCoverageSummary(result, undefined); + assert.ok(summary.includes('')); + assert.ok(summary.includes('/src/a.ts')); + assert.ok(!summary.includes('/src/b.ts')); // 100% covered, excluded + }); + + test('returns detailed summary for specified coverageFiles', async () => { + const details: CoverageDetails[] = [ + makeDeclaration('uncoveredFn', 10, 0), + makeStatement(20, 0, 25), + ]; + const coverageData = createTestCoverage([ + { uri: URI.file('/src/a.ts'), statement: { covered: 8, total: 10 }, declaration: { covered: 0, total: 1 }, details }, + ]); + const result = createResultWithCoverage(coverageData); + + const summary = await getCoverageSummary(result, ['/src/a.ts']); + assert.ok(summary.includes(' { + const result = createLiveTestResult(); + result.addTask({ id: 't', name: 'n', running: true, ctrlId: 'ctrl' }); + + const summary = await getCoverageSummary(result, ['/src/a.ts']); + assert.strictEqual(summary, ''); + }); + + test('handles multiple coverageFiles', async () => { + const coverageData = createTestCoverage([ + { uri: URI.file('/src/a.ts'), statement: { covered: 8, total: 10 }, details: [makeStatement(5, 0)] }, + { uri: URI.file('/src/b.ts'), statement: { covered: 3, total: 10 }, details: [makeDeclaration('fn', 1, 0)] }, + ]); + const result = createResultWithCoverage(coverageData); + + const summary = await getCoverageSummary(result, ['/src/a.ts', '/src/b.ts']); + assert.ok(summary.includes('/src/a.ts')); + assert.ok(summary.includes('/src/b.ts')); + }); + + test('skips non-matching coverageFiles gracefully', async () => { + const coverageData = createTestCoverage([ + { uri: URI.file('/src/a.ts'), statement: { covered: 8, total: 10 } }, + ]); + const result = createResultWithCoverage(coverageData); + + const summary = await getCoverageSummary(result, ['/src/nonexistent.ts']); + assert.strictEqual(summary, ''); + }); + }); + + suite('getOverallCoverageSummary', () => { + test('returns all-covered message when everything is 100%', () => { + const coverage = createTestCoverage([ + { uri: URI.file('/src/a.ts'), statement: { covered: 10, total: 10 } }, + { uri: URI.file('/src/b.ts'), statement: { covered: 5, total: 5 } }, + ]); + assert.strictEqual( + getOverallCoverageSummary(coverage), + 'All files have 100% coverage.\n', + ); + }); + + test('sorts files by coverage ascending', () => { + const coverage = createTestCoverage([ + { uri: URI.file('/src/high.ts'), statement: { covered: 9, total: 10 } }, + { uri: URI.file('/src/low.ts'), statement: { covered: 3, total: 10 } }, + { uri: URI.file('/src/mid.ts'), statement: { covered: 7, total: 10 } }, + ]); + const summary = getOverallCoverageSummary(coverage); + const lowIdx = summary.indexOf('/src/low.ts'); + const midIdx = summary.indexOf('/src/mid.ts'); + const highIdx = summary.indexOf('/src/high.ts'); + assert.ok(lowIdx < midIdx && midIdx < highIdx); + }); + + test('excludes 100% files from listing', () => { + const coverage = createTestCoverage([ + { uri: URI.file('/src/partial.ts'), statement: { covered: 5, total: 10 } }, + { uri: URI.file('/src/full.ts'), statement: { covered: 10, total: 10 } }, + ]); + const summary = getOverallCoverageSummary(coverage); + assert.ok(summary.includes('/src/partial.ts')); + assert.ok(!summary.includes('/src/full.ts')); + }); + + test('includes percentage in output', () => { + const coverage = createTestCoverage([ + { uri: URI.file('/src/a.ts'), statement: { covered: 7, total: 10 } }, + ]); + const summary = getOverallCoverageSummary(coverage); + assert.ok(summary.includes('percent=70.0')); + }); + }); + + suite('getFileCoverageDetails', () => { + test('shows header with statement counts', async () => { + const file = createFileCov(URI.file('/src/foo.ts'), { covered: 8, total: 10 }, []); + const output = await getFileCoverageDetails(file, '/src/foo.ts'); + assert.ok(output.includes('statements=8/10')); + assert.ok(output.includes('percent=80.0')); + assert.ok(output.startsWith('\n')); + }); + + test('includes branch counts when available', async () => { + const file = createFileCov(URI.file('/src/foo.ts'), { covered: 8, total: 10 }, [], { branch: { covered: 3, total: 5 } }); + const output = await getFileCoverageDetails(file, '/src/foo.ts'); + assert.ok(output.includes('branches=3/5')); + }); + + test('includes declaration counts when available', async () => { + const file = createFileCov(URI.file('/src/foo.ts'), { covered: 8, total: 10 }, [], { declaration: { covered: 2, total: 4 } }); + const output = await getFileCoverageDetails(file, '/src/foo.ts'); + assert.ok(output.includes('declarations=2/4')); + }); + + test('omits branch/declaration when not available', async () => { + const file = createFileCov(URI.file('/src/foo.ts'), { covered: 8, total: 10 }, []); + const output = await getFileCoverageDetails(file, '/src/foo.ts'); + assert.ok(!output.includes('branches=')); + assert.ok(!output.includes('declarations=')); + }); + + test('lists uncovered declarations', async () => { + const details: CoverageDetails[] = [ + makeDeclaration('handleError', 89, 0), + makeDeclaration('processQueue', 120, 0), + makeDeclaration('coveredFn', 50, 3), + ]; + const file = createFileCov(URI.file('/src/foo.ts'), { covered: 8, total: 10 }, details, { declaration: { covered: 1, total: 3 } }); + const output = await getFileCoverageDetails(file, '/src/foo.ts'); + assert.ok(output.includes('uncovered functions: handleError(L89), processQueue(L120)')); + assert.ok(!output.includes('coveredFn')); + }); + + test('lists uncovered branches with labels', async () => { + const details: CoverageDetails[] = [ + makeStatement(34, 5, undefined, [ + makeBranch(34, 5, 'then'), + makeBranch(36, 0, 'else'), + ]), + makeStatement(56, 2, undefined, [ + makeBranch(56, 0, 'case "foo"'), + makeBranch(58, 2, 'case "bar"'), + ]), + ]; + const file = createFileCov(URI.file('/src/foo.ts'), { covered: 8, total: 10 }, details, { branch: { covered: 2, total: 4 } }); + const output = await getFileCoverageDetails(file, '/src/foo.ts'); + assert.ok(output.includes('uncovered branches: L36(else), L56(case "foo")')); + }); + + test('lists uncovered branches without labels', async () => { + const details: CoverageDetails[] = [ + makeStatement(10, 1, undefined, [makeBranch(10, 0)]), + ]; + const file = createFileCov(URI.file('/src/foo.ts'), { covered: 8, total: 10 }, details); + const output = await getFileCoverageDetails(file, '/src/foo.ts'); + assert.ok(output.includes('uncovered branches: L10\n')); + }); + + test('uses parent statement location when branch has no location', async () => { + const details: CoverageDetails[] = [ + makeStatement(42, 1, undefined, [{ count: 0, label: 'else' }]), + ]; + const file = createFileCov(URI.file('/src/foo.ts'), { covered: 8, total: 10 }, details); + const output = await getFileCoverageDetails(file, '/src/foo.ts'); + assert.ok(output.includes('L42(else)')); + }); + + test('lists merged uncovered line ranges', async () => { + const details: CoverageDetails[] = [ + makeStatement(23, 0, 27), + makeStatement(28, 0, 30), + makeStatement(45, 0), + makeStatement(67, 0, 72), + makeStatement(100, 0, 105), + makeStatement(50, 5), // covered + ]; + const file = createFileCov(URI.file('/src/foo.ts'), { covered: 5, total: 11 }, details); + const output = await getFileCoverageDetails(file, '/src/foo.ts'); + assert.ok(output.includes('uncovered lines: 23-30, 45, 67-72, 100-105')); + }); + + test('omits uncovered sections when all covered', async () => { + const details: CoverageDetails[] = [ + makeDeclaration('fn', 10, 3), + makeStatement(20, 5), + makeStatement(30, 1, undefined, [makeBranch(30, 1, 'then'), makeBranch(32, 2, 'else')]), + ]; + const file = createFileCov(URI.file('/src/foo.ts'), { covered: 10, total: 10 }, details); + const output = await getFileCoverageDetails(file, '/src/foo.ts'); + assert.ok(!output.includes('uncovered')); + }); + + test('handles details() throwing gracefully', async () => { + const result = createLiveTestResult(); + const accessor: ICoverageAccessor = { + getCoverageDetails: () => Promise.reject(new Error('not available')), + }; + const file = new FileCoverage({ id: 'err', uri: URI.file('/src/err.ts'), statement: { covered: 5, total: 10 } }, result, accessor); + const output = await getFileCoverageDetails(file, '/src/err.ts'); + assert.ok(output.includes('')); + assert.ok(!output.includes('uncovered')); + }); + + test('full output snapshot', async () => { + const details: CoverageDetails[] = [ + makeDeclaration('uncoveredFn', 10, 0), + makeDeclaration('coveredFn', 20, 3), + makeStatement(30, 0, 32), + makeStatement(40, 5, undefined, [ + makeBranch(40, 5, 'then'), + makeBranch(42, 0, 'else'), + ]), + makeStatement(50, 3), + ]; + const file = createFileCov( + URI.file('/src/foo.ts'), + { covered: 8, total: 10 }, + details, + { branch: { covered: 1, total: 2 }, declaration: { covered: 1, total: 2 } }, + ); + assert.deepStrictEqual( + await getFileCoverageDetails(file, '/src/foo.ts'), + '\n' + + 'uncovered functions: uncoveredFn(L10)\n' + + 'uncovered branches: L42(else)\n' + + 'uncovered lines: 30-32\n' + + '\n', + ); + }); + }); + + suite('mergeLineRanges', () => { + test('returns empty for empty input', () => { + assert.strictEqual(mergeLineRanges([]), ''); + }); + + test('single range', () => { + assert.strictEqual(mergeLineRanges([[5, 10]]), '5-10'); + }); + + test('single line', () => { + assert.strictEqual(mergeLineRanges([[5, 5]]), '5'); + }); + + test('merges contiguous ranges', () => { + assert.strictEqual(mergeLineRanges([[1, 3], [4, 6]]), '1-6'); + }); + + test('keeps non-contiguous ranges separate', () => { + assert.strictEqual(mergeLineRanges([[1, 3], [10, 12]]), '1-3, 10-12'); + }); + + test('merges overlapping ranges', () => { + assert.strictEqual(mergeLineRanges([[1, 5], [3, 8]]), '1-8'); + }); + + test('merges adjacent single-line ranges', () => { + assert.strictEqual(mergeLineRanges([[5, 5], [6, 6], [10, 10]]), '5-6, 10'); + }); + + test('handles unsorted input', () => { + assert.strictEqual(mergeLineRanges([[10, 12], [1, 3], [4, 6]]), '1-6, 10-12'); + }); + + test('handles complex mixed ranges', () => { + assert.strictEqual(mergeLineRanges([[1, 1], [3, 5], [2, 2], [7, 9], [10, 10]]), '1-5, 7-10'); + }); + }); + + suite('getFailureDetails', () => { + test('formats expected/actual outputs', async () => { + const result = createResultWithTests([{ + extId: new TestId(['ctrlId', 'suite', 'myTest']).toString(), + label: 'myTest', + state: TestResultState.Failed, + messages: [{ + type: TestMessageType.Error, + message: 'Assertion failed', + expected: 'hello', + actual: 'world', + }], + }]); + result.markComplete(); + + const output = await getFailureDetails(result); + assert.ok(output.includes('\nhello\n')); + assert.ok(output.includes('\nworld\n')); + }); + + test('formats plain message when no expected/actual', async () => { + const result = createResultWithTests([{ + extId: new TestId(['ctrlId', 'myTest']).toString(), + label: 'myTest', + state: TestResultState.Failed, + messages: [{ + type: TestMessageType.Error, + message: 'Something went wrong', + }], + }]); + result.markComplete(); + + const output = await getFailureDetails(result); + assert.ok(output.includes('\nSomething went wrong\n')); + }); + + test('includes test name and path', async () => { + const result = createResultWithTests([{ + extId: new TestId(['ctrlId', 'suite1', 'suite2', 'myTest']).toString(), + label: 'myTest', + state: TestResultState.Failed, + messages: [{ type: TestMessageType.Error, message: 'fail' }], + }]); + result.markComplete(); + + const output = await getFailureDetails(result); + assert.ok(output.includes('name="myTest"')); + assert.ok(output.includes('path="suite1 > suite2"')); + }); + + test('includes stack trace frames', async () => { + const result = createResultWithTests([{ + extId: new TestId(['ctrlId', 'myTest']).toString(), + label: 'myTest', + state: TestResultState.Failed, + messages: [{ + type: TestMessageType.Error, + message: 'fail', + stackTrace: [ + { uri: URI.file('/src/test.ts'), position: { lineNumber: 10, column: 5 }, label: 'testFn' }, + { uri: URI.file('/src/helper.ts'), position: undefined, label: 'helperFn' }, + { uri: undefined, position: undefined, label: 'anonymous' }, + ], + }], + }]); + result.markComplete(); + + const output = await getFailureDetails(result); + assert.ok(output.includes('path="/src/test.ts" line="10" col="5"')); + assert.ok(output.includes('path="/src/helper.ts">helperFn')); + assert.ok(output.includes('>anonymous')); + }); + + test('includes location information', async () => { + const result = createResultWithTests([{ + extId: new TestId(['ctrlId', 'myTest']).toString(), + label: 'myTest', + state: TestResultState.Failed, + messages: [{ + type: TestMessageType.Error, + message: 'fail', + location: { uri: URI.file('/src/test.ts'), range: new Range(42, 8, 42, 20) }, + }], + }]); + result.markComplete(); + + const output = await getFailureDetails(result); + assert.ok(output.includes('path="/src/test.ts" line="42" col="8"')); + }); + + test('skips passing tests', async () => { + const result = createResultWithTests([ + { extId: new TestId(['ctrlId', 'pass']).toString(), label: 'pass', state: TestResultState.Passed }, + { extId: new TestId(['ctrlId', 'fail']).toString(), label: 'fail', state: TestResultState.Failed, messages: [{ type: TestMessageType.Error, message: 'boom' }] }, + ]); + result.markComplete(); + + const output = await getFailureDetails(result); + assert.ok(!output.includes('name="pass"')); + assert.ok(output.includes('name="fail"')); + }); + + test('shows task output when no per-test messages', async () => { + const result = createResultWithTests([{ + extId: new TestId(['ctrlId', 'myTest']).toString(), + label: 'myTest', + state: TestResultState.Failed, + }]); + result.appendOutput(VSBuffer.fromString('raw test output'), 't'); + result.markComplete(); + + const output = await getFailureDetails(result); + assert.ok(output.includes('\nraw test output\n')); + }); + }); + + suite('prepareToolInvocation', () => { + test('shows file names in confirmation', async () => { + const prepared = await tool.prepareToolInvocation( + upcastPartial({ parameters: { files: ['/path/to/test1.ts', '/path/to/test2.ts'] }, toolCallId: 'call-1', chatSessionResource: undefined }), + CancellationToken.None, + ); + assert.ok(prepared); + const msg = prepared.confirmationMessages?.message; + assert.ok(msg); + const msgStr = typeof msg === 'string' ? msg : msg.value; + assert.ok(msgStr.includes('test1.ts')); + assert.ok(msgStr.includes('test2.ts')); + }); + + test('shows all-tests message when no files', async () => { + const prepared = await tool.prepareToolInvocation( + upcastPartial({ parameters: {}, toolCallId: 'call-2', chatSessionResource: undefined }), + CancellationToken.None, + ); + assert.ok(prepared); + const msg = prepared.confirmationMessages?.message; + assert.ok(msg); + const msgStr = typeof msg === 'string' ? msg : msg.value; + assert.ok(msgStr.toLowerCase().includes('all tests')); + }); + }); +}); From ccd5ee84212be4ec9e3b859da94afcc532af3c46 Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Thu, 12 Mar 2026 14:42:25 -0700 Subject: [PATCH 2/4] address PR review comments - Fix getCoverageSummary() returning early from a task that had coverage but no matching coverageFiles, preventing subsequent tasks from being checked - Use URI.file().fsPath instead of hardcoded POSIX path strings in test assertions for cross-platform compatibility (Commit message generated by Copilot) --- .../testing/common/testingChatAgentTool.ts | 4 +- .../test/common/testingChatAgentTool.test.ts | 144 +++++++++++------- 2 files changed, 89 insertions(+), 59 deletions(-) diff --git a/src/vs/workbench/contrib/testing/common/testingChatAgentTool.ts b/src/vs/workbench/contrib/testing/common/testingChatAgentTool.ts index 5d3aae6010b..fe6f4d9e5ff 100644 --- a/src/vs/workbench/contrib/testing/common/testingChatAgentTool.ts +++ b/src/vs/workbench/contrib/testing/common/testingChatAgentTool.ts @@ -365,7 +365,9 @@ export async function getCoverageSummary(result: LiveTestResult, coverageFiles: } str += await getFileCoverageDetails(file, path); } - return str; + if (str) { + return str; + } } return ''; } diff --git a/src/vs/workbench/contrib/testing/test/common/testingChatAgentTool.test.ts b/src/vs/workbench/contrib/testing/test/common/testingChatAgentTool.test.ts index 914433c13bb..a6ede42a998 100644 --- a/src/vs/workbench/contrib/testing/test/common/testingChatAgentTool.test.ts +++ b/src/vs/workbench/contrib/testing/test/common/testingChatAgentTool.test.ts @@ -266,62 +266,70 @@ suite('Workbench - RunTestTool', () => { suite('getCoverageSummary', () => { test('returns overall summary when no coverageFiles specified', async () => { + const fileA = URI.file('/src/a.ts'); + const fileB = URI.file('/src/b.ts'); const coverageData = createTestCoverage([ - { uri: URI.file('/src/a.ts'), statement: { covered: 5, total: 10 } }, - { uri: URI.file('/src/b.ts'), statement: { covered: 10, total: 10 } }, + { uri: fileA, statement: { covered: 5, total: 10 } }, + { uri: fileB, statement: { covered: 10, total: 10 } }, ]); const result = createResultWithCoverage(coverageData); const summary = await getCoverageSummary(result, undefined); assert.ok(summary.includes('')); - assert.ok(summary.includes('/src/a.ts')); - assert.ok(!summary.includes('/src/b.ts')); // 100% covered, excluded + assert.ok(summary.includes(fileA.fsPath)); + assert.ok(!summary.includes(fileB.fsPath)); // 100% covered, excluded }); test('returns detailed summary for specified coverageFiles', async () => { + const fileA = URI.file('/src/a.ts'); const details: CoverageDetails[] = [ makeDeclaration('uncoveredFn', 10, 0), makeStatement(20, 0, 25), ]; const coverageData = createTestCoverage([ - { uri: URI.file('/src/a.ts'), statement: { covered: 8, total: 10 }, declaration: { covered: 0, total: 1 }, details }, + { uri: fileA, statement: { covered: 8, total: 10 }, declaration: { covered: 0, total: 1 }, details }, ]); const result = createResultWithCoverage(coverageData); - const summary = await getCoverageSummary(result, ['/src/a.ts']); - assert.ok(summary.includes(' { + const fileA = URI.file('/src/a.ts'); const result = createLiveTestResult(); result.addTask({ id: 't', name: 'n', running: true, ctrlId: 'ctrl' }); - const summary = await getCoverageSummary(result, ['/src/a.ts']); + const summary = await getCoverageSummary(result, [fileA.fsPath]); assert.strictEqual(summary, ''); }); test('handles multiple coverageFiles', async () => { + const fileA = URI.file('/src/a.ts'); + const fileB = URI.file('/src/b.ts'); const coverageData = createTestCoverage([ - { uri: URI.file('/src/a.ts'), statement: { covered: 8, total: 10 }, details: [makeStatement(5, 0)] }, - { uri: URI.file('/src/b.ts'), statement: { covered: 3, total: 10 }, details: [makeDeclaration('fn', 1, 0)] }, + { uri: fileA, statement: { covered: 8, total: 10 }, details: [makeStatement(5, 0)] }, + { uri: fileB, statement: { covered: 3, total: 10 }, details: [makeDeclaration('fn', 1, 0)] }, ]); const result = createResultWithCoverage(coverageData); - const summary = await getCoverageSummary(result, ['/src/a.ts', '/src/b.ts']); - assert.ok(summary.includes('/src/a.ts')); - assert.ok(summary.includes('/src/b.ts')); + const summary = await getCoverageSummary(result, [fileA.fsPath, fileB.fsPath]); + assert.ok(summary.includes(fileA.fsPath)); + assert.ok(summary.includes(fileB.fsPath)); }); test('skips non-matching coverageFiles gracefully', async () => { + const fileA = URI.file('/src/a.ts'); + const nonExistent = URI.file('/src/nonexistent.ts'); const coverageData = createTestCoverage([ - { uri: URI.file('/src/a.ts'), statement: { covered: 8, total: 10 } }, + { uri: fileA, statement: { covered: 8, total: 10 } }, ]); const result = createResultWithCoverage(coverageData); - const summary = await getCoverageSummary(result, ['/src/nonexistent.ts']); + const summary = await getCoverageSummary(result, [nonExistent.fsPath]); assert.strictEqual(summary, ''); }); }); @@ -339,26 +347,31 @@ suite('Workbench - RunTestTool', () => { }); test('sorts files by coverage ascending', () => { + const high = URI.file('/src/high.ts'); + const low = URI.file('/src/low.ts'); + const mid = URI.file('/src/mid.ts'); const coverage = createTestCoverage([ - { uri: URI.file('/src/high.ts'), statement: { covered: 9, total: 10 } }, - { uri: URI.file('/src/low.ts'), statement: { covered: 3, total: 10 } }, - { uri: URI.file('/src/mid.ts'), statement: { covered: 7, total: 10 } }, + { uri: high, statement: { covered: 9, total: 10 } }, + { uri: low, statement: { covered: 3, total: 10 } }, + { uri: mid, statement: { covered: 7, total: 10 } }, ]); const summary = getOverallCoverageSummary(coverage); - const lowIdx = summary.indexOf('/src/low.ts'); - const midIdx = summary.indexOf('/src/mid.ts'); - const highIdx = summary.indexOf('/src/high.ts'); + const lowIdx = summary.indexOf(low.fsPath); + const midIdx = summary.indexOf(mid.fsPath); + const highIdx = summary.indexOf(high.fsPath); assert.ok(lowIdx < midIdx && midIdx < highIdx); }); test('excludes 100% files from listing', () => { + const partial = URI.file('/src/partial.ts'); + const full = URI.file('/src/full.ts'); const coverage = createTestCoverage([ - { uri: URI.file('/src/partial.ts'), statement: { covered: 5, total: 10 } }, - { uri: URI.file('/src/full.ts'), statement: { covered: 10, total: 10 } }, + { uri: partial, statement: { covered: 5, total: 10 } }, + { uri: full, statement: { covered: 10, total: 10 } }, ]); const summary = getOverallCoverageSummary(coverage); - assert.ok(summary.includes('/src/partial.ts')); - assert.ok(!summary.includes('/src/full.ts')); + assert.ok(summary.includes(partial.fsPath)); + assert.ok(!summary.includes(full.fsPath)); }); test('includes percentage in output', () => { @@ -372,46 +385,52 @@ suite('Workbench - RunTestTool', () => { suite('getFileCoverageDetails', () => { test('shows header with statement counts', async () => { - const file = createFileCov(URI.file('/src/foo.ts'), { covered: 8, total: 10 }, []); - const output = await getFileCoverageDetails(file, '/src/foo.ts'); + const uri = URI.file('/src/foo.ts'); + const file = createFileCov(uri, { covered: 8, total: 10 }, []); + const output = await getFileCoverageDetails(file, uri.fsPath); assert.ok(output.includes('statements=8/10')); assert.ok(output.includes('percent=80.0')); - assert.ok(output.startsWith('\n')); }); test('includes branch counts when available', async () => { - const file = createFileCov(URI.file('/src/foo.ts'), { covered: 8, total: 10 }, [], { branch: { covered: 3, total: 5 } }); - const output = await getFileCoverageDetails(file, '/src/foo.ts'); + const uri = URI.file('/src/foo.ts'); + const file = createFileCov(uri, { covered: 8, total: 10 }, [], { branch: { covered: 3, total: 5 } }); + const output = await getFileCoverageDetails(file, uri.fsPath); assert.ok(output.includes('branches=3/5')); }); test('includes declaration counts when available', async () => { - const file = createFileCov(URI.file('/src/foo.ts'), { covered: 8, total: 10 }, [], { declaration: { covered: 2, total: 4 } }); - const output = await getFileCoverageDetails(file, '/src/foo.ts'); + const uri = URI.file('/src/foo.ts'); + const file = createFileCov(uri, { covered: 8, total: 10 }, [], { declaration: { covered: 2, total: 4 } }); + const output = await getFileCoverageDetails(file, uri.fsPath); assert.ok(output.includes('declarations=2/4')); }); test('omits branch/declaration when not available', async () => { - const file = createFileCov(URI.file('/src/foo.ts'), { covered: 8, total: 10 }, []); - const output = await getFileCoverageDetails(file, '/src/foo.ts'); + const uri = URI.file('/src/foo.ts'); + const file = createFileCov(uri, { covered: 8, total: 10 }, []); + const output = await getFileCoverageDetails(file, uri.fsPath); assert.ok(!output.includes('branches=')); assert.ok(!output.includes('declarations=')); }); test('lists uncovered declarations', async () => { + const uri = URI.file('/src/foo.ts'); const details: CoverageDetails[] = [ makeDeclaration('handleError', 89, 0), makeDeclaration('processQueue', 120, 0), makeDeclaration('coveredFn', 50, 3), ]; - const file = createFileCov(URI.file('/src/foo.ts'), { covered: 8, total: 10 }, details, { declaration: { covered: 1, total: 3 } }); - const output = await getFileCoverageDetails(file, '/src/foo.ts'); + const file = createFileCov(uri, { covered: 8, total: 10 }, details, { declaration: { covered: 1, total: 3 } }); + const output = await getFileCoverageDetails(file, uri.fsPath); assert.ok(output.includes('uncovered functions: handleError(L89), processQueue(L120)')); assert.ok(!output.includes('coveredFn')); }); test('lists uncovered branches with labels', async () => { + const uri = URI.file('/src/foo.ts'); const details: CoverageDetails[] = [ makeStatement(34, 5, undefined, [ makeBranch(34, 5, 'then'), @@ -422,30 +441,33 @@ suite('Workbench - RunTestTool', () => { makeBranch(58, 2, 'case "bar"'), ]), ]; - const file = createFileCov(URI.file('/src/foo.ts'), { covered: 8, total: 10 }, details, { branch: { covered: 2, total: 4 } }); - const output = await getFileCoverageDetails(file, '/src/foo.ts'); + const file = createFileCov(uri, { covered: 8, total: 10 }, details, { branch: { covered: 2, total: 4 } }); + const output = await getFileCoverageDetails(file, uri.fsPath); assert.ok(output.includes('uncovered branches: L36(else), L56(case "foo")')); }); test('lists uncovered branches without labels', async () => { + const uri = URI.file('/src/foo.ts'); const details: CoverageDetails[] = [ makeStatement(10, 1, undefined, [makeBranch(10, 0)]), ]; - const file = createFileCov(URI.file('/src/foo.ts'), { covered: 8, total: 10 }, details); - const output = await getFileCoverageDetails(file, '/src/foo.ts'); + const file = createFileCov(uri, { covered: 8, total: 10 }, details); + const output = await getFileCoverageDetails(file, uri.fsPath); assert.ok(output.includes('uncovered branches: L10\n')); }); test('uses parent statement location when branch has no location', async () => { + const uri = URI.file('/src/foo.ts'); const details: CoverageDetails[] = [ makeStatement(42, 1, undefined, [{ count: 0, label: 'else' }]), ]; - const file = createFileCov(URI.file('/src/foo.ts'), { covered: 8, total: 10 }, details); - const output = await getFileCoverageDetails(file, '/src/foo.ts'); + const file = createFileCov(uri, { covered: 8, total: 10 }, details); + const output = await getFileCoverageDetails(file, uri.fsPath); assert.ok(output.includes('L42(else)')); }); test('lists merged uncovered line ranges', async () => { + const uri = URI.file('/src/foo.ts'); const details: CoverageDetails[] = [ makeStatement(23, 0, 27), makeStatement(28, 0, 30), @@ -454,35 +476,38 @@ suite('Workbench - RunTestTool', () => { makeStatement(100, 0, 105), makeStatement(50, 5), // covered ]; - const file = createFileCov(URI.file('/src/foo.ts'), { covered: 5, total: 11 }, details); - const output = await getFileCoverageDetails(file, '/src/foo.ts'); + const file = createFileCov(uri, { covered: 5, total: 11 }, details); + const output = await getFileCoverageDetails(file, uri.fsPath); assert.ok(output.includes('uncovered lines: 23-30, 45, 67-72, 100-105')); }); test('omits uncovered sections when all covered', async () => { + const uri = URI.file('/src/foo.ts'); const details: CoverageDetails[] = [ makeDeclaration('fn', 10, 3), makeStatement(20, 5), makeStatement(30, 1, undefined, [makeBranch(30, 1, 'then'), makeBranch(32, 2, 'else')]), ]; - const file = createFileCov(URI.file('/src/foo.ts'), { covered: 10, total: 10 }, details); - const output = await getFileCoverageDetails(file, '/src/foo.ts'); + const file = createFileCov(uri, { covered: 10, total: 10 }, details); + const output = await getFileCoverageDetails(file, uri.fsPath); assert.ok(!output.includes('uncovered')); }); test('handles details() throwing gracefully', async () => { + const uri = URI.file('/src/err.ts'); const result = createLiveTestResult(); const accessor: ICoverageAccessor = { getCoverageDetails: () => Promise.reject(new Error('not available')), }; - const file = new FileCoverage({ id: 'err', uri: URI.file('/src/err.ts'), statement: { covered: 5, total: 10 } }, result, accessor); - const output = await getFileCoverageDetails(file, '/src/err.ts'); - assert.ok(output.includes('')); assert.ok(!output.includes('uncovered')); }); test('full output snapshot', async () => { + const uri = URI.file('/src/foo.ts'); const details: CoverageDetails[] = [ makeDeclaration('uncoveredFn', 10, 0), makeDeclaration('coveredFn', 20, 3), @@ -494,14 +519,14 @@ suite('Workbench - RunTestTool', () => { makeStatement(50, 3), ]; const file = createFileCov( - URI.file('/src/foo.ts'), + uri, { covered: 8, total: 10 }, details, { branch: { covered: 1, total: 2 }, declaration: { covered: 1, total: 2 } }, ); assert.deepStrictEqual( - await getFileCoverageDetails(file, '/src/foo.ts'), - '\n' + + await getFileCoverageDetails(file, uri.fsPath), + `\n` + 'uncovered functions: uncoveredFn(L10)\n' + 'uncovered branches: L42(else)\n' + 'uncovered lines: 30-32\n' + @@ -599,6 +624,8 @@ suite('Workbench - RunTestTool', () => { }); test('includes stack trace frames', async () => { + const testUri = URI.file('/src/test.ts'); + const helperUri = URI.file('/src/helper.ts'); const result = createResultWithTests([{ extId: new TestId(['ctrlId', 'myTest']).toString(), label: 'myTest', @@ -607,8 +634,8 @@ suite('Workbench - RunTestTool', () => { type: TestMessageType.Error, message: 'fail', stackTrace: [ - { uri: URI.file('/src/test.ts'), position: { lineNumber: 10, column: 5 }, label: 'testFn' }, - { uri: URI.file('/src/helper.ts'), position: undefined, label: 'helperFn' }, + { uri: testUri, position: { lineNumber: 10, column: 5 }, label: 'testFn' }, + { uri: helperUri, position: undefined, label: 'helperFn' }, { uri: undefined, position: undefined, label: 'anonymous' }, ], }], @@ -616,12 +643,13 @@ suite('Workbench - RunTestTool', () => { result.markComplete(); const output = await getFailureDetails(result); - assert.ok(output.includes('path="/src/test.ts" line="10" col="5"')); - assert.ok(output.includes('path="/src/helper.ts">helperFn')); + assert.ok(output.includes(`path="${testUri.fsPath}" line="10" col="5"`)); + assert.ok(output.includes(`path="${helperUri.fsPath}">helperFn`)); assert.ok(output.includes('>anonymous')); }); test('includes location information', async () => { + const testUri = URI.file('/src/test.ts'); const result = createResultWithTests([{ extId: new TestId(['ctrlId', 'myTest']).toString(), label: 'myTest', @@ -629,13 +657,13 @@ suite('Workbench - RunTestTool', () => { messages: [{ type: TestMessageType.Error, message: 'fail', - location: { uri: URI.file('/src/test.ts'), range: new Range(42, 8, 42, 20) }, + location: { uri: testUri, range: new Range(42, 8, 42, 20) }, }], }]); result.markComplete(); const output = await getFailureDetails(result); - assert.ok(output.includes('path="/src/test.ts" line="42" col="8"')); + assert.ok(output.includes(`path="${testUri.fsPath}" line="42" col="8"`)); }); test('skips passing tests', async () => { From 1f0667cde178f9a024ca82506c4bcd7f484357c7 Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Thu, 12 Mar 2026 14:46:12 -0700 Subject: [PATCH 3/4] getCoverageSummary: merge coverage from all tasks instead of returning first match --- .../contrib/testing/common/testingChatAgentTool.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/vs/workbench/contrib/testing/common/testingChatAgentTool.ts b/src/vs/workbench/contrib/testing/common/testingChatAgentTool.ts index fe6f4d9e5ff..2cd089f7c1f 100644 --- a/src/vs/workbench/contrib/testing/common/testingChatAgentTool.ts +++ b/src/vs/workbench/contrib/testing/common/testingChatAgentTool.ts @@ -341,6 +341,7 @@ export async function buildTestRunSummary(result: LiveTestResult, mode: Mode, co /** Gets a coverage summary from a test result, either overall or per-file. */ export async function getCoverageSummary(result: LiveTestResult, coverageFiles: string[] | undefined): Promise { + let str = ''; for (const task of result.tasks) { const coverage = task.coverage.get(); if (!coverage) { @@ -348,7 +349,8 @@ export async function getCoverageSummary(result: LiveTestResult, coverageFiles: } if (!coverageFiles || !coverageFiles.length) { - return getOverallCoverageSummary(coverage); + str += getOverallCoverageSummary(coverage); + continue; } const normalized = coverageFiles.map(file => URI.file(file).fsPath); @@ -357,7 +359,6 @@ export async function getCoverageSummary(result: LiveTestResult, coverageFiles: coveredFilesMap.set(file.uri.fsPath, file); } - let str = ''; for (const path of normalized) { const file = coveredFilesMap.get(path); if (!file) { @@ -365,11 +366,8 @@ export async function getCoverageSummary(result: LiveTestResult, coverageFiles: } str += await getFileCoverageDetails(file, path); } - if (str) { - return str; - } } - return ''; + return str; } /** Gets a file-level coverage overview sorted by lowest coverage first. */ From fa144f0be8501015601b3fa5f64dcb8bdc5106ec Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Thu, 12 Mar 2026 16:08:50 -0700 Subject: [PATCH 4/4] up --- .../workbench/contrib/testing/common/testingChatAgentTool.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/testing/common/testingChatAgentTool.ts b/src/vs/workbench/contrib/testing/common/testingChatAgentTool.ts index 2cd089f7c1f..cf9ae07107f 100644 --- a/src/vs/workbench/contrib/testing/common/testingChatAgentTool.ts +++ b/src/vs/workbench/contrib/testing/common/testingChatAgentTool.ts @@ -383,7 +383,7 @@ export function getOverallCoverageSummary(coverage: TestCoverage): string { let str = '\n'; for (const f of files) { - str += `\n`; + str += `\n`; } str += '\n'; return str; @@ -392,7 +392,7 @@ export function getOverallCoverageSummary(coverage: TestCoverage): string { /** Gets detailed coverage information for a single file including uncovered items. */ export async function getFileCoverageDetails(file: FileCoverage, path: string): Promise { const pct = getTotalCoveragePercent(file.statement, file.branch, file.declaration) * 100; - let str = `