From b260c2acef01b7b3a9d605f0478aa6620ad9617c Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Tue, 3 Mar 2020 16:28:13 -0800 Subject: [PATCH 01/33] Add conda environments --- build/ci/conda_env_1.yml | 9 ++++++++ build/ci/conda_env_2.yml | 9 ++++++++ build/ci/templates/test_phases.yml | 31 ++++++++++++++------------ build/functional-test-requirements.txt | 7 ------ 4 files changed, 35 insertions(+), 21 deletions(-) create mode 100644 build/ci/conda_env_1.yml create mode 100644 build/ci/conda_env_2.yml delete mode 100644 build/functional-test-requirements.txt diff --git a/build/ci/conda_env_1.yml b/build/ci/conda_env_1.yml new file mode 100644 index 000000000000..29a2158352b3 --- /dev/null +++ b/build/ci/conda_env_1.yml @@ -0,0 +1,9 @@ +name: conda_env_1 +dependencies: + - python=3.7 + - pandas + - jupyter + - numpy + - matplotlib + - livelossplot + - versioneer \ No newline at end of file diff --git a/build/ci/conda_env_2.yml b/build/ci/conda_env_2.yml new file mode 100644 index 000000000000..9d74a41333a7 --- /dev/null +++ b/build/ci/conda_env_2.yml @@ -0,0 +1,9 @@ +name: conda_env_2 +dependencies: + - python=3.8 + - pandas + - jupyter + - numpy + - matplotlib + - livelossplot + - versioneer \ No newline at end of file diff --git a/build/ci/templates/test_phases.yml b/build/ci/templates/test_phases.yml index 5a407d9dee10..21d1b14a75ba 100644 --- a/build/ci/templates/test_phases.yml +++ b/build/ci/templates/test_phases.yml @@ -98,27 +98,30 @@ steps: displayName: 'pip install system test requirements' condition: and(succeeded(), eq(variables['NeedsPythonTestReqs'], 'true')) - # Install the additional sqlite requirements + # Add CONDA to the path so anaconda works # # 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')) + echo "##vso[task.prependpath]$CONDA/bin" + displayName: 'Add conda to the path' + condition: and(succeeded(), eq(variables['NeedsPythonFunctionalReqs'], 'true')) - # Install the requirements for functional tests. + # On MAC let CONDA update install paths # # This task will only run if variable `NeedsPythonFunctionalReqs` is true. - # - # Example command line (windows pwsh): - # > python -m pip install numpy - # > python -m pip install --upgrade -r build/functional-test-requirements.txt - # > python -m pip --disable-pip-version-check install -t ./pythonFiles/lib/python --no-cache-dir --implementation py --no-deps --upgrade -r requirements.txt - bash: | - python -m pip install numpy - python -m pip install --upgrade -r ./build/functional-test-requirements.txt - displayName: 'pip install functional requirements' + 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 ../conda_env_1.yml + conda env create --quiet --force --file ../conda_env_2.yml + displayName: 'Create CONDA Environments' condition: and(succeeded(), eq(variables['NeedsPythonFunctionalReqs'], 'true')) # Downgrade pywin32 on Windows due to bug https://github.com/jupyter/notebook/issues/4909 diff --git a/build/functional-test-requirements.txt b/build/functional-test-requirements.txt deleted file mode 100644 index dce60486ae64..000000000000 --- a/build/functional-test-requirements.txt +++ /dev/null @@ -1,7 +0,0 @@ -# List of requirements for functional tests -versioneer -jupyter -numpy -matplotlib -pandas -livelossplot From 885f972cc76bf59257a4ad53dc757fa685b81cf2 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Tue, 3 Mar 2020 16:56:51 -0800 Subject: [PATCH 02/33] Reenable multiple interpreters --- build/ci/templates/test_phases.yml | 4 +- .../interactive-window/interactiveWindow.ts | 2 +- .../datascience/dataScienceIocContainer.ts | 5 ++ .../interactiveWindow.functional.test.tsx | 74 ++++++++++--------- .../interactiveWindowTestHelpers.tsx | 11 +-- 5 files changed, 52 insertions(+), 44 deletions(-) diff --git a/build/ci/templates/test_phases.yml b/build/ci/templates/test_phases.yml index 21d1b14a75ba..291f653d0eb0 100644 --- a/build/ci/templates/test_phases.yml +++ b/build/ci/templates/test_phases.yml @@ -119,8 +119,8 @@ steps: # This task will only run if variable `NeedsPythonFunctionalReqs` is true. # - script: | - conda env create --quiet --force --file ../conda_env_1.yml - conda env create --quiet --force --file ../conda_env_2.yml + conda env create --quiet --force --file conda_env_1.yml + conda env create --quiet --force --file conda_env_2.yml displayName: 'Create CONDA Environments' condition: and(succeeded(), eq(variables['NeedsPythonFunctionalReqs'], 'true')) diff --git a/src/client/datascience/interactive-window/interactiveWindow.ts b/src/client/datascience/interactive-window/interactiveWindow.ts index 537736c40691..d992136830d8 100644 --- a/src/client/datascience/interactive-window/interactiveWindow.ts +++ b/src/client/datascience/interactive-window/interactiveWindow.ts @@ -268,7 +268,7 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi this.postMessage(InteractiveWindowMessages.ScrollToCell, { id }).ignoreErrors(); } - protected async getOwningResource(): Promise { + public async getOwningResource(): Promise { if (this.lastFile) { return Uri.file(this.lastFile); } diff --git a/src/test/datascience/dataScienceIocContainer.ts b/src/test/datascience/dataScienceIocContainer.ts index d2e60a4d4266..c40c6d36bb2a 100644 --- a/src/test/datascience/dataScienceIocContainer.ts +++ b/src/test/datascience/dataScienceIocContainer.ts @@ -1198,6 +1198,11 @@ export class DataScienceIocContainer extends UnitTestIocContainer { }); } + public async getJupyterInterpreters(): Promise { + const list = await this.get(IInterpreterService).getInterpreters(undefined); + return list.filter(f => this.hasJupyter(f.path)); + } + public async addNewSetting(resource: Uri, pythonPath: string | undefined) { // Force a new config setting to appear. if (!pythonPath) { diff --git a/src/test/datascience/interactiveWindow.functional.test.tsx b/src/test/datascience/interactiveWindow.functional.test.tsx index 7fdb88007e0b..cacd83417520 100644 --- a/src/test/datascience/interactiveWindow.functional.test.tsx +++ b/src/test/datascience/interactiveWindow.functional.test.tsx @@ -17,6 +17,8 @@ import { generateCellsFromDocument } from '../../client/datascience/cellFactory' import { EditorContexts } from '../../client/datascience/constants'; import { InteractiveWindowMessages } from '../../client/datascience/interactive-common/interactiveWindowTypes'; import { InteractiveWindow } from '../../client/datascience/interactive-window/interactiveWindow'; +import { IInteractiveWindowProvider } from '../../client/datascience/types'; +import { IInterpreterService } from '../../client/interpreter/contracts'; import { concatMultilineStringInput } from '../../datascience-ui/common'; import { InteractivePanel } from '../../datascience-ui/history-react/interactivePanel'; import { ImageButton } from '../../datascience-ui/react-common/imageButton'; @@ -25,6 +27,7 @@ import { createDocument } from './editor-integration/helpers'; import { defaultDataScienceSettings } from './helpers'; import { addCode, + closeInteractiveWindow, getInteractiveCellResults, getOrCreateInteractiveWindow, runMountedTest @@ -43,6 +46,7 @@ import { findButton, getInteractiveEditor, getLastOutputCell, + mountWebView, srcDirectory, submitInput, toggleCellExpansion, @@ -570,42 +574,40 @@ Type: builtin_function_or_method`, runMountedTest( 'Multiple Interpreters', - async _wrapper => { - // if (!ioc.mockJupyter) { - // const interactiveWindowProvider = ioc.get(IInteractiveWindowProvider); - // const interpreterService = ioc.get(IInterpreterService); - // const window = (await interactiveWindowProvider.getOrCreateActive()) as InteractiveWindow; - // await addCode(ioc, wrapper, 'a=1\na'); - // const activeInterpreter = await interpreterService.getActiveInterpreter( - // await window.getNotebookResource() - // ); - // verifyHtmlOnCell(wrapper, 'InteractiveCell', '1', CellPosition.Last); - // assert.equal( - // window.notebook!.getMatchingInterpreter()?.path, - // activeInterpreter?.path, - // 'Active intrepreter not used to launch notebook' - // ); - // await closeInteractiveWindow(window, wrapper); - - // // Add another python path (hopefully there's more than one on the machine?) - // const secondUri = Uri.file('bar.py'); - // await ioc.addNewSetting(secondUri, undefined); - // const newWrapper = mountWebView(ioc, 'interactive'); - // assert.ok(newWrapper, 'Could not mount a second time'); - // const newWindow = (await interactiveWindowProvider.getOrCreateActive()) as InteractiveWindow; - // await addCode(ioc, wrapper, 'a=1\na', false, secondUri); - // verifyHtmlOnCell(wrapper, 'InteractiveCell', '1', CellPosition.Last); - // assert.notEqual( - // newWindow.notebook!.getMatchingInterpreter()?.path, - // activeInterpreter?.path, - // 'Active intrepreter used to launch second notebook when it should not have' - // ); - // } else { - // tslint:disable-next-line: no-console - console.log( - 'Multiple interpreters test skipped for now. Reenable after fixing https://github.com/microsoft/vscode-python/issues/10134' - ); - // } + async (wrapper, context) => { + if (!ioc.mockJupyter) { + const interactiveWindowProvider = ioc.get(IInteractiveWindowProvider); + const interpreterService = ioc.get(IInterpreterService); + const interpreters = await ioc.getJupyterInterpreters(); + const window = (await interactiveWindowProvider.getOrCreateActive()) as InteractiveWindow; + await addCode(ioc, wrapper, 'a=1\na'); + const activeInterpreter = await interpreterService.getActiveInterpreter( + await window.getOwningResource() + ); + verifyHtmlOnCell(wrapper, 'InteractiveCell', '1', CellPosition.Last); + assert.equal( + window.notebook!.getMatchingInterpreter()?.path, + activeInterpreter?.path, + 'Active intrepreter not used to launch notebook' + ); + await closeInteractiveWindow(window, wrapper); + + // Add another python path + const secondUri = Uri.file('bar.py'); + await ioc.addNewSetting(secondUri, interpreters[1].path); + const newWrapper = mountWebView(ioc, 'interactive'); + assert.ok(newWrapper, 'Could not mount a second time'); + const newWindow = (await interactiveWindowProvider.getOrCreateActive()) as InteractiveWindow; + await addCode(ioc, wrapper, 'a=1\na', false, secondUri); + verifyHtmlOnCell(wrapper, 'InteractiveCell', '1', CellPosition.Last); + assert.notEqual( + newWindow.notebook!.getMatchingInterpreter()?.path, + activeInterpreter?.path, + 'Active intrepreter used to launch second notebook when it should not have' + ); + } else { + context.skip(); + } }, () => { return ioc; diff --git a/src/test/datascience/interactiveWindowTestHelpers.tsx b/src/test/datascience/interactiveWindowTestHelpers.tsx index 28d96909779a..46d890f1a273 100644 --- a/src/test/datascience/interactiveWindowTestHelpers.tsx +++ b/src/test/datascience/interactiveWindowTestHelpers.tsx @@ -39,19 +39,20 @@ export function closeInteractiveWindow( export function runMountedTest( name: string, // tslint:disable-next-line:no-any - testFunc: (wrapper: ReactWrapper, React.Component>) => Promise, + testFunc: (wrapper: ReactWrapper, React.Component>, context: Mocha.Context) => Promise, getIOC: () => DataScienceIocContainer ) { - test(name, async () => { + test(name, async function() { const ioc = getIOC(); const jupyterExecution = ioc.get(IJupyterExecution); if (await jupyterExecution.isNotebookSupported()) { addMockData(ioc, 'a=1\na', 1); const wrapper = mountWebView(ioc, 'interactive'); - await testFunc(wrapper); + // tslint:disable-next-line: no-invalid-this + await testFunc(wrapper, this); } else { - // tslint:disable-next-line:no-console - console.log(`${name} skipped, no Jupyter installed.`); + // tslint:disable-next-line:no-invalid-this + this.skip(); } }); } From d3f04f6fe019d2e1b010aebf4db718214f4cfcb5 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Wed, 4 Mar 2020 08:37:21 -0800 Subject: [PATCH 03/33] Change path for conda environments --- build/ci/templates/test_phases.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build/ci/templates/test_phases.yml b/build/ci/templates/test_phases.yml index 291f653d0eb0..6edf58f23c82 100644 --- a/build/ci/templates/test_phases.yml +++ b/build/ci/templates/test_phases.yml @@ -119,8 +119,8 @@ steps: # This task will only run if variable `NeedsPythonFunctionalReqs` is true. # - script: | - conda env create --quiet --force --file conda_env_1.yml - conda env create --quiet --force --file conda_env_2.yml + 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')) From 11c609709ac9b9de59c903234ef827056ebf147c Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Wed, 4 Mar 2020 09:08:43 -0800 Subject: [PATCH 04/33] Add pip requirements --- build/ci/conda_env_1.yml | 4 +--- build/ci/conda_env_2.yml | 4 +--- build/ci/templates/test_phases.yml | 9 +++++++++ build/conda-functional-requirements.txt | 3 +++ 4 files changed, 14 insertions(+), 6 deletions(-) create mode 100644 build/conda-functional-requirements.txt diff --git a/build/ci/conda_env_1.yml b/build/ci/conda_env_1.yml index 29a2158352b3..7949c0f767f8 100644 --- a/build/ci/conda_env_1.yml +++ b/build/ci/conda_env_1.yml @@ -4,6 +4,4 @@ dependencies: - pandas - jupyter - numpy - - matplotlib - - livelossplot - - versioneer \ No newline at end of file + - matplotlib \ No newline at end of file diff --git a/build/ci/conda_env_2.yml b/build/ci/conda_env_2.yml index 9d74a41333a7..6c1fb6a0d335 100644 --- a/build/ci/conda_env_2.yml +++ b/build/ci/conda_env_2.yml @@ -4,6 +4,4 @@ dependencies: - pandas - jupyter - numpy - - matplotlib - - livelossplot - - versioneer \ No newline at end of file + - matplotlib \ No newline at end of file diff --git a/build/ci/templates/test_phases.yml b/build/ci/templates/test_phases.yml index 6edf58f23c82..3ab3470a9cd6 100644 --- a/build/ci/templates/test_phases.yml +++ b/build/ci/templates/test_phases.yml @@ -124,6 +124,15 @@ steps: displayName: 'Create CONDA Environments' condition: and(succeeded(), eq(variables['NeedsPythonFunctionalReqs'], 'true')) + # Run the pip installs in the two environments + - bash: | + 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 + displayName: 'Install Pip requirements for CONDA envs' + condition: and(succeeded(), eq(variables['NeedsPythonFunctionalReqs'], 'true')) + # Downgrade pywin32 on Windows due to bug https://github.com/jupyter/notebook/issues/4909 # # This task will only run if variable `NeedsPythonFunctionalReqs` is true. diff --git a/build/conda-functional-requirements.txt b/build/conda-functional-requirements.txt new file mode 100644 index 000000000000..a065e22f21ac --- /dev/null +++ b/build/conda-functional-requirements.txt @@ -0,0 +1,3 @@ +# List of requirements for conda environments that cannot be installed using conda +livelossplot +versioneer \ No newline at end of file From db60dedee58171a8ea7da3dddc96c711c24e343b Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Wed, 4 Mar 2020 12:24:02 -0800 Subject: [PATCH 05/33] Get multiple interpreters test to work --- .../interactive-common/interactiveBase.ts | 7 + .../interactive-ipynb/nativeEditor.ts | 12 - .../interactive-window/interactiveWindow.ts | 3 +- .../jupyter/liveshare/guestJupyterServer.ts | 2 +- .../jupyter/liveshare/hostJupyterServer.ts | 4 +- .../datascience/dataScienceIocContainer.ts | 308 ++++++++++-------- .../interactiveWindow.functional.test.tsx | 17 +- 7 files changed, 201 insertions(+), 152 deletions(-) diff --git a/src/client/datascience/interactive-common/interactiveBase.ts b/src/client/datascience/interactive-common/interactiveBase.ts index b469af0d5f94..f32942c56262 100644 --- a/src/client/datascience/interactive-common/interactiveBase.ts +++ b/src/client/datascience/interactive-common/interactiveBase.ts @@ -323,6 +323,13 @@ export abstract class InteractiveBase extends WebViewHost 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() { diff --git a/src/client/datascience/interactive-ipynb/nativeEditor.ts b/src/client/datascience/interactive-ipynb/nativeEditor.ts index 245d14cf7c77..24b8e1acba51 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditor.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditor.ts @@ -492,18 +492,6 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { protected async close(): Promise { // 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(); - if (oldAsk && settings && settings.datascience) { - settings.datascience.askForKernelRestart = true; - } } protected saveAll() { diff --git a/src/client/datascience/interactive-window/interactiveWindow.ts b/src/client/datascience/interactive-window/interactiveWindow.ts index d992136830d8..16c9446e7ae2 100644 --- a/src/client/datascience/interactive-window/interactiveWindow.ts +++ b/src/client/datascience/interactive-window/interactiveWindow.ts @@ -149,10 +149,11 @@ export class InteractiveWindow extends InteractiveBase implements IInteractiveWi } public dispose() { - super.dispose(); + const promise = super.dispose(); if (this.closedEvent) { this.closedEvent.fire(this); } + return promise; } public addMessage(message: string): Promise { diff --git a/src/client/datascience/jupyter/liveshare/guestJupyterServer.ts b/src/client/datascience/jupyter/liveshare/guestJupyterServer.ts index 79ec86f16be3..4a571ac1635c 100644 --- a/src/client/datascience/jupyter/liveshare/guestJupyterServer.ts +++ b/src/client/datascience/jupyter/liveshare/guestJupyterServer.ts @@ -74,7 +74,7 @@ export class GuestJupyterServer this.dataScience.activationStartTime ); this.notebooks.set(identity.toString(), result); - const oldDispose = result.dispose; + const oldDispose = result.dispose.bind(result); result.dispose = () => { this.notebooks.delete(identity.toString()); return oldDispose(); diff --git a/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts b/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts index b510f2be3e13..3932895e36c0 100644 --- a/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts +++ b/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts @@ -10,6 +10,7 @@ import * as vsls from 'vsls/vscode'; import { nbformat } from '@jupyterlab/coreutils'; import { IApplicationShell, ILiveShareApi, IWorkspaceService } from '../../../common/application/types'; +import { isTestExecution } from '../../../common/constants'; import { traceInfo } from '../../../common/logger'; import { IFileSystem } from '../../../common/platform/types'; import { @@ -266,7 +267,7 @@ export class HostJupyterServer extends LiveShareParticipantHost(JupyterServerBas resource, sessionManager, notebookMetadata, - false, + isTestExecution(), cancelToken ) : this.kernelSelector.getKernelForRemoteConnection( @@ -279,6 +280,7 @@ export class HostJupyterServer extends LiveShareParticipantHost(JupyterServerBas const kernelInfoToUse = kernelInfo?.kernelSpec || kernelInfo?.kernelModel; if (kernelInfoToUse) { launchInfo.kernelSpec = kernelInfoToUse; + launchInfo.interpreter = resourceInterpreter; changedKernel = true; } } diff --git a/src/test/datascience/dataScienceIocContainer.ts b/src/test/datascience/dataScienceIocContainer.ts index c40c6d36bb2a..ebbe761ace64 100644 --- a/src/test/datascience/dataScienceIocContainer.ts +++ b/src/test/datascience/dataScienceIocContainer.ts @@ -147,7 +147,9 @@ import { IPersistentStateFactory, IPythonExtensionBanner, IsWindows, + Product, ProductType, + Resource, WORKSPACE_MEMENTO } from '../../client/common/types'; import { Deferred, sleep } from '../../client/common/utils/async'; @@ -287,7 +289,6 @@ import { } from '../../client/interpreter/contracts'; import { ShebangCodeLensProvider } from '../../client/interpreter/display/shebangCodeLensProvider'; import { InterpreterHelper } from '../../client/interpreter/helpers'; -import { InterpreterService } from '../../client/interpreter/interpreterService'; import { InterpreterVersionService } from '../../client/interpreter/interpreterVersion'; import { PythonInterpreterLocatorService } from '../../client/interpreter/locators'; import { InterpreterLocatorHelper } from '../../client/interpreter/locators/helpers'; @@ -320,6 +321,7 @@ import { } from '../../client/interpreter/locators/services/workspaceVirtualEnvService'; import { WorkspaceVirtualEnvWatcherService } from '../../client/interpreter/locators/services/workspaceVirtualEnvWatcherService'; import { IPipEnvServiceHelper, IPythonInPathCommandProvider } from '../../client/interpreter/locators/types'; +import { registerInterpreterTypes } from '../../client/interpreter/serviceRegistry'; import { VirtualEnvironmentManager } from '../../client/interpreter/virtualEnvs'; import { IVirtualEnvironmentManager } from '../../client/interpreter/virtualEnvs/types'; import { LanguageServerSurveyBanner } from '../../client/languageServices/languageServerSurveyBanner'; @@ -480,18 +482,10 @@ export class DataScienceIocContainer extends UnitTestIocContainer { this.serviceManager.addSingleton(IThemeFinder, ThemeFinder); this.serviceManager.addSingleton(ICodeCssGenerator, CodeCssGenerator); this.serviceManager.addSingleton(IStatusProvider, StatusProvider); - this.serviceManager.add( - IKnownSearchPathsForInterpreters, - KnownSearchPathsForInterpreters - ); this.serviceManager.addSingletonInstance( IAsyncDisposableRegistry, this.asyncRegistry ); - this.serviceManager.addSingleton( - IPythonInPathCommandProvider, - PythonInPathCommandProvider - ); this.serviceManager.addSingleton( IEnvironmentActivationService, EnvironmentActivationService @@ -571,7 +565,6 @@ export class DataScienceIocContainer extends UnitTestIocContainer { TerminalActivationProviders.pipenv ); this.serviceManager.addSingleton(ITerminalManager, TerminalManager); - this.serviceManager.addSingleton(IPipEnvServiceHelper, PipEnvServiceHelper); //const configuration = this.serviceManager.get(IConfigurationService); //const pythonSettings = configuration.getSettings(); @@ -621,13 +614,6 @@ export class DataScienceIocContainer extends UnitTestIocContainer { this.serviceManager.add(IGatherExecution, GatherExecution); this.serviceManager.addSingleton(ICodeLensFactory, CodeLensFactory); this.serviceManager.addSingleton(IShellDetector, TerminalNameShellDetector); - this.serviceManager.addSingleton( - InterpeterHashProviderFactory, - InterpeterHashProviderFactory - ); - this.serviceManager.addSingleton(WindowsStoreInterpreter, WindowsStoreInterpreter); - this.serviceManager.addSingleton(InterpreterHashProvider, InterpreterHashProvider); - this.serviceManager.addSingleton(InterpreterFilter, InterpreterFilter); this.serviceManager.addSingleton(JupyterCommandFinder, JupyterCommandFinder); this.serviceManager.addSingleton( IDiagnosticsService, @@ -647,8 +633,6 @@ export class DataScienceIocContainer extends UnitTestIocContainer { this.serviceManager.addSingleton(NotebookStarter, NotebookStarter); this.serviceManager.addSingleton(KernelSelector, KernelSelector); this.serviceManager.addSingleton(KernelSelectionProvider, KernelSelectionProvider); - this.serviceManager.addSingleton(IInterpreterSelector, InterpreterSelector); - this.serviceManager.addSingleton(IShebangCodeLensProvider, ShebangCodeLensProvider); this.serviceManager.addSingleton(IProductService, ProductService); this.serviceManager.addSingleton( IProductPathService, @@ -842,20 +826,6 @@ export class DataScienceIocContainer extends UnitTestIocContainer { IEnvironmentVariablesProvider, EnvironmentVariablesProvider ); - this.serviceManager.addSingleton( - IVirtualEnvironmentsSearchPathProvider, - GlobalVirtualEnvironmentsSearchPathProvider, - 'global' - ); - this.serviceManager.addSingleton( - IVirtualEnvironmentsSearchPathProvider, - WorkspaceVirtualEnvironmentsSearchPathProvider, - 'workspace' - ); - this.serviceManager.addSingleton( - IVirtualEnvironmentManager, - VirtualEnvironmentManager - ); this.serviceManager.addSingletonInstance(IApplicationShell, appShell.object); this.serviceManager.addSingletonInstance(IDocumentManager, this.documentManager); @@ -873,75 +843,6 @@ export class DataScienceIocContainer extends UnitTestIocContainer { this.serviceManager.addSingleton(IPathUtils, PathUtils); this.serviceManager.addSingletonInstance(IsWindows, IS_WINDOWS); - this.serviceManager.add( - IInterpreterWatcher, - WorkspaceVirtualEnvWatcherService, - WORKSPACE_VIRTUAL_ENV_SERVICE - ); - this.serviceManager.addSingleton( - IInterpreterWatcherBuilder, - InterpreterWatcherBuilder - ); - - this.serviceManager.addSingleton( - IInterpreterLocatorService, - PythonInterpreterLocatorService, - INTERPRETER_LOCATOR_SERVICE - ); - this.serviceManager.addSingleton( - IInterpreterLocatorService, - CondaEnvFileService, - CONDA_ENV_FILE_SERVICE - ); - this.serviceManager.addSingleton( - IInterpreterLocatorService, - CondaEnvService, - CONDA_ENV_SERVICE - ); - this.serviceManager.addSingleton( - IInterpreterLocatorService, - CurrentPathService, - CURRENT_PATH_SERVICE - ); - this.serviceManager.addSingleton( - IInterpreterLocatorService, - GlobalVirtualEnvService, - GLOBAL_VIRTUAL_ENV_SERVICE - ); - this.serviceManager.addSingleton( - IInterpreterLocatorService, - WorkspaceVirtualEnvService, - WORKSPACE_VIRTUAL_ENV_SERVICE - ); - this.serviceManager.addSingleton( - IInterpreterLocatorService, - PipEnvService, - PIPENV_SERVICE - ); - this.serviceManager.addSingleton(IPipEnvService, PipEnvService); - this.serviceManager.addSingleton( - IInterpreterLocatorService, - WindowsRegistryService, - WINDOWS_REGISTRY_SERVICE - ); - - this.serviceManager.addSingleton( - IInterpreterLocatorService, - KnownPathsService, - KNOWN_PATH_SERVICE - ); - - this.serviceManager.addSingleton(IInterpreterHelper, InterpreterHelper); - this.serviceManager.addSingleton( - IInterpreterLocatorHelper, - InterpreterLocatorHelper - ); - this.serviceManager.addSingleton(IInterpreterComparer, InterpreterComparer); - this.serviceManager.addSingleton( - IInterpreterVersionService, - InterpreterVersionService - ); - const globalStorage = this.serviceManager.get(IMemento, GLOBAL_MEMENTO); const localStorage = this.serviceManager.get(IMemento, WORKSPACE_MEMENTO); @@ -956,15 +857,6 @@ export class DataScienceIocContainer extends UnitTestIocContainer { this.serviceManager.addSingletonInstance(IInterpreterDisplay, interpreterDisplay.object); - this.serviceManager.addSingleton( - IPythonPathUpdaterServiceFactory, - PythonPathUpdaterServiceFactory - ); - this.serviceManager.addSingleton( - IPythonPathUpdaterServiceManager, - PythonPathUpdaterService - ); - const currentProcess = new CurrentProcess(); this.serviceManager.addSingletonInstance(ICurrentProcess, currentProcess); this.serviceManager.addSingleton(IRegistry, RegistryImplementation); @@ -1018,15 +910,6 @@ export class DataScienceIocContainer extends UnitTestIocContainer { ); } - // Don't use conda at all during functional tests. - const condaService = TypeMoq.Mock.ofType(); - this.serviceManager.addSingletonInstance(ICondaService, condaService.object); - condaService.setup(c => c.isCondaAvailable()).returns(() => Promise.resolve(false)); - condaService - .setup(c => c.isCondaEnvironment(TypeMoq.It.isValue(pythonPath))) - .returns(() => Promise.resolve(false)); - condaService.setup(c => c.condaEnvironmentsFile).returns(() => undefined); - // Create our jupyter mock if necessary if (this.shouldMockJupyter) { this.jupyterMock = new MockJupyterManagerFactory(this.serviceManager); @@ -1034,12 +917,150 @@ export class DataScienceIocContainer extends UnitTestIocContainer { const kernelService = mock(KernelService); when(kernelService.searchAndRegisterKernel(anything(), anything())).thenResolve(undefined); this.serviceManager.addSingletonInstance(KernelService, instance(kernelService)); + this.serviceManager.addSingleton( + InterpeterHashProviderFactory, + InterpeterHashProviderFactory + ); + this.serviceManager.addSingleton(WindowsStoreInterpreter, WindowsStoreInterpreter); + this.serviceManager.addSingleton(InterpreterHashProvider, InterpreterHashProvider); + this.serviceManager.addSingleton(InterpreterFilter, InterpreterFilter); + this.serviceManager.add( + IInterpreterWatcher, + WorkspaceVirtualEnvWatcherService, + WORKSPACE_VIRTUAL_ENV_SERVICE + ); + this.serviceManager.addSingleton( + IInterpreterWatcherBuilder, + InterpreterWatcherBuilder + ); + this.serviceManager.add( + IInterpreterWatcher, + WorkspaceVirtualEnvWatcherService, + WORKSPACE_VIRTUAL_ENV_SERVICE + ); + this.serviceManager.addSingleton( + IInterpreterWatcherBuilder, + InterpreterWatcherBuilder + ); + + this.serviceManager.addSingleton( + IInterpreterLocatorService, + PythonInterpreterLocatorService, + INTERPRETER_LOCATOR_SERVICE + ); + this.serviceManager.addSingleton( + IInterpreterLocatorService, + CondaEnvFileService, + CONDA_ENV_FILE_SERVICE + ); + this.serviceManager.addSingleton( + IInterpreterLocatorService, + CondaEnvService, + CONDA_ENV_SERVICE + ); + this.serviceManager.addSingleton( + IInterpreterLocatorService, + CurrentPathService, + CURRENT_PATH_SERVICE + ); + this.serviceManager.addSingleton( + IInterpreterLocatorService, + GlobalVirtualEnvService, + GLOBAL_VIRTUAL_ENV_SERVICE + ); + this.serviceManager.addSingleton( + IInterpreterLocatorService, + WorkspaceVirtualEnvService, + WORKSPACE_VIRTUAL_ENV_SERVICE + ); + this.serviceManager.addSingleton( + IInterpreterLocatorService, + PipEnvService, + PIPENV_SERVICE + ); + this.serviceManager.addSingleton(IPipEnvService, PipEnvService); + this.serviceManager.addSingleton( + IInterpreterLocatorService, + WindowsRegistryService, + WINDOWS_REGISTRY_SERVICE + ); + + this.serviceManager.addSingleton( + IInterpreterLocatorService, + KnownPathsService, + KNOWN_PATH_SERVICE + ); + + this.serviceManager.addSingleton(IInterpreterHelper, InterpreterHelper); + this.serviceManager.addSingleton( + IInterpreterLocatorHelper, + InterpreterLocatorHelper + ); + this.serviceManager.addSingleton(IInterpreterComparer, InterpreterComparer); + this.serviceManager.addSingleton( + IInterpreterVersionService, + InterpreterVersionService + ); + this.serviceManager.addSingleton( + IPythonInPathCommandProvider, + PythonInPathCommandProvider + ); + + this.serviceManager.addSingleton(IPipEnvServiceHelper, PipEnvServiceHelper); + this.serviceManager.addSingleton(IInterpreterSelector, InterpreterSelector); + this.serviceManager.addSingleton( + IShebangCodeLensProvider, + ShebangCodeLensProvider + ); + this.serviceManager.addSingleton( + IPythonPathUpdaterServiceFactory, + PythonPathUpdaterServiceFactory + ); + this.serviceManager.addSingleton( + IPythonPathUpdaterServiceManager, + PythonPathUpdaterService + ); + + // Don't use conda at all when mocking + const condaService = TypeMoq.Mock.ofType(); + this.serviceManager.addSingletonInstance(ICondaService, condaService.object); + condaService.setup(c => c.isCondaAvailable()).returns(() => Promise.resolve(false)); + condaService + .setup(c => c.isCondaEnvironment(TypeMoq.It.isValue(pythonPath))) + .returns(() => Promise.resolve(false)); + condaService.setup(c => c.condaEnvironmentsFile).returns(() => undefined); + + this.serviceManager.addSingleton( + IVirtualEnvironmentsSearchPathProvider, + GlobalVirtualEnvironmentsSearchPathProvider, + 'global' + ); + this.serviceManager.addSingleton( + IVirtualEnvironmentsSearchPathProvider, + WorkspaceVirtualEnvironmentsSearchPathProvider, + 'workspace' + ); + this.serviceManager.addSingleton( + IVirtualEnvironmentManager, + VirtualEnvironmentManager + ); + this.serviceManager.add( + IKnownSearchPathsForInterpreters, + KnownSearchPathsForInterpreters + ); + this.serviceManager.addSingleton( + IPythonInPathCommandProvider, + PythonInPathCommandProvider + ); } else { this.serviceManager.addSingleton(IInstaller, ProductInstaller); this.serviceManager.addSingleton(KernelService, KernelService); this.serviceManager.addSingleton(IProcessServiceFactory, ProcessServiceFactory); this.serviceManager.addSingleton(IPythonExecutionFactory, PythonExecutionFactory); - this.serviceManager.addSingleton(IInterpreterService, InterpreterService); + + // Make sure full interpreter services are available. + registerInterpreterTypes(this.serviceManager); + this.serviceManager.addSingleton( IJupyterSessionManagerFactory, JupyterSessionManagerFactory @@ -1079,8 +1100,13 @@ export class DataScienceIocContainer extends UnitTestIocContainer { const interpreterManager = this.serviceContainer.get(IInterpreterService); interpreterManager.initialize(); - this.addInterpreter(this.workingPython2, SupportedCommands.all); - this.addInterpreter(this.workingPython, SupportedCommands.all); + if (this.mockJupyter) { + this.addInterpreter(this.workingPython2, SupportedCommands.all); + this.addInterpreter(this.workingPython, SupportedCommands.all); + } else { + // When not mocking jupyter, see if the active intepreter supports jupyter or not + this.forceJupyterInterpreter(); + } } public setFileContents(uri: Uri, contents: string) { const fileSystem = this.serviceManager.get(IFileSystem) as MockFileSystem; @@ -1093,6 +1119,16 @@ export class DataScienceIocContainer extends UnitTestIocContainer { IExtensionSingleActivationService ); await Promise.all(activationServices.map(a => a.activate())); + + // Then force our interpreter to be one that supports jupyter (unless in a mock state when we don't have to) + if (!this.mockJupyter) { + const interpreterService = this.serviceManager.get(IInterpreterService); + const activeInterpreter = await interpreterService.getActiveInterpreter(); + if (!activeInterpreter || !(await this.hasJupyter(activeInterpreter))) { + const jupyterInterpreters = await this.getJupyterInterpreters(); + await this.addNewSetting(undefined); + } + } } public createWebPanel(): IWebPanel { @@ -1200,18 +1236,17 @@ export class DataScienceIocContainer extends UnitTestIocContainer { public async getJupyterInterpreters(): Promise { const list = await this.get(IInterpreterService).getInterpreters(undefined); - return list.filter(f => this.hasJupyter(f.path)); + const promises = list.map(f => this.hasJupyter(f).then(b => (b ? f : undefined))); + const resolved = await Promise.all(promises); + return resolved.filter(r => r) as PythonInterpreter[]; } - public async addNewSetting(resource: Uri, pythonPath: string | undefined) { + public async addNewSetting(resource: Uri | undefined, pythonPath: string | undefined) { // Force a new config setting to appear. if (!pythonPath) { const active = await this.get(IInterpreterService).getActiveInterpreter(undefined); - const list = await this.get(IInterpreterService).getInterpreters(undefined); - - // Should support jupyter? How to enforce this - const supportsJupyter = list.filter(l => l.path !== active?.path).filter(f => this.hasJupyter(f.path)); - pythonPath = supportsJupyter ? supportsJupyter[0].path : undefined; + const list = await this.getJupyterInterpreters(); + pythonPath = list.filter(l => l.path !== active?.path)[0].path; } if (pythonPath) { const newSettings = { ...this.pythonSettings, pythonPath }; @@ -1267,11 +1302,16 @@ export class DataScienceIocContainer extends UnitTestIocContainer { } } - private hasJupyter(pythonPath: string) { + private getResourceKey(resource: Resource): string { + const workspace = this.serviceManager.get(IWorkspaceService); + const workspaceFolderUri = PythonSettings.getSettingsUriAndTarget(resource, workspace).uri; + return workspaceFolderUri ? workspaceFolderUri.fsPath : ''; + } + + private async hasJupyter(interpreter: PythonInterpreter): Promise { try { - // Try importing jupyter - const output = child_process.execFileSync(pythonPath, ['-c', 'import jupyter;'], { encoding: 'utf8' }); - return !output.includes('ModuleNotFoundError'); + const installer = this.serviceManager.get(IInstaller); + return installer.isInstalled(Product.ipykernel, interpreter); } catch (ex) { return false; } diff --git a/src/test/datascience/interactiveWindow.functional.test.tsx b/src/test/datascience/interactiveWindow.functional.test.tsx index cacd83417520..301c93be964f 100644 --- a/src/test/datascience/interactiveWindow.functional.test.tsx +++ b/src/test/datascience/interactiveWindow.functional.test.tsx @@ -579,6 +579,14 @@ Type: builtin_function_or_method`, const interactiveWindowProvider = ioc.get(IInteractiveWindowProvider); const interpreterService = ioc.get(IInterpreterService); const interpreters = await ioc.getJupyterInterpreters(); + if (interpreters.length < 2) { + // tslint:disable-next-line: no-console + console.log( + 'Multiple interpreters skipped because local machine does not have more than one jupyter environment' + ); + context.skip(); + return; + } const window = (await interactiveWindowProvider.getOrCreateActive()) as InteractiveWindow; await addCode(ioc, wrapper, 'a=1\na'); const activeInterpreter = await interpreterService.getActiveInterpreter( @@ -594,17 +602,20 @@ Type: builtin_function_or_method`, // Add another python path const secondUri = Uri.file('bar.py'); - await ioc.addNewSetting(secondUri, interpreters[1].path); + await ioc.addNewSetting( + secondUri, + interpreters.filter(i => i.path !== activeInterpreter?.path)[0].path + ); const newWrapper = mountWebView(ioc, 'interactive'); assert.ok(newWrapper, 'Could not mount a second time'); const newWindow = (await interactiveWindowProvider.getOrCreateActive()) as InteractiveWindow; - await addCode(ioc, wrapper, 'a=1\na', false, secondUri); - verifyHtmlOnCell(wrapper, 'InteractiveCell', '1', CellPosition.Last); + await addCode(ioc, newWrapper, 'a=1\na', false, secondUri); assert.notEqual( newWindow.notebook!.getMatchingInterpreter()?.path, activeInterpreter?.path, 'Active intrepreter used to launch second notebook when it should not have' ); + verifyHtmlOnCell(newWrapper, 'InteractiveCell', '1', CellPosition.Last); } else { context.skip(); } From fca098eb903d1cc287e3280ae8529837d45fdc98 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Wed, 4 Mar 2020 17:13:08 -0800 Subject: [PATCH 06/33] Partially working conda search --- src/client/common/terminal/helper.ts | 9 +- .../interpreter/jupyterInterpreterService.ts | 17 +- .../datascience/dataScienceIocContainer.ts | 248 ++++++++++-------- .../dataviewer.functional.test.tsx | 3 +- .../datascience/debugger.functional.test.tsx | 1 + .../gotocell.functional.test.ts | 3 +- .../errorHandler.functional.test.tsx | 3 +- .../intellisense.functional.test.tsx | 5 +- .../interactiveWindow.functional.test.tsx | 11 +- .../datascience/liveshare.functional.test.tsx | 3 +- src/test/datascience/mockWorkspaceConfig.ts | 22 +- .../nativeEditor.functional.test.tsx | 51 ++-- .../datascience/notebook.functional.test.ts | 5 +- .../plotViewer.functional.test.tsx | 3 +- .../variableexplorer.functional.test.tsx | 3 +- src/test/unittests.ts | 14 +- 16 files changed, 228 insertions(+), 173 deletions(-) diff --git a/src/client/common/terminal/helper.ts b/src/client/common/terminal/helper.ts index 5d6ff1ad06f5..b56be21ed664 100644 --- a/src/client/common/terminal/helper.ts +++ b/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 ): Promise { let hasCommands = false; - const interpreter = await this.interpreterService.getActiveInterpreter(resource); let failed = false; try { const cmds = await promise; diff --git a/src/client/datascience/jupyter/interpreter/jupyterInterpreterService.ts b/src/client/datascience/jupyter/interpreter/jupyterInterpreterService.ts index aec8156a216d..bff29827eb28 100644 --- a/src/client/datascience/jupyter/interpreter/jupyterInterpreterService.ts +++ b/src/client/datascience/jupyter/interpreter/jupyterInterpreterService.ts @@ -139,6 +139,15 @@ export class JupyterInterpreterService { } } + // Set the specified interpreter as our current selected interpreter. Public so can + // be set by the test code. + public async setAsSelectedInterpreter(interpreter: PythonInterpreter): Promise { + // Make sure that our initial set has happened before we allow a set so that + // calculation of the initial interpreter doesn't clobber the existing one + await this.setInitialInterpreter(); + this.changeSelectedInterpreterProperty(interpreter); + } + // Check the location that we stored jupyter launch path in the old version // if it's there, return it and clear the location private getInterpreterFromChangeOfOlderVersionOfExtension(): string | undefined { @@ -152,14 +161,6 @@ export class JupyterInterpreterService { return pythonPath; } - // Set the specified interpreter as our current selected interpreter - private async setAsSelectedInterpreter(interpreter: PythonInterpreter): Promise { - // Make sure that our initial set has happened before we allow a set so that - // calculation of the initial interpreter doesn't clobber the existing one - await this.setInitialInterpreter(); - this.changeSelectedInterpreterProperty(interpreter); - } - private changeSelectedInterpreterProperty(interpreter: PythonInterpreter) { this._selectedInterpreter = interpreter; this._onDidChangeInterpreter.fire(interpreter); diff --git a/src/test/datascience/dataScienceIocContainer.ts b/src/test/datascience/dataScienceIocContainer.ts index ebbe761ace64..f981c9cf70dd 100644 --- a/src/test/datascience/dataScienceIocContainer.ts +++ b/src/test/datascience/dataScienceIocContainer.ts @@ -18,7 +18,6 @@ import { FileSystemWatcher, Memento, Uri, - WorkspaceConfiguration, WorkspaceFolder, WorkspaceFoldersChangeEvent } from 'vscode'; @@ -349,21 +348,31 @@ import { TestNativeEditorProvider } from './testNativeEditorProvider'; import { TestPersistentStateFactory } from './testPersistentStateFactory'; export class DataScienceIocContainer extends UnitTestIocContainer { + public get workingInterpreter() { + return this.workingPython; + } + + public get workingInterpreter2() { + return this.workingPython2; + } + + public get onContextSet(): Event<{ name: string; value: boolean }> { + return this.contextSetEvent.event; + } + + public get mockJupyter(): MockJupyterManager | undefined { + return this.jupyterMock ? this.jupyterMock.getManager() : undefined; + } public webPanelListener: IWebPanelMessageListener | undefined; public readonly useCommandFinderForJupyterServer = false; public wrapper: ReactWrapper, React.Component> | undefined; public wrapperCreatedPromise: Deferred | undefined; public postMessage: ((ev: MessageEvent) => void) | undefined; - public mockedWorkspaceConfig!: WorkspaceConfiguration; public applicationShell!: TypeMoq.IMock; // tslint:disable-next-line:no-any public datascience!: TypeMoq.IMock; + public pythonWorkspaceConfig = new MockWorkspaceConfiguration(); private missedMessages: any[] = []; - private pythonSettings = new (class extends PythonSettings { - public fireChangeEvent() { - this.changed.fire(); - } - })(undefined, new MockAutoSelectionService()); private commandManager: MockCommandManager = new MockCommandManager(); private setContexts: Record = {}; private contextSetEvent: EventEmitter<{ name: string; value: boolean }> = new EventEmitter<{ @@ -398,6 +407,8 @@ export class DataScienceIocContainer extends UnitTestIocContainer { private webPanelProvider: TypeMoq.IMock | undefined; private settingsMap = new Map(); + private workspaceFolders: WorkspaceFolder[] = []; + private workspaceService: WorkspaceService | undefined; constructor() { super(); @@ -407,18 +418,6 @@ export class DataScienceIocContainer extends UnitTestIocContainer { this.asyncRegistry = new AsyncDisposableRegistry(); } - public get workingInterpreter() { - return this.workingPython; - } - - public get workingInterpreter2() { - return this.workingPython2; - } - - public get onContextSet(): Event<{ name: string; value: boolean }> { - return this.contextSetEvent.event; - } - public async dispose(): Promise { await this.asyncRegistry.dispose(); await super.dispose(); @@ -458,7 +457,9 @@ export class DataScienceIocContainer extends UnitTestIocContainer { //tslint:disable:max-func-body-length public registerDataScienceTypes(useCustomEditor: boolean = false) { - const testWorkspaceFolder = path.join(EXTENSION_ROOT_DIR, 'src', 'test', 'datascience'); + // Make sure the default python path is set. + const pythonPath = this.findPythonPath(); + this.pythonWorkspaceConfig.update('pythonPath', pythonPath).ignoreErrors(); this.registerFileSystemTypes(); this.serviceManager.rebindInstance(IFileSystem, new MockFileSystem()); @@ -721,16 +722,18 @@ export class DataScienceIocContainer extends UnitTestIocContainer { }); this.serviceManager.addSingletonInstance(ICommandManager, this.commandManager); - // Also setup a mock execution service and interpreter service + // Mock the app shell and workspace service + this.workspaceService = this.createWorkspaceService(); const appShell = (this.applicationShell = TypeMoq.Mock.ofType()); - // const workspaceService = TypeMoq.Mock.ofType(); - const workspaceService = mock(WorkspaceService); const configurationService = TypeMoq.Mock.ofType(); const interpreterDisplay = TypeMoq.Mock.ofType(); this.datascience = TypeMoq.Mock.ofType(); + configurationService.setup(c => c.getSettings(TypeMoq.It.isAny())).returns(this.getSettings.bind(this)); + // Setup default settings - this.pythonSettings.datascience = { + const pythonSettings = this.getSettings(undefined); + pythonSettings.datascience = { allowImportFromNotebook: true, jupyterLaunchTimeout: 60000, jupyterLaunchRetries: 3, @@ -762,66 +765,26 @@ export class DataScienceIocContainer extends UnitTestIocContainer { variableQueries: [], jupyterCommandLineArguments: [] }; - this.pythonSettings.jediEnabled = false; - this.pythonSettings.downloadLanguageServer = false; - - const workspaceConfig = (this.mockedWorkspaceConfig = mock(MockWorkspaceConfiguration)); - configurationService.setup(c => c.getSettings(TypeMoq.It.isAny())).returns(this.getSettings.bind(this)); - when(workspaceConfig.get(anything(), anything())).thenCall((_, defaultValue) => defaultValue); - when(workspaceConfig.has(anything())).thenReturn(false); - when((workspaceConfig as any).then).thenReturn(undefined); - when(workspaceService.getConfiguration(anything())).thenReturn(instance(workspaceConfig)); - when(workspaceService.getConfiguration(anything(), anything())).thenReturn(instance(workspaceConfig)); - when(workspaceService.onDidChangeConfiguration).thenReturn(this.configChangeEvent.event); - when(workspaceService.onDidChangeWorkspaceFolders).thenReturn(this.worksaceFoldersChangedEvent.event); - - interpreterDisplay.setup(i => i.refresh(TypeMoq.It.isAny())).returns(() => Promise.resolve()); - const startTime = Date.now(); - this.datascience.setup(d => d.activationStartTime).returns(() => startTime); - - class MockFileSystemWatcher implements FileSystemWatcher { - public ignoreCreateEvents: boolean = false; - public ignoreChangeEvents: boolean = false; - public ignoreDeleteEvents: boolean = false; - //tslint:disable-next-line:no-any - public onDidChange(_listener: (e: Uri) => any, _thisArgs?: any, _disposables?: Disposable[]): Disposable { - return { dispose: noop }; - } - //tslint:disable-next-line:no-any - public onDidDelete(_listener: (e: Uri) => any, _thisArgs?: any, _disposables?: Disposable[]): Disposable { - return { dispose: noop }; - } - //tslint:disable-next-line:no-any - public onDidCreate(_listener: (e: Uri) => any, _thisArgs?: any, _disposables?: Disposable[]): Disposable { - return { dispose: noop }; - } - public dispose() { - noop(); - } - } - when(workspaceService.createFileSystemWatcher(anything(), anything(), anything(), anything())).thenReturn( - new MockFileSystemWatcher() - ); - when(workspaceService.createFileSystemWatcher(anything())).thenReturn(new MockFileSystemWatcher()); - when(workspaceService.hasWorkspaceFolders).thenReturn(true); - const workspaceFolder = this.createMoqWorkspaceFolder(testWorkspaceFolder); - when(workspaceService.workspaceFolders).thenReturn([workspaceFolder]); - when(workspaceService.rootPath).thenReturn('~'); - - // Look on the path for python - const pythonPath = this.findPythonPath(); - - this.pythonSettings.pythonPath = pythonPath; + pythonSettings.jediEnabled = false; + pythonSettings.downloadLanguageServer = false; const folders = ['Envs', '.virtualenvs']; - this.pythonSettings.venvFolders = folders; - this.pythonSettings.venvPath = path.join('~', 'foo'); - this.pythonSettings.terminal = { + pythonSettings.venvFolders = folders; + pythonSettings.venvPath = path.join('~', 'foo'); + pythonSettings.terminal = { executeInFileDir: false, launchArgs: [], activateEnvironment: true, activateEnvInCurrentTerminal: false }; + // Once we have our initial settings, make them the workspace config values + const keys = [...Object.keys(pythonSettings)]; + keys.forEach(k => this.pythonWorkspaceConfig.update(k, pythonSettings[k])); + + interpreterDisplay.setup(i => i.refresh(TypeMoq.It.isAny())).returns(() => Promise.resolve()); + const startTime = Date.now(); + this.datascience.setup(d => d.activationStartTime).returns(() => startTime); + this.serviceManager.addSingleton( IEnvironmentVariablesProvider, EnvironmentVariablesProvider @@ -829,7 +792,6 @@ export class DataScienceIocContainer extends UnitTestIocContainer { this.serviceManager.addSingletonInstance(IApplicationShell, appShell.object); this.serviceManager.addSingletonInstance(IDocumentManager, this.documentManager); - this.serviceManager.addSingletonInstance(IWorkspaceService, instance(workspaceService)); this.serviceManager.addSingletonInstance( IConfigurationService, configurationService.object @@ -1025,9 +987,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer { const condaService = TypeMoq.Mock.ofType(); this.serviceManager.addSingletonInstance(ICondaService, condaService.object); condaService.setup(c => c.isCondaAvailable()).returns(() => Promise.resolve(false)); - condaService - .setup(c => c.isCondaEnvironment(TypeMoq.It.isValue(pythonPath))) - .returns(() => Promise.resolve(false)); + condaService.setup(c => c.isCondaEnvironment(TypeMoq.It.isAny())).returns(() => Promise.resolve(false)); condaService.setup(c => c.condaEnvironmentsFile).returns(() => undefined); this.serviceManager.addSingleton( @@ -1103,9 +1063,6 @@ export class DataScienceIocContainer extends UnitTestIocContainer { if (this.mockJupyter) { this.addInterpreter(this.workingPython2, SupportedCommands.all); this.addInterpreter(this.workingPython, SupportedCommands.all); - } else { - // When not mocking jupyter, see if the active intepreter supports jupyter or not - this.forceJupyterInterpreter(); } } public setFileContents(uri: Uri, contents: string) { @@ -1118,6 +1075,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer { const activationServices = this.serviceManager.getAll( IExtensionSingleActivationService ); + await Promise.all(activationServices.map(a => a.activate())); // Then force our interpreter to be one that supports jupyter (unless in a mock state when we don't have to) @@ -1125,8 +1083,13 @@ export class DataScienceIocContainer extends UnitTestIocContainer { const interpreterService = this.serviceManager.get(IInterpreterService); const activeInterpreter = await interpreterService.getActiveInterpreter(); if (!activeInterpreter || !(await this.hasJupyter(activeInterpreter))) { - const jupyterInterpreters = await this.getJupyterInterpreters(); - await this.addNewSetting(undefined); + const list = await this.getJupyterInterpreters(); + this.forceSettingsChanged(undefined, list[0].path); + + // Also set this as the interpreter to use for jupyter + await this.serviceManager + .get(JupyterInterpreterService) + .setAsSelectedInterpreter(list[0]); } } } @@ -1204,12 +1167,6 @@ export class DataScienceIocContainer extends UnitTestIocContainer { this.mountReactControl(mount); } - public createMoqWorkspaceFolder(folderPath: string) { - const folder = TypeMoq.Mock.ofType(); - folder.setup(f => f.uri).returns(() => Uri.file(folderPath)); - return folder.object; - } - public getContext(name: string): boolean { if (this.setContexts.hasOwnProperty(name)) { return this.setContexts[name]; @@ -1219,14 +1176,24 @@ export class DataScienceIocContainer extends UnitTestIocContainer { } public getSettings(resource?: Uri) { - const setting = resource ? this.settingsMap.get(resource.toString()) : this.pythonSettings; - return setting ? setting : this.pythonSettings; + const key = this.getResourceKey(resource); + let setting = this.settingsMap.get(key); + if (!setting) { + setting = new (class extends PythonSettings { + public fireChangeEvent() { + this.changed.fire(); + } + })(resource, new MockAutoSelectionService(), this.serviceManager.get(IWorkspaceService)); + this.settingsMap.set(key, setting); + } + return setting; } - public forceSettingsChanged(newPath: string, datascienceSettings?: IDataScienceSettings) { - this.pythonSettings.pythonPath = newPath; - this.pythonSettings.datascience = datascienceSettings ? datascienceSettings : this.pythonSettings.datascience; - this.pythonSettings.fireChangeEvent(); + public forceSettingsChanged(resource: Resource, newPath: string, datascienceSettings?: IDataScienceSettings) { + const defaultSettings = this.getSettings(resource); + defaultSettings.pythonPath = newPath; + defaultSettings.datascience = datascienceSettings ? datascienceSettings : defaultSettings.datascience; + defaultSettings.fireChangeEvent(); this.configChangeEvent.fire({ affectsConfiguration(_s: string, _r?: Uri): boolean { return true; @@ -1241,21 +1208,24 @@ export class DataScienceIocContainer extends UnitTestIocContainer { return resolved.filter(r => r) as PythonInterpreter[]; } - public async addNewSetting(resource: Uri | undefined, pythonPath: string | undefined) { - // Force a new config setting to appear. - if (!pythonPath) { - const active = await this.get(IInterpreterService).getActiveInterpreter(undefined); - const list = await this.getJupyterInterpreters(); - pythonPath = list.filter(l => l.path !== active?.path)[0].path; - } - if (pythonPath) { - const newSettings = { ...this.pythonSettings, pythonPath }; - this.settingsMap.set(resource.toString(), newSettings); - } + public addWorkspaceFolder(folderPath: string) { + const index = this.workspaceFolders.length; + const folder = TypeMoq.Mock.ofType(); + folder.setup(f => f.uri).returns(() => Uri.file(folderPath)); + folder.setup(f => f.index).returns(() => index); + folder.setup(f => f.name).returns(() => folderPath); + this.workspaceFolders.push(folder.object); + return folder.object; } - public get mockJupyter(): MockJupyterManager | undefined { - return this.jupyterMock ? this.jupyterMock.getManager() : undefined; + public addResourceToFolder(resource: Uri, folderPath: string) { + let folder = this.workspaceFolders.find(f => f.uri.fsPath === folderPath); + if (!folder) { + folder = this.addWorkspaceFolder(folderPath); + } + if (this.workspaceService) { + when(this.workspaceService.getWorkspaceFolder(resource)).thenReturn(folder); + } } public get(serviceIdentifier: interfaces.ServiceIdentifier, name?: string | number | symbol): T { @@ -1302,6 +1272,56 @@ export class DataScienceIocContainer extends UnitTestIocContainer { } } + private createWorkspaceService() { + class MockFileSystemWatcher implements FileSystemWatcher { + public ignoreCreateEvents: boolean = false; + public ignoreChangeEvents: boolean = false; + public ignoreDeleteEvents: boolean = false; + //tslint:disable-next-line:no-any + public onDidChange(_listener: (e: Uri) => any, _thisArgs?: any, _disposables?: Disposable[]): Disposable { + return { dispose: noop }; + } + //tslint:disable-next-line:no-any + public onDidDelete(_listener: (e: Uri) => any, _thisArgs?: any, _disposables?: Disposable[]): Disposable { + return { dispose: noop }; + } + //tslint:disable-next-line:no-any + public onDidCreate(_listener: (e: Uri) => any, _thisArgs?: any, _disposables?: Disposable[]): Disposable { + return { dispose: noop }; + } + public dispose() { + noop(); + } + } + + const workspaceService = mock(WorkspaceService); + this.serviceManager.addSingletonInstance(IWorkspaceService, instance(workspaceService)); + when(workspaceService.onDidChangeConfiguration).thenReturn(this.configChangeEvent.event); + when(workspaceService.onDidChangeWorkspaceFolders).thenReturn(this.worksaceFoldersChangedEvent.event); + + // Create another config for other parts of the workspace config. + const otherConfig = mock(MockWorkspaceConfiguration); + when(otherConfig.get(anything(), anything())).thenCall((_, defaultValue) => defaultValue); + when(otherConfig.has(anything())).thenReturn(false); + when(workspaceService.getConfiguration(anything())).thenCall(key => + key === 'python' ? this.pythonWorkspaceConfig : instance(otherConfig) + ); + when(workspaceService.getConfiguration(anything(), anything())).thenCall(key => + key === 'python' ? this.pythonWorkspaceConfig : instance(otherConfig) + ); + const testWorkspaceFolder = path.join(EXTENSION_ROOT_DIR, 'src', 'test', 'datascience'); + + when(workspaceService.createFileSystemWatcher(anything(), anything(), anything(), anything())).thenReturn( + new MockFileSystemWatcher() + ); + when(workspaceService.createFileSystemWatcher(anything())).thenReturn(new MockFileSystemWatcher()); + when(workspaceService.hasWorkspaceFolders).thenReturn(true); + when(workspaceService.workspaceFolders).thenReturn(this.workspaceFolders); + when(workspaceService.rootPath).thenReturn(testWorkspaceFolder); + this.addWorkspaceFolder(testWorkspaceFolder); + return workspaceService; + } + private getResourceKey(resource: Resource): string { const workspace = this.serviceManager.get(IWorkspaceService); const workspaceFolderUri = PythonSettings.getSettingsUriAndTarget(resource, workspace).uri; @@ -1310,8 +1330,10 @@ export class DataScienceIocContainer extends UnitTestIocContainer { private async hasJupyter(interpreter: PythonInterpreter): Promise { try { - const installer = this.serviceManager.get(IInstaller); - return installer.isInstalled(Product.ipykernel, interpreter); + const dependencyChecker = this.serviceManager.get( + JupyterInterpreterDependencyService + ); + return dependencyChecker.areDependenciesInstalled(interpreter); } catch (ex) { return false; } diff --git a/src/test/datascience/dataviewer.functional.test.tsx b/src/test/datascience/dataviewer.functional.test.tsx index 43879ef0088b..f30cb782cdf9 100644 --- a/src/test/datascience/dataviewer.functional.test.tsx +++ b/src/test/datascience/dataviewer.functional.test.tsx @@ -46,9 +46,10 @@ suite('DataScience DataViewer tests', () => { } }); - setup(() => { + setup(async () => { ioc = new DataScienceIocContainer(); ioc.registerDataScienceTypes(); + return ioc.activate(); }); function mountWebView(): ReactWrapper, React.Component> { diff --git a/src/test/datascience/debugger.functional.test.tsx b/src/test/datascience/debugger.functional.test.tsx index da1476094795..3b269a55239a 100644 --- a/src/test/datascience/debugger.functional.test.tsx +++ b/src/test/datascience/debugger.functional.test.tsx @@ -54,6 +54,7 @@ suite('DataScience Debugger tests', () => { ioc = createContainer(); mockDebuggerService = ioc.serviceManager.get(IDebugService) as MockDebuggerService; processFactory = ioc.serviceManager.get(IProcessServiceFactory); + return ioc.activate(); }); teardown(async () => { diff --git a/src/test/datascience/editor-integration/gotocell.functional.test.ts b/src/test/datascience/editor-integration/gotocell.functional.test.ts index 56acb1b96bbb..fa225016db27 100644 --- a/src/test/datascience/editor-integration/gotocell.functional.test.ts +++ b/src/test/datascience/editor-integration/gotocell.functional.test.ts @@ -34,13 +34,14 @@ suite('DataScience gotocell tests', () => { let documentManager: MockDocumentManager; let visibleCells: ICell[] = []; - setup(() => { + setup(async () => { ioc = new DataScienceIocContainer(); ioc.registerDataScienceTypes(); codeLensProvider = ioc.serviceManager.get(IDataScienceCodeLensProvider); jupyterExecution = ioc.serviceManager.get(IJupyterExecution); documentManager = ioc.serviceManager.get(IDocumentManager) as MockDocumentManager; codeLensFactory = ioc.serviceManager.get(ICodeLensFactory); + await ioc.activate(); }); teardown(async () => { diff --git a/src/test/datascience/errorHandler.functional.test.tsx b/src/test/datascience/errorHandler.functional.test.tsx index 9b1bc3d6abc1..760a5cf17dfb 100644 --- a/src/test/datascience/errorHandler.functional.test.tsx +++ b/src/test/datascience/errorHandler.functional.test.tsx @@ -18,7 +18,7 @@ suite('DataScience Error Handler Functional Tests', () => { let ioc: DataScienceIocContainer; let channels: TypeMoq.IMock; let stubbedInstallMissingDependencies: sinon.SinonStub<[(JupyterInstallError | undefined)?], Promise>; - setup(() => { + setup(async () => { stubbedInstallMissingDependencies = sinon.stub( JupyterInterpreterSubCommandExecutionService.prototype, 'installMissingDependencies' @@ -26,6 +26,7 @@ suite('DataScience Error Handler Functional Tests', () => { ioc = new DataScienceIocContainer(); ioc.registerDataScienceTypes(); ioc = modifyContainer(); + return ioc.activate(); }); teardown(async () => { diff --git a/src/test/datascience/intellisense.functional.test.tsx b/src/test/datascience/intellisense.functional.test.tsx index bf4a8843ff4f..4e5388e813a1 100644 --- a/src/test/datascience/intellisense.functional.test.tsx +++ b/src/test/datascience/intellisense.functional.test.tsx @@ -19,9 +19,10 @@ suite('DataScience Intellisense tests', () => { const disposables: Disposable[] = []; let ioc: DataScienceIocContainer; - setup(() => { + setup(async () => { ioc = new DataScienceIocContainer(); ioc.registerDataScienceTypes(); + return ioc.activate(); }); teardown(async () => { @@ -153,7 +154,7 @@ suite('DataScience Intellisense tests', () => { const interpreters = await interpreterService.getInterpreters(undefined); if (interpreters.length > 1 && oldActive) { const firstOther = interpreters.filter(i => i.path !== oldActive.path); - ioc.forceSettingsChanged(firstOther[0].path); + ioc.forceSettingsChanged(undefined, firstOther[0].path); const active = await interpreterService.getActiveInterpreter(undefined); assert.notDeepEqual(active, oldActive, 'Should have changed interpreter'); } diff --git a/src/test/datascience/interactiveWindow.functional.test.tsx b/src/test/datascience/interactiveWindow.functional.test.tsx index 301c93be964f..5dd1511a4b5e 100644 --- a/src/test/datascience/interactiveWindow.functional.test.tsx +++ b/src/test/datascience/interactiveWindow.functional.test.tsx @@ -13,6 +13,7 @@ import { IApplicationShell, IDocumentManager } from '../../client/common/applica import { IDataScienceSettings } from '../../client/common/types'; import { createDeferred, waitForPromise } from '../../client/common/utils/async'; import { noop } from '../../client/common/utils/misc'; +import { EXTENSION_ROOT_DIR } from '../../client/constants'; import { generateCellsFromDocument } from '../../client/datascience/cellFactory'; import { EditorContexts } from '../../client/datascience/constants'; import { InteractiveWindowMessages } from '../../client/datascience/interactive-common/interactiveWindowTypes'; @@ -64,9 +65,10 @@ suite('DataScience Interactive Window output tests', () => { let ioc: DataScienceIocContainer; const defaultCellMarker = '# %%'; - setup(() => { + setup(async () => { ioc = new DataScienceIocContainer(); ioc.registerDataScienceTypes(); + return ioc.activate(); }); teardown(async () => { @@ -87,7 +89,7 @@ suite('DataScience Interactive Window output tests', () => { const window = await getOrCreateInteractiveWindow(ioc); await window.show(); const update = waitForMessage(ioc, InteractiveWindowMessages.SettingsUpdated); - ioc.forceSettingsChanged(ioc.getSettings().pythonPath, newSettings); + ioc.forceSettingsChanged(undefined, ioc.getSettings().pythonPath, newSettings); return update; } @@ -602,10 +604,13 @@ Type: builtin_function_or_method`, // Add another python path const secondUri = Uri.file('bar.py'); - await ioc.addNewSetting( + ioc.addResourceToFolder(secondUri, path.join(EXTENSION_ROOT_DIR, 'src', 'test', 'datascience2')); + ioc.forceSettingsChanged( secondUri, interpreters.filter(i => i.path !== activeInterpreter?.path)[0].path ); + + // Then open a second time and make sure it uses this new path const newWrapper = mountWebView(ioc, 'interactive'); assert.ok(newWrapper, 'Could not mount a second time'); const newWindow = (await interactiveWindowProvider.getOrCreateActive()) as InteractiveWindow; diff --git a/src/test/datascience/liveshare.functional.test.tsx b/src/test/datascience/liveshare.functional.test.tsx index ba3139608b89..a03a1cbec255 100644 --- a/src/test/datascience/liveshare.functional.test.tsx +++ b/src/test/datascience/liveshare.functional.test.tsx @@ -39,9 +39,10 @@ suite('DataScience LiveShare tests', () => { let guestContainer: DataScienceIocContainer; let lastErrorMessage: string | undefined; - setup(() => { + setup(async () => { hostContainer = createContainer(vsls.Role.Host); guestContainer = createContainer(vsls.Role.Guest); + return Promise.all([hostContainer.activate(), guestContainer.activate()]); }); teardown(async () => { diff --git a/src/test/datascience/mockWorkspaceConfig.ts b/src/test/datascience/mockWorkspaceConfig.ts index f71ba64e064f..4db0ebd24007 100644 --- a/src/test/datascience/mockWorkspaceConfig.ts +++ b/src/test/datascience/mockWorkspaceConfig.ts @@ -7,15 +7,18 @@ import { ConfigurationTarget, WorkspaceConfiguration } from 'vscode'; export class MockWorkspaceConfiguration implements WorkspaceConfiguration { // tslint:disable: no-any - public get(key: string): any; - public get(section: string): T | undefined; - public get(section: string, defaultValue: T): T; - public get(section: any, defaultValue?: any): any; - public get(_: string, defaultValue?: any): any { + private values = new Map(); + + public get(key: string, defaultValue?: T): T | undefined { + // tslint:disable-next-line: use-named-parameter + if (this.values.has(key)) { + return this.values.get(key); + } + return arguments.length > 1 ? defaultValue : (undefined as any); } - public has(_section: string): boolean { - return false; + public has(section: string): boolean { + return this.values.has(section); } public inspect( _section: string @@ -31,10 +34,11 @@ export class MockWorkspaceConfiguration implements WorkspaceConfiguration { return; } public update( - _section: string, - _value: any, + section: string, + value: any, _configurationTarget?: boolean | ConfigurationTarget | undefined ): Promise { + this.values.set(section, value); return Promise.resolve(); } } diff --git a/src/test/datascience/nativeEditor.functional.test.tsx b/src/test/datascience/nativeEditor.functional.test.tsx index f724679e8898..ebacfc619613 100644 --- a/src/test/datascience/nativeEditor.functional.test.tsx +++ b/src/test/datascience/nativeEditor.functional.test.tsx @@ -10,7 +10,6 @@ import { EventEmitter } from 'events'; import * as fs from 'fs-extra'; import * as path from 'path'; import * as sinon from 'sinon'; -import { anything, when } from 'ts-mockito'; import * as TypeMoq from 'typemoq'; import { Disposable, TextDocument, TextEditor, Uri, WindowState } from 'vscode'; import { IApplicationShell, ICustomEditorService, IDocumentManager } from '../../client/common/application/types'; @@ -129,6 +128,7 @@ suite('DataScience Native Editor', () => { setup(async () => { ioc = new DataScienceIocContainer(); ioc.registerDataScienceTypes(useCustomEditorApi); + await ioc.activate(); const appShell = TypeMoq.Mock.ofType(); appShell @@ -830,6 +830,7 @@ df.head()`; function initIoc() { ioc = new DataScienceIocContainer(); ioc.registerDataScienceTypes(useCustomEditorApi); + return ioc.activate(); } async function setupFunction(this: Mocha.Context, fileContents?: any) { const wrapperPossiblyUndefined = await setupWebview(ioc); @@ -969,7 +970,7 @@ df.head()`; suite('Selection/Focus', () => { setup(async function() { - initIoc(); + await initIoc(); // tslint:disable-next-line: no-invalid-this await setupFunction.call(this); }); @@ -1066,7 +1067,7 @@ df.head()`; suite('Model updates', () => { setup(async function() { - initIoc(); + await initIoc(); // tslint:disable-next-line: no-invalid-this await setupFunction.call(this); }); @@ -1328,7 +1329,7 @@ df.head()`; suite('Keyboard Shortcuts', () => { setup(async function() { (window.navigator as any).platform = originalPlatform; - initIoc(); + await initIoc(); // tslint:disable-next-line: no-invalid-this await setupFunction.call(this); }); @@ -2050,7 +2051,7 @@ df.head()`; // tslint:disable-next-line: no-invalid-this return this.skip(); } - initIoc(); + await initIoc(); windowStateChangeHandlers = []; // Keep track of all handlers for the onDidChangeWindowState event. @@ -2075,9 +2076,9 @@ df.head()`; test('Auto save notebook every 1s', async () => { // Configure notebook to save automatically ever 1s. - when(ioc.mockedWorkspaceConfig.get('autoSave', 'off')).thenReturn('afterDelay'); - when(ioc.mockedWorkspaceConfig.get('autoSaveDelay', anything())).thenReturn(1_000); - ioc.forceSettingsChanged(ioc.getSettings().pythonPath); + await ioc.pythonWorkspaceConfig.update('autoSave', 'afterDelay'); + await ioc.pythonWorkspaceConfig.update('autoSaveDelay', 1_000); + ioc.forceSettingsChanged(undefined, ioc.getSettings().pythonPath); /** * Make some changes to a cell of a notebook, then verify the notebook is auto saved. @@ -2109,9 +2110,10 @@ df.head()`; test('File saved with same format', async () => { // Configure notebook to save automatically ever 1s. - when(ioc.mockedWorkspaceConfig.get('autoSave', 'off')).thenReturn('afterDelay'); - when(ioc.mockedWorkspaceConfig.get('autoSaveDelay', anything())).thenReturn(2_000); - ioc.forceSettingsChanged(ioc.getSettings().pythonPath); + await ioc.pythonWorkspaceConfig.update('autoSave', 'afterDelay'); + await ioc.pythonWorkspaceConfig.update('autoSaveDelay', 2_000); + + ioc.forceSettingsChanged(undefined, ioc.getSettings().pythonPath); const notebookFileContents = await fs.readFile(notebookFile.filePath, 'utf8'); const dirtyPromise = waitForMessage(ioc, InteractiveWindowMessages.NotebookDirty); const cleanPromise = waitForMessage(ioc, InteractiveWindowMessages.NotebookClean); @@ -2133,11 +2135,12 @@ df.head()`; const notebookFileContents = await fs.readFile(notebookFile.filePath, 'utf8'); // Configure notebook to to never save. - when(ioc.mockedWorkspaceConfig.get('autoSave', 'off')).thenReturn('off'); - when(ioc.mockedWorkspaceConfig.get('autoSaveDelay', anything())).thenReturn(1000); + await ioc.pythonWorkspaceConfig.update('autoSave', 'off'); + await ioc.pythonWorkspaceConfig.update('autoSaveDelay', 1_000); + // Update the settings and wait for the component to receive it and process it. const promise = waitForMessage(ioc, InteractiveWindowMessages.SettingsUpdated); - ioc.forceSettingsChanged(ioc.getSettings().pythonPath, { + ioc.forceSettingsChanged(undefined, ioc.getSettings().pythonPath, { ...defaultDataScienceSettings(), showCellInputCode: false }); @@ -2173,8 +2176,8 @@ df.head()`; await dirtyPromise; // Configure notebook to save when active editor changes. - when(ioc.mockedWorkspaceConfig.get('autoSave', 'off')).thenReturn('onFocusChange'); - ioc.forceSettingsChanged(ioc.getSettings().pythonPath); + await ioc.pythonWorkspaceConfig.update('autoSave', 'onFocusChange'); + ioc.forceSettingsChanged(undefined, ioc.getSettings().pythonPath); // Now that the notebook is dirty, change the active editor. const docManager = ioc.get(IDocumentManager) as MockDocumentManager; @@ -2205,8 +2208,8 @@ df.head()`; await dirtyPromise; // Configure notebook to save when window state changes. - when(ioc.mockedWorkspaceConfig.get('autoSave', 'off')).thenReturn('onWindowChange'); - ioc.forceSettingsChanged(ioc.getSettings().pythonPath); + await ioc.pythonWorkspaceConfig.update('autoSave', 'onWindowChange'); + ioc.forceSettingsChanged(undefined, ioc.getSettings().pythonPath); // Now that the notebook is dirty, change the active editor. // This should not trigger a save of notebook (as its configured to save only when window state changes). @@ -2228,8 +2231,8 @@ df.head()`; await dirtyPromise; // Configure notebook to save when active editor changes. - when(ioc.mockedWorkspaceConfig.get('autoSave', 'off')).thenReturn('onWindowChange'); - ioc.forceSettingsChanged(ioc.getSettings().pythonPath); + await ioc.pythonWorkspaceConfig.update('autoSave', 'onWindowChange'); + ioc.forceSettingsChanged(undefined, ioc.getSettings().pythonPath); // Now that the notebook is dirty, send notification about changes to window state. windowStateChangeHandlers.forEach(item => item({ focused })); @@ -2258,8 +2261,8 @@ df.head()`; await dirtyPromise; // Configure notebook to save when active editor changes. - when(ioc.mockedWorkspaceConfig.get('autoSave', 'off')).thenReturn('onFocusChange'); - ioc.forceSettingsChanged(ioc.getSettings().pythonPath); + await ioc.pythonWorkspaceConfig.update('autoSave', 'onFocusChange'); + ioc.forceSettingsChanged(undefined, ioc.getSettings().pythonPath); // Now that the notebook is dirty, change window state. // This should not trigger a save of notebook (as its configured to save only when focus is changed). @@ -2343,7 +2346,7 @@ df.head()`; suite('Update Metadata', () => { setup(async function() { - initIoc(); + await initIoc(); // tslint:disable-next-line: no-invalid-this await setupFunction.call(this, JSON.stringify(oldJson)); }); @@ -2389,7 +2392,7 @@ df.head()`; suite('Clear Outputs', () => { setup(async function() { - initIoc(); + await initIoc(); // tslint:disable-next-line: no-invalid-this await setupFunction.call(this, JSON.stringify(oldJson)); }); diff --git a/src/test/datascience/notebook.functional.test.ts b/src/test/datascience/notebook.functional.test.ts index 3228f686cb83..e85eb2b3feef 100644 --- a/src/test/datascience/notebook.functional.test.ts +++ b/src/test/datascience/notebook.functional.test.ts @@ -68,9 +68,10 @@ suite('DataScience notebook tests', () => { let modifiedConfig = false; const baseUri = Uri.file('foo.py'); - setup(() => { + setup(async () => { ioc = new DataScienceIocContainer(); ioc.registerDataScienceTypes(); + await ioc.activate(); }); teardown(async () => { @@ -923,7 +924,7 @@ suite('DataScience notebook tests', () => { } // Force a settings changed so that all of the cached data is cleared - ioc.forceSettingsChanged('/usr/bin/test3/python'); + ioc.forceSettingsChanged(undefined, '/usr/bin/test3/python'); assert.ok( await testCancelableMethod( diff --git a/src/test/datascience/plotViewer.functional.test.tsx b/src/test/datascience/plotViewer.functional.test.tsx index 3a64652f82d2..4e666e68fb64 100644 --- a/src/test/datascience/plotViewer.functional.test.tsx +++ b/src/test/datascience/plotViewer.functional.test.tsx @@ -21,9 +21,10 @@ suite('DataScience PlotViewer tests', () => { let plotViewerProvider: IPlotViewerProvider; let ioc: DataScienceIocContainer; - setup(() => { + setup(async () => { ioc = new DataScienceIocContainer(); ioc.registerDataScienceTypes(); + await ioc.activate(); }); function mountWebView(): ReactWrapper, React.Component> { diff --git a/src/test/datascience/variableexplorer.functional.test.tsx b/src/test/datascience/variableexplorer.functional.test.tsx index f8b1e4e6cd5e..1d20a8cd47b4 100644 --- a/src/test/datascience/variableexplorer.functional.test.tsx +++ b/src/test/datascience/variableexplorer.functional.test.tsx @@ -37,10 +37,11 @@ suite('DataScience Interactive Window variable explorer tests', () => { } }); - setup(() => { + setup(async () => { ioc = new DataScienceIocContainer(); ioc.registerDataScienceTypes(); createdNotebook = false; + await ioc.activate(); }); teardown(async () => { diff --git a/src/test/unittests.ts b/src/test/unittests.ts index 4d77117a1cfd..b57e17d241a1 100644 --- a/src/test/unittests.ts +++ b/src/test/unittests.ts @@ -2,8 +2,20 @@ // Licensed under the MIT License. 'use strict'; -// tslint:disable:no-any no-require-imports no-var-requires +// Not sure why but on windows, if you execute a process from the System32 directory, it will just crash Node. +// Not throw an exception, just make node exit. +// However if a system32 process is run first, everything works. +import * as child_process from 'child_process'; +import * as os from 'os'; +if (os.platform() === 'win32') { + const proc = child_process.spawn('C:\\Windows\\System32\\Reg.exe', ['/?']); + proc.on('error', () => { + // tslint:disable-next-line: no-console + console.error('error during reg.exe'); + }); +} +// tslint:disable:no-any no-require-imports no-var-requires if ((Reflect as any).metadata === undefined) { require('reflect-metadata'); } From ba0fc56079ecb1150e0f54ae07c0e2e77a37bcdf Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Thu, 5 Mar 2020 11:32:24 -0800 Subject: [PATCH 07/33] Get conda working locally --- .vscode/launch.json | 3 +- .../datascience/dataScienceIocContainer.ts | 196 ++++++++++-------- src/test/datascience/mockWorkspaceConfig.ts | 17 ++ src/test/datascience/mockWorkspaceFolder.ts | 12 ++ .../nativeEditor.functional.test.tsx | 34 ++- 5 files changed, 166 insertions(+), 96 deletions(-) create mode 100644 src/test/datascience/mockWorkspaceFolder.ts diff --git a/.vscode/launch.json b/.vscode/launch.json index 39f2c4888690..d993f2622cf5 100644 --- a/.vscode/launch.json +++ b/.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": "" }, "outFiles": [ diff --git a/src/test/datascience/dataScienceIocContainer.ts b/src/test/datascience/dataScienceIocContainer.ts index f981c9cf70dd..09f1bf3aedbf 100644 --- a/src/test/datascience/dataScienceIocContainer.ts +++ b/src/test/datascience/dataScienceIocContainer.ts @@ -23,6 +23,7 @@ import { } from 'vscode'; import * as vsls from 'vsls/vscode'; +import { deepEqual } from 'assert'; import { LanguageServerExtensionActivationService } from '../../client/activation/activationService'; import { LanguageServerDownloader } from '../../client/activation/common/downloader'; import { JediExtensionActivator } from '../../client/activation/jedi'; @@ -146,7 +147,6 @@ import { IPersistentStateFactory, IPythonExtensionBanner, IsWindows, - Product, ProductType, Resource, WORKSPACE_MEMENTO @@ -342,6 +342,7 @@ import { MockLanguageServerAnalysisOptions } from './mockLanguageServerAnalysisO import { MockLanguageServerProxy } from './mockLanguageServerProxy'; import { MockLiveShareApi } from './mockLiveShare'; import { MockWorkspaceConfiguration } from './mockWorkspaceConfig'; +import { MockWorkspaceFolder } from './mockWorkspaceFolder'; import { blurWindow, createMessageEvent } from './reactHelpers'; import { TestInteractiveWindowProvider } from './testInteractiveWindowProvider'; import { TestNativeEditorProvider } from './testNativeEditorProvider'; @@ -371,7 +372,6 @@ export class DataScienceIocContainer extends UnitTestIocContainer { public applicationShell!: TypeMoq.IMock; // tslint:disable-next-line:no-any public datascience!: TypeMoq.IMock; - public pythonWorkspaceConfig = new MockWorkspaceConfiguration(); private missedMessages: any[] = []; private commandManager: MockCommandManager = new MockCommandManager(); private setContexts: Record = {}; @@ -407,8 +407,11 @@ export class DataScienceIocContainer extends UnitTestIocContainer { private webPanelProvider: TypeMoq.IMock | undefined; private settingsMap = new Map(); - private workspaceFolders: WorkspaceFolder[] = []; + private configMap = new Map(); + private emptyConfig = new MockWorkspaceConfiguration(); + private workspaceFolders: MockWorkspaceFolder[] = []; private workspaceService: WorkspaceService | undefined; + private defaultPythonPath: string | undefined; constructor() { super(); @@ -458,8 +461,10 @@ export class DataScienceIocContainer extends UnitTestIocContainer { //tslint:disable:max-func-body-length public registerDataScienceTypes(useCustomEditor: boolean = false) { // Make sure the default python path is set. - const pythonPath = this.findPythonPath(); - this.pythonWorkspaceConfig.update('pythonPath', pythonPath).ignoreErrors(); + this.defaultPythonPath = this.findPythonPath(); + + // Create the workspace service first as it's used to set config values. + this.workspaceService = this.createWorkspaceService(); this.registerFileSystemTypes(); this.serviceManager.rebindInstance(IFileSystem, new MockFileSystem()); @@ -722,66 +727,13 @@ export class DataScienceIocContainer extends UnitTestIocContainer { }); this.serviceManager.addSingletonInstance(ICommandManager, this.commandManager); - // Mock the app shell and workspace service - this.workspaceService = this.createWorkspaceService(); + // Mock the app shell const appShell = (this.applicationShell = TypeMoq.Mock.ofType()); const configurationService = TypeMoq.Mock.ofType(); - const interpreterDisplay = TypeMoq.Mock.ofType(); this.datascience = TypeMoq.Mock.ofType(); configurationService.setup(c => c.getSettings(TypeMoq.It.isAny())).returns(this.getSettings.bind(this)); - // Setup default settings - const pythonSettings = this.getSettings(undefined); - pythonSettings.datascience = { - allowImportFromNotebook: true, - jupyterLaunchTimeout: 60000, - jupyterLaunchRetries: 3, - enabled: true, - jupyterServerURI: 'local', - // tslint:disable-next-line: no-invalid-template-strings - notebookFileRoot: '${fileDirname}', - changeDirOnImportExport: false, - useDefaultConfigForJupyter: true, - jupyterInterruptTimeout: 10000, - searchForJupyter: true, - showCellInputCode: true, - collapseCellInputCodeByDefault: true, - allowInput: true, - maxOutputSize: 400, - errorBackgroundColor: '#FFFFFF', - sendSelectionToInteractiveWindow: false, - codeRegularExpression: '^(#\\s*%%|#\\s*\\|#\\s*In\\[\\d*?\\]|#\\s*In\\[ \\])', - markdownRegularExpression: '^(#\\s*%%\\s*\\[markdown\\]|#\\s*\\)', - variableExplorerExclude: 'module;function;builtin_function_or_method', - liveShareConnectionTimeout: 100, - enablePlotViewer: true, - stopOnFirstLineWhileDebugging: true, - stopOnError: true, - addGotoCodeLenses: true, - enableCellCodeLens: true, - runStartupCommands: '', - debugJustMyCode: true, - variableQueries: [], - jupyterCommandLineArguments: [] - }; - pythonSettings.jediEnabled = false; - pythonSettings.downloadLanguageServer = false; - const folders = ['Envs', '.virtualenvs']; - pythonSettings.venvFolders = folders; - pythonSettings.venvPath = path.join('~', 'foo'); - pythonSettings.terminal = { - executeInFileDir: false, - launchArgs: [], - activateEnvironment: true, - activateEnvInCurrentTerminal: false - }; - - // Once we have our initial settings, make them the workspace config values - const keys = [...Object.keys(pythonSettings)]; - keys.forEach(k => this.pythonWorkspaceConfig.update(k, pythonSettings[k])); - - interpreterDisplay.setup(i => i.refresh(TypeMoq.It.isAny())).returns(() => Promise.resolve()); const startTime = Date.now(); this.datascience.setup(d => d.activationStartTime).returns(() => startTime); @@ -817,8 +769,6 @@ export class DataScienceIocContainer extends UnitTestIocContainer { // Inform the cacheable locator service to use a static map so that it stays in memory in between tests CacheableLocatorPromiseCache.forceUseStatic(); - this.serviceManager.addSingletonInstance(IInterpreterDisplay, interpreterDisplay.object); - const currentProcess = new CurrentProcess(); this.serviceManager.addSingletonInstance(ICurrentProcess, currentProcess); this.serviceManager.addSingleton(IRegistry, RegistryImplementation); @@ -872,6 +822,9 @@ export class DataScienceIocContainer extends UnitTestIocContainer { ); } + const interpreterDisplay = TypeMoq.Mock.ofType(); + interpreterDisplay.setup(i => i.refresh(TypeMoq.It.isAny())).returns(() => Promise.resolve()); + // Create our jupyter mock if necessary if (this.shouldMockJupyter) { this.jupyterMock = new MockJupyterManagerFactory(this.serviceManager); @@ -1012,6 +965,10 @@ export class DataScienceIocContainer extends UnitTestIocContainer { IPythonInPathCommandProvider, PythonInPathCommandProvider ); + this.serviceManager.addSingletonInstance( + IInterpreterDisplay, + interpreterDisplay.object + ); } else { this.serviceManager.addSingleton(IInstaller, ProductInstaller); this.serviceManager.addSingleton(KernelService, KernelService); @@ -1021,6 +978,9 @@ export class DataScienceIocContainer extends UnitTestIocContainer { // Make sure full interpreter services are available. registerInterpreterTypes(this.serviceManager); + // Rebind the interpreter display as we don't want to use the real one + this.serviceManager.rebindInstance(IInterpreterDisplay, interpreterDisplay.object); + this.serviceManager.addSingleton( IJupyterSessionManagerFactory, JupyterSessionManagerFactory @@ -1179,6 +1139,8 @@ export class DataScienceIocContainer extends UnitTestIocContainer { const key = this.getResourceKey(resource); let setting = this.settingsMap.get(key); if (!setting) { + // Make sure we have the default config for this resource first. + this.getWorkspaceConfig('python', resource); setting = new (class extends PythonSettings { public fireChangeEvent() { this.changed.fire(); @@ -1190,10 +1152,16 @@ export class DataScienceIocContainer extends UnitTestIocContainer { } public forceSettingsChanged(resource: Resource, newPath: string, datascienceSettings?: IDataScienceSettings) { - const defaultSettings = this.getSettings(resource); - defaultSettings.pythonPath = newPath; - defaultSettings.datascience = datascienceSettings ? datascienceSettings : defaultSettings.datascience; - defaultSettings.fireChangeEvent(); + const settings = this.getSettings(resource); + settings.pythonPath = newPath; + settings.datascience = datascienceSettings ? datascienceSettings : settings.datascience; + + // The workspace config must be updated too as a config change event will cause the data to be reread from + // the config. + const config = this.getWorkspaceConfig('python', resource); + config.update('pythonPath', newPath).ignoreErrors(); + config.update('dataScience', settings.datascience).ignoreErrors(); + settings.fireChangeEvent(); this.configChangeEvent.fire({ affectsConfiguration(_s: string, _r?: Uri): boolean { return true; @@ -1209,13 +1177,9 @@ export class DataScienceIocContainer extends UnitTestIocContainer { } public addWorkspaceFolder(folderPath: string) { - const index = this.workspaceFolders.length; - const folder = TypeMoq.Mock.ofType(); - folder.setup(f => f.uri).returns(() => Uri.file(folderPath)); - folder.setup(f => f.index).returns(() => index); - folder.setup(f => f.name).returns(() => folderPath); - this.workspaceFolders.push(folder.object); - return folder.object; + const workspaceFolder = new MockWorkspaceFolder(folderPath, this.workspaceFolders.length); + this.workspaceFolders.push(workspaceFolder); + return workspaceFolder; } public addResourceToFolder(resource: Uri, folderPath: string) { @@ -1223,9 +1187,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer { if (!folder) { folder = this.addWorkspaceFolder(folderPath); } - if (this.workspaceService) { - when(this.workspaceService.getWorkspaceFolder(resource)).thenReturn(folder); - } + folder.ownedResources.add(resource.toString()); } public get(serviceIdentifier: interfaces.ServiceIdentifier, name?: string | number | symbol): T { @@ -1272,6 +1234,71 @@ export class DataScienceIocContainer extends UnitTestIocContainer { } } + public getWorkspaceConfig(section: string | undefined, resource?: Resource): MockWorkspaceConfiguration { + if (!section || section !== 'python') { + return this.emptyConfig; + } + const key = this.getResourceKey(resource); + let result = this.configMap.get(key); + if (!result) { + result = this.generatePythonWorkspaceConfig(); + this.configMap.set(key, result); + } + return result; + } + + private generatePythonWorkspaceConfig(): MockWorkspaceConfiguration { + // Create a dummy settings just to setup the workspace config + const pythonSettings = new PythonSettings(undefined, new MockAutoSelectionService()); + pythonSettings.pythonPath = this.defaultPythonPath!; + pythonSettings.datascience = { + allowImportFromNotebook: true, + jupyterLaunchTimeout: 60000, + jupyterLaunchRetries: 3, + enabled: true, + jupyterServerURI: 'local', + // tslint:disable-next-line: no-invalid-template-strings + notebookFileRoot: '${fileDirname}', + changeDirOnImportExport: false, + useDefaultConfigForJupyter: true, + jupyterInterruptTimeout: 10000, + searchForJupyter: true, + showCellInputCode: true, + collapseCellInputCodeByDefault: true, + allowInput: true, + maxOutputSize: 400, + errorBackgroundColor: '#FFFFFF', + sendSelectionToInteractiveWindow: false, + codeRegularExpression: '^(#\\s*%%|#\\s*\\|#\\s*In\\[\\d*?\\]|#\\s*In\\[ \\])', + markdownRegularExpression: '^(#\\s*%%\\s*\\[markdown\\]|#\\s*\\)', + variableExplorerExclude: 'module;function;builtin_function_or_method', + liveShareConnectionTimeout: 100, + enablePlotViewer: true, + stopOnFirstLineWhileDebugging: true, + stopOnError: true, + addGotoCodeLenses: true, + enableCellCodeLens: true, + runStartupCommands: '', + debugJustMyCode: true, + variableQueries: [], + jupyterCommandLineArguments: [] + }; + pythonSettings.jediEnabled = false; + pythonSettings.downloadLanguageServer = false; + const folders = ['Envs', '.virtualenvs']; + pythonSettings.venvFolders = folders; + pythonSettings.venvPath = path.join('~', 'foo'); + pythonSettings.terminal = { + executeInFileDir: false, + launchArgs: [], + activateEnvironment: true, + activateEnvInCurrentTerminal: false + }; + + // Use these settings to default all of the settings in a python configuration + return new MockWorkspaceConfiguration(pythonSettings); + } + private createWorkspaceService() { class MockFileSystemWatcher implements FileSystemWatcher { public ignoreCreateEvents: boolean = false; @@ -1300,15 +1327,8 @@ export class DataScienceIocContainer extends UnitTestIocContainer { when(workspaceService.onDidChangeWorkspaceFolders).thenReturn(this.worksaceFoldersChangedEvent.event); // Create another config for other parts of the workspace config. - const otherConfig = mock(MockWorkspaceConfiguration); - when(otherConfig.get(anything(), anything())).thenCall((_, defaultValue) => defaultValue); - when(otherConfig.has(anything())).thenReturn(false); - when(workspaceService.getConfiguration(anything())).thenCall(key => - key === 'python' ? this.pythonWorkspaceConfig : instance(otherConfig) - ); - when(workspaceService.getConfiguration(anything(), anything())).thenCall(key => - key === 'python' ? this.pythonWorkspaceConfig : instance(otherConfig) - ); + when(workspaceService.getConfiguration(anything())).thenCall(this.getWorkspaceConfig.bind(this)); + when(workspaceService.getConfiguration(anything(), anything())).thenCall(this.getWorkspaceConfig.bind(this)); const testWorkspaceFolder = path.join(EXTENSION_ROOT_DIR, 'src', 'test', 'datascience'); when(workspaceService.createFileSystemWatcher(anything(), anything(), anything(), anything())).thenReturn( @@ -1318,10 +1338,18 @@ export class DataScienceIocContainer extends UnitTestIocContainer { when(workspaceService.hasWorkspaceFolders).thenReturn(true); when(workspaceService.workspaceFolders).thenReturn(this.workspaceFolders); when(workspaceService.rootPath).thenReturn(testWorkspaceFolder); + when(workspaceService.getWorkspaceFolder(anything())).thenCall(this.getWorkspaceFolder.bind(this)); this.addWorkspaceFolder(testWorkspaceFolder); return workspaceService; } + private getWorkspaceFolder(uri: Resource): WorkspaceFolder | undefined { + if (uri) { + return this.workspaceFolders.find(w => w.ownedResources.has(uri.toString())); + } + return undefined; + } + private getResourceKey(resource: Resource): string { const workspace = this.serviceManager.get(IWorkspaceService); const workspaceFolderUri = PythonSettings.getSettingsUriAndTarget(resource, workspace).uri; diff --git a/src/test/datascience/mockWorkspaceConfig.ts b/src/test/datascience/mockWorkspaceConfig.ts index 4db0ebd24007..aa8c5706c09a 100644 --- a/src/test/datascience/mockWorkspaceConfig.ts +++ b/src/test/datascience/mockWorkspaceConfig.ts @@ -9,6 +9,23 @@ export class MockWorkspaceConfiguration implements WorkspaceConfiguration { // tslint:disable: no-any private values = new Map(); + constructor(defaultSettings?: any) { + if (defaultSettings) { + const keys = [...Object.keys(defaultSettings)]; + keys.forEach(k => this.values.set(k, defaultSettings[k])); + } + + // Special case python path (not in the object) + if (defaultSettings && defaultSettings.pythonPath) { + this.values.set('pythonPath', defaultSettings.pythonPath); + } + + // Special case datascience. Not the same case + if (defaultSettings && defaultSettings.datascience) { + this.values.set('dataScience', defaultSettings.datascience); + } + } + public get(key: string, defaultValue?: T): T | undefined { // tslint:disable-next-line: use-named-parameter if (this.values.has(key)) { diff --git a/src/test/datascience/mockWorkspaceFolder.ts b/src/test/datascience/mockWorkspaceFolder.ts new file mode 100644 index 000000000000..3b56a9860bf5 --- /dev/null +++ b/src/test/datascience/mockWorkspaceFolder.ts @@ -0,0 +1,12 @@ +import { Uri, WorkspaceFolder } from 'vscode'; + +export class MockWorkspaceFolder implements WorkspaceFolder { + public uri: Uri; + public name: string; + public ownedResources = new Set(); + + constructor(folder: string, public index: number) { + this.uri = Uri.file(folder); + this.name = folder; + } +} diff --git a/src/test/datascience/nativeEditor.functional.test.tsx b/src/test/datascience/nativeEditor.functional.test.tsx index ebacfc619613..6dee0226a234 100644 --- a/src/test/datascience/nativeEditor.functional.test.tsx +++ b/src/test/datascience/nativeEditor.functional.test.tsx @@ -12,7 +12,12 @@ import * as path from 'path'; import * as sinon from 'sinon'; import * as TypeMoq from 'typemoq'; import { Disposable, TextDocument, TextEditor, Uri, WindowState } from 'vscode'; -import { IApplicationShell, ICustomEditorService, IDocumentManager } from '../../client/common/application/types'; +import { + IApplicationShell, + ICustomEditorService, + IDocumentManager, + IWorkspaceService +} from '../../client/common/application/types'; import { IFileSystem } from '../../client/common/platform/types'; import { createDeferred, sleep, waitForPromise } from '../../client/common/utils/async'; import { noop } from '../../client/common/utils/misc'; @@ -2074,10 +2079,17 @@ df.head()`; await addCell(wrapper, ioc, 'a', false); } + async function updateFileConfig(key: string, value: any) { + return ioc + .get(IWorkspaceService) + .getConfiguration('file') + .update(key, value); + } + test('Auto save notebook every 1s', async () => { // Configure notebook to save automatically ever 1s. - await ioc.pythonWorkspaceConfig.update('autoSave', 'afterDelay'); - await ioc.pythonWorkspaceConfig.update('autoSaveDelay', 1_000); + await updateFileConfig('autoSave', 'afterDelay'); + await updateFileConfig('autoSaveDelay', 1_000); ioc.forceSettingsChanged(undefined, ioc.getSettings().pythonPath); /** @@ -2110,8 +2122,8 @@ df.head()`; test('File saved with same format', async () => { // Configure notebook to save automatically ever 1s. - await ioc.pythonWorkspaceConfig.update('autoSave', 'afterDelay'); - await ioc.pythonWorkspaceConfig.update('autoSaveDelay', 2_000); + await updateFileConfig('autoSave', 'afterDelay'); + await updateFileConfig('autoSaveDelay', 2_000); ioc.forceSettingsChanged(undefined, ioc.getSettings().pythonPath); const notebookFileContents = await fs.readFile(notebookFile.filePath, 'utf8'); @@ -2135,8 +2147,8 @@ df.head()`; const notebookFileContents = await fs.readFile(notebookFile.filePath, 'utf8'); // Configure notebook to to never save. - await ioc.pythonWorkspaceConfig.update('autoSave', 'off'); - await ioc.pythonWorkspaceConfig.update('autoSaveDelay', 1_000); + await updateFileConfig('autoSave', 'off'); + await updateFileConfig('autoSaveDelay', 1_000); // Update the settings and wait for the component to receive it and process it. const promise = waitForMessage(ioc, InteractiveWindowMessages.SettingsUpdated); @@ -2176,7 +2188,7 @@ df.head()`; await dirtyPromise; // Configure notebook to save when active editor changes. - await ioc.pythonWorkspaceConfig.update('autoSave', 'onFocusChange'); + await updateFileConfig('autoSave', 'onFocusChange'); ioc.forceSettingsChanged(undefined, ioc.getSettings().pythonPath); // Now that the notebook is dirty, change the active editor. @@ -2208,7 +2220,7 @@ df.head()`; await dirtyPromise; // Configure notebook to save when window state changes. - await ioc.pythonWorkspaceConfig.update('autoSave', 'onWindowChange'); + await updateFileConfig('autoSave', 'onWindowChange'); ioc.forceSettingsChanged(undefined, ioc.getSettings().pythonPath); // Now that the notebook is dirty, change the active editor. @@ -2231,7 +2243,7 @@ df.head()`; await dirtyPromise; // Configure notebook to save when active editor changes. - await ioc.pythonWorkspaceConfig.update('autoSave', 'onWindowChange'); + await updateFileConfig('autoSave', 'onWindowChange'); ioc.forceSettingsChanged(undefined, ioc.getSettings().pythonPath); // Now that the notebook is dirty, send notification about changes to window state. @@ -2261,7 +2273,7 @@ df.head()`; await dirtyPromise; // Configure notebook to save when active editor changes. - await ioc.pythonWorkspaceConfig.update('autoSave', 'onFocusChange'); + await updateFileConfig('autoSave', 'onFocusChange'); ioc.forceSettingsChanged(undefined, ioc.getSettings().pythonPath); // Now that the notebook is dirty, change window state. From d4f3feee8c9d205e41ca9329f9c7409033e21c43 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Thu, 5 Mar 2020 11:49:08 -0800 Subject: [PATCH 08/33] Make windows supported environments --- build/ci/templates/test_phases.yml | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/build/ci/templates/test_phases.yml b/build/ci/templates/test_phases.yml index 3ab3470a9cd6..435fe47e2f26 100644 --- a/build/ci/templates/test_phases.yml +++ b/build/ci/templates/test_phases.yml @@ -104,7 +104,15 @@ steps: - bash: | echo "##vso[task.prependpath]$CONDA/bin" displayName: 'Add conda to the path' - condition: and(succeeded(), eq(variables['NeedsPythonFunctionalReqs'], 'true')) + 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 # @@ -124,14 +132,23 @@ steps: displayName: 'Create CONDA Environments' condition: and(succeeded(), eq(variables['NeedsPythonFunctionalReqs'], 'true')) - # Run the pip installs in the two environments + # Run the pip installs in the two environments (darwin linux) - bash: | 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 displayName: 'Install Pip requirements for CONDA envs' - condition: and(succeeded(), eq(variables['NeedsPythonFunctionalReqs'], 'true')) + condition: and(succeeded(), eq(variables['NeedsPythonFunctionalReqs'], 'true'), ne(variables['Agent.Os'], 'Windows_NT')) + + # Run the pip installs in the two environments (windows) + - script: | + 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 # From fb162614b85079feaff5f788f3bb5f051cfd6991 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Thu, 5 Mar 2020 11:56:54 -0800 Subject: [PATCH 09/33] Fix hygiene --- src/test/datascience/dataScienceIocContainer.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/test/datascience/dataScienceIocContainer.ts b/src/test/datascience/dataScienceIocContainer.ts index 09f1bf3aedbf..e20fc4d25561 100644 --- a/src/test/datascience/dataScienceIocContainer.ts +++ b/src/test/datascience/dataScienceIocContainer.ts @@ -23,7 +23,6 @@ import { } from 'vscode'; import * as vsls from 'vsls/vscode'; -import { deepEqual } from 'assert'; import { LanguageServerExtensionActivationService } from '../../client/activation/activationService'; import { LanguageServerDownloader } from '../../client/activation/common/downloader'; import { JediExtensionActivator } from '../../client/activation/jedi'; @@ -410,7 +409,6 @@ export class DataScienceIocContainer extends UnitTestIocContainer { private configMap = new Map(); private emptyConfig = new MockWorkspaceConfiguration(); private workspaceFolders: MockWorkspaceFolder[] = []; - private workspaceService: WorkspaceService | undefined; private defaultPythonPath: string | undefined; constructor() { @@ -464,7 +462,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer { this.defaultPythonPath = this.findPythonPath(); // Create the workspace service first as it's used to set config values. - this.workspaceService = this.createWorkspaceService(); + this.createWorkspaceService(); this.registerFileSystemTypes(); this.serviceManager.rebindInstance(IFileSystem, new MockFileSystem()); From bfb4cec5a30257d1735fdc4b3e5fd24ab19c6258 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Thu, 5 Mar 2020 16:33:17 -0800 Subject: [PATCH 10/33] Fix memory leak for daemons --- src/client/common/process/pythonDaemon.ts | 1 + src/client/common/process/pythonDaemonPool.ts | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/client/common/process/pythonDaemon.ts b/src/client/common/process/pythonDaemon.ts index 168a5ae9739e..11117f104fbe 100644 --- a/src/client/common/process/pythonDaemon.ts +++ b/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(); } diff --git a/src/client/common/process/pythonDaemonPool.ts b/src/client/common/process/pythonDaemonPool.ts index 1248a0d8756a..e8488900b930 100644 --- a/src/client/common/process/pythonDaemonPool.ts +++ b/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 { const msg = { args: ['GetPythonVersion'] }; @@ -231,7 +233,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(); } From 43cf4a16af7d900537a9ef3969e32bf0fa450b35 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Thu, 5 Mar 2020 16:37:11 -0800 Subject: [PATCH 11/33] Fix hang on conda --- src/client/interpreter/activation/service.ts | 28 ++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/src/client/interpreter/activation/service.ts b/src/client/interpreter/activation/service.ts index 1cc5f3541f95..dd151b4012a2 100644 --- a/src/client/interpreter/activation/service.ts +++ b/src/client/interpreter/activation/service.ts @@ -10,9 +10,10 @@ import { IWorkspaceService } from '../../common/application/types'; import { PYTHON_WARNINGS } from '../../common/constants'; import { LogOptions, traceDecorators, traceError, traceVerbose } from '../../common/logger'; import { IPlatformService } from '../../common/platform/types'; -import { IProcessServiceFactory } from '../../common/process/types'; +import { ExecutionResult, IProcessServiceFactory } from '../../common/process/types'; import { ITerminalHelper, TerminalShellType } from '../../common/terminal/types'; import { ICurrentProcess, IDisposable, Resource } from '../../common/types'; +import { sleep } from '../../common/utils/async'; import { InMemoryCache } from '../../common/utils/cacheUtils'; import { OSType } from '../../common/utils/platform'; import { IEnvironmentVariablesProvider } from '../../common/variables/types'; @@ -136,7 +137,30 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi // See the discussion from hidesoon in this issue: https://github.com/Microsoft/vscode-python/issues/4424 // His issue is conda never finishing during activate. This is a conda issue, but we // should at least tell the user. - const result = await processService.shellExec(command, { + let result: ExecutionResult | undefined; + while (!result) { + result = await processService.shellExec(command, { + env, + shell: shellInfo.shell, + timeout: getEnvironmentTimeout, + maxBuffer: 1000 * 1000 + }); + if (result.stderr && result.stderr.length > 0) { + // Special case. Conda for some versions will state a file is in use. If + // that's the case, wait and try again. This happens especially on AzDo + if ( + result.stderr.includes( + 'The process cannot access the file because it is being used by another process' + ) + ) { + result = undefined; + await sleep(500); + } else { + throw new Error(`StdErr from ShellExec, ${result.stderr}`); + } + } + } + result = await processService.shellExec(command, { env, shell: shellInfo.shell, timeout: getEnvironmentTimeout, From 8e11f39c99b7e70b69f38a6480b9772def72c333 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Thu, 5 Mar 2020 16:37:52 -0800 Subject: [PATCH 12/33] Actually retry --- src/client/interpreter/activation/service.ts | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/client/interpreter/activation/service.ts b/src/client/interpreter/activation/service.ts index dd151b4012a2..bf11495d2c15 100644 --- a/src/client/interpreter/activation/service.ts +++ b/src/client/interpreter/activation/service.ts @@ -160,15 +160,6 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi } } } - result = await processService.shellExec(command, { - env, - shell: shellInfo.shell, - timeout: getEnvironmentTimeout, - maxBuffer: 1000 * 1000 - }); - if (result.stderr && result.stderr.length > 0) { - throw new Error(`StdErr from ShellExec, ${result.stderr}`); - } const returnedEnv = this.parseEnvironmentOutput(result.stdout); // Put back the PYTHONWARNINGS value From 2573ea08d6c598bf644e20d3612d742f277c0a58 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Thu, 5 Mar 2020 17:03:34 -0800 Subject: [PATCH 13/33] Two missing setting change updates --- src/test/datascience/notebook.functional.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/datascience/notebook.functional.test.ts b/src/test/datascience/notebook.functional.test.ts index e85eb2b3feef..8e36fbf34358 100644 --- a/src/test/datascience/notebook.functional.test.ts +++ b/src/test/datascience/notebook.functional.test.ts @@ -698,7 +698,7 @@ suite('DataScience notebook tests', () => { // Make sure we have a change dir happening const settings = { ...ioc.getSettings().datascience }; settings.changeDirOnImportExport = true; - ioc.forceSettingsChanged(ioc.getSettings().pythonPath, settings); + ioc.forceSettingsChanged(undefined, ioc.getSettings().pythonPath, settings); const exporter = ioc.serviceManager.get(INotebookExporter); const newFolderPath = path.join( @@ -1225,7 +1225,7 @@ plt.show()`, const tempDir = os.tmpdir(); const settings = ioc.getSettings(); settings.datascience.jupyterCommandLineArguments = ['--NotebookApp.port=9975', `--notebook-dir=${tempDir}`]; - ioc.forceSettingsChanged(settings.pythonPath, settings.datascience); + ioc.forceSettingsChanged(undefined, settings.pythonPath, settings.datascience); const notebook = await createNotebook(true); assert.ok(notebook, 'Server should have started on port 9975'); const hs = notebook as HostJupyterNotebook; From c418e119d771571b5a9937048a8afb41f9bd3b45 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Fri, 6 Mar 2020 09:18:07 -0800 Subject: [PATCH 14/33] Rework the failure capture --- src/client/interpreter/activation/service.ts | 30 ++++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/client/interpreter/activation/service.ts b/src/client/interpreter/activation/service.ts index bf11495d2c15..5926dafb0f24 100644 --- a/src/client/interpreter/activation/service.ts +++ b/src/client/interpreter/activation/service.ts @@ -8,7 +8,7 @@ import * as path from 'path'; import { IWorkspaceService } from '../../common/application/types'; import { PYTHON_WARNINGS } from '../../common/constants'; -import { LogOptions, traceDecorators, traceError, traceVerbose } from '../../common/logger'; +import { LogOptions, traceDecorators, traceError, traceInfo, traceVerbose } from '../../common/logger'; import { IPlatformService } from '../../common/platform/types'; import { ExecutionResult, IProcessServiceFactory } from '../../common/process/types'; import { ITerminalHelper, TerminalShellType } from '../../common/terminal/types'; @@ -139,24 +139,30 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi // should at least tell the user. let result: ExecutionResult | undefined; while (!result) { - result = await processService.shellExec(command, { - env, - shell: shellInfo.shell, - timeout: getEnvironmentTimeout, - maxBuffer: 1000 * 1000 - }); - if (result.stderr && result.stderr.length > 0) { + try { + result = await processService.shellExec(command, { + env, + shell: shellInfo.shell, + timeout: getEnvironmentTimeout, + maxBuffer: 1000 * 1000, + throwOnStdErr: false + }); + if (result.stderr && result.stderr.length > 0) { + throw new Error(`StdErr from ShellExec, ${result.stderr}`); + } + } catch (exc) { // Special case. Conda for some versions will state a file is in use. If // that's the case, wait and try again. This happens especially on AzDo if ( - result.stderr.includes( - 'The process cannot access the file because it is being used by another process' - ) + exc + .toString() + .includes('The process cannot access the file because it is being used by another process') ) { + traceInfo(`Conda is busy, attempting to retry ...`); result = undefined; await sleep(500); } else { - throw new Error(`StdErr from ShellExec, ${result.stderr}`); + throw exc; } } } From 318b82c8427d2dd17cddad8c37e4564c1ddb26d0 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Fri, 6 Mar 2020 11:40:56 -0800 Subject: [PATCH 15/33] Get rid of memory leak when session dies --- src/client/datascience/jupyter/jupyterSession.ts | 4 ++++ src/client/datascience/jupyter/notebookStarter.ts | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/client/datascience/jupyter/jupyterSession.ts b/src/client/datascience/jupyter/jupyterSession.ts index 676a836a8cd2..d20765705fde 100644 --- a/src/client/datascience/jupyter/jupyterSession.ts +++ b/src/client/datascience/jupyter/jupyterSession.ts @@ -330,6 +330,8 @@ export class JupyterSession implements IJupyterSession { resolve(); } else if (e === 'dead') { traceError('Kernel died while waiting for idle'); + // If we throw an exception, make sure to shutdown the session as it's not usable anymore + this.shutdownSession(session, this.statusHandler).ignoreErrors(); reject( new JupyterWaitForIdleError(localize.DataScience.kernelIsDead().format(session.kernel.name)) ); @@ -365,6 +367,8 @@ export class JupyterSession implements IJupyterSession { return; } + // If we throw an exception, make sure to shutdown the session as it's not usable anymore + this.shutdownSession(session, this.statusHandler).ignoreErrors(); throw new JupyterWaitForIdleError(localize.DataScience.jupyterLaunchTimedOut()); } } diff --git a/src/client/datascience/jupyter/notebookStarter.ts b/src/client/datascience/jupyter/notebookStarter.ts index f5544d4378f8..77f7c4e757ea 100644 --- a/src/client/datascience/jupyter/notebookStarter.ts +++ b/src/client/datascience/jupyter/notebookStarter.ts @@ -147,7 +147,7 @@ export class NotebookStarter implements Disposable { // Something else went wrong. See if the local proc died or not. if (exitCode !== 0) { - throw new Error(localize.DataScience.jupyterServerCrashed().format(exitCode.toString())); + throw new Error(localize.DataScience.jupyterServerCrashed().format(exitCode?.toString())); } else { throw new Error(localize.DataScience.jupyterNotebookFailure().format(err)); } From cd9112ace13da6213fda22703fbc8ed3b4a27830 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Fri, 6 Mar 2020 13:53:57 -0800 Subject: [PATCH 16/33] Make tests leak less memory --- build/conda-functional-requirements.txt | 16 +++++++++++++++- .../datascience/jupyter/jupyterSessionManager.ts | 10 ++++++++++ src/client/datascience/webViewHost.ts | 6 ++++-- 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/build/conda-functional-requirements.txt b/build/conda-functional-requirements.txt index a065e22f21ac..3b267106ba10 100644 --- a/build/conda-functional-requirements.txt +++ b/build/conda-functional-requirements.txt @@ -1,3 +1,17 @@ # List of requirements for conda environments that cannot be installed using conda livelossplot -versioneer \ No newline at end of file +versioneer +flake8 +autopep8 +bandit +black ; python_version>='3.6' +yapf +pylint +pycodestyle +prospector +pydocstyle +nose +rope +flask +django +isort diff --git a/src/client/datascience/jupyter/jupyterSessionManager.ts b/src/client/datascience/jupyter/jupyterSessionManager.ts index a2faf3eb0779..387e98fb2323 100644 --- a/src/client/datascience/jupyter/jupyterSessionManager.ts +++ b/src/client/datascience/jupyter/jupyterSessionManager.ts @@ -45,8 +45,18 @@ export class JupyterSessionManager implements IJupyterSessionManager { } if (this.sessionManager && !this.sessionManager.isDisposed) { traceInfo('ShutdownSessionAndConnection - dispose session manager'); + const sessionManager = this.sessionManager as any; this.sessionManager.dispose(); 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 + if (sessionManager._specs) { + sessionManager._specs = {}; + } + if (sessionManager._sessions && sessionManager._sessions.clear) { + sessionManager._sessions.clear(); + } } } diff --git a/src/client/datascience/webViewHost.ts b/src/client/datascience/webViewHost.ts index 0d265ddec2bf..a3e265197201 100644 --- a/src/client/datascience/webViewHost.ts +++ b/src/client/datascience/webViewHost.ts @@ -7,6 +7,7 @@ import { injectable, unmanaged } from 'inversify'; import { ConfigurationChangeEvent, ViewColumn, WebviewPanel, WorkspaceConfiguration } from 'vscode'; import { IWebPanel, IWebPanelMessageListener, IWebPanelProvider, IWorkspaceService } from '../common/application/types'; +import { isTestExecution } from '../common/constants'; import { traceInfo, traceWarning } from '../common/logger'; import { IConfigurationService, IDisposable, Resource } from '../common/types'; import { createDeferred, Deferred } from '../common/utils/async'; @@ -66,8 +67,9 @@ export abstract class WebViewHost implements IDisposable { // Send the first settings message this.onDataScienceSettingsChanged().ignoreErrors(); - // Send the loc strings - this.postMessageInternal(SharedMessages.LocInit, localize.getCollectionJSON()).ignoreErrors(); + // Send the loc strings (skip during testing as it takes up a lot of memory) + const locStrings = isTestExecution() ? '{}' : localize.getCollectionJSON(); + this.postMessageInternal(SharedMessages.LocInit, locStrings).ignoreErrors(); } public async show(preserveFocus: boolean): Promise { From fd76344b78b8709289e1c3063284d69885e12878 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Fri, 6 Mar 2020 14:11:54 -0800 Subject: [PATCH 17/33] Hygiene problem --- src/client/datascience/jupyter/jupyterSessionManager.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/client/datascience/jupyter/jupyterSessionManager.ts b/src/client/datascience/jupyter/jupyterSessionManager.ts index 387e98fb2323..78c707f19f70 100644 --- a/src/client/datascience/jupyter/jupyterSessionManager.ts +++ b/src/client/datascience/jupyter/jupyterSessionManager.ts @@ -45,6 +45,7 @@ export class JupyterSessionManager implements IJupyterSessionManager { } if (this.sessionManager && !this.sessionManager.isDisposed) { traceInfo('ShutdownSessionAndConnection - dispose session manager'); + // tslint:disable-next-line: no-any const sessionManager = this.sessionManager as any; this.sessionManager.dispose(); this.sessionManager = undefined; From a83acca9151534b6f4f834a0fbcd60fe455cd314 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Fri, 6 Mar 2020 15:38:42 -0800 Subject: [PATCH 18/33] More potential cleanup --- .../datascience/jupyter/jupyterSession.ts | 8 +++++++- .../jupyter/jupyterSessionManager.ts | 19 +++++++++++++++++++ src/client/interpreter/activation/service.ts | 14 ++++++++------ .../interactiveWindow.functional.test.tsx | 6 +++++- src/test/serviceRegistry.ts | 1 + 5 files changed, 40 insertions(+), 8 deletions(-) diff --git a/src/client/datascience/jupyter/jupyterSession.ts b/src/client/datascience/jupyter/jupyterSession.ts index d20765705fde..7d8adcfd5765 100644 --- a/src/client/datascience/jupyter/jupyterSession.ts +++ b/src/client/datascience/jupyter/jupyterSession.ts @@ -136,6 +136,12 @@ export class JupyterSession implements IJupyterSession { await this.session.kernel.restart(); return; } + + // Start the restart session now in case it wasn't started + if (!this.restartSessionPromise) { + this.startRestartSession(); + } + // Just kill the current session and switch to the other if (this.restartSessionPromise && this.session && this.sessionManager && this.contentsManager) { traceInfo(`Restarting ${this.session.kernel.id}`); @@ -195,7 +201,7 @@ export class JupyterSession implements IJupyterSession { : undefined; // It has been observed that starting the restart session slows down first time to execute a cell. // Solution is to start the restart session after the first execution of user code. - if (!content.silent && result) { + if (!content.silent && result && !isTestExecution()) { result.done.finally(() => this.startRestartSession()).ignoreErrors(); } return result; diff --git a/src/client/datascience/jupyter/jupyterSessionManager.ts b/src/client/datascience/jupyter/jupyterSessionManager.ts index 78c707f19f70..ee864a0bc4c9 100644 --- a/src/client/datascience/jupyter/jupyterSessionManager.ts +++ b/src/client/datascience/jupyter/jupyterSessionManager.ts @@ -5,6 +5,7 @@ import { ContentsManager, Kernel, ServerConnection, Session, SessionManager } fr import { Agent as HttpsAgent } from 'https'; import { CancellationToken } from 'vscode-jsonrpc'; +import { noop } from 'jquery'; import { traceInfo } from '../../common/logger'; import { IConfigurationService, IOutputChannel } from '../../common/types'; import * as localize from '../../common/utils/localize'; @@ -45,6 +46,9 @@ export class JupyterSessionManager implements IJupyterSessionManager { } if (this.sessionManager && !this.sessionManager.isDisposed) { traceInfo('ShutdownSessionAndConnection - dispose session manager'); + // Make sure it finishes startup. + await this.sessionManager.ready; + // tslint:disable-next-line: no-any const sessionManager = this.sessionManager as any; this.sessionManager.dispose(); @@ -58,6 +62,12 @@ export class JupyterSessionManager implements IJupyterSessionManager { if (sessionManager._sessions && sessionManager._sessions.clear) { sessionManager._sessions.clear(); } + if (sessionManager._pollModels) { + this.clearPoll(sessionManager._pollModels); + } + if (sessionManager._pollSpecs) { + this.clearPoll(sessionManager._pollSpecs); + } } } @@ -164,6 +174,15 @@ export class JupyterSessionManager implements IJupyterSessionManager { } } + // tslint:disable-next-line: no-any + private clearPoll(poll: { _timeout: any }) { + try { + clearTimeout(poll._timeout); + } catch { + noop(); + } + } + private getSessionCookieString(pwSettings: IJupyterPasswordConnectInfo): string { return `_xsrf=${pwSettings.xsrfCookie}; ${pwSettings.sessionCookieName}=${pwSettings.sessionCookieValue}`; } diff --git a/src/client/interpreter/activation/service.ts b/src/client/interpreter/activation/service.ts index 5926dafb0f24..97aa6d4c3cda 100644 --- a/src/client/interpreter/activation/service.ts +++ b/src/client/interpreter/activation/service.ts @@ -35,6 +35,11 @@ const defaultShells = { [OSType.Unknown]: undefined }; +const condaRetryMessages = [ + 'The process cannot access the file because it is being used by another process', + 'The directory is not empty' +]; + @injectable() export class EnvironmentActivationService implements IEnvironmentActivationService, IDisposable { private readonly disposables: IDisposable[] = []; @@ -148,16 +153,13 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi throwOnStdErr: false }); if (result.stderr && result.stderr.length > 0) { - throw new Error(`StdErr from ShellExec, ${result.stderr}`); + throw new Error(`StdErr from ShellExec, ${result.stderr} for ${command}`); } } catch (exc) { // Special case. Conda for some versions will state a file is in use. If // that's the case, wait and try again. This happens especially on AzDo - if ( - exc - .toString() - .includes('The process cannot access the file because it is being used by another process') - ) { + const excString = exc.toString(); + if (condaRetryMessages.find(m => excString.includes(m))) { traceInfo(`Conda is busy, attempting to retry ...`); result = undefined; await sleep(500); diff --git a/src/test/datascience/interactiveWindow.functional.test.tsx b/src/test/datascience/interactiveWindow.functional.test.tsx index 5dd1511a4b5e..5ad4e6413304 100644 --- a/src/test/datascience/interactiveWindow.functional.test.tsx +++ b/src/test/datascience/interactiveWindow.functional.test.tsx @@ -23,6 +23,7 @@ import { IInterpreterService } from '../../client/interpreter/contracts'; import { concatMultilineStringInput } from '../../datascience-ui/common'; import { InteractivePanel } from '../../datascience-ui/history-react/interactivePanel'; import { ImageButton } from '../../datascience-ui/react-common/imageButton'; +import { asyncDump } from '../common/asyncDump'; import { DataScienceIocContainer } from './dataScienceIocContainer'; import { createDocument } from './editor-integration/helpers'; import { defaultDataScienceSettings } from './helpers'; @@ -58,7 +59,6 @@ import { waitForMessageResponse } from './testHelpers'; -//import { asyncDump } from '../common/asyncDump'; // tslint:disable:max-func-body-length trailing-comma no-any no-multiline-string suite('DataScience Interactive Window output tests', () => { const disposables: Disposable[] = []; @@ -85,6 +85,10 @@ suite('DataScience Interactive Window output tests', () => { await ioc.dispose(); }); + suiteTeardown(() => { + asyncDump(); + }); + async function forceSettingsChange(newSettings: IDataScienceSettings) { const window = await getOrCreateInteractiveWindow(ioc); await window.show(); diff --git a/src/test/serviceRegistry.ts b/src/test/serviceRegistry.ts index 3c1a4f8c45cb..45a5cc616fa4 100644 --- a/src/test/serviceRegistry.ts +++ b/src/test/serviceRegistry.ts @@ -244,6 +244,7 @@ export class IocContainer { await promise; } } + this.disposables = []; } public registerCommonTypes(registerFileSystem: boolean = true) { From be983b2fea6983b11329c6161235812269ee6d5a Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Mon, 9 Mar 2020 12:13:09 -0700 Subject: [PATCH 19/33] More changes to clean up memory as soon as possible --- src/client/common/asyncDisposableRegistry.ts | 1 + src/client/common/process/proc.ts | 3 +- src/client/interpreter/activation/service.ts | 41 +++++- src/client/ioc/serviceManager.ts | 5 + src/client/ioc/types.ts | 3 +- .../datascience/dataScienceIocContainer.ts | 135 +++++++++--------- .../interactiveWindow.functional.test.tsx | 5 - .../datascience/notebook.functional.test.ts | 5 - src/test/datascience/reactHelpers.ts | 28 ---- src/test/serviceRegistry.ts | 1 + 10 files changed, 121 insertions(+), 106 deletions(-) diff --git a/src/client/common/asyncDisposableRegistry.ts b/src/client/common/asyncDisposableRegistry.ts index 0b4f18af81b7..590727a445f1 100644 --- a/src/client/common/asyncDisposableRegistry.ts +++ b/src/client/common/asyncDisposableRegistry.ts @@ -12,6 +12,7 @@ export class AsyncDisposableRegistry implements IAsyncDisposableRegistry { public async dispose(): Promise { const promises = this._list.map(l => l.dispose()); await Promise.all(promises); + this._list = []; } public push(disposable?: IDisposable | IAsyncDisposable) { diff --git a/src/client/common/process/proc.ts b/src/client/common/process/proc.ts index bfe2d496d7c4..89b5d4a8b05b 100644 --- a/src/client/common/process/proc.ts +++ b/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); } diff --git a/src/client/interpreter/activation/service.ts b/src/client/interpreter/activation/service.ts index 97aa6d4c3cda..51716435d16a 100644 --- a/src/client/interpreter/activation/service.ts +++ b/src/client/interpreter/activation/service.ts @@ -40,10 +40,49 @@ const condaRetryMessages = [ 'The directory is not empty' ]; +export class EnvironmentActivationServiceCache { + private static useStatic = false; + private static staticMap = new Map>(); + private normalMap = new Map>(); + + public static forceUseStatic() { + EnvironmentActivationServiceCache.useStatic = true; + } + public get(key: string): InMemoryCache | undefined { + if (EnvironmentActivationServiceCache.useStatic) { + return EnvironmentActivationServiceCache.staticMap.get(key); + } + return this.normalMap.get(key); + } + + public set(key: string, value: InMemoryCache) { + if (EnvironmentActivationServiceCache.useStatic) { + EnvironmentActivationServiceCache.staticMap.set(key, value); + } else { + this.normalMap.set(key, value); + } + } + + public delete(key: string) { + if (EnvironmentActivationServiceCache.useStatic) { + EnvironmentActivationServiceCache.staticMap.delete(key); + } else { + this.normalMap.delete(key); + } + } + + public clear() { + // Don't clear during a test as the environment isn't going to change + if (!EnvironmentActivationServiceCache.useStatic) { + this.normalMap.clear(); + } + } +} + @injectable() export class EnvironmentActivationService implements IEnvironmentActivationService, IDisposable { private readonly disposables: IDisposable[] = []; - private readonly activatedEnvVariablesCache = new Map>(); + private readonly activatedEnvVariablesCache = new EnvironmentActivationServiceCache(); constructor( @inject(ITerminalHelper) private readonly helper: ITerminalHelper, @inject(IPlatformService) private readonly platform: IPlatformService, diff --git a/src/client/ioc/serviceManager.ts b/src/client/ioc/serviceManager.ts index 115f8dd7a4ed..da0105f6f2cf 100644 --- a/src/client/ioc/serviceManager.ts +++ b/src/client/ioc/serviceManager.ts @@ -106,4 +106,9 @@ export class ServiceManager implements IServiceManager { this.container.rebind(serviceIdentifier).toConstantValue(instance); } } + + public dispose() { + this.container.unbindAll(); + this.container.unload(); + } } diff --git a/src/client/ioc/types.ts b/src/client/ioc/types.ts index 02787d4a1ea0..378989429bf3 100644 --- a/src/client/ioc/types.ts +++ b/src/client/ioc/types.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. import { interfaces } from 'inversify'; +import { IDisposable } from '../common/types'; //tslint:disable:callable-types // tslint:disable-next-line:interface-name @@ -25,7 +26,7 @@ export type ClassType = { export const IServiceManager = Symbol('IServiceManager'); -export interface IServiceManager { +export interface IServiceManager extends IDisposable { add( serviceIdentifier: interfaces.ServiceIdentifier, constructor: ClassType, diff --git a/src/test/datascience/dataScienceIocContainer.ts b/src/test/datascience/dataScienceIocContainer.ts index e20fc4d25561..7d3054ca8e62 100644 --- a/src/test/datascience/dataScienceIocContainer.ts +++ b/src/test/datascience/dataScienceIocContainer.ts @@ -7,7 +7,7 @@ import { interfaces } from 'inversify'; import * as os from 'os'; import * as path from 'path'; import { SemVer } from 'semver'; -import { anything, instance, mock, when } from 'ts-mockito'; +import { anything, instance, mock, reset, when } from 'ts-mockito'; import * as TypeMoq from 'typemoq'; import { CancellationTokenSource, @@ -77,9 +77,10 @@ import { IWebPanelMessageListener, IWebPanelOptions, IWebPanelProvider, - IWorkspaceService, - WebPanelMessage + IWorkspaceService } from '../../client/common/application/types'; +import { WebPanel } from '../../client/common/application/webPanels/webPanel'; +import { WebPanelProvider } from '../../client/common/application/webPanels/webPanelProvider'; import { WorkspaceService } from '../../client/common/application/workspace'; import { AsyncDisposableRegistry } from '../../client/common/asyncDisposableRegistry'; import { PythonSettings } from '../../client/common/configSettings'; @@ -247,7 +248,10 @@ import { } from '../../client/datascience/types'; import { ProtocolParser } from '../../client/debugger/debugAdapter/Common/protocolParser'; import { IProtocolParser } from '../../client/debugger/debugAdapter/types'; -import { EnvironmentActivationService } from '../../client/interpreter/activation/service'; +import { + EnvironmentActivationService, + EnvironmentActivationServiceCache +} from '../../client/interpreter/activation/service'; import { IEnvironmentActivationService } from '../../client/interpreter/activation/types'; import { InterpreterComparer } from '../../client/interpreter/configuration/interpreterComparer'; import { InterpreterSelector } from '../../client/interpreter/configuration/interpreterSelector'; @@ -404,7 +408,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer { }; private extraListeners: ((m: string, p: any) => void)[] = []; - private webPanelProvider: TypeMoq.IMock | undefined; + private webPanelProvider = mock(WebPanelProvider); private settingsMap = new Map(); private configMap = new Map(); private emptyConfig = new MockWorkspaceConfiguration(); @@ -432,7 +436,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer { } // Bounce this so that our editor has time to shutdown - await sleep(10); + await sleep(150); // Clear out the monaco global services. Some of these services are preventing shutdown. // tslint:disable: no-require-imports @@ -454,6 +458,15 @@ export class DataScienceIocContainer extends UnitTestIocContainer { if (config.getCSSBasedConfiguration) { config.getCSSBasedConfiguration().dispose(); } + + // Because there are outstanding promises holding onto this object, clear out everything we can + this.workspaceFolders = []; + this.settingsMap.clear(); + this.configMap.clear(); + this.setContexts = {}; + this.extraListeners = []; + this.webPanelListener = undefined; + reset(this.webPanelProvider); } //tslint:disable:max-func-body-length @@ -464,6 +477,10 @@ export class DataScienceIocContainer extends UnitTestIocContainer { // Create the workspace service first as it's used to set config values. this.createWorkspaceService(); + // Setup our webpanel provider to create our dummy web panel + when(this.webPanelProvider.create(anything())).thenCall(this.onCreateWebPanel.bind(this)); + this.serviceManager.addSingletonInstance(IWebPanelProvider, instance(this.webPanelProvider)); + this.registerFileSystemTypes(); this.serviceManager.rebindInstance(IFileSystem, new MockFileSystem()); this.serviceManager.addSingleton(IJupyterExecution, JupyterExecutionFactory); @@ -767,6 +784,9 @@ export class DataScienceIocContainer extends UnitTestIocContainer { // Inform the cacheable locator service to use a static map so that it stays in memory in between tests CacheableLocatorPromiseCache.forceUseStatic(); + // Do the same thing for the environment variable activation service. + EnvironmentActivationServiceCache.forceUseStatic(); + const currentProcess = new CurrentProcess(); this.serviceManager.addSingletonInstance(ICurrentProcess, currentProcess); this.serviceManager.addSingleton(IRegistry, RegistryImplementation); @@ -1052,26 +1072,6 @@ export class DataScienceIocContainer extends UnitTestIocContainer { } } - public createWebPanel(): IWebPanel { - const webPanel = TypeMoq.Mock.ofType(); - webPanel - .setup(p => p.postMessage(TypeMoq.It.isAny())) - .callback((m: WebPanelMessage) => { - const message = createMessageEvent(m); - if (this.postMessage) { - this.postMessage(message); - } else { - throw new Error('postMessage callback not defined'); - } - }); - webPanel.setup(p => p.show(TypeMoq.It.isAny())).returns(() => Promise.resolve()); - - // See https://github.com/florinn/typemoq/issues/67 for why this is necessary - webPanel.setup((p: any) => p.then).returns(() => undefined); - - return webPanel.object; - } - // tslint:disable:any public createWebView( mount: () => ReactWrapper, React.Component>, @@ -1083,44 +1083,6 @@ export class DataScienceIocContainer extends UnitTestIocContainer { liveShareTest.forceRole(role); } - if (!this.webPanelProvider) { - this.webPanelProvider = TypeMoq.Mock.ofType(); - this.serviceManager.addSingletonInstance( - IWebPanelProvider, - this.webPanelProvider.object - ); - } else { - this.webPanelProvider.reset(); - } - const webPanel = this.createWebPanel(); - - // Setup the webpanel provider so that it returns our dummy web panel. It will have to talk to our global JSDOM window so that the react components can link into it - this.webPanelProvider - .setup(p => p.create(TypeMoq.It.isAny())) - .returns((options: IWebPanelOptions) => { - // Keep track of the current listener. It listens to messages through the vscode api - this.webPanelListener = options.listener; - - // Send messages that were already posted but were missed. - // During normal operation, the react control will not be created before - // the webPanelListener - if (this.missedMessages.length && this.webPanelListener) { - // This needs to be async because we are being called in the ctor of the webpanel. It can't - // handle some messages during the ctor. - setTimeout(() => { - this.missedMessages.forEach(m => - this.webPanelListener ? this.webPanelListener.onMessage(m.type, m.payload) : noop() - ); - }, 0); - - // Note, you might think we should clean up the messages. However since the mount only occurs once, we might - // create multiple webpanels with the same mount. We need to resend these messages to - // other webpanels that get created with the same mount. - } - - // Return our dummy web panel - return Promise.resolve(webPanel); - }); // We need to mount the react control before we even create an interactive window object. Otherwise the mount will miss rendering some parts this.mountReactControl(mount); } @@ -1230,6 +1192,9 @@ export class DataScienceIocContainer extends UnitTestIocContainer { if (this.wrapperCreatedPromise && !this.wrapperCreatedPromise.resolved) { this.wrapperCreatedPromise.resolve(); } + + // Clear out msg payload + delete msg.payload; } public getWorkspaceConfig(section: string | undefined, resource?: Resource): MockWorkspaceConfiguration { @@ -1245,13 +1210,53 @@ export class DataScienceIocContainer extends UnitTestIocContainer { return result; } + private createWebPanel(): IWebPanel { + const webPanel = mock(WebPanel); + when(webPanel.postMessage(anything())).thenCall(m => { + const message = createMessageEvent(m); + if (this.postMessage) { + this.postMessage(message); + } + if (m.payload) { + delete m.payload; + } + }); + when((webPanel as any).then).thenReturn(undefined); + return instance(webPanel); + } + + private async onCreateWebPanel(options: IWebPanelOptions) { + // Keep track of the current listener. It listens to messages through the vscode api + this.webPanelListener = options.listener; + + // Send messages that were already posted but were missed. + // During normal operation, the react control will not be created before + // the webPanelListener + if (this.missedMessages.length && this.webPanelListener) { + // This needs to be async because we are being called in the ctor of the webpanel. It can't + // handle some messages during the ctor. + setTimeout(() => { + this.missedMessages.forEach(m => + this.webPanelListener ? this.webPanelListener.onMessage(m.type, m.payload) : noop() + ); + }, 0); + + // Note, you might think we should clean up the messages. However since the mount only occurs once, we might + // create multiple webpanels with the same mount. We need to resend these messages to + // other webpanels that get created with the same mount. + } + + // Return our dummy web panel + return this.createWebPanel(); + } + private generatePythonWorkspaceConfig(): MockWorkspaceConfiguration { // Create a dummy settings just to setup the workspace config const pythonSettings = new PythonSettings(undefined, new MockAutoSelectionService()); pythonSettings.pythonPath = this.defaultPythonPath!; pythonSettings.datascience = { allowImportFromNotebook: true, - jupyterLaunchTimeout: 60000, + jupyterLaunchTimeout: 20000, jupyterLaunchRetries: 3, enabled: true, jupyterServerURI: 'local', diff --git a/src/test/datascience/interactiveWindow.functional.test.tsx b/src/test/datascience/interactiveWindow.functional.test.tsx index 5ad4e6413304..a72176d069af 100644 --- a/src/test/datascience/interactiveWindow.functional.test.tsx +++ b/src/test/datascience/interactiveWindow.functional.test.tsx @@ -23,7 +23,6 @@ import { IInterpreterService } from '../../client/interpreter/contracts'; import { concatMultilineStringInput } from '../../datascience-ui/common'; import { InteractivePanel } from '../../datascience-ui/history-react/interactivePanel'; import { ImageButton } from '../../datascience-ui/react-common/imageButton'; -import { asyncDump } from '../common/asyncDump'; import { DataScienceIocContainer } from './dataScienceIocContainer'; import { createDocument } from './editor-integration/helpers'; import { defaultDataScienceSettings } from './helpers'; @@ -85,10 +84,6 @@ suite('DataScience Interactive Window output tests', () => { await ioc.dispose(); }); - suiteTeardown(() => { - asyncDump(); - }); - async function forceSettingsChange(newSettings: IDataScienceSettings) { const window = await getOrCreateInteractiveWindow(ioc); await window.show(); diff --git a/src/test/datascience/notebook.functional.test.ts b/src/test/datascience/notebook.functional.test.ts index 8e36fbf34358..d8b6ca8c34d2 100644 --- a/src/test/datascience/notebook.functional.test.ts +++ b/src/test/datascience/notebook.functional.test.ts @@ -52,7 +52,6 @@ import { } from '../../client/interpreter/contracts'; import { concatMultilineStringInput } from '../../datascience-ui/common'; import { generateTestState, ICellViewModel } from '../../datascience-ui/interactive-common/mainState'; -import { asyncDump } from '../common/asyncDump'; import { sleep } from '../core'; import { DataScienceIocContainer } from './dataScienceIocContainer'; import { getConnectionInfo, getIPConnectionInfo, getNotebookCapableInterpreter } from './jupyterHelpers'; @@ -107,10 +106,6 @@ suite('DataScience notebook tests', () => { } }); - suiteTeardown(() => { - asyncDump(); - }); - function escapePath(p: string) { return p.replace(/\\/g, '\\\\'); } diff --git a/src/test/datascience/reactHelpers.ts b/src/test/datascience/reactHelpers.ts index a80ad2f07fa2..e83b03062509 100644 --- a/src/test/datascience/reactHelpers.ts +++ b/src/test/datascience/reactHelpers.ts @@ -399,34 +399,6 @@ export function setUpDomEnvironment() { (global as any)['Headers'] = fetchMod.Headers; // tslint:disable-next-line:no-string-literal no-eval no-any (global as any)['WebSocket'] = eval('require')('ws'); - - // For the loc test to work, we have to have a global getter for loc strings - // tslint:disable-next-line:no-string-literal no-eval no-any - (global as any)['getLocStrings'] = () => { - return { 'DataScience.unknownMimeType': 'Unknown mime type from helper' }; - }; - - // tslint:disable-next-line:no-string-literal no-eval no-any - (global as any)['getInitialSettings'] = () => { - return { - allowImportFromNotebook: true, - jupyterLaunchTimeout: 10, - jupyterLaunchRetries: 3, - enabled: true, - jupyterServerURI: 'local', - // tslint:disable-next-line: no-invalid-template-strings - notebookFileRoot: '${fileDirname}', - changeDirOnImportExport: false, - useDefaultConfigForJupyter: true, - jupyterInterruptTimeout: 10000, - searchForJupyter: true, - showCellInputCode: true, - collapseCellInputCodeByDefault: true, - allowInput: true, - variableExplorerExclude: 'module;function;builtin_function_or_method' - }; - }; - (global as any)['DOMParser'] = dom.window.DOMParser; (global as any)['Blob'] = dom.window.Blob; diff --git a/src/test/serviceRegistry.ts b/src/test/serviceRegistry.ts index 45a5cc616fa4..280538257167 100644 --- a/src/test/serviceRegistry.ts +++ b/src/test/serviceRegistry.ts @@ -245,6 +245,7 @@ export class IocContainer { } } this.disposables = []; + this.serviceManager.dispose(); } public registerCommonTypes(registerFileSystem: boolean = true) { From 39f5e4b0dd2c531432139ac4aa46e02bc639c9ca Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Mon, 9 Mar 2020 14:21:06 -0700 Subject: [PATCH 20/33] Dont auto start jupyter --- src/client/datascience/jupyter/jupyterServer.ts | 10 +++++++--- src/test/datascience/dataScienceIocContainer.ts | 9 +++------ 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/client/datascience/jupyter/jupyterServer.ts b/src/client/datascience/jupyter/jupyterServer.ts index 49f4fad38caa..02df56fff004 100644 --- a/src/client/datascience/jupyter/jupyterServer.ts +++ b/src/client/datascience/jupyter/jupyterServer.ts @@ -68,9 +68,13 @@ export class JupyterServerBase implements INotebookServer { // Listen to the process going down if (this.launchInfo && this.launchInfo.connectionInfo) { this.connectionInfoDisconnectHandler = this.launchInfo.connectionInfo.disconnected(c => { - traceError(localize.DataScience.jupyterServerCrashed().format(c.toString())); - this.serverExitCode = c; - this.shutdown().ignoreErrors(); + try { + this.serverExitCode = c; + traceError(localize.DataScience.jupyterServerCrashed().format(c.toString())); + this.shutdown().ignoreErrors(); + } catch { + noop(); + } }); } diff --git a/src/test/datascience/dataScienceIocContainer.ts b/src/test/datascience/dataScienceIocContainer.ts index 7d3054ca8e62..13aa317b3866 100644 --- a/src/test/datascience/dataScienceIocContainer.ts +++ b/src/test/datascience/dataScienceIocContainer.ts @@ -1013,11 +1013,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer { } }; - appShell - .setup(a => a.showErrorMessage(TypeMoq.It.isAnyString())) - .returns(e => { - throw e; - }); + appShell.setup(a => a.showErrorMessage(TypeMoq.It.isAnyString())).returns(() => Promise.resolve('')); appShell .setup(a => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny())) .returns(() => Promise.resolve('')); @@ -1284,7 +1280,8 @@ export class DataScienceIocContainer extends UnitTestIocContainer { runStartupCommands: '', debugJustMyCode: true, variableQueries: [], - jupyterCommandLineArguments: [] + jupyterCommandLineArguments: [], + disableJupyterAutoStart: true }; pythonSettings.jediEnabled = false; pythonSettings.downloadLanguageServer = false; From dcd6c0f5d0590b316491250a66093c218afd0b18 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Mon, 9 Mar 2020 16:15:24 -0700 Subject: [PATCH 21/33] More leaks found and fixes for breaking tests --- src/client/common/types.ts | 2 +- .../datascience/jupyter/jupyterExecution.ts | 1 + .../datascience/dataScienceIocContainer.ts | 13 ++- .../datascience/debugger.functional.test.tsx | 4 +- src/test/datascience/jupyterHelpers.ts | 24 ----- .../datascience/notebook.functional.test.ts | 94 +++++++++---------- 6 files changed, 62 insertions(+), 76 deletions(-) diff --git a/src/client/common/types.ts b/src/client/common/types.ts index c8ec93ed0633..b7d91a2cbc35 100644 --- a/src/client/common/types.ts +++ b/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'); diff --git a/src/client/datascience/jupyter/jupyterExecution.ts b/src/client/datascience/jupyter/jupyterExecution.ts index 2c884917387c..56bce4404885 100644 --- a/src/client/datascience/jupyter/jupyterExecution.ts +++ b/src/client/datascience/jupyter/jupyterExecution.ts @@ -181,6 +181,7 @@ export class JupyterExecutionBase implements IJupyterExecution { options?.metadata, cancelToken ); + await sessionManager.dispose(); } // If no kernel and not going to pick one, exit early diff --git a/src/test/datascience/dataScienceIocContainer.ts b/src/test/datascience/dataScienceIocContainer.ts index 13aa317b3866..0d1762e2f480 100644 --- a/src/test/datascience/dataScienceIocContainer.ts +++ b/src/test/datascience/dataScienceIocContainer.ts @@ -367,6 +367,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer { public get mockJupyter(): MockJupyterManager | undefined { return this.jupyterMock ? this.jupyterMock.getManager() : undefined; } + private static jupyterInterpreters: PythonInterpreter[] = []; public webPanelListener: IWebPanelMessageListener | undefined; public readonly useCommandFinderForJupyterServer = false; public wrapper: ReactWrapper, React.Component> | undefined; @@ -1125,11 +1126,21 @@ export class DataScienceIocContainer extends UnitTestIocContainer { }); } + public async getJupyterCapableInterpreter(): Promise { + const list = await this.getJupyterInterpreters(); + return list ? list[0] : undefined; + } + public async getJupyterInterpreters(): Promise { + // This should be cacheable as we don't install new interpreters during tests + if (DataScienceIocContainer.jupyterInterpreters.length > 0) { + return DataScienceIocContainer.jupyterInterpreters; + } const list = await this.get(IInterpreterService).getInterpreters(undefined); const promises = list.map(f => this.hasJupyter(f).then(b => (b ? f : undefined))); const resolved = await Promise.all(promises); - return resolved.filter(r => r) as PythonInterpreter[]; + DataScienceIocContainer.jupyterInterpreters = resolved.filter(r => r) as PythonInterpreter[]; + return DataScienceIocContainer.jupyterInterpreters; } public addWorkspaceFolder(folderPath: string) { diff --git a/src/test/datascience/debugger.functional.test.tsx b/src/test/datascience/debugger.functional.test.tsx index 3b269a55239a..6d1ae82079d4 100644 --- a/src/test/datascience/debugger.functional.test.tsx +++ b/src/test/datascience/debugger.functional.test.tsx @@ -22,7 +22,7 @@ import { } from '../../client/datascience/types'; import { DataScienceIocContainer } from './dataScienceIocContainer'; import { getInteractiveCellResults, getOrCreateInteractiveWindow } from './interactiveWindowTestHelpers'; -import { getConnectionInfo, getNotebookCapableInterpreter } from './jupyterHelpers'; +import { getConnectionInfo } from './jupyterHelpers'; import { MockDebuggerService } from './mockDebugService'; import { MockDocument } from './mockDocument'; import { MockDocumentManager } from './mockDocumentManager'; @@ -224,7 +224,7 @@ suite('DataScience Debugger tests', () => { }); test('Debug remote', async () => { - const python = await getNotebookCapableInterpreter(ioc, processFactory); + const python = await ioc.getJupyterCapableInterpreter(); const procService = await processFactory.create(); if (procService && python) { diff --git a/src/test/datascience/jupyterHelpers.ts b/src/test/datascience/jupyterHelpers.ts index 25d4bde79bc7..fd60a39737ce 100644 --- a/src/test/datascience/jupyterHelpers.ts +++ b/src/test/datascience/jupyterHelpers.ts @@ -1,30 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. 'use strict'; -import { IProcessServiceFactory } from '../../client/common/process/types'; -import { IInterpreterService, PythonInterpreter } from '../../client/interpreter/contracts'; -import { DataScienceIocContainer } from './dataScienceIocContainer'; - -export async function getNotebookCapableInterpreter( - ioc: DataScienceIocContainer, - processFactory: IProcessServiceFactory -): Promise { - const is = ioc.serviceContainer.get(IInterpreterService); - const list = await is.getInterpreters(); - const procService = await processFactory.create(); - if (procService) { - // tslint:disable-next-line:prefer-for-of - for (let i = 0; i < list.length; i += 1) { - const result = await procService.exec(list[i].path, ['-m', 'jupyter', 'notebook', '--version'], { - env: process.env - }); - if (!result.stderr) { - return list[i]; - } - } - } - return undefined; -} // IP = * format is a bit different from localhost format export function getIPConnectionInfo(output: string): string | undefined { diff --git a/src/test/datascience/notebook.functional.test.ts b/src/test/datascience/notebook.functional.test.ts index d8b6ca8c34d2..e81a387cb3d6 100644 --- a/src/test/datascience/notebook.functional.test.ts +++ b/src/test/datascience/notebook.functional.test.ts @@ -21,7 +21,7 @@ import { Cancellation, CancellationError } from '../../client/common/cancellatio import { EXTENSION_ROOT_DIR } from '../../client/common/constants'; import { traceError, traceInfo } from '../../client/common/logger'; import { IFileSystem } from '../../client/common/platform/types'; -import { IProcessServiceFactory, IPythonExecutionFactory, Output } from '../../client/common/process/types'; +import { IPythonExecutionFactory, IPythonExecutionService, Output } from '../../client/common/process/types'; import { Product } from '../../client/common/types'; import { createDeferred, waitForPromise } from '../../client/common/utils/async'; import { noop } from '../../client/common/utils/misc'; @@ -54,7 +54,7 @@ import { concatMultilineStringInput } from '../../datascience-ui/common'; import { generateTestState, ICellViewModel } from '../../datascience-ui/interactive-common/mainState'; import { sleep } from '../core'; import { DataScienceIocContainer } from './dataScienceIocContainer'; -import { getConnectionInfo, getIPConnectionInfo, getNotebookCapableInterpreter } from './jupyterHelpers'; +import { getConnectionInfo, getIPConnectionInfo } from './jupyterHelpers'; import { SupportedCommands } from './mockJupyterManager'; import { MockPythonService } from './mockPythonService'; @@ -62,7 +62,7 @@ import { MockPythonService } from './mockPythonService'; suite('DataScience notebook tests', () => { const disposables: Disposable[] = []; let jupyterExecution: IJupyterExecution; - let processFactory: IProcessServiceFactory; + let pythonFactory: IPythonExecutionFactory; let ioc: DataScienceIocContainer; let modifiedConfig = false; const baseUri = Uri.file('foo.py'); @@ -77,12 +77,9 @@ suite('DataScience notebook tests', () => { try { if (modifiedConfig) { traceInfo('Attempting to put jupyter default config back'); - const python = await getNotebookCapableInterpreter(ioc, processFactory); - const procService = await processFactory.create(); - if (procService && python) { - await procService.exec(python.path, ['-m', 'jupyter', 'notebook', '--generate-config', '-y'], { - env: process.env - }); + const procService = await createPythonService(); + if (procService) { + await procService.exec(['-m', 'jupyter', 'notebook', '--generate-config', '-y'], {}); } } traceInfo('Shutting down after test.'); @@ -270,7 +267,7 @@ suite('DataScience notebook tests', () => { rebindFunc(); } jupyterExecution = ioc.serviceManager.get(IJupyterExecution); - processFactory = ioc.serviceManager.get(IProcessServiceFactory); + pythonFactory = ioc.serviceManager.get(IPythonExecutionFactory); console.log(`Starting test ${name} ...`); if (await jupyterExecution.isNotebookSupported()) { // tslint:disable-next-line: no-invalid-this @@ -341,12 +338,29 @@ suite('DataScience notebook tests', () => { } } - runTest('Remote Self Certs', async (_this: Mocha.Context) => { - const python = await getNotebookCapableInterpreter(ioc, processFactory); + async function createPythonService(versionRequirement?: number): Promise { + if (!ioc.mockJupyter) { + const python = await ioc.getJupyterCapableInterpreter(); + + if ( + python && + python.version?.major && + (!versionRequirement || python.version?.major > versionRequirement) + ) { + return pythonFactory.createActivatedEnvironment({ + resource: undefined, + interpreter: python, + allowEnvironmentFetchExceptions: true, + bypassCondaExecution: true + }); + } + } + } - if (python && python.version?.major && python.version?.major > 2) { - const procService = await processFactory.create(); + runTest('Remote Self Certs', async (_this: Mocha.Context) => { + const pythonService = await createPythonService(2); + if (pythonService) { // We will only connect if we allow for self signed cert connections ioc.getSettings().datascience.allowUnauthorizedRemoteConnection = true; @@ -376,8 +390,7 @@ suite('DataScience notebook tests', () => { 'jkey.key' ); - const exeResult = procService.execObservable( - python.path, + const exeResult = pythonService.execObservable( [ '-m', 'jupyter', @@ -387,7 +400,6 @@ suite('DataScience notebook tests', () => { `--keyfile=${keyFile}` ], { - env: process.env, throwOnStdErr: false } ); @@ -425,10 +437,9 @@ suite('DataScience notebook tests', () => { runTest( 'Remote No Auth', async () => { - const python = await getNotebookCapableInterpreter(ioc, processFactory); - const procService = await processFactory.create(); + const pythonService = await createPythonService(); - if (procService && python) { + if (pythonService) { const connectionFound = createDeferred(); const configFile = path.join( EXTENSION_ROOT_DIR, @@ -438,10 +449,9 @@ suite('DataScience notebook tests', () => { 'serverConfigFiles', 'remoteNoAuth.py' ); - const exeResult = procService.execObservable( - python.path, + const exeResult = pythonService.execObservable( ['-m', 'jupyter', 'notebook', `--config=${configFile}`], - { env: process.env, throwOnStdErr: false } + { throwOnStdErr: false } ); disposables.push(exeResult); @@ -510,10 +520,9 @@ suite('DataScience notebook tests', () => { ); runTest('Remote Password', async () => { - const python = await getNotebookCapableInterpreter(ioc, processFactory); - const procService = await processFactory.create(); + const pythonService = await createPythonService(); - if (procService && python) { + if (pythonService) { const connectionFound = createDeferred(); const configFile = path.join( EXTENSION_ROOT_DIR, @@ -523,11 +532,9 @@ suite('DataScience notebook tests', () => { 'serverConfigFiles', 'remotePassword.py' ); - const exeResult = procService.execObservable( - python.path, - ['-m', 'jupyter', 'notebook', `--config=${configFile}`], - { env: process.env, throwOnStdErr: false } - ); + const exeResult = pythonService.execObservable(['-m', 'jupyter', 'notebook', `--config=${configFile}`], { + throwOnStdErr: false + }); disposables.push(exeResult); exeResult.out.subscribe((output: Output) => { @@ -557,10 +564,9 @@ suite('DataScience notebook tests', () => { }); runTest('Remote', async () => { - const python = await getNotebookCapableInterpreter(ioc, processFactory); - const procService = await processFactory.create(); + const pythonService = await createPythonService(); - if (procService && python) { + if (pythonService) { const connectionFound = createDeferred(); const configFile = path.join( EXTENSION_ROOT_DIR, @@ -570,11 +576,9 @@ suite('DataScience notebook tests', () => { 'serverConfigFiles', 'remoteToken.py' ); - const exeResult = procService.execObservable( - python.path, - ['-m', 'jupyter', 'notebook', `--config=${configFile}`], - { env: process.env, throwOnStdErr: false } - ); + const exeResult = pythonService.execObservable(['-m', 'jupyter', 'notebook', `--config=${configFile}`], { + throwOnStdErr: false + }); disposables.push(exeResult); exeResult.out.subscribe((output: Output) => { @@ -622,7 +626,6 @@ suite('DataScience notebook tests', () => { test('Not installed', async () => { jupyterExecution = ioc.serviceManager.get(IJupyterExecution); - processFactory = ioc.serviceManager.get(IProcessServiceFactory); // Rewire our data we use to search for processes @injectable() class EmptyInterpreterService implements IInterpreterService { @@ -1169,17 +1172,13 @@ plt.show()`, ]); async function generateNonDefaultConfig() { - const usable = await getNotebookCapableInterpreter(ioc, processFactory); + const usable = await ioc.getJupyterCapableInterpreter(); assert.ok(usable, 'Cant find jupyter enabled python'); // Manually generate an invalid jupyter config - const procService = await processFactory.create(); + const procService = await createPythonService(); assert.ok(procService, 'Can not get a process service'); - const results = await procService!.exec( - usable!.path, - ['-m', 'jupyter', 'notebook', '--generate-config', '-y'], - { env: process.env } - ); + const results = await procService!.exec(['-m', 'jupyter', 'notebook', '--generate-config', '-y'], {}); // Results should have our path to the config. const match = /^.*\s+(.*jupyter_notebook_config.py)\s+.*$/m.exec(results.stdout); @@ -1437,7 +1436,6 @@ plt.show()`, ioc.serviceManager.rebindInstance(IApplicationShell, instance(application)); jupyterExecution = ioc.serviceManager.get(IJupyterExecution); - processFactory = ioc.serviceManager.get(IProcessServiceFactory); // Change notebook command to fail with some goofy output await disableJupyter(ioc.workingInterpreter.path); From 87fbdbb2e9957a271110d12bef30dc940460ee2a Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Mon, 9 Mar 2020 17:02:56 -0700 Subject: [PATCH 22/33] Fix missing args when test is shutting down --- .../datascience/interactive-common/interactiveBase.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/client/datascience/interactive-common/interactiveBase.ts b/src/client/datascience/interactive-common/interactiveBase.ts index 7cb0f7a7e04d..cadd8538381f 100644 --- a/src/client/datascience/interactive-common/interactiveBase.ts +++ b/src/client/datascience/interactive-common/interactiveBase.ts @@ -1419,7 +1419,12 @@ export abstract class InteractiveBase extends WebViewHost Date: Mon, 9 Mar 2020 18:13:03 -0700 Subject: [PATCH 23/33] Put back some of the functional requirements --- build/ci/templates/test_phases.yml | 14 ++++++++++++++ build/functional-test-requirements.txt | 2 ++ 2 files changed, 16 insertions(+) create mode 100644 build/functional-test-requirements.txt diff --git a/build/ci/templates/test_phases.yml b/build/ci/templates/test_phases.yml index 435fe47e2f26..d404c97e20bb 100644 --- a/build/ci/templates/test_phases.yml +++ b/build/ci/templates/test_phases.yml @@ -97,6 +97,20 @@ 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 requirements for functional tests. + # + # This task will only run if variable `NeedsPythonFunctionalReqs` is true. + # + # Example command line (windows pwsh): + # > python -m pip install numpy + # > python -m pip install --upgrade -r build/functional-test-requirements.txt + # > python -m pip --disable-pip-version-check install -t ./pythonFiles/lib/python --no-cache-dir --implementation py --no-deps --upgrade -r requirements.txt + - bash: | + python -m pip install numpy + python -m pip install --upgrade -r ./build/functional-test-requirements.txt + displayName: 'pip install functional requirements' + condition: and(succeeded(), eq(variables['NeedsPythonFunctionalReqs'], 'true')) # Add CONDA to the path so anaconda works # diff --git a/build/functional-test-requirements.txt b/build/functional-test-requirements.txt new file mode 100644 index 000000000000..d2f1977a7be4 --- /dev/null +++ b/build/functional-test-requirements.txt @@ -0,0 +1,2 @@ +# List of requirements for functional tests +versioneer From 587bafc633bf2388bd86ccec6ae1669a2d0c6c9e Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Tue, 10 Mar 2020 08:52:40 -0700 Subject: [PATCH 24/33] Update tests to not use static --- build/ci/vscode-python-nightly-flake-ci.yaml | 48 ------------------- src/client/interpreter/activation/service.ts | 3 ++ .../services/cacheableLocatorService.ts | 3 ++ .../datascience/dataScienceIocContainer.ts | 17 ++++--- 4 files changed, 17 insertions(+), 54 deletions(-) diff --git a/build/ci/vscode-python-nightly-flake-ci.yaml b/build/ci/vscode-python-nightly-flake-ci.yaml index d9a749d90e59..99b952aa4191 100644 --- a/build/ci/vscode-python-nightly-flake-ci.yaml +++ b/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 diff --git a/src/client/interpreter/activation/service.ts b/src/client/interpreter/activation/service.ts index 51716435d16a..4275bccfa061 100644 --- a/src/client/interpreter/activation/service.ts +++ b/src/client/interpreter/activation/service.ts @@ -48,6 +48,9 @@ export class EnvironmentActivationServiceCache { public static forceUseStatic() { EnvironmentActivationServiceCache.useStatic = true; } + public static forceUseNormal() { + EnvironmentActivationServiceCache.useStatic = false; + } public get(key: string): InMemoryCache | undefined { if (EnvironmentActivationServiceCache.useStatic) { return EnvironmentActivationServiceCache.staticMap.get(key); diff --git a/src/client/interpreter/locators/services/cacheableLocatorService.ts b/src/client/interpreter/locators/services/cacheableLocatorService.ts index f2608fed6a34..2e2a34b83b79 100644 --- a/src/client/interpreter/locators/services/cacheableLocatorService.ts +++ b/src/client/interpreter/locators/services/cacheableLocatorService.ts @@ -25,6 +25,9 @@ export class CacheableLocatorPromiseCache { public static forceUseStatic() { CacheableLocatorPromiseCache.useStatic = true; } + public static forceUseNormal() { + CacheableLocatorPromiseCache.useStatic = false; + } public get(key: string): Deferred | undefined { if (CacheableLocatorPromiseCache.useStatic) { return CacheableLocatorPromiseCache.staticMap.get(key); diff --git a/src/test/datascience/dataScienceIocContainer.ts b/src/test/datascience/dataScienceIocContainer.ts index ee4bcd5bc0fa..d8217add53f5 100644 --- a/src/test/datascience/dataScienceIocContainer.ts +++ b/src/test/datascience/dataScienceIocContainer.ts @@ -476,10 +476,21 @@ export class DataScienceIocContainer extends UnitTestIocContainer { this.extraListeners = []; this.webPanelListener = undefined; reset(this.webPanelProvider); + + // Turn off the static maps for the environment and conda services. Otherwise this + // can mess up tests that don't depend upon them + CacheableLocatorPromiseCache.forceUseNormal(); + EnvironmentActivationServiceCache.forceUseNormal(); } //tslint:disable:max-func-body-length public registerDataScienceTypes(useCustomEditor: boolean = false) { + // Inform the cacheable locator service to use a static map so that it stays in memory in between tests + CacheableLocatorPromiseCache.forceUseStatic(); + + // Do the same thing for the environment variable activation service. + EnvironmentActivationServiceCache.forceUseStatic(); + // Make sure the default python path is set. this.defaultPythonPath = this.findPythonPath(); @@ -797,12 +808,6 @@ export class DataScienceIocContainer extends UnitTestIocContainer { new TestPersistentStateFactory(globalStorage, localStorage) ); - // Inform the cacheable locator service to use a static map so that it stays in memory in between tests - CacheableLocatorPromiseCache.forceUseStatic(); - - // Do the same thing for the environment variable activation service. - EnvironmentActivationServiceCache.forceUseStatic(); - const currentProcess = new CurrentProcess(); this.serviceManager.addSingletonInstance(ICurrentProcess, currentProcess); this.serviceManager.addSingleton(IRegistry, RegistryImplementation); From 94b75fdebbb7dbb716a7c486a9b65bdb9b59ceba Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Tue, 10 Mar 2020 11:14:45 -0700 Subject: [PATCH 25/33] Fix linter tests --- src/test/linters/lint.functional.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/test/linters/lint.functional.test.ts b/src/test/linters/lint.functional.test.ts index 6050e5caf373..bf8327e0298f 100644 --- a/src/test/linters/lint.functional.test.ts +++ b/src/test/linters/lint.functional.test.ts @@ -662,7 +662,9 @@ class TestFixture extends BaseTestFixture { undefined, TypeMoq.MockBehavior.Strict ); - envVarsService.setup(e => e.getEnvironmentVariables(TypeMoq.It.isAny())).returns(() => Promise.resolve({})); + envVarsService + .setup(e => e.getEnvironmentVariables(TypeMoq.It.isAny())) + .returns(() => Promise.resolve(process.env)); serviceContainer .setup(c => c.get(TypeMoq.It.isValue(IEnvironmentVariablesProvider), TypeMoq.It.isAny())) .returns(() => envVarsService.object); From 7712c4745ccce7a33bab3fce442ccc8df639496f Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Tue, 10 Mar 2020 12:42:47 -0700 Subject: [PATCH 26/33] More output for linter tests --- build/.mocha.functional.opts | 1 + build/ci/templates/test_phases.yml | 1 + src/test/linters/lint.functional.test.ts | 5 +++++ 3 files changed, 7 insertions(+) diff --git a/build/.mocha.functional.opts b/build/.mocha.functional.opts index 47ae4d6eeb25..d98784902d7e 100644 --- a/build/.mocha.functional.opts +++ b/build/.mocha.functional.opts @@ -8,3 +8,4 @@ --timeout=180000 --reporter mocha-multi-reporters --reporter-options configFile=build/.mocha-multi-reporters.config +--grep="Linting Functional" diff --git a/build/ci/templates/test_phases.yml b/build/ci/templates/test_phases.yml index d404c97e20bb..f7a11a69ba79 100644 --- a/build/ci/templates/test_phases.yml +++ b/build/ci/templates/test_phases.yml @@ -109,6 +109,7 @@ 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')) diff --git a/src/test/linters/lint.functional.test.ts b/src/test/linters/lint.functional.test.ts index bf8327e0298f..9d5980d27b80 100644 --- a/src/test/linters/lint.functional.test.ts +++ b/src/test/linters/lint.functional.test.ts @@ -3,6 +3,7 @@ 'use strict'; import * as assert from 'assert'; +import * as child_process from 'child_process'; import * as fs from 'fs-extra'; import * as os from 'os'; import * as path from 'path'; @@ -754,6 +755,10 @@ suite('Linting Functional Tests', () => { fixture.serviceContainer.object ); + const pythonPath = child_process.execSync(`python -c "import sys;print(sys.executable)"`); + // tslint:disable-next-line: no-console + console.log(`Testing linter with python ${pythonPath}`); + const messages = await linter.lint(doc, new CancellationTokenSource().token); if (messagesToBeReceived.length === 0) { From b9fc1393b0fac5ed1554dc9e3bd819bcb1a43571 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Tue, 10 Mar 2020 13:43:40 -0700 Subject: [PATCH 27/33] Linter only fails with other tests --- build/.mocha.functional.opts | 1 - 1 file changed, 1 deletion(-) diff --git a/build/.mocha.functional.opts b/build/.mocha.functional.opts index d98784902d7e..47ae4d6eeb25 100644 --- a/build/.mocha.functional.opts +++ b/build/.mocha.functional.opts @@ -8,4 +8,3 @@ --timeout=180000 --reporter mocha-multi-reporters --reporter-options configFile=build/.mocha-multi-reporters.config ---grep="Linting Functional" From 6f85ffb4e2287e82a96d969d70febb4e7119ca0c Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Tue, 10 Mar 2020 15:04:41 -0700 Subject: [PATCH 28/33] Install linters in conda too --- build/ci/templates/test_phases.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/build/ci/templates/test_phases.yml b/build/ci/templates/test_phases.yml index f7a11a69ba79..b50f838338b5 100644 --- a/build/ci/templates/test_phases.yml +++ b/build/ci/templates/test_phases.yml @@ -149,15 +149,20 @@ steps: # Run the pip installs in the two environments (darwin linux) - bash: | + source activate base + 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 two environments (windows) - script: | + call activate base + 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 From 130fc50c15a84d7d813e64c871e81dee327f9758 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Tue, 10 Mar 2020 16:31:13 -0700 Subject: [PATCH 29/33] Make base also have dependencies as it's being picked up anyway --- build/ci/conda_base.yml | 4 ++++ build/ci/templates/test_phases.yml | 6 ++++-- 2 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 build/ci/conda_base.yml diff --git a/build/ci/conda_base.yml b/build/ci/conda_base.yml new file mode 100644 index 000000000000..171868c9b879 --- /dev/null +++ b/build/ci/conda_base.yml @@ -0,0 +1,4 @@ +pandas +jupyter +numpy +matplotlib \ No newline at end of file diff --git a/build/ci/templates/test_phases.yml b/build/ci/templates/test_phases.yml index b50f838338b5..558aeafc0c95 100644 --- a/build/ci/templates/test_phases.yml +++ b/build/ci/templates/test_phases.yml @@ -147,9 +147,10 @@ steps: displayName: 'Create CONDA Environments' condition: and(succeeded(), eq(variables['NeedsPythonFunctionalReqs'], 'true')) - # Run the pip installs in the two environments (darwin linux) + # 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 @@ -159,9 +160,10 @@ steps: 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 two environments (windows) + # 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 From 48cc263154031472af3466f1a70fc9ef6070fc2b Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Wed, 11 Mar 2020 09:03:49 -0700 Subject: [PATCH 30/33] Add news entry --- news/3 Code Health/10134.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/3 Code Health/10134.md diff --git a/news/3 Code Health/10134.md b/news/3 Code Health/10134.md new file mode 100644 index 000000000000..4030f2477b91 --- /dev/null +++ b/news/3 Code Health/10134.md @@ -0,0 +1 @@ +Add conda environments to nightly test runs \ No newline at end of file From bcdfdaf16e3ad8609ac217a2dfaa79e0a996665c Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Wed, 11 Mar 2020 10:39:35 -0700 Subject: [PATCH 31/33] Review feedback --- .../datascience/jupyter/jupyterSessionManager.ts | 8 ++++++-- src/client/interpreter/activation/service.ts | 15 +++++++++++++-- .../locators/services/cacheableLocatorService.ts | 6 ++++++ 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/client/datascience/jupyter/jupyterSessionManager.ts b/src/client/datascience/jupyter/jupyterSessionManager.ts index ee864a0bc4c9..f51796d8a51f 100644 --- a/src/client/datascience/jupyter/jupyterSessionManager.ts +++ b/src/client/datascience/jupyter/jupyterSessionManager.ts @@ -51,8 +51,12 @@ export class JupyterSessionManager implements IJupyterSessionManager { // tslint:disable-next-line: no-any const sessionManager = this.sessionManager as any; - this.sessionManager.dispose(); - this.sessionManager = undefined; + try { + await this.sessionManager.shutdownAll(); + } finally { + this.sessionManager.dispose(); + 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/interpreter/activation/service.ts b/src/client/interpreter/activation/service.ts index 4275bccfa061..247b87e9ebbb 100644 --- a/src/client/interpreter/activation/service.ts +++ b/src/client/interpreter/activation/service.ts @@ -40,6 +40,12 @@ const condaRetryMessages = [ 'The directory is not empty' ]; +/** + * This class exists so that the environment variable fetching can be cached in between tests. Normally + * this cache resides in memory for the duration of the EnvironmentActivationService's lifetime, but in the case + * of our functional tests, we want the cached data to exist outside of each test (where each test will destroy the EnvironmentActivationService) + * This gives each test a 3 or 4 second speedup. + */ export class EnvironmentActivationServiceCache { private static useStatic = false; private static staticMap = new Map>(); @@ -180,11 +186,15 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi const command = `${activationCommand} && echo '${getEnvironmentPrefix}' && python ${printEnvPyFile.fileToCommandArgument()}`; traceVerbose(`Activating Environment to capture Environment variables, ${command}`); - // Conda activate can hang on certain systems. Fail after 30 seconds. + // Do some wrapping of the call. For two reasons: + // 1) Conda activate can hang on certain systems. Fail after 30 seconds. // See the discussion from hidesoon in this issue: https://github.com/Microsoft/vscode-python/issues/4424 // His issue is conda never finishing during activate. This is a conda issue, but we // should at least tell the user. + // 2) Retry because of this issue here: https://github.com/microsoft/vscode-python/issues/9244 + // This happens on AzDo machines a bunch when using Conda (and we can't dictate the conda version in order to get the fix) let result: ExecutionResult | undefined; + let tryCount = 1; while (!result) { try { result = await processService.shellExec(command, { @@ -201,9 +211,10 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi // Special case. Conda for some versions will state a file is in use. If // that's the case, wait and try again. This happens especially on AzDo const excString = exc.toString(); - if (condaRetryMessages.find(m => excString.includes(m))) { + if (condaRetryMessages.find(m => excString.includes(m)) && tryCount < 10) { traceInfo(`Conda is busy, attempting to retry ...`); result = undefined; + tryCount += 1; await sleep(500); } else { throw exc; diff --git a/src/client/interpreter/locators/services/cacheableLocatorService.ts b/src/client/interpreter/locators/services/cacheableLocatorService.ts index 2e2a34b83b79..d30625dc81f2 100644 --- a/src/client/interpreter/locators/services/cacheableLocatorService.ts +++ b/src/client/interpreter/locators/services/cacheableLocatorService.ts @@ -17,6 +17,12 @@ import { sendTelemetryEvent } from '../../../telemetry'; import { EventName } from '../../../telemetry/constants'; import { IInterpreterLocatorService, IInterpreterWatcher, PythonInterpreter } from '../../contracts'; +/** + * This class exists so that the interpreter fetching can be cached in between tests. Normally + * this cache resides in memory for the duration of the CacheableLocatorService's lifetime, but in the case + * of our functional tests, we want the cached data to exist outside of each test (where each test will destroy the CacheableLocatorService) + * This gives each test a 20 second speedup. + */ export class CacheableLocatorPromiseCache { private static useStatic = false; private static staticMap = new Map>(); From 04d6caac7b5c404a1a23fbd52e526aa96bc12c22 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Wed, 11 Mar 2020 13:32:14 -0700 Subject: [PATCH 32/33] Fix unit tests and one of the functional tests --- .../datascience/jupyter/jupyterSession.ts | 2 +- .../jupyter/jupyterSessionManager.ts | 2 +- src/test/common/moduleInstaller.test.ts | 1 - .../jupyter/jupyterSession.unit.test.ts | 62 +++++++------------ .../nativeEditor.functional.test.tsx | 4 ++ .../activation/service.unit.test.ts | 8 ++- 6 files changed, 34 insertions(+), 45 deletions(-) diff --git a/src/client/datascience/jupyter/jupyterSession.ts b/src/client/datascience/jupyter/jupyterSession.ts index 5dde9b0ba275..be26ef274e02 100644 --- a/src/client/datascience/jupyter/jupyterSession.ts +++ b/src/client/datascience/jupyter/jupyterSession.ts @@ -442,7 +442,7 @@ export class JupyterSession implements IJupyterSession { this.sessionManager!.startNew(options) .then(s => { this.logRemoteOutput( - localize.DataScience.createdNewKernel().format(this.connInfo.baseUrl, s.kernel.id) + localize.DataScience.createdNewKernel().format(this.connInfo.baseUrl, s?.kernel?.id) ); return s; }) diff --git a/src/client/datascience/jupyter/jupyterSessionManager.ts b/src/client/datascience/jupyter/jupyterSessionManager.ts index f51796d8a51f..c9ab3091006c 100644 --- a/src/client/datascience/jupyter/jupyterSessionManager.ts +++ b/src/client/datascience/jupyter/jupyterSessionManager.ts @@ -5,10 +5,10 @@ import { ContentsManager, Kernel, ServerConnection, Session, SessionManager } fr import { Agent as HttpsAgent } from 'https'; import { CancellationToken } from 'vscode-jsonrpc'; -import { noop } from 'jquery'; import { traceInfo } from '../../common/logger'; import { IConfigurationService, IOutputChannel } from '../../common/types'; import * as localize from '../../common/utils/localize'; +import { noop } from '../../common/utils/misc'; import { IConnection, IJupyterKernel, diff --git a/src/test/common/moduleInstaller.test.ts b/src/test/common/moduleInstaller.test.ts index d9f3bd8eeffc..89f951e3863b 100644 --- a/src/test/common/moduleInstaller.test.ts +++ b/src/test/common/moduleInstaller.test.ts @@ -86,7 +86,6 @@ suite('Module Installer', () => { }); suiteTeardown(async () => { await closeActiveWindows(); - await resetSettings(); }); teardown(async () => { await ioc.dispose(); diff --git a/src/test/datascience/jupyter/jupyterSession.unit.test.ts b/src/test/datascience/jupyter/jupyterSession.unit.test.ts index a2ed45bc6e03..0789f5ad70c0 100644 --- a/src/test/datascience/jupyter/jupyterSession.unit.test.ts +++ b/src/test/datascience/jupyter/jupyterSession.unit.test.ts @@ -10,6 +10,7 @@ import * as sinon from 'sinon'; import { anything, deepEqual, instance, mock, verify, when } from 'ts-mockito'; import * as typemoq from 'typemoq'; +import { traceInfo } from '../../../client/common/logger'; import { createDeferred, Deferred } from '../../../client/common/utils/async'; import { DataScience } from '../../../client/common/utils/localize'; import { noop } from '../../../client/common/utils/misc'; @@ -259,22 +260,22 @@ suite('Data Science - JupyterSession', () => { }); }); suite('Local Sessions', async () => { - let restartSession: Session.ISession; - let restartKernel: Kernel.IKernelConnection; - let restartStatusChangedSignal: ISignal; - let restartKernelChangedSignal: ISignal; + let newSession: Session.ISession; + let newKernelConnection: Kernel.IKernelConnection; + let newStatusChangedSignal: ISignal; + let newKernelChangedSignal: ISignal; let kernelAddedToIgnoreList: Deferred; let kernelRemovedFromIgnoreList: Deferred; let newSessionCreated: Deferred; setup(async () => { - restartSession = mock(DefaultSession); - restartKernel = mock(DefaultKernel); - restartStatusChangedSignal = mock(Signal); - restartKernelChangedSignal = mock(Signal); + newSession = mock(DefaultSession); + newKernelConnection = mock(DefaultKernel); + newStatusChangedSignal = mock(Signal); + newKernelChangedSignal = mock(Signal); kernelAddedToIgnoreList = createDeferred(); kernelRemovedFromIgnoreList = createDeferred(); - when(restartSession.statusChanged).thenReturn(instance(restartStatusChangedSignal)); - when(restartSession.kernelChanged).thenReturn(instance(restartKernelChangedSignal)); + when(newSession.statusChanged).thenReturn(instance(newStatusChangedSignal)); + when(newSession.kernelChanged).thenReturn(instance(newKernelChangedSignal)); when(kernelSelector.addKernelToIgnoreList(anything())).thenCall(() => kernelAddedToIgnoreList.resolve() ); @@ -282,17 +283,17 @@ suite('Data Science - JupyterSession', () => { kernelRemovedFromIgnoreList.resolve() ); // tslint:disable-next-line: no-any - (instance(restartSession) as any).then = undefined; + (instance(newSession) as any).then = undefined; newSessionCreated = createDeferred(); when(session.isRemoteSession).thenReturn(false); when(session.isDisposed).thenReturn(false); - when(restartKernel.id).thenReturn('restartId'); - when(restartKernel.clientId).thenReturn('restartClientId'); - when(restartKernel.status).thenReturn('idle'); - when(restartSession.kernel).thenReturn(instance(restartKernel)); + when(newKernelConnection.id).thenReturn('restartId'); + when(newKernelConnection.clientId).thenReturn('restartClientId'); + when(newKernelConnection.status).thenReturn('idle'); + when(newSession.kernel).thenReturn(instance(newKernelConnection)); when(sessionManager.startNew(anything())).thenCall(() => { newSessionCreated.resolve(); - return Promise.resolve(instance(restartSession)); + return Promise.resolve(instance(newSession)); }); }); teardown(() => { @@ -313,7 +314,7 @@ suite('Data Science - JupyterSession', () => { // Wait untill a new session has been started. await newSessionCreated.promise; - // One original, one new session and one restart session. + // One original, one new session. verify(sessionManager.startNew(anything())).thrice(); }); suite('Executing user code', async () => { @@ -330,30 +331,8 @@ suite('Data Science - JupyterSession', () => { assert.isOk(result); await result!.done; - - // Wait untill a new session has been started. - await newSessionCreated.promise; } - test('Must start a restart session', async () => { - verify(sessionManager.startNew(anything())).twice(); - }); - test('Restart session must be excluded from kernel picker', async () => { - await kernelAddedToIgnoreList.promise; - verify(kernelSelector.addKernelToIgnoreList(anything())).once(); - }); - test('Shutdown kills restart Session', async () => { - connection.setup(c => c.localLaunch).returns(() => true); - when(session.isRemoteSession).thenReturn(false); - when(session.isDisposed).thenReturn(false); - when(session.shutdown()).thenResolve(); - when(session.dispose()).thenReturn(); - - await jupyterSession.shutdown(); - - verify(restartSession.shutdown()).once(); - verify(restartSession.dispose()).once(); - }); test('Restart should create a new session & kill old session', async () => { const oldSessionShutDown = createDeferred(); connection.setup(c => c.localLaunch).returns(() => true); @@ -363,7 +342,10 @@ suite('Data Science - JupyterSession', () => { oldSessionShutDown.resolve(); return Promise.resolve(); }); - when(session.dispose()).thenReturn(); + when(session.dispose()).thenCall(() => { + traceInfo('Shutting down'); + return Promise.resolve(); + }); await jupyterSession.restart(0); diff --git a/src/test/datascience/nativeEditor.functional.test.tsx b/src/test/datascience/nativeEditor.functional.test.tsx index 55459f4e0d88..c8e76b560771 100644 --- a/src/test/datascience/nativeEditor.functional.test.tsx +++ b/src/test/datascience/nativeEditor.functional.test.tsx @@ -482,6 +482,10 @@ df.head()`; async (_wrapper, context) => { if (ioc.mockJupyter) { await ioc.activate(); + ioc.forceSettingsChanged(undefined, ioc.getSettings().pythonPath, { + ...ioc.getSettings().datascience, + disableJupyterAutoStart: false + }); // Create an editor so something is listening to messages const editor = await createNewEditor(ioc); diff --git a/src/test/interpreters/activation/service.unit.test.ts b/src/test/interpreters/activation/service.unit.test.ts index cea844a7c247..18c9d9014dc9 100644 --- a/src/test/interpreters/activation/service.unit.test.ts +++ b/src/test/interpreters/activation/service.unit.test.ts @@ -171,11 +171,13 @@ suite('Interpreters Activation - Python Environment Variables', () => { const options = capture(processService.shellExec).first()[1]; const expectedShell = defaultShells[osType.value]; + // tslint:disable-next-line: chai-vague-errors expect(options).to.deep.equal({ shell: expectedShell, env: envVars, timeout: 30000, - maxBuffer: 1000 * 1000 + maxBuffer: 1000 * 1000, + throwOnStdErr: false }); }); test('Use current process variables if there are no custom variables', async () => { @@ -204,11 +206,13 @@ suite('Interpreters Activation - Python Environment Variables', () => { const options = capture(processService.shellExec).first()[1]; const expectedShell = defaultShells[osType.value]; + // tslint:disable-next-line: chai-vague-errors expect(options).to.deep.equal({ env: envVars, shell: expectedShell, timeout: 30000, - maxBuffer: 1000 * 1000 + maxBuffer: 1000 * 1000, + throwOnStdErr: false }); }); test('Error must be swallowed when activation fails', async () => { From 1f03b7c30353b9f141cc7dd8794a3c8fceaf0e09 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Wed, 11 Mar 2020 14:10:44 -0700 Subject: [PATCH 33/33] Functional tests and workspace tests at the same time fail --- build/conda-functional-requirements.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/build/conda-functional-requirements.txt b/build/conda-functional-requirements.txt index 3b267106ba10..7f11f53981a7 100644 --- a/build/conda-functional-requirements.txt +++ b/build/conda-functional-requirements.txt @@ -11,7 +11,9 @@ 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)