diff --git a/.vscode-test.mjs b/.vscode-test.mjs index 6673a6cef..d6d4fdb46 100644 --- a/.vscode-test.mjs +++ b/.vscode-test.mjs @@ -8,6 +8,7 @@ import * as path from 'path'; // (e.g. /Users/runner/work///.vscode-test/user-data) overflows. const userDataDir = path.join(os.tmpdir(), 'vsct-ud'); const inlineScriptsEnabledUserDataDir = path.join(os.tmpdir(), 'vsct-is-ud'); +const inlineScriptsExperimentUserDataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'vsct-is-exp-')); const diagnosticsDir = process.env.VSCODE_TEST_DIAGNOSTICS_DIR; const diagnosticArgs = diagnosticsDir ? [`--logsPath=${path.join(diagnosticsDir, 'vscode')}`, '--verbose'] @@ -19,20 +20,25 @@ const diagnosticMocha = diagnosticsDir // Seed explicit user values so tests are deterministic even when VS Code applies // experimentation defaults. The environments extension must activate, while the // inline-script preview stays off unless a dedicated test host opts in. -function writeUserSettings(targetUserDataDir, inlineScriptsEnabled) { +function writeUserSettings(targetUserDataDir, settings) { const userDir = path.join(targetUserDataDir, 'User'); fs.mkdirSync(userDir, { recursive: true }); fs.writeFileSync( path.join(userDir, 'settings.json'), JSON.stringify({ 'python.useEnvironmentsExtension': true, - 'python-envs.inlineScripts.enabled': inlineScriptsEnabled, + ...settings, }) + '\n', ); } -writeUserSettings(userDataDir, false); -writeUserSettings(inlineScriptsEnabledUserDataDir, true); +writeUserSettings(userDataDir, { 'python-envs.inlineScripts.enabled': false }); +writeUserSettings(inlineScriptsEnabledUserDataDir, { 'python-envs.inlineScripts.enabled': true }); +writeUserSettings(inlineScriptsExperimentUserDataDir, { + 'experiments.override./pythonenvs/config.python-envs.inlineScripts.enabled': true, + // Keep the override late enough to reproduce activation before VS Code applies the experimental default. + 'experiments.overrideDelay': 30_000, +}); export default defineConfig([ { @@ -75,6 +81,24 @@ export default defineConfig([ '--disable-workspace-trust', ], }, + { + label: 'smokeTestsInlineScriptsExperimentEnabled', + files: 'out/test/smoke/inlineScriptsExperimentEnabled.smoke.test.js', + mocha: { + ...diagnosticMocha, + ui: 'tdd', + timeout: 120000, + }, + env: { + VSC_PYTHON_SMOKE_TEST: '1', + VSC_PYTHON_INLINE_SCRIPTS_EXPERIMENT_SMOKE_TEST: '1', + }, + launchArgs: [ + ...diagnosticArgs, + `--user-data-dir=${inlineScriptsExperimentUserDataDir}`, + '--disable-workspace-trust', + ], + }, { label: 'e2eTests', files: 'out/test/e2e/**/*.e2e.test.js', diff --git a/package.json b/package.json index 6bd4526a1..91a1b3364 100644 --- a/package.json +++ b/package.json @@ -772,7 +772,7 @@ "pretest": "npm run compile-tests && npm run compile", "lint": "eslint --config=eslint.config.mjs src", "unittest": "mocha --config=./build/.mocha.unittests.json", - "smoke-test": "vscode-test --label smokeTests --label smokeTestsInlineScriptsEnabled", + "smoke-test": "vscode-test --label smokeTests --label smokeTestsInlineScriptsEnabled --label smokeTestsInlineScriptsExperimentEnabled", "e2e-test": "vscode-test --label e2eTests --install-extensions ms-python.python", "integration-test": "vscode-test --label integrationTests --install-extensions ms-python.python", "integration-test-multiroot": "vscode-test --label integrationTestsMultiRoot --install-extensions ms-python.python", diff --git a/src/extension.ts b/src/extension.ts index 007e128e0..e088619c2 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -1,5 +1,6 @@ import { commands, + Disposable, ExtensionContext, extensions, l10n, @@ -13,6 +14,7 @@ import { PackageManagementOptions, PythonEnvironment, PythonEnvironmentApi, Pyth import { ENVS_EXTENSION_ID } from './common/constants'; import { ensureCorrectVersion } from './common/extVersion'; import { InlineScriptPackagesNotManagedError } from './common/inlineScript/errors'; +import { InlineScriptRoutingRegistry } from './common/inlineScript/routingRegistry'; import { registerLogger, traceError, traceInfo, traceWarn } from './common/logging'; import { setPersistentState } from './common/persistentState'; import { newProjectSelection } from './common/pickers/managers'; @@ -67,10 +69,9 @@ import { setEnvManagerCommand, setPackageManagerCommand, } from './features/envCommands'; -import type { EnvironmentManagers } from './features/envManagers'; import { PythonEnvironmentManagers } from './features/envManagers'; import { EnvVarManager, PythonEnvVariableManager } from './features/execution/envVariableManager'; -import { latchInlineScriptFeatureActivation } from './features/inlineScript/activation'; +import { createInlineScriptFeatureActivation } from './features/inlineScript/activation'; import { registerInlineScriptDiagnostics } from './features/inlineScript/diagnostics'; import { InlineScriptLazyDetector } from './features/inlineScript/lazyDetector'; import { registerInlineScriptUx } from './features/inlineScript/setupEnvironment'; @@ -107,7 +108,10 @@ import { getEnvManagerAndPackageManagerConfigLevels, runPetInTerminalImpl, } from './helpers'; -import { registerInlineScriptFeatures } from './managers/builtin/inlineScript/main'; +import { + prepareInlineScriptFeatures, + scheduleInlineScriptActivationDiscovery, +} from './managers/builtin/inlineScript/main'; import { registerSystemPythonFeatures } from './managers/builtin/main'; import { SysPythonManager } from './managers/builtin/sysPythonManager'; import { @@ -174,18 +178,14 @@ export async function activate(context: ExtensionContext): Promise(); + void inlineScriptManagerDependencies.promise.catch(() => undefined); + let inlineScriptFeatureRegistration: Promise | undefined; + const activateInlineScriptFeature = async ( + nativeFinder: NativePythonFinder, + baseManager: SysPythonManager, + ): Promise => { + const featureDisposables: Disposable[] = []; + const routingRegistry = new InlineScriptRoutingRegistry(); + featureDisposables.push(routingRegistry); + try { + const prepared = await prepareInlineScriptFeatures( + nativeFinder, + featureDisposables, + outputChannel, + baseManager, + context.globalStorageUri, + routingRegistry, + context.workspaceState, + ); + featureDisposables.push(envManagers.attachInlineScriptRouting(routingRegistry)); + featureDisposables.push(registerInlineScriptDiagnostics()); + featureDisposables.push( + commands.registerCommand('python-envs.clearScriptEnvCache', async () => { + await clearScriptEnvironmentCacheCommand(envManagers); + }), + ); + featureDisposables.push(...registerInlineScriptUx(envManagers, routingRegistry)); + prepared.manager.start(); + featureDisposables.push(prepared.registerManager()); + await commands.executeCommand('setContext', 'pythonEnvsInlineScriptsEnabled', true); + inlineScriptLazyDetector.enableRouting(routingRegistry); + featureDisposables.push(scheduleInlineScriptActivationDiscovery(prepared.manager)); + context.subscriptions.push(...featureDisposables); + traceInfo('PEP 723 inline-script feature activation completed.'); + } catch (error) { + for (const disposable of featureDisposables.reverse()) { + try { + await Promise.resolve(disposable.dispose()); + } catch (disposeError) { + traceError('Failed to roll back inline-script feature activation:', disposeError); + } + } + try { + await commands.executeCommand('setContext', 'pythonEnvsInlineScriptsEnabled', false); + } catch (contextError) { + traceError('Failed to reset the inline-script context after activation failure:', contextError); + } + traceError('Failed to activate PEP 723 inline-script environments:', error); + try { + sendTelemetryEvent(EventNames.MANAGER_REGISTRATION_FAILED, undefined, { + managerName: 'inlineScript', + errorType: classifyError(error), + failureStage: 'featureActivation', + }); + } catch (telemetryError) { + traceError('Failed to report inline-script activation telemetry:', telemetryError); + } + void window + .showErrorMessage( + l10n.t( + 'Failed to enable PEP 723 inline-script environments. Check the Python Environments output for details.', + ), + ) + .then(undefined, (notificationError) => + traceError('Failed to show the inline-script activation error:', notificationError), + ); + throw error; + } + }; + const enableInlineScriptFeature = (): Promise => { + inlineScriptFeatureRegistration ??= inlineScriptManagerDependencies.promise.then( + ({ nativeFinder, baseManager }) => activateInlineScriptFeature(nativeFinder, baseManager), + ); + return inlineScriptFeatureRegistration; + }; + const requestInlineScriptFeatureActivation = () => { + void enableInlineScriptFeature().catch(() => undefined); + }; + context.subscriptions.push(inlineScriptFeatureActivation.onDidRequestActivation(requestInlineScriptFeatureActivation)); + if (inlineScriptFeatureActivation.activationRequested) { + requestInlineScriptFeatureActivation(); } const environmentManagerRegistration = createDeferred(); @@ -442,14 +525,6 @@ export async function activate(context: ExtensionContext): Promise { await clearEnvironmentCachesCommand(envManagers, shellStartupProviders, context.workspaceState); }), - ...(inlineScriptFeatureActivation.enabled - ? [ - commands.registerCommand('python-envs.clearScriptEnvCache', async () => { - await clearScriptEnvironmentCacheCommand(envManagers); - }), - ] - : []), - ...(inlineScriptRouting ? registerInlineScriptUx(envManagers, inlineScriptRouting) : []), commands.registerCommand('python-envs.runInTerminal', (item) => { return runInTerminalCommand(item, api, terminalManager); }), @@ -670,6 +745,7 @@ export async function activate(context: ExtensionContext): Promise undefined) + : Promise.resolve(), ]); environmentManagerRegistration.resolve(); @@ -744,6 +809,7 @@ export async function activate(context: ExtensionContext): Promise class ManagerReadyImpl implements ManagerReady { private readonly envManagers: Map> = new Map(); private readonly pkgManagers: Map> = new Map(); + private readonly envManagerInstances = new Map(); + private readonly pkgManagerInstances = new Map(); private readonly checked: Set = new Set(); private readonly disposables: Disposable[] = []; + private disposed = false; constructor( em: EnvironmentManagers, @@ -155,6 +159,16 @@ class ManagerReadyImpl implements ManagerReady { ) { this.disposables.push( em.onDidChangeEnvironmentManager((e) => { + if (e.kind === 'unregistered') { + if (this.envManagerInstances.get(e.manager.id) !== e.manager) { + return; + } + this.envManagerInstances.delete(e.manager.id); + this.envManagers.delete(e.manager.id); + this.checked.delete(e.manager.id); + return; + } + this.envManagerInstances.set(e.manager.id, e.manager); if (this.envManagers.has(e.manager.id)) { this.envManagers.get(e.manager.id)?.resolve(); } else { @@ -164,6 +178,16 @@ class ManagerReadyImpl implements ManagerReady { } }), em.onDidChangePackageManager((e) => { + if (e.kind === 'unregistered') { + if (this.pkgManagerInstances.get(e.manager.id) !== e.manager) { + return; + } + this.pkgManagerInstances.delete(e.manager.id); + this.pkgManagers.delete(e.manager.id); + this.checked.delete(e.manager.id); + return; + } + this.pkgManagerInstances.set(e.manager.id, e.manager); if (this.pkgManagers.has(e.manager.id)) { this.pkgManagers.get(e.manager.id)?.resolve(); } else { @@ -188,6 +212,9 @@ class ManagerReadyImpl implements ManagerReady { const extId = getExtensionId(managerId); if (extId) { setImmediate(async () => { + if (this.disposed) { + return; + } const ext = getExtension(extId); if (ext && !ext.isActive) { traceInfo(`Extension for manager ${managerId} is not active: Activating...`); @@ -203,12 +230,20 @@ class ManagerReadyImpl implements ManagerReady { } public dispose(): void { + this.disposed = true; this.disposables.forEach((d) => d.dispose()); + this.envManagers.forEach((deferred) => deferred.resolve()); + this.pkgManagers.forEach((deferred) => deferred.resolve()); this.envManagers.clear(); this.pkgManagers.clear(); + this.envManagerInstances.clear(); + this.pkgManagerInstances.clear(); } private _waitForEnvManager(managerId: string): Promise { + if (this.disposed) { + return Promise.resolve(); + } if (this.envManagers.has(managerId)) { return this.envManagers.get(managerId)!.promise; } @@ -258,6 +293,9 @@ class ManagerReadyImpl implements ManagerReady { } private _waitForPkgManager(managerId: string): Promise { + if (this.disposed) { + return Promise.resolve(); + } if (this.pkgManagers.has(managerId)) { return this.pkgManagers.get(managerId)!.promise; } diff --git a/src/features/envManagers.ts b/src/features/envManagers.ts index 6d0ed3998..1eace126e 100644 --- a/src/features/envManagers.ts +++ b/src/features/envManagers.ts @@ -195,6 +195,7 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { private readonly packageManagerEventSubscriptions = new Map(); private readonly projectPackageManagers: ProjectScopedPackageManagerCache; private readonly subscriptions: Disposable[] = []; + private inlineScriptRouting: InlineScriptRoutingRegistry | undefined; /** * The last environment announced as "active" for each scope. @@ -260,7 +261,7 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { constructor( private readonly pm: PythonProjectManager, - private readonly inlineScriptRouting?: InlineScriptRoutingRegistry, + inlineScriptRouting?: InlineScriptRoutingRegistry, ) { this.projectPackageManagers = new ProjectScopedPackageManagerCache( (manager) => this.subscribeToPackageManagerEvents(manager), @@ -275,22 +276,44 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { this.subscriptions.push(subscription); } } + if (inlineScriptRouting) { + this.attachInlineScriptRouting(inlineScriptRouting); + } + } + + /** + * Attaches PEP 723 routing after the rollout setting becomes available. + * A manager collection accepts at most one routing registry per activation. + */ + public attachInlineScriptRouting(routing: InlineScriptRoutingRegistry): Disposable { if (this.inlineScriptRouting) { - this.subscriptions.push( - this.inlineScriptRouting.onDidChangeRouteability((e) => { - void this.handleInlineScriptRouteabilityChange(e).catch((error) => - traceError('Failed to refresh inline-script routing:', error), - ); - }), - this.inlineScriptRouting.onDidChangeAvailability((uri) => { - if (this.getEnvironmentManager(uri)?.id === INLINE_SCRIPT_MANAGER_ID) { - void this.refreshEnvironment(uri, true).catch((error) => - traceError('Failed to refresh inline-script availability:', error), - ); - } - }), - ); + throw new Error('Inline-script routing is already attached.'); } + + this.inlineScriptRouting = routing; + const listeners = Disposable.from( + routing.onDidChangeRouteability((e) => { + void this.handleInlineScriptRouteabilityChange(e).catch((error) => + traceError('Failed to refresh inline-script routing:', error), + ); + }), + routing.onDidChangeAvailability((uri) => { + if (this.getEnvironmentManager(uri)?.id === INLINE_SCRIPT_MANAGER_ID) { + void this.refreshEnvironment(uri, true).catch((error) => + traceError('Failed to refresh inline-script availability:', error), + ); + } + }), + ); + const registration = new Disposable(() => { + listeners.dispose(); + if (this.inlineScriptRouting === routing) { + this.inlineScriptRouting = undefined; + this._inlineRoutingOverrides.clear(); + } + }); + this.subscriptions.push(registration); + return registration; } public registerEnvironmentManager(manager: EnvironmentManager, options?: { extensionId?: string }): Disposable { diff --git a/src/features/inlineScript/activation.ts b/src/features/inlineScript/activation.ts index 205f39498..e00a01fd8 100644 --- a/src/features/inlineScript/activation.ts +++ b/src/features/inlineScript/activation.ts @@ -1,23 +1,83 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { InlineScriptRoutingRegistry } from '../../common/inlineScript/routingRegistry'; -import { isInlineScriptsFeatureEnabled } from '../../helpers'; +import { Disposable, Event, EventEmitter } from 'vscode'; +import { traceInfo } from '../../common/logging'; +import { onDidChangeConfiguration } from '../../common/workspace.apis'; +import { getUserConfiguredSetting, isInlineScriptsFeatureEnabled } from '../../helpers'; -export interface InlineScriptFeatureActivation { - readonly enabled: boolean; - readonly routingRegistry: InlineScriptRoutingRegistry | undefined; +const INLINE_SCRIPTS_SETTING = 'python-envs.inlineScripts.enabled'; + +export interface InlineScriptFeatureActivation extends Disposable { + readonly activationRequested: boolean; + readonly onDidRequestActivation: Event; } /** - * Latch the inline-script preview setting once during activation. - * The setting requires a window reload, so later config changes - * in the same activation must not change the chosen mode. + * Requests inline-script activation from an explicit startup value or from a late + * experimental default. Explicit setting changes still require a window reload. */ -export function latchInlineScriptFeatureActivation(): InlineScriptFeatureActivation { - const enabled = isInlineScriptsFeatureEnabled(); - return { - enabled, - routingRegistry: enabled ? new InlineScriptRoutingRegistry() : undefined, - }; +export function createInlineScriptFeatureActivation(): InlineScriptFeatureActivation { + return new InlineScriptFeatureActivationImpl(); +} + +class InlineScriptFeatureActivationImpl implements InlineScriptFeatureActivation { + private readonly activationEmitter = new EventEmitter(); + private configurationSubscription: Disposable | undefined; + private requested = false; + + public readonly onDidRequestActivation = this.activationEmitter.event; + + constructor() { + const explicitValue = getUserConfiguredSetting('python-envs', 'inlineScripts.enabled'); + if (explicitValue !== undefined) { + if (explicitValue) { + this.requestActivation(); + } + return; + } + + // Rollout bridge: remove this listener when the inline-script experiment gate is retired. + // Subscribe before re-reading so a late VS Code experimental default cannot be missed. + this.configurationSubscription = onDidChangeConfiguration((event) => { + if (event.affectsConfiguration(INLINE_SCRIPTS_SETTING)) { + this.handleDefaultChange(); + } + }); + this.handleDefaultChange(); + } + + public get activationRequested(): boolean { + return this.requested; + } + + private handleDefaultChange(): void { + if (getUserConfiguredSetting('python-envs', 'inlineScripts.enabled') !== undefined) { + this.stopWatching(); + return; + } + if (isInlineScriptsFeatureEnabled()) { + traceInfo('PEP 723 inline-script activation requested by the effective default setting.'); + this.requestActivation(); + } + } + + private requestActivation(): void { + if (this.requested) { + return; + } + this.requested = true; + this.stopWatching(); + this.activationEmitter.fire(); + } + + private stopWatching(): void { + this.configurationSubscription?.dispose(); + this.configurationSubscription = undefined; + } + + public dispose(): void { + this.stopWatching(); + this.activationEmitter.dispose(); + } } diff --git a/src/features/inlineScript/lazyDetector.ts b/src/features/inlineScript/lazyDetector.ts index ed4bc0c40..756821faf 100644 --- a/src/features/inlineScript/lazyDetector.ts +++ b/src/features/inlineScript/lazyDetector.ts @@ -68,8 +68,11 @@ export class InlineScriptLazyDetector implements Disposable { // disposal does not emit telemetry on a detector the host has // already torn down. private disposed = false; + private activated = false; + private routingEventsRegistered = false; + private activationReplayPending = false; - constructor(private readonly routingRegistry?: InlineScriptRoutingRegistry) {} + constructor(private routingRegistry?: InlineScriptRoutingRegistry) {} /** * Subscribe to workspace text-document events. Safe to call once @@ -92,34 +95,78 @@ export class InlineScriptLazyDetector implements Disposable { * arrive too. */ public activate(): void { + if (this.activated || this.disposed) { + return; + } + this.activated = true; this.subscriptions.push( onDidOpenTextDocument((doc) => this.handleDocument(doc, 'open')), onDidSaveTextDocument((doc) => this.handleDocument(doc, 'save')), onDidChangeTextDocument((e) => this.handleChange(e)), ); - if (this.routingRegistry) { - this.subscriptions.push( - onDidDeleteFiles((e) => e.files.forEach((uri) => this.clearRouteability(uri))), - onDidRenameFiles((e) => - Promise.all(e.files.map((file) => this.handleRename(file.oldUri, file.newUri))), - ), - ); - } + this.registerRoutingEvents(); // Defer the catch-up pass so we observe `workspace.textDocuments` // AFTER VS Code finishes registering the document that triggered // our activation. Running the loop synchronously here can race // against VS Code's own initialization on `onLanguage:*` activation. - const handle = setImmediate(() => this.replayOpenDocuments('activate')); + this.activationReplayPending = true; + const handle = setImmediate(() => { + this.activationReplayPending = false; + this.replayOpenDocuments('activate'); + }); + this.subscriptions.push( + new Disposable(() => { + this.activationReplayPending = false; + clearImmediate(handle); + }), + ); + } + + /** + * Enables route-aware metadata tracking after a late experimental default activates PEP 723. + * Already-open documents are replayed so they gain routing without being reopened. + */ + public enableRouting(routingRegistry: InlineScriptRoutingRegistry): void { + if (this.disposed) { + throw new Error('Cannot enable routing on a disposed inline-script detector.'); + } + if (this.routingRegistry && this.routingRegistry !== routingRegistry) { + throw new Error('Inline-script routing is already enabled with a different registry.'); + } + if (this.routingRegistry === routingRegistry) { + return; + } + + this.routingRegistry = routingRegistry; + if (!this.activated) { + return; + } + this.registerRoutingEvents(); + if (this.activationReplayPending) { + return; + } + const handle = setImmediate(() => this.replayOpenDocuments('routing-enabled')); this.subscriptions.push(new Disposable(() => clearImmediate(handle))); } + private registerRoutingEvents(): void { + if (!this.routingRegistry || this.routingEventsRegistered) { + return; + } + this.routingEventsRegistered = true; + this.subscriptions.push( + onDidDeleteFiles((e) => e.files.forEach((uri) => this.clearRouteability(uri))), + onDidRenameFiles((e) => Promise.all(e.files.map((file) => this.handleRename(file.oldUri, file.newUri)))), + ); + } + /** * Walk every currently-open text document and run it through * `handleDocument` as if a synthetic `open` event had fired. Used * for the deferred activation catch-up. The per-URI dedup in * `handleDocument` keeps this safe to call repeatedly. */ - private replayOpenDocuments(source: 'activate'): void { + private replayOpenDocuments(source: 'activate' | 'routing-enabled'): void { const openDocs = getOpenTextDocuments().filter((d) => this.shouldTrackUri(d.uri)); const candidateDescription = this.routingRegistry ? 'candidate local .py' : 'candidate .py'; if (openDocs.length === 0) { @@ -229,7 +276,10 @@ export class InlineScriptLazyDetector implements Disposable { return; } if (this.routingRegistry) { - if (this.routingReadGenerations.get(this.getReadKey(uri)) === routingGeneration) { + if ( + routingGeneration !== undefined && + this.routingReadGenerations.get(this.getReadKey(uri)) === routingGeneration + ) { this.routingRegistry.setMetadata(uri, metadata); } if (!shouldEmitTelemetry || metadata === undefined) { diff --git a/src/features/inlineScript/setupEnvironment.ts b/src/features/inlineScript/setupEnvironment.ts index 58101bf5b..aff36cd36 100644 --- a/src/features/inlineScript/setupEnvironment.ts +++ b/src/features/inlineScript/setupEnvironment.ts @@ -1,8 +1,9 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { commands, Disposable, l10n, QuickPickItem, TextDocument, Uri, window } from 'vscode'; +import { Disposable, l10n, QuickPickItem, TextDocument, Uri, window } from 'vscode'; import { PythonEnvironment } from '../../api'; +import { registerCommand } from '../../common/command.api'; import { INLINE_SCRIPT_MANAGER_ID } from '../../common/constants'; import { getInlineScriptSetupProblem, @@ -411,19 +412,28 @@ async function filterInlineScriptFiles(files: readonly Uri[]): Promise { * Register the inline-script user-facing surfaces (the CodeLens, the quick fix, and their setup * commands). Only called when the PEP 723 inline-script preview is active. The single-file * setup command is invoked by both surfaces and stays out of `package.json`; the bulk command is - * palette-gated behind the activation-latched context key. + * palette-gated behind the one-way activation context key. */ export function registerInlineScriptUx(em: EnvironmentManagers, routing: InlineScriptRoutingRegistry): Disposable[] { - const codeLens = registerInlineScriptCodeLens(routing, SETUP_INLINE_SCRIPT_ENV_COMMAND); - return [ - codeLens.disposable, - registerInlineScriptSetupCodeAction(routing, SETUP_INLINE_SCRIPT_ENV_COMMAND), - commands.registerCommand( - SETUP_INLINE_SCRIPT_ENV_COMMAND, - setupInlineScriptEnvironmentHandler(em, routing), - ), - commands.registerCommand(SETUP_INLINE_SCRIPT_ENVS_COMMAND, () => - setUpInlineScriptEnvironmentsInWorkspace(em, routing), - ), - ]; + const disposables: Disposable[] = []; + try { + const codeLens = registerInlineScriptCodeLens(routing, SETUP_INLINE_SCRIPT_ENV_COMMAND); + disposables.push(codeLens.disposable); + disposables.push(registerInlineScriptSetupCodeAction(routing, SETUP_INLINE_SCRIPT_ENV_COMMAND)); + disposables.push( + registerCommand( + SETUP_INLINE_SCRIPT_ENV_COMMAND, + setupInlineScriptEnvironmentHandler(em, routing), + ), + ); + disposables.push( + registerCommand(SETUP_INLINE_SCRIPT_ENVS_COMMAND, () => + setUpInlineScriptEnvironmentsInWorkspace(em, routing), + ), + ); + return disposables; + } catch (error) { + disposables.reverse().forEach((disposable) => disposable.dispose()); + throw error; + } } diff --git a/src/managers/builtin/inlineScript/envManager.ts b/src/managers/builtin/inlineScript/envManager.ts index fd87cf043..85e763c9e 100644 --- a/src/managers/builtin/inlineScript/envManager.ts +++ b/src/managers/builtin/inlineScript/envManager.ts @@ -347,7 +347,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private readonly pendingLastUsedTouches = new Set>(); private readonly subscriptions: Disposable[] = []; private readonly associationStore: InlineScriptAssociationStore; - private readonly persistedAssociationsLoaded: Promise; + private persistedAssociationsLoaded: Promise = Promise.resolve(); private selectionQueue: Promise = Promise.resolve(); private cacheMaintenanceQueue: Promise = Promise.resolve(); private cacheMaintenanceBarrier: Deferred | undefined; @@ -356,6 +356,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private ttlEviction: Promise | undefined; private cacheMutationRevision = 0; private disposed = false; + private started = false; private readonly _onDidChangeEnvironments = new EventEmitter(); public readonly onDidChangeEnvironments: Event = @@ -412,8 +413,23 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { public readonly log: LogOutputChannel, workspaceState: Memento, private readonly routingRegistry: InlineScriptRoutingRegistry = new InlineScriptRoutingRegistry(), + deferStartup = false, ) { this.associationStore = new InlineScriptAssociationStore(workspaceState); + if (!deferStartup) { + this.start(); + } + } + + /** + * Starts persisted-association and file-event processing after feature registration commits. + * Calling this more than once has no effect. + */ + public start(): void { + if (this.started || this.disposed) { + return; + } + this.started = true; this.subscriptions.push( this.routingRegistry.onDidChangeMetadata((event) => { void this.handleSavedMetadataChange(event).catch((error) => { @@ -2745,6 +2761,9 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private loadPersistedAssociations(): Promise { return this.enqueuePersistence(async (state) => { const rawAssociations = await state.get(); + if (this.disposed) { + return; + } const parsed = this.parsePersistedAssociations(rawAssociations); this.applyPersistedAssociations(parsed?.records ?? {}); }); @@ -2755,7 +2774,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { return; } const metadata = await readInlineScriptMetadataFromFile(uri); - if (metadata && !this.routingRegistry.getMetadata(scriptPath)) { + if (!this.disposed && metadata && !this.routingRegistry.getMetadata(scriptPath)) { this.routingRegistry.setMetadata(uri, metadata); } } @@ -2768,9 +2787,15 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private initializePersistedAssociations(): Promise { return this.persistedAssociationsLoaded.then(async () => { + if (this.disposed) { + return; + } await Promise.all( [...this.fsPathToPersistedAssociation.keys()].map(async (scriptPath) => { await this.seedRoutingMetadataFromSavedFile(Uri.file(scriptPath), scriptPath); + if (this.disposed) { + return; + } const uri = this.routingRegistry.getUri(scriptPath); const metadata = this.routingRegistry.getMetadata(scriptPath); if (uri && metadata) { diff --git a/src/managers/builtin/inlineScript/main.ts b/src/managers/builtin/inlineScript/main.ts index 61293dbf9..842c6ec45 100644 --- a/src/managers/builtin/inlineScript/main.ts +++ b/src/managers/builtin/inlineScript/main.ts @@ -3,34 +3,29 @@ import { Disposable, LogOutputChannel, Memento, Uri } from 'vscode'; import { EnvironmentManager, PythonEnvironmentApi } from '../../../api'; -import { traceInfo, traceVerbose } from '../../../common/logging'; -import { InlineScriptFeatureActivation } from '../../../features/inlineScript/activation'; +import { InlineScriptRoutingRegistry } from '../../../common/inlineScript/routingRegistry'; +import { traceInfo } from '../../../common/logging'; import { getPythonApi } from '../../../extensionApi'; import { NativePythonFinder } from '../../common/nativePythonFinder'; import { InlineScriptEnvManager } from './envManager'; +export interface PreparedInlineScriptFeatures { + readonly manager: InlineScriptEnvManager; + registerManager(): Disposable; +} + /** - * Register the inline-script env manager when the activation-latched - * `python-envs.inlineScripts.enabled` preview setting is true. + * Prepares an inert inline-script manager for publication after the rest of the feature is ready. */ -export async function registerInlineScriptFeatures( +export async function prepareInlineScriptFeatures( nativeFinder: NativePythonFinder, disposables: Disposable[], log: LogOutputChannel, baseManager: EnvironmentManager, globalStorageUri: Uri, - activation: InlineScriptFeatureActivation, + routingRegistry: InlineScriptRoutingRegistry, workspaceState: Memento, -): Promise { - if (!activation.enabled) { - traceVerbose('Inline-script env manager: skipping registration (preview setting is off)'); - return; - } - const { routingRegistry } = activation; - if (!routingRegistry) { - throw new Error('Inline-script env manager requires a routing registry when the feature flag is on'); - } - +): Promise { const api: PythonEnvironmentApi = await getPythonApi(); const mgr = new InlineScriptEnvManager( nativeFinder, @@ -40,8 +35,23 @@ export async function registerInlineScriptFeatures( log, workspaceState, routingRegistry, + true, ); - disposables.push(mgr, api.registerEnvironmentManager(mgr)); - setImmediate(() => mgr.startActivationDiscovery()); - traceInfo('Inline-script env manager: registered (preview setting is on)'); + disposables.push(mgr); + return { + manager: mgr, + registerManager: () => { + const registration = api.registerEnvironmentManager(mgr); + traceInfo('Inline-script env manager: registered'); + return registration; + }, + }; +} + +/** + * Starts restored-association discovery after the complete feature registration commits. + */ +export function scheduleInlineScriptActivationDiscovery(manager: InlineScriptEnvManager): Disposable { + const handle = setImmediate(() => manager.startActivationDiscovery()); + return new Disposable(() => clearImmediate(handle)); } diff --git a/src/test/features/common/managerReady.unit.test.ts b/src/test/features/common/managerReady.unit.test.ts index efa7091b5..79613c523 100644 --- a/src/test/features/common/managerReady.unit.test.ts +++ b/src/test/features/common/managerReady.unit.test.ts @@ -261,6 +261,56 @@ suite('ManagerReady - race condition handling', () => { assert.ok(!showErrorStub.called, 'should not prompt when manager already registered'); }); + test('an unregistered manager no longer satisfies future waits', async () => { + const manager = { id: VENV_MANAGER_ID } as unknown as InternalEnvironmentManager; + envManagerEmitter.fire({ kind: 'registered', manager }); + envManagerEmitter.fire({ kind: 'unregistered', manager }); + + let settled = false; + const waitPromise = waitForEnvManagerId([VENV_MANAGER_ID]).then(() => { + settled = true; + }); + await clock.tickAsync(0); + assert.strictEqual(settled, false, 'the removed manager should not remain ready'); + + clock.tick(MANAGER_READY_TIMEOUT_MS); + await clock.tickAsync(0); + await waitPromise; + assert.strictEqual(settled, true, 'the normal readiness timeout should still complete the wait'); + }); + + test('a stale unregister event does not clear a replacement manager', async () => { + const original = { id: VENV_MANAGER_ID } as unknown as InternalEnvironmentManager; + const replacement = { id: VENV_MANAGER_ID } as unknown as InternalEnvironmentManager; + envManagerEmitter.fire({ kind: 'registered', manager: original }); + envManagerEmitter.fire({ kind: 'registered', manager: replacement }); + envManagerEmitter.fire({ kind: 'unregistered', manager: original }); + + await waitForEnvManagerId([VENV_MANAGER_ID]); + + const showErrorStub = windowApis.showErrorMessage as sinon.SinonStub; + assert.ok(!showErrorStub.called, 'the replacement manager should remain ready'); + }); + + test('disposing readiness resolves pending waits and cancels timeout effects', async () => { + let settled = false; + const waitPromise = waitForEnvManagerId([VENV_MANAGER_ID]).then(() => { + settled = true; + }); + await clock.tickAsync(0); + + disposables.forEach((disposable) => disposable.dispose()); + disposables = []; + await clock.tickAsync(0); + assert.strictEqual(settled, true, 'dispose should settle pending manager waits'); + + clock.tick(MANAGER_READY_TIMEOUT_MS); + await clock.tickAsync(0); + await waitPromise; + assert.strictEqual((telemetrySender.sendTelemetryEvent as sinon.SinonStub).callCount, 0); + assert.strictEqual((windowApis.showErrorMessage as sinon.SinonStub).callCount, 0); + }); + test('pkg manager wait resolves when registration event fires', async () => { const waitPromise = waitForPkgManagerId([DEFAULT_PACKAGE_MANAGER_ID]); @@ -275,4 +325,17 @@ suite('ManagerReady - race condition handling', () => { const showErrorStub = windowApis.showErrorMessage as sinon.SinonStub; assert.ok(!showErrorStub.called, 'should not prompt when pkg manager registered'); }); + + test('a stale package unregister event does not clear a replacement manager', async () => { + const original = { id: DEFAULT_PACKAGE_MANAGER_ID } as unknown as InternalPackageManager; + const replacement = { id: DEFAULT_PACKAGE_MANAGER_ID } as unknown as InternalPackageManager; + pkgManagerEmitter.fire({ kind: 'registered', manager: original }); + pkgManagerEmitter.fire({ kind: 'registered', manager: replacement }); + pkgManagerEmitter.fire({ kind: 'unregistered', manager: original }); + + await waitForPkgManagerId([DEFAULT_PACKAGE_MANAGER_ID]); + + const showErrorStub = windowApis.showErrorMessage as sinon.SinonStub; + assert.ok(!showErrorStub.called, 'the replacement package manager should remain ready'); + }); }); diff --git a/src/test/features/envManagers.inlineRoutingAttachment.unit.test.ts b/src/test/features/envManagers.inlineRoutingAttachment.unit.test.ts new file mode 100644 index 000000000..e55ed1918 --- /dev/null +++ b/src/test/features/envManagers.inlineRoutingAttachment.unit.test.ts @@ -0,0 +1,29 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import assert from 'assert'; +import { InlineScriptRoutingRegistry } from '../../common/inlineScript/routingRegistry'; +import { PythonEnvironmentManagers } from '../../features/envManagers'; +import { PythonProjectManager } from '../../features/projectManager'; + +suite('PythonEnvironmentManagers inline routing attachment', () => { + test('attaches once and permits replacement only after disposal', () => { + const projectManager = { + getProjects: () => [], + } as unknown as PythonProjectManager; + const managers = new PythonEnvironmentManagers(projectManager); + const firstRouting = new InlineScriptRoutingRegistry(); + const secondRouting = new InlineScriptRoutingRegistry(); + + const registration = managers.attachInlineScriptRouting(firstRouting); + assert.throws(() => managers.attachInlineScriptRouting(secondRouting), /already attached/i); + + registration.dispose(); + const replacement = managers.attachInlineScriptRouting(secondRouting); + + replacement.dispose(); + firstRouting.dispose(); + secondRouting.dispose(); + managers.dispose(); + }); +}); diff --git a/src/test/features/inlineScript/activation.unit.test.ts b/src/test/features/inlineScript/activation.unit.test.ts new file mode 100644 index 000000000..b7adcb480 --- /dev/null +++ b/src/test/features/inlineScript/activation.unit.test.ts @@ -0,0 +1,125 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import assert from 'assert'; +import * as sinon from 'sinon'; +import { ConfigurationChangeEvent, Disposable } from 'vscode'; +import * as workspaceApis from '../../../common/workspace.apis'; +import { createInlineScriptFeatureActivation } from '../../../features/inlineScript/activation'; +import * as helpers from '../../../helpers'; + +suite('InlineScriptFeatureActivation', () => { + let getUserConfiguredSetting: sinon.SinonStub; + let isInlineScriptsFeatureEnabled: sinon.SinonStub; + let configurationListener: ((event: ConfigurationChangeEvent) => void) | undefined; + let configurationSubscriptionDispose: sinon.SinonSpy; + + setup(() => { + getUserConfiguredSetting = sinon.stub(helpers, 'getUserConfiguredSetting'); + isInlineScriptsFeatureEnabled = sinon.stub(helpers, 'isInlineScriptsFeatureEnabled'); + configurationSubscriptionDispose = sinon.spy(); + sinon.stub(workspaceApis, 'onDidChangeConfiguration').callsFake((listener) => { + configurationListener = listener; + return new Disposable(configurationSubscriptionDispose); + }); + }); + + teardown(() => { + sinon.restore(); + }); + + function configurationChange(affected: boolean): ConfigurationChangeEvent { + return { + affectsConfiguration: sinon.stub().returns(affected), + }; + } + + test('an explicit true value requests activation immediately without watching defaults', () => { + getUserConfiguredSetting.returns(true); + + const activation = createInlineScriptFeatureActivation(); + + assert.strictEqual(activation.activationRequested, true); + assert.strictEqual(configurationListener, undefined); + assert.strictEqual(isInlineScriptsFeatureEnabled.callCount, 0); + activation.dispose(); + }); + + test('an explicit false value stays disabled without watching defaults', () => { + getUserConfiguredSetting.returns(false); + + const activation = createInlineScriptFeatureActivation(); + + assert.strictEqual(activation.activationRequested, false); + assert.strictEqual(configurationListener, undefined); + assert.strictEqual(isInlineScriptsFeatureEnabled.callCount, 0); + activation.dispose(); + }); + + test('an effective true default requests activation after subscribing', () => { + getUserConfiguredSetting.returns(undefined); + isInlineScriptsFeatureEnabled.returns(true); + + const activation = createInlineScriptFeatureActivation(); + + assert.strictEqual(activation.activationRequested, true); + assert.ok(configurationListener, 'the default listener should be registered before the effective read'); + sinon.assert.calledOnce(configurationSubscriptionDispose); + activation.dispose(); + }); + + test('a late experimental default requests activation exactly once', () => { + getUserConfiguredSetting.returns(undefined); + isInlineScriptsFeatureEnabled.onFirstCall().returns(false).onSecondCall().returns(true); + const activation = createInlineScriptFeatureActivation(); + let requests = 0; + const requestSubscription = activation.onDidRequestActivation(() => { + requests += 1; + }); + + assert.strictEqual(activation.activationRequested, false); + assert.ok(configurationListener); + configurationListener!(configurationChange(true)); + configurationListener!(configurationChange(true)); + + assert.strictEqual(activation.activationRequested, true); + assert.strictEqual(requests, 1); + sinon.assert.calledOnce(configurationSubscriptionDispose); + requestSubscription.dispose(); + activation.dispose(); + }); + + test('a user setting change remains reload-only and stops watching defaults', () => { + getUserConfiguredSetting.onFirstCall().returns(undefined).onSecondCall().returns(undefined).onThirdCall().returns(true); + isInlineScriptsFeatureEnabled.returns(false); + const activation = createInlineScriptFeatureActivation(); + let requested = false; + const requestSubscription = activation.onDidRequestActivation(() => { + requested = true; + }); + + assert.ok(configurationListener); + configurationListener!(configurationChange(true)); + + assert.strictEqual(requested, false); + assert.strictEqual(activation.activationRequested, false); + assert.strictEqual(isInlineScriptsFeatureEnabled.callCount, 1); + sinon.assert.calledOnce(configurationSubscriptionDispose); + requestSubscription.dispose(); + activation.dispose(); + }); + + test('unrelated configuration changes do not re-read the setting', () => { + getUserConfiguredSetting.returns(undefined); + isInlineScriptsFeatureEnabled.returns(false); + const activation = createInlineScriptFeatureActivation(); + + assert.ok(configurationListener); + configurationListener!(configurationChange(false)); + + assert.strictEqual(getUserConfiguredSetting.callCount, 2); + assert.strictEqual(isInlineScriptsFeatureEnabled.callCount, 1); + assert.strictEqual(activation.activationRequested, false); + activation.dispose(); + }); +}); diff --git a/src/test/features/inlineScript/lazyDetector.unit.test.ts b/src/test/features/inlineScript/lazyDetector.unit.test.ts index fa524776e..72be15f31 100644 --- a/src/test/features/inlineScript/lazyDetector.unit.test.ts +++ b/src/test/features/inlineScript/lazyDetector.unit.test.ts @@ -205,6 +205,47 @@ suite('InlineScriptLazyDetector', () => { detector.dispose(); }); + test('enableRouting() subscribes to file events and replays already-open documents', async () => { + const uri = Uri.file(path.resolve('/ws/late-enabled.py')); + readMetadataStub.resolves(VALID_METADATA); + const detector = createDetectorWithoutRouting(); + getOpenTextDocumentsStub.returns([makeDoc(uri)]); + + detector.enableRouting(routingRegistry); + await flushImmediate(); + await flushImmediate(); + + assert.ok(onDidDeleteStub.calledOnce, 'late routing should subscribe to onDidDeleteFiles'); + assert.ok(onDidRenameStub.calledOnce, 'late routing should subscribe to onDidRenameFiles'); + assert.ok(readMetadataStub.calledOnceWithExactly(uri), 'late routing should replay the open script'); + assert.deepStrictEqual(routingRegistry.getMetadata(uri)?.dependencies, VALID_METADATA.dependencies); + detector.enableRouting(routingRegistry); + assert.strictEqual(onDidDeleteStub.callCount, 1, 'repeated enablement should remain idempotent'); + assert.strictEqual(onDidRenameStub.callCount, 1, 'repeated enablement should remain idempotent'); + detector.dispose(); + }); + + test('a telemetry-only read already in flight cannot publish routing metadata after enablement', async () => { + const uri = Uri.file(path.resolve('/ws/in-flight-before-routing.py')); + const metadata = createDeferred(); + readMetadataStub.returns(metadata.promise); + const detector = createDetectorWithoutRouting(); + await flushImmediate(); + + const open = fireOpen(uri); + await Promise.resolve(); + detector.enableRouting(routingRegistry); + metadata.resolve(VALID_METADATA); + await open; + + assert.strictEqual( + routingRegistry.getMetadata(uri), + undefined, + 'the route-aware replay, not an earlier telemetry-only read, must seed routing', + ); + detector.dispose(); + }); + test('skips non-file URI schemes', async () => { const detector = createDetector(); await fireOpen(Uri.parse('untitled:foo.py')); diff --git a/src/test/features/inlineScript/setupEnvironment.registration.unit.test.ts b/src/test/features/inlineScript/setupEnvironment.registration.unit.test.ts new file mode 100644 index 000000000..47912a954 --- /dev/null +++ b/src/test/features/inlineScript/setupEnvironment.registration.unit.test.ts @@ -0,0 +1,45 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import assert from 'assert'; +import * as sinon from 'sinon'; +import { Disposable } from 'vscode'; +import * as commandApis from '../../../common/command.api'; +import { InlineScriptRoutingRegistry } from '../../../common/inlineScript/routingRegistry'; +import { EnvironmentManagers } from '../../../features/envManagers'; +import * as codeLensModule from '../../../features/inlineScript/codeLens'; +import { registerInlineScriptUx } from '../../../features/inlineScript/setupEnvironment'; +import * as setupCodeActionModule from '../../../features/inlineScript/setupCodeAction'; + +suite('registerInlineScriptUx', () => { + teardown(() => { + sinon.restore(); + }); + + test('rolls back earlier registrations when a later command fails', () => { + const codeLensDispose = sinon.spy(); + const codeActionDispose = sinon.spy(); + const firstCommandDispose = sinon.spy(); + sinon.stub(codeLensModule, 'registerInlineScriptCodeLens').returns({ + provider: {} as never, + disposable: new Disposable(codeLensDispose), + }); + sinon + .stub(setupCodeActionModule, 'registerInlineScriptSetupCodeAction') + .returns(new Disposable(codeActionDispose)); + sinon + .stub(commandApis, 'registerCommand') + .onFirstCall() + .returns(new Disposable(firstCommandDispose)) + .onSecondCall() + .throws(new Error('duplicate command')); + + assert.throws( + () => registerInlineScriptUx({} as EnvironmentManagers, new InlineScriptRoutingRegistry()), + /duplicate command/, + ); + sinon.assert.calledOnce(codeLensDispose); + sinon.assert.calledOnce(codeActionDispose); + sinon.assert.calledOnce(firstCommandDispose); + }); +}); diff --git a/src/test/managers/builtin/inlineScript/main.unit.test.ts b/src/test/managers/builtin/inlineScript/main.unit.test.ts index 07f03d75f..e1c8f7455 100644 --- a/src/test/managers/builtin/inlineScript/main.unit.test.ts +++ b/src/test/managers/builtin/inlineScript/main.unit.test.ts @@ -5,16 +5,14 @@ import assert from 'assert'; import * as sinon from 'sinon'; import { Disposable, LogOutputChannel, Memento, Uri } from 'vscode'; import { EnvironmentManager, PythonEnvironmentApi } from '../../../../api'; -import * as cacheLayout from '../../../../common/inlineScript/cacheLayout'; import { InlineScriptRoutingRegistry } from '../../../../common/inlineScript/routingRegistry'; -import * as persistentState from '../../../../common/persistentState'; import * as workspaceApis from '../../../../common/workspace.apis'; -import { latchInlineScriptFeatureActivation } from '../../../../features/inlineScript/activation'; -import { InlineScriptLazyDetector } from '../../../../features/inlineScript/lazyDetector'; import * as pythonApi from '../../../../extensionApi'; -import * as helpers from '../../../../helpers'; import { InlineScriptEnvManager } from '../../../../managers/builtin/inlineScript/envManager'; -import { registerInlineScriptFeatures } from '../../../../managers/builtin/inlineScript/main'; +import { + prepareInlineScriptFeatures, + scheduleInlineScriptActivationDiscovery, +} from '../../../../managers/builtin/inlineScript/main'; import { NativePythonFinder } from '../../../../managers/common/nativePythonFinder'; function makeFakeLog(): LogOutputChannel { @@ -38,8 +36,7 @@ function nextTurn(): Promise { return new Promise((resolve) => setImmediate(resolve)); } -suite('registerInlineScriptFeatures (feature-flag gate)', () => { - let isEnabledStub: sinon.SinonStub; +suite('prepareInlineScriptFeatures', () => { let getPythonApiStub: sinon.SinonStub; let registerEnvironmentManagerStub: sinon.SinonStub; let startActivationDiscoveryStub: sinon.SinonStub; @@ -56,7 +53,6 @@ suite('registerInlineScriptFeatures (feature-flag gate)', () => { } as unknown as Memento; setup(() => { - isEnabledStub = sinon.stub(helpers, 'isInlineScriptsFeatureEnabled'); registerEnvironmentManagerStub = sinon.stub<[unknown], Disposable>().returns({ dispose: () => undefined }); startActivationDiscoveryStub = sinon.stub(InlineScriptEnvManager.prototype, 'startActivationDiscovery'); onDidDeleteFilesStub = sinon @@ -75,188 +71,85 @@ suite('registerInlineScriptFeatures (feature-flag gate)', () => { sinon.restore(); }); - test('when the feature flag is FALSE: does not register, does not even fetch the API', async () => { + test('prepares an inert manager and publishes it only at the commit point', async () => { const disposables: Disposable[] = []; + const getPersistedState = sinon.spy(workspaceMemento, 'get'); - await registerInlineScriptFeatures( + const prepared = await prepareInlineScriptFeatures( nativeFinder, disposables, makeFakeLog(), baseManager, globalStorageUri, - { enabled: false, routingRegistry: undefined }, + routingRegistry, workspaceMemento, ); - assert.strictEqual(disposables.length, 0, 'no disposables should be added when flag is off'); - assert.strictEqual(getPythonApiStub.called, false, 'should not even call getPythonApi when gated off'); - assert.strictEqual(registerEnvironmentManagerStub.called, false); - }); - - test('when the feature flag is TRUE without a routing registry: fails before touching the API', async () => { - const disposables: Disposable[] = []; + assert.strictEqual(getPythonApiStub.callCount, 1); + assert.strictEqual(registerEnvironmentManagerStub.callCount, 0); + assert.strictEqual(onDidDeleteFilesStub.callCount, 0); + assert.strictEqual(onDidRenameFilesStub.callCount, 0); + assert.strictEqual(getPersistedState.callCount, 0); + assert.strictEqual(disposables.length, 1); + assert.strictEqual(disposables[0], prepared.manager); - await assert.rejects( - registerInlineScriptFeatures( - nativeFinder, - disposables, - makeFakeLog(), - baseManager, - globalStorageUri, - { enabled: true, routingRegistry: undefined }, - workspaceMemento, - ), - /routing registry/i, - ); + prepared.manager.start(); + await nextTurn(); + const registration = prepared.registerManager(); + disposables.push(registration); - assert.strictEqual(disposables.length, 0, 'no disposables should be added when the registry is missing'); - assert.strictEqual(getPythonApiStub.called, false, 'should fail before getPythonApi when the registry is missing'); - assert.strictEqual(registerEnvironmentManagerStub.called, false); + assert.strictEqual(registerEnvironmentManagerStub.callCount, 1); + assert.strictEqual(registerEnvironmentManagerStub.firstCall.args[0], prepared.manager); + assert.strictEqual(onDidDeleteFilesStub.callCount, 1); + assert.strictEqual(onDidRenameFilesStub.callCount, 1); + assert.ok(getPersistedState.calledOnce, 'persisted state should load only after start'); + assert.strictEqual(typeof prepared.manager.create, 'function'); + disposables.forEach((disposable) => disposable.dispose()); }); - test('when the feature flag is TRUE: registers the manager and pushes the disposable', async () => { + test('keeps the manager tracked for rollback when API publication fails', async () => { + registerEnvironmentManagerStub.throws(new Error('registration failed')); const disposables: Disposable[] = []; - - await registerInlineScriptFeatures( + const prepared = await prepareInlineScriptFeatures( nativeFinder, disposables, makeFakeLog(), baseManager, globalStorageUri, - { enabled: true, routingRegistry }, + routingRegistry, workspaceMemento, ); - assert.strictEqual(getPythonApiStub.callCount, 1); - assert.strictEqual(registerEnvironmentManagerStub.callCount, 1); - assert.strictEqual(disposables.length, 2, 'expected manager + registration disposable'); - const manager = registerEnvironmentManagerStub.firstCall.args[0]; - assert.ok(disposables.includes(manager), 'manager itself should be disposed'); - assert.ok( - disposables.includes(registerEnvironmentManagerStub.firstCall.returnValue), - 'registration disposable should be disposed', - ); - assert.strictEqual(typeof manager.create, 'function'); - await nextTurn(); + assert.throws(() => prepared.registerManager(), /registration failed/); + assert.strictEqual(disposables.length, 1); + assert.strictEqual(disposables[0], prepared.manager); disposables.forEach((disposable) => disposable.dispose()); }); - test('when the feature flag is TRUE: defers activation-time discovery to the next turn', async () => { + test('schedules activation-time discovery only when requested by the caller', async () => { const disposables: Disposable[] = []; - await registerInlineScriptFeatures( + const prepared = await prepareInlineScriptFeatures( nativeFinder, disposables, makeFakeLog(), baseManager, globalStorageUri, - { enabled: true, routingRegistry }, + routingRegistry, workspaceMemento, ); assert.strictEqual( startActivationDiscoveryStub.callCount, 0, - 'activation should not synchronously start bootstrap discovery', + 'registration should not start discovery before the full feature transaction commits', ); + const scheduled = scheduleInlineScriptActivationDiscovery(prepared.manager); await nextTurn(); sinon.assert.calledOnceWithExactly(startActivationDiscoveryStub); + scheduled.dispose(); disposables.forEach((disposable) => disposable.dispose()); }); - - test('latches FALSE through deferred registration even if the live setting flips TRUE later', async () => { - isEnabledStub.onFirstCall().returns(false); - isEnabledStub.onSecondCall().returns(true); - const activation = latchInlineScriptFeatureActivation(); - const disposables: Disposable[] = []; - - await (activation.enabled - ? registerInlineScriptFeatures( - nativeFinder, - disposables, - makeFakeLog(), - baseManager, - globalStorageUri, - activation, - workspaceMemento, - ) - : Promise.resolve()); - - assert.strictEqual(activation.enabled, false); - assert.strictEqual(activation.routingRegistry, undefined); - assert.strictEqual(isEnabledStub.callCount, 1, 'activation should read the setting only once'); - assert.strictEqual(disposables.length, 0, 'disabled activation should not add disposables later'); - assert.strictEqual(getPythonApiStub.called, false, 'disabled activation should never touch the API later'); - assert.strictEqual(registerEnvironmentManagerStub.called, false); - }); - - test('absent flag performs no inline registration, persistence, cache, or routing-listener work', async () => { - isEnabledStub.returns(false); - const persistentStateStub = sinon.stub(persistentState, 'getWorkspacePersistentState'); - const inspectMetaJsonStub = sinon.stub(cacheLayout, 'inspectMetaJson'); - const writeMetaJsonStub = sinon.stub(cacheLayout, 'writeMetaJson'); - sinon.stub(workspaceApis, 'onDidOpenTextDocument').returns(new Disposable(() => undefined)); - sinon.stub(workspaceApis, 'onDidSaveTextDocument').returns(new Disposable(() => undefined)); - sinon.stub(workspaceApis, 'onDidChangeTextDocument').returns(new Disposable(() => undefined)); - sinon.stub(workspaceApis, 'getOpenTextDocuments').returns([]); - - const activation = latchInlineScriptFeatureActivation(); - const detector = new InlineScriptLazyDetector(activation.routingRegistry); - detector.activate(); - const disposables: Disposable[] = []; - await (activation.enabled - ? registerInlineScriptFeatures( - nativeFinder, - disposables, - makeFakeLog(), - baseManager, - globalStorageUri, - activation, - workspaceMemento, - ) - : Promise.resolve()); - await nextTurn(); - - assert.strictEqual(activation.enabled, false); - assert.strictEqual(activation.routingRegistry, undefined); - assert.strictEqual(isEnabledStub.callCount, 1); - assert.strictEqual(getPythonApiStub.callCount, 0); - assert.strictEqual(registerEnvironmentManagerStub.callCount, 0); - assert.strictEqual(startActivationDiscoveryStub.callCount, 0); - assert.strictEqual(persistentStateStub.callCount, 0); - assert.strictEqual(inspectMetaJsonStub.callCount, 0); - assert.strictEqual(writeMetaJsonStub.callCount, 0); - assert.strictEqual(onDidDeleteFilesStub.callCount, 0); - assert.strictEqual(onDidRenameFilesStub.callCount, 0); - assert.strictEqual(disposables.length, 0); - detector.dispose(); - }); - - test('latches TRUE through deferred registration even if the live setting flips FALSE later', async () => { - isEnabledStub.onFirstCall().returns(true); - isEnabledStub.onSecondCall().returns(false); - const activation = latchInlineScriptFeatureActivation(); - const disposables: Disposable[] = []; - - await (activation.enabled - ? registerInlineScriptFeatures( - nativeFinder, - disposables, - makeFakeLog(), - baseManager, - globalStorageUri, - activation, - workspaceMemento, - ) - : Promise.resolve()); - - assert.strictEqual(activation.enabled, true); - assert.ok(activation.routingRegistry, 'enabled activation should latch a routing registry'); - assert.strictEqual(isEnabledStub.callCount, 1, 'deferred registration should not reread the setting'); - assert.strictEqual(getPythonApiStub.callCount, 1); - assert.strictEqual(registerEnvironmentManagerStub.callCount, 1); - assert.strictEqual(disposables.length, 2, 'enabled activation should still register later'); - }); }); diff --git a/src/test/smoke/inlineScriptsExperimentEnabled.smoke.test.ts b/src/test/smoke/inlineScriptsExperimentEnabled.smoke.test.ts new file mode 100644 index 000000000..30e2df45c --- /dev/null +++ b/src/test/smoke/inlineScriptsExperimentEnabled.smoke.test.ts @@ -0,0 +1,105 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as assert from 'assert'; +import * as fs from 'fs/promises'; +import * as os from 'os'; +import * as path from 'path'; +import * as vscode from 'vscode'; +import { PythonEnvironmentApi } from '../../api'; +import { ENVS_EXTENSION_ID } from '../constants'; +import { waitForCondition } from '../testUtils'; + +suite('Smoke: Delayed Inline Scripts Experiment Default', function () { + this.timeout(120_000); + + test('a late experimental default activates all inline-script registrations', async function () { + if (process.env.VSC_PYTHON_INLINE_SCRIPTS_EXPERIMENT_SMOKE_TEST !== '1') { + this.skip(); + } + + const config = vscode.workspace.getConfiguration('python-envs'); + const inspectedBeforeActivation = config.inspect('inlineScripts.enabled'); + assert.strictEqual(inspectedBeforeActivation?.globalValue, undefined); + assert.strictEqual(inspectedBeforeActivation?.workspaceValue, undefined); + assert.strictEqual(inspectedBeforeActivation?.workspaceFolderValue, undefined); + assert.strictEqual( + config.get('inlineScripts.enabled'), + false, + 'the delayed experiment override should still be pending before extension activation', + ); + + const extension = vscode.extensions.getExtension(ENVS_EXTENSION_ID); + assert.ok(extension, `Extension ${ENVS_EXTENSION_ID} not found`); + if (!extension.isActive) { + await extension.activate(); + } + assert.strictEqual(extension.isActive, true); + assert.strictEqual( + vscode.workspace.getConfiguration('python-envs').get('inlineScripts.enabled'), + false, + 'the extension must be active before the delayed experimental default is applied', + ); + + const commandsImmediatelyAfterActivation = await vscode.commands.getCommands(true); + assert.ok( + !commandsImmediatelyAfterActivation.includes('python-envs.setupInlineScriptEnv'), + 'the hidden setup command should not register before the experimental default arrives', + ); + + await waitForCondition( + () => vscode.workspace.getConfiguration('python-envs').get('inlineScripts.enabled') === true, + 45_000, + 'VS Code did not apply the delayed inline-script experimental default', + ); + await waitForCondition( + async () => { + const commands = await vscode.commands.getCommands(true); + return ( + commands.includes('python-envs.setupInlineScriptEnv') && + commands.includes('python-envs.clearScriptEnvCache') && + commands.includes('python-envs.setupInlineScriptEnvs') + ); + }, + 45_000, + 'the inline-script feature did not register after the experimental default became true', + ); + + const inspectedAfterActivation = vscode.workspace + .getConfiguration('python-envs') + .inspect('inlineScripts.enabled'); + assert.strictEqual(inspectedAfterActivation?.globalValue, undefined); + assert.strictEqual(inspectedAfterActivation?.defaultValue, true); + + const tempDirectory = await fs.mkdtemp(path.join(os.tmpdir(), 'pep723-exp-smoke-')); + const scriptUri = vscode.Uri.file(path.join(tempDirectory, 'script.py')); + try { + await fs.writeFile( + scriptUri.fsPath, + '# /// script\n# requires-python = ">=3.10"\n# dependencies = []\n# ///\n\nprint("hello")\n', + ); + const document = await vscode.workspace.openTextDocument(scriptUri); + await vscode.window.showTextDocument(document); + await waitForCondition( + async () => { + const lenses = + (await vscode.commands.executeCommand( + 'vscode.executeCodeLensProvider', + scriptUri, + )) ?? []; + return lenses.some((lens) => lens.command?.command === 'python-envs.setupInlineScriptEnv'); + }, + 10_000, + 'the delayed activation did not surface the inline-script setup CodeLens', + ); + } finally { + await vscode.commands.executeCommand('workbench.action.revertAndCloseActiveEditor'); + await fs.rm(tempDirectory, { + recursive: true, + force: true, + maxRetries: 5, + retryDelay: 100, + }); + } + }); +});