Fix some intermittent test failures (microsoft#4973)

For #4951, #4936 <!-- If an item below does not apply to you, then go ahead and check it off as "done" and strikethrough the text, e.g.: - [x] ~Has unit tests & system/integration tests~ --> - [x] Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR) - [x] Title summarizes what is changing - [x] Has a [news entry](https://github.com/Microsoft/vscode-python/tree/master/news) file (remember to thank yourself!) - [ ] Has sufficient logging. - [ ] Has telemetry for enhancements. - [x] Unit tests & system/integration tests are added/updated - [ ] [Test plan](https://github.com/Microsoft/vscode-python/blob/master/.github/test_plan.md) is updated as appropriate - [ ] [`package-lock.json`](https://github.com/Microsoft/vscode-python/blob/master/package-lock.json) has been regenerated by running `npm install` (if dependencies have changed) - [ ] The wiki is updated with any design decisions/details.

Author: rchiodo

news/3 Code Health/4936.md

@@ -0,0 +1 @@
+Fix intermittent test failure with listeners.

news/3 Code Health/4951.md

@@ -0,0 +1 @@
+Fix intermittent test failure with kernel shutdown.

src/client/common/utils/async.ts

@@ -84,18 +84,3 @@ export function createDeferredFromPromise<T>(promise: Promise<T>): Deferred<T> {
  .catch(deferred.reject.bind(deferred));
  return deferred;
 }
-export function callWithTimeout<T>(func: () => T, timeoutMS: number): Promise<T> {
- return new Promise<T>((resolve, reject) => {
- const timeout = setTimeout(() => {
- reject(new Error('Timed out'));
- }, timeoutMS);
- try {
- const result = func();
- resolve(result);
- timeout.unref();
- } catch (e) {
- reject(e);
- timeout.unref();
- }
- });
-}

src/client/datascience/jupyter/jupyterServer.ts

@@ -15,6 +15,7 @@ import { CancellationToken } from 'vscode-jsonrpc';
 
 import { ILiveShareApi } from '../../common/application/types';
 import { CancellationError } from '../../common/cancellation';
+import { traceWarning } from '../../common/logger';
 import { IAsyncDisposableRegistry, IConfigurationService, IDisposableRegistry, ILogger } from '../../common/types';
 import { createDeferred, Deferred, sleep } from '../../common/utils/async';
 import * as localize from '../../common/utils/localize';
@@ -161,7 +162,7 @@ export class JupyterServerBase implements INotebookServer {
  await this.session.waitForIdle();
 
  // Run our initial setup and plot magics
- this.initialNotebookSetup(cancelToken);
+ await this.initialNotebookSetup(cancelToken);
  }
  }
 
