Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Main VariableView test code #4400

Merged
merged 22 commits into from
Jan 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -436,10 +436,8 @@ module.exports = {
'src/datascience-ui/interactive-common/redux/reducers/transfer.ts',
'src/datascience-ui/interactive-common/redux/reducers/types.ts',
'src/datascience-ui/interactive-common/redux/reducers/variables.ts',
'src/datascience-ui/interactive-common/redux/reducers/commonEffects.ts',
'src/datascience-ui/interactive-common/redux/reducers/kernel.ts',
'src/datascience-ui/interactive-common/redux/postOffice.ts',
'src/datascience-ui/interactive-common/redux/store.ts',
'src/datascience-ui/interactive-common/transforms.tsx',
'src/datascience-ui/interactive-common/contentPanel.tsx',
'src/datascience-ui/interactive-common/inputHistory.ts',
Expand Down Expand Up @@ -760,7 +758,6 @@ module.exports = {
'src/client/common/terminal/environmentActivationProviders/commandPrompt.ts',
'src/client/common/terminal/environmentActivationProviders/bash.ts',
'src/client/common/terminal/environmentActivationProviders/pyenvActivationProvider.ts',
'src/client/common/utils/decorators.ts',
'src/client/common/utils/enum.ts',
'src/client/common/utils/async.ts',
'src/client/common/utils/text.ts',
Expand Down Expand Up @@ -1033,8 +1030,6 @@ module.exports = {
'src/client/datascience/interactive-common/types.ts',
'src/client/datascience/interactive-common/linkProvider.ts',
'src/client/datascience/interactive-common/notebookUsageTracker.ts',
'src/client/datascience/interactive-common/interactiveWindowTypes.ts',
'src/client/datascience/interactive-common/synchronization.ts',
'src/client/datascience/interactive-common/notebookProvider.ts',
'src/client/datascience/interactive-common/interactiveWindowMessageListener.ts',
'src/client/datascience/interactive-common/intellisense/wordHelper.ts',
Expand Down
1 change: 1 addition & 0 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@
"VSC_FORCE_REAL_JUPYTER": "true", // Enalbe tests that require Jupyter.
"VSC_JUPYTER_CI_RUN_NON_PYTHON_NB_TEST": "", // Initialize this to run tests again Julia & other kernels.
"VSC_JUPYTER_RUN_NB_TEST": "true", // Initialize this to run notebook tests (must be using VSC Insiders).
"VSC_JUPYTER_WEBVIEW_TEST_MIDDLEWARE": "true", // Initialize to create the webview test middleware
Copy link
Member Author

@IanMatthewHuff IanMatthewHuff Jan 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add an env var to force on the test middle ware, needed for the middleware VariablesComplete message, and handy to be able to turn on quick just when running VS Code from debugger.

"VSC_JUPYTER_LOAD_EXPERIMENTS_FROM_FILE": "true",
"TEST_FILES_SUFFIX": "vscode.test",
"XVSC_JUPYTER_INSTRUMENT_CODE_FOR_COVERAGE": "1",
Expand Down
1 change: 1 addition & 0 deletions news/3 Code Health/4355.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add .vscode tests to test the new variable view
6 changes: 6 additions & 0 deletions src/client/common/application/webviews/webview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ export abstract class Webview implements IWebview {
)
)
.toString();

// Check to see if we should force on Test middleware for our react code
const forceTestMiddleware = process.env.VSC_JUPYTER_WEBVIEW_TEST_MIDDLEWARE || 'false';
return `<!doctype html>
<html lang="en">
<head>
Expand Down Expand Up @@ -121,6 +124,9 @@ export abstract class Webview implements IWebview {

return "${uriBase}" + relativePath;
}
function forceTestMiddleware() {
return ${forceTestMiddleware};
}
</script>
${uris.map((uri) => `<script type="text/javascript" src="${uri}"></script>`).join('\n')}
</body>
Expand Down
19 changes: 18 additions & 1 deletion src/client/common/utils/decorators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ export function displayProgress(title: string, location = ProgressLocation.Windo
// eslint-disable-next-line no-invalid-this
const promise = originalMethod.apply(this, args);
if (!isTestExecution()) {
window.withProgress(progressOptions, () => promise);
void window.withProgress(progressOptions, () => promise);
}
return promise;
};
Expand Down Expand Up @@ -244,3 +244,20 @@ export function trace(log: (c: CallInfo, t: TraceInfo) => void) {
return descriptor;
};
}

// Mark a method to be used only in tests
export function testOnlyMethod() {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
return function (_target: Object, propertyKey: string, descriptor: TypedPropertyDescriptor<any>) {
const originalMethod = descriptor.value;
// eslint-disable-next-line , @typescript-eslint/no-explicit-any
descriptor.value = function (...args: any[]) {
if (!isTestExecution()) {
throw new Error(`Function: ${propertyKey} can only be called from test code`);
}
return originalMethod.apply(this, args);
};

return descriptor;
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,9 @@ export enum InteractiveWindowMessages {
GetCellCode = 'get_cell_code',
ReturnCellCode = 'return_cell_code',
GetAllCellCode = 'get_all_cell_code',
ReturnAllCellCode = 'return_all_cell_code'
ReturnAllCellCode = 'return_all_cell_code',
GetHTMLByIdRequest = 'get_html_by_id_request',
GetHTMLByIdResponse = 'get_html_by_id_response'
}

export enum IPyWidgetMessages {
Expand Down Expand Up @@ -715,4 +717,6 @@ export class IInteractiveWindowMapping {
public [InteractiveWindowMessages.HasCellResponse]: { id: string; result: boolean };
public [InteractiveWindowMessages.UpdateExternalCellButtons]: IExternalWebviewCellButton[];
public [InteractiveWindowMessages.ExecuteExternalCommand]: IExternalCommandFromWebview;
public [InteractiveWindowMessages.GetHTMLByIdRequest]: string;
public [InteractiveWindowMessages.GetHTMLByIdResponse]: string;
}
2 changes: 2 additions & 0 deletions src/client/datascience/interactive-common/synchronization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,8 @@ const messageWithMessageTypes: MessageMapping<IInteractiveWindowMapping> & Messa
[InteractiveWindowMessages.ConvertUriForUseInWebViewResponse]: MessageType.other,
[InteractiveWindowMessages.UpdateExternalCellButtons]: MessageType.other,
[InteractiveWindowMessages.ExecuteExternalCommand]: MessageType.other,
[InteractiveWindowMessages.GetHTMLByIdRequest]: MessageType.other,
[InteractiveWindowMessages.GetHTMLByIdResponse]: MessageType.other,
// Types from CssMessages
[CssMessages.GetCssRequest]: MessageType.other,
[CssMessages.GetCssResponse]: MessageType.other,
Expand Down
6 changes: 5 additions & 1 deletion src/client/datascience/variablesView/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ export class IVariableViewPanelMapping {
public [SharedMessages.LocInit]: string;
public [InteractiveWindowMessages.FinishCell]: IFinishCell;
public [InteractiveWindowMessages.UpdateVariableViewExecutionCount]: { executionCount: number };
public [InteractiveWindowMessages.GetHTMLByIdRequest]: string;
public [InteractiveWindowMessages.GetHTMLByIdResponse]: string;
}

export const INotebookWatcher = Symbol('INotebookWatcher');
Expand All @@ -37,4 +39,6 @@ export interface INotebookWatcher {
}

export const IVariableViewProvider = Symbol('IVariableViewProvider');
export interface IVariableViewProvider extends IVSCWebviewViewProvider {}
export interface IVariableViewProvider extends IVSCWebviewViewProvider {
//readonly activeVariableView: Promise<VariableView>;
}
24 changes: 24 additions & 0 deletions src/client/datascience/variablesView/variableViewProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
import { inject, injectable, named } from 'inversify';
import { CancellationToken, WebviewView, WebviewViewResolveContext } from 'vscode';
import { IApplicationShell, IWebviewViewProvider, IWorkspaceService } from '../../common/application/types';
import { isTestExecution } from '../../common/constants';
import { IConfigurationService, IDisposableRegistry } from '../../common/types';
import { createDeferred, Deferred } from '../../common/utils/async';
import { Identifiers } from '../constants';
import { IDataViewerFactory } from '../data-viewing/types';
import { ICodeCssGenerator, IJupyterVariableDataProviderFactory, IJupyterVariables, IThemeFinder } from '../types';
Expand All @@ -17,6 +19,23 @@ import { VariableView } from './variableView';
export class VariableViewProvider implements IVariableViewProvider {
public readonly viewType = 'jupyterViewVariables';

// Either return the active variable view or wait until it's created and return it
// @ts-ignore Property will be accessed in test code via casting to ITestVariableViewProviderInterface
private get activeVariableView(): Promise<VariableView> {
if (!isTestExecution()) {
throw new Error('activeVariableView only for test code');
}
// If we have already created the view, then just return it
if (this.variableView) {
return Promise.resolve(this.variableView);
}

// If not wait until created and then return
this.activeVariableViewPromise = createDeferred<VariableView>();
return this.activeVariableViewPromise.promise;
}
private activeVariableViewPromise?: Deferred<VariableView>;

private variableView?: VariableView;

constructor(
Expand Down Expand Up @@ -56,6 +75,11 @@ export class VariableViewProvider implements IVariableViewProvider {
this.notebookWatcher
);

// If someone is waiting for the variable view resolve that here
if (this.activeVariableViewPromise) {
this.activeVariableViewPromise.resolve(this.variableView);
}

await this.variableView.load(webviewView);
}
}
57 changes: 57 additions & 0 deletions src/client/datascience/webviews/webviewHost.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ import * as localize from '../../common/utils/localize';
import { StopWatch } from '../../common/utils/stopWatch';
import { captureTelemetry, sendTelemetryEvent } from '../../telemetry';
import { DefaultTheme, PythonExtension, Telemetry } from '../constants';
import { InteractiveWindowMessages } from '../interactive-common/interactiveWindowTypes';
import { CssMessages, IGetCssRequest, IGetMonacoThemeRequest, SharedMessages } from '../messages';
import { ICodeCssGenerator, IJupyterExtraSettings, IThemeFinder } from '../types';
import { testOnlyMethod } from '../../common/utils/decorators';

@injectable() // For some reason this is necessary to get the class hierarchy to work.
export abstract class WebviewHost<IMapping> implements IDisposable {
Expand All @@ -42,6 +44,14 @@ export abstract class WebviewHost<IMapping> implements IDisposable {

protected readonly _disposables: IDisposable[] = [];
private startupStopwatch = new StopWatch();

// For testing, holds the current request for webview HTML
private activeHTMLRequest?: Deferred<string>;

// For testing, broadcast messages to the following listeners
// tslint:disable-next-line:no-any
private onMessageListeners: ((message: string, payload: any) => void)[] = [];

constructor(
@unmanaged() protected configService: IConfigurationService,
@unmanaged() private cssGenerator: ICodeCssGenerator,
Expand Down Expand Up @@ -79,6 +89,40 @@ export abstract class WebviewHost<IMapping> implements IDisposable {
}
}

// This function is used for testing webview by fetching HTML from the webview via a message
// @ts-ignore Property will be accessed in test code via casting to ITestWebviewHost
@testOnlyMethod()
// @ts-ignore Property will be accessed in test code via casting to ITestWebviewHost
private getHTMLById(id: string): Promise<string> {
if (!this.activeHTMLRequest) {
this.activeHTMLRequest = createDeferred<string>();
this.postMessageInternal(InteractiveWindowMessages.GetHTMLByIdRequest, id).ignoreErrors();
} else {
throw new Error('getHTMLById request already in progress');
}

return this.activeHTMLRequest.promise;
}

// For testing add a callback listening to messages from the webview
// tslint:disable-next-line:no-any
@testOnlyMethod()
// @ts-ignore Property will be accessed in test code via casting to ITestWebviewHost
private addMessageListener(callback: (message: string, payload: any) => void) {
this.onMessageListeners.push(callback);
}

// For testing remove a callback listening to messages from the webview
// tslint:disable-next-line:no-any
@testOnlyMethod()
// @ts-ignore Property will be accessed in test code via casting to ITestWebviewHost
private removeMessageListener(callback: (message: string, payload: any) => void) {
const index = this.onMessageListeners.indexOf(callback);
if (index >= 0) {
this.onMessageListeners.splice(index, 1);
}
}

protected abstract provideWebview(
cwd: string,
settings: IJupyterExtraSettings,
Expand Down Expand Up @@ -120,9 +164,22 @@ export abstract class WebviewHost<IMapping> implements IDisposable {
this.handleMonacoThemeRequest(payload as IGetMonacoThemeRequest).ignoreErrors();
break;

case InteractiveWindowMessages.GetHTMLByIdResponse:
// Webview has returned HTML, resolve the request and clear it
if (this.activeHTMLRequest) {
this.activeHTMLRequest.resolve(payload);
this.activeHTMLRequest = undefined;
}
break;

default:
break;
}

// Broadcast to any onMessage listeners
this.onMessageListeners.forEach((listener) => {
listener(message, payload);
});
}

protected async loadWebview(cwd: string, webView?: vscodeWebviewPanel | vscodeWebviewView) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -318,4 +318,14 @@ export namespace CommonEffects {
externalButtons: arg.payload.data
};
}

// Extension has requested HTML for the webview, get it by ID and send it back as a message
export function getHTMLByIdRequest(arg: CommonReducerArg<CommonActionType, string>): IMainState {
const element = document.getElementById(arg.payload.data);

if (element) {
postActionToExtension(arg, InteractiveWindowMessages.GetHTMLByIdResponse, element.innerHTML);
}
return arg.prevState;
}
}
9 changes: 8 additions & 1 deletion src/datascience-ui/interactive-common/redux/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ import { generateMonacoReducer, IMonacoState } from './reducers/monaco';
import { CommonActionType } from './reducers/types';
import { generateVariableReducer, IVariableState } from './reducers/variables';

// Externally defined function to see if we need to force on test middleware
export declare function forceTestMiddleware(): boolean;

function generateDefaultState(
skipDefault: boolean,
testMode: boolean,
Expand Down Expand Up @@ -306,7 +309,11 @@ function createMiddleWare(testMode: boolean, postOffice: PostOffice): Redux.Midd
// Or if testing in UI Test.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const isUITest = (postOffice.acquireApi() as any)?.handleMessage ? true : false;
const testMiddleware = testMode || isUITest ? createTestMiddleware() : undefined;
let forceOnTestMiddleware = false;
if (typeof forceTestMiddleware !== 'undefined') {
forceOnTestMiddleware = forceTestMiddleware();
}
const testMiddleware = forceOnTestMiddleware || testMode || isUITest ? createTestMiddleware() : undefined;

// Create the logger if we're not in production mode or we're forcing logging
const reduceLogMessage = '<payload too large to displayed in logs (at least on CI)>';
Expand Down
3 changes: 2 additions & 1 deletion src/datascience-ui/variable-view/redux/reducers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,6 @@ export const reducerMap: Partial<IVariableViewPanelActionMapping> = {
[CssMessages.GetCssResponse]: CommonEffects.handleCss,
[SharedMessages.UpdateSettings]: Effects.updateSettings,
[SharedMessages.LocInit]: CommonEffects.handleLocInit,
[CommonActionType.VARIABLE_VIEW_LOADED]: Transfer.variableViewStarted
[CommonActionType.VARIABLE_VIEW_LOADED]: Transfer.variableViewStarted,
[InteractiveWindowMessages.GetHTMLByIdRequest]: CommonEffects.getHTMLByIdRequest
};
12 changes: 12 additions & 0 deletions src/test/datascience/testInterfaces.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
'use strict';

// Interfaces here to expose specific private functionality to test code
export interface ITestWebviewHost {
getHTMLById(id: string): Promise<string>;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
addMessageListener(callback: (message: string, payload: any) => void): void;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
removeMessageListener(callback: (message: string, payload: any) => void): void;
}
Loading