Treat Native notebook tests as VS Code tests (#14282)

Author: DonJayamanne

.eslintignore

@@ -366,7 +366,7 @@ src/test/common/insidersBuild/insidersExtensionService.unit.test.ts
 src/test/pythonFiles/formatting/dummy.ts
 
 src/test/format/extension.dispatch.test.ts
-src/test/format/extension.format.ds.test.ts
+src/test/format/extension.format.native.vscode.test.ts
 src/test/format/extension.onTypeFormat.test.ts
 src/test/format/extension.lineFormatter.test.ts
 src/test/format/extension.sort.test.ts
@@ -574,20 +574,20 @@ src/test/datascience/notebook.functional.test.ts
 src/test/datascience/mockLanguageClient.ts
 src/test/datascience/errorHandler.functional.test.tsx
 src/test/datascience/notebook/notebookStorage.unit.test.ts
-src/test/datascience/notebook/notebookTrust.ds.test.ts
+src/test/datascience/notebook/notebookTrust.native.vscode.test.ts
 src/test/datascience/notebook/rendererExtensionDownloader.unit.test.ts
 src/test/datascience/notebook/survey.unit.test.ts
-src/test/datascience/notebook/interrupRestart.ds.test.ts
-src/test/datascience/notebook/contentProvider.ds.test.ts
+src/test/datascience/notebook/interrupRestart.native.vscode.test.ts
+src/test/datascience/notebook/contentProvider.native.vscode.test.ts
 src/test/datascience/notebook/helper.ts
 src/test/datascience/notebook/contentProvider.unit.test.ts
-src/test/datascience/notebook/edit.ds.test.ts
+src/test/datascience/notebook/edit.native.vscode.test.ts
 src/test/datascience/notebook/rendererExension.unit.test.ts
-src/test/datascience/notebook/saving.ds.test.ts
-src/test/datascience/notebook/notebookEditorProvider.ds.test.ts
+src/test/datascience/notebook/saving.native.vscode.test.ts
+src/test/datascience/notebook/notebookEditorProvider.native.vscode.test.ts
 src/test/datascience/notebook/helpers.unit.test.ts
-src/test/datascience/notebook/executionService.ds.test.ts
-src/test/datascience/notebook/cellOutput.ds.test.ts
+src/test/datascience/notebook/executionService.native.vscode.test.ts
+src/test/datascience/notebook/cellOutput.native.vscode.test.ts
 src/test/datascience/interactiveWindowTestHelpers.tsx
 src/test/datascience/export/exportUtil.test.ts
 src/test/datascience/export/exportToHTML.test.ts

.vscode/launch.json

@@ -152,7 +152,7 @@
             "skipFiles": ["<node_internals>/**"]
         },
         {
-            "name": "Tests (DataScience, *.ds.test.ts)",
+            "name": "Tests (DataScience, *.native.vscode.test.ts)",
             "type": "extensionHost",
             "request": "launch",
             "runtimeExecutable": "${execPath}",
@@ -169,7 +169,7 @@
                 "CI_PYTHON_PATH": "<PythonPath>", // Initialize this to invert the grep (exclude tests with value defined in grep).
 
                 "VSC_PYTHON_LOAD_EXPERIMENTS_FROM_FILE": "true",
-                "TEST_FILES_SUFFIX": "ds.test"
+                "TEST_FILES_SUFFIX": "native.vscode.test"
             },
             "stopOnEntry": false,
             "sourceMaps": true,

build/ci/templates/test_phases.yml

@@ -452,7 +452,7 @@ steps:
       DISPLAY: :10
       VSC_PYTHON_CI_TEST_VSC_CHANNEL: 'insiders'
       VSC_PYTHON_LOAD_EXPERIMENTS_FROM_FILE: 'true'
-      TEST_FILES_SUFFIX: 'ds.test'
+      TEST_FILES_SUFFIX: 'native.vscode.test'
 
   # Run the single workspace tests for DS in VS Code Stable.
   - script: |

build/ci/vscode-python-ci.yaml

@@ -138,6 +138,10 @@ stages:
             'Debugger':
               TestsToRun: 'testDebugger'
               NeedsPythonTestReqs: true
