Skip to content

Commit 2cbded2

Browse files
andravinclaude
andcommitted
fix(testing): respect TestRunRequest.exclude when running tests
The VS Code Test Explorer API specifies that TestRunRequest.exclude contains tests the user has marked as excluded (e.g., via filtering). Per the API contract, "exclusions should apply after inclusions." Previously, the Python extension ignored request.exclude entirely, causing "Run Tests" to run all tests even when the user had filtered the Test Explorer view. This fix adds exclude handling at two levels: 1. In getTestItemsForWorkspace(): Filter out excluded items before passing to the test adapter (checks ancestors for top-level items) 2. In WorkspaceTestAdapter.executeTests(): Pre-expand the exclude set to include all descendants, then pass to getTestCaseNodes() which skips excluded nodes with O(1) set lookups during traversal Also adds a visited set to getTestCaseNodes() to avoid expanding the same node multiple times when includes contains overlapping items. Fixes the issue where filtering tests in Test Explorer (e.g., by tag) and clicking "Run Tests" would still run all tests. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 3c0301d commit 2cbded2

File tree

3 files changed

+74
-13
lines changed

3 files changed

+74
-13
lines changed

src/client/testing/testController/common/testItemUtilities.ts

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -498,14 +498,69 @@ export async function updateTestItemFromRawData(
498498
item.busy = false;
499499
}
500500

