Skip to content

Commit

Permalink
Add Positron instantiation service test helpers; refactor tests to us…
Browse files Browse the repository at this point in the history 
…e runtime services (#5335)

This is in preparation for writing unit tests for the runtime session
service since I intend on making changes to it for #2671.

The main addition is the `positronWorkbenchInstantiationService`
function, which sets up a test instantiation service with a bunch of
test dummies, following upstream's `workbenchInstantiationService`
function. I also refactored our existing unit tests to use the new
function.

#### QA Notes

1. Positron unit tests should pass. They can be run locally with:

    ```sh
    ./scripts/test-positron.sh
    ```
2. There are a few minor code changes to runtime-related code, mainly to
fix leaked disposables, which could use a review.
  • Loading branch information
@seeM
seeM authored Nov 13, 2024
1 parent 7998f17 commit fbd8b46
Show file tree
Hide file tree
Showing 11 changed files with 390 additions and 186 deletions.
4 changes: 4 additions & 0 deletions src/vs/workbench/api/browser/positron/mainThreadLanguageRuntime.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -898,6 +898,10 @@ class ExtHostLanguageRuntimeSessionAdapter implements ILanguageRuntimeSession {
}

static clientCounter = 0;

dispose(): void {
// Do nothing.
}
}

/**
Expand Down
96 changes: 26 additions & 70 deletions src/vs/workbench/contrib/positronIPyWidgets/test/browser/positronIPyWidgetsService.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,51 +7,23 @@ import { timeout } from 'vs/base/common/async';
import { Emitter } from 'vs/base/common/event';
import { URI } from 'vs/base/common/uri';
import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';
import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions';
import { ILogService, NullLogService } from 'vs/platform/log/common/log';
import { TestThemeService } from 'vs/platform/theme/test/common/testThemeService';
import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock';
import { ILogService } from 'vs/platform/log/common/log';
import { INotebookEditor } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
import { INotebookEditorService } from 'vs/workbench/contrib/notebook/browser/services/notebookEditorService';
import { NotebookEditorWidgetService } from 'vs/workbench/contrib/notebook/browser/services/notebookEditorServiceImpl';
import { NotebookRendererMessagingService } from 'vs/workbench/contrib/notebook/browser/services/notebookRendererMessagingServiceImpl';
import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel';
import { INotebookRendererInfo, INotebookStaticPreloadInfo } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { NotebookOutputRendererInfo } from 'vs/workbench/contrib/notebook/common/notebookOutputRenderer';
import { INotebookRendererMessagingService } from 'vs/workbench/contrib/notebook/common/notebookRendererMessagingService';
import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService';
import { IPyWidgetsInstance, PositronIPyWidgetsService } from 'vs/workbench/contrib/positronIPyWidgets/browser/positronIPyWidgetsService';
import { IPositronNotebookOutputWebviewService } from 'vs/workbench/contrib/positronOutputWebview/browser/notebookOutputWebviewService';
import { PositronNotebookOutputWebviewService } from 'vs/workbench/contrib/positronOutputWebview/browser/notebookOutputWebviewServiceImpl';
import { WebviewPlotClient } from 'vs/workbench/contrib/positronPlots/browser/webviewPlotClient';
import { IWebviewService } from 'vs/workbench/contrib/webview/browser/webview';
import { WebviewService } from 'vs/workbench/contrib/webview/browser/webviewService';
import { RuntimeClientState } from 'vs/workbench/services/languageRuntime/common/languageRuntimeClientInstance';
import { ILanguageRuntimeMessageClearOutput, ILanguageRuntimeMessageError, ILanguageRuntimeMessageOutput, ILanguageRuntimeMessageResult, ILanguageRuntimeMessageStream, LanguageRuntimeMessageType, LanguageRuntimeSessionMode, RuntimeOutputKind, RuntimeState } from 'vs/workbench/services/languageRuntime/common/languageRuntimeService';
import { ILanguageRuntimeMessageClearOutput, ILanguageRuntimeMessageError, ILanguageRuntimeMessageOutput, ILanguageRuntimeMessageResult, ILanguageRuntimeMessageStream, LanguageRuntimeMessageType, LanguageRuntimeSessionMode, RuntimeOutputKind } from 'vs/workbench/services/languageRuntime/common/languageRuntimeService';
import { ToWebviewMessage } from 'vs/workbench/services/languageRuntime/common/positronIPyWidgetsWebviewMessages';
import { TestIPyWidgetsWebviewMessaging } from 'vs/workbench/services/languageRuntime/test/common/testIPyWidgetsWebviewMessaging';
import { INotebookDocumentService, NotebookDocumentWorkbenchService } from 'vs/workbench/services/notebook/common/notebookDocumentService';
import { IRuntimeSessionService, RuntimeClientType } from 'vs/workbench/services/runtimeSession/common/runtimeSessionService';
import { RuntimeClientType } from 'vs/workbench/services/runtimeSession/common/runtimeSessionService';
import { TestLanguageRuntimeSession } from 'vs/workbench/services/runtimeSession/test/common/testLanguageRuntimeSession';
import { TestRuntimeSessionService } from 'vs/workbench/services/runtimeSession/test/common/testRuntimeSessionService';
import { IWorkbenchThemeService } from 'vs/workbench/services/themes/common/workbenchThemeService';
import { workbenchInstantiationService } from 'vs/workbench/test/browser/workbenchTestServices';

export class TestNotebookService implements Partial<INotebookService> {
getRenderers(): INotebookRendererInfo[] {
return [];
}

getPreferredRenderer(_mimeType: string): NotebookOutputRendererInfo | undefined {
return <NotebookOutputRendererInfo>{
id: 'positron-ipywidgets',
extensionId: new ExtensionIdentifier('vscode.positron-ipywidgets'),
};
}

*getStaticPreloads(_viewType: string): Iterable<INotebookStaticPreloadInfo> {
// Yield nothing.
}
}
import { startTestLanguageRuntimeSession } from 'vs/workbench/services/runtimeSession/test/common/testRuntimeSessionService';
import { PositronTestServiceAccessor, positronWorkbenchInstantiationService } from 'vs/workbench/test/browser/positronWorkbenchTestServices';
import { TestNotebookService } from 'vs/workbench/test/common/positronWorkbenchTestServices';

interface TestNotebookEditor extends INotebookEditor {
changeModel(uri: URI): void;
Expand All @@ -60,32 +32,17 @@ interface TestNotebookEditor extends INotebookEditor {
suite('Positron - PositronIPyWidgetsService', () => {
const disposables = ensureNoDisposablesAreLeakedInTestSuite();

let instantiationService: TestInstantiationService;
let positronIpywidgetsService: PositronIPyWidgetsService;
let runtimeSessionService: TestRuntimeSessionService;
let notebookEditorService: INotebookEditorService;

setup(() => {
const instantiationService = workbenchInstantiationService(undefined, disposables);
instantiationService.stub(INotebookRendererMessagingService, disposables.add(instantiationService.createInstance(NotebookRendererMessagingService)));
notebookEditorService = disposables.add(instantiationService.createInstance(NotebookEditorWidgetService));
instantiationService.stub(INotebookEditorService, notebookEditorService);
instantiationService.stub(IWorkbenchThemeService, new TestThemeService() as any);
instantiationService.stub(INotebookDocumentService, new NotebookDocumentWorkbenchService());
instantiationService.stub(INotebookService, new TestNotebookService());
instantiationService.stub(IWebviewService, disposables.add(new WebviewService(instantiationService)));
instantiationService.stub(IPositronNotebookOutputWebviewService, instantiationService.createInstance(PositronNotebookOutputWebviewService));
runtimeSessionService = disposables.add(new TestRuntimeSessionService());
instantiationService.stub(IRuntimeSessionService, runtimeSessionService);
positronIpywidgetsService = disposables.add(instantiationService.createInstance(PositronIPyWidgetsService));
instantiationService = positronWorkbenchInstantiationService(disposables);
const accessor = instantiationService.createInstance(PositronTestServiceAccessor);
notebookEditorService = accessor.notebookEditorService;
positronIpywidgetsService = accessor.positronIPyWidgetsService;
});

async function startConsoleSession() {
const session = disposables.add(new TestLanguageRuntimeSession(LanguageRuntimeSessionMode.Console));
runtimeSessionService.startSession(session);
await timeout(0);
return session;
}

async function receiveIPyWidgetsResultMessage(
session: TestLanguageRuntimeSession,
parentId?: string,
Expand All @@ -110,7 +67,7 @@ suite('Positron - PositronIPyWidgetsService', () => {
disposables.add(positronIpywidgetsService.onDidCreatePlot(client => plotClient = client));

// Start a console session.
const session = await startConsoleSession();
const session = await startTestLanguageRuntimeSession(instantiationService, disposables);

// Simulate the runtime sending an IPyWidgets output message.
const message = await receiveIPyWidgetsResultMessage(session);
Expand Down Expand Up @@ -184,12 +141,11 @@ suite('Positron - PositronIPyWidgetsService', () => {
notebookEditorService.addNotebookEditor(notebookEditor);

// Start a notebook session.
const session = disposables.add(new TestLanguageRuntimeSession(
LanguageRuntimeSessionMode.Notebook,
notebookUri,
));
runtimeSessionService.startSession(session);
await timeout(0);
const session = await startTestLanguageRuntimeSession(
instantiationService,
disposables,
{ sessionMode: LanguageRuntimeSessionMode.Notebook, notebookUri },
);

// Check that an instance was created.
assert(positronIpywidgetsService.hasInstance(session.sessionId));
Expand Down Expand Up @@ -247,13 +203,12 @@ suite('Positron - IPyWidgetsInstance constructor', () => {
let notebookService: INotebookService;

setup(async () => {
logService = new NullLogService();
session = disposables.add(new TestLanguageRuntimeSession());
const instantiationService = positronWorkbenchInstantiationService(disposables);
const accessor = instantiationService.createInstance(PositronTestServiceAccessor);
logService = accessor.logService;
session = await startTestLanguageRuntimeSession(instantiationService, disposables);
messaging = disposables.add(new TestIPyWidgetsWebviewMessaging());
notebookService = new TestNotebookService() as INotebookService;

// Set the runtime state to ready.
session.setRuntimeState(RuntimeState.Ready);
});

async function createIPyWidgetsInstance() {
Expand Down Expand Up @@ -299,15 +254,16 @@ suite('Positron - IPyWidgetsInstance', () => {
let ipywidgetsInstance: IPyWidgetsInstance;

setup(async () => {
const logService = new NullLogService();
session = disposables.add(new TestLanguageRuntimeSession());
const instantiationService = positronWorkbenchInstantiationService(disposables);
const accessor = instantiationService.createInstance(PositronTestServiceAccessor);
session = await startTestLanguageRuntimeSession(instantiationService, disposables);
messaging = disposables.add(new TestIPyWidgetsWebviewMessaging());
const notebookService = new TestNotebookService() as INotebookService;
ipywidgetsInstance = disposables.add(new IPyWidgetsInstance(
session,
messaging,
notebookService,
logService,
accessor.logService,
));

// Clear initial messages.
Expand Down
38 changes: 12 additions & 26 deletions src/vs/workbench/contrib/positronPlots/test/electron-sandbox/positronPlotsService.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,44 +4,30 @@
*--------------------------------------------------------------------------------------------*/

