Use resolveKernel instead of resolveDocument which is due for depreca…

…tion and re-enable and update associated tests. (#5255)



Co-authored-by: Ian Huff <ianhuff@MININT-ST2TT53.redmond.corp.microsoft.com>

build/conda-functional-requirements.txt

@@ -9,3 +9,4 @@ beakerx_kernel_java
 py4j
 bqplot
 K3D
+ipyleaflet

news/2 Fixes/5217.md

@@ -0,0 +1 @@
+Changes to proposed API for using resolveKernel instead of resolveNotebook. Since this change goes along with widget tests also renable and fix those tests.

src/client/datascience/notebook/contentProvider.ts

@@ -36,23 +36,16 @@ import { NotebookEditorCompatibilitySupport } from './notebookEditorCompatibilit
  */
 @injectable()
 export class NotebookContentProvider implements VSCNotebookContentProvider {
-    public webviews = new Map<string, NotebookCommunication[]>();
     constructor(
         @inject(INotebookStorageProvider) private readonly notebookStorage: INotebookStorageProvider,
         @inject(NotebookEditorCompatibilitySupport)
         private readonly compatibilitySupport: NotebookEditorCompatibilitySupport,
         @inject(IVSCodeNotebook) readonly notebookProvider: IVSCodeNotebook
-    ) {
-        notebookProvider.onDidCloseNotebookDocument(this.onDidCloseNotebook.bind(this));
-    }
-    public async resolveNotebook(document: NotebookDocument, webview: NotebookCommunication): Promise<void> {
-        // Add webviews to list. Used for testing
-        let list = this.webviews.get(document.uri.toString());
-        if (!list) {
-            list = [];
-            this.webviews.set(document.uri.toString(), list);
-        }
-        list.push(webview);
+    ) {}
+    public async resolveNotebook(_document: NotebookDocument, _webview: NotebookCommunication): Promise<void> {
+        // This function is due for deprecation. Associated code has been moved to
+        // NotebookKernelProvider will remove when removed from the API fully
+        return Promise.resolve();
     }
     public async openNotebook(uri: Uri, openContext: NotebookDocumentOpenContext): Promise<NotebookData> {
         if (!this.compatibilitySupport.canOpenWithVSCodeNotebookEditor(uri)) {
@@ -116,8 +109,4 @@ export class NotebookContentProvider implements VSCNotebookContentProvider {
             delete: () => this.notebookStorage.deleteBackup(model, id).ignoreErrors()
         };
     }
-
-    private onDidCloseNotebook(e: NotebookDocument) {
-        this.webviews.delete(e.uri.toString());
-    }
 }

src/client/datascience/notebook/kernelProvider.ts

@@ -50,6 +50,9 @@ import { traceInfo, traceInfoIf } from '../../common/logger';
 
 @injectable()
 export class VSCodeKernelPickerProvider implements INotebookKernelProvider {
+    // Keep a mapping of Document Uri => NotebookCommunication for widget tests
+    //public webviews = new Map<string, NotebookCommunication[]>();
+    public webviews = new WeakMap<NotebookDocument, NotebookCommunication[]>();
     public get onDidChangeKernels(): Event<NotebookDocument | undefined> {
         return this._onDidChangeKernels.event;
     }
@@ -75,6 +78,7 @@ export class VSCodeKernelPickerProvider implements INotebookKernelProvider {
         this.isLocalLaunch = isLocalLaunch(this.configuration);
         this.notebook.onDidChangeActiveNotebookKernel(this.onDidChangeActiveNotebookKernel, this, disposables);
         this.extensions.onDidChange(this.onDidChangeExtensions, this, disposables);
+        this.notebook.onDidCloseNotebookDocument(this.onDidCloseNotebook.bind(this));
     }
 
     public async resolveKernel?(
@@ -83,6 +87,14 @@ export class VSCodeKernelPickerProvider implements INotebookKernelProvider {
         webview: NotebookCommunication,
         token: CancellationToken
     ): Promise<void> {
+        // Add webviews to our document mapping. Used for testing
+        let list = this.webviews.get(document);
+        if (!list) {
+            list = [];
+            this.webviews.set(document, list);
+        }
+        list.push(webview);
+
         return this.kernelResolver.resolveKernel(kernel, document, webview, token);
     }
     @captureTelemetry(Telemetry.KernelProviderPerf)
@@ -279,4 +291,9 @@ export class VSCodeKernelPickerProvider implements INotebookKernelProvider {
             await newKernel.start({ disableUI: true, document }).catch(noop);
         }
     }
+
+    // On notebook document close delete our webview mapping
+    private onDidCloseNotebook(doc: NotebookDocument) {
+        this.webviews.delete(doc);
+    }
 }

src/test/datascience/notebook/ipywidget.vscode.test.ts

@@ -4,97 +4,89 @@
 'use strict';
 
 /* eslint-disable @typescript-eslint/no-require-imports, @typescript-eslint/no-var-requires */
-// import * as path from 'path';
-// import * as sinon from 'sinon';
+import * as path from 'path';
+import * as sinon from 'sinon';
 import { assert } from 'chai';
-import { Uri, NotebookContentProvider as VSCNotebookContentProvider } from 'vscode';
+import { NotebookDocument, Uri } from 'vscode';
 import { IVSCodeNotebook } from '../../../client/common/application/types';
 import { IDisposable } from '../../../client/common/types';
-import { NotebookContentProvider } from '../../../client/datascience/notebook/contentProvider';
-import { INotebookContentProvider } from '../../../client/datascience/notebook/types';
+import { INotebookKernelProvider } from '../../../client/datascience/notebook/types';
 import { IExtensionTestApi } from '../../common';
 import { initialize } from '../../initialize';
 import { openNotebook } from '../helpers';
 import {
     canRunNotebookTests,
     closeNotebooks,
     closeNotebooksAndCleanUpAfterTests,
+    createTemporaryNotebook,
     runCell,
-    waitForExecutionCompletedSuccessfully
+    trustAllNotebooks,
+    waitForExecutionCompletedSuccessfully,
+    waitForKernelToGetAutoSelected
 } from './helper';
 import { InteractiveWindowMessages } from '../../../client/datascience/interactive-common/interactiveWindowTypes';
+import { EXTENSION_ROOT_DIR_FOR_TESTS, IS_WEBVIEW_BUILD_SKIPPED } from '../../constants';
+import { VSCodeKernelPickerProvider } from '../../../client/datascience/notebook/kernelProvider';
+import { createDeferred, Deferred } from '../../../client/common/utils/async';
 
 /* eslint-disable @typescript-eslint/no-explicit-any, no-invalid-this */
 suite('DataScience - VSCode Notebook - IPyWidget test', () => {
-    // const widgetsNB = path.join(
-    //     EXTENSION_ROOT_DIR_FOR_TESTS,
-    //     'src',
-    //     'test',
-    //     'datascience',
-    //     'notebook',
-    //     'standard_widgets.ipynb'
-    // );
+    const widgetsNB = path.join(
+        EXTENSION_ROOT_DIR_FOR_TESTS,
+        'src',
+        'test',
+        'datascience',
+        'notebook',
+        'standard_widgets.ipynb'
+    );
 
     let api: IExtensionTestApi;
     const disposables: IDisposable[] = [];
     let vscodeNotebook: IVSCodeNotebook;
     let testWidgetNb: Uri;
-    //let editorProvider: INotebookEditorProvider;
-    //let languageService: NotebookCellLanguageService;
     suiteSetup(async function () {
-        api = await initialize();
-        if (!process.env.VSC_JUPYTER_CI_RUN_NON_PYTHON_NB_TEST || !(await canRunNotebookTests())) {
+        // We need to have webviews built to run this, so skip if we don't have them
+        if (IS_WEBVIEW_BUILD_SKIPPED) {
+            console.log('Widget notebook tests require webview build to be enabled');
             return this.skip();
         }
-        // Skip for now. Have to wait for this commit to get into insiders
-        // https://github.com/microsoft/vscode/commit/2b900dcf1184ab2424f21a860179f2d97c9928a7
-        // Logged this issue to fix this: https://github.com/microsoft/vscode-jupyter/issues/1103
-        this.skip();
-        // await trustAllNotebooks();
-        // sinon.restore();
-        // vscodeNotebook = api.serviceContainer.get<IVSCodeNotebook>(IVSCodeNotebook);
-        // editorProvider = api.serviceContainer.get<INotebookEditorProvider>(VSCodeNotebookProvider);
-        // languageService = api.serviceContainer.get<NotebookCellLanguageService>(NotebookCellLanguageService);
+
+        if (!process.env.VSC_JUPYTER_RUN_NB_TEST || !(await canRunNotebookTests())) {
+            return this.skip();
+        }
+        api = await initialize();
+
+        await trustAllNotebooks();
+        sinon.restore();
+        vscodeNotebook = api.serviceContainer.get<IVSCodeNotebook>(IVSCodeNotebook);
     });
     setup(async function () {
-        // Skip for now. Have to wait for this commit to get into insiders
-        // https://github.com/microsoft/vscode/commit/2b900dcf1184ab2424f21a860179f2d97c9928a7
-        this.skip();
-        // sinon.restore();
-        // await closeNotebooks();
-        // // Don't use same file (due to dirty handling, we might save in dirty.)
-        // testWidgetNb = Uri.file(await createTemporaryNotebook(widgetsNB, disposables));
+        sinon.restore();
+        await closeNotebooks();
+        // Don't use same file (due to dirty handling, we might save in dirty.)
+        testWidgetNb = Uri.file(await createTemporaryNotebook(widgetsNB, disposables));
     });
     suiteTeardown(() => closeNotebooksAndCleanUpAfterTests(disposables));
-    test('Can run a widget notebook', async function () {
+    test('Can run a widget notebook (webview-test)', async function () {
         await openNotebook(api.serviceContainer, testWidgetNb.fsPath);
+        await waitForKernelToGetAutoSelected();
         const cell = vscodeNotebook.activeNotebookEditor?.document.cells![0]!;
-        const contentProvider = api.serviceContainer.get<VSCNotebookContentProvider>(
-            INotebookContentProvider
-        ) as NotebookContentProvider;
 
-        // Content provider should have a public member that maps webviews. Listen to messages on this webview
-        const webviews = contentProvider.webviews.get(cell.document.uri.toString());
-        assert.equal(webviews?.length, 1, 'No webviews found in content provider');
-        let loaded = false;
-        if (webviews) {
-            webviews[0].onDidReceiveMessage((e) => {
-                if (e.type === InteractiveWindowMessages.IPyWidgetLoadSuccess) {
-                    loaded = true;
-                }
-            });
-        }
+        // This flag will be resolved when the widget loads
+        const flag = createDeferred<boolean>();
+        flagForWebviewLoad(flag, vscodeNotebook.activeNotebookEditor?.document!);
 
         // Execute cell. It should load and render the widget
         await runCell(cell);
 
         // Wait till execution count changes and status is success.
         await waitForExecutionCompletedSuccessfully(cell);
 
-        assert.ok(loaded, 'Widget did not load successfully during execution');
+        assert.ok(flag.completed, 'Widget did not load successfully during execution');
     });
-    test('Can run a widget notebook twice', async function () {
+    test('Can run a widget notebook twice (webview-test)', async function () {
         await openNotebook(api.serviceContainer, testWidgetNb.fsPath);
+        await waitForKernelToGetAutoSelected();
         let cell = vscodeNotebook.activeNotebookEditor?.document.cells![0]!;
         // Execute cell. It should load and render the widget
         await runCell(cell);
@@ -106,58 +98,58 @@ suite('DataScience - VSCode Notebook - IPyWidget test', () => {
         await closeNotebooks();
 
         await openNotebook(api.serviceContainer, testWidgetNb.fsPath);
+        await waitForKernelToGetAutoSelected();
         cell = vscodeNotebook.activeNotebookEditor?.document.cells![0]!;
 
-        const contentProvider = api.serviceContainer.get<VSCNotebookContentProvider>(
-            INotebookContentProvider
-        ) as NotebookContentProvider;
-
-        // Content provider should have a public member that maps webviews. Listen to messages on this webview
-        const webviews = contentProvider.webviews.get(cell.document.uri.toString());
-        assert.equal(webviews?.length, 1, 'No webviews found in content provider');
-        let loaded = false;
-        if (webviews) {
-            webviews[0].onDidReceiveMessage((e) => {
-                if (e.type === InteractiveWindowMessages.IPyWidgetLoadSuccess) {
-                    loaded = true;
-                }
-            });
-        }
+        // This flag will be resolved when the widget loads
+        const flag = createDeferred<boolean>();
+        flagForWebviewLoad(flag, vscodeNotebook.activeNotebookEditor?.document!);
 
         // Execute cell. It should load and render the widget
         await runCell(cell);
 
         // Wait till execution count changes and status is success.
         await waitForExecutionCompletedSuccessfully(cell);
 
-        assert.ok(loaded, 'Widget did not load successfully on second execution');
+        assert.ok(flag.completed, 'Widget did not load successfully on second execution');
     });
-    test('Can run widget cells that need requireJS', async function () {
+    test('Can run widget cells that need requireJS (webview-test)', async function () {
+        // Test runs locally but fails on CI, disabling to be fixed in 5265
+        this.skip();
         await openNotebook(api.serviceContainer, testWidgetNb.fsPath);
+        await waitForKernelToGetAutoSelected();
         // 6th cell has code that needs requireJS
         const cell = vscodeNotebook.activeNotebookEditor?.document.cells![6]!;
-        const contentProvider = api.serviceContainer.get<VSCNotebookContentProvider>(
-            INotebookContentProvider
-        ) as NotebookContentProvider;
 
-        // Content provider should have a public member that maps webviews. Listen to messages on this webview
-        const webviews = contentProvider.webviews.get(cell.document.uri.toString());
-        assert.equal(webviews?.length, 1, 'No webviews found in content provider');
-        let loaded = false;
-        if (webviews) {
-            webviews[0].onDidReceiveMessage((e) => {
-                if (e.type === InteractiveWindowMessages.IPyWidgetLoadSuccess) {
-                    loaded = true;
-                }
-            });
-        }
+        // This flag will be resolved when the widget loads
+        const flag = createDeferred<boolean>();
+        flagForWebviewLoad(flag, vscodeNotebook.activeNotebookEditor?.document!);
 
         // Execute cell. It should load and render the widget
         await runCell(cell);
 
         // Wait till execution count changes and status is success.
         await waitForExecutionCompletedSuccessfully(cell);
 
-        assert.ok(loaded, 'Widget did not load successfully during execution');
+        assert.ok(flag.completed, 'Widget did not load successfully during execution');
     });
+
+    // Resolve a deferred when we see the target uri has an associated webview and the webview
+    // loaded a widget successfully
+    function flagForWebviewLoad(flag: Deferred<boolean>, targetDoc: NotebookDocument) {
+        const notebookKernelProvider = api.serviceContainer.get<INotebookKernelProvider>(
+            INotebookKernelProvider
+        ) as VSCodeKernelPickerProvider;
+
+        // Content provider should have a public member that maps webviews. Listen to messages on this webview
+        const webviews = notebookKernelProvider.webviews.get(targetDoc);
+        assert.equal(webviews?.length, 1, 'No webviews found in kernel provider');
+        if (webviews) {
+            webviews[0].onDidReceiveMessage((e) => {
+                if (e.type === InteractiveWindowMessages.IPyWidgetLoadSuccess) {
+                    flag.resolve(true);
+                }
+            });
+        }
+    }
 });

src/test/datascience/notebook/standard_widgets.ipynb

@@ -156,15 +156,6 @@
  "metadata": {
   "celltoolbar": "Attachments",
   "file_extension": ".py",
-  "kernelspec": {
-   "display_name": "Python 3.8.6 64-bit",
-   "metadata": {
-    "interpreter": {
-     "hash": "5c7437588f5ad65b3fb2510dff59138dda524824913550626013373b675d5274"
-    }
-   },
-   "name": "python3"
-  },
   "language_info": {
    "codemirror_mode": {
     "name": "ipython",