Add functional test for debugging jupyter cells (microsoft#6510)

* Preliminary idea * More logging * Actual responses coming back * Get debugger basic test working Fix all the Promise.race(sleep) code to cancel the timer, as this timer can hang a test * Fix restart problems Add breakpoint test * Add breakpoint in another file test Add functional requirements so can find ptvsd on test machine * Add news entries * Fix logging to a file and a linter issue * Add back logToFile but commented out. * Fix functional tests and rework how waitForPromise behaves

Author: rchiodo

build/functional-test-requirements.txt

@@ -5,3 +5,5 @@ numpy
 matplotlib
 pandas
 livelossplot
+# This is temporary until these bits are ready
+git+https://s3h4llbub6kmnxk53k53ricbg4n7s6tif3ls6qsywersiroezaea@ptvsd.visualstudio.com/ptvsd-pr/_git/ptvsd-pr@ptvsd_jupyter_cells

news/2 Fixes/6502md

@@ -0,0 +1 @@
+Fix debugging to work on restarting the jupyter kernel.

news/3 Code Health/6377.md

@@ -0,0 +1 @@
+Functional test for debugging jupyter cells.

src/client/common/utils/async.ts

@@ -3,12 +3,28 @@
 
 'use strict';
 
-export async function sleep(timeout: number) : Promise<number> {
+export async function sleep(timeout: number): Promise<number> {
  return new Promise<number>((resolve) => {
  setTimeout(() => resolve(timeout), timeout);
  });
 }
 
+export function waitForPromise<T>(promise: Promise<T>, timeout: number): Promise<T | null> {
+ // Set a timer that will resolve with null
+ return new Promise<T | null>((resolve, reject) => {
+ const timer = setTimeout(() => resolve(null), timeout);
+ promise.then(result => {
+ // When the promise resolves, make sure to clear the timer or
+ // the timer may stick around causing tests to wait
+ clearTimeout(timer);
+ resolve(result);
+ }).catch(e => {
+ clearTimeout(timer);
+ reject(e);
+ });
+ });
+}
+
 //======================
 // Deferred
 

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

@@ -14,13 +14,22 @@ import {
 } from 'vscode';
 
 import { IDebugService, IDocumentManager } from '../../common/application/types';
+import { traceError } from '../../common/logger';
 import { IConfigurationService } from '../../common/types';
 import { noop } from '../../common/utils/misc';
 import { CellMatcher } from '../cellMatcher';
 import { splitMultilineString } from '../common';
 import { Identifiers } from '../constants';
 import { InteractiveWindowMessages, SysInfoReason } from '../interactive-window/interactiveWindowTypes';
-import { ICell, ICellHash, ICellHashListener, ICellHashProvider, IFileHashes, IInteractiveWindowListener, INotebookExecutionLogger } from '../types';
+import {
+ ICell,
+ ICellHash,
+ ICellHashListener,
+ ICellHashProvider,
+ IFileHashes,
+ IInteractiveWindowListener,
+ INotebookExecutionLogger
+} from '../types';
 
 interface IRangedCellHash extends ICellHash {
  code: string;
@@ -73,6 +82,7 @@ export class CellHashProvider implements ICellHashProvider, IInteractiveWindowLi
  const reason = payload.type as SysInfoReason;
  if (reason !== SysInfoReason.Interrupt) {
  this.hashes.clear();
+ this.executionCount = 0;
  }
  }
  break;
@@ -92,14 +102,19 @@ export class CellHashProvider implements ICellHashProvider, IInteractiveWindowLi
  }
 
  public async preExecute(cell: ICell, silent: boolean): Promise<void> {
- if (!silent) {
- // When the user adds new code, we know the execution count is increasing
- this.executionCount += 1;
-
- // Skip hash on unknown file though
- if (cell.file !== Identifiers.EmptyFileName) {
- return this.addCellHash(cell, this.executionCount);
+ try {
+ if (!silent) {
+ // When the user adds new code, we know the execution count is increasing
+ this.executionCount += 1;
+
+ // Skip hash on unknown file though
+ if (cell.file !== Identifiers.EmptyFileName) {
+ await this.addCellHash(cell, this.executionCount);
+ }
  }
+ } catch (exc) {
+ // Don't let exceptions in a preExecute mess up normal operation
+ traceError(exc);
  }
  }
 
@@ -195,7 +210,7 @@ export class CellHashProvider implements ICellHashProvider, IInteractiveWindowLi
  }
 
  // Compute the runtime line and adjust our cell/stripped source for debugging
- const runtimeLine = this.adjustForDebugging(cell, stripped, startOffset, endOffset);
+ const runtimeLine = this.adjustRuntimeForDebugging(cell, stripped, startOffset, endOffset);
  const hashedCode = stripped.join('');
  const realCode = doc.getText(new Range(new Position(cell.line, 0), endLine.rangeIncludingLineBreak.end));
 
@@ -245,7 +260,7 @@ export class CellHashProvider implements ICellHashProvider, IInteractiveWindowLi
  }
  }
 
