Skip to content

Commit 9dec031

Browse files
authored
First steps in functional tests for the native window (#7513)
* First step in native editor functional test * Tests building * Native functional tests passing locally * Fix code watcher functional * Fix non finished executes from crashing shutdown * Add news entries
1 parent 574a9e3 commit 9dec031

24 files changed

+985
-590
lines changed

news/3 Code Health/7367.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Test scaffolding for notebook editor.

news/3 Code Health/7371.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Tests for the notebook editor for different mime types.

src/client/datascience/interactive-common/interactiveBase.ts

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
8989
private variableRequestStopWatch: StopWatch | undefined;
9090
private variableRequestPendingCount: number = 0;
9191
private loadPromise: Promise<void> | undefined;
92+
private setDark: boolean = false;
9293

9394
constructor(
9495
@unmanaged() private readonly listeners: IInteractiveWindowListener[],
@@ -466,6 +467,9 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
466467
// Make sure we're loaded first.
467468
await this.ensureServerActive();
468469

470+
// Make sure we set the dark setting
471+
await this.ensureDarkSet();
472+
469473
// Then show our webpanel
470474
await this.show();
471475

@@ -1025,12 +1029,23 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
10251029
}
10261030
}
10271031

1032+
private async ensureDarkSet(): Promise<void> {
1033+
if (!this.setDark) {
1034+
this.setDark = true;
1035+
1036+
// Wait for the web panel to get the isDark setting
1037+
const knownDark = await this.isDark();
1038+
1039+
// Before we run any cells, update the dark setting
1040+
if (this.notebook) {
1041+
await this.notebook.setMatplotLibStyle(knownDark);
1042+
}
1043+
}
1044+
}
1045+
10281046
private async createNotebook(): Promise<void> {
10291047
traceInfo('Getting jupyter server options ...');
10301048

1031-
// Wait for the webpanel to pass back our current theme darkness
1032-
const knownDark = await this.isDark();
1033-
10341049
// Extract our options
10351050
const options = await this.getNotebookOptions();
10361051

@@ -1044,11 +1059,6 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
10441059
this.notebook = await server.createNotebook(await this.getNotebookIdentity());
10451060
}
10461061

1047-
// Before we run any cells, update the dark setting
1048-
if (this.notebook) {
1049-
await this.notebook.setMatplotLibStyle(knownDark);
1050-
}
1051-
10521062
traceInfo('Connected to jupyter server.');
10531063
}
10541064

