From 14bbccaee1349960eeb8960b08f3e68ac0dfd22a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 12 Dec 2025 17:54:16 +0000 Subject: [PATCH 1/4] Initial plan From 29a3c8a42ea4ac1e9f0b2e08f22a3c9b0cef9e6f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 12 Dec 2025 18:04:23 +0000 Subject: [PATCH 2/4] Register test commands early in activation to fix race condition - Extract command registration logic from UnitTestManagementService into standalone registerTestCommands function - Call registerTestCommands synchronously in activateLegacy before async operations - This ensures commands are available immediately when extension activates, preventing "command not found" errors Co-authored-by: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> --- src/client/extensionActivation.ts | 5 + src/client/testing/main.ts | 151 +++++++++++++++++------------- 2 files changed, 90 insertions(+), 66 deletions(-) diff --git a/src/client/extensionActivation.ts b/src/client/extensionActivation.ts index 8330d5010f7a..89aa99c62456 100644 --- a/src/client/extensionActivation.ts +++ b/src/client/extensionActivation.ts @@ -34,6 +34,7 @@ import { registerTypes as tensorBoardRegisterTypes } from './tensorBoard/service import { registerTypes as commonRegisterTerminalTypes } from './terminals/serviceRegistry'; import { ICodeExecutionHelper, ICodeExecutionManager, ITerminalAutoActivation } from './terminals/types'; import { registerTypes as unitTestsRegisterTypes } from './testing/serviceRegistry'; +import { registerTestCommands } from './testing/main'; // components import * as pythonEnvironments from './pythonEnvironments'; @@ -186,6 +187,10 @@ async function activateLegacy(ext: ExtensionState, startupStopWatch: StopWatch): await registerPythonStartup(ext.context); serviceManager.get(ICodeExecutionManager).registerCommands(); + + // Register test commands early to prevent race conditions where commands + // are invoked before extension activation completes + registerTestCommands(serviceContainer); disposables.push(new ReplProvider(serviceContainer)); diff --git a/src/client/testing/main.ts b/src/client/testing/main.ts index e794d5711f2a..f63858351177 100644 --- a/src/client/testing/main.ts +++ b/src/client/testing/main.ts @@ -42,6 +42,89 @@ export class TestingService implements ITestingService { } } +/** + * Registers test commands early in the extension activation to prevent race conditions. + * This function should be called synchronously before any async operations in the activation flow. + */ +export function registerTestCommands(serviceContainer: IServiceContainer): void { + const disposableRegistry = serviceContainer.get(IDisposableRegistry); + const commandManager = serviceContainer.get(ICommandManager); + const workspaceService = serviceContainer.get(IWorkspaceService); + + // Get test controller if available + let testController: ITestController | undefined; + if (tests && !!tests.createTestController) { + testController = serviceContainer.get(ITestController); + } + + // Helper function to configure tests + const configureTestsHandler = async (resource?: Uri) => { + // Send telemetry for test configuration + sendTelemetryEvent(EventName.UNITTEST_CONFIGURE); + + let wkspace: Uri | undefined; + if (resource) { + const wkspaceFolder = workspaceService.getWorkspaceFolder(resource); + wkspace = wkspaceFolder ? wkspaceFolder.uri : undefined; + } else { + const appShell = serviceContainer.get(IApplicationShell); + wkspace = await selectTestWorkspace(appShell); + } + if (!wkspace) { + return; + } + const interpreterService = serviceContainer.get(IInterpreterService); + const cmdManager = serviceContainer.get(ICommandManager); + if (!(await interpreterService.getActiveInterpreter(wkspace))) { + cmdManager.executeCommand(constants.Commands.TriggerEnvironmentSelection, wkspace); + return; + } + const configurationService = serviceContainer.get(ITestConfigurationService); + await configurationService.promptToEnableAndConfigureTestFramework(wkspace); + }; + + disposableRegistry.push( + commandManager.registerCommand( + constants.Commands.Tests_Configure, + (_, _cmdSource: constants.CommandSource = constants.CommandSource.commandPalette, resource?: Uri) => { + // Ignore the exceptions returned. + // This command will be invoked from other places of the extension. + configureTestsHandler(resource).ignoreErrors(); + traceVerbose('Testing: Trigger refresh after config change'); + testController?.refreshTestData(resource, { forceRefresh: true }); + }, + ), + commandManager.registerCommand(constants.Commands.Tests_CopilotSetup, (resource?: Uri): + | { message: string; command: Command } + | undefined => { + const wkspaceFolder = + workspaceService.getWorkspaceFolder(resource) || workspaceService.workspaceFolders?.at(0); + if (!wkspaceFolder) { + return undefined; + } + + const configurationService = serviceContainer.get( + ITestConfigurationService, + ); + if (configurationService.hasConfiguredTests(wkspaceFolder.uri)) { + return undefined; + } + + return { + message: Testing.copilotSetupMessage, + command: { + title: Testing.configureTests, + command: constants.Commands.Tests_Configure, + arguments: [undefined, constants.CommandSource.ui, resource], + }, + }; + }), + commandManager.registerCommand(constants.Commands.CopyTestId, async (testItem: TestItem) => { + writeTestIdToClipboard(testItem); + }), + ); +} + @injectable() export class UnitTestManagementService implements IExtensionActivationService { private activatedOnce: boolean = false; @@ -80,7 +163,8 @@ export class UnitTestManagementService implements IExtensionActivationService { this.activatedOnce = true; this.registerHandlers(); - this.registerCommands(); + // Note: Commands are now registered early in extensionActivation.ts via registerTestCommands() + // to avoid race conditions where commands are invoked before extension activation completes. if (!!tests.testResults) { await this.updateTestUIButtons(); @@ -130,72 +214,7 @@ export class UnitTestManagementService implements IExtensionActivationService { await Promise.all(changedWorkspaces.map((u) => this.testController?.refreshTestData(u))); } - @captureTelemetry(EventName.UNITTEST_CONFIGURE, undefined, false) - private async configureTests(resource?: Uri) { - let wkspace: Uri | undefined; - if (resource) { - const wkspaceFolder = this.workspaceService.getWorkspaceFolder(resource); - wkspace = wkspaceFolder ? wkspaceFolder.uri : undefined; - } else { - const appShell = this.serviceContainer.get(IApplicationShell); - wkspace = await selectTestWorkspace(appShell); - } - if (!wkspace) { - return; - } - const interpreterService = this.serviceContainer.get(IInterpreterService); - const commandManager = this.serviceContainer.get(ICommandManager); - if (!(await interpreterService.getActiveInterpreter(wkspace))) { - commandManager.executeCommand(constants.Commands.TriggerEnvironmentSelection, wkspace); - return; - } - const configurationService = this.serviceContainer.get(ITestConfigurationService); - await configurationService.promptToEnableAndConfigureTestFramework(wkspace!); - } - - private registerCommands(): void { - const commandManager = this.serviceContainer.get(ICommandManager); - this.disposableRegistry.push( - commandManager.registerCommand( - constants.Commands.Tests_Configure, - (_, _cmdSource: constants.CommandSource = constants.CommandSource.commandPalette, resource?: Uri) => { - // Ignore the exceptions returned. - // This command will be invoked from other places of the extension. - this.configureTests(resource).ignoreErrors(); - traceVerbose('Testing: Trigger refresh after config change'); - this.testController?.refreshTestData(resource, { forceRefresh: true }); - }, - ), - commandManager.registerCommand(constants.Commands.Tests_CopilotSetup, (resource?: Uri): - | { message: string; command: Command } - | undefined => { - const wkspaceFolder = - this.workspaceService.getWorkspaceFolder(resource) || this.workspaceService.workspaceFolders?.at(0); - if (!wkspaceFolder) { - return undefined; - } - - const configurationService = this.serviceContainer.get( - ITestConfigurationService, - ); - if (configurationService.hasConfiguredTests(wkspaceFolder.uri)) { - return undefined; - } - return { - message: Testing.copilotSetupMessage, - command: { - title: Testing.configureTests, - command: constants.Commands.Tests_Configure, - arguments: [undefined, constants.CommandSource.ui, resource], - }, - }; - }), - commandManager.registerCommand(constants.Commands.CopyTestId, async (testItem: TestItem) => { - writeTestIdToClipboard(testItem); - }), - ); - } private registerHandlers() { const interpreterService = this.serviceContainer.get(IInterpreterService); From 9a866c6ef60405c225d22ea9f7cb82a3725e0ef9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 12 Dec 2025 18:08:19 +0000 Subject: [PATCH 3/4] Move test service and command registration before first await in activation - Register test services (unitTestsRegisterTypes) before any async operations in extension.ts - Register test commands immediately after service registration, before experimentService.activate() - Remove duplicate registrations from extensionActivation.ts activateLegacy() - This ensures commands are available synchronously when extension activates Co-authored-by: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> --- src/client/extension.ts | 8 ++++++++ src/client/extensionActivation.ts | 7 +------ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/client/extension.ts b/src/client/extension.ts index af26a5657330..7025a015e1dd 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -45,6 +45,8 @@ import { buildProposedApi } from './proposedApi'; import { GLOBAL_PERSISTENT_KEYS } from './common/persistentState'; import { registerTools } from './chat'; import { IRecommendedEnvironmentService } from './interpreter/configuration/types'; +import { registerTypes as unitTestsRegisterTypes } from './testing/serviceRegistry'; +import { registerTestCommands } from './testing/main'; durations.codeLoadingTime = stopWatch.elapsedTime; @@ -121,6 +123,12 @@ async function activateUnsafe( // Note standard utils especially experiment and platform code are fundamental to the extension // and should be available before we activate anything else.Hence register them first. initializeStandard(ext); + + // Register test services and commands early, before any async operations, to prevent race conditions + // where commands are invoked before the extension has fully activated. + unitTestsRegisterTypes(ext.legacyIOC.serviceManager); + registerTestCommands(activatedServiceContainer); + // We need to activate experiments before initializing components as objects are created or not created based on experiments. const experimentService = activatedServiceContainer.get(IExperimentService); // This guarantees that all experiment information has loaded & all telemetry will contain experiment info. diff --git a/src/client/extensionActivation.ts b/src/client/extensionActivation.ts index 89aa99c62456..0c13d2bfc8d1 100644 --- a/src/client/extensionActivation.ts +++ b/src/client/extensionActivation.ts @@ -34,7 +34,6 @@ import { registerTypes as tensorBoardRegisterTypes } from './tensorBoard/service import { registerTypes as commonRegisterTerminalTypes } from './terminals/serviceRegistry'; import { ICodeExecutionHelper, ICodeExecutionManager, ITerminalAutoActivation } from './terminals/types'; import { registerTypes as unitTestsRegisterTypes } from './testing/serviceRegistry'; -import { registerTestCommands } from './testing/main'; // components import * as pythonEnvironments from './pythonEnvironments'; @@ -145,7 +144,7 @@ async function activateLegacy(ext: ExtensionState, startupStopWatch: StopWatch): const { enableProposedApi } = applicationEnv.packageJson; serviceManager.addSingletonInstance(UseProposedApi, enableProposedApi); // Feature specific registrations. - unitTestsRegisterTypes(serviceManager); + // Note: unitTestsRegisterTypes is now called earlier in extension.ts before the first await installerRegisterTypes(serviceManager); commonRegisterTerminalTypes(serviceManager); debugConfigurationRegisterTypes(serviceManager); @@ -187,10 +186,6 @@ async function activateLegacy(ext: ExtensionState, startupStopWatch: StopWatch): await registerPythonStartup(ext.context); serviceManager.get(ICodeExecutionManager).registerCommands(); - - // Register test commands early to prevent race conditions where commands - // are invoked before extension activation completes - registerTestCommands(serviceContainer); disposables.push(new ReplProvider(serviceContainer)); From d29aac7a1a8a731c2e16c955629f2ed79b8d1e19 Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Fri, 12 Dec 2025 13:43:14 -0800 Subject: [PATCH 4/4] updates --- src/client/extension.ts | 7 +++-- src/client/extensionActivation.ts | 2 -- src/client/testing/main.ts | 44 +++++++++++++++---------------- 3 files changed, 24 insertions(+), 29 deletions(-) diff --git a/src/client/extension.ts b/src/client/extension.ts index 7025a015e1dd..c3fb2a3ab3b0 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -123,12 +123,11 @@ async function activateUnsafe( // Note standard utils especially experiment and platform code are fundamental to the extension // and should be available before we activate anything else.Hence register them first. initializeStandard(ext); - - // Register test services and commands early, before any async operations, to prevent race conditions - // where commands are invoked before the extension has fully activated. + + // Register test services and commands early to prevent race conditions. unitTestsRegisterTypes(ext.legacyIOC.serviceManager); registerTestCommands(activatedServiceContainer); - + // We need to activate experiments before initializing components as objects are created or not created based on experiments. const experimentService = activatedServiceContainer.get(IExperimentService); // This guarantees that all experiment information has loaded & all telemetry will contain experiment info. diff --git a/src/client/extensionActivation.ts b/src/client/extensionActivation.ts index 0c13d2bfc8d1..6e870e37ef3e 100644 --- a/src/client/extensionActivation.ts +++ b/src/client/extensionActivation.ts @@ -33,7 +33,6 @@ import { setExtensionInstallTelemetryProperties } from './telemetry/extensionIns import { registerTypes as tensorBoardRegisterTypes } from './tensorBoard/serviceRegistry'; import { registerTypes as commonRegisterTerminalTypes } from './terminals/serviceRegistry'; import { ICodeExecutionHelper, ICodeExecutionManager, ITerminalAutoActivation } from './terminals/types'; -import { registerTypes as unitTestsRegisterTypes } from './testing/serviceRegistry'; // components import * as pythonEnvironments from './pythonEnvironments'; @@ -144,7 +143,6 @@ async function activateLegacy(ext: ExtensionState, startupStopWatch: StopWatch): const { enableProposedApi } = applicationEnv.packageJson; serviceManager.addSingletonInstance(UseProposedApi, enableProposedApi); // Feature specific registrations. - // Note: unitTestsRegisterTypes is now called earlier in extension.ts before the first await installerRegisterTypes(serviceManager); commonRegisterTerminalTypes(serviceManager); debugConfigurationRegisterTypes(serviceManager); diff --git a/src/client/testing/main.ts b/src/client/testing/main.ts index f63858351177..eed4d70e852c 100644 --- a/src/client/testing/main.ts +++ b/src/client/testing/main.ts @@ -18,7 +18,7 @@ import { IDisposableRegistry, Product } from '../common/types'; import { IInterpreterService } from '../interpreter/contracts'; import { IServiceContainer } from '../ioc/types'; import { EventName } from '../telemetry/constants'; -import { captureTelemetry, sendTelemetryEvent } from '../telemetry/index'; +import { sendTelemetryEvent } from '../telemetry/index'; import { selectTestWorkspace } from './common/testUtils'; import { TestSettingsPropertyNames } from './configuration/types'; import { ITestConfigurationService, ITestsHelper } from './common/types'; @@ -43,25 +43,21 @@ export class TestingService implements ITestingService { } /** - * Registers test commands early in the extension activation to prevent race conditions. - * This function should be called synchronously before any async operations in the activation flow. + * Registers command handlers but defers service resolution until the commands are actually invoked, + * allowing registration to happen before all services are fully initialized. */ export function registerTestCommands(serviceContainer: IServiceContainer): void { + // Resolve only the essential services needed for command registration itself const disposableRegistry = serviceContainer.get(IDisposableRegistry); const commandManager = serviceContainer.get(ICommandManager); - const workspaceService = serviceContainer.get(IWorkspaceService); - - // Get test controller if available - let testController: ITestController | undefined; - if (tests && !!tests.createTestController) { - testController = serviceContainer.get(ITestController); - } - // Helper function to configure tests + // Helper function to configure tests - services are resolved when invoked, not at registration time const configureTestsHandler = async (resource?: Uri) => { - // Send telemetry for test configuration sendTelemetryEvent(EventName.UNITTEST_CONFIGURE); - + + // Resolve services lazily when the command is invoked + const workspaceService = serviceContainer.get(IWorkspaceService); + let wkspace: Uri | undefined; if (resource) { const wkspaceFolder = workspaceService.getWorkspaceFolder(resource); @@ -84,28 +80,33 @@ export function registerTestCommands(serviceContainer: IServiceContainer): void }; disposableRegistry.push( + // Command: python.configureTests - prompts user to configure test framework commandManager.registerCommand( constants.Commands.Tests_Configure, (_, _cmdSource: constants.CommandSource = constants.CommandSource.commandPalette, resource?: Uri) => { - // Ignore the exceptions returned. - // This command will be invoked from other places of the extension. + // Invoke configuration handler (errors are ignored as this can be called from multiple places) configureTestsHandler(resource).ignoreErrors(); traceVerbose('Testing: Trigger refresh after config change'); - testController?.refreshTestData(resource, { forceRefresh: true }); + // Refresh test data if test controller is available (resolved lazily) + if (tests && !!tests.createTestController) { + const testController = serviceContainer.get(ITestController); + testController?.refreshTestData(resource, { forceRefresh: true }); + } }, ), + // Command: python.tests.copilotSetup - Copilot integration for test setup commandManager.registerCommand(constants.Commands.Tests_CopilotSetup, (resource?: Uri): | { message: string; command: Command } | undefined => { + // Resolve services lazily when the command is invoked + const workspaceService = serviceContainer.get(IWorkspaceService); const wkspaceFolder = workspaceService.getWorkspaceFolder(resource) || workspaceService.workspaceFolders?.at(0); if (!wkspaceFolder) { return undefined; } - const configurationService = serviceContainer.get( - ITestConfigurationService, - ); + const configurationService = serviceContainer.get(ITestConfigurationService); if (configurationService.hasConfiguredTests(wkspaceFolder.uri)) { return undefined; } @@ -119,6 +120,7 @@ export function registerTestCommands(serviceContainer: IServiceContainer): void }, }; }), + // Command: python.copyTestId - copies test ID to clipboard commandManager.registerCommand(constants.Commands.CopyTestId, async (testItem: TestItem) => { writeTestIdToClipboard(testItem); }), @@ -163,8 +165,6 @@ export class UnitTestManagementService implements IExtensionActivationService { this.activatedOnce = true; this.registerHandlers(); - // Note: Commands are now registered early in extensionActivation.ts via registerTestCommands() - // to avoid race conditions where commands are invoked before extension activation completes. if (!!tests.testResults) { await this.updateTestUIButtons(); @@ -214,8 +214,6 @@ export class UnitTestManagementService implements IExtensionActivationService { await Promise.all(changedWorkspaces.map((u) => this.testController?.refreshTestData(u))); } - - private registerHandlers() { const interpreterService = this.serviceContainer.get(IInterpreterService); this.disposableRegistry.push(