diff --git a/src/vs/workbench/api/browser/positron/mainThreadLanguageRuntime.ts b/src/vs/workbench/api/browser/positron/mainThreadLanguageRuntime.ts index a90926338cf..fb6abe518c1 100644 --- a/src/vs/workbench/api/browser/positron/mainThreadLanguageRuntime.ts +++ b/src/vs/workbench/api/browser/positron/mainThreadLanguageRuntime.ts @@ -1177,11 +1177,15 @@ export class MainThreadLanguageRuntime this._proxy = extHostContext.getProxy(ExtHostPositronContext.ExtHostLanguageRuntime); this._id = MainThreadLanguageRuntime.MAX_ID++; - this._languageRuntimeService.onDidChangeRuntimeStartupPhase((phase) => { - if (phase === RuntimeStartupPhase.Discovering) { - this._proxy.$discoverLanguageRuntimes(); - } - }); + this._runtimeStartupService.registerMainThreadLanguageRuntime(this._id); + + this._disposables.add( + this._languageRuntimeService.onDidChangeRuntimeStartupPhase((phase) => { + if (phase === RuntimeStartupPhase.Discovering) { + this._proxy.$discoverLanguageRuntimes(); + } + }) + ); this._disposables.add(this._runtimeSessionService.registerSessionManager(this)); } @@ -1259,7 +1263,7 @@ export class MainThreadLanguageRuntime // Signals that language runtime discovery is complete. $completeLanguageRuntimeDiscovery(): void { - this._runtimeStartupService.completeDiscovery(); + this._runtimeStartupService.completeDiscovery(this._id); } $unregisterLanguageRuntime(handle: number): void { @@ -1289,6 +1293,8 @@ export class MainThreadLanguageRuntime session.emitExit(exit); } }); + + this._runtimeStartupService.unregisterMainThreadLanguageRuntime(this._id); this._disposables.dispose(); } diff --git a/src/vs/workbench/services/runtimeSession/common/runtimeSessionService.ts b/src/vs/workbench/services/runtimeSession/common/runtimeSessionService.ts index e4fc6edab8c..f713fff964b 100644 --- a/src/vs/workbench/services/runtimeSession/common/runtimeSessionService.ts +++ b/src/vs/workbench/services/runtimeSession/common/runtimeSessionService.ts @@ -376,6 +376,16 @@ export interface IRuntimeSessionService { runtimeMetadata: ILanguageRuntimeMetadata, sessionId: string): Promise; + /** + * Validates a persisted runtime session before reconnecting to it. + * + * @param runtimeMetadata The metadata of the runtime. + * @param sesionId The ID of the session to validate. + */ + validateRuntimeSession( + runtimeMetadata: ILanguageRuntimeMetadata, + sessionId: string): Promise; + /** * Restores (reconnects to) a runtime session that was previously started. * diff --git a/src/vs/workbench/services/runtimeStartup/common/runtimeStartup.ts b/src/vs/workbench/services/runtimeStartup/common/runtimeStartup.ts index b2d76541e40..b026f0616dc 100644 --- a/src/vs/workbench/services/runtimeStartup/common/runtimeStartup.ts +++ b/src/vs/workbench/services/runtimeStartup/common/runtimeStartup.ts @@ -97,7 +97,13 @@ export class RuntimeStartupService extends Disposable implements IRuntimeStartup // (metadata.languageId) of the runtime. private readonly _mostRecentlyStartedRuntimesByLanguageId = new Map(); - // The current startup phase an observeable value. + // A map of each extension host and its runtime discovery completion state. + // This is keyed by the the extension host's mainThreadLanguageRuntime's id + // This map is used to determine if runtime discovery has been completed + // across all extension hosts. + private readonly _discoveryCompleteByExtHostId = new Map(); + + // The current startup phase private _startupPhase: RuntimeStartupPhase; // Whether we are shutting down @@ -362,11 +368,66 @@ export class RuntimeStartupService extends Disposable implements IRuntimeStartup } /** - * Completes the language runtime discovery phase. If no runtimes were - * started or will be started, automatically start one. + /** + * Signals that the runtime discovery phase is completed only after all + * extension hosts have completed runtime discovery. + * + * If no runtimes were started or will be started, automatically start one. */ - completeDiscovery(): void { - this.setStartupPhase(RuntimeStartupPhase.Complete); + public completeDiscovery(id: number): void { + // Update the extension host's runtime discovery state to 'Complete' + this._discoveryCompleteByExtHostId.set(id, true); + this._logService.debug(`[Runtime startup] Discovery completed for extension host with id: ${id}.`); + + // Determine if all extension hosts have completed discovery + let discoveryCompletedByAllExtensionHosts = true; + for (const disoveryCompleted of this._discoveryCompleteByExtHostId.values()) { + if (!disoveryCompleted) { + discoveryCompletedByAllExtensionHosts = false; + break; + } + } + + // The 'Discovery' phase is considered complete only after all extension hosts + // have signaled they have completed their own runtime discovery + if (discoveryCompletedByAllExtensionHosts) { + this.setStartupPhase(RuntimeStartupPhase.Complete); + // Reset the discovery state for each ext host so we are ready + // for possible re-discovery of runtimes + this._discoveryCompleteByExtHostId.forEach((_, extHostId, m) => { + m.set(extHostId, false); + }); + } + } + + /** + * Used to register an instance of a MainThreadLanguageRuntime. + * + * This is required because there can be multiple extension hosts + * and the startup service needs to know of all of them to track + * the startup phase across all extension hosts. + * + * @param id The id of the MainThreadLanguageRuntime instance being registered. + */ + public registerMainThreadLanguageRuntime(id: number): void { + // Add the mainThreadLanguageRuntime instance id to the set of mainThreadLanguageRuntimes. + this._discoveryCompleteByExtHostId.set(id, false); + this._logService.debug(`[Runtime startup] Registered extension host with id: ${id}.`); + } + + /** + * Used to un-registers an instance of a MainThreadLanguageRuntime. + * + * This is required because there can be multiple extension hosts + * and the startup service needs to know of all of them to track + * the startup phase across all extension hosts. + * + * @param id The id of the MainThreadLanguageRuntime instance being un-registered. + */ + public unregisterMainThreadLanguageRuntime(id: number): void { + // Remove the mainThreadLanguageRuntime instance id to the set of mainThreadLanguageRuntimes. + this._discoveryCompleteByExtHostId.delete(id); + this._logService.debug(`[Runtime startup] Unregistered extension host with id: ${id}.`); } /** @@ -826,6 +887,7 @@ export class RuntimeStartupService extends Disposable implements IRuntimeStartup * @param sessions The set of sessions to restore. */ private async restoreWorkspaceSessions(sessions: SerializedSessionMetadata[]) { + this.setStartupPhase(RuntimeStartupPhase.Reconnecting); // Activate any extensions needed for the sessions that are persistent on the machine. diff --git a/src/vs/workbench/services/runtimeStartup/common/runtimeStartupService.ts b/src/vs/workbench/services/runtimeStartup/common/runtimeStartupService.ts index 314448cd8db..11cf4fbce2e 100644 --- a/src/vs/workbench/services/runtimeStartup/common/runtimeStartupService.ts +++ b/src/vs/workbench/services/runtimeStartup/common/runtimeStartupService.ts @@ -43,8 +43,31 @@ export interface IRuntimeStartupService { getAffiliatedRuntimeMetadata(languageId: string): ILanguageRuntimeMetadata | undefined; /** - * Signal that discovery of language runtimes is complete. Called from the - * extension host. + * Signal that discovery of language runtimes is completed for an extension host. + * + * @param id the id of the MainThreadLanguageRuntime instance for the extension host + */ + completeDiscovery(id: number): void; + + /** + * Used to register an instance of a MainThreadLanguageRuntime. + * + * This is required because there can be multiple extension hosts + * and the startup service needs to know of all of them to track + * the startup phase across all extension hosts. + * + * @param id The id of the MainThreadLanguageRuntime instance for the extension host. + */ + registerMainThreadLanguageRuntime(id: number): void; + + /** + * Used to un-register an instance of a MainThreadLanguageRuntime. + * + * This is required because there can be multiple extension hosts + * and the startup service needs to know of all of them to track + * the startup phase across all extension hosts. + * + * @param id The id of the MainThreadLanguageRuntime instance for the extension host. */ - completeDiscovery(): void; + unregisterMainThreadLanguageRuntime(id: number): void; } diff --git a/test/e2e/infra/fixtures/interpreter.ts b/test/e2e/infra/fixtures/interpreter.ts index 2d3c91070c0..b247cea065c 100644 --- a/test/e2e/infra/fixtures/interpreter.ts +++ b/test/e2e/infra/fixtures/interpreter.ts @@ -46,14 +46,8 @@ export class Interpreter { await test.step(`Select interpreter via Quick Access: ${interpreterType}`, async () => { interpreterType === 'Python' - ? await this.console.selectInterpreter(InterpreterType.Python, DESIRED_PYTHON) - : await this.console.selectInterpreter(InterpreterType.R, DESIRED_R); - - if (waitForReady) { - interpreterType === 'Python' - ? await this.console.waitForReady('>>>', 30000) - : await this.console.waitForReady('>', 30000); - } + ? await this.console.selectInterpreter(InterpreterType.Python, DESIRED_PYTHON, waitForReady) + : await this.console.selectInterpreter(InterpreterType.R, DESIRED_R, waitForReady); }); }; diff --git a/test/e2e/tests/_test.setup.ts b/test/e2e/tests/_test.setup.ts index 153d88e80e5..c792ff1876d 100644 --- a/test/e2e/tests/_test.setup.ts +++ b/test/e2e/tests/_test.setup.ts @@ -109,13 +109,13 @@ export const test = base.extend({ }, { scope: 'worker', auto: true, timeout: 60000 }], interpreter: [async ({ app, page }, use) => { - const setInterpreter = async (desiredInterpreter: 'Python' | 'R') => { + const setInterpreter = async (desiredInterpreter: 'Python' | 'R', waitForReady = true) => { const currentInterpreter = await page.locator('.top-action-bar-interpreters-manager').textContent() || ''; if (!currentInterpreter.startsWith(desiredInterpreter)) { desiredInterpreter === 'Python' - ? await app.workbench.interpreter.startInterpreterViaQuickAccess('Python') - : await app.workbench.interpreter.startInterpreterViaQuickAccess('R'); + ? await app.workbench.interpreter.startInterpreterViaQuickAccess('Python', waitForReady) + : await app.workbench.interpreter.startInterpreterViaQuickAccess('R', waitForReady); } }; diff --git a/test/e2e/tests/reticulate/reticulate.test.ts b/test/e2e/tests/reticulate/reticulate.test.ts index 9cee32559a5..d33bdbcc9af 100644 --- a/test/e2e/tests/reticulate/reticulate.test.ts +++ b/test/e2e/tests/reticulate/reticulate.test.ts @@ -19,10 +19,7 @@ test.describe('Reticulate', { }, () => { test.beforeAll(async function ({ app, userSettings }) { try { - // remove this once https://github.com/posit-dev/positron/issues/5226 - // is resolved await userSettings.set([ - ['kernelSupervisor.enable', 'false'], ['positron.reticulate.enabled', 'true'] ]); @@ -32,6 +29,10 @@ test.describe('Reticulate', { } }); + // if running tests in sequence, we will need to skip waiting for ready because interpreters + // will already be running + let sequential = false; + test('R - Verify Basic Reticulate Functionality [C...]', async function ({ app, r, interpreter }) { await app.workbench.console.pasteCodeToConsole('reticulate::repl_python()'); @@ -46,19 +47,69 @@ test.describe('Reticulate', { } await app.workbench.console.waitForReady('>>>'); - await app.workbench.console.pasteCodeToConsole('x=100'); - await app.workbench.console.sendEnterKey(); - await interpreter.set('R'); + await verifyReticulateFunctionality(app, interpreter, false); - await app.workbench.console.pasteCodeToConsole('y<-reticulate::py$x'); - await app.workbench.console.sendEnterKey(); - await app.workbench.layouts.enterLayout('fullSizedAuxBar'); + sequential = true; + + }); + + test('R - Verify Reticulate Stop/Restart Functionality [C...]', async function ({ app, interpreter }) { + + // web only test but we don't have another way to skip electron tests + if (!app.web) { + return; + } + + await app.workbench.interpreter.selectInterpreter('Python', 'Python (reticulate)', !sequential); + + await verifyReticulateFunctionality(app, interpreter, sequential); + + await app.workbench.layouts.enterLayout('stacked'); + + await app.workbench.console.barPowerButton.click(); + + await app.workbench.console.waitForConsoleContents('shut down successfully'); + + await app.code.driver.page.locator('.positron-console').getByRole('button', { name: 'Restart R' }).click(); + + await app.workbench.console.waitForReady('>'); - await expect(async () => { - const variablesMap = await app.workbench.variables.getFlatVariables(); - expect(variablesMap.get('y')).toStrictEqual({ value: '100', type: 'int' }); - }).toPass({ timeout: 60000 }); + await app.code.driver.page.locator('.positron-console').locator('.action-bar-button-drop-down-arrow').click(); + + await app.code.driver.page.locator('.action-label', { hasText: 'Python (reticulate)' }).hover(); + + await app.code.driver.page.keyboard.press('Enter'); + + await app.code.driver.page.locator('.positron-console').getByRole('button', { name: 'Restart Python' }).click(); + + await app.workbench.console.waitForReady('>>>'); + + await verifyReticulateFunctionality(app, interpreter, sequential); }); }); + +async function verifyReticulateFunctionality(app, interpreter, sequential) { + + await app.workbench.console.pasteCodeToConsole('x=100'); + await app.workbench.console.sendEnterKey(); + + await app.workbench.console.barClearButton.click(); + + await interpreter.set('R', !sequential); + + await app.workbench.console.pasteCodeToConsole('y<-reticulate::py$x'); + await app.workbench.console.sendEnterKey(); + + await app.workbench.console.barClearButton.click(); + + await app.workbench.layouts.enterLayout('fullSizedAuxBar'); + + await expect(async () => { + const variablesMap = await app.workbench.variables.getFlatVariables(); + expect(variablesMap.get('y')).toStrictEqual({ value: '100', type: 'int' }); + }).toPass({ timeout: 60000 }); + + await app.workbench.layouts.enterLayout('stacked'); +};