Get more nightly tests to pass (#11881)

* More logging for ipywidgets failures.

* Dont output images

* Fix commands to not execute after shutdown

* Add some comments describing command dispose

* Fix up logging and add missing dependencies

* Add news entries for Barry's fix

* Dont add news entries here. Too confusing

Author: rchiodo

build/conda-functional-requirements.txt

@@ -17,8 +17,10 @@ django
 isort
 pathlib2>=2.2.0 ; python_version<'3.6' # Python 2.7 compatibility (pytest)
 pythreejs
+ipysheet
 ipyvolume
 beakerx
+py4j
 bqplot
 K3D
 debugpy

src/client/datascience/common.ts

@@ -5,7 +5,9 @@ import type { nbformat } from '@jupyterlab/coreutils';
 import { Memento } from 'vscode';
 import { splitMultilineString } from '../../datascience-ui/common';
 import { noop } from '../common/utils/misc';
+import { traceError, traceInfo } from '../logging';
 import { Settings } from './constants';
+import { ICell } from './types';
 
 // Can't figure out a better way to do this. Enumerate
 // the allowed keys of different output formats.
@@ -98,3 +100,23 @@ export function pruneCell(cell: nbformat.ICell): nbformat.ICell {
 
     return result;
 }
+
+export function traceCellResults(prefix: string, results: ICell[]) {
+    if (results.length > 0 && results[0].data.cell_type === 'code') {
+        const cell = results[0].data as nbformat.ICodeCell;
+        const error = cell.outputs && cell.outputs[0] ? cell.outputs[0].evalue : undefined;
+        if (error) {
+            traceError(`${prefix} Error : ${error}`);
+        } else if (cell.outputs && cell.outputs[0]) {
+            if (cell.outputs[0].output_type.includes('image')) {
+                traceInfo(`${prefix} Output: image`);
+            } else {
+                const data = cell.outputs[0].data;
+                const text = cell.outputs[0].text;
+                traceInfo(`${prefix} Output: ${text || JSON.stringify(data)}`);
+            }
+        }
+    } else {
+        traceInfo(`${prefix} no output.`);
+    }
+}

src/client/datascience/jupyter/jupyterDebugger.ts

@@ -17,6 +17,7 @@ import { IConfigurationService, IExperimentsManager, Version } from '../../commo
 import * as localize from '../../common/utils/localize';
 import { EXTENSION_ROOT_DIR } from '../../constants';
 import { captureTelemetry, sendTelemetryEvent } from '../../telemetry';
+import { traceCellResults } from '../common';
 import { Identifiers, Telemetry } from '../constants';
 import {
     CellState,
@@ -164,30 +165,14 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener {
             if (importResults.length === 0 || importResults[0].state === CellState.error) {
                 traceWarning(`${this.debuggerPackage} not found in path.`);
             } else {
-                this.traceCellResults('import startup', importResults);
+                traceCellResults('import startup', importResults);
             }
 
             // Then enable tracing
             await this.executeSilently(notebook, this.tracingEnableCode);
         }
     }
 
-    private traceCellResults(prefix: string, results: ICell[]) {
-        if (results.length > 0 && results[0].data.cell_type === 'code') {
-            const cell = results[0].data as nbformat.ICodeCell;
-            const error = cell.outputs && cell.outputs[0] ? cell.outputs[0].evalue : undefined;
-            if (error) {
-                traceError(`${prefix} Error : ${error}`);
-            } else if (cell.outputs && cell.outputs[0]) {
-                const data = cell.outputs[0].data;
-                const text = cell.outputs[0].text;
-                traceInfo(`${prefix} Output: ${text || JSON.stringify(data)}`);
-            }
-        } else {
-            traceInfo(`${prefix} no output.`);
-        }
-    }
-
     private async connect(
         notebook: INotebook,
         runByLine: boolean,
@@ -322,7 +307,7 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener {
                 notebook,
                 `import sys\r\nsys.path.extend([${debuggerPathList}])\r\nsys.path`
             );
-            this.traceCellResults('Appending paths', result);
+            traceCellResults('Appending paths', result);
         }
     }
 
@@ -369,7 +354,7 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener {
         purpose: 'parsePtvsdVersionInfo' | 'parseDebugpyVersionInfo' | 'pythonVersionInfo'
     ): Version | undefined {
         if (cells.length < 1 || cells[0].state !== CellState.finished) {
-            this.traceCellResults(purpose, cells);
+            traceCellResults(purpose, cells);
             return undefined;
         }
 
@@ -397,7 +382,7 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener {
             }
         }
 
-        this.traceCellResults(purpose, cells);
+        traceCellResults(purpose, cells);
 
         return undefined;
     }
@@ -452,7 +437,7 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener {
                 return;
             }
         }
-        this.traceCellResults(`Installing ${this.debuggerPackage}`, debuggerInstallResults);
+        traceCellResults(`Installing ${this.debuggerPackage}`, debuggerInstallResults);
         sendTelemetryEvent(Telemetry.PtvsdInstallFailed);
         traceError(`Failed to install ${this.debuggerPackage}`);
         // Failed to install debugger, throw to exit debugging

