diff --git a/src/client/testing/testController/common/projectTestExecution.ts b/src/client/testing/testController/common/projectTestExecution.ts index 8d4f62b508ac..46f7d682c7ea 100644 --- a/src/client/testing/testController/common/projectTestExecution.ts +++ b/src/client/testing/testController/common/projectTestExecution.ts @@ -14,6 +14,7 @@ import { TestProjectRegistry } from './testProjectRegistry'; import { getProjectId } from './projectUtils'; import { getEnvExtApi, useEnvExtension } from '../../../envExt/api.internal'; import { isParentPath } from '../../../pythonEnvironments/common/externalDependencies'; +import { expandExcludeSet } from './testItemUtilities'; /** Dependencies for project-based test execution. */ export interface ProjectExecutionDependencies { @@ -53,6 +54,9 @@ export async function executeTestsForProjects( setupCoverageForProjects(request, projects); } + // Excluded items and their descendants, built once for all projects + const excludeSet = expandExcludeSet(request.exclude); + // Execute tests for each project in parallel // For debug mode, multiple debug sessions will be launched in parallel // Each execution respects cancellation via runInstance.token @@ -76,7 +80,7 @@ export async function executeTestsForProjects( let failed = false; let failureCategory: UnitTestRunFailureCategory | undefined; try { - await executeTestsForProject(project, items, runInstance, request, deps); + await executeTestsForProject(project, items, runInstance, request, deps, excludeSet); } catch (error) { failed = true; failureCategory = token.isCancellationRequested ? 'cancelled' : 'unknown'; @@ -235,13 +239,14 @@ export async function executeTestsForProject( runInstance: TestRun, request: TestRunRequest, deps: ProjectExecutionDependencies, + excludeSet?: Set, ): Promise { const processedTestItemIds = new Set(); const uniqueTestCaseIds = new Set(); // Mark items as started and collect test IDs (deduplicated to handle overlapping selections) for (const item of testItems) { - const testCaseNodes = getTestCaseNodesRecursive(item); + const testCaseNodes = getTestCaseNodesRecursive(item, excludeSet); for (const node of testCaseNodes) { if (processedTestItemIds.has(node.id)) { continue; @@ -277,16 +282,22 @@ export async function executeTestsForProject( ); } -/** Recursively gets all leaf test case nodes from a test item tree. */ -export function getTestCaseNodesRecursive(item: TestItem): TestItem[] { +/** + * Recursively gets all leaf test case nodes from a test item tree, + * skipping items in `excludeSet` (built by `expandExcludeSet`). + */ +export function getTestCaseNodesRecursive(item: TestItem, excludeSet?: Set): TestItem[] { const results: TestItem[] = []; + if (excludeSet?.has(item)) { + return results; + } if (item.children.size === 0) { // This is a leaf node (test case) results.push(item); } else { // Recursively get children item.children.forEach((child) => { - results.push(...getTestCaseNodesRecursive(child)); + results.push(...getTestCaseNodesRecursive(child, excludeSet)); }); } return results; diff --git a/src/client/testing/testController/common/testItemUtilities.ts b/src/client/testing/testController/common/testItemUtilities.ts index 43624bba2527..bae42a654a2d 100644 --- a/src/client/testing/testController/common/testItemUtilities.ts +++ b/src/client/testing/testController/common/testItemUtilities.ts @@ -498,14 +498,42 @@ export async function updateTestItemFromRawData( item.busy = false; } -export function getTestCaseNodes(testNode: TestItem, collection: TestItem[] = []): TestItem[] { +/** + * Builds the set of test items excluded from a run: the items in `TestRunRequest.exclude` + * plus all of their descendants, so a single set lookup tells whether a node is excluded. + */ +export function expandExcludeSet(excludes: readonly TestItem[] | undefined): Set | undefined { + if (!excludes || excludes.length === 0) { + return undefined; + } + const expanded = new Set(); + const add = (item: TestItem) => { + if (expanded.has(item)) { + return; + } + expanded.add(item); + item.children.forEach(add); + }; + excludes.forEach(add); + return expanded; +} + +export function getTestCaseNodes( + testNode: TestItem, + collection: TestItem[] = [], + excludeSet?: Set, +): TestItem[] { + 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, excludeSet); } else { collection.push(testNode); } diff --git a/src/client/testing/testController/controller.ts b/src/client/testing/testController/controller.ts index cd7b516af2b7..255d9a306d07 100644 --- a/src/client/testing/testController/controller.ts +++ b/src/client/testing/testController/controller.ts @@ -1052,6 +1052,7 @@ export class PythonTestController implements ITestController, IExtensionSingleAc request.profile?.kind, this.debugLauncher, await this.interpreterService.getActiveInterpreter(workspace.uri), + request.exclude, ); } catch (ex) { failed = true; diff --git a/src/client/testing/testController/workspaceTestAdapter.ts b/src/client/testing/testController/workspaceTestAdapter.ts index f81a5b4a7da7..b79077606646 100644 --- a/src/client/testing/testController/workspaceTestAdapter.ts +++ b/src/client/testing/testController/workspaceTestAdapter.ts @@ -6,11 +6,11 @@ import { CancellationToken, TestController, TestItem, TestRun, TestRunProfileKin import { createDeferred, Deferred } from '../../common/utils/async'; import { Testing } from '../../common/utils/localize'; import { StopWatch } from '../../common/utils/stopWatch'; -import { traceError } from '../../logging'; +import { traceError, traceVerbose } 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'; @@ -51,6 +51,7 @@ export class WorkspaceTestAdapter { profileKind?: boolean | TestRunProfileKind, debugLauncher?: ITestDebugLauncher, interpreter?: PythonEnvironment, + excludes?: readonly TestItem[], project?: ProjectAdapter, ): Promise { if (this.executing) { @@ -63,10 +64,11 @@ export class WorkspaceTestAdapter { const testCaseNodes: TestItem[] = []; const testCaseIdsSet = new Set(); + const excludeSet = expandExcludeSet(excludes); try { - // first fetch all the individual test Items that we necessarily want + // first fetch all the individual test Items that we necessarily want, minus the excluded ones includes.forEach((t) => { - const nodes = getTestCaseNodes(t); + const nodes = getTestCaseNodes(t, [], excludeSet); testCaseNodes.push(...nodes); }); // iterate through testItems nodes and fetch their unittest runID to pass in as argument @@ -78,6 +80,12 @@ export class WorkspaceTestAdapter { } }); const testCaseIds = Array.from(testCaseIdsSet); + if (excludeSet && testCaseIds.length === 0) { + // Everything was excluded. Don't start a run: an empty ID list makes pytest run all tests. + traceVerbose('All requested tests were excluded, nothing to run.'); + deferred.resolve(); + return deferred.promise; + } 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 2fe71e3a0697..07786cb31d40 100644 --- a/src/test/testing/testController/common/projectTestExecution.unit.test.ts +++ b/src/test/testing/testController/common/projectTestExecution.unit.test.ts @@ -28,6 +28,7 @@ import { groupTestItemsByProject, setupCoverageForProjects, } from '../../../../client/testing/testController/common/projectTestExecution'; +import { expandExcludeSet } from '../../../../client/testing/testController/common/testItemUtilities'; import * as telemetry from '../../../../client/telemetry'; import { EventName } from '../../../../client/telemetry/constants'; import * as envExtApi from '../../../../client/envExt/api.internal'; @@ -340,6 +341,21 @@ suite('Project Test Execution', () => { expect(result).to.have.length(3); expect(new Set(result)).to.deep.equal(new Set([leaf1, leaf2, leaf3])); }); + + test('should skip excluded items and their descendants', () => { + // Mock - file with two classes, one class excluded + const leaf1 = createMockTestItem('test1', '/test.py'); + const leaf2 = createMockTestItem('test2', '/test.py'); + const class1 = createMockTestItem('Class1', '/test.py', [leaf1]); + const class2 = createMockTestItem('Class2', '/test.py', [leaf2]); + const fileItem = createMockTestItem('test_file.py', '/test.py', [class1, class2]); + + // Run + const result = getTestCaseNodesRecursive(fileItem, expandExcludeSet([class2])); + + // Assert + expect(result).to.deep.equal([leaf1]); + }); }); // ===== executeTestsForProject Tests ===== @@ -439,6 +455,90 @@ suite('Project Test Execution', () => { const passedTestIds = project.executionAdapterStub.firstCall.args[1] as string[]; expect(passedTestIds).to.have.length(2); }); + + test('should not run test items in the exclude set', 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(); + + // Run + await executeTestsForProject( + project, + [classItem], + runMock.object, + request, + deps, + expandExcludeSet([leaf2]), + ); + + // Assert - only leaf1 is started and executed + 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 not run anything when the whole selection is excluded', async () => { + // Mock - the requested class is itself 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(); + + // Run + await executeTestsForProject( + project, + [classItem], + runMock.object, + request, + deps, + expandExcludeSet([classItem]), + ); + + // Assert - nothing is started and the adapter is never called (an empty ID list would run all tests) + runMock.verify((r) => r.started(typemoq.It.isAny()), typemoq.Times.never()); + expect(project.executionAdapterStub.called).to.be.false; + }); + + test('should still deduplicate test IDs when exclusions are used', async () => { + // Mock - the class and one of its methods are both selected, the other method is 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(); + + // Run + await executeTestsForProject( + project, + [classItem, leaf1], + runMock.object, + request, + deps, + expandExcludeSet([leaf2]), + ); + + // Assert - leaf1 is started and sent once + runMock.verify((r) => r.started(leaf1), typemoq.Times.once()); + const passedTestIds = project.executionAdapterStub.firstCall.args[1] as string[]; + expect(passedTestIds).to.deep.equal(['runId1']); + }); }); // ===== executeTestsForProjects Tests ===== @@ -623,6 +723,58 @@ suite('Project Test Execution', () => { const telemetryProps = runDoneCalls[0].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; + 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 is executed + 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 only skip the project whose tests are all excluded', async () => { + // Mock - two projects with one test each, the test in proj1 is excluded + 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; + 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 - proj1 is not run at all, proj2 runs item2 + runMock.verify((r) => r.started(item1), typemoq.Times.never()); + runMock.verify((r) => r.started(item2), typemoq.Times.once()); + expect(proj1.executionAdapterStub.called).to.be.false; + expect(proj2.executionAdapterStub.calledOnce).to.be.true; + expect(proj2.executionAdapterStub.firstCall.args[1]).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..616b2a68970b --- /dev/null +++ b/src/test/testing/testController/testItemUtilities.unit.test.ts @@ -0,0 +1,82 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as assert from 'assert'; +import { TestItem } from 'vscode'; +import { + DebugTestTag, + expandExcludeSet, + getTestCaseNodes, + RunTestTag, +} from '../../../client/testing/testController/common/testItemUtilities'; + +function createMockTestItem(id: string, canResolveChildren: boolean, children: TestItem[] = []): TestItem { + return ({ + id, + canResolveChildren, + tags: [RunTestTag, DebugTestTag], + children: { + forEach: (callback: (item: TestItem) => void) => children.forEach(callback), + size: children.length, + }, + } as unknown) as TestItem; +} + +suite('expandExcludeSet', () => { + test('should return undefined when there is nothing to exclude', () => { + assert.strictEqual(expandExcludeSet(undefined), undefined); + assert.strictEqual(expandExcludeSet([]), undefined); + }); + + test('should include the excluded item itself', () => { + const item = createMockTestItem('leaf', false); + const result = expandExcludeSet([item]); + assert.deepStrictEqual(result, new Set([item])); + }); + + test('should include all descendants of excluded items', () => { + const grandchild = createMockTestItem('grandchild', false); + const child1 = createMockTestItem('child1', true, [grandchild]); + const child2 = createMockTestItem('child2', false); + const parent = createMockTestItem('parent', true, [child1, child2]); + const result = expandExcludeSet([parent]); + assert.deepStrictEqual(result, new Set([parent, child1, child2, grandchild])); + }); + + test('should handle overlapping excluded items', () => { + const child = createMockTestItem('child', false); + const parent = createMockTestItem('parent', true, [child]); + const result = expandExcludeSet([parent, child]); + assert.deepStrictEqual(result, new Set([parent, child])); + }); +}); + +suite('getTestCaseNodes with an exclude set', () => { + test('should collect leaf test nodes without an exclude set', () => { + const leaf1 = createMockTestItem('leaf1', false); + const leaf2 = createMockTestItem('leaf2', false); + const parent = createMockTestItem('parent', true, [leaf1, leaf2]); + assert.deepStrictEqual(getTestCaseNodes(parent), [leaf1, leaf2]); + }); + + test('should skip excluded leaf nodes', () => { + const leaf1 = createMockTestItem('leaf1', false); + const leaf2 = createMockTestItem('leaf2', false); + const parent = createMockTestItem('parent', true, [leaf1, leaf2]); + assert.deepStrictEqual(getTestCaseNodes(parent, [], expandExcludeSet([leaf1])), [leaf2]); + }); + + test('should skip the whole subtree of an excluded node', () => { + const leaf1 = createMockTestItem('leaf1', false); + const leaf2 = createMockTestItem('leaf2', false); + const folder = createMockTestItem('folder', true, [leaf1]); + const root = createMockTestItem('root', true, [folder, leaf2]); + assert.deepStrictEqual(getTestCaseNodes(root, [], expandExcludeSet([folder])), [leaf2]); + }); + + test('should return nothing when the requested node is excluded', () => { + const leaf = createMockTestItem('leaf', false); + const root = createMockTestItem('root', true, [leaf]); + assert.deepStrictEqual(getTestCaseNodes(root, [], expandExcludeSet([root])), []); + }); +}); diff --git a/src/test/testing/testController/workspaceTestAdapter.unit.test.ts b/src/test/testing/testController/workspaceTestAdapter.unit.test.ts index 0f5c0c2a2c4c..bce6dc957cbe 100644 --- a/src/test/testing/testController/workspaceTestAdapter.unit.test.ts +++ b/src/test/testing/testController/workspaceTestAdapter.unit.test.ts @@ -357,7 +357,7 @@ suite('Workspace test adapter', () => { ); resultResolver.runIdToVSid.set('mockTestItem1', 'mockTestItem1'); - sinon.stub(testItemUtilities, 'getTestCaseNodes').callsFake((testNode: TestItem) => + sandbox.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 @@ -414,6 +414,66 @@ suite('Workspace test adapter', () => { sinon.assert.calledOnce(executionTestsStub); }); + test('Excluded tests are not started or sent to the execution adapter', async () => { + const workspaceTestAdapter = new WorkspaceTestAdapter( + 'pytest', + new UnittestTestDiscoveryAdapter(stubConfigSettings), + new UnittestTestExecutionAdapter(stubConfigSettings), + Uri.parse('foo'), + resultResolver, + ); + const test1 = createRunnableTestItem('test1'); + const test2 = createRunnableTestItem('test2'); + const testClass = createRunnableTestItem('TestClass', [test1, test2]); + resultResolver.vsIdToRunId.set('test1', 'run1'); + resultResolver.vsIdToRunId.set('test2', 'run2'); + + await workspaceTestAdapter.executeTests( + testController, + runInstance.object, + [testClass], + execFactory.object, + undefined, + undefined, + undefined, + undefined, + [test2], + ); + + runInstance.verify((r) => r.started(test1), typemoq.Times.once()); + runInstance.verify((r) => r.started(test2), typemoq.Times.never()); + sinon.assert.calledOnce(executionTestsStub); + assert.deepStrictEqual(executionTestsStub.firstCall.args[1], ['run1']); + }); + + test('If every requested test is excluded, the execution adapter is not called', async () => { + const workspaceTestAdapter = new WorkspaceTestAdapter( + 'pytest', + new UnittestTestDiscoveryAdapter(stubConfigSettings), + new UnittestTestExecutionAdapter(stubConfigSettings), + Uri.parse('foo'), + resultResolver, + ); + const test1 = createRunnableTestItem('test1'); + const testClass = createRunnableTestItem('TestClass', [test1]); + resultResolver.vsIdToRunId.set('test1', 'run1'); + + await workspaceTestAdapter.executeTests( + testController, + runInstance.object, + [testClass], + execFactory.object, + undefined, + undefined, + undefined, + undefined, + [testClass], + ); + + runInstance.verify((r) => r.started(typemoq.It.isAny()), typemoq.Times.never()); + sinon.assert.notCalled(executionTestsStub); + }); + test('If execution is already running, do not call executionAdapter.runTests again', async () => { executionTestsStub.callsFake( async () => @@ -521,3 +581,17 @@ function createMockTestItem(id: string): TestItem { return mockTestItem; } + +/** A test item that getTestCaseNodes treats as runnable: a leaf with tags, or a parent that can resolve children. */ +function createRunnableTestItem(id: string, children: TestItem[] = []): TestItem { + return ({ + id, + canResolveChildren: children.length > 0, + tags: [testItemUtilities.RunTestTag], + children: { + size: children.length, + forEach: (callback: (item: TestItem) => void) => children.forEach(callback), + }, + uri: Uri.file('/foo/bar'), + } as unknown) as TestItem; +}