Port Don's fixes (#6115)

* Fix test failures resulting from VSCodes Notebook Start page (#6111)

* Disable kernel auto startup in untrusted workspace (#6088)

* Disable kernel auto startup in untrusted workspace

* Fixes

* Misc

* oops

* misc

* Fixes to breaking tests (#6074)

Co-authored-by: Don Jayamanne <don.jayamanne@outlook.com>

.github/workflows/build-test.yml

@@ -192,6 +192,67 @@ jobs:
       - name: Check dependencies
         run: npm run checkDependencies
 
+  vsc_api_check:
+    needs: pick_environment
+    name: VSC Stable & Proposed API
+    runs-on: ${{ matrix.os }}
+    if: github.repository == 'microsoft/vscode-jupyter'
+    strategy:
+      fail-fast: false
+      matrix:
+        os: ${{fromJson(needs.pick_environment.outputs.test_matrix_os)}}
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v2
+
+      - name: Use Node ${{env.NODE_VERSION}}
+        uses: actions/setup-node@v2.1.5
+        with:
+          node-version: ${{env.NODE_VERSION}}
+
+      # Caching of npm packages (https://github.com/actions/cache/blob/main/examples.md#node---npm)
+      - name: Cache npm on linux/mac
+        uses: actions/cache@v2.1.4
+        if: matrix.os != 'windows-latest'
+        with:
+          path: ~/.npm
+          key: ${{ runner.os }}-node-${{ hashFiles('**/package-lock.json') }}
+          restore-keys: |
+            ${{ runner.os }}-node-
+
+      - name: Get npm cache directory
+        if: matrix.os == 'windows-latest'
+        id: npm-cache
+        run: |
+          echo "::set-output name=dir::$(npm config get cache)"
+      - name: Cache npm on windows
+        uses: actions/cache@v2.1.4
+        if: matrix.os == 'windows-latest'
+        with:
+          path: ${{ steps.npm-cache.outputs.dir }}
+          key: ${{ runner.os }}-node-${{ hashFiles('**/package-lock.json') }}
+          restore-keys: |
+            ${{ runner.os }}-node-
+
+      - name: Cache compiled TS files
+        # Use an id for this step so that its cache-hit output can be accessed and checked in the next step.
+        id: out-cache
+        uses: actions/cache@v2.1.4
+        with:
+          path: ./out
+          key: ${{runner.os}}-${{env.CACHE_OUT_DIRECTORY}}-${{hashFiles('src/**')}}
+
+      - name: Install dependencies (npm ci)
+        run: npm ci --prefer-offline
+
+      - name: Install Stable + Proposed API
+        run: npm run download-api
+
+      - name: Compile if not cached
+        run: npx gulp prePublishNonBundle
+        env:
+          CI_JUPYTER_FAST_COMPILATION: 'true'
+
   ts_tests:
     needs: pick_environment
     name: Type Script Tests

package.json

@@ -1862,7 +1862,7 @@
         "build-ipywidgets-compile": "tsc -p ./src/ipywidgets && rimraf ./out/tsconfig.tsbuildinfo && node ./src/ipywidgets/scripts/copyfiles.js",
         "build-ipywidgets-webpack": "cross-env NODE_OPTIONS=--max_old_space_size=9096 webpack --config ./src/ipywidgets/webpack.config.js",
         "checkDependencies": "gulp checkDependencies",
-        "postinstall": "npm run download-api && node ./build/ci/postInstall.js",
+        "postinstall": "node ./build/ci/postInstall.js",
         "test:unittests": "mocha --config ./build/.mocha.unittests.js.json",
         "test:functional": "mocha --require source-map-support/register --config ./build/.mocha.functional.json",
         "test:functional:perf": "node --inspect-brk ./node_modules/mocha/bin/_mocha --require source-map-support/register --config ./build/.mocha.functional.perf.json",

src/client/common/application/notebook.ts

@@ -44,7 +44,7 @@ export class VSCodeNotebook implements IVSCodeNotebook {
         try {
             return window.activeNotebookEditor;
         } catch {
-            return undefined;
+            return;
         }
     }
     private readonly _onDidChangeNotebookDocument = new EventEmitter<NotebookCellChangedEvent>();

src/client/datascience/commands/notebookCommands.ts

@@ -5,10 +5,8 @@
 
 import { inject, injectable } from 'inversify';
 import { Uri } from 'vscode';
-import { ICommandManager, IVSCodeNotebook } from '../../common/application/types';
-import { UseVSCodeNotebookEditorApi } from '../../common/constants';
+import { ICommandManager } from '../../common/application/types';
 import { IDisposable } from '../../common/types';
-import { noop } from '../../common/utils/misc';
 import { Commands } from '../constants';
 import {
     getDisplayNameOrNameOfKernelConnection,
@@ -27,8 +25,6 @@ export class NotebookCommands implements IDisposable {
         @inject(INotebookEditorProvider) private notebookEditorProvider: INotebookEditorProvider,
         @inject(IInteractiveWindowProvider) private interactiveWindowProvider: IInteractiveWindowProvider,
         @inject(INotebookProvider) private readonly notebookProvider: INotebookProvider,
-        @inject(UseVSCodeNotebookEditorApi) private readonly useNativeNotebook: boolean,
-        @inject(IVSCodeNotebook) private readonly notebook: IVSCodeNotebook,
         @inject(KernelSelector) private readonly kernelSelector: KernelSelector,
         @inject(KernelSwitcher) private readonly kernelSwitcher: KernelSwitcher
     ) {}
@@ -48,17 +44,8 @@ export class NotebookCommands implements IDisposable {
     }
 
     private toggleOutput() {
-        if (this.useNativeNotebook && this.notebook.activeNotebookEditor?.selections.length) {
-            const cells = this.notebook.activeNotebookEditor.document.getCells(
-                this.notebook.activeNotebookEditor?.selections[0]
-            );
-            if (cells.length) {
-                const cmd =
-                    cells[0].metadata.outputCollapsed === true
-                        ? 'notebook.cell.expandCellOutput'
-                        : 'notebook.cell.collapseCellOutput';
-                this.commandManager.executeCommand(cmd).then(noop, noop);
-            }
+        if (this.notebookEditorProvider.activeEditor?.toggleOutput) {
+            this.notebookEditorProvider.activeEditor.toggleOutput();
         }
     }
 

src/client/datascience/jupyter/serverPreload.ts

@@ -3,6 +3,7 @@
 'use strict';
 import { inject, injectable } from 'inversify';
 import { IExtensionSingleActivationService } from '../../activation/types';
+import { IWorkspaceService } from '../../common/application/types';
 import { traceError, traceInfo } from '../../common/logger';
 import { IConfigurationService } from '../../common/types';
 import {
@@ -21,7 +22,8 @@ export class ServerPreload implements IExtensionSingleActivationService {
         @inject(INotebookEditorProvider) private notebookEditorProvider: INotebookEditorProvider,
         @inject(IInteractiveWindowProvider) private interactiveProvider: IInteractiveWindowProvider,
         @inject(IConfigurationService) private configService: IConfigurationService,
-        @inject(INotebookProvider) private notebookProvider: INotebookProvider
+        @inject(INotebookProvider) private notebookProvider: INotebookProvider,
+        @inject(IWorkspaceService) private readonly workspace: IWorkspaceService
     ) {
         this.notebookEditorProvider.onDidOpenNotebookEditor(this.onDidOpenNotebook.bind(this));
         this.interactiveProvider.onDidChangeActiveInteractiveWindow(this.onDidOpenOrCloseInteractive.bind(this));
@@ -55,6 +57,9 @@ export class ServerPreload implements IExtensionSingleActivationService {
     }
 
     private async createServerIfNecessary() {
+        if (!this.workspace.isTrusted) {
+            return;
+        }
         try {
             traceInfo(`Attempting to start a server because of preload conditions ...`);
 

src/client/datascience/notebook/creation/notebookCreator.ts

@@ -3,8 +3,10 @@
 
 import { inject, injectable } from 'inversify';
 import { QuickPickItem } from 'vscode';
+import { IS_CI_SERVER } from '../../../../test/ciConstants';
 import { IApplicationShell } from '../../../common/application/types';
 import { JVSC_EXTENSION_DisplayName, JVSC_EXTENSION_ID, PYTHON_LANGUAGE } from '../../../common/constants';
+import { traceInfoIf } from '../../../common/logger';
 import { DataScience } from '../../../common/utils/localize';
 import { sendTelemetryEvent } from '../../../telemetry';
 import { Telemetry, VSCodeNotebookProvider } from '../../constants';
@@ -21,6 +23,7 @@ export class NotebookCreator {
 
     public async createNewNotebook() {
         if (this.creationOptionsService.registrations.length === 0) {
+            console.error('Create using createNew');
             await this.editorProvider.createNew();
             return;
         }
@@ -45,6 +48,7 @@ export class NotebookCreator {
             label: JVSC_EXTENSION_DisplayName
         });
         const placeHolder = DataScience.placeHolderToSelectOptionForNotebookCreation();
+        traceInfoIf(IS_CI_SERVER, `Display quick pick for creation of notebooks ${items.length}`);
         const item = await this.appShell.showQuickPick(items, {
             matchOnDescription: true,
             matchOnDetail: true,

src/client/datascience/notebook/helpers/helpers.ts

@@ -659,7 +659,7 @@ function convertOutputMimeToJupyterOutput(mime: string, value: Uint8Array) {
             // VS Code expects bytes when rendering images.
             return Buffer.from(value).toString('base64');
         } else if (mime.toLowerCase().includes('json')) {
-            return JSON.parse(stringValue);
+            return stringValue.length > 0 ? JSON.parse(stringValue) : stringValue;
         } else {
             return stringValue;
         }

src/client/datascience/notebook/notebookEditor.ts

@@ -13,7 +13,8 @@ import {
     ProgressLocation,
     Uri,
     WebviewPanel,
-    NotebookCellData
+    NotebookCellData,
+    NotebookCell
 } from 'vscode';
 import { IApplicationShell, ICommandManager, IVSCodeNotebook } from '../../common/application/types';
 import { traceError, traceInfo } from '../../common/logger';
@@ -138,6 +139,30 @@ export class NotebookEditor implements INotebookEditor {
     public redoCells(): void {
         this.commandManager.executeCommand('notebook.redo').then(noop, noop);
     }
+    public toggleOutput(): void {
+        if (!this.vscodeNotebook.activeNotebookEditor) {
+            return;
+        }
+
+        const editor = this.vscodeNotebook.notebookEditors.find((item) => item.document === this.document);
+        if (editor) {
+            const cells: NotebookCell[] = [];
+            editor.selections.map((cr) => {
+                if (!cr.isEmpty) {
+                    for (let index = cr.start; index < cr.end; index++) {
+                        cells.push(editor.document.cellAt(index));
+                    }
+                }
+            });
+            chainWithPendingUpdates(editor.document, (edit) => {
+                cells.forEach((cell) => {
+                    const collapsed = cell.metadata.outputCollapsed || false;
+                    const metadata = { ...cell.metadata, outputCollapsed: !collapsed };
+                    edit.replaceNotebookCellMetadata(editor.document.uri, cell.index, metadata);
+                });
+            }).then(noop, noop);
+        }
+    }
     public removeAllCells(): void {
         if (!this.vscodeNotebook.activeNotebookEditor) {
             return;

src/client/datascience/notebook/vscodeNotebookController.ts

@@ -132,13 +132,18 @@ export class VSCodeNotebookController implements Disposable {
     }
 
     public async updateNotebookAffinity(notebook: NotebookDocument, affinity: NotebookControllerAffinity) {
+        traceInfo(`Setting controller affinity for ${notebook.uri.toString()} ${this.id}`);
         this.controller.updateNotebookAffinity(notebook, affinity);
         // Only on CI Server.
         if (this.context.extensionMode === ExtensionMode.Test) {
+            traceInfo(`Force selection of controller for ${notebook.uri.toString()} ${this.id}`);
             await this.commandManager.executeCommand('notebook.selectKernel', {
                 id: this.id,
                 extension: JVSC_EXTENSION_ID
             });
+            traceInfo(
+                `VSCodeNotebookController.kernelAssociatedWithDocument set for ${notebook.uri.toString()} ${this.id}`
+            );
             VSCodeNotebookController.kernelAssociatedWithDocument = true;
         }
     }

src/client/datascience/types.ts

@@ -526,6 +526,7 @@ export interface IInteractiveBase extends Disposable {
     stopProgress(): void;
     undoCells(): void;
     redoCells(): void;
+    toggleOutput?(): void;
     removeAllCells(): void;
     interruptKernel(): Promise<void>;
     restartKernel(): Promise<void>;

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

@@ -7,7 +7,7 @@ import { EventEmitter, Uri } from 'vscode';
 import { PythonExtensionChecker } from '../../../client/api/pythonApi';
 import { ApplicationShell } from '../../../client/common/application/applicationShell';
 import { CommandManager } from '../../../client/common/application/commandManager';
-import { ICommandManager, IVSCodeNotebook } from '../../../client/common/application/types';
+import { ICommandManager } from '../../../client/common/application/types';
 import { ConfigurationService } from '../../../client/common/configuration/service';
 import { NotebookCommands } from '../../../client/datascience/commands/notebookCommands';
 import { Commands } from '../../../client/datascience/constants';
@@ -148,8 +148,6 @@ suite('DataScience - Notebook Commands', () => {
                     instance(notebookEditorProvider),
                     instance(interactiveWindowProvider),
                     instance(notebookProvider),
-                    false,
-                    instance(mock<IVSCodeNotebook>()),
                     kernelSelector,
                     kernelSwitcher
                 );

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

@@ -31,7 +31,8 @@ import {
     canRunNotebookTests,
     closeNotebooksAndCleanUpAfterTests,
     createTemporaryNotebook,
-    saveActiveNotebook
+    saveActiveNotebook,
+    workAroundVSCodeNotebookStartPages
 } from './helper';
 
 /* eslint-disable @typescript-eslint/no-explicit-any, no-invalid-this */
@@ -64,6 +65,7 @@ suite('DataScience - VSCode Notebook - (Open)', function () {
         if (IS_REMOTE_NATIVE_TEST || IS_NON_RAW_NATIVE_TEST || !(await canRunNotebookTests())) {
             return this.skip();
         }
+        await workAroundVSCodeNotebookStartPages();
         vscodeNotebook = api.serviceContainer.get<IVSCodeNotebook>(IVSCodeNotebook);
         contentProvider = api.serviceContainer.get<NotebookContentProvider>(INotebookContentProvider);
     });

src/test/datascience/notebook/creation/notebookCreation.vscode.test.ts

@@ -16,7 +16,12 @@ import { CreationOptionService } from '../../../../client/datascience/notebook/c
 import { IExtensionTestApi, waitForCondition } from '../../../common';
 import { IS_REMOTE_NATIVE_TEST } from '../../../constants';
 import { closeActiveWindows, initialize } from '../../../initialize';
-import { canRunNotebookTests, closeNotebooksAndCleanUpAfterTests, ensureNewNotebooksHavePythonCells } from '../helper';
+import {
+    canRunNotebookTests,
+    closeNotebooksAndCleanUpAfterTests,
+    ensureNewNotebooksHavePythonCells,
+    workAroundVSCodeNotebookStartPages
+} from '../helper';
 
 /* eslint-disable @typescript-eslint/no-explicit-any, no-invalid-this */
 suite('DataScience - VSCode Notebook - (Creation Integration)', function () {
@@ -33,18 +38,29 @@ suite('DataScience - VSCode Notebook - (Creation Integration)', function () {
         creationOptions = api.serviceContainer.get<CreationOptionService>(CreationOptionService);
         vscodeNotebook = api.serviceContainer.get<IVSCodeNotebook>(IVSCodeNotebook);
         creationOptions.clear();
+        await workAroundVSCodeNotebookStartPages();
         await ensureNewNotebooksHavePythonCells();
     });
-    teardown(async () => {
+    teardown(async function () {
+        traceInfo(`Ended Test ${this.currentTest?.title}`);
         sinon.restore();
         creationOptions.clear();
         await closeNotebooksAndCleanUpAfterTests(disposables);
+        traceInfo(`Ended Test (completed) ${this.currentTest?.title}`);
     });
-    setup(async () => {
+    setup(async function () {
+        traceInfo(`Start Test ${this.currentTest?.title}`);
         sinon.restore();
         await closeNotebooksAndCleanUpAfterTests(disposables);
+        traceInfo(`Start Test (completed) ${this.currentTest?.title}`);
+    });
+    suiteTeardown(() => {
+        try {
+            creationOptions.clear();
+        } catch {
+            //
+        }
     });
-    teardown(async () => closeNotebooksAndCleanUpAfterTests(disposables));
     async function createNotebookAndValidateLanguageOfFirstCell(expectedLanguage: string) {
         await commands.executeCommand(Commands.CreateNewNotebook);
         await waitForCondition(async () => !!vscodeNotebook.activeNotebookEditor, 10_000, 'New Notebook not created');

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

@@ -38,7 +38,8 @@ import {
     assertVSCCellIsNotRunning,
     createEmptyPythonNotebook,
     assertNotHasTextOutputInVSCode,
-    waitForQueuedForExecutionOrExecuting
+    waitForQueuedForExecutionOrExecuting,
+    workAroundVSCodeNotebookStartPages
 } from './helper';
 import { ProductNames } from '../../../client/common/installer/productNames';
 import { openNotebook } from '../helpers';
@@ -75,6 +76,7 @@ suite('DataScience - VSCode Notebook - (Execution) (slow)', function () {
         if (!(await canRunNotebookTests())) {
             return this.skip();
         }
+        await workAroundVSCodeNotebookStartPages();
         await hijackPrompt(
             'showErrorMessage',
             { endsWith: expectedPromptMessageSuffix },

src/test/datascience/notebook/helper.ts

@@ -416,6 +416,26 @@ export async function stopJupyterServer() {
     await JupyterServer.instance.dispose().catch(noop);
 }
 
+let workedAroundVSCodeNotebookStartPage = false;
+/**
+ * VS Code displays a start page when opening notebooks for the first time.
+ * This takes focus from the notebook, hence our tests can fail as a result of this.
+ * Solution, try to trigger the display of the start page displayed before starting the tests.
+ */
+export async function workAroundVSCodeNotebookStartPages() {
+    if (workedAroundVSCodeNotebookStartPage) {
+        return;
+    }
+    workedAroundVSCodeNotebookStartPage = true;
+    const { editorProvider } = await getServices();
+    await closeActiveWindows();
+
+    // Open a notebook, VS Code will open the start page (wait for 5s for VSCode to react & open it)
+    await editorProvider.createNew();
+    await sleep(5_000);
+    await closeActiveWindows();
+}
+
 export async function prewarmNotebooks() {
     const { editorProvider, vscodeNotebook, serviceContainer } = await getServices();
     await closeActiveWindows();