+            'DataScience in VSCode':
+              TestsToRun: 'testDataScienceInVSCode'
+              NeedsPythonTestReqs: true
+              NeedsPythonFunctionalReqs: true
             'Smoke':
               TestsToRun: 'testSmoke'
               NeedsPythonTestReqs: true

package.json

@@ -3471,7 +3471,7 @@
         "preTestJediLSP": "node ./out/test/languageServers/jedi/lspSetup.js",
         "testJediLSP": "node ./out/test/languageServers/jedi/lspSetup.js && cross-env CODE_TESTS_WORKSPACE=src/test VSC_PYTHON_CI_TEST_GREP='Language Server:' node ./out/test/testBootstrap.js ./out/test/standardTest.js && node ./out/test/languageServers/jedi/lspTeardown.js",
         "pretestDataScience": "node ./out/test/datascience/dsTestSetup.js",
-        "testDataScience": "cross-env CODE_TESTS_WORKSPACE=src/test/datascience VSC_PYTHON_CI_TEST_VSC_CHANNEL=insiders TEST_FILES_SUFFIX=ds.test VSC_PYTHON_FORCE_LOGGING=1 VSC_PYTHON_LOAD_EXPERIMENTS_FROM_FILE=true node ./out/test/testBootstrap.js ./out/test/standardTest.js",
+        "testDataScience": "cross-env CODE_TESTS_WORKSPACE=src/test/datascience VSC_PYTHON_CI_TEST_VSC_CHANNEL=insiders TEST_FILES_SUFFIX=native.vscode.test VSC_PYTHON_FORCE_LOGGING=1 VSC_PYTHON_LOAD_EXPERIMENTS_FROM_FILE=true node ./out/test/testBootstrap.js ./out/test/standardTest.js",
         "pretestDataScienceInVSCode": "node ./out/test/datascience/dsTestSetup.js",
         "testDataScienceInVSCode": "cross-env CODE_TESTS_WORKSPACE=src/test/datascience VSC_PYTHON_CI_TEST_VSC_CHANNEL=stable TEST_FILES_SUFFIX=vscode.test VSC_PYTHON_FORCE_LOGGING=1 VSC_PYTHON_LOAD_EXPERIMENTS_FROM_FILE=true node ./out/test/testBootstrap.js ./out/test/standardTest.js",
         "testMultiWorkspace": "node ./out/test/testBootstrap.js ./out/test/multiRootTest.js",

src/client/datascience/jupyter/kernels/cellExecution.ts

