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

Enable debugging IW in vscode.web and add tests for IW on web #10297

Merged
merged 19 commits into from
Jun 3, 2022
4 changes: 2 additions & 2 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@
"CI_PYTHON_PATH": "<Python Path>",
"VSC_JUPYTER_PERF_TEST": "1",
"TEST_FILES_SUFFIX": "notebookCellExecution.perf.test",
"VSC_JUPYTER_TEST_TIMEOUT": "60000",
"VSC_JUPYTER_TEST_TIMEOUT": "60000"
},
"sourceMaps": true,
"outFiles": ["${workspaceFolder}/out/**/*.js", "!${workspaceFolder}/**/node_modules**/*"],
Expand Down Expand Up @@ -206,7 +206,7 @@
"VSC_JUPYTER_WEBVIEW_TEST_MIDDLEWARE": "true", // Initialize to create the webview test middleware
"VSC_JUPYTER_LOAD_EXPERIMENTS_FROM_FILE": "true",
// "TF_BUILD": "", // Set to anything to force full logging
"TEST_FILES_SUFFIX": "vscode.test",
"TEST_FILES_SUFFIX": "*.vscode.test,*.vscode.common.test",
"VSC_JUPYTER_REMOTE_NATIVE_TEST": "false", // Change to `true` to run the Native Notebook tests with remote jupyter connections.
"VSC_JUPYTER_NON_RAW_NATIVE_TEST": "false", // Change to `true` to run the Native Notebook tests with non-raw kernels (i.e. local jupyter server).
"XVSC_JUPYTER_INSTRUMENT_CODE_FOR_COVERAGE": "1",
Expand Down
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2125,11 +2125,11 @@
"preTestJediLSP": "node ./out/test/languageServers/jedi/lspSetup.js",
"testJediLSP": "node ./out/test/languageServers/jedi/lspSetup.js && cross-env CODE_TESTS_WORKSPACE=src/test VSC_JUPYTER_CI_TEST_GREP='Language Server:' node ./out/test/testBootstrap.node.js ./out/test/standardTest.node.js && node ./out/test/languageServers/jedi/lspTeardown.node.js",
"pretestNativeNotebooksInVSCode": "cross-env VSC_JUPYTER_CI_TEST_VSC_CHANNEL=insiders node ./out/test/datascience/dsTestSetup.js",
"testNativeNotebooksInVSCode": "cross-env CODE_TESTS_WORKSPACE=src/test/datascience VSC_JUPYTER_CI_TEST_VSC_CHANNEL=insiders TEST_FILES_SUFFIX=vscode.test* VSC_JUPYTER_FORCE_LOGGING=1 VSC_JUPYTER_LOAD_EXPERIMENTS_FROM_FILE=true node ./out/test/testBootstrap.node.js ./out/test/standardTest.node.js",
"testNativeNotebooksInVSCode": "cross-env CODE_TESTS_WORKSPACE=src/test/datascience VSC_JUPYTER_CI_TEST_VSC_CHANNEL=insiders TEST_FILES_SUFFIX=*.vscode.test,*.vscode.common.test VSC_JUPYTER_FORCE_LOGGING=1 VSC_JUPYTER_LOAD_EXPERIMENTS_FROM_FILE=true node ./out/test/testBootstrap.node.js ./out/test/standardTest.node.js",
"testNativeNotebooksInVSCodeWithoutTestSuffix": "cross-env CODE_TESTS_WORKSPACE=src/test/datascience VSC_JUPYTER_CI_TEST_VSC_CHANNEL=insiders VSC_JUPYTER_FORCE_LOGGING=1 VSC_JUPYTER_LOAD_EXPERIMENTS_FROM_FILE=true node ./out/test/testBootstrap.node.js ./out/test/standardTest.node.js",
"pretestNativeNotebooksWithoutPythonInVSCode": "cross-env VSC_JUPYTER_CI_TEST_VSC_CHANNEL=insiders node ./out/test/datascience/dsTestSetup.js",
"testNativeNotebooksWithoutPythonInVSCode": "cross-env CODE_TESTS_WORKSPACE=src/test/datascience VSC_JUPYTER_CI_TEST_VSC_CHANNEL=insiders TEST_FILES_SUFFIX=vscode.test* VSC_JUPYTER_FORCE_LOGGING=1 VSC_JUPYTER_LOAD_EXPERIMENTS_FROM_FILE=true VSC_JUPYTER_CI_TEST_GREP=non-python-kernel VSC_JUPYTER_CI_TEST_DO_NOT_INSTALL_PYTHON_EXT=true node ./out/test/testBootstrap.node.js ./out/test/standardTest.node.js",
"testNativeNotebooksAndWebviews": "cross-env CODE_TESTS_WORKSPACE=src/test/datascience VSC_JUPYTER_CI_TEST_VSC_CHANNEL=insiders TEST_FILES_SUFFIX=vscode.test* VSC_JUPYTER_FORCE_LOGGING=1 VSC_JUPYTER_CI_TEST_GREP=webview-test VSC_JUPYTER_LOAD_EXPERIMENTS_FROM_FILE=true node ./out/test/testBootstrap.node.js ./out/test/standardTest.node.js",
"testNativeNotebooksWithoutPythonInVSCode": "cross-env CODE_TESTS_WORKSPACE=src/test/datascience VSC_JUPYTER_CI_TEST_VSC_CHANNEL=insiders TEST_FILES_SUFFIX=*.vscode.test,*.vscode.common.test VSC_JUPYTER_FORCE_LOGGING=1 VSC_JUPYTER_LOAD_EXPERIMENTS_FROM_FILE=true VSC_JUPYTER_CI_TEST_GREP=non-python-kernel VSC_JUPYTER_CI_TEST_DO_NOT_INSTALL_PYTHON_EXT=true node ./out/test/testBootstrap.node.js ./out/test/standardTest.node.js",
"testNativeNotebooksAndWebviews": "cross-env CODE_TESTS_WORKSPACE=src/test/datascience VSC_JUPYTER_CI_TEST_VSC_CHANNEL=insiders TEST_FILES_SUFFIX=*.vscode.test,*.vscode.common.test VSC_JUPYTER_FORCE_LOGGING=1 VSC_JUPYTER_CI_TEST_GREP=webview-test VSC_JUPYTER_LOAD_EXPERIMENTS_FROM_FILE=true node ./out/test/testBootstrap.node.js ./out/test/standardTest.node.js",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New pattern to run *.vscode.test.ts and *.common.test.ts on desktop.
Unfortunately i cannot use a real glob patter like (*.vscode.test|*.common.test) as this breaks the npm script. Using ( and | in the npm scripts don't work.

"testWebExtension": "node ./build/launchWebTest.js",
"testPerformance": "node ./out/test/perfTest.node.js",
"testSmoke": "node ./out/test/testBootstrap.node.js ./out/test/smokeTest.node.js",
Expand Down
4 changes: 2 additions & 2 deletions pvsc.code-workspace
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@
"${workspaceFolder:vscode-jupyter}/out/**/*.js",
"!${workspaceFolder:vscode-jupyter}/**/node_modules**/*",
"${workspaceFolder:vscode-jupyter-powertoys}/out/**/*.js",
"!${workspaceFolder:vscode-jupyter-powertoys}/**/node_modules**/*",
"!${workspaceFolder:vscode-jupyter-powertoys}/**/node_modules**/*"
],
"skipFiles": ["<node_internals>/**"]
},
Expand Down Expand Up @@ -162,7 +162,7 @@
"VSC_JUPYTER_CI_RUN_NON_PYTHON_NB_TEST": "", // Initialize this to run tests again Julia & other kernels.
"VSC_JUPYTER_WEBVIEW_TEST_MIDDLEWARE": "true", // Initialize to create the webview test middleware
"VSC_JUPYTER_LOAD_EXPERIMENTS_FROM_FILE": "true",
"TEST_FILES_SUFFIX": "vscode.test*",
"TEST_FILES_SUFFIX": "*.vscode.test,*.vscode.common.test",
"VSC_JUPYTER_REMOTE_NATIVE_TEST": "false", // Change to `true` to run the Native Notebook tests with remote jupyter connections.
"VSC_JUPYTER_NON_RAW_NATIVE_TEST": "false", // Change to `true` to run the Native Notebook tests with non-raw kernels (i.e. local jupyter server).
"XVSC_JUPYTER_INSTRUMENT_CODE_FOR_COVERAGE": "1",
Expand Down
15 changes: 2 additions & 13 deletions src/interactive-window/editor-integration/codeLensFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,7 @@ import {
import { IDocumentManager, IVSCodeNotebook, IWorkspaceService } from '../../platform/common/application/types';
import { traceWarning, traceInfoIfCI } from '../../platform/logging';

import {
ICellRange,
IConfigurationService,
IDisposableRegistry,
IsWebExtension,
Resource
} from '../../platform/common/types';
import { ICellRange, IConfigurationService, IDisposableRegistry, Resource } from '../../platform/common/types';
import * as localize from '../../platform/common/utils/localize';
import { getInteractiveCellMetadata } from '../helpers';
import { IKernelProvider } from '../../kernels/types';
Expand Down Expand Up @@ -63,8 +57,7 @@ export class CodeLensFactory implements ICodeLensFactory {
@inject(IDisposableRegistry) disposables: IDisposableRegistry,
@inject(IGeneratedCodeStorageFactory)
private readonly generatedCodeStorageFactory: IGeneratedCodeStorageFactory,
@inject(IKernelProvider) kernelProvider: IKernelProvider,
@inject(IsWebExtension) private readonly isWebExtension: boolean
@inject(IKernelProvider) kernelProvider: IKernelProvider
) {
this.documentManager.onDidCloseTextDocument(this.onClosedDocument, this, disposables);
this.workspace.onDidGrantWorkspaceTrust(() => this.codeLensCache.clear(), this, disposables);
Expand Down Expand Up @@ -265,13 +258,9 @@ export class CodeLensFactory implements ICodeLensFactory {
Commands.RunCellAndAllBelowPalette
];
}
if (this.isWebExtension) {
commandsToBeDisabled.push(Commands.DebugCell);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Debugging cells in IW web is now enabled.

}
if (commandsToBeDisabled) {
fullCommandList = fullCommandList.filter((item) => !commandsToBeDisabled.includes(item));
}

return fullCommandList;
}

