Mock jupyter apis such that they can be faked to have tests run more reliably (microsoft#3853)

* Implement jupyter session wrapping * Fix remote test to not leave a dangling session * Fix dispose problems in functional tests * Add news entry * Fix guid regex

Author: rchiodo

news/3 Code Health/3556.md

@@ -0,0 +1 @@
+Add mock of Jupyter API to allow functional tests to run more quickly and more consistently.

src/client/datascience/editor-integration/codewatcher.ts

@@ -10,7 +10,7 @@ import * as localize from '../../common/utils/localize';
 import { captureTelemetry } from '../../telemetry';
 import { generateCellRanges } from '../cellFactory';
 import { Commands, Telemetry } from '../constants';
-import { JupyterInstallError } from '../jupyterInstallError';
+import { JupyterInstallError } from '../jupyter/jupyterInstallError';
 import { ICodeWatcher, IHistoryProvider } from '../types';
 
 @injectable()

src/client/datascience/history.ts

@@ -25,11 +25,12 @@ import { EXTENSION_ROOT_DIR } from '../common/constants';
 import { ContextKey } from '../common/contextKey';
 import { IFileSystem } from '../common/platform/types';
 import { IConfigurationService, IDisposableRegistry, ILogger } from '../common/types';
+import { createDeferred } from '../common/utils/async';
 import * as localize from '../common/utils/localize';
 import { IInterpreterService } from '../interpreter/contracts';
 import { captureTelemetry, sendTelemetryEvent } from '../telemetry';
 import { EditorContexts, HistoryMessages, Settings, Telemetry } from './constants';
-import { JupyterInstallError } from './jupyterInstallError';
+import { JupyterInstallError } from './jupyter/jupyterInstallError';
 import {
  CellState,
  ICell,
@@ -112,6 +113,14 @@ export class History implements IWebPanelMessageListener, IHistory {
  // Start a status item
  const status = this.setStatus(localize.DataScience.executingCode());
 
+ // Create a deferred object that will wait until the status is disposed
+ const finishedAddingCode = createDeferred<void>();
+ const actualDispose = status.dispose;
+ status.dispose = () => {
+ finishedAddingCode.resolve();
+ actualDispose();
+ };
+
  try {
 
  // Make sure we're loaded first.
@@ -151,6 +160,9 @@ export class History implements IWebPanelMessageListener, IHistory {
  // Indicate executing until this cell is done.
  status.dispose();
  });
+
+ // Wait for the cell to finish
+ await finishedAddingCode.promise;
  }
  } catch (err) {
  status.dispose();

src/client/datascience/historycommandlistener.ts

@@ -141,13 +141,14 @@ export class HistoryCommandListener implements IDataScienceCommandListener {
 
  // When all done, show a notice that it completed.
  const openQuestion = localize.DataScience.exportOpenQuestion();
- this.applicationShell.showInformationMessage(localize.DataScience.exportDialogComplete().format(uri.fsPath), openQuestion).then((str: string | undefined) => {
- if (str === openQuestion) {
- // If the user wants to, open the notebook they just generated.
- this.jupyterExecution.spawnNotebook(uri.fsPath).ignoreErrors();
- }
- });
-
+ if (uri && uri.fsPath) {
+ this.applicationShell.showInformationMessage(localize.DataScience.exportDialogComplete().format(uri.fsPath), openQuestion).then((str: string | undefined) => {
+ if (str === openQuestion) {
+ // If the user wants to, open the notebook they just generated.
+ this.jupyterExecution.spawnNotebook(uri.fsPath).ignoreErrors();
+ }
+ });
+ }
  }
  }
  }

