diff --git a/src/client/telemetry/constants.ts b/src/client/telemetry/constants.ts index eff32a6e3299..1a5e26503f3b 100644 --- a/src/client/telemetry/constants.ts +++ b/src/client/telemetry/constants.ts @@ -48,6 +48,7 @@ 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_DISABLED = 'UNITTEST.DISABLED', @@ -101,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 763f7405aa0d..fe1ce81ac192 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -12,6 +12,7 @@ import { StopWatch } from '../common/utils/stopWatch'; import { EnvironmentType, PythonEnvironment } from '../pythonEnvironments/info'; import { TensorBoardPromptSelection } from '../tensorBoard/constants'; import { EventName } from './constants'; +import type { UnitTestRunFailureCategory } from './constants'; import type { TestTool } from './types'; /** @@ -2165,7 +2166,12 @@ 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 } } */ [EventName.UNITTEST_DISCOVERY_DONE]: { @@ -2181,6 +2187,36 @@ export interface IEventNamePropertyMapping { * @type {boolean} */ failed: boolean; + /** + * 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'; + /** + * Source that triggered the discovery. + */ + 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; }; /** * Telemetry event sent when cancelling discovering tests @@ -2222,6 +2258,43 @@ 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" }, + "durationMs" : { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd", "isMeasurement": true }, + "requestedCount" : { "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?: UnitTestRunFailureCategory; + /** + * Wall-clock duration of the run in milliseconds. + */ + durationMs?: number; + /** + * Number of test items the user asked to run. + */ + requestedCount?: 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..318d68104b8c 100644 --- a/src/client/testing/main.ts +++ b/src/client/testing/main.ts @@ -223,7 +223,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/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/projectTestExecution.ts b/src/client/testing/testController/common/projectTestExecution.ts index fe3b4f91491a..9e87fde89407 100644 --- a/src/client/testing/testController/common/projectTestExecution.ts +++ b/src/client/testing/testController/common/projectTestExecution.ts @@ -5,6 +5,8 @@ import { CancellationToken, FileCoverageDetail, TestItem, TestRun, TestRunProfil import { traceError, traceInfo, traceVerbose, traceWarn } from '../../../logging'; import { sendTelemetryEvent } from '../../../telemetry'; 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'; import { ProjectAdapter } from './projectAdapter'; @@ -70,13 +72,28 @@ export async function executeTestsForProjects( debugging: isDebugMode, }); + const stopWatch = new StopWatch(); + let failed = false; + let failureCategory: UnitTestRunFailureCategory | 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..871fe136f321 100644 --- a/src/client/testing/testController/common/resultResolver.ts +++ b/src/client/testing/testController/common/resultResolver.ts @@ -11,6 +11,7 @@ import { TestItemIndex } from './testItemIndex'; import { TestDiscoveryHandler } from './testDiscoveryHandler'; import { TestExecutionHandler } from './testExecutionHandler'; import { TestCoverageHandler } from './testCoverageHandler'; +import { DiscoveryTelemetryState } from './discoveryTelemetry'; export class PythonResultResolver implements ITestResultResolver { testController: TestController; @@ -26,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. @@ -52,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) @@ -76,7 +80,7 @@ export class PythonResultResolver implements ITestResultResolver { } public resolveDiscovery(payload: DiscoveredTestPayload, token?: CancellationToken): void { - PythonResultResolver.discoveryHandler.processDiscovery( + const testCount = PythonResultResolver.discoveryHandler.processDiscovery( payload, this.testController, this.testItemIndex, @@ -86,9 +90,17 @@ export class PythonResultResolver implements ITestResultResolver { this.projectId, this.projectName, ); + 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/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 017c41cf3d97..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, @@ -34,7 +35,10 @@ export interface TestData { kind: TestDataKinds; } -export type TestRefreshOptions = { forceRefresh: boolean }; +export type TestRefreshOptions = { + forceRefresh: boolean; + trigger?: DiscoveryTriggerKind; +}; export const ITestController = Symbol('ITestController'); export interface ITestController { diff --git a/src/client/testing/testController/common/utils.ts b/src/client/testing/testController/common/utils.ts index 9782487d940b..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, @@ -231,7 +235,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 +282,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 +310,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 04de209c171d..d93e91803a9b 100644 --- a/src/client/testing/testController/controller.ts +++ b/src/client/testing/testController/controller.ts @@ -28,10 +28,12 @@ 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'; +import { sendTelemetryEvent } from '../../telemetry'; 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'; @@ -40,6 +42,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'; @@ -49,11 +52,6 @@ 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 TriggerKeyType = keyof EventPropertyType; -type TriggerType = EventPropertyType[TriggerKeyType]; - @injectable() export class PythonTestController implements ITestController, IExtensionSingleActivationService { public readonly supportedWorkspaceTypes = { untrustedWorkspace: false, virtualWorkspace: false }; @@ -64,7 +62,13 @@ 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[] = []; + 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. + */ + private currentDiscoveryTrigger?: DiscoveryTriggerKind; private readonly testController: TestController; @@ -161,9 +165,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 }); }; } @@ -425,6 +427,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. @@ -469,6 +474,7 @@ export class PythonTestController implements ITestController, IExtensionSingleAc } } finally { this.refreshingCompletedEvent.fire(); + this.currentDiscoveryTrigger = undefined; } } @@ -578,6 +584,13 @@ export class PythonTestController implements ITestController, IExtensionSingleAc traceInfo(`[test-by-project] Discovering tests for project: ${project.projectName}`); project.isDiscovering = true; + // 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, + }); + // 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 +608,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).discoveryTelemetry?.complete(); + sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_DONE, undefined, { + tool: project.testProvider, + failed: true, + mode: 'project', + trigger: cycle?.trigger ?? this.currentDiscoveryTrigger, + failureCategory: this.refreshCancellation.token.isCancellationRequested ? 'cancelled' : 'unknown', + totalDurationMs: cycle?.stopWatch.elapsedTime, + }); } finally { project.isDiscovering = false; } @@ -652,6 +674,7 @@ export class PythonTestController implements ITestController, IExtensionSingleAc this.pythonExecFactory, this.refreshCancellation.token, await this.interpreterService.getActiveInterpreter(workspaceUri), + this.currentDiscoveryTrigger, ); } @@ -893,21 +916,41 @@ 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: UnitTestRunFailureCategory | 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) { @@ -976,17 +1019,15 @@ export class PythonTestController implements ITestController, IExtensionSingleAc ); } - /** - * Send UNITTEST_DISCOVERY_TRIGGER telemetry event only once per trigger type. - * - * @param triggerType The trigger type to send telemetry for. - */ - private sendTriggerTelemetry(trigger: TriggerType): void { + private setDiscoveryTrigger(trigger: DiscoveryTriggerKind): void { + this.currentDiscoveryTrigger = trigger; + } + + private sendTriggerTelemetry(trigger: DiscoveryTriggerKind): void { + this.setDiscoveryTrigger(trigger); if (!this.triggerTypes.includes(trigger)) { - sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_TRIGGER, undefined, { - trigger, - }); this.triggerTypes.push(trigger); + sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_TRIGGER, undefined, { trigger }); } } diff --git a/src/client/testing/testController/workspaceTestAdapter.ts b/src/client/testing/testController/workspaceTestAdapter.ts index f17687732f57..d43e58fd2aaf 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,8 @@ 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'; +import { DiscoveryTriggerKind } from './common/discoveryTelemetry'; /** * This class exposes a test-provider-agnostic way of discovering tests. @@ -120,6 +123,7 @@ export class WorkspaceTestAdapter { executionFactory: IPythonExecutionFactory, token?: CancellationToken, interpreter?: PythonEnvironment, + trigger?: DiscoveryTriggerKind, ): Promise { sendTelemetryEvent(EventName.UNITTEST_DISCOVERING, undefined, { tool: this.testProvider }); @@ -131,6 +135,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 property working. + (this.resultResolver as Partial).discoveryTelemetry?.start({ + mode: 'legacy', + trigger, + }); try { if (executionFactory === undefined) { @@ -139,7 +152,15 @@ 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 }); + const cycle = (this.resultResolver as Partial).discoveryTelemetry?.complete(); + sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_DONE, undefined, { + tool: this.testProvider, + failed: true, + mode: 'legacy', + trigger: cycle?.trigger ?? trigger, + failureCategory: token?.isCancellationRequested ? 'cancelled' : 'unknown', + totalDurationMs: cycle?.stopWatch.elapsedTime ?? 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..2fe71e3a0697 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', () => { @@ -501,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 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' }); @@ -517,8 +518,13 @@ 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 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; }); test('should stop processing remaining projects when cancellation requested mid-execution', async () => { @@ -594,7 +600,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 run telemetry when run profile is Debug', async () => { // Mock const project = createMockProjectAdapter({ projectPath: '/workspace/proj', projectName: 'proj' }); project.resultResolver.vsIdToRunId.set('test1', 'runId1'); @@ -607,9 +613,14 @@ 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]; + 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); + const telemetryProps = runDoneCalls[0].args[2]; expect(telemetryProps.debugging).to.be.true; }); }); 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()); }); }); diff --git a/src/test/testing/testController/resultResolver.unit.test.ts b/src/test/testing/testController/resultResolver.unit.test.ts index e4b350a20750..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', () => { @@ -85,7 +87,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); @@ -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'; @@ -171,7 +194,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 +255,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'); 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 () => {