Skip to content

Commit f4957e6

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 c8b6f50 commit f4957e6

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 {
4040
ITestController,
@@ -507,12 +507,14 @@ export class PythonTestController implements ITestController, IExtensionSingleAc
507507
*/
508508
private getTestItemsForWorkspace(workspace: WorkspaceFolder, request: TestRunRequest): TestItem[] {
509509
const testItems: TestItem[] = [];
510+
const excludeSet = request.exclude?.length ? new Set(request.exclude) : undefined;
510511
// If the run request includes test items then collect only items that belong to
511512
// `workspace`. If there are no items in the run request then just run the `workspace`
512513
// root test node. Include will be `undefined` in the "run all" scenario.
514+
// Exclusions are applied after inclusions per VS Code API contract.
513515
(request.include ?? this.testController.items).forEach((i: TestItem) => {
514516
const w = this.workspaceService.getWorkspaceFolder(i.uri);
515-
if (w?.uri.fsPath === workspace.uri.fsPath) {
517+
if (w?.uri.fsPath === workspace.uri.fsPath && !isTestItemExcluded(i, excludeSet)) {
516518
testItems.push(i);
517519
}
518520
});
@@ -566,6 +568,7 @@ export class PythonTestController implements ITestController, IExtensionSingleAc
566568
request.profile?.kind,
567569
this.debugLauncher,
568570
await this.interpreterService.getActiveInterpreter(workspace.uri),
571+
request.exclude,
569572
);
570573
}
571574

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';
@@ -47,6 +47,7 @@ export class WorkspaceTestAdapter {
4747
profileKind?: boolean | TestRunProfileKind,
4848
debugLauncher?: ITestDebugLauncher,
4949
interpreter?: PythonEnvironment,
50+
excludes?: readonly TestItem[],
5051
): Promise<void> {
5152
if (this.executing) {
5253
traceError('Test execution already in progress, not starting a new one.');
@@ -57,22 +58,24 @@ export class WorkspaceTestAdapter {
5758
this.executing = deferred;
5859

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

0 commit comments

Comments
 (0)