501-
export function getTestCaseNodes(testNode: TestItem, collection: TestItem[] = []): TestItem[] {
501+
/**
502+
* Checks if a test item or any of its ancestors is in the exclude set.
503+
*/
504+
export function isTestItemExcluded(item: TestItem, excludeSet: Set<TestItem> | undefined): boolean {
505+
if (!excludeSet || excludeSet.size === 0) {
506+
return false;
507+
}
508+
let current: TestItem | undefined = item;
509+
while (current) {
510+
if (excludeSet.has(current)) {
511+
return true;
512+
}
513+
current = current.parent;
514+
}
515+
return false;
516+
}
517+
518+
/**
519+
* Expands an exclude set to include all descendants of excluded items.
520+
* After expansion, checking if a node is excluded is O(1) - just check set membership.
521+
*/
522+
export function expandExcludeSet(excludeSet: Set<TestItem> | undefined): Set<TestItem> | undefined {
523+
if (!excludeSet || excludeSet.size === 0) {
524+
return excludeSet;
525+
}
526+
const expanded = new Set<TestItem>();
527+
excludeSet.forEach((item) => {
528+
addWithDescendants(item, expanded);
529+
});
530+
return expanded;
531+
}
532+
533+
function addWithDescendants(item: TestItem, set: Set<TestItem>): void {
534+
if (set.has(item)) {
535+
return;
536+
}
537+
set.add(item);
538+
item.children.forEach((child) => addWithDescendants(child, set));
539+
}
540+
541+
export function getTestCaseNodes(
542+
testNode: TestItem,
543+
collection: TestItem[] = [],
544+
visited?: Set<TestItem>,
545+
excludeSet?: Set<TestItem>,
546+
): TestItem[] {
547+
if (visited?.has(testNode)) {
548+
return collection;
549+
}
550+
visited?.add(testNode);
551+
552+
// Skip excluded nodes (excludeSet should be pre-expanded to include descendants)
553+
if (excludeSet?.has(testNode)) {
554+
return collection;
555+
}
556+
502557
if (!testNode.canResolveChildren && testNode.tags.length > 0) {
503558
collection.push(testNode);
504559
}
505560

506561
testNode.children.forEach((c) => {
507562
if (testNode.canResolveChildren) {
508-
getTestCaseNodes(c, collection);
563+
getTestCaseNodes(c, collection, visited, excludeSet);
509564
} else {
510565
collection.push(testNode);
511566
}

src/client/testing/testController/controller.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import { IEventNamePropertyMapping, sendTelemetryEvent } from '../../telemetry';
3434
import { EventName } from '../../telemetry/constants';
3535
import { PYTEST_PROVIDER, UNITTEST_PROVIDER } from '../common/constants';
3636
import { TestProvider } from '../types';
37-
import { createErrorTestItem, DebugTestTag, getNodeByUri, RunTestTag } from './common/testItemUtilities';
37+
import { createErrorTestItem, DebugTestTag, getNodeByUri, isTestItemExcluded, RunTestTag } from './common/testItemUtilities';
3838
import { buildErrorNodeOptions } from './common/utils';
3939
import { ITestController, ITestFrameworkController, TestRefreshOptions } from './common/types';
4040
import { WorkspaceTestAdapter } from './workspaceTestAdapter';
@@ -848,12 +848,14 @@ export class PythonTestController implements ITestController, IExtensionSingleAc
848848
*/
849849
private getTestItemsForWorkspace(workspace: WorkspaceFolder, request: TestRunRequest): TestItem[] {
850850
const testItems: TestItem[] = [];
851+
const excludeSet = request.exclude?.length ? new Set(request.exclude) : undefined;
851852
// If the run request includes test items then collect only items that belong to
852853
// `workspace`. If there are no items in the run request then just run the `workspace`
853854
// root test node. Include will be `undefined` in the "run all" scenario.
855+
// Exclusions are applied after inclusions per VS Code API contract.
854856
(request.include ?? this.testController.items).forEach((i: TestItem) => {
855857
const w = this.workspaceService.getWorkspaceFolder(i.uri);
856-
if (w?.uri.fsPath === workspace.uri.fsPath) {
858+
if (w?.uri.fsPath === workspace.uri.fsPath && !isTestItemExcluded(i, excludeSet)) {
857859
testItems.push(i);
858860
}
859861
});
@@ -907,6 +909,7 @@ export class PythonTestController implements ITestController, IExtensionSingleAc
907909
request.profile?.kind,
908910
this.debugLauncher,
909911
await this.interpreterService.getActiveInterpreter(workspace.uri),
912+
request.exclude,
910913
);
911914
}
912915

src/client/testing/testController/workspaceTestAdapter.ts

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { traceError } from '../../logging';
99
import { sendTelemetryEvent } from '../../telemetry';
1010
import { EventName } from '../../telemetry/constants';
1111
import { TestProvider } from '../types';
12-
import { createErrorTestItem, getTestCaseNodes } from './common/testItemUtilities';
12+
import { createErrorTestItem, expandExcludeSet, getTestCaseNodes } from './common/testItemUtilities';
1313
import { ITestDiscoveryAdapter, ITestExecutionAdapter, ITestResultResolver } from './common/types';
1414
import { IPythonExecutionFactory } from '../../common/process/types';
1515
import { ITestDebugLauncher } from '../common/types';
@@ -48,6 +48,7 @@ export class WorkspaceTestAdapter {
4848
profileKind?: boolean | TestRunProfileKind,
4949
debugLauncher?: ITestDebugLauncher,
5050
interpreter?: PythonEnvironment,
51+
excludes?: readonly TestItem[],
5152
project?: ProjectAdapter,
5253
): Promise<void> {
5354
if (this.executing) {
@@ -59,22 +60,24 @@ export class WorkspaceTestAdapter {
5960
this.executing = deferred;
6061

6162
const testCaseNodes: TestItem[] = [];
62-
const testCaseIdsSet = new Set<string>();
63+
const visitedNodes = new Set<TestItem>();
64+
const rawExcludeSet = excludes?.length ? new Set(excludes) : undefined;
65+
const excludeSet = expandExcludeSet(rawExcludeSet);
66+
const testCaseIds: string[] = [];
6367
try {
64-
// first fetch all the individual test Items that we necessarily want
68+
// Expand included items to leaf test nodes.
69+
// getTestCaseNodes handles visited tracking and exclusion filtering.
6570
includes.forEach((t) => {
66-
const nodes = getTestCaseNodes(t);
67-
testCaseNodes.push(...nodes);
71+
getTestCaseNodes(t, testCaseNodes, visitedNodes, excludeSet);
6872
});
69-
// iterate through testItems nodes and fetch their unittest runID to pass in as argument
73+
// Collect runIDs for the test nodes to execute.
7074
testCaseNodes.forEach((node) => {
71-
runInstance.started(node); // do the vscode ui test item start here before runtest
75+
runInstance.started(node);
7276
const runId = this.resultResolver.vsIdToRunId.get(node.id);
7377
if (runId) {
74-
testCaseIdsSet.add(runId);
78+
testCaseIds.push(runId);
7579
}
7680
});
77-
const testCaseIds = Array.from(testCaseIdsSet);
7881
if (executionFactory === undefined) {
7982
throw new Error('Execution factory is required for test execution');
8083
}

0 commit comments

Comments
 (0)