From 1423907152796377b3df8927b691dd6c5316f99c Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Mon, 25 May 2026 17:09:59 -0700 Subject: [PATCH 1/7] Add telemetry events for unittest execution and discovery cycles - Introduced new telemetry events: UNITTEST_RUN_DONE and UNITTEST_TREE_UPDATE. - Enhanced existing telemetry events to capture additional properties such as mode, trigger, and duration. - Updated the telemetry context for test discovery to include more detailed metrics. - Refactored telemetry emission to ensure accurate reporting during test execution and discovery processes. --- src/client/telemetry/constants.ts | 2 + src/client/telemetry/index.ts | 149 +++++++++++++++++- .../common/projectTestExecution.ts | 23 +++ .../testController/common/resultResolver.ts | 80 ++++++++++ .../common/testDiscoveryHandler.ts | 18 +++ .../testing/testController/controller.ts | 133 +++++++++++++--- .../testController/workspaceTestAdapter.ts | 26 ++- .../common/projectTestExecution.unit.test.ts | 15 +- .../workspaceTestAdapter.unit.test.ts | 14 +- 9 files changed, 419 insertions(+), 41 deletions(-) diff --git a/src/client/telemetry/constants.ts b/src/client/telemetry/constants.ts index eff32a6e3299..7dac306424ab 100644 --- a/src/client/telemetry/constants.ts +++ b/src/client/telemetry/constants.ts @@ -48,7 +48,9 @@ export enum EventName { UNITTEST_DISCOVERY_DONE = 'UNITTEST.DISCOVERY.DONE', UNITTEST_RUN_STOP = 'UNITTEST.RUN.STOP', UNITTEST_RUN = 'UNITTEST.RUN', + UNITTEST_RUN_DONE = 'UNITTEST.RUN.DONE', UNITTEST_RUN_ALL_FAILED = 'UNITTEST.RUN_ALL_FAILED', + UNITTEST_TREE_UPDATE = 'UNITTEST.TREE.UPDATE', UNITTEST_DISABLED = 'UNITTEST.DISABLED', PYTHON_EXPERIMENTS_INIT_PERFORMANCE = 'PYTHON_EXPERIMENTS_INIT_PERFORMANCE', diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index 763f7405aa0d..a5e57397d32b 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -2127,7 +2127,9 @@ export interface IEventNamePropertyMapping { */ /* __GDPR__ "unittest.discovery.trigger" : { - "trigger" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" } + "trigger" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }, + "filekind" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }, + "mssincelasttrigger" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd", "isMeasurement": true } } */ [EventName.UNITTEST_DISCOVERY_TRIGGER]: { @@ -2142,6 +2144,16 @@ export interface IEventNamePropertyMapping { * interpreter : Triggered by interpreter change. */ trigger: 'auto' | 'ui' | 'commandpalette' | 'watching' | 'interpreter'; + /** + * For 'auto' / 'watching' triggers, classifies the file whose change triggered discovery. + * Used to detect whether discovery is firing on non-test files (see #25866). + */ + fileKind?: 'test' | 'conftest' | 'non-test' | 'unknown'; + /** + * Milliseconds since the previous discovery trigger fired (any source). + * Helps detect chatty re-trigger storms. + */ + msSinceLastTrigger?: number; }; /** * Telemetry event sent with details about discovering tests @@ -2165,7 +2177,13 @@ export interface IEventNamePropertyMapping { /* __GDPR__ "unittest.discovery.done" : { "tool" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }, - "failed" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" } + "failed" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }, + "mode" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }, + "trigger" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }, + "failurecategory" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd" }, + "totaldurationms" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd", "isMeasurement": true }, + "testcount" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd", "isMeasurement": true }, + "exitcode" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd", "isMeasurement": true } } */ [EventName.UNITTEST_DISCOVERY_DONE]: { @@ -2181,6 +2199,39 @@ export interface IEventNamePropertyMapping { * @type {boolean} */ failed: boolean; + /** + * Discovery code path: 'project' = Python Environments API / project-based; + * 'legacy' = single-workspace adapter fallback. + */ + mode?: 'project' | 'legacy'; + /** + * Source that triggered the discovery (mirrors UNITTEST_DISCOVERY_TRIGGER.trigger). + */ + trigger?: 'auto' | 'ui' | 'commandpalette' | 'watching' | 'interpreter'; + /** + * Coarse failure category. Only populated when `failed` is true. + */ + failureCategory?: + | 'subprocess-exit-non-zero' + | 'pipe-error' + | 'pytest-collect-error' + | 'plugin-exception' + | 'timeout' + | 'env-resolution' + | 'cancelled' + | 'unknown'; + /** + * Wall-clock duration of the discovery cycle in milliseconds. + */ + totalDurationMs?: number; + /** + * Number of test items discovered (leaf nodes). + */ + testCount?: number; + /** + * Subprocess exit code, when known (failed discoveries). + */ + exitCode?: number; }; /** * Telemetry event sent when cancelling discovering tests @@ -2222,6 +2273,100 @@ export interface IEventNamePropertyMapping { "unittest.run.all_failed" : { "owner": "eleanorjboyd" } */ [EventName.UNITTEST_RUN_ALL_FAILED]: never | undefined; + /** + * Telemetry event sent at the end of a test run, capturing duration and pipe health. + * Companion to UNITTEST_RUN (which is emitted at run start). + */ + /* __GDPR__ + "unittest.run.done" : { + "tool" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }, + "debugging" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }, + "mode" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }, + "failed" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }, + "failurecategory" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd" }, + "pipeclosedearly" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd" }, + "durationms" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd", "isMeasurement": true }, + "requestedcount" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd", "isMeasurement": true }, + "reportedcount" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd", "isMeasurement": true }, + "missingcount" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd", "isMeasurement": true }, + "exitcode" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd", "isMeasurement": true } + } + */ + [EventName.UNITTEST_RUN_DONE]: { + tool: TestTool; + debugging: boolean; + mode: 'project' | 'legacy'; + /** + * `true` if the run ended without reporting all requested results, + * or if the subprocess crashed / threw. + */ + failed: boolean; + /** + * Coarse failure category when `failed` is true. + */ + failureCategory?: + | 'pipe-cancelled' + | 'subprocess-crash' + | 'no-results' + | 'env-mismatch' + | 'cancelled' + | 'unknown'; + /** + * `true` if the result pipe was disposed before the subprocess fully + * reported all requested test results (see #25872). + */ + pipeClosedEarly?: boolean; + /** + * Wall-clock duration of the run in milliseconds. + */ + durationMs?: number; + /** + * Number of test items the user asked to run. + */ + requestedCount?: number; + /** + * Number of distinct test results reported back over the pipe. + */ + reportedCount?: number; + /** + * requestedCount - reportedCount (signals #25892 "all skipped" pattern). + */ + missingCount?: number; + /** + * Subprocess exit code when known. + */ + exitCode?: number; + }; + /** + * Telemetry event emitted after the test tree is updated with new discovery results. + * Used to detect full-rebuild-on-every-save pattern (see #25822, #25866). + */ + /* __GDPR__ + "unittest.tree.update" : { + "tool" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }, + "mode" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }, + "rebuiltfromscratch" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd" }, + "beforecount" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd", "isMeasurement": true }, + "aftercount" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd", "isMeasurement": true } + } + */ + [EventName.UNITTEST_TREE_UPDATE]: { + tool: TestTool; + mode: 'project' | 'legacy'; + /** + * `true` if the discovery handler cleared and rebuilt all test items rather + * than performing an incremental update. + */ + rebuiltFromScratch: boolean; + /** + * Number of root test items in the controller before this update. + */ + beforeCount: number; + /** + * Number of root test items in the controller after this update. + */ + afterCount: number; + }; /** * Telemetry event sent when testing is disabled for a workspace. */ diff --git a/src/client/testing/testController/common/projectTestExecution.ts b/src/client/testing/testController/common/projectTestExecution.ts index fe3b4f91491a..99aee4d64874 100644 --- a/src/client/testing/testController/common/projectTestExecution.ts +++ b/src/client/testing/testController/common/projectTestExecution.ts @@ -5,6 +5,7 @@ import { CancellationToken, FileCoverageDetail, TestItem, TestRun, TestRunProfil import { traceError, traceInfo, traceVerbose, traceWarn } from '../../../logging'; import { sendTelemetryEvent } from '../../../telemetry'; import { EventName } from '../../../telemetry/constants'; +import { StopWatch } from '../../../common/utils/stopWatch'; import { IPythonExecutionFactory } from '../../../common/process/types'; import { ITestDebugLauncher } from '../../common/types'; import { ProjectAdapter } from './projectAdapter'; @@ -70,13 +71,35 @@ export async function executeTestsForProjects( debugging: isDebugMode, }); + const stopWatch = new StopWatch(); + let failed = false; + let failureCategory: + | 'pipe-cancelled' + | 'subprocess-crash' + | 'no-results' + | 'env-mismatch' + | 'cancelled' + | 'unknown' + | undefined; try { await executeTestsForProject(project, items, runInstance, request, deps); } catch (error) { + failed = true; + failureCategory = token.isCancellationRequested ? 'cancelled' : 'unknown'; // Don't log cancellation as an error if (!token.isCancellationRequested) { traceError(`[test-by-project] Execution failed for project ${project.projectName}:`, error); } + } finally { + sendTelemetryEvent(EventName.UNITTEST_RUN_DONE, undefined, { + tool: project.testProvider, + debugging: isDebugMode, + mode: 'project', + failed, + failureCategory, + durationMs: stopWatch.elapsedTime, + requestedCount: items.length, + }); } }); diff --git a/src/client/testing/testController/common/resultResolver.ts b/src/client/testing/testController/common/resultResolver.ts index c126d233de1b..2086316b871d 100644 --- a/src/client/testing/testController/common/resultResolver.ts +++ b/src/client/testing/testController/common/resultResolver.ts @@ -7,10 +7,29 @@ import { TestProvider } from '../../types'; import { traceInfo } from '../../../logging'; import { sendTelemetryEvent } from '../../../telemetry'; import { EventName } from '../../../telemetry/constants'; +import { StopWatch } from '../../../common/utils/stopWatch'; import { TestItemIndex } from './testItemIndex'; import { TestDiscoveryHandler } from './testDiscoveryHandler'; import { TestExecutionHandler } from './testExecutionHandler'; import { TestCoverageHandler } from './testCoverageHandler'; +import { DiscoveredTestNode, DiscoveredTestItem } from './types'; + +/** + * Trigger source label for the current discovery cycle (matches + * UNITTEST_DISCOVERY_TRIGGER.trigger values). + */ +export type DiscoveryTriggerKind = 'auto' | 'ui' | 'commandpalette' | 'watching' | 'interpreter'; + +/** + * Per-cycle context the controller passes to the resolver so DISCOVERY_DONE can + * include trigger source, mode, and wall-clock duration without having to plumb + * these through every adapter call. + */ +export interface DiscoveryCycleContext { + mode: 'project' | 'legacy'; + trigger?: DiscoveryTriggerKind; + stopWatch: StopWatch; +} export class PythonResultResolver implements ITestResultResolver { testController: TestController; @@ -39,6 +58,12 @@ export class PythonResultResolver implements ITestResultResolver { */ private projectName?: string; + /** + * Per-cycle telemetry context set by the controller before invoking discovery. + * Consumed (and cleared) by resolveDiscovery to emit UNITTEST_DISCOVERY_DONE. + */ + private discoveryCycle?: DiscoveryCycleContext; + constructor( testController: TestController, testProvider: TestProvider, @@ -75,6 +100,39 @@ export class PythonResultResolver implements ITestResultResolver { return this.projectId; } + /** + * Set per-discovery-cycle telemetry context. Called by the controller right + * before invoking the discovery adapter so resolveDiscovery / failure paths + * can include trigger, mode, and duration in UNITTEST_DISCOVERY_DONE. + */ + public beginDiscoveryCycle(ctx: Omit): void { + this.discoveryCycle = { ...ctx, stopWatch: new StopWatch() }; + } + + /** + * Returns and clears the current discovery cycle context, if any. + */ + private takeDiscoveryCycle(): DiscoveryCycleContext | undefined { + const cycle = this.discoveryCycle; + this.discoveryCycle = undefined; + return cycle; + } + + /** + * Returns the current discovery cycle context without clearing it. + * Used by error paths that still want to clear via takeDiscoveryCycle. + */ + public peekDiscoveryCycle(): DiscoveryCycleContext | undefined { + return this.discoveryCycle; + } + + /** + * Clears the current discovery cycle context. + */ + public clearDiscoveryCycle(): void { + this.discoveryCycle = undefined; + } + public resolveDiscovery(payload: DiscoveredTestPayload, token?: CancellationToken): void { PythonResultResolver.discoveryHandler.processDiscovery( payload, @@ -86,9 +144,15 @@ export class PythonResultResolver implements ITestResultResolver { this.projectId, this.projectName, ); + const cycle = this.takeDiscoveryCycle(); + const mode = cycle?.mode ?? (this.projectId ? 'project' : 'legacy'); sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_DONE, undefined, { tool: this.testProvider, failed: false, + mode, + trigger: cycle?.trigger, + totalDurationMs: cycle?.stopWatch.elapsedTime, + testCount: payload?.tests ? countDiscoveredTests(payload.tests) : 0, }); } @@ -135,3 +199,19 @@ export class PythonResultResolver implements ITestResultResolver { this.testItemIndex.cleanupStaleReferences(this.testController); } } + +/** + * Recursively counts leaf test items in a discovered test tree. + * Used to populate UNITTEST_DISCOVERY_DONE.testCount. + */ +function countDiscoveredTests(node: DiscoveredTestNode | DiscoveredTestItem): number { + if ((node as DiscoveredTestNode).children === undefined) { + // No children -> leaf (DiscoveredTestItem). + return 1; + } + let total = 0; + for (const child of (node as DiscoveredTestNode).children) { + total += countDiscoveredTests(child); + } + return total; +} diff --git a/src/client/testing/testController/common/testDiscoveryHandler.ts b/src/client/testing/testController/common/testDiscoveryHandler.ts index 3f70e6b68594..9220f85862b8 100644 --- a/src/client/testing/testController/common/testDiscoveryHandler.ts +++ b/src/client/testing/testController/common/testDiscoveryHandler.ts @@ -6,6 +6,8 @@ import * as util from 'util'; import { DiscoveredTestPayload } from './types'; import { TestProvider } from '../../types'; import { traceError, traceWarn } from '../../../logging'; +import { sendTelemetryEvent } from '../../../telemetry'; +import { EventName } from '../../../telemetry/constants'; import { Testing } from '../../../common/utils/localize'; import { createErrorTestItem } from './testItemUtilities'; import { buildErrorNodeOptions, populateTestTree } from './utils'; @@ -54,6 +56,9 @@ export class TestDiscoveryHandler { // if any tests exist, they should be populated in the test tree, regardless of whether there were errors or not. // parse and insert test data. + // Snapshot root-item count before update for UNITTEST_TREE_UPDATE telemetry. + const beforeCount = testController.items.size; + // Clear existing mappings before rebuilding test tree testItemIndex.clear(); @@ -73,6 +78,19 @@ export class TestDiscoveryHandler { projectId, projectName, ); + + // Emit TREE_UPDATE so we can quantify how often the tree is being rebuilt + // from scratch versus changing incrementally (see #25822, #25866). + // populateTestTree currently always rebuilds the subtree under the workspace/project root, + // so rebuiltFromScratch is true today; we keep the field so future incremental updates + // can flip it to false without a schema change. + sendTelemetryEvent(EventName.UNITTEST_TREE_UPDATE, undefined, { + tool: testProvider, + mode: projectId ? 'project' : 'legacy', + rebuiltFromScratch: true, + beforeCount, + afterCount: testController.items.size, + }); } } diff --git a/src/client/testing/testController/controller.ts b/src/client/testing/testController/controller.ts index 04de209c171d..745a0c8bd381 100644 --- a/src/client/testing/testController/controller.ts +++ b/src/client/testing/testController/controller.ts @@ -28,6 +28,7 @@ import { IPythonExecutionFactory } from '../../common/process/types'; import { IConfigurationService, IDisposableRegistry, Resource } from '../../common/types'; import { DelayedTrigger, IDelayedTrigger } from '../../common/utils/delayTrigger'; import { noop } from '../../common/utils/misc'; +import { StopWatch } from '../../common/utils/stopWatch'; import { IInterpreterService } from '../../interpreter/contracts'; import { traceError, traceInfo, traceVerbose } from '../../logging'; import { IEventNamePropertyMapping, sendTelemetryEvent } from '../../telemetry'; @@ -51,8 +52,30 @@ import { DidChangePythonProjectsEventArgs, PythonProject } from '../../envExt/ty // Types gymnastics to make sure that sendTriggerTelemetry only accepts the correct types. type EventPropertyType = IEventNamePropertyMapping[EventName.UNITTEST_DISCOVERY_TRIGGER]; -type TriggerKeyType = keyof EventPropertyType; -type TriggerType = EventPropertyType[TriggerKeyType]; +type TriggerType = EventPropertyType['trigger']; +type FileKind = NonNullable; + +/** + * Classifies a Python file for UNITTEST_DISCOVERY_TRIGGER.fileKind so we can + * detect whether auto-discovery is firing on non-test files (see #25866). + */ +function classifyFileKind(fsPath: string | undefined): FileKind { + if (!fsPath) { + return 'unknown'; + } + const lower = fsPath.replace(/\\/g, '/').toLowerCase(); + const base = lower.substring(lower.lastIndexOf('/') + 1); + if (base === 'conftest.py') { + return 'conftest'; + } + if (/^test_.*\.py$/.test(base) || /_test\.py$/.test(base)) { + return 'test'; + } + if (base.endsWith('.py')) { + return 'non-test'; + } + return 'unknown'; +} @injectable() export class PythonTestController implements ITestController, IExtensionSingleActivationService { @@ -66,6 +89,18 @@ export class PythonTestController implements ITestController, IExtensionSingleAc private readonly triggerTypes: TriggerType[] = []; + /** + * Timestamp (ms since epoch) of the last UNITTEST_DISCOVERY_TRIGGER fired. + * Used to populate msSinceLastTrigger for chatty-retrigger detection (#25866). + */ + private lastDiscoveryTriggerMs?: number; + + /** + * Source of the trigger that initiated the in-flight (or most recent) discovery. + * Forwarded to the resolver so UNITTEST_DISCOVERY_DONE can attribute duration to a trigger. + */ + private currentDiscoveryTrigger?: TriggerType; + private readonly testController: TestController; private readonly refreshData: IDelayedTrigger; @@ -161,9 +196,7 @@ export class PythonTestController implements ITestController, IExtensionSingleAc ); traceVerbose('Testing: Manually triggered test refresh'); - sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_TRIGGER, undefined, { - trigger: constants.CommandSource.commandPalette, - }); + this.sendTriggerTelemetry(constants.CommandSource.commandPalette); return this.refreshTestData(undefined, { forceRefresh: true }); }; } @@ -578,6 +611,13 @@ export class PythonTestController implements ITestController, IExtensionSingleAc traceInfo(`[test-by-project] Discovering tests for project: ${project.projectName}`); project.isDiscovering = true; + // Start the discovery cycle on the resolver so UNITTEST_DISCOVERY_DONE + // can report mode + trigger + totalDurationMs for this project. + (project.resultResolver as Partial).beginDiscoveryCycle?.({ + mode: 'project', + trigger: this.currentDiscoveryTrigger, + }); + // In project-based mode, the discovery adapter uses the Python Environments API // to get the environment directly, so we don't need to pass the interpreter await project.discoveryAdapter.discoverTests( @@ -595,6 +635,9 @@ export class PythonTestController implements ITestController, IExtensionSingleAc traceError(`[test-by-project] Discovery failed for project ${project.projectName}:`, error); // Individual project failures don't block others projectsCompleted.add(project.projectUri.toString()); // Still mark as completed + // Clear the cycle so the next discovery starts clean (the failure case + // emits its own DONE event from workspaceTestAdapter / adapter). + (project.resultResolver as Partial).clearDiscoveryCycle?.(); } finally { project.isDiscovering = false; } @@ -652,6 +695,7 @@ export class PythonTestController implements ITestController, IExtensionSingleAc this.pythonExecFactory, this.refreshCancellation.token, await this.interpreterService.getActiveInterpreter(workspaceUri), + this.currentDiscoveryTrigger, ); } @@ -893,21 +937,48 @@ export class PythonTestController implements ITestController, IExtensionSingleAc token: CancellationToken, provider: TestProvider, ): Promise { + const debugging = request.profile?.kind === TestRunProfileKind.Debug; sendTelemetryEvent(EventName.UNITTEST_RUN, undefined, { tool: provider, - debugging: request.profile?.kind === TestRunProfileKind.Debug, + debugging, }); - await testAdapter.executeTests( - this.testController, - runInstance, - testItems, - this.pythonExecFactory, - token, - request.profile?.kind, - this.debugLauncher, - await this.interpreterService.getActiveInterpreter(workspace.uri), - ); + const stopWatch = new StopWatch(); + let failed = false; + let failureCategory: + | 'pipe-cancelled' + | 'subprocess-crash' + | 'no-results' + | 'env-mismatch' + | 'cancelled' + | 'unknown' + | undefined; + try { + await testAdapter.executeTests( + this.testController, + runInstance, + testItems, + this.pythonExecFactory, + token, + request.profile?.kind, + this.debugLauncher, + await this.interpreterService.getActiveInterpreter(workspace.uri), + ); + } catch (ex) { + failed = true; + failureCategory = token.isCancellationRequested ? 'cancelled' : 'unknown'; + throw ex; + } finally { + sendTelemetryEvent(EventName.UNITTEST_RUN_DONE, undefined, { + tool: provider, + debugging, + mode: 'legacy', + failed, + failureCategory, + durationMs: stopWatch.elapsedTime, + requestedCount: testItems.length, + }); + } } private invalidateTests(uri: Uri) { @@ -937,7 +1008,7 @@ export class PythonTestController implements ITestController, IExtensionSingleAc file.includes('pyproject.toml') ) { traceVerbose(`Testing: Trigger refresh after saving ${doc.uri.fsPath}`); - this.sendTriggerTelemetry('watching'); + this.sendTriggerTelemetry('watching', doc.uri.fsPath); this.refreshData.trigger(doc.uri, false); } }), @@ -947,14 +1018,14 @@ export class PythonTestController implements ITestController, IExtensionSingleAc this.disposables.push( watcher.onDidCreate((uri) => { traceVerbose(`Testing: Trigger refresh after creating ${uri.fsPath}`); - this.sendTriggerTelemetry('watching'); + this.sendTriggerTelemetry('watching', uri.fsPath); this.refreshData.trigger(uri, false); }), ); this.disposables.push( watcher.onDidDelete((uri) => { traceVerbose(`Testing: Trigger refresh after deleting in ${uri.fsPath}`); - this.sendTriggerTelemetry('watching'); + this.sendTriggerTelemetry('watching', uri.fsPath); this.refreshData.trigger(uri, false); }), ); @@ -969,7 +1040,7 @@ export class PythonTestController implements ITestController, IExtensionSingleAc minimatch.default(doc.uri.fsPath, settings.testing.autoTestDiscoverOnSavePattern) ) { traceVerbose(`Testing: Trigger refresh after saving ${doc.uri.fsPath}`); - this.sendTriggerTelemetry('watching'); + this.sendTriggerTelemetry('watching', doc.uri.fsPath); this.refreshData.trigger(doc.uri, false); } }), @@ -977,17 +1048,27 @@ export class PythonTestController implements ITestController, IExtensionSingleAc } /** - * Send UNITTEST_DISCOVERY_TRIGGER telemetry event only once per trigger type. + * Send a UNITTEST_DISCOVERY_TRIGGER telemetry event. Emits on every trigger + * (no per-type dedupe) so chatty re-trigger patterns (see #25866) are visible + * in the data; msSinceLastTrigger lets dashboards collapse burst-y firings. * - * @param triggerType The trigger type to send telemetry for. + * @param trigger The trigger source. + * @param fsPath Optional file path that caused the trigger (for 'auto' / 'watching'). */ - private sendTriggerTelemetry(trigger: TriggerType): void { + private sendTriggerTelemetry(trigger: TriggerType, fsPath?: string): void { + const now = Date.now(); + const msSinceLastTrigger = + this.lastDiscoveryTriggerMs !== undefined ? now - this.lastDiscoveryTriggerMs : undefined; + this.lastDiscoveryTriggerMs = now; + this.currentDiscoveryTrigger = trigger; if (!this.triggerTypes.includes(trigger)) { - sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_TRIGGER, undefined, { - trigger, - }); this.triggerTypes.push(trigger); } + sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_TRIGGER, undefined, { + trigger, + fileKind: fsPath ? classifyFileKind(fsPath) : undefined, + msSinceLastTrigger, + }); } private surfaceErrorNode(workspaceUri: Uri, message: string, testProvider: TestProvider): void { diff --git a/src/client/testing/testController/workspaceTestAdapter.ts b/src/client/testing/testController/workspaceTestAdapter.ts index f17687732f57..86000ca29c26 100644 --- a/src/client/testing/testController/workspaceTestAdapter.ts +++ b/src/client/testing/testController/workspaceTestAdapter.ts @@ -5,6 +5,7 @@ import * as util from 'util'; import { CancellationToken, TestController, TestItem, TestRun, TestRunProfileKind, Uri } from 'vscode'; import { createDeferred, Deferred } from '../../common/utils/async'; import { Testing } from '../../common/utils/localize'; +import { StopWatch } from '../../common/utils/stopWatch'; import { traceError } from '../../logging'; import { sendTelemetryEvent } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; @@ -16,6 +17,7 @@ import { ITestDebugLauncher } from '../common/types'; import { buildErrorNodeOptions } from './common/utils'; import { PythonEnvironment } from '../../pythonEnvironments/info'; import { ProjectAdapter } from './common/projectAdapter'; +import { PythonResultResolver } from './common/resultResolver'; /** * This class exposes a test-provider-agnostic way of discovering tests. @@ -120,6 +122,7 @@ export class WorkspaceTestAdapter { executionFactory: IPythonExecutionFactory, token?: CancellationToken, interpreter?: PythonEnvironment, + trigger?: 'auto' | 'ui' | 'commandpalette' | 'watching' | 'interpreter', ): Promise { sendTelemetryEvent(EventName.UNITTEST_DISCOVERING, undefined, { tool: this.testProvider }); @@ -131,6 +134,15 @@ export class WorkspaceTestAdapter { const deferred = createDeferred(); this.discovering = deferred; + const stopWatch = new StopWatch(); + + // Hand the resolver per-cycle context so resolveDiscovery can report + // mode + trigger + totalDurationMs in the success-path DISCOVERY_DONE event. + // Optional chaining keeps test doubles that don't implement the method working. + (this.resultResolver as Partial).beginDiscoveryCycle?.({ + mode: 'legacy', + trigger, + }); try { if (executionFactory === undefined) { @@ -139,7 +151,16 @@ export class WorkspaceTestAdapter { await this.discoveryAdapter.discoverTests(this.workspaceUri, executionFactory, token, interpreter); deferred.resolve(); } catch (ex) { - sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_DONE, undefined, { tool: this.testProvider, failed: true }); + // Clear the resolver cycle so the next discovery starts clean. + (this.resultResolver as Partial).clearDiscoveryCycle?.(); + sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_DONE, undefined, { + tool: this.testProvider, + failed: true, + mode: 'legacy', + trigger, + failureCategory: token?.isCancellationRequested ? 'cancelled' : 'unknown', + totalDurationMs: stopWatch.elapsedTime, + }); let cancel = token?.isCancellationRequested ? Testing.cancelUnittestDiscovery @@ -163,7 +184,8 @@ export class WorkspaceTestAdapter { this.discovering = undefined; } - sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_DONE, undefined, { tool: this.testProvider, failed: false }); + // Success path: resolveDiscovery (called by the adapter) already emitted + // UNITTEST_DISCOVERY_DONE with the per-cycle context. Nothing extra to do here. return Promise.resolve(); } diff --git a/src/test/testing/testController/common/projectTestExecution.unit.test.ts b/src/test/testing/testController/common/projectTestExecution.unit.test.ts index 1cce2d1a8ce0..dc9db674ef20 100644 --- a/src/test/testing/testController/common/projectTestExecution.unit.test.ts +++ b/src/test/testing/testController/common/projectTestExecution.unit.test.ts @@ -29,6 +29,7 @@ import { setupCoverageForProjects, } from '../../../../client/testing/testController/common/projectTestExecution'; import * as telemetry from '../../../../client/telemetry'; +import { EventName } from '../../../../client/telemetry/constants'; import * as envExtApi from '../../../../client/envExt/api.internal'; suite('Project Test Execution', () => { @@ -517,8 +518,10 @@ suite('Project Test Execution', () => { // Run await executeTestsForProjects([proj1, proj2], [item1, item2], runMock.object, request, token, deps); - // Assert - telemetry sent twice (once per project) - expect(telemetryStub.callCount).to.equal(2); + // Assert - UNITTEST_RUN telemetry sent twice (once per project). + // UNITTEST_RUN_DONE is also emitted per project but filtered out here. + const runCalls = telemetryStub.getCalls().filter((c) => c.args[0] === EventName.UNITTEST_RUN); + expect(runCalls.length).to.equal(2); }); test('should stop processing remaining projects when cancellation requested mid-execution', async () => { @@ -607,9 +610,11 @@ suite('Project Test Execution', () => { // Run await executeTestsForProjects([project], [item], runMock.object, request, token, deps); - // Assert - telemetry contains debugging=true - expect(telemetryStub.calledOnce).to.be.true; - const telemetryProps = telemetryStub.firstCall.args[2]; + // Assert - UNITTEST_RUN telemetry contains debugging=true. + // UNITTEST_RUN_DONE is also emitted but filtered out here. + const runCalls = telemetryStub.getCalls().filter((c) => c.args[0] === EventName.UNITTEST_RUN); + expect(runCalls.length).to.equal(1); + const telemetryProps = runCalls[0].args[2]; expect(telemetryProps.debugging).to.be.true; }); }); diff --git a/src/test/testing/testController/workspaceTestAdapter.unit.test.ts b/src/test/testing/testController/workspaceTestAdapter.unit.test.ts index 6d2895ca2979..0f5c0c2a2c4c 100644 --- a/src/test/testing/testController/workspaceTestAdapter.unit.test.ts +++ b/src/test/testing/testController/workspaceTestAdapter.unit.test.ts @@ -201,7 +201,7 @@ suite('Workspace test adapter', () => { sinon.assert.calledOnce(discoverTestsStub); }); - test('If discovery succeeds, send a telemetry event with the "failed" key set to false', async () => { + test('If discovery succeeds, the success-path telemetry event is emitted by the result resolver (not the adapter)', async () => { discoverTestsStub.resolves({ status: 'success' }); const testDiscoveryAdapter = new UnittestTestDiscoveryAdapter(stubConfigSettings); @@ -217,11 +217,13 @@ suite('Workspace test adapter', () => { await workspaceTestAdapter.discoverTests(testController, execFactory.object); - sinon.assert.calledWith(sendTelemetryStub, EventName.UNITTEST_DISCOVERY_DONE); - assert.strictEqual(telemetryEvent.length, 2); - - const lastEvent = telemetryEvent[1]; - assert.strictEqual(lastEvent.properties.failed, false); + // The success-path UNITTEST_DISCOVERY_DONE event is now emitted by + // PythonResultResolver.resolveDiscovery so it can include per-cycle context + // (mode, trigger, totalDurationMs, testCount). The mocked resolver in this + // test does not invoke resolveDiscovery, so the only telemetry the adapter + // itself emits in the success path is UNITTEST_DISCOVERING (start). + sinon.assert.calledWith(sendTelemetryStub, EventName.UNITTEST_DISCOVERING); + assert.strictEqual(telemetryEvent.length, 1); }); test('If discovery failed, send a telemetry event with the "failed" key set to true, and add an error node to the test controller', async () => { From 5d1450508c8dbeb62511caddd672a5f83aabf0f5 Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Fri, 29 May 2026 12:08:47 -0700 Subject: [PATCH 2/7] trim telemetry recorded --- src/client/telemetry/constants.ts | 2 - src/client/telemetry/index.ts | 103 ++---------------- src/client/testing/main.ts | 3 +- .../common/projectTestExecution.ts | 5 - .../testController/common/resultResolver.ts | 3 +- .../common/testDiscoveryHandler.ts | 18 --- .../testing/testController/common/types.ts | 5 +- .../testing/testController/controller.ts | 88 ++++----------- .../common/projectTestExecution.unit.test.ts | 21 ++-- 9 files changed, 46 insertions(+), 202 deletions(-) diff --git a/src/client/telemetry/constants.ts b/src/client/telemetry/constants.ts index 7dac306424ab..7a50a86e06cf 100644 --- a/src/client/telemetry/constants.ts +++ b/src/client/telemetry/constants.ts @@ -42,7 +42,6 @@ export enum EventName { // Python testing specific telemetry UNITTEST_CONFIGURING = 'UNITTEST.CONFIGURING', UNITTEST_CONFIGURE = 'UNITTEST.CONFIGURE', - UNITTEST_DISCOVERY_TRIGGER = 'UNITTEST.DISCOVERY.TRIGGER', UNITTEST_DISCOVERING = 'UNITTEST.DISCOVERING', UNITTEST_DISCOVERING_STOP = 'UNITTEST.DISCOVERY.STOP', UNITTEST_DISCOVERY_DONE = 'UNITTEST.DISCOVERY.DONE', @@ -50,7 +49,6 @@ export enum EventName { UNITTEST_RUN = 'UNITTEST.RUN', UNITTEST_RUN_DONE = 'UNITTEST.RUN.DONE', UNITTEST_RUN_ALL_FAILED = 'UNITTEST.RUN_ALL_FAILED', - UNITTEST_TREE_UPDATE = 'UNITTEST.TREE.UPDATE', UNITTEST_DISABLED = 'UNITTEST.DISABLED', PYTHON_EXPERIMENTS_INIT_PERFORMANCE = 'PYTHON_EXPERIMENTS_INIT_PERFORMANCE', diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index a5e57397d32b..3a5c1b203f9f 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -2122,39 +2122,6 @@ export interface IEventNamePropertyMapping { */ interpreterType?: EnvironmentType; }; - /** - * Telemetry event sent indicating the trigger source for discovery. - */ - /* __GDPR__ - "unittest.discovery.trigger" : { - "trigger" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }, - "filekind" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }, - "mssincelasttrigger" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd", "isMeasurement": true } - } - */ - [EventName.UNITTEST_DISCOVERY_TRIGGER]: { - /** - * Carries the source which triggered discovering of tests - * - * @type {('auto' | 'ui' | 'commandpalette' | 'watching' | 'interpreter')} - * auto : Triggered by VS Code editor. - * ui : Triggered by clicking a button. - * commandpalette : Triggered by running the command from the command palette. - * watching : Triggered by filesystem or content changes. - * interpreter : Triggered by interpreter change. - */ - trigger: 'auto' | 'ui' | 'commandpalette' | 'watching' | 'interpreter'; - /** - * For 'auto' / 'watching' triggers, classifies the file whose change triggered discovery. - * Used to detect whether discovery is firing on non-test files (see #25866). - */ - fileKind?: 'test' | 'conftest' | 'non-test' | 'unknown'; - /** - * Milliseconds since the previous discovery trigger fired (any source). - * Helps detect chatty re-trigger storms. - */ - msSinceLastTrigger?: number; - }; /** * Telemetry event sent with details about discovering tests */ @@ -2180,10 +2147,9 @@ export interface IEventNamePropertyMapping { "failed" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }, "mode" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }, "trigger" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }, - "failurecategory" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd" }, - "totaldurationms" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd", "isMeasurement": true }, - "testcount" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd", "isMeasurement": true }, - "exitcode" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd", "isMeasurement": true } + "failureCategory" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd" }, + "totalDurationMs" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd", "isMeasurement": true }, + "testCount" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd", "isMeasurement": true } } */ [EventName.UNITTEST_DISCOVERY_DONE]: { @@ -2205,7 +2171,7 @@ export interface IEventNamePropertyMapping { */ mode?: 'project' | 'legacy'; /** - * Source that triggered the discovery (mirrors UNITTEST_DISCOVERY_TRIGGER.trigger). + * Source that triggered the discovery. */ trigger?: 'auto' | 'ui' | 'commandpalette' | 'watching' | 'interpreter'; /** @@ -2228,10 +2194,6 @@ export interface IEventNamePropertyMapping { * Number of test items discovered (leaf nodes). */ testCount?: number; - /** - * Subprocess exit code, when known (failed discoveries). - */ - exitCode?: number; }; /** * Telemetry event sent when cancelling discovering tests @@ -2283,13 +2245,9 @@ export interface IEventNamePropertyMapping { "debugging" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }, "mode" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }, "failed" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }, - "failurecategory" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd" }, - "pipeclosedearly" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd" }, - "durationms" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd", "isMeasurement": true }, - "requestedcount" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd", "isMeasurement": true }, - "reportedcount" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd", "isMeasurement": true }, - "missingcount" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd", "isMeasurement": true }, - "exitcode" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd", "isMeasurement": true } + "failureCategory" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd" }, + "durationMs" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd", "isMeasurement": true }, + "requestedCount" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd", "isMeasurement": true } } */ [EventName.UNITTEST_RUN_DONE]: { @@ -2311,11 +2269,6 @@ export interface IEventNamePropertyMapping { | 'env-mismatch' | 'cancelled' | 'unknown'; - /** - * `true` if the result pipe was disposed before the subprocess fully - * reported all requested test results (see #25872). - */ - pipeClosedEarly?: boolean; /** * Wall-clock duration of the run in milliseconds. */ @@ -2324,48 +2277,6 @@ export interface IEventNamePropertyMapping { * Number of test items the user asked to run. */ requestedCount?: number; - /** - * Number of distinct test results reported back over the pipe. - */ - reportedCount?: number; - /** - * requestedCount - reportedCount (signals #25892 "all skipped" pattern). - */ - missingCount?: number; - /** - * Subprocess exit code when known. - */ - exitCode?: number; - }; - /** - * Telemetry event emitted after the test tree is updated with new discovery results. - * Used to detect full-rebuild-on-every-save pattern (see #25822, #25866). - */ - /* __GDPR__ - "unittest.tree.update" : { - "tool" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }, - "mode" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }, - "rebuiltfromscratch" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd" }, - "beforecount" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd", "isMeasurement": true }, - "aftercount" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd", "isMeasurement": true } - } - */ - [EventName.UNITTEST_TREE_UPDATE]: { - tool: TestTool; - mode: 'project' | 'legacy'; - /** - * `true` if the discovery handler cleared and rebuilt all test items rather - * than performing an incremental update. - */ - rebuiltFromScratch: boolean; - /** - * Number of root test items in the controller before this update. - */ - beforeCount: number; - /** - * Number of root test items in the controller after this update. - */ - afterCount: number; }; /** * Telemetry event sent when testing is disabled for a workspace. diff --git a/src/client/testing/main.ts b/src/client/testing/main.ts index eed4d70e852c..e7dac381766f 100644 --- a/src/client/testing/main.ts +++ b/src/client/testing/main.ts @@ -222,8 +222,7 @@ export class UnitTestManagementService implements IExtensionActivationService { }), interpreterService.onDidChangeInterpreter(async () => { traceVerbose('Testing: Triggered refresh due to interpreter change.'); - sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_TRIGGER, undefined, { trigger: 'interpreter' }); - await this.testController?.refreshTestData(undefined, { forceRefresh: true }); + await this.testController?.refreshTestData(undefined, { forceRefresh: true, trigger: 'interpreter' }); }), ); } diff --git a/src/client/testing/testController/common/projectTestExecution.ts b/src/client/testing/testController/common/projectTestExecution.ts index 99aee4d64874..a0dc8154acd5 100644 --- a/src/client/testing/testController/common/projectTestExecution.ts +++ b/src/client/testing/testController/common/projectTestExecution.ts @@ -66,11 +66,6 @@ export async function executeTestsForProjects( traceInfo(`[test-by-project] Executing ${items.length} test item(s) for project: ${project.projectName}`); - sendTelemetryEvent(EventName.UNITTEST_RUN, undefined, { - tool: project.testProvider, - debugging: isDebugMode, - }); - const stopWatch = new StopWatch(); let failed = false; let failureCategory: diff --git a/src/client/testing/testController/common/resultResolver.ts b/src/client/testing/testController/common/resultResolver.ts index 2086316b871d..da363e9a74f9 100644 --- a/src/client/testing/testController/common/resultResolver.ts +++ b/src/client/testing/testController/common/resultResolver.ts @@ -15,8 +15,7 @@ import { TestCoverageHandler } from './testCoverageHandler'; import { DiscoveredTestNode, DiscoveredTestItem } from './types'; /** - * Trigger source label for the current discovery cycle (matches - * UNITTEST_DISCOVERY_TRIGGER.trigger values). + * Trigger source label for the current discovery cycle. */ export type DiscoveryTriggerKind = 'auto' | 'ui' | 'commandpalette' | 'watching' | 'interpreter'; diff --git a/src/client/testing/testController/common/testDiscoveryHandler.ts b/src/client/testing/testController/common/testDiscoveryHandler.ts index 9220f85862b8..3f70e6b68594 100644 --- a/src/client/testing/testController/common/testDiscoveryHandler.ts +++ b/src/client/testing/testController/common/testDiscoveryHandler.ts @@ -6,8 +6,6 @@ import * as util from 'util'; import { DiscoveredTestPayload } from './types'; import { TestProvider } from '../../types'; import { traceError, traceWarn } from '../../../logging'; -import { sendTelemetryEvent } from '../../../telemetry'; -import { EventName } from '../../../telemetry/constants'; import { Testing } from '../../../common/utils/localize'; import { createErrorTestItem } from './testItemUtilities'; import { buildErrorNodeOptions, populateTestTree } from './utils'; @@ -56,9 +54,6 @@ export class TestDiscoveryHandler { // if any tests exist, they should be populated in the test tree, regardless of whether there were errors or not. // parse and insert test data. - // Snapshot root-item count before update for UNITTEST_TREE_UPDATE telemetry. - const beforeCount = testController.items.size; - // Clear existing mappings before rebuilding test tree testItemIndex.clear(); @@ -78,19 +73,6 @@ export class TestDiscoveryHandler { projectId, projectName, ); - - // Emit TREE_UPDATE so we can quantify how often the tree is being rebuilt - // from scratch versus changing incrementally (see #25822, #25866). - // populateTestTree currently always rebuilds the subtree under the workspace/project root, - // so rebuiltFromScratch is true today; we keep the field so future incremental updates - // can flip it to false without a schema change. - sendTelemetryEvent(EventName.UNITTEST_TREE_UPDATE, undefined, { - tool: testProvider, - mode: projectId ? 'project' : 'legacy', - rebuiltFromScratch: true, - beforeCount, - afterCount: testController.items.size, - }); } } diff --git a/src/client/testing/testController/common/types.ts b/src/client/testing/testController/common/types.ts index 017c41cf3d97..59d5ce3d899d 100644 --- a/src/client/testing/testController/common/types.ts +++ b/src/client/testing/testController/common/types.ts @@ -34,7 +34,10 @@ export interface TestData { kind: TestDataKinds; } -export type TestRefreshOptions = { forceRefresh: boolean }; +export type TestRefreshOptions = { + forceRefresh: boolean; + trigger?: 'auto' | 'ui' | 'commandpalette' | 'watching' | 'interpreter'; +}; export const ITestController = Symbol('ITestController'); export interface ITestController { diff --git a/src/client/testing/testController/controller.ts b/src/client/testing/testController/controller.ts index 745a0c8bd381..7d1edc7588a6 100644 --- a/src/client/testing/testController/controller.ts +++ b/src/client/testing/testController/controller.ts @@ -31,7 +31,7 @@ import { noop } from '../../common/utils/misc'; import { StopWatch } from '../../common/utils/stopWatch'; import { IInterpreterService } from '../../interpreter/contracts'; import { traceError, traceInfo, traceVerbose } from '../../logging'; -import { IEventNamePropertyMapping, sendTelemetryEvent } from '../../telemetry'; +import { sendTelemetryEvent } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; import { PYTEST_PROVIDER, UNITTEST_PROVIDER } from '../common/constants'; import { TestProvider } from '../types'; @@ -50,32 +50,7 @@ import { executeTestsForProjects } from './common/projectTestExecution'; import { useEnvExtension, getEnvExtApi } from '../../envExt/api.internal'; import { DidChangePythonProjectsEventArgs, PythonProject } from '../../envExt/types'; -// Types gymnastics to make sure that sendTriggerTelemetry only accepts the correct types. -type EventPropertyType = IEventNamePropertyMapping[EventName.UNITTEST_DISCOVERY_TRIGGER]; -type TriggerType = EventPropertyType['trigger']; -type FileKind = NonNullable; - -/** - * Classifies a Python file for UNITTEST_DISCOVERY_TRIGGER.fileKind so we can - * detect whether auto-discovery is firing on non-test files (see #25866). - */ -function classifyFileKind(fsPath: string | undefined): FileKind { - if (!fsPath) { - return 'unknown'; - } - const lower = fsPath.replace(/\\/g, '/').toLowerCase(); - const base = lower.substring(lower.lastIndexOf('/') + 1); - if (base === 'conftest.py') { - return 'conftest'; - } - if (/^test_.*\.py$/.test(base) || /_test\.py$/.test(base)) { - return 'test'; - } - if (base.endsWith('.py')) { - return 'non-test'; - } - return 'unknown'; -} +type DiscoveryTriggerKind = NonNullable; @injectable() export class PythonTestController implements ITestController, IExtensionSingleActivationService { @@ -87,19 +62,11 @@ export class PythonTestController implements ITestController, IExtensionSingleAc // Registry for multi-project testing (one registry instance manages all projects across workspaces) private readonly projectRegistry: TestProjectRegistry; - private readonly triggerTypes: TriggerType[] = []; - - /** - * Timestamp (ms since epoch) of the last UNITTEST_DISCOVERY_TRIGGER fired. - * Used to populate msSinceLastTrigger for chatty-retrigger detection (#25866). - */ - private lastDiscoveryTriggerMs?: number; - /** * Source of the trigger that initiated the in-flight (or most recent) discovery. * Forwarded to the resolver so UNITTEST_DISCOVERY_DONE can attribute duration to a trigger. */ - private currentDiscoveryTrigger?: TriggerType; + private currentDiscoveryTrigger?: DiscoveryTriggerKind; private readonly testController: TestController; @@ -196,7 +163,7 @@ export class PythonTestController implements ITestController, IExtensionSingleAc ); traceVerbose('Testing: Manually triggered test refresh'); - this.sendTriggerTelemetry(constants.CommandSource.commandPalette); + this.setDiscoveryTrigger(constants.CommandSource.commandPalette); return this.refreshTestData(undefined, { forceRefresh: true }); }; } @@ -458,6 +425,9 @@ export class PythonTestController implements ITestController, IExtensionSingleAc } public refreshTestData(uri?: Resource, options?: TestRefreshOptions): Promise { + if (options?.trigger) { + this.setDiscoveryTrigger(options.trigger); + } if (options?.forceRefresh) { if (uri === undefined) { // This is a special case where we want everything to be re-discovered. @@ -502,6 +472,7 @@ export class PythonTestController implements ITestController, IExtensionSingleAc } } finally { this.refreshingCompletedEvent.fire(); + this.currentDiscoveryTrigger = undefined; } } @@ -635,8 +606,15 @@ export class PythonTestController implements ITestController, IExtensionSingleAc traceError(`[test-by-project] Discovery failed for project ${project.projectName}:`, error); // Individual project failures don't block others projectsCompleted.add(project.projectUri.toString()); // Still mark as completed - // Clear the cycle so the next discovery starts clean (the failure case - // emits its own DONE event from workspaceTestAdapter / adapter). + const cycle = (project.resultResolver as Partial).peekDiscoveryCycle?.(); + sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_DONE, undefined, { + tool: project.testProvider, + failed: true, + mode: 'project', + trigger: cycle?.trigger, + failureCategory: this.refreshCancellation.token.isCancellationRequested ? 'cancelled' : 'unknown', + totalDurationMs: cycle?.stopWatch.elapsedTime, + }); (project.resultResolver as Partial).clearDiscoveryCycle?.(); } finally { project.isDiscovering = false; @@ -741,7 +719,7 @@ export class PythonTestController implements ITestController, IExtensionSingleAc } } else { traceVerbose('Testing: Refreshing all test data'); - this.sendTriggerTelemetry('auto'); + this.setDiscoveryTrigger('auto'); const workspaces: readonly WorkspaceFolder[] = this.workspaceService.workspaceFolders || []; await Promise.all( workspaces.map(async (workspace) => { @@ -1008,7 +986,7 @@ export class PythonTestController implements ITestController, IExtensionSingleAc file.includes('pyproject.toml') ) { traceVerbose(`Testing: Trigger refresh after saving ${doc.uri.fsPath}`); - this.sendTriggerTelemetry('watching', doc.uri.fsPath); + this.setDiscoveryTrigger('watching'); this.refreshData.trigger(doc.uri, false); } }), @@ -1018,14 +996,14 @@ export class PythonTestController implements ITestController, IExtensionSingleAc this.disposables.push( watcher.onDidCreate((uri) => { traceVerbose(`Testing: Trigger refresh after creating ${uri.fsPath}`); - this.sendTriggerTelemetry('watching', uri.fsPath); + this.setDiscoveryTrigger('watching'); this.refreshData.trigger(uri, false); }), ); this.disposables.push( watcher.onDidDelete((uri) => { traceVerbose(`Testing: Trigger refresh after deleting in ${uri.fsPath}`); - this.sendTriggerTelemetry('watching', uri.fsPath); + this.setDiscoveryTrigger('watching'); this.refreshData.trigger(uri, false); }), ); @@ -1040,35 +1018,15 @@ export class PythonTestController implements ITestController, IExtensionSingleAc minimatch.default(doc.uri.fsPath, settings.testing.autoTestDiscoverOnSavePattern) ) { traceVerbose(`Testing: Trigger refresh after saving ${doc.uri.fsPath}`); - this.sendTriggerTelemetry('watching', doc.uri.fsPath); + this.setDiscoveryTrigger('watching'); this.refreshData.trigger(doc.uri, false); } }), ); } - /** - * Send a UNITTEST_DISCOVERY_TRIGGER telemetry event. Emits on every trigger - * (no per-type dedupe) so chatty re-trigger patterns (see #25866) are visible - * in the data; msSinceLastTrigger lets dashboards collapse burst-y firings. - * - * @param trigger The trigger source. - * @param fsPath Optional file path that caused the trigger (for 'auto' / 'watching'). - */ - private sendTriggerTelemetry(trigger: TriggerType, fsPath?: string): void { - const now = Date.now(); - const msSinceLastTrigger = - this.lastDiscoveryTriggerMs !== undefined ? now - this.lastDiscoveryTriggerMs : undefined; - this.lastDiscoveryTriggerMs = now; + private setDiscoveryTrigger(trigger: DiscoveryTriggerKind): void { this.currentDiscoveryTrigger = trigger; - if (!this.triggerTypes.includes(trigger)) { - this.triggerTypes.push(trigger); - } - sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_TRIGGER, undefined, { - trigger, - fileKind: fsPath ? classifyFileKind(fsPath) : undefined, - msSinceLastTrigger, - }); } private surfaceErrorNode(workspaceUri: Uri, message: string, testProvider: TestProvider): void { diff --git a/src/test/testing/testController/common/projectTestExecution.unit.test.ts b/src/test/testing/testController/common/projectTestExecution.unit.test.ts index dc9db674ef20..18a0fe989257 100644 --- a/src/test/testing/testController/common/projectTestExecution.unit.test.ts +++ b/src/test/testing/testController/common/projectTestExecution.unit.test.ts @@ -502,7 +502,7 @@ suite('Project Test Execution', () => { expect(proj2.executionAdapterStub.calledOnce).to.be.true; }); - test('should emit telemetry event for each project execution', async () => { + test('should emit completion telemetry event for each project execution', async () => { // Mock const proj1 = createMockProjectAdapter({ projectPath: '/workspace/proj1', projectName: 'proj1' }); const proj2 = createMockProjectAdapter({ projectPath: '/workspace/proj2', projectName: 'proj2' }); @@ -518,10 +518,10 @@ suite('Project Test Execution', () => { // Run await executeTestsForProjects([proj1, proj2], [item1, item2], runMock.object, request, token, deps); - // Assert - UNITTEST_RUN telemetry sent twice (once per project). - // UNITTEST_RUN_DONE is also emitted per project but filtered out here. - const runCalls = telemetryStub.getCalls().filter((c) => c.args[0] === EventName.UNITTEST_RUN); - expect(runCalls.length).to.equal(2); + // Assert - UNITTEST_RUN_DONE telemetry sent twice (once per project). + const runDoneCalls = telemetryStub.getCalls().filter((c) => c.args[0] === EventName.UNITTEST_RUN_DONE); + expect(runDoneCalls.length).to.equal(2); + expect(runDoneCalls.every((c) => c.args[2].mode === 'project')).to.be.true; }); test('should stop processing remaining projects when cancellation requested mid-execution', async () => { @@ -597,7 +597,7 @@ suite('Project Test Execution', () => { expect(profileMock.loadDetailedCoverage).to.not.be.undefined; }); - test('should include debugging=true in telemetry when run profile is Debug', async () => { + test('should include debugging=true in completion telemetry when run profile is Debug', async () => { // Mock const project = createMockProjectAdapter({ projectPath: '/workspace/proj', projectName: 'proj' }); project.resultResolver.vsIdToRunId.set('test1', 'runId1'); @@ -610,11 +610,10 @@ suite('Project Test Execution', () => { // Run await executeTestsForProjects([project], [item], runMock.object, request, token, deps); - // Assert - UNITTEST_RUN telemetry contains debugging=true. - // UNITTEST_RUN_DONE is also emitted but filtered out here. - const runCalls = telemetryStub.getCalls().filter((c) => c.args[0] === EventName.UNITTEST_RUN); - expect(runCalls.length).to.equal(1); - const telemetryProps = runCalls[0].args[2]; + // Assert - UNITTEST_RUN_DONE telemetry contains debugging=true. + const runDoneCalls = telemetryStub.getCalls().filter((c) => c.args[0] === EventName.UNITTEST_RUN_DONE); + expect(runDoneCalls.length).to.equal(1); + const telemetryProps = runDoneCalls[0].args[2]; expect(telemetryProps.debugging).to.be.true; }); }); From 6131f6f7bcd549f6b1714c2736fdcade87fa2417 Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Fri, 29 May 2026 12:29:02 -0700 Subject: [PATCH 3/7] cleanup --- src/client/telemetry/constants.ts | 1 + src/client/telemetry/index.ts | 26 +++++- src/client/testing/main.ts | 1 + .../common/discoveryTelemetry.ts | 32 +++++++ .../testController/common/resultResolver.ts | 85 ++----------------- .../common/testDiscoveryHandler.ts | 12 ++- .../testing/testController/common/types.ts | 3 +- .../testing/testController/common/utils.ts | 18 +++- .../testing/testController/controller.ts | 36 +++++--- .../testController/workspaceTestAdapter.ts | 14 +-- .../resultResolver.unit.test.ts | 6 +- 11 files changed, 125 insertions(+), 109 deletions(-) create mode 100644 src/client/testing/testController/common/discoveryTelemetry.ts diff --git a/src/client/telemetry/constants.ts b/src/client/telemetry/constants.ts index 7a50a86e06cf..d6f6cced8c06 100644 --- a/src/client/telemetry/constants.ts +++ b/src/client/telemetry/constants.ts @@ -42,6 +42,7 @@ export enum EventName { // Python testing specific telemetry UNITTEST_CONFIGURING = 'UNITTEST.CONFIGURING', UNITTEST_CONFIGURE = 'UNITTEST.CONFIGURE', + UNITTEST_DISCOVERY_TRIGGER = 'UNITTEST.DISCOVERY.TRIGGER', UNITTEST_DISCOVERING = 'UNITTEST.DISCOVERING', UNITTEST_DISCOVERING_STOP = 'UNITTEST.DISCOVERY.STOP', UNITTEST_DISCOVERY_DONE = 'UNITTEST.DISCOVERY.DONE', diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index 3a5c1b203f9f..bdf8f81c30c0 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -2122,6 +2122,27 @@ export interface IEventNamePropertyMapping { */ interpreterType?: EnvironmentType; }; + /** + * Telemetry event sent indicating the trigger source for discovery. + */ + /* __GDPR__ + "unittest.discovery.trigger" : { + "trigger" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" } + } + */ + [EventName.UNITTEST_DISCOVERY_TRIGGER]: { + /** + * Carries the source which triggered discovering of tests + * + * @type {('auto' | 'ui' | 'commandpalette' | 'watching' | 'interpreter')} + * auto : Triggered by VS Code editor. + * ui : Triggered by clicking a button. + * commandpalette : Triggered by running the command from the command palette. + * watching : Triggered by filesystem or content changes. + * interpreter : Triggered by interpreter change. + */ + trigger: 'auto' | 'ui' | 'commandpalette' | 'watching' | 'interpreter'; + }; /** * Telemetry event sent with details about discovering tests */ @@ -2166,8 +2187,9 @@ export interface IEventNamePropertyMapping { */ failed: boolean; /** - * Discovery code path: 'project' = Python Environments API / project-based; - * 'legacy' = single-workspace adapter fallback. + * Testing architecture used for discovery: + * 'project' = per-project discovery through the Python Environments API; + * 'legacy' = workspace-level discovery through the existing WorkspaceTestAdapter. */ mode?: 'project' | 'legacy'; /** diff --git a/src/client/testing/main.ts b/src/client/testing/main.ts index e7dac381766f..318d68104b8c 100644 --- a/src/client/testing/main.ts +++ b/src/client/testing/main.ts @@ -222,6 +222,7 @@ export class UnitTestManagementService implements IExtensionActivationService { }), interpreterService.onDidChangeInterpreter(async () => { traceVerbose('Testing: Triggered refresh due to interpreter change.'); + sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_TRIGGER, undefined, { trigger: 'interpreter' }); await this.testController?.refreshTestData(undefined, { forceRefresh: true, trigger: 'interpreter' }); }), ); diff --git a/src/client/testing/testController/common/discoveryTelemetry.ts b/src/client/testing/testController/common/discoveryTelemetry.ts new file mode 100644 index 000000000000..5fcd624e8ab9 --- /dev/null +++ b/src/client/testing/testController/common/discoveryTelemetry.ts @@ -0,0 +1,32 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { StopWatch } from '../../../common/utils/stopWatch'; + +/** Source that requested a test discovery refresh. */ +export type DiscoveryTriggerKind = 'auto' | 'ui' | 'commandpalette' | 'watching' | 'interpreter'; + +/** Testing architecture used for discovery. */ +export type DiscoveryMode = 'project' | 'legacy'; + +export interface DiscoveryTelemetryCycle { + mode: DiscoveryMode; + trigger?: DiscoveryTriggerKind; + stopWatch: StopWatch; +} + +export class DiscoveryTelemetryState { + private activeCycle?: DiscoveryTelemetryCycle; + + constructor(public readonly defaultMode: DiscoveryMode) {} + + public start(context: Omit): void { + this.activeCycle = { ...context, stopWatch: new StopWatch() }; + } + + public complete(): DiscoveryTelemetryCycle | undefined { + const cycle = this.activeCycle; + this.activeCycle = undefined; + return cycle; + } +} diff --git a/src/client/testing/testController/common/resultResolver.ts b/src/client/testing/testController/common/resultResolver.ts index da363e9a74f9..4b6e49065bb7 100644 --- a/src/client/testing/testController/common/resultResolver.ts +++ b/src/client/testing/testController/common/resultResolver.ts @@ -7,28 +7,11 @@ import { TestProvider } from '../../types'; import { traceInfo } from '../../../logging'; import { sendTelemetryEvent } from '../../../telemetry'; import { EventName } from '../../../telemetry/constants'; -import { StopWatch } from '../../../common/utils/stopWatch'; import { TestItemIndex } from './testItemIndex'; import { TestDiscoveryHandler } from './testDiscoveryHandler'; import { TestExecutionHandler } from './testExecutionHandler'; import { TestCoverageHandler } from './testCoverageHandler'; -import { DiscoveredTestNode, DiscoveredTestItem } from './types'; - -/** - * Trigger source label for the current discovery cycle. - */ -export type DiscoveryTriggerKind = 'auto' | 'ui' | 'commandpalette' | 'watching' | 'interpreter'; - -/** - * Per-cycle context the controller passes to the resolver so DISCOVERY_DONE can - * include trigger source, mode, and wall-clock duration without having to plumb - * these through every adapter call. - */ -export interface DiscoveryCycleContext { - mode: 'project' | 'legacy'; - trigger?: DiscoveryTriggerKind; - stopWatch: StopWatch; -} +import { DiscoveryTelemetryState } from './discoveryTelemetry'; export class PythonResultResolver implements ITestResultResolver { testController: TestController; @@ -44,6 +27,8 @@ export class PythonResultResolver implements ITestResultResolver { public detailedCoverageMap = new Map(); + public readonly discoveryTelemetry: DiscoveryTelemetryState; + /** * Optional project ID for scoping test IDs. * When set, all test IDs are prefixed with `{projectId}@@vsc@@` for project-based testing. @@ -57,12 +42,6 @@ export class PythonResultResolver implements ITestResultResolver { */ private projectName?: string; - /** - * Per-cycle telemetry context set by the controller before invoking discovery. - * Consumed (and cleared) by resolveDiscovery to emit UNITTEST_DISCOVERY_DONE. - */ - private discoveryCycle?: DiscoveryCycleContext; - constructor( testController: TestController, testProvider: TestProvider, @@ -76,6 +55,7 @@ export class PythonResultResolver implements ITestResultResolver { this.projectName = projectName; // Initialize a new TestItemIndex which will be used to track test items in this workspace/project this.testItemIndex = new TestItemIndex(); + this.discoveryTelemetry = new DiscoveryTelemetryState(projectId ? 'project' : 'legacy'); } // Expose for backward compatibility (WorkspaceTestAdapter accesses these) @@ -99,41 +79,8 @@ export class PythonResultResolver implements ITestResultResolver { return this.projectId; } - /** - * Set per-discovery-cycle telemetry context. Called by the controller right - * before invoking the discovery adapter so resolveDiscovery / failure paths - * can include trigger, mode, and duration in UNITTEST_DISCOVERY_DONE. - */ - public beginDiscoveryCycle(ctx: Omit): void { - this.discoveryCycle = { ...ctx, stopWatch: new StopWatch() }; - } - - /** - * Returns and clears the current discovery cycle context, if any. - */ - private takeDiscoveryCycle(): DiscoveryCycleContext | undefined { - const cycle = this.discoveryCycle; - this.discoveryCycle = undefined; - return cycle; - } - - /** - * Returns the current discovery cycle context without clearing it. - * Used by error paths that still want to clear via takeDiscoveryCycle. - */ - public peekDiscoveryCycle(): DiscoveryCycleContext | undefined { - return this.discoveryCycle; - } - - /** - * Clears the current discovery cycle context. - */ - public clearDiscoveryCycle(): void { - this.discoveryCycle = undefined; - } - public resolveDiscovery(payload: DiscoveredTestPayload, token?: CancellationToken): void { - PythonResultResolver.discoveryHandler.processDiscovery( + const testCount = PythonResultResolver.discoveryHandler.processDiscovery( payload, this.testController, this.testItemIndex, @@ -143,15 +90,15 @@ export class PythonResultResolver implements ITestResultResolver { this.projectId, this.projectName, ); - const cycle = this.takeDiscoveryCycle(); - const mode = cycle?.mode ?? (this.projectId ? 'project' : 'legacy'); + const cycle = this.discoveryTelemetry.complete(); + const mode = cycle?.mode ?? this.discoveryTelemetry.defaultMode; sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_DONE, undefined, { tool: this.testProvider, failed: false, mode, trigger: cycle?.trigger, totalDurationMs: cycle?.stopWatch.elapsedTime, - testCount: payload?.tests ? countDiscoveredTests(payload.tests) : 0, + testCount, }); } @@ -198,19 +145,3 @@ export class PythonResultResolver implements ITestResultResolver { this.testItemIndex.cleanupStaleReferences(this.testController); } } - -/** - * Recursively counts leaf test items in a discovered test tree. - * Used to populate UNITTEST_DISCOVERY_DONE.testCount. - */ -function countDiscoveredTests(node: DiscoveredTestNode | DiscoveredTestItem): number { - if ((node as DiscoveredTestNode).children === undefined) { - // No children -> leaf (DiscoveredTestItem). - return 1; - } - let total = 0; - for (const child of (node as DiscoveredTestNode).children) { - total += countDiscoveredTests(child); - } - return total; -} diff --git a/src/client/testing/testController/common/testDiscoveryHandler.ts b/src/client/testing/testController/common/testDiscoveryHandler.ts index 3f70e6b68594..9564e0342f13 100644 --- a/src/client/testing/testController/common/testDiscoveryHandler.ts +++ b/src/client/testing/testController/common/testDiscoveryHandler.ts @@ -30,10 +30,10 @@ export class TestDiscoveryHandler { token?: CancellationToken, projectId?: string, projectName?: string, - ): void { + ): number { if (!payload) { // No test data is available - return; + return 0; } const workspacePath = workspaceUri.fsPath; @@ -57,10 +57,14 @@ export class TestDiscoveryHandler { // Clear existing mappings before rebuilding test tree testItemIndex.clear(); + if (rawTestData.tests === null) { + return 0; + } + // If the test root for this folder exists: Workspace refresh, update its children. // Otherwise, it is a freshly discovered workspace, and we need to create a new test root and populate the test tree. // Note: populateTestTree will call testItemIndex.registerTestItem() for each discovered test - populateTestTree( + return populateTestTree( testController, rawTestData.tests, undefined, @@ -74,6 +78,8 @@ export class TestDiscoveryHandler { projectName, ); } + + return 0; } /** diff --git a/src/client/testing/testController/common/types.ts b/src/client/testing/testController/common/types.ts index 59d5ce3d899d..07a83e1bc54d 100644 --- a/src/client/testing/testController/common/types.ts +++ b/src/client/testing/testController/common/types.ts @@ -17,6 +17,7 @@ import { ITestDebugLauncher } from '../../common/types'; import { IPythonExecutionFactory } from '../../../common/process/types'; import { PythonEnvironment } from '../../../pythonEnvironments/info'; import { ProjectAdapter } from './projectAdapter'; +import { DiscoveryTriggerKind } from './discoveryTelemetry'; export enum TestDataKinds { Workspace, @@ -36,7 +37,7 @@ export interface TestData { export type TestRefreshOptions = { forceRefresh: boolean; - trigger?: 'auto' | 'ui' | 'commandpalette' | 'watching' | 'interpreter'; + trigger?: DiscoveryTriggerKind; }; export const ITestController = Symbol('ITestController'); diff --git a/src/client/testing/testController/common/utils.ts b/src/client/testing/testController/common/utils.ts index 9782487d940b..e4474c1b69d5 100644 --- a/src/client/testing/testController/common/utils.ts +++ b/src/client/testing/testController/common/utils.ts @@ -231,7 +231,10 @@ export function populateTestTree( token?: CancellationToken, projectId?: string, projectName?: string, -): void { +): number { + // Count leaf tests while walking the tree so telemetry does not need a second traversal. + let testCount = 0; + // If testRoot is undefined, use the info of the root item of testTreeData to create a test item, and append it to the test controller. if (!testRoot) { // Create project-scoped ID if projectId is provided @@ -275,6 +278,7 @@ export function populateTestTree( testItemMappings.runIdToTestItem.set(child.runID, testItem); testItemMappings.runIdToVSid.set(child.runID, vsId); testItemMappings.vsIdToRunId.set(vsId, child.runID); + testCount += 1; } else { // Use project-scoped ID for non-test nodes and look up within the current root const nodeId = projectId ? `${projectId}${PROJECT_ID_SEPARATOR}${child.id_}` : child.id_; @@ -302,10 +306,20 @@ export function populateTestTree( testRoot!.children.add(node); } - populateTestTree(testController, child, node, testItemMappings, token, projectId, projectName); + testCount += populateTestTree( + testController, + child, + node, + testItemMappings, + token, + projectId, + projectName, + ); } } }); + + return testCount; } function isTestItem(test: DiscoveredTestNode | DiscoveredTestItem): test is DiscoveredTestItem { diff --git a/src/client/testing/testController/controller.ts b/src/client/testing/testController/controller.ts index 7d1edc7588a6..dfa35fb384a4 100644 --- a/src/client/testing/testController/controller.ts +++ b/src/client/testing/testController/controller.ts @@ -41,6 +41,7 @@ import { ITestController, ITestFrameworkController, TestRefreshOptions } from '. import { WorkspaceTestAdapter } from './workspaceTestAdapter'; import { ITestDebugLauncher } from '../common/types'; import { PythonResultResolver } from './common/resultResolver'; +import { DiscoveryTriggerKind } from './common/discoveryTelemetry'; import { onDidSaveTextDocument } from '../../common/vscodeApis/workspaceApis'; import { IEnvironmentVariablesProvider } from '../../common/variables/types'; import { ProjectAdapter } from './common/projectAdapter'; @@ -50,8 +51,6 @@ import { executeTestsForProjects } from './common/projectTestExecution'; import { useEnvExtension, getEnvExtApi } from '../../envExt/api.internal'; import { DidChangePythonProjectsEventArgs, PythonProject } from '../../envExt/types'; -type DiscoveryTriggerKind = NonNullable; - @injectable() export class PythonTestController implements ITestController, IExtensionSingleActivationService { public readonly supportedWorkspaceTypes = { untrustedWorkspace: false, virtualWorkspace: false }; @@ -62,6 +61,8 @@ export class PythonTestController implements ITestController, IExtensionSingleAc // Registry for multi-project testing (one registry instance manages all projects across workspaces) private readonly projectRegistry: TestProjectRegistry; + private readonly triggerTypes: DiscoveryTriggerKind[] = []; + /** * Source of the trigger that initiated the in-flight (or most recent) discovery. * Forwarded to the resolver so UNITTEST_DISCOVERY_DONE can attribute duration to a trigger. @@ -163,7 +164,7 @@ export class PythonTestController implements ITestController, IExtensionSingleAc ); traceVerbose('Testing: Manually triggered test refresh'); - this.setDiscoveryTrigger(constants.CommandSource.commandPalette); + this.sendTriggerTelemetry(constants.CommandSource.commandPalette); return this.refreshTestData(undefined, { forceRefresh: true }); }; } @@ -582,9 +583,9 @@ export class PythonTestController implements ITestController, IExtensionSingleAc traceInfo(`[test-by-project] Discovering tests for project: ${project.projectName}`); project.isDiscovering = true; - // Start the discovery cycle on the resolver so UNITTEST_DISCOVERY_DONE - // can report mode + trigger + totalDurationMs for this project. - (project.resultResolver as Partial).beginDiscoveryCycle?.({ + // Start the telemetry cycle so UNITTEST_DISCOVERY_DONE can report + // mode + trigger + totalDurationMs for this project. + (project.resultResolver as Partial).discoveryTelemetry?.start({ mode: 'project', trigger: this.currentDiscoveryTrigger, }); @@ -606,16 +607,15 @@ export class PythonTestController implements ITestController, IExtensionSingleAc traceError(`[test-by-project] Discovery failed for project ${project.projectName}:`, error); // Individual project failures don't block others projectsCompleted.add(project.projectUri.toString()); // Still mark as completed - const cycle = (project.resultResolver as Partial).peekDiscoveryCycle?.(); + const cycle = (project.resultResolver as Partial).discoveryTelemetry?.complete(); sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_DONE, undefined, { tool: project.testProvider, failed: true, mode: 'project', - trigger: cycle?.trigger, + trigger: cycle?.trigger ?? this.currentDiscoveryTrigger, failureCategory: this.refreshCancellation.token.isCancellationRequested ? 'cancelled' : 'unknown', totalDurationMs: cycle?.stopWatch.elapsedTime, }); - (project.resultResolver as Partial).clearDiscoveryCycle?.(); } finally { project.isDiscovering = false; } @@ -719,7 +719,7 @@ export class PythonTestController implements ITestController, IExtensionSingleAc } } else { traceVerbose('Testing: Refreshing all test data'); - this.setDiscoveryTrigger('auto'); + this.sendTriggerTelemetry('auto'); const workspaces: readonly WorkspaceFolder[] = this.workspaceService.workspaceFolders || []; await Promise.all( workspaces.map(async (workspace) => { @@ -986,7 +986,7 @@ export class PythonTestController implements ITestController, IExtensionSingleAc file.includes('pyproject.toml') ) { traceVerbose(`Testing: Trigger refresh after saving ${doc.uri.fsPath}`); - this.setDiscoveryTrigger('watching'); + this.sendTriggerTelemetry('watching'); this.refreshData.trigger(doc.uri, false); } }), @@ -996,14 +996,14 @@ export class PythonTestController implements ITestController, IExtensionSingleAc this.disposables.push( watcher.onDidCreate((uri) => { traceVerbose(`Testing: Trigger refresh after creating ${uri.fsPath}`); - this.setDiscoveryTrigger('watching'); + this.sendTriggerTelemetry('watching'); this.refreshData.trigger(uri, false); }), ); this.disposables.push( watcher.onDidDelete((uri) => { traceVerbose(`Testing: Trigger refresh after deleting in ${uri.fsPath}`); - this.setDiscoveryTrigger('watching'); + this.sendTriggerTelemetry('watching'); this.refreshData.trigger(uri, false); }), ); @@ -1018,7 +1018,7 @@ export class PythonTestController implements ITestController, IExtensionSingleAc minimatch.default(doc.uri.fsPath, settings.testing.autoTestDiscoverOnSavePattern) ) { traceVerbose(`Testing: Trigger refresh after saving ${doc.uri.fsPath}`); - this.setDiscoveryTrigger('watching'); + this.sendTriggerTelemetry('watching'); this.refreshData.trigger(doc.uri, false); } }), @@ -1029,6 +1029,14 @@ export class PythonTestController implements ITestController, IExtensionSingleAc this.currentDiscoveryTrigger = trigger; } + private sendTriggerTelemetry(trigger: DiscoveryTriggerKind): void { + this.setDiscoveryTrigger(trigger); + if (!this.triggerTypes.includes(trigger)) { + this.triggerTypes.push(trigger); + sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_TRIGGER, undefined, { trigger }); + } + } + private surfaceErrorNode(workspaceUri: Uri, message: string, testProvider: TestProvider): void { let errorNode = this.testController.items.get(`DiscoveryError:${workspaceUri.fsPath}`); if (errorNode === undefined) { diff --git a/src/client/testing/testController/workspaceTestAdapter.ts b/src/client/testing/testController/workspaceTestAdapter.ts index 86000ca29c26..d43e58fd2aaf 100644 --- a/src/client/testing/testController/workspaceTestAdapter.ts +++ b/src/client/testing/testController/workspaceTestAdapter.ts @@ -18,6 +18,7 @@ import { buildErrorNodeOptions } from './common/utils'; import { PythonEnvironment } from '../../pythonEnvironments/info'; import { ProjectAdapter } from './common/projectAdapter'; import { PythonResultResolver } from './common/resultResolver'; +import { DiscoveryTriggerKind } from './common/discoveryTelemetry'; /** * This class exposes a test-provider-agnostic way of discovering tests. @@ -122,7 +123,7 @@ export class WorkspaceTestAdapter { executionFactory: IPythonExecutionFactory, token?: CancellationToken, interpreter?: PythonEnvironment, - trigger?: 'auto' | 'ui' | 'commandpalette' | 'watching' | 'interpreter', + trigger?: DiscoveryTriggerKind, ): Promise { sendTelemetryEvent(EventName.UNITTEST_DISCOVERING, undefined, { tool: this.testProvider }); @@ -138,8 +139,8 @@ export class WorkspaceTestAdapter { // Hand the resolver per-cycle context so resolveDiscovery can report // mode + trigger + totalDurationMs in the success-path DISCOVERY_DONE event. - // Optional chaining keeps test doubles that don't implement the method working. - (this.resultResolver as Partial).beginDiscoveryCycle?.({ + // Optional chaining keeps test doubles that don't implement the property working. + (this.resultResolver as Partial).discoveryTelemetry?.start({ mode: 'legacy', trigger, }); @@ -151,15 +152,14 @@ export class WorkspaceTestAdapter { await this.discoveryAdapter.discoverTests(this.workspaceUri, executionFactory, token, interpreter); deferred.resolve(); } catch (ex) { - // Clear the resolver cycle so the next discovery starts clean. - (this.resultResolver as Partial).clearDiscoveryCycle?.(); + const cycle = (this.resultResolver as Partial).discoveryTelemetry?.complete(); sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_DONE, undefined, { tool: this.testProvider, failed: true, mode: 'legacy', - trigger, + trigger: cycle?.trigger ?? trigger, failureCategory: token?.isCancellationRequested ? 'cancelled' : 'unknown', - totalDurationMs: stopWatch.elapsedTime, + totalDurationMs: cycle?.stopWatch.elapsedTime ?? stopWatch.elapsedTime, }); let cancel = token?.isCancellationRequested diff --git a/src/test/testing/testController/resultResolver.unit.test.ts b/src/test/testing/testController/resultResolver.unit.test.ts index e4b350a20750..067423c25c3a 100644 --- a/src/test/testing/testController/resultResolver.unit.test.ts +++ b/src/test/testing/testController/resultResolver.unit.test.ts @@ -85,7 +85,7 @@ suite('Result Resolver tests', () => { }; // stub out functionality of populateTestTreeStub which is called in resolveDiscovery - const populateTestTreeStub = sinon.stub(util, 'populateTestTree').returns(); + const populateTestTreeStub = sinon.stub(util, 'populateTestTree').returns(0); // call resolve discovery resultResolver.resolveDiscovery(payload, cancelationToken); @@ -171,7 +171,7 @@ suite('Result Resolver tests', () => { const createErrorTestItemStub = sinon.stub(testItemUtilities, 'createErrorTestItem').returns(blankTestItem); // stub out functionality of populateTestTreeStub which is called in resolveDiscovery - const populateTestTreeStub = sinon.stub(util, 'populateTestTree').returns(); + const populateTestTreeStub = sinon.stub(util, 'populateTestTree').returns(0); // call resolve discovery resultResolver.resolveDiscovery(payload, cancelationToken); @@ -232,7 +232,7 @@ suite('Result Resolver tests', () => { const createErrorTestItemStub = sinon.stub(testItemUtilities, 'createErrorTestItem').returns(blankTestItem); // stub out functionality of populateTestTreeStub which is called in resolveDiscovery - sinon.stub(util, 'populateTestTree').returns(); + sinon.stub(util, 'populateTestTree').returns(0); // add spies to insure these aren't called const deleteSpy = sinon.spy(testController.items, 'delete'); const replaceSpy = sinon.spy(testController.items, 'replace'); From 964b874b72d2824d1641ac283baad8ac5afa93e2 Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Fri, 29 May 2026 12:42:58 -0700 Subject: [PATCH 4/7] address copilots comments --- .../common/projectTestExecution.ts | 5 ++++ .../testController/common/resultResolver.ts | 4 +++- .../common/projectTestExecution.unit.test.ts | 13 ++++++++--- .../resultResolver.unit.test.ts | 23 +++++++++++++++++++ 4 files changed, 41 insertions(+), 4 deletions(-) diff --git a/src/client/testing/testController/common/projectTestExecution.ts b/src/client/testing/testController/common/projectTestExecution.ts index a0dc8154acd5..99aee4d64874 100644 --- a/src/client/testing/testController/common/projectTestExecution.ts +++ b/src/client/testing/testController/common/projectTestExecution.ts @@ -66,6 +66,11 @@ export async function executeTestsForProjects( traceInfo(`[test-by-project] Executing ${items.length} test item(s) for project: ${project.projectName}`); + sendTelemetryEvent(EventName.UNITTEST_RUN, undefined, { + tool: project.testProvider, + debugging: isDebugMode, + }); + const stopWatch = new StopWatch(); let failed = false; let failureCategory: diff --git a/src/client/testing/testController/common/resultResolver.ts b/src/client/testing/testController/common/resultResolver.ts index 4b6e49065bb7..871fe136f321 100644 --- a/src/client/testing/testController/common/resultResolver.ts +++ b/src/client/testing/testController/common/resultResolver.ts @@ -92,11 +92,13 @@ export class PythonResultResolver implements ITestResultResolver { ); const cycle = this.discoveryTelemetry.complete(); const mode = cycle?.mode ?? this.discoveryTelemetry.defaultMode; + const failed = payload?.status === 'error'; sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_DONE, undefined, { tool: this.testProvider, - failed: false, + failed, mode, trigger: cycle?.trigger, + failureCategory: failed ? (token?.isCancellationRequested ? 'cancelled' : 'unknown') : undefined, totalDurationMs: cycle?.stopWatch.elapsedTime, testCount, }); diff --git a/src/test/testing/testController/common/projectTestExecution.unit.test.ts b/src/test/testing/testController/common/projectTestExecution.unit.test.ts index 18a0fe989257..2fe71e3a0697 100644 --- a/src/test/testing/testController/common/projectTestExecution.unit.test.ts +++ b/src/test/testing/testController/common/projectTestExecution.unit.test.ts @@ -502,7 +502,7 @@ suite('Project Test Execution', () => { expect(proj2.executionAdapterStub.calledOnce).to.be.true; }); - test('should emit completion telemetry event for each project execution', async () => { + test('should emit start and completion telemetry events for each project execution', async () => { // Mock const proj1 = createMockProjectAdapter({ projectPath: '/workspace/proj1', projectName: 'proj1' }); const proj2 = createMockProjectAdapter({ projectPath: '/workspace/proj2', projectName: 'proj2' }); @@ -518,7 +518,10 @@ suite('Project Test Execution', () => { // Run await executeTestsForProjects([proj1, proj2], [item1, item2], runMock.object, request, token, deps); - // Assert - UNITTEST_RUN_DONE telemetry sent twice (once per project). + // Assert - UNITTEST_RUN and UNITTEST_RUN_DONE telemetry sent twice (once per project). + const runCalls = telemetryStub.getCalls().filter((c) => c.args[0] === EventName.UNITTEST_RUN); + expect(runCalls.length).to.equal(2); + const runDoneCalls = telemetryStub.getCalls().filter((c) => c.args[0] === EventName.UNITTEST_RUN_DONE); expect(runDoneCalls.length).to.equal(2); expect(runDoneCalls.every((c) => c.args[2].mode === 'project')).to.be.true; @@ -597,7 +600,7 @@ suite('Project Test Execution', () => { expect(profileMock.loadDetailedCoverage).to.not.be.undefined; }); - test('should include debugging=true in completion telemetry when run profile is Debug', async () => { + test('should include debugging=true in run telemetry when run profile is Debug', async () => { // Mock const project = createMockProjectAdapter({ projectPath: '/workspace/proj', projectName: 'proj' }); project.resultResolver.vsIdToRunId.set('test1', 'runId1'); @@ -610,6 +613,10 @@ suite('Project Test Execution', () => { // Run await executeTestsForProjects([project], [item], runMock.object, request, token, deps); + const runCalls = telemetryStub.getCalls().filter((c) => c.args[0] === EventName.UNITTEST_RUN); + expect(runCalls.length).to.equal(1); + expect(runCalls[0].args[2].debugging).to.be.true; + // Assert - UNITTEST_RUN_DONE telemetry contains debugging=true. const runDoneCalls = telemetryStub.getCalls().filter((c) => c.args[0] === EventName.UNITTEST_RUN_DONE); expect(runDoneCalls.length).to.equal(1); diff --git a/src/test/testing/testController/resultResolver.unit.test.ts b/src/test/testing/testController/resultResolver.unit.test.ts index 067423c25c3a..c56f53750336 100644 --- a/src/test/testing/testController/resultResolver.unit.test.ts +++ b/src/test/testing/testController/resultResolver.unit.test.ts @@ -14,6 +14,8 @@ import { import * as testItemUtilities from '../../../client/testing/testController/common/testItemUtilities'; import * as ResultResolver from '../../../client/testing/testController/common/resultResolver'; import * as util from '../../../client/testing/testController/common/utils'; +import * as telemetry from '../../../client/telemetry'; +import { EventName } from '../../../client/telemetry/constants'; import { traceLog } from '../../../client/logging'; suite('Result Resolver tests', () => { @@ -137,6 +139,27 @@ suite('Result Resolver tests', () => { // header of createErrorTestItem is (options: ErrorTestItemOptions, testController: TestController, uri: Uri) sinon.assert.calledWithMatch(createErrorTestItemStub, sinon.match.any, sinon.match.any); }); + test('resolveDiscovery should emit failed telemetry for error payloads', async () => { + testProvider = 'pytest'; + workspaceUri = Uri.file('/foo/bar'); + resultResolver = new ResultResolver.PythonResultResolver(testController, testProvider, workspaceUri); + const sendTelemetryStub = sinon.stub(telemetry, 'sendTelemetryEvent'); + sinon.stub(util, 'buildErrorNodeOptions').returns({ id: 'id', label: 'label', error: 'error' }); + sinon.stub(testItemUtilities, 'createErrorTestItem').returns(blankTestItem); + + const payload: DiscoveredTestPayload = { + cwd: workspaceUri.fsPath, + status: 'error', + error: ['discovery failed'], + }; + + resultResolver.resolveDiscovery(payload, cancelationToken); + + sinon.assert.calledWithMatch(sendTelemetryStub, EventName.UNITTEST_DISCOVERY_DONE, undefined, { + failed: true, + failureCategory: 'unknown', + }); + }); test('resolveDiscovery should create error and root node when error and tests exist on payload', async () => { // test specific constants used expected values testProvider = 'pytest'; From 1f8ced2bff21f9a419d00b47dddc0645380776de Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Fri, 29 May 2026 12:48:13 -0700 Subject: [PATCH 5/7] remove duplicates clarify changes to poptestTree --- src/client/telemetry/constants.ts | 11 +++++++++++ src/client/telemetry/index.ts | 10 ++-------- .../testController/common/projectTestExecution.ts | 11 ++--------- src/client/testing/testController/common/utils.ts | 4 ++++ src/client/testing/testController/controller.ts | 11 ++--------- 5 files changed, 21 insertions(+), 26 deletions(-) diff --git a/src/client/telemetry/constants.ts b/src/client/telemetry/constants.ts index d6f6cced8c06..1a5e26503f3b 100644 --- a/src/client/telemetry/constants.ts +++ b/src/client/telemetry/constants.ts @@ -102,6 +102,17 @@ export enum EventName { ENVIRONMENT_TERMINAL_GLOBAL_PIP = 'ENVIRONMENT.TERMINAL.GLOBAL_PIP', } +export const UNITTEST_RUN_FAILURE_CATEGORIES = [ + 'pipe-cancelled', + 'subprocess-crash', + 'no-results', + 'env-mismatch', + 'cancelled', + 'unknown', +] as const; + +export type UnitTestRunFailureCategory = typeof UNITTEST_RUN_FAILURE_CATEGORIES[number]; + export enum PlatformErrors { FailedToParseVersion = 'FailedToParseVersion', FailedToDetermineOS = 'FailedToDetermineOS', diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index bdf8f81c30c0..3a6a27c01f08 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -11,7 +11,7 @@ import { isPromise } from '../common/utils/async'; import { StopWatch } from '../common/utils/stopWatch'; import { EnvironmentType, PythonEnvironment } from '../pythonEnvironments/info'; import { TensorBoardPromptSelection } from '../tensorBoard/constants'; -import { EventName } from './constants'; +import { EventName, type UnitTestRunFailureCategory } from './constants'; import type { TestTool } from './types'; /** @@ -2284,13 +2284,7 @@ export interface IEventNamePropertyMapping { /** * Coarse failure category when `failed` is true. */ - failureCategory?: - | 'pipe-cancelled' - | 'subprocess-crash' - | 'no-results' - | 'env-mismatch' - | 'cancelled' - | 'unknown'; + failureCategory?: UnitTestRunFailureCategory; /** * Wall-clock duration of the run in milliseconds. */ diff --git a/src/client/testing/testController/common/projectTestExecution.ts b/src/client/testing/testController/common/projectTestExecution.ts index 99aee4d64874..e03fee1acbde 100644 --- a/src/client/testing/testController/common/projectTestExecution.ts +++ b/src/client/testing/testController/common/projectTestExecution.ts @@ -4,7 +4,7 @@ import { CancellationToken, FileCoverageDetail, TestItem, TestRun, TestRunProfileKind, TestRunRequest } from 'vscode'; import { traceError, traceInfo, traceVerbose, traceWarn } from '../../../logging'; import { sendTelemetryEvent } from '../../../telemetry'; -import { EventName } from '../../../telemetry/constants'; +import { EventName, type UnitTestRunFailureCategory } from '../../../telemetry/constants'; import { StopWatch } from '../../../common/utils/stopWatch'; import { IPythonExecutionFactory } from '../../../common/process/types'; import { ITestDebugLauncher } from '../../common/types'; @@ -73,14 +73,7 @@ export async function executeTestsForProjects( const stopWatch = new StopWatch(); let failed = false; - let failureCategory: - | 'pipe-cancelled' - | 'subprocess-crash' - | 'no-results' - | 'env-mismatch' - | 'cancelled' - | 'unknown' - | undefined; + let failureCategory: UnitTestRunFailureCategory | undefined; try { await executeTestsForProject(project, items, runInstance, request, deps); } catch (error) { diff --git a/src/client/testing/testController/common/utils.ts b/src/client/testing/testController/common/utils.ts index e4474c1b69d5..dc1872012c0f 100644 --- a/src/client/testing/testController/common/utils.ts +++ b/src/client/testing/testController/common/utils.ts @@ -223,6 +223,10 @@ export function buildErrorNodeOptions( }; } +/** + * Populates the VS Code test tree from discovered test data. + * @returns The number of leaf test items added or updated while walking the tree. + */ export function populateTestTree( testController: TestController, testTreeData: DiscoveredTestNode, diff --git a/src/client/testing/testController/controller.ts b/src/client/testing/testController/controller.ts index dfa35fb384a4..f4046a9419c0 100644 --- a/src/client/testing/testController/controller.ts +++ b/src/client/testing/testController/controller.ts @@ -32,7 +32,7 @@ import { StopWatch } from '../../common/utils/stopWatch'; import { IInterpreterService } from '../../interpreter/contracts'; import { traceError, traceInfo, traceVerbose } from '../../logging'; import { sendTelemetryEvent } from '../../telemetry'; -import { EventName } from '../../telemetry/constants'; +import { EventName, type UnitTestRunFailureCategory } from '../../telemetry/constants'; import { PYTEST_PROVIDER, UNITTEST_PROVIDER } from '../common/constants'; import { TestProvider } from '../types'; import { createErrorTestItem, DebugTestTag, getNodeByUri, RunTestTag } from './common/testItemUtilities'; @@ -923,14 +923,7 @@ export class PythonTestController implements ITestController, IExtensionSingleAc const stopWatch = new StopWatch(); let failed = false; - let failureCategory: - | 'pipe-cancelled' - | 'subprocess-crash' - | 'no-results' - | 'env-mismatch' - | 'cancelled' - | 'unknown' - | undefined; + let failureCategory: UnitTestRunFailureCategory | undefined; try { await testAdapter.executeTests( this.testController, From d9d610b68674d0c9c31ddee495ca97deb28ff16c Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Fri, 29 May 2026 12:51:36 -0700 Subject: [PATCH 6/7] linting --- src/client/telemetry/index.ts | 3 ++- .../testing/testController/common/projectTestExecution.ts | 3 ++- src/client/testing/testController/controller.ts | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index 3a6a27c01f08..fe1ce81ac192 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -11,7 +11,8 @@ import { isPromise } from '../common/utils/async'; import { StopWatch } from '../common/utils/stopWatch'; import { EnvironmentType, PythonEnvironment } from '../pythonEnvironments/info'; import { TensorBoardPromptSelection } from '../tensorBoard/constants'; -import { EventName, type UnitTestRunFailureCategory } from './constants'; +import { EventName } from './constants'; +import type { UnitTestRunFailureCategory } from './constants'; import type { TestTool } from './types'; /** diff --git a/src/client/testing/testController/common/projectTestExecution.ts b/src/client/testing/testController/common/projectTestExecution.ts index e03fee1acbde..9e87fde89407 100644 --- a/src/client/testing/testController/common/projectTestExecution.ts +++ b/src/client/testing/testController/common/projectTestExecution.ts @@ -4,7 +4,8 @@ import { CancellationToken, FileCoverageDetail, TestItem, TestRun, TestRunProfileKind, TestRunRequest } from 'vscode'; import { traceError, traceInfo, traceVerbose, traceWarn } from '../../../logging'; import { sendTelemetryEvent } from '../../../telemetry'; -import { EventName, type UnitTestRunFailureCategory } from '../../../telemetry/constants'; +import { EventName } from '../../../telemetry/constants'; +import type { UnitTestRunFailureCategory } from '../../../telemetry/constants'; import { StopWatch } from '../../../common/utils/stopWatch'; import { IPythonExecutionFactory } from '../../../common/process/types'; import { ITestDebugLauncher } from '../../common/types'; diff --git a/src/client/testing/testController/controller.ts b/src/client/testing/testController/controller.ts index f4046a9419c0..d93e91803a9b 100644 --- a/src/client/testing/testController/controller.ts +++ b/src/client/testing/testController/controller.ts @@ -32,7 +32,8 @@ import { StopWatch } from '../../common/utils/stopWatch'; import { IInterpreterService } from '../../interpreter/contracts'; import { traceError, traceInfo, traceVerbose } from '../../logging'; import { sendTelemetryEvent } from '../../telemetry'; -import { EventName, type UnitTestRunFailureCategory } from '../../telemetry/constants'; +import { EventName } from '../../telemetry/constants'; +import type { UnitTestRunFailureCategory } from '../../telemetry/constants'; import { PYTEST_PROVIDER, UNITTEST_PROVIDER } from '../common/constants'; import { TestProvider } from '../types'; import { createErrorTestItem, DebugTestTag, getNodeByUri, RunTestTag } from './common/testItemUtilities'; From 2d15d7e52c9f3a217877b4fe5870516afe6b7882 Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Fri, 29 May 2026 14:03:09 -0700 Subject: [PATCH 7/7] fix test --- .../common/testDiscoveryHandler.unit.test.ts | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/test/testing/testController/common/testDiscoveryHandler.unit.test.ts b/src/test/testing/testController/common/testDiscoveryHandler.unit.test.ts index 458e3d984405..88742a820234 100644 --- a/src/test/testing/testController/common/testDiscoveryHandler.unit.test.ts +++ b/src/test/testing/testController/common/testDiscoveryHandler.unit.test.ts @@ -288,9 +288,6 @@ suite('TestDiscoveryHandler', () => { const populateTestTreeStub = sinon.stub(utils, 'populateTestTree'); testItemIndexMock.setup((x) => x.clear()).returns(() => undefined); - testItemIndexMock.setup((x) => x.runIdToTestItemMap).returns(() => new Map()); - testItemIndexMock.setup((x) => x.runIdToVSidMap).returns(() => new Map()); - testItemIndexMock.setup((x) => x.vsIdToRunIdMap).returns(() => new Map()); discoveryHandler.processDiscovery( payload, @@ -316,11 +313,8 @@ suite('TestDiscoveryHandler', () => { const populateTestTreeStub = sinon.stub(utils, 'populateTestTree'); testItemIndexMock.setup((x) => x.clear()).returns(() => undefined); - testItemIndexMock.setup((x) => x.runIdToTestItemMap).returns(() => new Map()); - testItemIndexMock.setup((x) => x.runIdToVSidMap).returns(() => new Map()); - testItemIndexMock.setup((x) => x.vsIdToRunIdMap).returns(() => new Map()); - discoveryHandler.processDiscovery( + const testCount = discoveryHandler.processDiscovery( payload, testControllerMock.object, testItemIndexMock.object, @@ -329,8 +323,8 @@ suite('TestDiscoveryHandler', () => { cancelationToken, ); - // Should still call populateTestTree with null - assert.ok(populateTestTreeStub.calledOnce); + assert.strictEqual(testCount, 0); + assert.strictEqual(populateTestTreeStub.called, false); testItemIndexMock.verify((x) => x.clear(), typemoq.Times.once()); }); });