src/client/datascience/jupyterConnectError.ts renamed to src/client/datascience/jupyter/jupyterConnectError.ts

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 'use strict';
-import '../common/extensions';
+import '../../common/extensions';
 
 export class JupyterConnectError extends Error {
  constructor(message: string, stderr?: string) {

src/client/datascience/jupyterConnection.ts renamed to src/client/datascience/jupyter/jupyterConnection.ts

@@ -4,15 +4,15 @@
 import * as path from 'path';
 import { CancellationToken, Disposable } from 'vscode-jsonrpc';
 
-import { CancellationError } from '../common/cancellation';
-import { IFileSystem } from '../common/platform/types';
-import { ObservableExecutionResult, Output } from '../common/process/types';
-import { IConfigurationService, ILogger } from '../common/types';
-import { createDeferred, Deferred } from '../common/utils/async';
-import * as localize from '../common/utils/localize';
-import { IServiceContainer } from '../ioc/types';
+import { CancellationError } from '../../common/cancellation';
+import { IFileSystem } from '../../common/platform/types';
+import { ObservableExecutionResult, Output } from '../../common/process/types';
+import { IConfigurationService, ILogger } from '../../common/types';
+import { createDeferred, Deferred } from '../../common/utils/async';
+import * as localize from '../../common/utils/localize';
+import { IServiceContainer } from '../../ioc/types';
 import { JupyterConnectError } from './jupyterConnectError';
-import { IConnection } from './types';
+import { IConnection } from '../types';
 
 const UrlPatternRegEx = /(https?:\/\/[^\s]+)/ ;
 const HttpPattern = /https?:\/\//;

src/client/datascience/jupyterExecution.ts renamed to src/client/datascience/jupyter/jupyterExecution.ts

@@ -10,33 +10,34 @@ import { URL } from 'url';
 import * as uuid from 'uuid/v4';
 import { CancellationToken, Disposable } from 'vscode-jsonrpc';
 
-import { Cancellation, CancellationError } from '../common/cancellation';
-import { IS_WINDOWS } from '../common/platform/constants';
-import { IFileSystem, TemporaryDirectory } from '../common/platform/types';
+import { Cancellation, CancellationError } from '../../common/cancellation';
+import { IS_WINDOWS } from '../../common/platform/constants';
+import { IFileSystem, TemporaryDirectory } from '../../common/platform/types';
 import {
  ExecutionResult,
  IProcessService,
  IProcessServiceFactory,
  IPythonExecutionFactory,
  ObservableExecutionResult,
  SpawnOptions
-} from '../common/process/types';
-import { IAsyncDisposableRegistry, IDisposableRegistry, ILogger } from '../common/types';
-import * as localize from '../common/utils/localize';
-import { noop } from '../common/utils/misc';
-import { EXTENSION_ROOT_DIR } from '../constants';
+} from '../../common/process/types';
+import { IAsyncDisposableRegistry, IDisposableRegistry, ILogger } from '../../common/types';
+import * as localize from '../../common/utils/localize';
+import { noop } from '../../common/utils/misc';
+import { EXTENSION_ROOT_DIR } from '../../constants';
 import {
  ICondaService,
  IInterpreterService,
  IKnownSearchPathsForInterpreters,
  InterpreterType,
  PythonInterpreter
-} from '../interpreter/contracts';
-import { IServiceContainer } from '../ioc/types';
-import { captureTelemetry } from '../telemetry';
-import { Telemetry } from './constants';
+} from '../../interpreter/contracts';
+import { IServiceContainer } from '../../ioc/types';
+import { captureTelemetry } from '../../telemetry';
+import { Telemetry } from '../constants';
 import { JupyterConnection, JupyterServerInfo } from './jupyterConnection';
-import { IConnection, IJupyterExecution, IJupyterKernelSpec, INotebookServer } from './types';
+import { IConnection, IJupyterExecution, IJupyterKernelSpec, INotebookServer, IJupyterSessionManager } from '../types';
+import { JupyterKernelSpec } from './jupyterKernelSpec';
 
 const CheckJupyterRegEx = IS_WINDOWS ? /^jupyter?\.exe$/ : /^jupyter?$/;
 const NotebookCommand = 'notebook';
@@ -45,33 +46,6 @@ const KernelSpecCommand = 'kernelspec';
 const KernelCreateCommand = 'ipykernel';
 const PyKernelOutputRegEx = /.*\s+(.+)$/m;
 const KernelSpecOutputRegEx = /^\s*(\S+)\s+(\S+)$/;