src/client/datascience/interactive-window/interactiveWindowProvider.ts

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,19 @@ interface ISyncData {
2323
@injectable()
2424
export class InteractiveWindowProvider implements IInteractiveWindowProvider, IAsyncDisposable {
2525

26-
private activeInteractiveWindow : IInteractiveWindow | undefined;
27-
private postOffice : PostOffice;
26+
private activeInteractiveWindow: IInteractiveWindow | undefined;
27+
private postOffice: PostOffice;
2828
private id: string;
29-
private pendingSyncs : Map<string, ISyncData> = new Map<string, ISyncData>();
29+
private pendingSyncs: Map<string, ISyncData> = new Map<string, ISyncData>();
3030
private executedCode: EventEmitter<string> = new EventEmitter<string>();
3131
private activeInteractiveWindowExecuteHandler: Disposable | undefined;
3232
constructor(
3333
@inject(ILiveShareApi) liveShare: ILiveShareApi,
3434
@inject(IServiceContainer) private serviceContainer: IServiceContainer,
35-
@inject(IAsyncDisposableRegistry) asyncRegistry : IAsyncDisposableRegistry,
35+
@inject(IAsyncDisposableRegistry) asyncRegistry: IAsyncDisposableRegistry,
3636
@inject(IDisposableRegistry) private disposables: IDisposableRegistry,
3737
@inject(IConfigurationService) private configService: IConfigurationService
38-
) {
38+
) {
3939
asyncRegistry.push(this);
4040

4141
// Create a post office so we can make sure interactive windows are created at the same time
@@ -53,15 +53,15 @@ export class InteractiveWindowProvider implements IInteractiveWindowProvider, IA
5353
this.id = uuid();
5454
}
5555

56-
public getActive() : IInteractiveWindow | undefined {
56+
public getActive(): IInteractiveWindow | undefined {
5757
return this.activeInteractiveWindow;
5858
}
5959

60-
public get onExecutedCode() : Event<string> {
60+
public get onExecutedCode(): Event<string> {
6161
return this.executedCode.event;
6262
}
6363

64-
public async getOrCreateActive() : Promise<IInteractiveWindow> {
64+
public async getOrCreateActive(): Promise<IInteractiveWindow> {
6565
if (!this.activeInteractiveWindow) {
6666
await this.create();
6767
}
@@ -77,7 +77,7 @@ export class InteractiveWindowProvider implements IInteractiveWindowProvider, IA
7777
throw new Error(localize.DataScience.pythonInteractiveCreateFailed());
7878
}
7979

80-
public async getNotebookOptions() : Promise<INotebookServerOptions> {
80+
public async getNotebookOptions(): Promise<INotebookServerOptions> {
8181
// Find the settings that we are going to launch our server with
8282
const settings = this.configService.getSettings();
8383
let serverURI: string | undefined = settings.datascience.jupyterServerURI;
@@ -96,11 +96,11 @@ export class InteractiveWindowProvider implements IInteractiveWindowProvider, IA
9696
};
9797
}
9898

99-
public dispose() : Promise<void> {
99+
public dispose(): Promise<void> {
100100
return this.postOffice.dispose();
101101
}
102102

103-
private async create() : Promise<void> {
103+
private async create(): Promise<IInteractiveWindow> {
104104
// Set it as soon as we create it. The .ctor for the interactive window
105105
// may cause a subclass to talk to the IInteractiveWindowProvider to get the active interactive window.
106106
this.activeInteractiveWindow = this.serviceContainer.get<IInteractiveWindow>(IInteractiveWindow);
@@ -110,6 +110,7 @@ export class InteractiveWindowProvider implements IInteractiveWindowProvider, IA
110110
this.activeInteractiveWindowExecuteHandler = this.activeInteractiveWindow.onExecutedCode(this.onInteractiveWindowExecute);
111111
this.disposables.push(this.activeInteractiveWindowExecuteHandler);
112112
await this.activeInteractiveWindow.ready;
113+
return this.activeInteractiveWindow;
113114
}
114115

115116
private onPeerCountChanged(newCount: number) {
@@ -162,7 +163,7 @@ export class InteractiveWindowProvider implements IInteractiveWindowProvider, IA
162163
}
163164
}
164165

165-
private async synchronizeCreate() : Promise<void> {
166+
private async synchronizeCreate(): Promise<void> {
166167
// Create a new pending wait if necessary
167168
if (this.postOffice.peerCount > 0 || this.postOffice.role === vsls.Role.Guest) {
168169
const key = uuid();

src/client/datascience/jupyter/jupyterNotebook.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ export class JupyterNotebookBase implements INotebook {
138138
private pendingCellSubscriptions: CellSubscriber[] = [];
139139
private ranInitialSetup = false;
140140
private _resource: Uri;
141+
private _disposed: boolean = false;
141142

142143
constructor(
143144
_liveShare: ILiveShareApi, // This is so the liveshare mixin works
@@ -160,8 +161,12 @@ export class JupyterNotebookBase implements INotebook {
160161

161162
public dispose(): Promise<void> {
162163
traceInfo(`Shutting down session ${this.resource.toString()}`);
163-
const dispose = this.session ? this.session.dispose() : undefined;
164-
return dispose ? dispose : Promise.resolve();
164+
if (!this._disposed) {
165+
this._disposed = true;
166+
const dispose = this.session ? this.session.dispose() : undefined;
167+
return dispose ? dispose : Promise.resolve();
168+
}
169+
return Promise.resolve();
165170
}
166171

167172
public get resource(): Uri {
@@ -615,7 +620,10 @@ export class JupyterNotebookBase implements INotebook {
615620
// If the server crashes, cancel the current observable
616621
exitHandlerDisposable = this.launchInfo.connectionInfo.disconnected((c) => {
617622
const str = c ? c.toString() : '';
618-
subscriber.error(this.sessionStartTime, new Error(localize.DataScience.jupyterServerCrashed().format(str)));
623+
// Only do an error if we're not disposed. If we're disposed we already shutdown.
624+
if (!this._disposed) {
625+
subscriber.error(this.sessionStartTime, new Error(localize.DataScience.jupyterServerCrashed().format(str)));
626+
}
619627
subscriber.complete(this.sessionStartTime);
620628
});
621629
}

src/client/datascience/webViewHost.ts

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,15 @@ export class WebViewHost<IMapping> implements IDisposable {
8686
}
8787
}
8888

89+
public setTheme(isDark: boolean) {
90+
if (this.themeIsDarkPromise && !this.themeIsDarkPromise.resolved) {
91+
this.themeIsDarkPromise.resolve(isDark);
92+
} else {
93+
this.themeIsDarkPromise = createDeferred<boolean>();
94+
this.themeIsDarkPromise.resolve(isDark);
95+
}
96+
}
97+
8998
protected reload() {
9099
// Make not disposed anymore
91100
this.disposed = false;
@@ -200,12 +209,7 @@ export class WebViewHost<IMapping> implements IDisposable {
200209

201210
@captureTelemetry(Telemetry.WebviewStyleUpdate)
202211
private async handleCssRequest(request: IGetCssRequest): Promise<void> {
203-
if (this.themeIsDarkPromise && !this.themeIsDarkPromise.resolved) {
204-
this.themeIsDarkPromise.resolve(request.isDark);
205-
} else {
206-
this.themeIsDarkPromise = createDeferred<boolean>();
207-
this.themeIsDarkPromise.resolve(request.isDark);
208-
}
212+
this.setTheme(request.isDark);
209213
const settings = this.generateDataScienceExtraSettings();
210214
const isDark = await this.themeFinder.isThemeDark(settings.extraSettings.theme);
211215
const css = await this.cssGenerator.generateThemeCss(request.isDark, settings.extraSettings.theme);
@@ -214,12 +218,7 @@ export class WebViewHost<IMapping> implements IDisposable {
214218

215219
@captureTelemetry(Telemetry.WebviewMonacoStyleUpdate)
216220
private async handleMonacoThemeRequest(request: IGetMonacoThemeRequest): Promise<void> {
217-
if (this.themeIsDarkPromise && !this.themeIsDarkPromise.resolved) {
218-
this.themeIsDarkPromise.resolve(request.isDark);
219-
} else {
220-
this.themeIsDarkPromise = createDeferred<boolean>();
221-
this.themeIsDarkPromise.resolve(request.isDark);
222-
}
221+
this.setTheme(request.isDark);
223222
const settings = this.generateDataScienceExtraSettings();
224223
const monacoTheme = await this.cssGenerator.generateMonacoTheme(request.isDark, settings.extraSettings.theme);
225224
return this.postMessageInternal(CssMessages.GetMonacoThemeResponse, { theme: monacoTheme });

src/datascience-ui/interactive-common/cellOutput.tsx

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@
33
'use strict';
44
import '../../client/common/extensions';
55

6+
// tslint:disable-next-line: no-var-requires no-require-imports
7+
const ansiToHtml = require('ansi-to-html');
8+
69
import { nbformat } from '@jupyterlab/coreutils';
710
import { JSONObject } from '@phosphor/coreutils';
811
import ansiRegex from 'ansi-regex';
9-
import ansiToHtml from 'ansi-to-html';
1012
// tslint:disable-next-line: no-require-imports
1113
import cloneDeep = require('lodash/cloneDeep');
1214
import * as monacoEditor from 'monaco-editor/esm/vs/editor/editor.api';
@@ -15,6 +17,7 @@ import * as React from 'react';
1517
import { concatMultilineString } from '../../client/datascience/common';
1618
import { Identifiers } from '../../client/datascience/constants';
1719
import { CellState } from '../../client/datascience/types';
20+
import { ClassType } from '../../client/ioc/types';
1821
import { noop } from '../../test/core';
1922
import { Image, ImageName } from '../react-common/image';
2023
import { ImageButton } from '../react-common/imageButton';
@@ -42,11 +45,26 @@ interface ICellOutput {
4245
}
4346
// tslint:disable: react-this-binding-issue
4447
export class CellOutput extends React.Component<ICellOutputProps> {
45-
48+
// tslint:disable-next-line: no-any
49+
private static ansiToHtmlClass_ctor: ClassType<any> | undefined;
4650
constructor(prop: ICellOutputProps) {
4751
super(prop);
4852
}
4953

54+
// tslint:disable-next-line: no-any
55+
private static get ansiToHtmlClass(): ClassType<any> {
56+
if (!CellOutput.ansiToHtmlClass_ctor) {
57+
// ansiToHtml is different between the tests running and webpack. figure out which one
58+
// tslint:disable-next-line: no-any
59+
if (ansiToHtml instanceof Function) {
60+
CellOutput.ansiToHtmlClass_ctor = ansiToHtml;
61+
} else {
62+
CellOutput.ansiToHtmlClass_ctor = ansiToHtml.default;
63+
}
64+
}
65+
return CellOutput.ansiToHtmlClass_ctor!;
66+
}
67+
5068
private static getAnsiToHtmlOptions() : { fg: string; bg: string; colors: string [] } {
5169
// Here's the default colors for ansiToHtml. We need to use the
5270
// colors from our current theme.
@@ -192,7 +210,7 @@ export class CellOutput extends React.Component<ICellOutputProps> {
192210
// colorizing if we don't have html that needs <xmp> around it (ex. <type ='string'>)
193211
try {
194212
if (ansiRegex().test(formatted)) {
195-
const converter = new ansiToHtml(CellOutput.getAnsiToHtmlOptions());
213+
const converter = new CellOutput.ansiToHtmlClass(CellOutput.getAnsiToHtmlOptions());
196214
const html = converter.toHtml(formatted);
197215
copy.data = {
198216
'text/html': html
@@ -208,7 +226,7 @@ export class CellOutput extends React.Component<ICellOutputProps> {
208226
renderWithScrollbars = true;
209227
const error = copy as nbformat.IError;
210228
try {
211-
const converter = new ansiToHtml(CellOutput.getAnsiToHtmlOptions());
229+
const converter = new CellOutput.ansiToHtmlClass(CellOutput.getAnsiToHtmlOptions());
212230
const trace = converter.toHtml(error.traceback.join('\n'));
213231
copy.data = {
214232
'text/html': trace

src/datascience-ui/interactive-common/mainStateController.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,11 @@ export class MainStateController implements IMessageHandler {
106106
// Tell the interactive window code we have started.
107107
this.postOffice.sendMessage<IInteractiveWindowMapping, 'started'>(InteractiveWindowMessages.Started);
108108

109-
// Get our monaco theme and css
110-
this.postOffice.sendUnsafeMessage(CssMessages.GetCssRequest, { isDark: this.props.expectingDark });
111-
this.postOffice.sendUnsafeMessage(CssMessages.GetMonacoThemeRequest, { isDark: this.props.expectingDark });
109+
// Get our monaco theme and css if not running a test, because these make everything async too
110+
if (!this.props.testMode) {
111+
this.postOffice.sendUnsafeMessage(CssMessages.GetCssRequest, { isDark: this.props.expectingDark });
112+
this.postOffice.sendUnsafeMessage(CssMessages.GetMonacoThemeRequest, { isDark: this.props.expectingDark });
113+
}
112114
}
113115

114116
public dispose() {

src/datascience-ui/native-editor/nativeEditor.tsx

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,12 @@ interface INativeEditorProps {
3636
}
3737

3838
export class NativeEditor extends React.Component<INativeEditorProps, IMainState> {
39+
// Public so can access it from test code
40+
public stateController: NativeEditorStateController;
41+
private renderCount: number = 0;
3942
private mainPanelRef: React.RefObject<HTMLDivElement> = React.createRef<HTMLDivElement>();
4043
private contentPanelScrollRef: React.RefObject<HTMLElement> = React.createRef<HTMLElement>();
4144
private contentPanelRef: React.RefObject<ContentPanel> = React.createRef<ContentPanel>();
42-
private stateController: NativeEditorStateController;
4345
private debounceUpdateVisibleCells = debounce(this.updateVisibleCells.bind(this), 100);
4446
private cellRefs: Map<string, React.RefObject<NativeCell>> = new Map<string, React.RefObject<NativeCell>>();
4547
private cellContainerRefs: Map<string, React.RefObject<HTMLDivElement>> = new Map<string, React.RefObject<HTMLDivElement>>();
@@ -82,6 +84,11 @@ export class NativeEditor extends React.Component<INativeEditorProps, IMainState
8284
}
8385

8486
public render() {
87+
// If in test mode, update our count. Use this to determine how many renders a normal update takes.
88+
if (this.props.testMode) {
89+
this.renderCount = this.renderCount + 1;
90+
}
91+
8592
// Update the state controller with our new state
8693
this.stateController.renderUpdate(this.state);
8794
const progressBar = this.state.busy && !this.props.testMode ? <Progress /> : undefined;

src/datascience-ui/native-editor/nativeEditorStateController.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ export class NativeEditorStateController extends MainStateController {
8484
this.resumeUpdates();
8585
}
8686

87-
public addNewCell = () => {
87+
public addNewCell = (): ICellViewModel | undefined => {
8888
const cells = this.getState().cellVMs;
8989
this.suspendUpdates();
9090
const id = uuid();
@@ -95,6 +95,7 @@ export class NativeEditorStateController extends MainStateController {
9595
vm.useQuickEdit = false;
9696
}
9797
this.resumeUpdates();
98+
return vm;
9899
}
99100

100101
public runAbove = (cellId?: string) => {

0 commit comments

Comments
 (0)