Merge branch 'master' into ds/ipyWidgets

* master: Turning new debugger experiment on for all users and reload experiment for 20% of users (microsoft#10674) Disable blank issues (microsoft#10678) update mockito and minor test updates (microsoft#10671) Fix save on old editor (microsoft#10665) Job names were invalid (microsoft#10646) Telemetry updates (microsoft#10631) Remote session shutdownAll is killing all sessions (microsoft#10621)
ErwanDL · Mar 20, 2020 · aaae5ca · aaae5ca
1 parent b248deb
commit aaae5ca
Show file tree

Hide file tree

Showing 24 changed files with 84 additions and 41 deletions.
diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml
@@ -1,4 +1,4 @@
-blank_issues_enabled: true
+blank_issues_enabled: false
 contact_links:
   - name: Stack Overflow
     url: https://stackoverflow.com/questions/tagged/visual-studio-code+python

diff --git a/build/ci/vscode-python-nightly-flake-ci.yaml b/build/ci/vscode-python-nightly-flake-ci.yaml
@@ -35,7 +35,7 @@ stages:
   dependsOn:
   - Build
   jobs:
-  - job: 'Py3x'
+  - job: 'Py3x_Linux'
     dependsOn: []
     timeoutInMinutes: 120
     strategy:
@@ -54,7 +54,7 @@ stages:
   dependsOn:
   - Build
   jobs:
-  - job: 'Py3x'
+  - job: 'Py3x_Mac'
     dependsOn: []
     timeoutInMinutes: 120
     strategy:
@@ -73,7 +73,7 @@ stages:
   dependsOn:
   - Build
   jobs:
-  - job: 'Py3x'
+  - job: 'Py3x_Windows'
     dependsOn: []
     timeoutInMinutes: 120
     strategy:

diff --git a/experiments.json b/experiments.json
@@ -45,24 +45,24 @@
         "name": "PtvsdWheels37 - control",
         "salt": "DebugAdapterFactory",
         "min": 0,
-        "max": 50
+        "max": 0
     },
     {
         "name": "PtvsdWheels37 - experiment",
         "salt": "DebugAdapterFactory",
-        "min": 50,
+        "min": 0,
         "max": 100
     },
     {
         "name": "Reload - control",
         "salt": "DebugAdapterFactory",
-        "min": 50,
-        "max": 60
+        "min": 0,
+        "max": 20
     },
     {
         "name": "Reload - experiment",
         "salt": "DebugAdapterFactory",
-        "min": 90,
+        "min": 80,
         "max": 100
     },
     {

diff --git a/news/2 Fixes/10647.md b/news/2 Fixes/10647.md
@@ -0,0 +1 @@
+Fix save button not working on notebooks.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -3158,7 +3158,7 @@
         "terser-webpack-plugin": "^2.3.2",
         "transform-loader": "^0.2.4",
         "ts-loader": "^5.3.0",
-        "ts-mockito": "^2.3.1",
+        "ts-mockito": "^2.5.0",
         "ts-node": "^8.3.0",
         "tsconfig-paths-webpack-plugin": "^3.2.0",
         "tslint": "^5.20.1",

diff --git a/package.nls.json b/package.nls.json
@@ -417,8 +417,8 @@
     "DataScience.findJupyterCommandProgress": "Active interpreter does not support {0}. Searching for the best available interpreter.",
     "DataScience.findJupyterCommandProgressCheckInterpreter": "Checking {0}.",
     "DataScience.findJupyterCommandProgressSearchCurrentPath": "Searching current path.",
-    "DataScience.gatheredScriptDescription": "# This file contains only the code required to produce the results of the gathered cell.\n",
-    "DataScience.gatheredNotebookDescriptionInMarkdown": "## Gathered Notebook\nGenerated from ```{0}```\n\nThis notebook contains only the code and cells required to produce the same results as the gathered cell.\n\nPlease note that the python analysis is quite conservative, so if it is unsure whether a line of code is necessary for execution, it will err on the side of including it.\n\nAs this is an experimental feature, please let us know how well Gather works for you at [https://aka.ms/gathersurvey](https://aka.ms/gathersurvey)",
+    "DataScience.gatheredScriptDescription":"# This file was generated by an experimental feature called 'Gather'.\n#\n#     The intent is that it contains only the code required to produce\n#     the same results as the cell originally selected for gathering.\n#     Please note that the Python analysis is quite conservative, so if\n#     it is unsure whether a line of code is necessary for execution, it\n#     will err on the side of including it.\n#\n# Please let us know if you are satisfied with what was gathered here:\n# https://aka.ms/gathersurvey\n\n",
+    "DataScience.gatheredNotebookDescriptionInMarkdown": "## Gathered Notebook\nGathered from ```{0}```\n\n|   |   |\n|---|---|\n|&nbsp;&nbsp;&nbsp|This notebook was generated by an experimental feature called \"Gather\". The intent is that it contains only the code and cells required to produce the same results as the cell originally selected for gathering. Please note that the Python analysis is quite conservative, so if it is unsure whether a line of code is necessary for execution, it will err on the side of including it.|\n\n**Are you satisfied with the code that was gathered?**\n\n[Yes](https://aka.ms/gathersurvey?succeed_value=1) [No](https://aka.ms/gathersurvey?succeed_value=0)",
     "DataScience.savePngTitle": "Save Image",
     "DataScience.jupyterSelectURIQuickPickTitle": "Pick how to connect to Jupyter",
     "DataScience.jupyterSelectURIQuickPickPlaceholder": "Choose an option",

diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts
@@ -722,11 +722,11 @@ export namespace DataScience {
     );
     export const gatheredScriptDescription = localize(
         'DataScience.gatheredScriptDescription',
-        '# This file contains only the code required to produce the results of the gathered cell.\n'
+        '# This file was generated by an experimental feature called "Gather".\n#\n#     The intent is that it contains only the code required to produce\n#     the same results as the cell originally selected for gathering.\n#     Please note that the Python analysis is quite conservative, so if\n#     it is unsure whether a line of code is necessary for execution, it\n#     will err on the side of including it.\n#\n# Please let us know if you are satisfied with what was gathered here:\n# https://aka.ms/gathersurvey\n\n'
     );
     export const gatheredNotebookDescriptionInMarkdown = localize(
         'DataScience.gatheredNotebookDescriptionInMarkdown',
-        '## Gathered Notebook\nGenerated from ```{0}```\n\nThis notebook contains only the code and cells required to produce the same results as the gathered cell.\n\nPlease note that the python analysis is quite conservative, so if it is unsure whether a line of code is necessary for execution, it will err on the side of including it.\n\nAs this is an experimental feature, please let us know how well Gather works for you at [https://aka.ms/gathersurvey](https://aka.ms/gathersurvey)'
+        '# Gathered Notebook\nGathered from ```{0}```\n\n|   |   |\n|---|---|\n|&nbsp;&nbsp;&nbsp|This notebook was generated by an experimental feature called "Gather". The intent is that it contains only the code and cells required to produce the same results as the cell originally selected for gathering. Please note that the Python analysis is quite conservative, so if it is unsure whether a line of code is necessary for execution, it will err on the side of including it.|\n\n**Are you satisfied with the code that was gathered?**\n\n[Yes](https://aka.ms/gathersurvey?succeed_value=1) [No](https://aka.ms/gathersurvey?succeed_value=0)'
     );
     export const savePngTitle = localize('DataScience.savePngTitle', 'Save Image');
     export const fallbackToUseActiveInterpeterAsKernel = localize(

diff --git a/src/client/datascience/constants.ts b/src/client/datascience/constants.ts
@@ -284,6 +284,7 @@ export enum Telemetry {
     NewFileForInteractiveWindow = 'DS_INTERNAL.NEW_FILE_USED_IN_INTERACTIVE',
     KernelInvalid = 'DS_INTERNAL.INVALID_KERNEL_USED',
     GatherCompleted = 'DATASCIENCE.GATHER_COMPLETED',
+    GatheredNotebookSaved = 'DATASCIENCE.GATHERED_NOTEBOOK_SAVED',
     ZMQNotSupported = 'DATASCIENCE.ZMQ_NATIVE_BINARIES_NOT_LOADING'
 }
 

diff --git a/src/client/datascience/gather/gather.ts b/src/client/datascience/gather/gather.ts
@@ -7,7 +7,8 @@ import { IConfigurationService, IDisposableRegistry } from '../../common/types';
 import * as localize from '../../common/utils/localize';
 // tslint:disable-next-line: no-duplicate-imports
 import { Common } from '../../common/utils/localize';
-import { Identifiers } from '../constants';
+import { sendTelemetryEvent } from '../../telemetry';
+import { Identifiers, Telemetry } from '../constants';
 import { CellState, ICell as IVscCell, IGatherProvider } from '../types';
 
 /**
@@ -46,7 +47,9 @@ export class GatherProvider implements IGatherProvider {
                     );
                 }
             } catch (ex) {
-                traceInfo('Gathering tools could not be activated. Indicates build of VSIX was not');
+                traceInfo(
+                    'Gathering tools could not be activated. Indicates build of VSIX could not find @msrvida/python-program-analysis'
+                );
             }
         }
     }
@@ -72,6 +75,7 @@ export class GatherProvider implements IGatherProvider {
      */
     public gatherCode(vscCell: IVscCell): string {
         if (!this._executionSlicer) {
+            sendTelemetryEvent(Telemetry.GatherCompleted, undefined, { result: 'unavailable' });
             return '# %% [markdown]\n## Gather not available';
         }
 

diff --git a/src/client/datascience/gather/gatherListener.ts b/src/client/datascience/gather/gatherListener.ts
@@ -1,4 +1,5 @@
 import { inject, injectable } from 'inversify';
+import { IDisposable } from 'monaco-editor';
 import * as uuid from 'uuid/v4';
 import { Event, EventEmitter, Position, Uri, ViewColumn } from 'vscode';
 import { createMarkdownCell } from '../../../datascience-ui/common/cellFactory';
@@ -161,9 +162,17 @@ export class GatherListener implements IInteractiveWindowListener {
 
             const notebook = await this.jupyterExporter.translateToNotebook(cells);
             if (notebook) {
-                notebook.metadata.gatheredNotebook = true;
                 const contents = JSON.stringify(notebook);
-                await this.ipynbProvider.createNew(contents);
+                const editor = await this.ipynbProvider.createNew(contents);
+
+                let disposable: IDisposable;
+                const handler = () => {
+                    sendTelemetryEvent(Telemetry.GatheredNotebookSaved);
+                    if (disposable) {
+                        disposable.dispose();
+                    }
+                };
+                disposable = editor.saved(handler);
             }
         }
     }

diff --git a/src/client/datascience/interactive-ipynb/nativeEditorProviderOld.ts b/src/client/datascience/interactive-ipynb/nativeEditorProviderOld.ts
@@ -3,7 +3,7 @@
 'use strict';
 import { inject, injectable } from 'inversify';
 import * as path from 'path';
-import { CancellationTokenSource, TextDocument, TextEditor, Uri } from 'vscode';
+import { CancellationTokenSource, EventEmitter, TextDocument, TextEditor, Uri } from 'vscode';
 
 import {
     ICommandManager,
@@ -88,6 +88,16 @@ export class NativeEditorProviderOld extends NativeEditorProvider {
     }
 
     public async open(file: Uri): Promise<INotebookEditor> {
+        // Save a custom document as we use it to search for the object later.
+        if (!this.customDocuments.has(file.fsPath)) {
+            // Required for old editor
+            this.customDocuments.set(file.fsPath, {
+                uri: file,
+                viewType: NativeEditorProvider.customEditorViewType,
+                onDidDispose: new EventEmitter<void>().event
+            });
+        }
+
         // See if this file is open or not already
         let editor = this.activeEditors.get(file.fsPath);
         if (!editor) {

diff --git a/src/client/datascience/jupyter/jupyterExecution.ts b/src/client/datascience/jupyter/jupyterExecution.ts
@@ -173,8 +173,8 @@ export class JupyterExecutionBase implements IJupyterExecution {
                     // Create a server tha  t we will then attempt to connect to.
                     result = this.serviceContainer.get<INotebookServer>(INotebookServer);
 
-                    // In a remote situation, figure out a kernel spec too.
-                    if (!kernelSpecInterpreter.kernelSpec && connection) {
+                    // In a remote non quest situation, figure out a kernel spec too.
+                    if (!kernelSpecInterpreter.kernelSpec && connection && !options?.skipSearchingForKernel) {
                         const sessionManagerFactory = this.serviceContainer.get<IJupyterSessionManagerFactory>(
                             IJupyterSessionManagerFactory
                         );

diff --git a/src/client/datascience/jupyter/jupyterSessionManager.ts b/src/client/datascience/jupyter/jupyterSessionManager.ts
@@ -51,12 +51,8 @@ export class JupyterSessionManager implements IJupyterSessionManager {
 
             // tslint:disable-next-line: no-any
             const sessionManager = this.sessionManager as any;
-            try {
-                await this.sessionManager.shutdownAll();
-            } finally {
-                this.sessionManager.dispose();
-                this.sessionManager = undefined;
-            }
+            this.sessionManager.dispose(); // Note, shutting down all will kill all kernels on the same connection. We don't want that.
+            this.sessionManager = undefined;
 
             // The session manager can actually be stuck in the context of a timer. Clear out the specs inside of
             // it so the memory for the session is minimized. Otherwise functional tests can run out of memory

diff --git a/src/client/datascience/jupyter/liveshare/guestJupyterExecution.ts b/src/client/datascience/jupyter/liveshare/guestJupyterExecution.ts
@@ -102,7 +102,8 @@ export class GuestJupyterExecution extends LiveShareParticipantGuest(
                         skipUsingDefaultConfig: options && options.skipUsingDefaultConfig,
                         workingDir: options ? options.workingDir : undefined,
                         purpose,
-                        allowUI: () => false
+                        allowUI: () => false,
+                        skipSearchingForKernel: true
                     },
                     cancelToken
                 );

diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts
@@ -166,6 +166,8 @@ export interface INotebookServerOptions {
     workingDir?: string;
     purpose: string;
     metadata?: nbformat.INotebookMetadata;
+    disableUI?: boolean;
+    skipSearchingForKernel?: boolean;
     allowUI(): boolean;
 }
 

diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts
@@ -1894,8 +1894,12 @@ export interface IEventNamePropertyMapping {
         /**
          * result indicates whether the gather was completed to a script, notebook or suffered an internal error.
          */
-        result: 'err' | 'script' | 'notebook';
+        result: 'err' | 'script' | 'notebook' | 'unavailable';
     };
+    /**
+     * Telemetry event sent when a gathered notebook has been saved by the user.
+     */
+    [Telemetry.GatheredNotebookSaved]: undefined | never;
     /**
      * Telemetry event sent when the ZMQ native binaries do not work.
      */

diff --git a/src/test/datascience/execution.unit.test.ts b/src/test/datascience/execution.unit.test.ts
@@ -769,7 +769,7 @@ suite('Jupyter Execution', async () => {
         when(interpreterService.getInterpreterDetails(match('/foo/baz/python.exe'))).thenResolve(missingKernelPython);
         when(interpreterService.getInterpreterDetails(match('/bar/baz/python.exe'))).thenResolve(missingNotebookPython);
         when(interpreterService.getInterpreterDetails(argThat(o => !o.includes || !o.includes('python')))).thenReject(
-            'Unknown interpreter'
+            ('Unknown interpreter' as any) as Error
         );
         if (runInDocker) {
             when(fileSystem.readFile('/proc/self/cgroup')).thenResolve('hello docker world');

diff --git a/src/test/datascience/interactiveWindowCommandListener.unit.test.ts b/src/test/datascience/interactiveWindowCommandListener.unit.test.ts
@@ -107,7 +107,7 @@ suite('Interactive window command listener', async () => {
         // Setup defaults
         when(interpreterService.onDidChangeInterpreter).thenReturn(dummyEvent.event);
         when(interpreterService.getInterpreterDetails(argThat(o => !o.includes || !o.includes('python')))).thenReject(
-            'Unknown interpreter'
+            ('Unknown interpreter' as any) as Error
         );
 
         // Service container needs logger, file system, and config service

diff --git a/src/test/datascience/jupyter/jupyterSession.unit.test.ts b/src/test/datascience/jupyter/jupyterSession.unit.test.ts
@@ -1,8 +1,14 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
-import { ContentsManager, Kernel, ServerConnection, Session, SessionManager } from '@jupyterlab/services';
+import {
+    ContentsManager,
+    Kernel,
+    KernelMessage,
+    ServerConnection,
+    Session,
+    SessionManager
+} from '@jupyterlab/services';
 import { DefaultKernel } from '@jupyterlab/services/lib/kernel/default';
-import { KernelFutureHandler } from '@jupyterlab/services/lib/kernel/future';
 import { DefaultSession } from '@jupyterlab/services/lib/session/default';
 import { ISignal, Signal } from '@phosphor/commands/node_modules/@phosphor/signaling';
 import { assert } from 'chai';
@@ -321,7 +327,9 @@ suite('Data Science - JupyterSession', () => {
                 setup(executeUserCode);
 
                 async function executeUserCode() {
-                    const future = mock(KernelFutureHandler);
+                    const future = mock<
+                        Kernel.IFuture<KernelMessage.IShellControlMessage, KernelMessage.IShellControlMessage>
+                    >();
                     // tslint:disable-next-line: no-any
                     when(future.done).thenReturn(Promise.resolve(undefined as any));
                     // tslint:disable-next-line: no-any

diff --git a/src/test/datascience/notebook.functional.test.ts b/src/test/datascience/notebook.functional.test.ts
@@ -359,6 +359,10 @@ suite('DataScience notebook tests', () => {
     }
 
     runTest('Remote Self Certs', async (_this: Mocha.Context) => {
+        // Skip this. Entered a bug here to fix: https://github.com/microsoft/vscode-python/issues/10622
+        _this.skip();
+
+        /*
         const pythonService = await createPythonService(2);
 
         if (pythonService) {
@@ -436,6 +440,7 @@ suite('DataScience notebook tests', () => {
             traceInfo('Remote Self Cert is not supported on 2.7');
             _this.skip();
         }
+        */
     });
 
     // Connect to a server that doesn't have a token or password, customers use this and we regressed it once

diff --git a/src/test/datascience/uiTests/nativeEditor.ui.functional.test.ts b/src/test/datascience/uiTests/nativeEditor.ui.functional.test.ts
@@ -15,7 +15,6 @@ import { Disposable } from 'vscode';
 import { EXTENSION_ROOT_DIR } from '../../../client/constants';
 import { UseCustomEditor } from '../../../datascience-ui/react-common/constants';
 import { getOSType, OSType } from '../../common';
-import { sleep } from '../../core';
 import { mockedVSCodeNamespaces } from '../../vscode-mock';
 import { DataScienceIocContainer } from '../dataScienceIocContainer';
 import { addMockData } from '../testHelpersCore';

diff --git a/src/test/linters/linterCommands.unit.test.ts b/src/test/linters/linterCommands.unit.test.ts
@@ -12,6 +12,7 @@ import { CommandManager } from '../../client/common/application/commandManager';
 import { DocumentManager } from '../../client/common/application/documentManager';
 import { IApplicationShell, ICommandManager, IDocumentManager } from '../../client/common/application/types';
 import { Commands } from '../../client/common/constants';
+import { Product } from '../../client/common/types';
 import { ServiceContainer } from '../../client/ioc/container';
 import { LinterCommands } from '../../client/linters/linterCommands';
 import { LinterManager } from '../../client/linters/linterManager';
@@ -163,6 +164,6 @@ suite('Linting - Linter Commands', () => {
         verify(shell.showWarningMessage(anything(), 'Yes', 'No')).once();
         const quickPickOptions = capture(shell.showQuickPick).last()[1];
         expect(quickPickOptions).to.deep.equal(expectedQuickPickOptions);
-        verify(manager.setActiveLintersAsync(deepEqual(['Three']), anything())).once();
+        verify(manager.setActiveLintersAsync(deepEqual([('Three' as any) as Product]), anything())).once();
     });
 });
diff --git a/src/test/testing/pytest/services/discoveryService.unit.test.ts b/src/test/testing/pytest/services/discoveryService.unit.test.ts
@@ -91,11 +91,13 @@ suite('Unit Tests - PyTest - Discovery', () => {
             throw new Error('Unrecognized directory');
         };
         when(argsService.getTestFolders(deepEqual(options.args))).thenReturn(directories);
-        when(helper.mergeTests(deepEqual(['Result A', 'Result B']))).thenReturn('mergedTests' as any);
+        when(helper.mergeTests(deepEqual([('Result A' as any) as Tests, ('Result B' as any) as Tests]))).thenReturn(
+            'mergedTests' as any
+        );
 
         const tests = await discoveryService.discoverTests(options);
 
-        verify(helper.mergeTests(deepEqual(['Result A', 'Result B']))).once();
+        verify(helper.mergeTests(deepEqual([('Result A' as any) as Tests, ('Result B' as any) as Tests]))).once();
         expect(tests).equal('mergedTests');
     });
     test('Build collection arguments', async () => {