-const IsGuidRegEx = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;
-
-class JupyterKernelSpec implements IJupyterKernelSpec {
- public name: string;
- public language: string;
- public path: string;
- public specFile: string | undefined;
- constructor(specModel: Kernel.ISpecModel, file?: string) {
- this.name = specModel.name;
- this.language = specModel.language;
- this.path = specModel.argv.length > 0 ? specModel.argv[0] : '';
- this.specFile = file;
- }
- public dispose = async () => {
- if (this.specFile &&
- IsGuidRegEx.test(path.basename(path.dirname(this.specFile)))) {
- // There is more than one location for the spec file directory
- // to be cleaned up. If one fails, the other likely deleted it already.
- try {
- await fs.remove(path.dirname(this.specFile));
- } catch {
- noop();
- }
- this.specFile = undefined;
- }
- }
-}
 
 // JupyterCommand objects represent some process that can be launched that should be guaranteed to work because it
 // was found by testing it previously
@@ -155,21 +129,22 @@ export class JupyterExecution implements IJupyterExecution, Disposable {
  private usablePythonInterpreter: PythonInterpreter | undefined;
 
  constructor(@inject(IPythonExecutionFactory) private executionFactory: IPythonExecutionFactory,
- @inject(ICondaService) private condaService: ICondaService,
- @inject(IInterpreterService) private interpreterService: IInterpreterService,
- @inject(IProcessServiceFactory) private processServiceFactory: IProcessServiceFactory,
- @inject(IKnownSearchPathsForInterpreters) private knownSearchPaths: IKnownSearchPathsForInterpreters,
- @inject(ILogger) private logger: ILogger,
- @inject(IDisposableRegistry) private disposableRegistry: IDisposableRegistry,
- @inject(IAsyncDisposableRegistry) private asyncRegistry: IAsyncDisposableRegistry,
- @inject(IFileSystem) private fileSystem: IFileSystem,
- @inject(IServiceContainer) private serviceContainer: IServiceContainer) {
+ @inject(ICondaService) private condaService: ICondaService,
+ @inject(IInterpreterService) private interpreterService: IInterpreterService,
+ @inject(IProcessServiceFactory) private processServiceFactory: IProcessServiceFactory,
+ @inject(IKnownSearchPathsForInterpreters) private knownSearchPaths: IKnownSearchPathsForInterpreters,
+ @inject(ILogger) private logger: ILogger,
+ @inject(IDisposableRegistry) private disposableRegistry: IDisposableRegistry,
+ @inject(IAsyncDisposableRegistry) private asyncRegistry: IAsyncDisposableRegistry,
+ @inject(IFileSystem) private fileSystem: IFileSystem,
+ @inject(IJupyterSessionManager) private sessionManager: IJupyterSessionManager,
+ @inject(IServiceContainer) private serviceContainer: IServiceContainer) {
  this.processServicePromise = this.processServiceFactory.create();
- this.disposableRegistry.push(this.interpreterService.onDidChangeInterpreter(this.onSettingsChanged));
+ this.disposableRegistry.push(this.interpreterService.onDidChangeInterpreter(() => this.onSettingsChanged()));
  this.disposableRegistry.push(this);
  }
 
- public dispose = () => {
+ public dispose() {
  // Clear our usableJupyterInterpreter
  this.usablePythonInterpreter = undefined;
  this.commands = {};
@@ -241,7 +216,6 @@ export class JupyterExecution implements IJupyterExecution, Disposable {
 
  // Try to connect to our jupyter process
  const result = this.serviceContainer.get<INotebookServer>(INotebookServer);
- this.disposableRegistry.push(result);
  await result.connect(connection, kernelSpec, cancelToken, workingDir);
  return result;
  } catch (err) {
@@ -298,7 +272,7 @@ export class JupyterExecution implements IJupyterExecution, Disposable {
  }
 
  // Now enumerate them again
- const enumerator = connection ? () => this.getActiveKernelSpecs(connection) : () => this.enumerateSpecs(cancelToken);
+ const enumerator = connection ? () => this.sessionManager.getActiveKernelSpecs(connection) : () => this.enumerateSpecs(cancelToken);
 
  // Then find our match
  return this.findSpecMatch(enumerator);
@@ -365,8 +339,10 @@ export class JupyterExecution implements IJupyterExecution, Disposable {
  const launchResult = await notebookCommand.execObservable(args, { throwOnStdErr: false, encoding: 'utf8', token: cancelToken });
 
  // Make sure this process gets cleaned up. We might be canceled before the connection finishes.
- if (launchResult) {
- this.asyncRegistry.push({ dispose: () => Promise.resolve(launchResult.dispose()) });
+ if (launchResult && cancelToken) {
+ cancelToken.onCancellationRequested(() => {
+ launchResult.dispose();
+ });
  }
 
  // Wait for the connection information on this result
@@ -475,10 +451,10 @@ export class JupyterExecution implements IJupyterExecution, Disposable {
  return undefined;
  }
 
- private onSettingsChanged = (): Promise<void> => {
+ private onSettingsChanged() {
  // Do the same thing as dispose so that we regenerate
  // all of our commands
- return Promise.resolve(this.dispose());
+ this.dispose();
  }
 
  private async addMatchingSpec(bestInterpreter: PythonInterpreter, cancelToken?: CancellationToken): Promise<void> {
@@ -684,32 +660,7 @@ export class JupyterExecution implements IJupyterExecution, Disposable {
  return bestSpec;
  }
 
- private getActiveKernelSpecs = async (connection: IConnection): Promise<IJupyterKernelSpec[]> => {
- // Use our connection to create a session manager
- const serverSettings = ServerConnection.makeSettings(
- {
- baseUrl: connection.baseUrl,
- token: connection.token,
- pageUrl: '',
- // A web socket is required to allow token authentication (what if there is no token authentication?)
- wsUrl: connection.baseUrl.replace('http', 'ws'),
- init: { cache: 'no-store', credentials: 'same-origin' }
- });
- const sessionManager = new SessionManager({ serverSettings: serverSettings });
-
- // Ask the session manager to refresh its list of kernel specs.
- await sessionManager.refreshSpecs();
-
- // Enumerate all of the kernel specs, turning each into a JupyterKernelSpec
- const kernelspecs = sessionManager.specs && sessionManager.specs.kernelspecs ? sessionManager.specs.kernelspecs : {};
- const keys = Object.keys(kernelspecs);
- return keys.map(k => {
- const spec = kernelspecs[k];
- return new JupyterKernelSpec(spec);
- });
- }
-
- private async readSpec(kernelSpecOutputLine: string): Promise<JupyterKernelSpec | undefined> {
+ private async readSpec(kernelSpecOutputLine: string) : Promise<JupyterKernelSpec | undefined> {
  const match = KernelSpecOutputRegEx.exec(kernelSpecOutputLine);
  if (match && match !== null && match.length > 2) {
  // Second match should be our path to the kernel spec

src/client/datascience/jupyterExporter.ts renamed to src/client/datascience/jupyter/jupyterExporter.ts

@@ -7,13 +7,13 @@ import * as uuid from 'uuid/v4';
 
 import * as os from 'os';
 import * as path from 'path';
-import { IWorkspaceService } from '../common/application/types';
-import { IFileSystem } from '../common/platform/types';
-import { ILogger } from '../common/types';
-import * as localize from '../common/utils/localize';
-import { noop } from '../common/utils/misc';
-import { CodeSnippits, RegExpValues } from './constants';
-import { CellState, ICell, IJupyterExecution, INotebookExporter, ISysInfo } from './types';
+import { IWorkspaceService } from '../../common/application/types';
+import { IFileSystem } from '../../common/platform/types';
+import { ILogger } from '../../common/types';
+import * as localize from '../../common/utils/localize';
+import { noop } from '../../common/utils/misc';
+import { CodeSnippits, RegExpValues } from '../constants';
+import { CellState, ICell, IJupyterExecution, INotebookExporter, ISysInfo } from '../types';
 
 @injectable()
 export class JupyterExporter implements INotebookExporter {