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 8c8ce422e3c1..d7e9c6f0b7bc 100644 --- a/src/client/testing/testController/controller.ts +++ b/src/client/testing/testController/controller.ts @@ -566,6 +566,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 75b9489f708e..071edf8aa0a7 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'; @@ -47,6 +47,7 @@ export class WorkspaceTestAdapter { profileKind?: boolean | TestRunProfileKind, debugLauncher?: ITestDebugLauncher, interpreter?: PythonEnvironment, + excludes?: readonly TestItem[], ): Promise { if (this.executing) { traceError('Test execution already in progress, not starting a new one.'); @@ -57,22 +58,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/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/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');