- private adjustForDebugging(cell: ICell, source: string[], cellStartOffset: number, cellEndOffset: number): number {
+ private adjustRuntimeForDebugging(cell: ICell, source: string[], cellStartOffset: number, cellEndOffset: number): number {
  if (this.debugService.activeDebugSession && this.configService.getSettings().datascience.stopOnFirstLineWhileDebugging) {
  // See if any breakpoints in any cell that's already run or in the cell we're about to run
  const anyExisting = this.debugService.breakpoints.filter(b => {

src/client/datascience/interactive-window/intellisense/baseIntellisenseProvider.ts

@@ -20,7 +20,7 @@ import { IWorkspaceService } from '../../../common/application/types';
 import { CancellationError } from '../../../common/cancellation';
 import { traceWarning } from '../../../common/logger';
 import { IFileSystem, TemporaryFile } from '../../../common/platform/types';
-import { createDeferred, Deferred, sleep } from '../../../common/utils/async';
+import { createDeferred, Deferred, waitForPromise } from '../../../common/utils/async';
 import { Identifiers, Settings } from '../../constants';
 import { IInteractiveWindowListener, IInteractiveWindowProvider, IJupyterExecution } from '../../types';
 import {
@@ -43,8 +43,8 @@ export abstract class BaseIntellisenseProvider implements IInteractiveWindowList
 
  private documentPromise: Deferred<IntellisenseDocument> | undefined;
  private temporaryFile: TemporaryFile | undefined;
- private postEmitter: EventEmitter<{message: string; payload: any}> = new EventEmitter<{message: string; payload: any}>();
- private cancellationSources : Map<string, CancellationTokenSource> = new Map<string, CancellationTokenSource>();
+ private postEmitter: EventEmitter<{ message: string; payload: any }> = new EventEmitter<{ message: string; payload: any }>();
+ private cancellationSources: Map<string, CancellationTokenSource> = new Map<string, CancellationTokenSource>();
 
  constructor(
  @unmanaged() private workspaceService: IWorkspaceService,
@@ -60,7 +60,7 @@ export abstract class BaseIntellisenseProvider implements IInteractiveWindowList
  }
  }
 
- public get postMessage(): Event<{message: string; payload: any}> {
+ public get postMessage(): Event<{ message: string; payload: any }> {
  return this.postEmitter.event;
  }
 
@@ -116,7 +116,7 @@ export abstract class BaseIntellisenseProvider implements IInteractiveWindowList
  }
  }
 
- protected getDocument(resource?: Uri) : Promise<IntellisenseDocument> {
+ protected getDocument(resource?: Uri): Promise<IntellisenseDocument> {
  if (!this.documentPromise) {
  this.documentPromise = createDeferred<IntellisenseDocument>();
 
@@ -142,17 +142,17 @@ export abstract class BaseIntellisenseProvider implements IInteractiveWindowList
  }
 
  protected abstract get isActive(): boolean;
- protected abstract provideCompletionItems(position: monacoEditor.Position, context: monacoEditor.languages.CompletionContext, cellId: string, token: CancellationToken) : Promise<monacoEditor.languages.CompletionList>;
- protected abstract provideHover(position: monacoEditor.Position, cellId: string, token: CancellationToken) : Promise<monacoEditor.languages.Hover>;
- protected abstract provideSignatureHelp(position: monacoEditor.Position, context: monacoEditor.languages.SignatureHelpContext, cellId: string, token: CancellationToken) : Promise<monacoEditor.languages.SignatureHelp>;
- protected abstract handleChanges(originalFile: string | undefined, document: IntellisenseDocument, changes: TextDocumentContentChangeEvent[]) : Promise<void>;
+ protected abstract provideCompletionItems(position: monacoEditor.Position, context: monacoEditor.languages.CompletionContext, cellId: string, token: CancellationToken): Promise<monacoEditor.languages.CompletionList>;
+ protected abstract provideHover(position: monacoEditor.Position, cellId: string, token: CancellationToken): Promise<monacoEditor.languages.Hover>;
+ protected abstract provideSignatureHelp(position: monacoEditor.Position, context: monacoEditor.languages.SignatureHelpContext, cellId: string, token: CancellationToken): Promise<monacoEditor.languages.SignatureHelp>;
+ protected abstract handleChanges(originalFile: string | undefined, document: IntellisenseDocument, changes: TextDocumentContentChangeEvent[]): Promise<void>;
 
- private dispatchMessage<M extends IInteractiveWindowMapping, T extends keyof M>(_message: T, payload: any, handler: (args : M[T]) => void) {
+ private dispatchMessage<M extends IInteractiveWindowMapping, T extends keyof M>(_message: T, payload: any, handler: (args: M[T]) => void) {
  const args = payload as M[T];
  handler.bind(this)(args);
  }
 
- private postResponse<M extends IInteractiveWindowMapping, T extends keyof M>(type: T, payload?: M[T]) : void {
+ private postResponse<M extends IInteractiveWindowMapping, T extends keyof M>(type: T, payload?: M[T]): void {
  const response = payload as any;
  if (response && response.id) {
  const cancelSource = this.cancellationSources.get(response.id);
@@ -161,7 +161,7 @@ export abstract class BaseIntellisenseProvider implements IInteractiveWindowList
  this.cancellationSources.delete(response.id);
  }
  }
- this.postEmitter.fire({message: type.toString(), payload});
+ this.postEmitter.fire({ message: type.toString(), payload });
  }
 
  private handleCancel(request: ICancelIntellisenseRequest) {
@@ -181,11 +181,11 @@ export abstract class BaseIntellisenseProvider implements IInteractiveWindowList
  // Combine all of the results together.
  this.postTimedResponse(
  [this.provideCompletionItems(request.position, request.context, request.cellId, cancelSource.token),
-  this.provideJupyterCompletionItems(request.position, request.context, cancelSource.token)],
+ this.provideJupyterCompletionItems(request.position, request.context, cancelSource.token)],
  InteractiveWindowMessages.ProvideCompletionItemsResponse,
  (c) => {
  const list = this.combineCompletions(c);
- return {list, requestId: request.requestId};
+ return { list, requestId: request.requestId };
  }
  );
  }
@@ -198,20 +198,20 @@ export abstract class BaseIntellisenseProvider implements IInteractiveWindowList
  InteractiveWindowMessages.ProvideHoverResponse,
  (h) => {
  if (h && h[0]) {
- return { hover: h[0]!, requestId: request.requestId};
+ return { hover: h[0]!, requestId: request.requestId };
  } else {
  return { hover: { contents: [] }, requestId: request.requestId };
  }
  });
  }
 
- private async provideJupyterCompletionItems(position: monacoEditor.Position, _context: monacoEditor.languages.CompletionContext, cancelToken: CancellationToken) : Promise<monacoEditor.languages.CompletionList> {
+ private async provideJupyterCompletionItems(position: monacoEditor.Position, _context: monacoEditor.languages.CompletionContext, cancelToken: CancellationToken): Promise<monacoEditor.languages.CompletionList> {
  try {
  const activeServer = await this.jupyterExecution.getServer(await this.interactiveWindowProvider.getNotebookOptions());
  const document = await this.getDocument();
  if (activeServer && document) {
  const code = document.getEditCellContent();
- const lines = code.splitLines({trim: false, removeEmptyEntries: false});
+ const lines = code.splitLines({ trim: false, removeEmptyEntries: false });
  const offsetInCode = lines.reduce((a: number, c: string, i: number) => {
  if (i < position.lineNumber - 1) {
  return a + c.length + 1;
@@ -226,7 +226,7 @@ export abstract class BaseIntellisenseProvider implements IInteractiveWindowList
  const baseOffset = document.getEditCellOffset();
  const basePosition = document.positionAt(baseOffset);
  const startPosition = document.positionAt(jupyterResults.cursor.start + baseOffset);
- const endPosition = document.positionAt(jupyterResults.cursor.end  + baseOffset);
+ const endPosition = document.positionAt(jupyterResults.cursor.end + baseOffset);
  const range: monacoEditor.IRange = {
  startLineNumber: startPosition.line + 1 - basePosition.line, // monaco is 1 based
  startColumn: startPosition.character + 1,
@@ -254,27 +254,18 @@ export abstract class BaseIntellisenseProvider implements IInteractiveWindowList
 
  private postTimedResponse<R, M extends IInteractiveWindowMapping, T extends keyof M>(promises: Promise<R>[], message: T, formatResponse: (val: (R | null)[]) => M[T]) {
  // Time all of the promises to make sure they don't take too long
- const timed = promises.map(p => Promise.race([p, sleep(Settings.IntellisenseTimeout)]));
+ const timed = promises.map(p => waitForPromise(p, Settings.IntellisenseTimeout));
 
  // Wait for all of of the timings.
  const all = Promise.all(timed);
  all.then(r => {
-
- // Check all of the results. If they timed out, turn into
- // a null so formatResponse can post the empty result.
- const nulled = r.map(v => {
- if (v === Settings.IntellisenseTimeout) {
- return null;
- }
- return v as R;
- });
- this.postResponse(message, formatResponse(nulled));
+ this.postResponse(message, formatResponse(r));
  }).catch(_e => {
  this.postResponse(message, formatResponse([null]));
  });
  }
 
- private combineCompletions(list: (monacoEditor.languages.CompletionList | null)[]) : monacoEditor.languages.CompletionList {
+ private combineCompletions(list: (monacoEditor.languages.CompletionList | null)[]): monacoEditor.languages.CompletionList {
  // Note to self. We're eliminating duplicates ourselves. The alternative would be to
  // have more than one intellisense provider at the monaco editor level and return jupyter
  // results independently. Maybe we switch to this when jupyter resides on the react side.
@@ -303,9 +294,9 @@ export abstract class BaseIntellisenseProvider implements IInteractiveWindowList
  InteractiveWindowMessages.ProvideSignatureHelpResponse,
  (s) => {
  if (s && s[0]) {
- return { signatureHelp: s[0]!, requestId: request.requestId};
+ return { signatureHelp: s[0]!, requestId: request.requestId };
  } else {
- return {signatureHelp: { signatures: [], activeParameter: 0, activeSignature: 0 }, requestId: request.requestId};
+ return { signatureHelp: { signatures: [], activeParameter: 0, activeSignature: 0 }, requestId: request.requestId };
  }
  });
  }