src/test/common/platform/filesystem.functional.test.ts

@@ -592,7 +592,7 @@ suite('FileSystem', () => {
 
         suite('createWriteStream', () => {
             test('wraps the low-level impl', async () => {
-                const filename = await fix.resolve('x/y/z/spam.py');
+                const filename = await fix.resolve('x/y/z/spam2.py');
                 const expected = fs.createWriteStream(filename);
                 expected.destroy();
 

src/test/datascience/dataScienceIocContainer.ts

@@ -381,6 +381,7 @@ import { registerInterpreterTypes } from '../../client/interpreter/serviceRegist
 import { VirtualEnvironmentManager } from '../../client/interpreter/virtualEnvs';
 import { IVirtualEnvironmentManager } from '../../client/interpreter/virtualEnvs/types';
 import { LanguageServerSurveyBanner } from '../../client/languageServices/languageServerSurveyBanner';
+import { traceInfo } from '../../client/logging';
 import { CodeExecutionHelper } from '../../client/terminals/codeExecution/helper';
 import { ICodeExecutionHelper } from '../../client/terminals/types';
 import { IVsCodeApi } from '../../datascience-ui/react-common/postOffice';
@@ -401,6 +402,7 @@ import { MockLiveShareApi } from './mockLiveShare';
 import { MockPythonSettings } from './mockPythonSettings';
 import { MockWorkspaceConfiguration } from './mockWorkspaceConfig';
 import { MockWorkspaceFolder } from './mockWorkspaceFolder';
+import { TestExecutionLogger } from './testexecutionLogger';
 import { TestInteractiveWindowProvider } from './testInteractiveWindowProvider';
 import { TestNativeEditorProvider } from './testNativeEditorProvider';
 import { TestPersistentStateFactory } from './testPersistentStateFactory';
@@ -489,6 +491,9 @@ export class DataScienceIocContainer extends UnitTestIocContainer {
     }
 
     public async dispose(): Promise<void> {
+        // Make sure to disable all command handling during dispose. Don't want
+        // anything to startup again.
+        this.commandManager.dispose();
         try {
             // Make sure to delete any temp files written by native editor storage
             const globPr = promisify(glob);
@@ -793,6 +798,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer {
         this.serviceManager.add<INotebookExecutionLogger>(INotebookExecutionLogger, HoverProvider);
         this.serviceManager.add<IGatherProvider>(IGatherProvider, GatherProvider);
         this.serviceManager.add<IGatherLogger>(IGatherLogger, GatherLogger, undefined, [INotebookExecutionLogger]);
+        this.serviceManager.add<INotebookExecutionLogger>(INotebookExecutionLogger, TestExecutionLogger);
         this.serviceManager.addSingleton<ICodeLensFactory>(ICodeLensFactory, CodeLensFactory, undefined, [
             IInteractiveWindowListener
         ]);
@@ -1244,8 +1250,10 @@ export class DataScienceIocContainer extends UnitTestIocContainer {
                     this.forceSettingsChanged(undefined, list[0].path);
 
                     // Log this all the time. Useful in determining why a test may not pass.
+                    const message = `Setting interpreter to ${list[0].displayName || list[0].path} -> ${list[0].path}`;
+                    traceInfo(message);
                     // tslint:disable-next-line: no-console
-                    console.log(`Setting interpreter to ${list[0].displayName || list[0].path} -> ${list[0].path}`);
+                    console.log(message);
 
                     // Also set this as the interpreter to use for jupyter
                     await this.serviceManager

src/test/datascience/mockCommandManager.ts

@@ -8,10 +8,12 @@ import { ICommandNameArgumentTypeMapping } from '../../client/common/application
 import { ICommandManager } from '../../client/common/application/types';
 
 // tslint:disable:no-any no-http-string no-multiline-string max-func-body-length
-
 export class MockCommandManager implements ICommandManager {
     private commands: Map<string, (...args: any[]) => any> = new Map<string, (...args: any[]) => any>();
 
+    public dispose() {
+        this.commands.clear();
+    }
     public registerCommand<
         E extends keyof ICommandNameArgumentTypeMapping,
         U extends ICommandNameArgumentTypeMapping[E]

src/test/datascience/testexecutionLogger.ts

@@ -0,0 +1,22 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+import { injectable } from 'inversify';
+import { traceCellResults } from '../../client/datascience/common';
+import { ICell, INotebookExecutionLogger } from '../../client/datascience/types';
+import { traceInfo } from '../../client/logging';
+import { concatMultilineStringInput } from '../../datascience-ui/common';
+
+@injectable()
+export class TestExecutionLogger implements INotebookExecutionLogger {
+    public preExecute(cell: ICell, _silent: boolean): Promise<void> {
+        traceInfo(`Cell Execution for ${cell.id} : \n${concatMultilineStringInput(cell.data.source)}\n`);
+        return Promise.resolve();
+    }
+    public postExecute(cell: ICell, _silent: boolean): Promise<void> {
+        traceCellResults(`Cell Execution complete for ${cell.id}\n`, [cell]);
+        return Promise.resolve();
+    }
+    public onKernelRestarted(): void {
+        // Can ignore this.
+    }
+}