import assert from 'assert';
import { raceTimeout, timeout } from 'vs/base/common/async';
import { raceTimeout } from 'vs/base/common/async';
import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';
import { PositronIPyWidgetsService } from 'vs/workbench/contrib/positronIPyWidgets/browser/positronIPyWidgetsService';
import { PositronPlotsService } from 'vs/workbench/contrib/positronPlots/browser/positronPlotsService';
import { PositronWebviewPreloadService } from 'vs/workbench/contrib/positronWebviewPreloads/browser/positronWebviewPreloadsService';
import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock';
import { PositronTestServiceAccessor, positronWorkbenchInstantiationService as positronWorkbenchInstantiationService } from 'vs/workbench/test/browser/positronWorkbenchTestServices';
import { IPositronPlotMetadata } from 'vs/workbench/services/languageRuntime/common/languageRuntimePlotClient';
import { LanguageRuntimeSessionMode } from 'vs/workbench/services/languageRuntime/common/languageRuntimeService';
import { IPositronIPyWidgetsService } from 'vs/workbench/services/positronIPyWidgets/common/positronIPyWidgetsService';
import { HistoryPolicy, IPositronPlotClient } from 'vs/workbench/services/positronPlots/common/positronPlots';
import { IPositronWebviewPreloadService } from 'vs/workbench/services/positronWebviewPreloads/common/positronWebviewPreloadService';
import { IRuntimeSessionService, RuntimeClientType } from 'vs/workbench/services/runtimeSession/common/runtimeSessionService';
import { HistoryPolicy, IPositronPlotClient, IPositronPlotsService } from 'vs/workbench/services/positronPlots/common/positronPlots';
import { RuntimeClientType } from 'vs/workbench/services/runtimeSession/common/runtimeSessionService';
import { TestLanguageRuntimeSession } from 'vs/workbench/services/runtimeSession/test/common/testLanguageRuntimeSession';
import { TestRuntimeSessionService } from 'vs/workbench/services/runtimeSession/test/common/testRuntimeSessionService';
import { IViewsService } from 'vs/workbench/services/views/common/viewsService';
import { TestViewsService, workbenchInstantiationService } from 'vs/workbench/test/browser/workbenchTestServices';
import { startTestLanguageRuntimeSession } from 'vs/workbench/services/runtimeSession/test/common/testRuntimeSessionService';

