cleanup

Author: eleanorjboyd

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',

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';
         /**

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

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<DiscoveryTelemetryCycle, 'stopWatch'>): void {
+        this.activeCycle = { ...context, stopWatch: new StopWatch() };
+    }
+
+    public complete(): DiscoveryTelemetryCycle | undefined {
+        const cycle = this.activeCycle;
+        this.activeCycle = undefined;
+        return cycle;
+    }
+}

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<string, FileCoverageDetail[]>();
 
+    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<DiscoveryCycleContext, 'stopWatch'>): 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;
-}

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;
     }
 
     /**

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');

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 {