@@ -184,9 +185,6 @@ export class JupyterServerBase implements INotebookServer {
  }
 
  public execute(code: string, file: string, line: number, id: string, cancelToken?: CancellationToken, silent?: boolean): Promise<ICell[]> {
- // Do initial setup if necessary
- this.initialNotebookSetup();
-
  // Create a deferred that we'll fire when we're done
  const deferred = createDeferred<ICell[]>();
 
@@ -271,7 +269,7 @@ export class JupyterServerBase implements INotebookServer {
 
  // Rerun our initial setup for the notebook
  this.ranInitialSetup = false;
- this.initialNotebookSetup();
+ await this.initialNotebookSetup();
 
  return;
  }
@@ -386,9 +384,6 @@ export class JupyterServerBase implements INotebookServer {
  }
 
  private executeSilently(code: string, cancelToken?: CancellationToken): Promise<ICell[]> {
- // Do initial setup if necessary
- this.initialNotebookSetup();
-
  // Create a deferred that we'll fire when we're done
  const deferred = createDeferred<ICell[]>();
 
@@ -438,9 +433,6 @@ export class JupyterServerBase implements INotebookServer {
  }
 
  private executeObservableImpl(code: string, file: string, line: number, id: string, silent?: boolean): Observable<ICell[]> {
- // Do initial setup if necessary
- this.initialNotebookSetup();
-
  // If we have a session, execute the code now.
  if (this.session) {
  // Generate our cells ahead of time
@@ -488,21 +480,25 @@ export class JupyterServerBase implements INotebookServer {
  }
 
  // Set up our initial plotting and imports
- private initialNotebookSetup = (cancelToken?: CancellationToken) => {
+ private async initialNotebookSetup(cancelToken?: CancellationToken) : Promise<void> {
  if (this.ranInitialSetup) {
  return;
  }
  this.ranInitialSetup = true;
 
- // When we start our notebook initial, change to our workspace or user specified root directory
- if (this.launchInfo && this.launchInfo.workingDir && this.launchInfo.connectionInfo.localLaunch) {
- this.changeDirectoryIfPossible(this.launchInfo.workingDir).ignoreErrors();
- }
+ try {
+ // When we start our notebook initial, change to our workspace or user specified root directory
+ if (this.launchInfo && this.launchInfo.workingDir && this.launchInfo.connectionInfo.localLaunch) {
+ await this.changeDirectoryIfPossible(this.launchInfo.workingDir);
+ }
 
- this.executeSilently(
- `%matplotlib inline${os.EOL}import matplotlib.pyplot as plt${(this.launchInfo && this.launchInfo.usingDarkTheme) ? `${os.EOL}from matplotlib import style${os.EOL}style.use(\'dark_background\')` : ''}`,
- cancelToken
- ).ignoreErrors();
+ await this.executeSilently(
+ `%matplotlib inline${os.EOL}import matplotlib.pyplot as plt${(this.launchInfo && this.launchInfo.usingDarkTheme) ? `${os.EOL}from matplotlib import style${os.EOL}style.use(\'dark_background\')` : ''}`,
+ cancelToken
+ );
+ } catch (e) {
+ traceWarning(e);
+ }
  }
 
  private combineObservables = (...args: Observable<ICell>[]): Observable<ICell[]> => {
@@ -574,7 +570,8 @@ export class JupyterServerBase implements INotebookServer {
  if (this.launchInfo && this.launchInfo.connectionInfo) {
  // If the server crashes, cancel the current observable
  exitHandlerDisposable = this.launchInfo.connectionInfo.disconnected((c) => {
- subscriber.error(this.sessionStartTime, new Error(localize.DataScience.jupyterServerCrashed().format(c.toString())));
+ const str = c ? c.toString() : '';
+ subscriber.error(this.sessionStartTime, new Error(localize.DataScience.jupyterServerCrashed().format(str)));
  subscriber.complete(this.sessionStartTime);
  });
  }

src/client/datascience/jupyter/jupyterSession.ts

@@ -16,7 +16,7 @@ import { Event, EventEmitter } from 'vscode';
 import { CancellationToken } from 'vscode-jsonrpc';
 
 import { Cancellation } from '../../common/cancellation';
-import { callWithTimeout, sleep } from '../../common/utils/async';
+import { sleep } from '../../common/utils/async';
 import * as localize from '../../common/utils/localize';
 import { noop } from '../../common/utils/misc';
 import { IConnection, IJupyterKernelSpec, IJupyterSession } from '../types';
@@ -182,18 +182,16 @@ export class JupyterSession implements IJupyterSession {
  if (this.session) {
  try {
  // Shutdown may fail if the process has been killed
- await Promise.race([this.session.shutdown(), sleep(100)]);
+ await Promise.race([this.session.shutdown(), sleep(1000)]);
  } catch {
  noop();
  }
- // Dispose may not return. Wrap in a promise instead. Kernel futures can die if
- // process is already dead.
  if (this.session) {
- await callWithTimeout(this.session.dispose.bind(this.session), 100);
+ this.session.dispose();
  }
  }
  if (this.sessionManager) {
- await callWithTimeout(this.sessionManager.dispose.bind(this.sessionManager), 100);
+ this.sessionManager.dispose();
  }
  } catch {
  noop();

src/test/datascience/mockLiveShare.ts

@@ -126,7 +126,10 @@ class Emitter<T> {
  if (typeof item.listener === 'function') {
  result = item.listener.call(undefined, item.event);
  } else {
- result = item.listener[0].call(item.listener[1], item.event);
+ const func = item.listener[0];
+ if (func) {
+ result = func.call(item.listener[1], item.event);
+ }
  }
  }
  } catch (e) {