suite('Positron - Plots Service', () => {

const disposables = ensureNoDisposablesAreLeakedInTestSuite();
let plotsService: PositronPlotsService;
let runtimeSessionService: TestRuntimeSessionService;
let instantiationService: TestInstantiationService;
let plotsService: IPositronPlotsService;

setup(() => {
const instantiationService = workbenchInstantiationService(undefined, disposables);
runtimeSessionService = disposables.add(instantiationService.createInstance(TestRuntimeSessionService));
instantiationService.stub(IRuntimeSessionService, runtimeSessionService);
instantiationService.stub(IPositronWebviewPreloadService, disposables.add(instantiationService.createInstance(PositronWebviewPreloadService)));
instantiationService.stub(IPositronIPyWidgetsService, disposables.add(instantiationService.createInstance(PositronIPyWidgetsService)));
instantiationService.stub(IViewsService, new TestViewsService());

plotsService = disposables.add(instantiationService.createInstance(PositronPlotsService));
instantiationService = positronWorkbenchInstantiationService(disposables);
const accessor = instantiationService.createInstance(PositronTestServiceAccessor);
plotsService = accessor.positronPlotsService;
});

async function createSession() {
const session = disposables.add(new TestLanguageRuntimeSession(LanguageRuntimeSessionMode.Console));
runtimeSessionService.startSession(session);

await timeout(0);
const session = await startTestLanguageRuntimeSession(instantiationService, disposables);

const out: {
session: TestLanguageRuntimeSession;
Expand Down
34 changes: 9 additions & 25 deletions ...kbench/contrib/positronWebviewPreloads/test/browser/positronWebviewPreloadService.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,13 @@
import assert from 'assert';
import { timeout } from 'vs/base/common/async';
import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';
import { NotebookRendererMessagingService } from 'vs/workbench/contrib/notebook/browser/services/notebookRendererMessagingServiceImpl';
import { INotebookRendererMessagingService } from 'vs/workbench/contrib/notebook/common/notebookRendererMessagingService';
import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService';
import { PositronWebviewPreloadService } from 'vs/workbench/contrib/positronWebviewPreloads/browser/positronWebviewPreloadsService';
import { TestNotebookService } from 'vs/workbench/contrib/positronIPyWidgets/test/browser/positronIPyWidgetsService.test';
import { IPositronNotebookOutputWebviewService } from 'vs/workbench/contrib/positronOutputWebview/browser/notebookOutputWebviewService';
import { PositronNotebookOutputWebviewService } from 'vs/workbench/contrib/positronOutputWebview/browser/notebookOutputWebviewServiceImpl';
import { PositronTestServiceAccessor, positronWorkbenchInstantiationService } from 'vs/workbench/test/browser/positronWorkbenchTestServices';
import { WebviewPlotClient } from 'vs/workbench/contrib/positronPlots/browser/webviewPlotClient';
import { IWebviewService } from 'vs/workbench/contrib/webview/browser/webview';
import { WebviewService } from 'vs/workbench/contrib/webview/browser/webviewService';
import { LanguageRuntimeSessionMode, RuntimeOutputKind } from 'vs/workbench/services/languageRuntime/common/languageRuntimeService';
import { IRuntimeSessionService } from 'vs/workbench/services/runtimeSession/common/runtimeSessionService';
import { RuntimeOutputKind } from 'vs/workbench/services/languageRuntime/common/languageRuntimeService';
import { TestLanguageRuntimeSession } from 'vs/workbench/services/runtimeSession/test/common/testLanguageRuntimeSession';
import { TestRuntimeSessionService } from 'vs/workbench/services/runtimeSession/test/common/testRuntimeSessionService';
import { workbenchInstantiationService } from 'vs/workbench/test/browser/workbenchTestServices';
import { startTestLanguageRuntimeSession } from 'vs/workbench/services/runtimeSession/test/common/testRuntimeSessionService';
import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock';


const hvPreloadMessage = {
Expand Down Expand Up @@ -56,27 +48,19 @@ const bokehDisplayMessage = {
suite('Positron - PositronWebviewPreloadService', () => {
const disposables = ensureNoDisposablesAreLeakedInTestSuite();

let instantiationService: TestInstantiationService;
let positronWebviewPreloadService: PositronWebviewPreloadService;
let runtimeSessionService: TestRuntimeSessionService;

setup(() => {
const instantiationService = workbenchInstantiationService(undefined, disposables);
instantiationService.stub(INotebookRendererMessagingService, disposables.add(instantiationService.createInstance(NotebookRendererMessagingService)));
instantiationService.stub(INotebookService, new TestNotebookService());
instantiationService.stub(IWebviewService, disposables.add(new WebviewService(instantiationService)));
instantiationService.stub(IPositronNotebookOutputWebviewService, instantiationService.createInstance(PositronNotebookOutputWebviewService));
runtimeSessionService = disposables.add(new TestRuntimeSessionService());
instantiationService.stub(IRuntimeSessionService, runtimeSessionService);
positronWebviewPreloadService = disposables.add(instantiationService.createInstance(PositronWebviewPreloadService));
instantiationService = positronWorkbenchInstantiationService(disposables);
const accessor = instantiationService.createInstance(PositronTestServiceAccessor);
positronWebviewPreloadService = accessor.positronWebviewPreloadService;
});

async function createConsoleSession() {

// Start a console session.
const session = disposables.add(new TestLanguageRuntimeSession(LanguageRuntimeSessionMode.Console));
runtimeSessionService.startSession(session);

await timeout(0);
const session = await startTestLanguageRuntimeSession(instantiationService, disposables);

const out: {
session: TestLanguageRuntimeSession;
Expand Down
6 changes: 3 additions & 3 deletions src/vs/workbench/services/languageRuntime/common/languageRuntime.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export class LanguageRuntimeService extends Disposable implements ILanguageRunti
registerRuntime(metadata: ILanguageRuntimeMetadata): IDisposable {
// If the runtime has already been registered, return early.
if (this._registeredRuntimesByRuntimeId.has(metadata.runtimeId)) {
return toDisposable(() => { });
return this._register(toDisposable(() => { }));
}

// Add the runtime to the registered runtimes.
Expand All @@ -93,9 +93,9 @@ export class LanguageRuntimeService extends Disposable implements ILanguageRunti
// Logging.
this._logService.trace(`Language runtime ${formatLanguageRuntimeMetadata(metadata)} successfully registered.`);

return toDisposable(() => {
return this._register(toDisposable(() => {
this._registeredRuntimesByRuntimeId.delete(metadata.runtimeId);
});
}));
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/services/languageRuntime/common/languageRuntimeUiClient.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export class UiClientInstance extends Disposable {
super();
this._register(this._client);

this._comm = new PositronUiCommInstance(this._client);
this._comm = this._register(new PositronUiCommInstance(this._client));
this.onDidBusy = this._comm.onDidBusy;
this.onDidClearConsole = this._comm.onDidClearConsole;
this.onDidSetEditorSelections = this._comm.onDidSetEditorSelections;
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/services/runtimeSession/common/runtimeSessionService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export interface IRuntimeSessionMetadata {
* The main interface for interacting with a language runtime session.
*/

export interface ILanguageRuntimeSession {
export interface ILanguageRuntimeSession extends IDisposable {
/** The language runtime's static metadata */
readonly runtimeMetadata: ILanguageRuntimeMetadata;

Expand Down
Loading

0 comments on commit fbd8b46

Please sign in to comment.