Expand Down
6 changes: 3 additions & 3 deletions src/interactive-window/editor-integration/codelensprovider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ import { traceInfoIfCI } from '../../platform/logging';
import {
CodeLensCommands,
EditorContexts,
PYTHON_FILE,
PYTHON_UNTITLED,
InteractiveInputScheme,
NotebookCellScheme,
Telemetry
} from '../../platform/common/constants';
import { IDataScienceCodeLensProvider, ICodeWatcher } from './types';
Expand Down Expand Up @@ -78,7 +78,7 @@ export class DataScienceCodeLensProvider implements IDataScienceCodeLensProvider
// CodeLensProvider interface
// Some implementation based on DonJayamanne's jupyter extension work
public provideCodeLenses(document: vscode.TextDocument, _token: vscode.CancellationToken): vscode.CodeLens[] {
if (document.uri.scheme != PYTHON_FILE.scheme && document.uri.scheme !== PYTHON_UNTITLED.scheme) {
if ([NotebookCellScheme, InteractiveInputScheme].includes(document.uri.scheme)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Running cells doesn't work in IW on the web unless you open a local folder, i.e. if you open a github repo or any other file system it will not work, hence now enabling it for all except notebook cells & IW input cell.

return [];
}
// Get the list of code lens for this document.
Expand Down
2 changes: 1 addition & 1 deletion src/kernels/raw/launcher/kernelProcess.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ export class KernelProcess implements IKernelProcess {
tcpPortUsed.waitUntilUsed(this.connection.shell_port, 200, timeout),
tcpPortUsed.waitUntilUsed(this.connection.iopub_port, 200, timeout)
]).catch((ex) => {
if (cancelToken.isCancellationRequested) {
if (cancelToken.isCancellationRequested || deferred.rejected) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No need to log errors if the other promise failed
This ensures we don't log errors unnecessarily (else we end up with false positives in the CI logs)

return;
}
traceError(`waitUntilUsed timed out`, ex);
Expand Down
1 change: 1 addition & 0 deletions src/platform/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export const JUPYTER_LANGUAGE = 'jupyter';
export const NotebookCellScheme = 'vscode-notebook-cell';
export const PYTHON_UNTITLED = { scheme: 'untitled', language: PYTHON_LANGUAGE };
export const PYTHON_FILE = { scheme: 'file', language: PYTHON_LANGUAGE };
export const PYTHON_FILE_ANY_SCHEME = { language: PYTHON_LANGUAGE };
export const PYTHON_CELL = { scheme: NotebookCellScheme, language: PYTHON_LANGUAGE };
export const PYTHON = [PYTHON_UNTITLED, PYTHON_FILE, PYTHON_CELL];
export const PYTHON_ALLFILES = [{ language: PYTHON_LANGUAGE }];
Expand Down
7 changes: 2 additions & 5 deletions src/platform/common/globalActivation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type { JSONObject } from '@lumino/coreutils';
import { inject, injectable, multiInject, optional } from 'inversify';
import * as vscode from 'vscode';
import { ICommandManager, IDocumentManager, IWorkspaceService } from './application/types';
import { PYTHON_FILE, PYTHON_LANGUAGE, PYTHON_UNTITLED } from './constants';
import { PYTHON_FILE_ANY_SCHEME, PYTHON_LANGUAGE } from './constants';
import { ContextKey } from './contextKey';
import './extensions';
import {
Expand Down Expand Up @@ -53,10 +53,7 @@ export class GlobalActivation implements IExtensionSingleActivationService {
public async activate(): Promise<void> {
if (this.dataScienceCodeLensProvider) {
this.extensionContext.subscriptions.push(
vscode.languages.registerCodeLensProvider(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Running cells doesn't work in IW on the web unless you open a local folder, i.e. if you open a github repo or any other file system it will not work, hence now enabling it for all except notebook cells & IW input cell.

[PYTHON_FILE, PYTHON_UNTITLED],
this.dataScienceCodeLensProvider
)
vscode.languages.registerCodeLensProvider([PYTHON_FILE_ANY_SCHEME], this.dataScienceCodeLensProvider)
);
}

Expand Down
54 changes: 52 additions & 2 deletions src/test/common.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,25 @@
import * as assert from 'assert';
import * as fs from 'fs-extra';
import * as path from '../platform/vscode-path/path';
import * as tmp from 'tmp';
import * as uuid from 'uuid/v4';
import { coerce, SemVer } from 'semver';
import type { ConfigurationTarget, TextDocument, Uri } from 'vscode';
import { IProcessService } from '../platform/common/process/types.node';
import { EXTENSION_ROOT_DIR_FOR_TESTS, IS_MULTI_ROOT_TEST, IS_PERF_TEST, IS_SMOKE_TEST } from './constants.node';
import {
EXTENSION_ROOT_DIR_FOR_TESTS,
IS_MULTI_ROOT_TEST,
IS_PERF_TEST,
IS_REMOTE_NATIVE_TEST,
IS_SMOKE_TEST
} from './constants.node';
import { noop } from './core';
import { isCI } from '../platform/common/constants';
import { IWorkspaceService } from '../platform/common/application/types';
import { initializeCommonApi } from './common';
import { IDisposable } from '../platform/common/types';
import { swallowExceptions } from '../platform/common/utils/misc';
import { JupyterServer } from './datascience/jupyterServer.node';
import type { ConfigurationTarget, NotebookDocument, TextDocument, Uri } from 'vscode';

export { createEventHandler } from './common';

Expand Down Expand Up @@ -320,3 +331,42 @@ export async function captureScreenShot(fileNamePrefix: string) {
console.error(`Failed to capture screenshot into ${filename}`, ex);
}
}

export function initializeCommonNodeApi() {
const { commands, Uri } = require('vscode');
const { initialize } = require('./initialize.node');

initializeCommonApi({
async createTemporaryFile(options: {
contents?: string;
extension: string;
}): Promise<{ file: Uri } & IDisposable> {
const extension = options.extension || '.py';
const tempFile = tmp.tmpNameSync({ postfix: extension });
if (options.contents) {
await fs.writeFile(tempFile, options.contents);
}
return { file: Uri.file(tempFile), dispose: () => swallowExceptions(() => fs.unlinkSync(tempFile)) };
},
async startJupyterServer(notebook?: NotebookDocument, useCert: boolean = false): Promise<any> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We end up duplicating the test files because some functions like StartJupyter, createTempFile are specific to web vs desktop.
Created a common layer so that we call the common function and inject the actual implementation during runtime.
This way we don't need to duplicate files or the like. I.e. we can write some code that will run in all places (web & desktop tests)

if (IS_REMOTE_NATIVE_TEST()) {
const uriString = useCert
? await JupyterServer.instance.startJupyterWithCert()
: await JupyterServer.instance.startJupyterWithToken();
console.info(`Jupyter started and listening at ${uriString}`);
return commands.executeCommand('jupyter.selectjupyteruri', false, Uri.parse(uriString), notebook);
} else {
console.info(`Jupyter not started and set to local`); // This is the default
}
},
async stopJupyterServer() {
if (IS_REMOTE_NATIVE_TEST()) {
return;
}
await JupyterServer.instance.dispose().catch(noop);
},
async initialize() {
return initialize();
}
});
}
44 changes: 43 additions & 1 deletion src/test/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// Licensed under the MIT License.
'use strict';

import { Event } from 'vscode';
import { NotebookDocument, Uri, Event } from 'vscode';
import { IExtensionApi } from '../platform/api';
import { IDisposable } from '../platform/common/types';
import { IServiceContainer, IServiceManager } from '../platform/ioc/types';
Expand Down Expand Up @@ -162,3 +162,45 @@ export function createEventHandler<T, K extends keyof T>(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
return new TestEventHandler(obj[eventName] as any, eventName as string, disposables) as any;
}
/**
* API common to web & desktop tests, but with different implementations
*/
export type CommonApi = {
createTemporaryFile(options: { extension: string; contents?: string }): Promise<{ file: Uri } & IDisposable>;
startJupyterServer(notebook?: NotebookDocument, useCert?: boolean): Promise<void>;
stopJupyterServer?(): Promise<void>;
captureScreenShot?(fileNamePrefix: string): Promise<void>;
initialize(): Promise<IExtensionTestApi>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are the functions that are different on each platform, and if we had a common function for them, then we won't have to create two 3 seprate files for the tests, now that we have a common layer for these functions, we have just one single file (tests that run on web & desktop)

};
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const API: CommonApi = {} as any;

export function initializeCommonApi(api: CommonApi) {
Object.assign(API, api);
}

export async function createTemporaryFile(options: {
contents?: string;
extension: string;
}): Promise<{ file: Uri } & IDisposable> {
return API.createTemporaryFile(options);
}

export async function startJupyterServer(notebook?: NotebookDocument, useCert: boolean = false): Promise<void> {
return API.startJupyterServer(notebook, useCert);
}

export async function stopJupyterServer() {
if (API.stopJupyterServer) {
return API.stopJupyterServer();
}
}
export async function captureScreenShot(fileNamePrefix: string) {
if (API.captureScreenShot) {
await API.captureScreenShot(fileNamePrefix);
}
}

export async function initialize() {
return API.initialize();
}
48 changes: 48 additions & 0 deletions src/test/common.web.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { commands, NotebookDocument, Uri, workspace } from 'vscode';
import { IDisposable } from '../platform/common/types';
import { initializeCommonApi } from './common';
import { JUPYTER_SERVER_URI } from './constants';
import * as uuid from 'uuid/v4';
import { noop } from './core';
import { initialize } from './initialize';

export function initializeCommonWebApi() {
initializeCommonApi({
async createTemporaryFile(options: {
contents?: string;
extension: string;
}): Promise<{ file: Uri } & IDisposable> {
const folder = workspace.workspaceFolders![0].uri;
const tmpDir = Uri.joinPath(folder, 'temp', 'testFiles');
try {
await workspace.fs.createDirectory(tmpDir);
} catch {
// ignore if it exists..
}
const extension = options.extension || '.py';
const file = Uri.joinPath(tmpDir, `${uuid()}${extension}`);
const contents = options.contents || '';

await workspace.fs.writeFile(file, Buffer.from(contents));
return {
file,
dispose: () => {
workspace.fs.delete(file).then(noop, noop);
}
};
},
async startJupyterServer(notebook?: NotebookDocument): Promise<void> {
// Server URI should have been embedded in the constants file
const uri = Uri.parse(JUPYTER_SERVER_URI);
console.log(`ServerURI for remote test: ${JUPYTER_SERVER_URI}`);
// Use this URI to set our jupyter server URI
await commands.executeCommand('jupyter.selectjupyteruri', false, uri, notebook);
},
async initialize() {
return initialize();
}
});
}
2 changes: 0 additions & 2 deletions src/test/constants.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ export const SMOKE_TEST_EXTENSIONS_DIR = path.join(
'smokeTestExtensionsFolder'
);

export const IPYTHON_VERSION_CODE = 'import IPython\nprint(int(IPython.__version__[0]))\n';

// Have to set these values in a '.node' based file.
setCI(process.env.TF_BUILD !== undefined || process.env.GITHUB_ACTIONS === 'true');
setTestExecution(process.env.VSC_JUPYTER_CI_TEST === '1');
Expand Down
2 changes: 2 additions & 0 deletions src/test/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,5 @@ function isMultirootTest() {
export function setTestSettings(newSettings: Partial<TestSettingsType>) {
testSettings = { ...testSettings, ...newSettings };
}

export const IPYTHON_VERSION_CODE = 'import IPython\nprint(int(IPython.__version__[0]))\n';
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,7 @@ suite('DataScience Code Watcher Unit Tests', () => {
instance(notebook),
disposables,
instance(storageFactory),
instance(kernelProvider),
false
instance(kernelProvider)
);
serviceContainer
.setup((c) => c.get(TypeMoq.It.isValue(ICodeWatcher)))
Expand Down
Loading