@@ -97,6 +97,7 @@ export class CellExecution {
      */
     private previousUpdatedToCellHasCompleted = Promise.resolve();
     private disposables: IDisposable[] = [];
+    private cancelHandled = false;
 
     private constructor(
         public readonly editor: VSCNotebookEditor,
@@ -146,6 +147,9 @@ export class CellExecution {
      * If execution has commenced, then interrupt (via cancellation token) else dequeue from execution.
      */
     public async cancel() {
+        if (this.cancelHandled) {
+            return;
+        }
         await this.initPromise;
         // We need to notify cancellation only if execution is in progress,
         // coz if not, we can safely reset the states.
@@ -156,7 +160,7 @@ export class CellExecution {
         if (!this.started) {
             await this.dequeue();
         }
-        this._result.resolve(this.cell.metadata.runState);
+        await this.completedDurToCancellation();
         this.dispose();
     }
     private dispose() {
@@ -213,6 +217,24 @@ export class CellExecution {
         this._result.resolve(this.cell.metadata.runState);
     }
 
+    private async completedDurToCancellation() {
+        await updateCellExecutionTimes(this.editor, this.cell, {
+            startTime: this.cell.metadata.runStartTime,
+            lastRunDuration: this.stopWatch.elapsedTime
+        });
+
+        await this.editor.edit((edit) =>
+            edit.replaceCellMetadata(this.cell.index, {
+                ...this.cell.metadata,
+                runState: vscodeNotebookEnums.NotebookCellRunState.Idle,
+                statusMessage: ''
+            })
+        );
+
+        this._completed = true;
+        this._result.resolve(this.cell.metadata.runState);
+    }
+
     /**
      * Notify other parts of extension about the cell execution.
      */
@@ -270,7 +292,7 @@ export class CellExecution {
         }
     }
 
-    private execute(session: IJupyterSession, loggers: INotebookExecutionLogger[]) {
+    private async execute(session: IJupyterSession, loggers: INotebookExecutionLogger[]) {
         // Generate metadata from our cell (some kernels expect this.)
         const metadata = {
             ...this.cell.metadata,
@@ -281,59 +303,58 @@ export class CellExecution {
         const code = this.cell.document.getText();
 
         // Skip if no code to execute
-        if (code.trim().length > 0) {
-            const request = session.requestExecute(
-                {
-                    code,
-                    silent: false,
-                    stop_on_error: false,
-                    allow_stdin: true,
-                    store_history: true // Silent actually means don't output anything. Store_history is what affects execution_count
-                },
-                false,
-                metadata
-            );
+        if (code.trim().length === 0) {
+            return this.completedSuccessfully().then(noop, noop);
+        }
 
-            // Listen to messages and update our cell execution state appropriately
+        const request = session.requestExecute(
+            {
+                code,
+                silent: false,
+                stop_on_error: false,
+                allow_stdin: true,
+                store_history: true // Silent actually means don't output anything. Store_history is what affects execution_count
+            },
+            false,
+            metadata
+        );
 
-            // Keep track of our clear state
-            const clearState = new RefBool(false);
+        // Listen to messages and update our cell execution state appropriately
 
-            // Listen to the reponse messages and update state as we go
-            if (request) {
-                // Stop handling the request if the subscriber is canceled.
-                const cancelDisposable = this.token.onCancellationRequested(() => {
-                    request.onIOPub = noop;
-                    request.onStdin = noop;
-                    request.onReply = noop;
-                });
+        // Keep track of our clear state
+        const clearState = new RefBool(false);
 
-                // Listen to messages.
-                request.onIOPub = this.handleIOPub.bind(this, clearState, loggers);
-                request.onStdin = this.handleInputRequest.bind(this, session);
-                request.onReply = this.handleReply.bind(this, clearState);
-
-                // When the request finishes we are done
-                request.done
-                    .then(() => this.completedSuccessfully())
-                    .catch(async (e) => {
-                        // @jupyterlab/services throws a `Canceled` error when the kernel is interrupted.
-                        // Such an error must be ignored.
-                        if (e && e instanceof Error && e.message === 'Canceled') {
-                            await this.completedSuccessfully();
-                        } else {
-                            await this.completedWithErrors(e);
-                        }
-                    })
-                    .finally(() => {
-                        cancelDisposable.dispose();
-                    })
-                    .ignoreErrors();
+        // Listen to the reponse messages and update state as we go
+        if (!request) {
+            return this.completedWithErrors(new Error('Session cannot generate requests')).then(noop, noop);
+        }
+
+        // Stop handling the request if the subscriber is canceled.
+        const cancelDisposable = this.token.onCancellationRequested(() => {
+            request.onIOPub = noop;
+            request.onStdin = noop;
+            request.onReply = noop;
+        });
+
+        // Listen to messages.
+        request.onIOPub = this.handleIOPub.bind(this, clearState, loggers);
+        request.onStdin = this.handleInputRequest.bind(this, session);
+        request.onReply = this.handleReply.bind(this, clearState);
+
+        // When the request finishes we are done
+        try {
+            await request.done;
+            await this.completedSuccessfully();
+        } catch (ex) {
+            // @jupyterlab/services throws a `Canceled` error when the kernel is interrupted.
+            // Such an error must be ignored.
+            if (ex && ex instanceof Error && ex.message === 'Canceled') {
+                await this.completedSuccessfully();
             } else {
-                this.completedWithErrors(new Error('Session cannot generate requrests')).then(noop, noop);
+                await this.completedWithErrors(ex);
             }
-        } else {
-            this.completedSuccessfully().then(noop, noop);
+        } finally {
+            cancelDisposable.dispose();
         }
     }
 

src/test/datascience/commands/notebookCommands.functional.test.ts

@@ -217,7 +217,7 @@ suite('DataScience - Notebook Commands', () => {
                         )
                     ).never();
                 });
-                test('Should switch kernel using the provided notebookxxx', async () => {
+                test('Should switch kernel using the provided notebook', async () => {
                     const notebook = createNotebookMock();
                     when((notebook as any).then).thenReturn(undefined);
                     const uri = Uri.file('test.ipynb');

src/test/datascience/dsTestSetup.ts

@@ -71,7 +71,10 @@ function updateTestsForOldNotebooks() {
 
 if (!IS_CI_SERVER) {
     // Noop.
-} else if (process.env.VSC_PYTHON_CI_TEST_VSC_CHANNEL === 'insiders') {
+} else if (
+    process.env.VSC_PYTHON_CI_TEST_VSC_CHANNEL === 'insiders' &&
+    process.env.TEST_FILES_SUFFIX === 'native.vscode.test'
+) {
     updateTestsForNativeNotebooks();
 } else {
     updateTestsForOldNotebooks();

src/test/datascience/jupyter/kernels/installationPrompts.vscode.test.ts

@@ -9,7 +9,7 @@ import { IApplicationShell } from '../../../../client/common/application/types';
 import { ProductNames } from '../../../../client/common/installer/productNames';
 import { BufferDecoder } from '../../../../client/common/process/decoder';
 import { ProcessService } from '../../../../client/common/process/proc';
-import { IInstaller, InstallerResponse, Product } from '../../../../client/common/types';
+import { IDisposable, IInstaller, InstallerResponse, Product } from '../../../../client/common/types';
 import { createDeferred } from '../../../../client/common/utils/async';
 import { Common, DataScience } from '../../../../client/common/utils/localize';
 import { INotebookEditorProvider } from '../../../../client/datascience/types';
@@ -22,6 +22,7 @@ import { closeNotebooksAndCleanUpAfterTests } from '../../notebook/helper';
 
 // tslint:disable: no-invalid-this max-func-body-length no-function-expression no-any
 suite('DataScience Install IPyKernel (slow) (install)', () => {
+    const disposables: IDisposable[] = [];
     const nbFile = path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src/test/datascience/jupyter/kernels/nbWithKernel.ipynb');
     const executable = getOSType() === OSType.Windows ? 'Scripts/python.exe' : 'bin/python'; // If running locally on Windows box.
     const venvPythonPath = path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src/test/datascience/.venvnokernel', executable);
@@ -55,7 +56,7 @@ suite('DataScience Install IPyKernel (slow) (install)', () => {
     });
 
     setup(closeActiveWindows);
-    teardown(closeNotebooksAndCleanUpAfterTests);
+    teardown(() => closeNotebooksAndCleanUpAfterTests(disposables));
 
     test('Test Install IPyKernel prompt message', async () => {
         // Confirm the message has not changed.
@@ -72,7 +73,7 @@ suite('DataScience Install IPyKernel (slow) (install)', () => {
         const installed = createDeferred();
 
         // Confirm it is installed.
-        sinon.stub(installer, 'install').callsFake(async function (product: Product) {
+        const showInformationMessage = sinon.stub(installer, 'install').callsFake(async function (product: Product) {
             // Call original method
             const result: InstallerResponse = await ((installer.install as any).wrappedMethod.apply(
                 installer,
@@ -83,6 +84,7 @@ suite('DataScience Install IPyKernel (slow) (install)', () => {
             }
             return result;
         });
+        disposables.push({ dispose: () => showInformationMessage.restore() });
 
         // Confirm message is displayed & we click 'Install` button.
         sinon.stub(appShell, 'showErrorMessage').callsFake(function (message: string) {

src/test/datascience/notebook/contentProvider.ds.test.ts renamed to src/test/datascience/notebook/contentProvider.native.vscode.test.ts

@@ -7,8 +7,9 @@ import { assert } from 'chai';
 import * as sinon from 'sinon';
 import { commands, NotebookEditor as VSCNotebookEditor } from 'vscode';
 import { IApplicationShell, IVSCodeNotebook } from '../../../client/common/application/types';
-import { IConfigurationService, IDataScienceSettings, IDisposable } from '../../../client/common/types';
+import { IDisposable } from '../../../client/common/types';
 import { createDeferredFromPromise } from '../../../client/common/utils/async';
+import { DataScience } from '../../../client/common/utils/localize';
 import { noop } from '../../../client/common/utils/misc';
 import { IKernelProvider } from '../../../client/datascience/jupyter/kernels/types';
 import { INotebookEditorProvider } from '../../../client/datascience/types';
@@ -27,9 +28,8 @@ import {
     trustAllNotebooks,
     waitForTextOutputInVSCode
 } from './helper';
-// tslint:disable-next-line: no-var-requires no-require-imports
 
-// tslint:disable: no-any no-invalid-this
+// tslint:disable: no-any no-invalid-this no-function-expression
 /*
  * This test focuses on interrupting, restarting kernels.
  * We will not use actual kernels, just ensure the appropriate methods are invoked on the appropriate classes.
@@ -45,8 +45,6 @@ suite('DataScience - VSCode Notebook - Restart/Interrupt/Cancel/Errors (slow)',
     let vscEditor: VSCNotebookEditor;
     let vscodeNotebook: IVSCodeNotebook;
     const suiteDisposables: IDisposable[] = [];
-    let oldAskForRestart: boolean | undefined;
-    let dsSettings: IDataScienceSettings;
     suiteSetup(async function () {
         this.timeout(60_000);
         api = await initialize();
@@ -59,11 +57,6 @@ suite('DataScience - VSCode Notebook - Restart/Interrupt/Cancel/Errors (slow)',
         editorProvider = api.serviceContainer.get<INotebookEditorProvider>(INotebookEditorProvider);
         editorProvider = api.serviceContainer.get<INotebookEditorProvider>(INotebookEditorProvider);
         kernelProvider = api.serviceContainer.get<IKernelProvider>(IKernelProvider);
-        dsSettings = api.serviceContainer.get<IConfigurationService>(IConfigurationService).getSettings(undefined)
-            .datascience;
-        oldAskForRestart = dsSettings.askForKernelRestart;
-        // Disable the prompt (when attempting to restart kernel).
-        dsSettings.askForKernelRestart = false;
     });
     setup(async () => {
         sinon.restore();
@@ -74,12 +67,7 @@ suite('DataScience - VSCode Notebook - Restart/Interrupt/Cancel/Errors (slow)',
         vscEditor = vscodeNotebook.activeNotebookEditor!;
     });
     teardown(() => closeNotebooks(disposables));
-    suiteTeardown(async () => {
-        oldAskForRestart = dsSettings.askForKernelRestart;
-        // Restore.
-        dsSettings.askForKernelRestart = oldAskForRestart;
-        await closeNotebooksAndCleanUpAfterTests(disposables.concat(suiteDisposables));
-    });
+    suiteTeardown(async () => closeNotebooksAndCleanUpAfterTests(disposables.concat(suiteDisposables)));
 
     test('Cancelling token will cancel cell execution', async () => {
         await insertPythonCell('import time\nfor i in range(10000):\n  print(i)\n  time.sleep(0.1)', 0);
@@ -118,7 +106,20 @@ suite('DataScience - VSCode Notebook - Restart/Interrupt/Cancel/Errors (slow)',
     test('Restarting kernel will cancel cell execution & we can re-run a cell', async () => {
         await insertPythonCell('import time\nfor i in range(10000):\n  print(i)\n  time.sleep(0.1)', 0);
         const cell = vscEditor.document.cells[0];
+        // Ensure we click `Yes` when prompted to restart the kernel.
+        const appShell = api.serviceContainer.get<IApplicationShell>(IApplicationShell);
+        const showInformationMessage = sinon
+            .stub(appShell, 'showInformationMessage')
+            .callsFake(function (message: string) {
+                if (message === DataScience.restartKernelMessage()) {
+                    // User clicked ok to restart it.
+                    return DataScience.restartKernelMessageYes();
+                }
+                return (appShell.showInformationMessage as any).wrappedMethod.apply(appShell, arguments);
+            });
+        disposables.push({ dispose: () => showInformationMessage.restore() });
 
+        (editorProvider.activeEditor as any).shouldAskForRestart = () => Promise.resolve(false);
         await executeActiveDocument();
 
         // Wait for cell to get busy.