Add conda support to nightly flake test (#10523)

* Add conda environments

* Reenable multiple interpreters

* Change path for conda environments

* Add pip requirements

* Get multiple interpreters test to work

* Partially working conda search

* Get conda working locally

* Make windows supported environments

* Fix hygiene

* Fix memory leak for daemons

* Fix hang on conda

* Actually retry

* Two missing setting change updates

* Rework the failure capture

* Get rid of memory leak when session dies

* Make tests leak less memory

* Hygiene problem

* More potential cleanup

* More changes to clean up memory as soon as possible

* Dont auto start jupyter

* More leaks found and fixes for breaking tests

* Fix missing args when test is shutting down

* Put back some of the functional requirements

* Update tests to not use static

* Fix linter tests

* More output for linter tests

* Linter only fails with other tests

* Install linters in conda too

* Make base also have dependencies as it's being picked up anyway

* Add news entry

* Review feedback

* Fix unit tests and one of the functional tests

* Functional tests and workspace tests at the same time fail

Author: rchiodo

.vscode/launch.json

@@ -240,8 +240,9 @@
             "env": {
                 // Remove `X` prefix to test with real python (for DS functional tests).
                 "XVSCODE_PYTHON_ROLLING": "1",
+                // Remove 'X' to turn on all logging in the debug output
+                "XVSC_PYTHON_FORCE_LOGGING": "1",
                 // Remove `X` prefix and update path to test with real python interpreter (for DS functional tests).
-                // Do not use a conda environment (as it needs to be activated and the like).
                 "XCI_PYTHON_PATH": "<Python Path>"
             },
             "outFiles": [

build/ci/conda_base.yml

@@ -0,0 +1,4 @@
+pandas
+jupyter
+numpy
+matplotlib

build/ci/conda_env_1.yml

@@ -0,0 +1,7 @@
+name: conda_env_1
+dependencies:
+  - python=3.7
+  - pandas
+  - jupyter
+  - numpy
+  - matplotlib

build/ci/conda_env_2.yml

@@ -0,0 +1,7 @@
+name: conda_env_2
+dependencies:
+  - python=3.8
+  - pandas
+  - jupyter
+  - numpy
+  - matplotlib

build/ci/templates/test_phases.yml

@@ -97,16 +97,7 @@ steps:
       python -m pip --disable-pip-version-check install -t ./pythonFiles/lib/python/debugpy/no_wheels --no-cache-dir --implementation py --no-deps --upgrade --pre debugpy
     displayName: 'pip install system test requirements'
     condition: and(succeeded(), eq(variables['NeedsPythonTestReqs'], 'true'))
-
-  # Install the additional sqlite requirements
-  #
-  # This task will only run if variable `NeedsPythonFunctionalReqs` is true.
-  - bash: |
-      sudo apt-get install libsqlite3-dev
-      python -m pip install pysqlite
-    displayName: 'Setup python to run with sqlite on 2.7'
-    condition: and(succeeded(), eq(variables['NeedsPythonFunctionalReqs'], 'true'), eq(variables['Agent.Os'], 'Linux'), eq(variables['PythonVersion'], '2.7'))
-
+    
   # Install the requirements for functional tests.
   #
   # This task will only run if variable `NeedsPythonFunctionalReqs` is true.
@@ -118,9 +109,69 @@ steps:
   - bash: |
       python -m pip install numpy
       python -m pip install --upgrade -r ./build/functional-test-requirements.txt
+      python -c "import sys;print(sys.executable)"
     displayName: 'pip install functional requirements'
+    condition: and(succeeded(), eq(variables['NeedsPythonFunctionalReqs'], 'true'))    
+
+  # Add CONDA to the path so anaconda works
+  #
+  # This task will only run if variable `NeedsPythonFunctionalReqs` is true.
+  - bash: |
+      echo "##vso[task.prependpath]$CONDA/bin"
+    displayName: 'Add conda to the path'
+    condition: and(succeeded(), eq(variables['NeedsPythonFunctionalReqs'], 'true'), ne(variables['Agent.Os'], 'Windows_NT'))
+
+  # Add CONDA to the path so anaconda works (windows)
+  #
+  # This task will only run if variable `NeedsPythonFunctionalReqs` is true.
+  - powershell: |
+      Write-Host "##vso[task.prependpath]$env:CONDA\Scripts"
+    displayName: 'Add conda to the path'
+    condition: and(succeeded(), eq(variables['NeedsPythonFunctionalReqs'], 'true'), eq(variables['Agent.Os'], 'Windows_NT'))
+
+  # On MAC let CONDA update install paths
+  #
+  # This task will only run if variable `NeedsPythonFunctionalReqs` is true.
+  - bash: |
+      sudo chown -R $USER $CONDA
+    displayName: 'Give CONDA permission to its own files'
+    condition: and(succeeded(), eq(variables['NeedsPythonFunctionalReqs'], 'true'), eq(variables['Agent.Os'], 'Darwin'))
+
+  # Create the two anaconda environments
+  #
+  # This task will only run if variable `NeedsPythonFunctionalReqs` is true.
+  #
+  - script: |
+      conda env create --quiet --force --file build/ci/conda_env_1.yml
+      conda env create --quiet --force --file build/ci/conda_env_2.yml
+    displayName: 'Create CONDA Environments'
     condition: and(succeeded(), eq(variables['NeedsPythonFunctionalReqs'], 'true'))
 
+  # Run the pip installs in the 3 environments (darwin linux)
+  - bash: |
+      source activate base
+      conda install --quiet -y --file build/ci/conda_base.yml
+      python -m pip install --upgrade -r build/conda-functional-requirements.txt
+      source activate conda_env_1
+      python -m pip install --upgrade -r build/conda-functional-requirements.txt
+      source activate conda_env_2
+      python -m pip install --upgrade -r build/conda-functional-requirements.txt
+      conda deactivate
+    displayName: 'Install Pip requirements for CONDA envs'
+    condition: and(succeeded(), eq(variables['NeedsPythonFunctionalReqs'], 'true'), ne(variables['Agent.Os'], 'Windows_NT'))
+
+  # Run the pip installs in the 3 environments (windows)
+  - script: |
+      call activate base
+      conda install --quiet -y --file build/ci/conda_base.yml      
+      python -m pip install --upgrade -r build/conda-functional-requirements.txt
+      call activate conda_env_1
+      python -m pip install --upgrade -r build/conda-functional-requirements.txt
+      call activate conda_env_2
+      python -m pip install --upgrade -r build/conda-functional-requirements.txt
+    displayName: 'Install Pip requirements for CONDA envs'
+    condition: and(succeeded(), eq(variables['NeedsPythonFunctionalReqs'], 'true'), eq(variables['Agent.Os'], 'Windows_NT'))
+
   # Downgrade pywin32 on Windows due to bug https://github.com/jupyter/notebook/issues/4909
   #
   # This task will only run if variable `NeedsPythonFunctionalReqs` is true.

build/ci/vscode-python-nightly-flake-ci.yaml

@@ -50,22 +50,6 @@ stages:
     steps:
       - template: templates/test_phases.yml
 
-  - job: 'Py36'
-    dependsOn: []
-    timeoutInMinutes: 120
-    strategy:
-      matrix:
-        'Functional':
-          PythonVersion: '3.6'
-          TestsToRun: 'testfunctional'
-          NeedsPythonTestReqs: true
-          NeedsPythonFunctionalReqs: true
-          VSCODE_PYTHON_ROLLING: true
-    pool:
-      vmImage: 'ubuntu-16.04'
-    steps:
-      - template: templates/test_phases.yml
-
 - stage: Mac
   dependsOn:
   - Build
@@ -85,22 +69,6 @@ stages:
     steps:
       - template: templates/test_phases.yml
 
-  - job: 'Py36'
-    dependsOn: []
-    timeoutInMinutes: 120
-    strategy:
-      matrix:
-        'Functional':
-          PythonVersion: '3.6'
-          TestsToRun: 'testfunctional'
-          NeedsPythonTestReqs: true
-          NeedsPythonFunctionalReqs: true
-          VSCODE_PYTHON_ROLLING: true
-    pool:
-      vmImage: '$(vmImageMacOS)'
-    steps:
-      - template: templates/test_phases.yml
-
 - stage: Windows
   dependsOn:
   - Build
@@ -119,19 +87,3 @@ stages:
       vmImage: 'vs2017-win2016'
     steps:
       - template: templates/test_phases.yml
-
-  - job: 'Py36'
-    dependsOn: []
-    timeoutInMinutes: 120
-    strategy:
-      matrix:
-        'Functional':
-          PythonVersion: '3.6'
-          TestsToRun: 'testfunctional'
-          NeedsPythonTestReqs: true
-          NeedsPythonFunctionalReqs: true
-          VSCODE_PYTHON_ROLLING: true
-    pool:
-      vmImage: 'vs2017-win2016'
-    steps:
-      - template: templates/test_phases.yml

build/conda-functional-requirements.txt

@@ -0,0 +1,19 @@
+# List of requirements for conda environments that cannot be installed using conda
+livelossplot
+versioneer
+flake8
+autopep8
+bandit
+black ; python_version>='3.6'
+yapf
+pylint
+pycodestyle
+prospector
+pydocstyle
+nose
+pytest==4.6.9 # Last version of pytest with Python 2.7 support
+rope
+flask
+django
+isort
+pathlib2>=2.2.0 ; python_version<'3.6' # Python 2.7 compatibility (pytest)

build/functional-test-requirements.txt

@@ -1,7 +1,2 @@
-# List of requirements for functional tests
-versioneer
-jupyter
-numpy
-matplotlib
-pandas
-livelossplot
+# List of requirements for functional tests
+versioneer

news/3 Code Health/10134.md

@@ -0,0 +1 @@
+Add conda environments to nightly test runs

src/client/common/asyncDisposableRegistry.ts

@@ -12,6 +12,7 @@ export class AsyncDisposableRegistry implements IAsyncDisposableRegistry {
     public async dispose(): Promise<void> {
         const promises = this._list.map(l => l.dispose());
         await Promise.all(promises);
+        this._list = [];
     }
 
     public push(disposable?: IDisposable | IAsyncDisposable) {

src/client/common/process/proc.ts

@@ -62,7 +62,8 @@ export class ProcessService extends EventEmitter implements IProcessService {
         const proc = spawn(file, args, spawnOptions);
         let procExited = false;
         const disposable: IDisposable = {
-            dispose: () => {
+            // tslint:disable-next-line: no-function-expression
+            dispose: function() {
                 if (proc && !proc.killed && !procExited) {
                     ProcessService.kill(proc.pid);
                 }

src/client/common/process/pythonDaemon.ts

@@ -66,6 +66,7 @@ export class PythonDaemonExecutionService implements IPythonDaemonExecutionServi
         try {
             // The daemon should die as a result of this.
             this.connection.sendNotification(new NotificationType('exit'));
+            this.proc.kill();
         } catch {
             noop();
         }

src/client/common/process/pythonDaemonPool.ts

@@ -38,6 +38,7 @@ export class PythonDaemonExecutionServicePool implements IPythonDaemonExecutionS
     private readonly observableDaemons: IPythonDaemonExecutionService[] = [];
     private readonly envVariables: NodeJS.ProcessEnv;
     private readonly pythonPath: string;
+    private _disposed = false;
     constructor(
         private readonly logger: IProcessLogger,
         private readonly disposables: IDisposableRegistry,
@@ -67,6 +68,7 @@ export class PythonDaemonExecutionServicePool implements IPythonDaemonExecutionS
 
         // Always ignore warnings as the user should never see the output of the daemon running
         this.envVariables[PYTHON_WARNINGS] = 'ignore';
+        this.disposables.push(this);
     }
     public async initialize() {
         const promises = Promise.all(
@@ -85,7 +87,7 @@ export class PythonDaemonExecutionServicePool implements IPythonDaemonExecutionS
         await Promise.all([promises, promises2]);
     }
     public dispose() {
-        noop();
+        this._disposed = true;
     }
     public async getInterpreterInformation(): Promise<InterpreterInfomation | undefined> {
         const msg = { args: ['GetPythonVersion'] };
@@ -235,7 +237,7 @@ export class PythonDaemonExecutionServicePool implements IPythonDaemonExecutionS
             completed = true;
             if (!daemonProc || (!daemonProc.killed && ProcessService.isAlive(daemonProc.pid))) {
                 this.pushDaemonIntoPool('ObservableDaemon', execService);
-            } else {
+            } else if (!this._disposed) {
                 // Possible daemon is dead (explicitly killed or died due to some error).
                 this.addDaemonService('ObservableDaemon').ignoreErrors();
             }

src/client/common/terminal/helper.ts

@@ -28,7 +28,7 @@ export class TerminalHelper implements ITerminalHelper {
         @inject(IPlatformService) private readonly platform: IPlatformService,
         @inject(ITerminalManager) private readonly terminalManager: ITerminalManager,
         @inject(ICondaService) private readonly condaService: ICondaService,
-        @inject(IInterpreterService) private readonly interpreterService: IInterpreterService,
+        @inject(IInterpreterService) readonly interpreterService: IInterpreterService,
         @inject(IConfigurationService) private readonly configurationService: IConfigurationService,
         @inject(ITerminalActivationCommandProvider)
         @named(TerminalActivationProviders.conda)
@@ -71,9 +71,9 @@ export class TerminalHelper implements ITerminalHelper {
         const providers = [this.pipenv, this.pyenv, this.bashCShellFish, this.commandPromptAndPowerShell];
         const promise = this.getActivationCommands(resource || undefined, interpreter, terminalShellType, providers);
         this.sendTelemetry(
-            resource,
             terminalShellType,
             EventName.PYTHON_INTERPRETER_ACTIVATION_FOR_TERMINAL,
+            interpreter,
             promise
         ).ignoreErrors();
         return promise;
@@ -89,22 +89,21 @@ export class TerminalHelper implements ITerminalHelper {
         const providers = [this.bashCShellFish, this.commandPromptAndPowerShell];
         const promise = this.getActivationCommands(resource, interpreter, shell, providers);
         this.sendTelemetry(
-            resource,
             shell,
             EventName.PYTHON_INTERPRETER_ACTIVATION_FOR_RUNNING_CODE,
+            interpreter,
             promise
         ).ignoreErrors();
         return promise;
     }
     @traceDecorators.error('Failed to capture telemetry')
     protected async sendTelemetry(
-        resource: Resource,
         terminalShellType: TerminalShellType,
         eventName: EventName,
+        interpreter: PythonInterpreter | undefined,
         promise: Promise<string[] | undefined>
     ): Promise<void> {
         let hasCommands = false;
-        const interpreter = await this.interpreterService.getActiveInterpreter(resource);
         let failed = false;
         try {
             const cmds = await promise;

src/client/common/types.ts

@@ -27,7 +27,7 @@ export interface IOutputChannel extends OutputChannel {}
 export const IDocumentSymbolProvider = Symbol('IDocumentSymbolProvider');
 export interface IDocumentSymbolProvider extends DocumentSymbolProvider {}
 export const IsWindows = Symbol('IS_WINDOWS');
-export const IDisposableRegistry = Symbol('IDiposableRegistry');
+export const IDisposableRegistry = Symbol('IDisposableRegistry');
 export type IDisposableRegistry = { push(disposable: Disposable): void };
 export const IMemento = Symbol('IGlobalMemento');
 export const GLOBAL_MEMENTO = Symbol('IGlobalMemento');

src/client/datascience/interactive-common/interactiveBase.ts

@@ -326,6 +326,13 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
         super.dispose();
         this.listeners.forEach(l => l.dispose());
         this.updateContexts(undefined);
+
+        // When closing an editor, dispose of the notebook associated with it.
+        // This won't work when we have multiple views of the notebook though. Notebook ownership
+        // should probably move to whatever owns the backing model.
+        return this.notebook?.dispose().then(() => {
+            this._notebook = undefined;
+        });
     }
 
     public startProgress() {
@@ -1412,7 +1419,12 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
         // Request our new list of variables
         const response: IJupyterVariablesResponse = this._notebook
             ? await this.jupyterVariables.getVariables(this._notebook, args)
-            : { totalCount: 0, pageResponse: [], pageStartIndex: args.startIndex, executionCount: args.executionCount };
+            : {
+                  totalCount: 0,
+                  pageResponse: [],
+                  pageStartIndex: args?.startIndex,
+                  executionCount: args?.executionCount
+              };
 
         this.postMessage(InteractiveWindowMessages.GetVariablesResponse, response).ignoreErrors();
         sendTelemetryEvent(Telemetry.VariableExplorerVariableCount, undefined, { variableCount: response.totalCount });

src/client/datascience/interactive-ipynb/nativeEditor.ts

@@ -512,18 +512,6 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
     protected async close(): Promise<void> {
         // Fire our event
         this.closedEvent.fire(this);
-
-        // Restart our kernel so that execution counts are reset
-        let oldAsk: boolean | undefined = false;
-        const settings = this.configuration.getSettings(await this.getOwningResource());
-        if (settings && settings.datascience) {
-            oldAsk = settings.datascience.askForKernelRestart;
-            settings.datascience.askForKernelRestart = false;
-        }
-        await this.restartKernel(true);
-        if (oldAsk && settings && settings.datascience) {
-            settings.datascience.askForKernelRestart = true;
-        }
     }
 
     protected saveAll() {