Merge fix/test-explorer-exclude-filtering: respect TestRunRequest.exclude (PR microsoft#25743)

Author: andravin

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<TestItem>,
 ): Promise<void> {
-    const processedTestItemIds = new Set<string>();
-    const uniqueTestCaseIds = new Set<string>();
+    const testCaseNodes: TestItem[] = [];
+    const visitedNodes = new Set<TestItem>();
 
-    // 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}`);

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<TestItem> | undefined): Set<TestItem> | undefined {
+    if (!excludeSet || excludeSet.size === 0) {
+        return excludeSet;
+    }
+    const expanded = new Set<TestItem>();
+    excludeSet.forEach((item) => {
+        addWithDescendants(item, expanded);
+    });
+    return expanded;
+}
+
+function addWithDescendants(item: TestItem, set: Set<TestItem>): 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<TestItem>,
+    excludeSet?: Set<TestItem>,
+): 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);
         }

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,
         );
     }
 

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<void> {
         if (this.executing) {
@@ -59,22 +60,24 @@ export class WorkspaceTestAdapter {
         this.executing = deferred;
 
         const testCaseNodes: TestItem[] = [];
-        const testCaseIdsSet = new Set<string>();
+        const visitedNodes = new Set<TestItem>();
+        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');
             }

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 =====