diff --git a/src/client/testing/testController/common/projectTestExecution.ts b/src/client/testing/testController/common/projectTestExecution.ts index fe3b4f91491a..2f0b302d0fe0 100644 --- a/src/client/testing/testController/common/projectTestExecution.ts +++ b/src/client/testing/testController/common/projectTestExecution.ts @@ -12,6 +12,7 @@ import { TestProjectRegistry } from './testProjectRegistry'; import { getProjectId } from './projectUtils'; import { getEnvExtApi, useEnvExtension } from '../../../envExt/api.internal'; import { isParentPath } from '../../../pythonEnvironments/common/externalDependencies'; +import { expandExcludeSet, getTestCaseNodes } from './testItemUtilities'; /** Dependencies for project-based test execution. */ export interface ProjectExecutionDependencies { @@ -46,6 +47,10 @@ export async function executeTestsForProjects( const isDebugMode = request.profile?.kind === TestRunProfileKind.Debug; traceInfo(`[test-by-project] Executing tests across ${testsByProject.size} project(s), debug=${isDebugMode}`); + // Expand exclude set once for all projects + const rawExcludeSet = request.exclude?.length ? new Set(request.exclude) : undefined; + const excludeSet = expandExcludeSet(rawExcludeSet); + // Setup coverage once for all projects (single callback that routes by file path) if (request.profile?.kind === TestRunProfileKind.Coverage) { setupCoverageForProjects(request, projects); @@ -71,7 +76,7 @@ export async function executeTestsForProjects( }); try { - await executeTestsForProject(project, items, runInstance, request, deps); + await executeTestsForProject(project, items, runInstance, request, deps, excludeSet); } catch (error) { // Don't log cancellation as an error if (!token.isCancellationRequested) { @@ -216,27 +221,26 @@ export async function executeTestsForProject( runInstance: TestRun, request: TestRunRequest, deps: ProjectExecutionDependencies, + excludeSet?: Set, ): Promise { - const processedTestItemIds = new Set(); - const uniqueTestCaseIds = new Set(); + const testCaseNodes: TestItem[] = []; + const visitedNodes = new Set(); - // Mark items as started and collect test IDs (deduplicated to handle overlapping selections) + // Expand included items to leaf test nodes, respecting exclusions. + // getTestCaseNodes handles visited tracking and exclusion filtering. for (const item of testItems) { - const testCaseNodes = getTestCaseNodesRecursive(item); - for (const node of testCaseNodes) { - if (processedTestItemIds.has(node.id)) { - continue; - } - processedTestItemIds.add(node.id); - runInstance.started(node); - const runId = project.resultResolver.vsIdToRunId.get(node.id); - if (runId) { - uniqueTestCaseIds.add(runId); - } - } + getTestCaseNodes(item, testCaseNodes, visitedNodes, excludeSet); } - const testCaseIds = Array.from(uniqueTestCaseIds); + // Mark items as started and collect test IDs + const testCaseIds: string[] = []; + for (const node of testCaseNodes) { + runInstance.started(node); + const runId = project.resultResolver.vsIdToRunId.get(node.id); + if (runId) { + testCaseIds.push(runId); + } + } if (testCaseIds.length === 0) { traceVerbose(`[test-by-project] No test IDs found for project ${project.projectName}`); diff --git a/src/client/testing/testController/common/testItemUtilities.ts b/src/client/testing/testController/common/testItemUtilities.ts index 43624bba2527..354fb25e605a 100644 --- a/src/client/testing/testController/common/testItemUtilities.ts +++ b/src/client/testing/testController/common/testItemUtilities.ts @@ -498,14 +498,52 @@ export async function updateTestItemFromRawData( item.busy = false; } -export function getTestCaseNodes(testNode: TestItem, collection: TestItem[] = []): TestItem[] { +/** + * Expands an exclude set to include all descendants of excluded items. + * After expansion, checking if a node is excluded is O(1) - just check set membership. + */ +export function expandExcludeSet(excludeSet: Set | undefined): Set | undefined { + if (!excludeSet || excludeSet.size === 0) { + return excludeSet; + } + const expanded = new Set(); + excludeSet.forEach((item) => { + addWithDescendants(item, expanded); + }); + return expanded; +} + +function addWithDescendants(item: TestItem, set: Set): void { + if (set.has(item)) { + return; + } + set.add(item); + item.children.forEach((child) => addWithDescendants(child, set)); +} + +export function getTestCaseNodes( + testNode: TestItem, + collection: TestItem[] = [], + visited?: Set, + excludeSet?: Set, +): TestItem[] { + if (visited?.has(testNode)) { + return collection; + } + visited?.add(testNode); + + // Skip excluded nodes (excludeSet should be pre-expanded to include descendants) + if (excludeSet?.has(testNode)) { + return collection; + } + if (!testNode.canResolveChildren && testNode.tags.length > 0) { collection.push(testNode); } testNode.children.forEach((c) => { if (testNode.canResolveChildren) { - getTestCaseNodes(c, collection); + getTestCaseNodes(c, collection, visited, excludeSet); } else { collection.push(testNode); } diff --git a/src/client/testing/testController/controller.ts b/src/client/testing/testController/controller.ts index 04de209c171d..80041c54494b 100644 --- a/src/client/testing/testController/controller.ts +++ b/src/client/testing/testController/controller.ts @@ -907,6 +907,7 @@ export class PythonTestController implements ITestController, IExtensionSingleAc request.profile?.kind, this.debugLauncher, await this.interpreterService.getActiveInterpreter(workspace.uri), + request.exclude, ); } diff --git a/src/client/testing/testController/workspaceTestAdapter.ts b/src/client/testing/testController/workspaceTestAdapter.ts index f17687732f57..703bb7dc4064 100644 --- a/src/client/testing/testController/workspaceTestAdapter.ts +++ b/src/client/testing/testController/workspaceTestAdapter.ts @@ -9,7 +9,7 @@ import { traceError } from '../../logging'; import { sendTelemetryEvent } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; import { TestProvider } from '../types'; -import { createErrorTestItem, getTestCaseNodes } from './common/testItemUtilities'; +import { createErrorTestItem, expandExcludeSet, getTestCaseNodes } from './common/testItemUtilities'; import { ITestDiscoveryAdapter, ITestExecutionAdapter, ITestResultResolver } from './common/types'; import { IPythonExecutionFactory } from '../../common/process/types'; import { ITestDebugLauncher } from '../common/types'; @@ -48,6 +48,7 @@ export class WorkspaceTestAdapter { profileKind?: boolean | TestRunProfileKind, debugLauncher?: ITestDebugLauncher, interpreter?: PythonEnvironment, + excludes?: readonly TestItem[], project?: ProjectAdapter, ): Promise { if (this.executing) { @@ -59,22 +60,24 @@ export class WorkspaceTestAdapter { this.executing = deferred; const testCaseNodes: TestItem[] = []; - const testCaseIdsSet = new Set(); + const visitedNodes = new Set(); + const rawExcludeSet = excludes?.length ? new Set(excludes) : undefined; + const excludeSet = expandExcludeSet(rawExcludeSet); + const testCaseIds: string[] = []; try { - // first fetch all the individual test Items that we necessarily want + // Expand included items to leaf test nodes. + // getTestCaseNodes handles visited tracking and exclusion filtering. includes.forEach((t) => { - const nodes = getTestCaseNodes(t); - testCaseNodes.push(...nodes); + getTestCaseNodes(t, testCaseNodes, visitedNodes, excludeSet); }); - // iterate through testItems nodes and fetch their unittest runID to pass in as argument + // Collect runIDs for the test nodes to execute. testCaseNodes.forEach((node) => { - runInstance.started(node); // do the vscode ui test item start here before runtest + runInstance.started(node); const runId = this.resultResolver.vsIdToRunId.get(node.id); if (runId) { - testCaseIdsSet.add(runId); + testCaseIds.push(runId); } }); - const testCaseIds = Array.from(testCaseIdsSet); if (executionFactory === undefined) { throw new Error('Execution factory is required for test execution'); } diff --git a/src/test/testing/testController/common/projectTestExecution.unit.test.ts b/src/test/testing/testController/common/projectTestExecution.unit.test.ts index 1cce2d1a8ce0..1e7f6b4a40d2 100644 --- a/src/test/testing/testController/common/projectTestExecution.unit.test.ts +++ b/src/test/testing/testController/common/projectTestExecution.unit.test.ts @@ -438,6 +438,54 @@ suite('Project Test Execution', () => { const passedTestIds = project.executionAdapterStub.firstCall.args[1] as string[]; expect(passedTestIds).to.have.length(2); }); + + test('should exclude test items in excludeSet from execution', async () => { + // Mock - class containing two test methods, one excluded + const project = createMockProjectAdapter({ projectPath: '/workspace/proj', projectName: 'proj' }); + const leaf1 = createMockTestItem('test1', '/workspace/proj/test.py'); + const leaf2 = createMockTestItem('test2', '/workspace/proj/test.py'); + const classItem = createMockTestItem('TestClass', '/workspace/proj/test.py', [leaf1, leaf2]); + project.resultResolver.vsIdToRunId.set('test1', 'runId1'); + project.resultResolver.vsIdToRunId.set('test2', 'runId2'); + const runMock = createMockTestRun(); + const request = { profile: { kind: TestRunProfileKind.Run } } as TestRunRequest; + const deps = createMockDependencies(); + + // Exclude leaf2 + const excludeSet = new Set([leaf2]); + + // Run + await executeTestsForProject(project, [classItem], runMock.object, request, deps, excludeSet); + + // Assert - only leaf1 should be started and executed, leaf2 should be excluded + runMock.verify((r) => r.started(leaf1), typemoq.Times.once()); + runMock.verify((r) => r.started(leaf2), typemoq.Times.never()); + const passedTestIds = project.executionAdapterStub.firstCall.args[1] as string[]; + expect(passedTestIds).to.deep.equal(['runId1']); + }); + + test('should exclude entire subtree when parent is in excludeSet', async () => { + // Mock - file containing a class with test methods + const project = createMockProjectAdapter({ projectPath: '/workspace/proj', projectName: 'proj' }); + const leaf1 = createMockTestItem('test1', '/workspace/proj/test.py'); + const leaf2 = createMockTestItem('test2', '/workspace/proj/test.py'); + const classItem = createMockTestItem('TestClass', '/workspace/proj/test.py', [leaf1, leaf2]); + project.resultResolver.vsIdToRunId.set('test1', 'runId1'); + project.resultResolver.vsIdToRunId.set('test2', 'runId2'); + const runMock = createMockTestRun(); + const request = { profile: { kind: TestRunProfileKind.Run } } as TestRunRequest; + const deps = createMockDependencies(); + + // Exclude the entire class (expandExcludeSet would add children too) + const excludeSet = new Set([classItem, leaf1, leaf2]); + + // Run + await executeTestsForProject(project, [classItem], runMock.object, request, deps, excludeSet); + + // Assert - nothing should be started or executed + runMock.verify((r) => r.started(typemoq.It.isAny()), typemoq.Times.never()); + expect(project.executionAdapterStub.called).to.be.false; + }); }); // ===== executeTestsForProjects Tests ===== @@ -612,6 +660,63 @@ suite('Project Test Execution', () => { const telemetryProps = telemetryStub.firstCall.args[2]; expect(telemetryProps.debugging).to.be.true; }); + + test('should respect request.exclude when executing tests', async () => { + // Mock - project with two test items, one excluded via request.exclude + const project = createMockProjectAdapter({ projectPath: '/workspace/proj', projectName: 'proj' }); + const item1 = createMockTestItem('test1', '/workspace/proj/test.py'); + const item2 = createMockTestItem('test2', '/workspace/proj/test.py'); + project.resultResolver.vsIdToRunId.set('test1', 'runId1'); + project.resultResolver.vsIdToRunId.set('test2', 'runId2'); + const runMock = createMockTestRun(); + const token = new CancellationTokenSource().token; + // Exclude item2 via request.exclude + const request = ({ + profile: { kind: TestRunProfileKind.Run }, + exclude: [item2], + } as unknown) as TestRunRequest; + const deps = createMockDependencies(); + + // Run + await executeTestsForProjects([project], [item1, item2], runMock.object, request, token, deps); + + // Assert - only item1 should be executed, item2 should be excluded + runMock.verify((r) => r.started(item1), typemoq.Times.once()); + runMock.verify((r) => r.started(item2), typemoq.Times.never()); + const passedTestIds = project.executionAdapterStub.firstCall.args[1] as string[]; + expect(passedTestIds).to.deep.equal(['runId1']); + }); + + test('should exclude items only from their own project in multi-project execution', async () => { + // Mock - two projects, each with one test item, exclude one item from proj1 + const proj1 = createMockProjectAdapter({ projectPath: '/workspace/proj1', projectName: 'proj1' }); + const proj2 = createMockProjectAdapter({ projectPath: '/workspace/proj2', projectName: 'proj2' }); + const item1 = createMockTestItem('test1', '/workspace/proj1/test.py'); + const item2 = createMockTestItem('test2', '/workspace/proj2/test.py'); + proj1.resultResolver.vsIdToRunId.set('test1', 'runId1'); + proj2.resultResolver.vsIdToRunId.set('test2', 'runId2'); + const runMock = createMockTestRun(); + const token = new CancellationTokenSource().token; + // Exclude item1 from proj1 - item2 in proj2 should still run + const request = ({ + profile: { kind: TestRunProfileKind.Run }, + exclude: [item1], + } as unknown) as TestRunRequest; + const deps = createMockDependencies(); + + // Run + await executeTestsForProjects([proj1, proj2], [item1, item2], runMock.object, request, token, deps); + + // Assert - item1 excluded, item2 still executed + runMock.verify((r) => r.started(item1), typemoq.Times.never()); + runMock.verify((r) => r.started(item2), typemoq.Times.once()); + // proj1 should not have called runTests (no items left after exclusion) + expect(proj1.executionAdapterStub.called).to.be.false; + // proj2 should have called runTests with item2 + expect(proj2.executionAdapterStub.calledOnce).to.be.true; + const proj2TestIds = proj2.executionAdapterStub.firstCall.args[1] as string[]; + expect(proj2TestIds).to.deep.equal(['runId2']); + }); }); // ===== setupCoverageForProjects Tests ===== diff --git a/src/test/testing/testController/testItemUtilities.unit.test.ts b/src/test/testing/testController/testItemUtilities.unit.test.ts new file mode 100644 index 000000000000..33b786e10cfb --- /dev/null +++ b/src/test/testing/testController/testItemUtilities.unit.test.ts @@ -0,0 +1,129 @@ +import * as assert from 'assert'; +import { TestItem } from 'vscode'; +import { + expandExcludeSet, + getTestCaseNodes, + RunTestTag, + DebugTestTag, +} from '../../../client/testing/testController/common/testItemUtilities'; + +function createMockTestItem(id: string, canResolveChildren: boolean, children: TestItem[] = []): TestItem { + const item = { + id, + canResolveChildren, + tags: [RunTestTag, DebugTestTag], + children: { + forEach: (callback: (item: TestItem) => void) => { + children.forEach(callback); + }, + size: children.length, + }, + parent: undefined as TestItem | undefined, + } as any; + // Set parent references on children + children.forEach((child) => { + (child as any).parent = item; + }); + return item; +} + +suite('expandExcludeSet', () => { + test('should return undefined when excludeSet is undefined', () => { + assert.strictEqual(expandExcludeSet(undefined), undefined); + }); + + test('should return empty set when excludeSet is empty', () => { + const result = expandExcludeSet(new Set()); + assert.ok(result); + assert.strictEqual(result!.size, 0); + }); + + test('should include the excluded item itself', () => { + const item = createMockTestItem('leaf', false); + const result = expandExcludeSet(new Set([item])); + assert.ok(result); + assert.ok(result!.has(item)); + }); + + test('should include all descendants of excluded items', () => { + const child1 = createMockTestItem('child1', false); + const child2 = createMockTestItem('child2', false); + const parent = createMockTestItem('parent', true, [child1, child2]); + const result = expandExcludeSet(new Set([parent])); + assert.ok(result); + assert.strictEqual(result!.size, 3); + assert.ok(result!.has(parent)); + assert.ok(result!.has(child1)); + assert.ok(result!.has(child2)); + }); + + test('should include deeply nested descendants', () => { + const grandchild = createMockTestItem('grandchild', false); + const child = createMockTestItem('child', true, [grandchild]); + const root = createMockTestItem('root', true, [child]); + const result = expandExcludeSet(new Set([root])); + assert.ok(result); + assert.strictEqual(result!.size, 3); + assert.ok(result!.has(root)); + assert.ok(result!.has(child)); + assert.ok(result!.has(grandchild)); + }); +}); + +suite('getTestCaseNodes with exclude and visited', () => { + test('should collect leaf test nodes', () => { + const leaf1 = createMockTestItem('leaf1', false); + const leaf2 = createMockTestItem('leaf2', false); + const parent = createMockTestItem('parent', true, [leaf1, leaf2]); + const result = getTestCaseNodes(parent); + assert.strictEqual(result.length, 2); + assert.ok(result.includes(leaf1)); + assert.ok(result.includes(leaf2)); + }); + + test('should skip nodes in excludeSet', () => { + const leaf1 = createMockTestItem('leaf1', false); + const leaf2 = createMockTestItem('leaf2', false); + const parent = createMockTestItem('parent', true, [leaf1, leaf2]); + const excludeSet = new Set([leaf1]); + const result = getTestCaseNodes(parent, [], new Set(), excludeSet); + assert.strictEqual(result.length, 1); + assert.ok(result.includes(leaf2)); + }); + + test('should skip entire subtree when parent is in excludeSet', () => { + const leaf = createMockTestItem('leaf', false); + const folder = createMockTestItem('folder', true, [leaf]); + const root = createMockTestItem('root', true, [folder]); + // Pre-expanded excludeSet (as expandExcludeSet would produce) + const excludeSet = new Set([folder, leaf]); + const result = getTestCaseNodes(root, [], new Set(), excludeSet); + assert.strictEqual(result.length, 0); + }); + + test('should not visit same node twice when visited set is used', () => { + const leaf = createMockTestItem('leaf', false); + const parent = createMockTestItem('parent', true, [leaf]); + const visited = new Set(); + const collection: TestItem[] = []; + getTestCaseNodes(parent, collection, visited); + getTestCaseNodes(parent, collection, visited); + assert.strictEqual(collection.length, 1); + }); + + test('should work without optional visited and excludeSet parameters', () => { + const leaf = createMockTestItem('leaf', false); + const parent = createMockTestItem('parent', true, [leaf]); + const result = getTestCaseNodes(parent); + assert.strictEqual(result.length, 1); + assert.ok(result.includes(leaf)); + }); + + test('should skip excluded root node entirely', () => { + const leaf = createMockTestItem('leaf', false); + const root = createMockTestItem('root', true, [leaf]); + const excludeSet = new Set([root, leaf]); + const result = getTestCaseNodes(root, [], new Set(), excludeSet); + assert.strictEqual(result.length, 0); + }); +}); diff --git a/src/test/testing/testController/testMocks.ts b/src/test/testing/testController/testMocks.ts index eb37d492f1d9..74f6efa2ccd6 100644 --- a/src/test/testing/testController/testMocks.ts +++ b/src/test/testing/testController/testMocks.ts @@ -14,6 +14,7 @@ import { ITestDebugLauncher } from '../../../client/testing/common/types'; import { ProjectAdapter } from '../../../client/testing/testController/common/projectAdapter'; import { ProjectExecutionDependencies } from '../../../client/testing/testController/common/projectTestExecution'; import { TestProjectRegistry } from '../../../client/testing/testController/common/testProjectRegistry'; +import { RunTestTag, DebugTestTag } from '../../../client/testing/testController/common/testItemUtilities'; import { ITestExecutionAdapter, ITestResultResolver } from '../../../client/testing/testController/common/types'; /** @@ -42,14 +43,16 @@ export function createMockTestItem(id: string, uriPath: string, children?: TestI }, } as TestItemCollection; + // Parent nodes (with children) have canResolveChildren=true, leaf nodes have canResolveChildren=false + const hasChildren = children && children.length > 0; return ({ id, uri: Uri.file(uriPath), children: mockChildren, label: id, - canResolveChildren: false, + canResolveChildren: hasChildren, busy: false, - tags: [], + tags: [RunTestTag, DebugTestTag], range: undefined, error: undefined, parent: undefined, diff --git a/src/test/testing/testController/workspaceTestAdapter.unit.test.ts b/src/test/testing/testController/workspaceTestAdapter.unit.test.ts index 6d2895ca2979..aa424b7cbda3 100644 --- a/src/test/testing/testController/workspaceTestAdapter.unit.test.ts +++ b/src/test/testing/testController/workspaceTestAdapter.unit.test.ts @@ -355,12 +355,14 @@ suite('Workspace test adapter', () => { ); resultResolver.runIdToVSid.set('mockTestItem1', 'mockTestItem1'); - sinon.stub(testItemUtilities, 'getTestCaseNodes').callsFake((testNode: TestItem) => - // Custom implementation logic here based on the provided testNode and collection - - // Example implementation: returning a predefined array of TestItem objects - [testNode], - ); + sinon + .stub(testItemUtilities, 'getTestCaseNodes') + .callsFake((testNode: TestItem, collection: TestItem[] = []) => { + // Custom implementation logic here based on the provided testNode and collection + // Push the testNode to the collection (matching the real implementation behavior) + collection.push(testNode); + return collection; + }); const mockTestItem1 = createMockTestItem('mockTestItem1'); const mockTestItem2 = createMockTestItem('mockTestItem2');