From 7b1295642485ad81cf41dd91450f127f85d2085e Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 21 Jul 2022 19:49:32 +1000 Subject: [PATCH] Use python.envFile to get env file name when running Python code (#10876) * Use python.envFile for python kernels * Added tests * Misc * Misc * Address code review comments * Logging * oops * Misc * Misc * Misc * Misc * Fixes * Misc * oops * Misc * Misc * Misc * oops * Misc * misc * Misc --- src/kernels/raw/finder/jupyterPaths.node.ts | 2 +- .../raw/launcher/kernelEnvVarsService.node.ts | 7 +- .../variables/preWarmVariables.node.ts | 6 +- .../environmentActivationService.node.ts | 40 +-- .../common/process/processFactory.node.ts | 2 +- ...customEnvironmentVariablesProvider.node.ts | 136 +++++--- src/platform/common/variables/types.ts | 11 +- .../process/processFactory.unit.test.ts | 4 +- .../kernelEnvVarsService.unit.test.ts | 28 +- .../kernels/jupyterKernelService.unit.test.ts | 2 +- .../localKernelFinder.unit.test.ts | 2 +- .../notebook/executionService.vscode.test.ts | 21 +- .../datascience/preWarmVariables.unit.test.ts | 2 +- .../raw/finder/jupyterPaths.node.unit.test.ts | 2 +- ...ronmentVariablesProvider.node.unit.test.ts | 294 ++++++++++++++++++ 15 files changed, 474 insertions(+), 85 deletions(-) create mode 100644 src/test/platform/interpreter/common/variables/customEnvironmentVariablesProvider.node.unit.test.ts diff --git a/src/kernels/raw/finder/jupyterPaths.node.ts b/src/kernels/raw/finder/jupyterPaths.node.ts index d350ddb5284..86239ed821f 100644 --- a/src/kernels/raw/finder/jupyterPaths.node.ts +++ b/src/kernels/raw/finder/jupyterPaths.node.ts @@ -349,7 +349,7 @@ export class JupyterPaths { subDir?: string ): Promise { const paths = new ResourceSet(); - const vars = await this.envVarsProvider.getEnvironmentVariables(); + const vars = await this.envVarsProvider.getEnvironmentVariables(undefined, 'RunPythonCode'); if (cancelToken?.isCancellationRequested) { return []; } diff --git a/src/kernels/raw/launcher/kernelEnvVarsService.node.ts b/src/kernels/raw/launcher/kernelEnvVarsService.node.ts index ae7789134ad..17243adfb42 100644 --- a/src/kernels/raw/launcher/kernelEnvVarsService.node.ts +++ b/src/kernels/raw/launcher/kernelEnvVarsService.node.ts @@ -16,6 +16,7 @@ import { IInterpreterService } from '../../../platform/interpreter/contracts'; import { PythonEnvironment } from '../../../platform/pythonEnvironments/info'; import { IJupyterKernelSpec } from '../../types'; import { Uri } from 'vscode'; +import { PYTHON_LANGUAGE } from '../../../platform/common/constants'; /** * Class used to fetch environment variables for a kernel. @@ -49,7 +50,7 @@ export class KernelEnvironmentVariablesService { kernelSpec: IJupyterKernelSpec ) { let kernelEnv = kernelSpec.env && Object.keys(kernelSpec.env).length > 0 ? kernelSpec.env : undefined; - + const isPythonKernel = (kernelSpec.language || '').toLowerCase() === PYTHON_LANGUAGE; // If an interpreter was not explicitly passed in, check for an interpreter path in the kernelspec to use if (!interpreter && kernelSpec.interpreterPath) { interpreter = await this.interpreterService @@ -61,7 +62,9 @@ export class KernelEnvironmentVariablesService { } let [customEnvVars, interpreterEnv] = await Promise.all([ - this.customEnvVars.getCustomEnvironmentVariables(resource).catch(noop), + this.customEnvVars + .getCustomEnvironmentVariables(resource, isPythonKernel ? 'RunPythonCode' : 'RunNonPythonCode') + .catch(noop), interpreter ? this.envActivation .getActivatedEnvironmentVariables(resource, interpreter, false) diff --git a/src/kernels/variables/preWarmVariables.node.ts b/src/kernels/variables/preWarmVariables.node.ts index 2aa7c571a53..1edc4a270b0 100644 --- a/src/kernels/variables/preWarmVariables.node.ts +++ b/src/kernels/variables/preWarmVariables.node.ts @@ -47,9 +47,11 @@ export class PreWarmActivatedJupyterEnvironmentVariables implements IExtensionSi // Don't try to pre-warm variables if user has too many workspace folders opened. const workspaceFolderCount = this.workspace.workspaceFolders?.length ?? 0; if (workspaceFolderCount <= 5) { - this.envVarsProvider.getEnvironmentVariables(undefined).ignoreErrors(); + this.envVarsProvider.getEnvironmentVariables(undefined, 'RunNonPythonCode').ignoreErrors(); + this.envVarsProvider.getEnvironmentVariables(undefined, 'RunPythonCode').ignoreErrors(); (this.workspace.workspaceFolders || []).forEach((folder) => { - this.envVarsProvider.getEnvironmentVariables(folder.uri).ignoreErrors(); + this.envVarsProvider.getEnvironmentVariables(folder.uri, 'RunNonPythonCode').ignoreErrors(); + this.envVarsProvider.getEnvironmentVariables(folder.uri, 'RunPythonCode').ignoreErrors(); }); } this.condaService.getCondaFile().ignoreErrors(); diff --git a/src/platform/common/process/environmentActivationService.node.ts b/src/platform/common/process/environmentActivationService.node.ts index 8bad2cb5225..489e84dc475 100644 --- a/src/platform/common/process/environmentActivationService.node.ts +++ b/src/platform/common/process/environmentActivationService.node.ts @@ -130,19 +130,9 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi ) { EnvironmentActivationService.minTimeAfterWhichWeShouldCacheEnvVariables = minTimeAfterWhichWeShouldCacheEnvVariables; - this.envVarsService.onDidEnvironmentVariablesChange( - () => this.activatedEnvVariablesCache.clear(), - this, - this.disposables - ); - - this.interpreterService.onDidChangeInterpreter( - () => this.activatedEnvVariablesCache.clear(), - this, - this.disposables - ); + this.envVarsService.onDidEnvironmentVariablesChange(this.clearCache, this, this.disposables); + this.interpreterService.onDidChangeInterpreter(this.clearCache, this, this.disposables); } - @testOnlyMethod() public clearCache() { this.activatedEnvVariablesCache.clear(); } @@ -195,6 +185,11 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi envVariablesFromPython.completed && !envVariablesFromPython.value ) { + traceWarning( + `Failed to get env vars from Python extension. Falling back to ours for ${getDisplayPath( + interpreter.uri + )}.` + ); await envVariablesOurSelves.promise; } @@ -206,8 +201,7 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi } else { traceVerbose(`Got env vars ourselves faster, but empty ${getDisplayPath(interpreter?.uri)}`); } - } - if (!envVariablesOurSelves.resolved) { + } else { traceVerbose(`Got env vars with python ext faster ${getDisplayPath(interpreter?.uri)}`); } return envVariablesFromPython.promise; @@ -222,7 +216,7 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi ): Promise { const stopWatch = new StopWatch(); // We'll need this later. - this.envVarsService.getEnvironmentVariables(resource).catch(noop); + this.envVarsService.getEnvironmentVariables(resource, 'RunPythonCode').catch(noop); // Check cache. let reasonForFailure: @@ -244,7 +238,7 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi return undefined; }) ), - this.envVarsService.getCustomEnvironmentVariables(resource).catch((ex) => { + this.envVarsService.getCustomEnvironmentVariables(resource, 'RunPythonCode').catch((ex) => { traceError( `Failed to get activated env variables from Python Extension for ${getDisplayPath( interpreter.uri @@ -301,6 +295,11 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi const workspaceKey = this.workspace.getWorkspaceFolderIdentifier(resource); const key = `${workspaceKey}_${interpreter && getInterpreterHash(interpreter)}`; + if (this.activatedEnvVariablesCache.has(key)) { + traceVerbose(`Got activation Env Vars from cached promise with key ${key}`); + return this.activatedEnvVariablesCache.get(key); + } + const shellInfo = defaultShells[this.platform.osType]; const envType = interpreter?.envType; if (!shellInfo) { @@ -319,11 +318,6 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi return; } - if (this.activatedEnvVariablesCache.has(key)) { - traceVerbose(`Got activation Env Vars from cached promise with key ${key}`); - return this.activatedEnvVariablesCache.get(key); - } - const promise = (async () => { const condaActivation = async () => { const stopWatch = new StopWatch(); @@ -378,7 +372,7 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi const [activationCommands, customEnvVars] = await Promise.all([ this.getActivationCommands(resource, interpreterDetails || interpreter), - this.envVarsService.getEnvironmentVariables(resource) + this.envVarsService.getEnvironmentVariables(resource, 'RunPythonCode') ]); const processService = await processServicePromise; const hasCustomEnvVars = Object.keys(customEnvVars).length; @@ -573,7 +567,7 @@ export class EnvironmentActivationService implements IEnvironmentActivationServi this.condaService.getCondaFile(), this.condaService.getCondaVersion(), this.fs.createTemporaryLocalFile('.json'), - this.envVarsService.getEnvironmentVariables(resource) + this.envVarsService.getEnvironmentVariables(resource, 'RunPythonCode') ]); const hasCustomEnvVars = Object.keys(customEnvVars).length; const env = hasCustomEnvVars diff --git a/src/platform/common/process/processFactory.node.ts b/src/platform/common/process/processFactory.node.ts index 8b8e73839d4..99ac60203e9 100644 --- a/src/platform/common/process/processFactory.node.ts +++ b/src/platform/common/process/processFactory.node.ts @@ -29,7 +29,7 @@ export class ProcessServiceFactory implements IProcessServiceFactory { if (!this.workspace.isTrusted) { throw new Error('Workspace not trusted'); } - const customEnvVars = await this.envVarsService.getEnvironmentVariables(resource); + const customEnvVars = await this.envVarsService.getEnvironmentVariables(resource, 'RunNonPythonCode'); const proc: IProcessService = new ProcessService(this.decoder, customEnvVars); this.disposableRegistry.push(proc); return proc.on('exec', this.processLogger.logProcess.bind(this.processLogger)); diff --git a/src/platform/common/variables/customEnvironmentVariablesProvider.node.ts b/src/platform/common/variables/customEnvironmentVariablesProvider.node.ts index ab29b6344a3..608cee2142f 100644 --- a/src/platform/common/variables/customEnvironmentVariablesProvider.node.ts +++ b/src/platform/common/variables/customEnvironmentVariablesProvider.node.ts @@ -2,14 +2,17 @@ // Licensed under the MIT License. import { inject, injectable, optional } from 'inversify'; import * as path from '../../vscode-path/path'; -import { ConfigurationChangeEvent, Disposable, Event, EventEmitter, FileSystemWatcher, Uri } from 'vscode'; +import { ConfigurationChangeEvent, Disposable, Event, EventEmitter, RelativePattern, Uri } from 'vscode'; import { TraceOptions } from '../../logging/types'; import { sendFileCreationTelemetry } from '../../telemetry/envFileTelemetry.node'; import { IWorkspaceService } from '../application/types'; -import { IDisposableRegistry } from '../types'; +import { IDisposableRegistry, Resource } from '../types'; import { InMemoryCache } from '../utils/cacheUtils'; import { EnvironmentVariables, ICustomEnvironmentVariablesProvider, IEnvironmentVariablesService } from './types'; -import { traceDecoratorVerbose, traceInfoIfCI, traceVerbose } from '../../logging'; +import { traceDecoratorVerbose, traceError, traceInfoIfCI, traceVerbose } from '../../logging'; +import { SystemVariables } from './systemVariables.node'; +import { disposeAllDisposables } from '../helpers'; +import { IPythonExtensionChecker } from '../../api/types'; const CACHE_DURATION = 60 * 60 * 1000; @injectable() @@ -18,58 +21,76 @@ export class CustomEnvironmentVariablesProvider implements ICustomEnvironmentVar return this.changeEventEmitter.event; } public trackedWorkspaceFolders = new Set(); - private fileWatchers = new Map(); + private fileWatchers = new Set(); private disposables: Disposable[] = []; - private changeEventEmitter: EventEmitter; + private changeEventEmitter = new EventEmitter(); constructor( @inject(IEnvironmentVariablesService) private envVarsService: IEnvironmentVariablesService, @inject(IDisposableRegistry) disposableRegistry: Disposable[], @inject(IWorkspaceService) private workspaceService: IWorkspaceService, + @inject(IPythonExtensionChecker) private readonly extensionChecker: IPythonExtensionChecker, @inject('number') @optional() private cacheDuration: number = CACHE_DURATION ) { disposableRegistry.push(this); - this.changeEventEmitter = new EventEmitter(); - const disposable = this.workspaceService.onDidChangeConfiguration(this.configurationChanged, this); - this.disposables.push(disposable); + this.workspaceService.onDidChangeConfiguration(this.configurationChanged, this, this.disposables); } public dispose() { this.changeEventEmitter.dispose(); - this.fileWatchers.forEach((watcher) => { - if (watcher) { - watcher.dispose(); - } - }); + disposeAllDisposables(this.disposables); } @traceDecoratorVerbose('Get Custom Env Variables', TraceOptions.BeforeCall | TraceOptions.Arguments) - public async getEnvironmentVariables(resource?: Uri): Promise { + public async getEnvironmentVariables( + resource: Resource, + purpose: 'RunPythonCode' | 'RunNonPythonCode' + ): Promise { // Cache resource specific interpreter data - const key = this.workspaceService.getWorkspaceFolderIdentifier(resource); - const cacheStoreIndexedByWorkspaceFolder = new InMemoryCache(this.cacheDuration, key); + const cacheStoreIndexedByWorkspaceFolder = new InMemoryCache( + this.cacheDuration, + this.getCacheKeyForMergedVars(resource, purpose) + ); if (cacheStoreIndexedByWorkspaceFolder.hasData && cacheStoreIndexedByWorkspaceFolder.data) { traceVerbose(`Cached data exists getEnvironmentVariables, ${resource ? resource.fsPath : ''}`); return cacheStoreIndexedByWorkspaceFolder.data!; } - const promise = this._getEnvironmentVariables(resource); + const promise = this._getEnvironmentVariables(resource, purpose); promise.then((result) => (cacheStoreIndexedByWorkspaceFolder.data = result)).ignoreErrors(); return promise; } - public async getCustomEnvironmentVariables(resource?: Uri): Promise { + public async getCustomEnvironmentVariables( + resource: Resource, + purpose: 'RunPythonCode' | 'RunNonPythonCode' + ): Promise { + // Cache resource specific interpreter data + const cacheStoreIndexedByWorkspaceFolder = new InMemoryCache( + this.cacheDuration, + this.getCacheKeyForCustomVars(resource, purpose) + ); + + if (cacheStoreIndexedByWorkspaceFolder.hasData) { + traceVerbose( + `Cached custom vars data exists getCustomEnvironmentVariables, ${ + resource ? resource.fsPath : '' + }` + ); + return cacheStoreIndexedByWorkspaceFolder.data!; + } + const workspaceFolderUri = this.getWorkspaceFolderUri(resource); if (!workspaceFolderUri) { traceInfoIfCI(`No workspace folder found for ${resource ? resource.fsPath : ''}`); return; } - this.trackedWorkspaceFolders.add(workspaceFolderUri ? workspaceFolderUri.fsPath : ''); + this.trackedWorkspaceFolders.add(workspaceFolderUri.fsPath || ''); - // eslint-disable-next-line - // TODO: This should be added to the python API (or this entire service should move there) - // https://github.com/microsoft/vscode-jupyter/issues/51 - const envFile = path.join(workspaceFolderUri.fsPath, '.env'); + const envFile = this.getEnvFile(workspaceFolderUri, purpose); this.createFileWatcher(envFile, workspaceFolderUri); - return this.envVarsService.parseFile(envFile, process.env); + + const promise = this.envVarsService.parseFile(envFile, process.env); + promise.then((result) => (cacheStoreIndexedByWorkspaceFolder.data = result)).ignoreErrors(); + return promise; } public configurationChanged(e: ConfigurationChangeEvent) { this.trackedWorkspaceFolders.forEach((item) => { @@ -80,19 +101,43 @@ export class CustomEnvironmentVariablesProvider implements ICustomEnvironmentVar }); } public createFileWatcher(envFile: string, workspaceFolderUri?: Uri) { - if (this.fileWatchers.has(envFile)) { + const key = this.getCacheKeyForMergedVars(workspaceFolderUri, 'RunNonPythonCode'); + if (this.fileWatchers.has(key)) { return; } - const envFileWatcher = this.workspaceService.createFileSystemWatcher(envFile); - this.fileWatchers.set(envFile, envFileWatcher); + const pattern = new RelativePattern(Uri.file(path.dirname(envFile)), path.basename(envFile)); + const envFileWatcher = this.workspaceService.createFileSystemWatcher(pattern, false, false, false); if (envFileWatcher) { - this.disposables.push(envFileWatcher.onDidChange(() => this.onEnvironmentFileChanged(workspaceFolderUri))); - this.disposables.push(envFileWatcher.onDidCreate(() => this.onEnvironmentFileCreated(workspaceFolderUri))); - this.disposables.push(envFileWatcher.onDidDelete(() => this.onEnvironmentFileChanged(workspaceFolderUri))); + this.disposables.push(envFileWatcher); + this.fileWatchers.add(key); + envFileWatcher.onDidChange(() => this.onEnvironmentFileChanged(workspaceFolderUri), this, this.disposables); + envFileWatcher.onDidCreate(() => this.onEnvironmentFileCreated(workspaceFolderUri), this, this.disposables); + envFileWatcher.onDidDelete(() => this.onEnvironmentFileChanged(workspaceFolderUri), this, this.disposables); + } else { + traceError('Failed to create file watcher for environment file'); } } - private async _getEnvironmentVariables(resource?: Uri): Promise { - let customEnvVars = await this.getCustomEnvironmentVariables(resource); + private getEnvFile(workspaceFolderUri: Uri, purpose: 'RunPythonCode' | 'RunNonPythonCode') { + if (purpose === 'RunPythonCode') { + return this.getPythonEnvFile(workspaceFolderUri) || path.join(workspaceFolderUri.fsPath, '.env'); + } else { + return path.join(workspaceFolderUri.fsPath, '.env'); + } + } + private getPythonEnvFile(resource?: Uri): string | undefined { + if (!this.extensionChecker.isPythonExtensionInstalled) { + return; + } + const workspaceFolderUri = this.getWorkspaceFolderUri(resource); + const sysVars = new SystemVariables(resource, workspaceFolderUri, this.workspaceService); + const pythonEnvSetting = this.workspaceService.getConfiguration('python', resource).get('envFile'); + return pythonEnvSetting ? sysVars.resolve(pythonEnvSetting) : undefined; + } + private async _getEnvironmentVariables( + resource: Resource, + purpose: 'RunPythonCode' | 'RunNonPythonCode' + ): Promise { + let customEnvVars = await this.getCustomEnvironmentVariables(resource, purpose); if (!customEnvVars) { customEnvVars = {}; } @@ -127,10 +172,31 @@ export class CustomEnvironmentVariablesProvider implements ICustomEnvironmentVar sendFileCreationTelemetry(); } - private onEnvironmentFileChanged(workspaceFolderUri?: Uri) { - const key = this.workspaceService.getWorkspaceFolderIdentifier(workspaceFolderUri); - new InMemoryCache(this.cacheDuration, `$getEnvironmentVariables-${key}`).clear(); + private onEnvironmentFileChanged(workspaceFolderUri: Resource) { + new InMemoryCache( + this.cacheDuration, + this.getCacheKeyForMergedVars(workspaceFolderUri, 'RunNonPythonCode') + ).clear(); + new InMemoryCache( + this.cacheDuration, + this.getCacheKeyForMergedVars(workspaceFolderUri, 'RunPythonCode') + ).clear(); + + new InMemoryCache( + this.cacheDuration, + this.getCacheKeyForCustomVars(workspaceFolderUri, 'RunNonPythonCode') + ).clear(); + new InMemoryCache( + this.cacheDuration, + this.getCacheKeyForCustomVars(workspaceFolderUri, 'RunPythonCode') + ).clear(); this.changeEventEmitter.fire(workspaceFolderUri); } + private getCacheKeyForMergedVars(workspaceFolderUri: Resource, purpose: 'RunPythonCode' | 'RunNonPythonCode') { + return `${this.workspaceService.getWorkspaceFolderIdentifier(workspaceFolderUri)}:${purpose || ''}`; + } + private getCacheKeyForCustomVars(workspaceFolderUri: Resource, purpose: 'RunPythonCode' | 'RunNonPythonCode') { + return `${this.workspaceService.getWorkspaceFolderIdentifier(workspaceFolderUri)}:${purpose || ''}:CustomVars`; + } } diff --git a/src/platform/common/variables/types.ts b/src/platform/common/variables/types.ts index 1900af27995..8ba6f065f94 100644 --- a/src/platform/common/variables/types.ts +++ b/src/platform/common/variables/types.ts @@ -3,6 +3,7 @@ import { Event, Uri } from 'vscode'; import { ClassType } from '../../ioc/types'; +import { Resource } from '../types'; export type EnvironmentVariables = Object & Record; @@ -59,9 +60,15 @@ export interface ICustomEnvironmentVariablesProvider { /** * Gets merged result of process.env and env variables defined in the .env file. */ - getEnvironmentVariables(resource?: Uri): Promise; + getEnvironmentVariables( + resource: Resource, + purpose: 'RunPythonCode' | 'RunNonPythonCode' + ): Promise; /** * Gets the env variables defined in the .env file. */ - getCustomEnvironmentVariables(resource?: Uri): Promise; + getCustomEnvironmentVariables( + resource: Resource, + purpose: 'RunPythonCode' | 'RunNonPythonCode' + ): Promise; } diff --git a/src/test/common/process/processFactory.unit.test.ts b/src/test/common/process/processFactory.unit.test.ts index 2648bed54de..127a437754c 100644 --- a/src/test/common/process/processFactory.unit.test.ts +++ b/src/test/common/process/processFactory.unit.test.ts @@ -52,10 +52,10 @@ suite('Process - ProcessServiceFactory', () => { [Uri.parse('test'), undefined].forEach((resource) => { test(`Ensure ProcessService is created with an ${resource ? 'existing' : 'undefined'} resource`, async () => { - when(envVariablesProvider.getEnvironmentVariables(resource)).thenResolve({ x: 'test' }); + when(envVariablesProvider.getEnvironmentVariables(resource, 'RunNonPythonCode')).thenResolve({ x: 'test' }); const proc = await factory.create(resource); - verify(envVariablesProvider.getEnvironmentVariables(resource)).once(); + verify(envVariablesProvider.getEnvironmentVariables(resource, 'RunNonPythonCode')).once(); const disposables = disposableRegistry as Disposable[]; expect(disposables.length).equal(1); diff --git a/src/test/common/variables/kernelEnvVarsService.unit.test.ts b/src/test/common/variables/kernelEnvVarsService.unit.test.ts index 8df33e960e4..f47c5c03137 100644 --- a/src/test/common/variables/kernelEnvVarsService.unit.test.ts +++ b/src/test/common/variables/kernelEnvVarsService.unit.test.ts @@ -71,7 +71,7 @@ suite('Kernel Environment Variables Service', () => { when(envActivation.getActivatedEnvironmentVariables(anything(), anything(), anything())).thenResolve({ PATH: 'foobar' }); - when(customVariablesService.getCustomEnvironmentVariables(anything())).thenResolve(); + when(customVariablesService.getCustomEnvironmentVariables(anything(), anything())).thenResolve(); const vars = await kernelVariablesService.getEnvironmentVariables(undefined, interpreter, kernelSpec); @@ -83,7 +83,7 @@ suite('Kernel Environment Variables Service', () => { when(envActivation.getActivatedEnvironmentVariables(anything(), anything(), anything())).thenResolve({ HELLO_VAR: 'new' }); - when(customVariablesService.getCustomEnvironmentVariables(anything())).thenResolve(); + when(customVariablesService.getCustomEnvironmentVariables(anything(), anything())).thenResolve(); const vars = await kernelVariablesService.getEnvironmentVariables(undefined, interpreter, kernelSpec); @@ -100,7 +100,9 @@ suite('Kernel Environment Variables Service', () => { when(envActivation.getActivatedEnvironmentVariables(anything(), anything(), anything())).thenResolve({ HELLO_VAR: 'interpreter' }); - when(customVariablesService.getCustomEnvironmentVariables(anything())).thenResolve({ HELLO_VAR: 'new' }); + when(customVariablesService.getCustomEnvironmentVariables(anything(), anything())).thenResolve({ + HELLO_VAR: 'new' + }); const vars = await kernelVariablesService.getEnvironmentVariables(undefined, interpreter, kernelSpec); @@ -116,7 +118,9 @@ suite('Kernel Environment Variables Service', () => { process.env['HELLO_VAR'] = 'very old'; delete kernelSpec.interpreterPath; when(envActivation.getActivatedEnvironmentVariables(anything(), anything(), anything())).thenResolve({}); - when(customVariablesService.getCustomEnvironmentVariables(anything())).thenResolve({ HELLO_VAR: 'new' }); + when(customVariablesService.getCustomEnvironmentVariables(anything(), anything())).thenResolve({ + HELLO_VAR: 'new' + }); const vars = await kernelVariablesService.getEnvironmentVariables(undefined, undefined, kernelSpec); @@ -130,7 +134,7 @@ suite('Kernel Environment Variables Service', () => { test('Returns process.env vars if no interpreter and no kernelspec.env', async () => { delete kernelSpec.interpreterPath; - when(customVariablesService.getCustomEnvironmentVariables(anything())).thenResolve(); + when(customVariablesService.getCustomEnvironmentVariables(anything(), anything())).thenResolve(); const vars = await kernelVariablesService.getEnvironmentVariables(undefined, undefined, kernelSpec); @@ -139,7 +143,7 @@ suite('Kernel Environment Variables Service', () => { test('Returns process.env vars if unable to get activated vars for interpreter and no kernelspec.env', async () => { when(envActivation.getActivatedEnvironmentVariables(anything(), anything(), anything())).thenResolve(); - when(customVariablesService.getCustomEnvironmentVariables(anything())).thenResolve(); + when(customVariablesService.getCustomEnvironmentVariables(anything(), anything())).thenResolve(); const vars = await kernelVariablesService.getEnvironmentVariables(undefined, interpreter, kernelSpec); @@ -158,7 +162,9 @@ suite('Kernel Environment Variables Service', () => { when(envActivation.getActivatedEnvironmentVariables(anything(), anything(), anything())).thenResolve({ PATH: 'foobar' }); - when(customVariablesService.getCustomEnvironmentVariables(anything())).thenResolve({ PATH: 'foobaz' }); + when(customVariablesService.getCustomEnvironmentVariables(anything(), anything())).thenResolve({ + PATH: 'foobaz' + }); const vars = await kernelVariablesService.getEnvironmentVariables(undefined, interpreter, kernelSpec); assert.isOk(processPath); @@ -177,7 +183,9 @@ suite('Kernel Environment Variables Service', () => { when(envActivation.getActivatedEnvironmentVariables(anything(), anything(), anything())).thenResolve({ PATH: 'pathInInterpreterEnv' }); - when(customVariablesService.getCustomEnvironmentVariables(anything())).thenResolve({ PATH: 'foobaz' }); + when(customVariablesService.getCustomEnvironmentVariables(anything(), anything())).thenResolve({ + PATH: 'foobaz' + }); // undefined for interpreter here, interpreterPath from the spec should be used const vars = await kernelVariablesService.getEnvironmentVariables(undefined, undefined, kernelSpec); @@ -199,7 +207,9 @@ suite('Kernel Environment Variables Service', () => { when(envActivation.getActivatedEnvironmentVariables(anything(), anything(), anything())).thenResolve({ PATH: 'foobar' }); - when(customVariablesService.getCustomEnvironmentVariables(anything())).thenResolve({ PATH: 'foobaz' }); + when(customVariablesService.getCustomEnvironmentVariables(anything(), anything())).thenResolve({ + PATH: 'foobaz' + }); when(settings.excludeUserSitePackages).thenReturn(shouldBeSet); // undefined for interpreter here, interpreterPath from the spec should be used diff --git a/src/test/datascience/jupyter/kernels/jupyterKernelService.unit.test.ts b/src/test/datascience/jupyter/kernels/jupyterKernelService.unit.test.ts index e8eb6312aa1..fa7ee077cd0 100644 --- a/src/test/datascience/jupyter/kernels/jupyterKernelService.unit.test.ts +++ b/src/test/datascience/jupyter/kernels/jupyterKernelService.unit.test.ts @@ -392,7 +392,7 @@ suite('DataScience - JupyterKernelService', () => { when(appEnv.getActivatedEnvironmentVariables(anything(), anything(), anything())).thenResolve({}); const variablesService = new EnvironmentVariablesService(instance(fs)); const customEnvVars = mock(); - when(customEnvVars.getCustomEnvironmentVariables(anything())).thenResolve(); + when(customEnvVars.getCustomEnvironmentVariables(anything(), anything())).thenResolve(); settings = mock(JupyterSettings); const configService = mock(ConfigurationService); settings = mock(JupyterSettings); diff --git a/src/test/datascience/kernel-launcher/localKernelFinder.unit.test.ts b/src/test/datascience/kernel-launcher/localKernelFinder.unit.test.ts index fdf48ade869..ab7b030bbc6 100644 --- a/src/test/datascience/kernel-launcher/localKernelFinder.unit.test.ts +++ b/src/test/datascience/kernel-launcher/localKernelFinder.unit.test.ts @@ -144,7 +144,7 @@ import { IApplicationEnvironment } from '../../../platform/common/application/ty }); when(workspaceService.rootFolder).thenReturn(testWorkspaceFolder); const envVarsProvider = mock(CustomEnvironmentVariablesProvider); - when(envVarsProvider.getEnvironmentVariables()).thenResolve({}); + when(envVarsProvider.getEnvironmentVariables(anything(), anything())).thenResolve({}); const event = new EventEmitter(); disposables.push(event); when(envVarsProvider.onDidEnvironmentVariablesChange).thenReturn(event.event); diff --git a/src/test/datascience/notebook/executionService.vscode.test.ts b/src/test/datascience/notebook/executionService.vscode.test.ts index f2f59bb8767..88d3513c7ef 100644 --- a/src/test/datascience/notebook/executionService.vscode.test.ts +++ b/src/test/datascience/notebook/executionService.vscode.test.ts @@ -16,7 +16,8 @@ import { NotebookCellKind, NotebookCellOutput, Uri, - window + window, + workspace } from 'vscode'; import { Common } from '../../../platform/common/utils/localize'; import { IVSCodeNotebook } from '../../../platform/common/application/types'; @@ -52,7 +53,7 @@ import { createTemporaryNotebookFromFile } from './helper.node'; import { openNotebook } from '../helpers.node'; -import { noop, swallowExceptions } from '../../../platform/common/utils/misc'; +import { isWeb, noop, swallowExceptions } from '../../../platform/common/utils/misc'; import { getDisplayPath } from '../../../platform/common/platform/fs-paths'; import { ProductNames } from '../../../kernels/installer/productNames'; import { Product } from '../../../kernels/installer/types'; @@ -75,7 +76,7 @@ suite('DataScience - VSCode Notebook - (Execution) (slow)', function () { const templateNbPath = Uri.file( path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'test', 'datascience', 'notebook', 'emptyCellWithOutput.ipynb') ); - + const envFile = Uri.joinPath(Uri.file(EXTENSION_ROOT_DIR_FOR_TESTS), 'src', 'test', 'datascience', '.env'); this.timeout(120_000); suiteSetup(async function () { traceInfo('Suite Setup VS Code Notebook - Execution'); @@ -88,7 +89,11 @@ suite('DataScience - VSCode Notebook - (Execution) (slow)', function () { { result: Common.install(), clickImmediately: true }, disposables ); - + if (!IS_REMOTE_NATIVE_TEST() && !isWeb()) { + await workspace + .getConfiguration('python', workspace.workspaceFolders![0].uri) + .update('envFile', '${workspaceFolder}/.env'); + } await startJupyterServer(); await prewarmNotebooks(); sinon.restore(); @@ -198,6 +203,14 @@ suite('DataScience - VSCode Notebook - (Execution) (slow)', function () { if (IS_REMOTE_NATIVE_TEST()) { return this.skip(); } + await fs.writeFileSync( + envFile.fsPath, + dedent` + ENV_VAR_TESTING_CI=HelloWorldEnvVariable + PYTHONPATH=./dummyFolderForPythonPath + ` + ); + const cell = await insertCodeCell( dedent` import sys diff --git a/src/test/datascience/preWarmVariables.unit.test.ts b/src/test/datascience/preWarmVariables.unit.test.ts index 0b03cf7ecf3..e306e4cc4e0 100644 --- a/src/test/datascience/preWarmVariables.unit.test.ts +++ b/src/test/datascience/preWarmVariables.unit.test.ts @@ -44,7 +44,7 @@ suite('DataScience - PreWarm Env Vars', () => { when(extensionChecker.isPythonExtensionActive).thenReturn(true); zmqSupported = mock(); const envVarsProvider = mock(); - when(envVarsProvider.getEnvironmentVariables(anything())).thenResolve(); + when(envVarsProvider.getEnvironmentVariables(anything(), anything())).thenResolve(); const workspace = mock(); when(workspace.workspaceFolders).thenReturn(); when(zmqSupported.isSupported).thenReturn(false); diff --git a/src/test/kernels/raw/finder/jupyterPaths.node.unit.test.ts b/src/test/kernels/raw/finder/jupyterPaths.node.unit.test.ts index 053594e6830..a9313ca741f 100644 --- a/src/test/kernels/raw/finder/jupyterPaths.node.unit.test.ts +++ b/src/test/kernels/raw/finder/jupyterPaths.node.unit.test.ts @@ -58,7 +58,7 @@ suite('Jupyter Paths', () => { fs = mock(); context = mock(); when(context.extensionUri).thenReturn(extensionUri); - when(envVarsProvider.getEnvironmentVariables()).thenResolve(process.env); + when(envVarsProvider.getEnvironmentVariables(anything(), anything())).thenResolve(process.env); jupyterPaths = new JupyterPaths( instance(platformService), instance(envVarsProvider), diff --git a/src/test/platform/interpreter/common/variables/customEnvironmentVariablesProvider.node.unit.test.ts b/src/test/platform/interpreter/common/variables/customEnvironmentVariablesProvider.node.unit.test.ts new file mode 100644 index 00000000000..994c4027fd1 --- /dev/null +++ b/src/test/platform/interpreter/common/variables/customEnvironmentVariablesProvider.node.unit.test.ts @@ -0,0 +1,294 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +'use strict'; +import { assert } from 'chai'; +import { ConfigurationChangeEvent, EventEmitter, FileSystemWatcher, Uri, WorkspaceConfiguration } from 'vscode'; +import { IWorkspaceService } from '../../../../../platform/common/application/types'; +import { disposeAllDisposables } from '../../../../../platform/common/helpers'; +import { IDisposable, IExtensionContext, IHttpClient } from '../../../../../platform/common/types'; +import { CustomEnvironmentVariablesProvider } from '../../../../../platform/common/variables/customEnvironmentVariablesProvider.node'; +import { IEnvironmentVariablesService } from '../../../../../platform/common/variables/types'; +import * as fs from 'fs-extra'; +import { EXTENSION_ROOT_DIR_FOR_TESTS } from '../../../../constants.node'; +import * as dedent from 'dedent'; +import { IPythonExtensionChecker } from '../../../../../platform/api/types'; +import { captureScreenShot, createEventHandler } from '../../../../common'; +import { traceInfo } from '../../../../../platform/logging'; +import { anything, instance, mock, when } from 'ts-mockito'; +import { clearCache } from '../../../../../platform/common/utils/cacheUtils'; +import { EnvironmentVariablesService } from '../../../../../platform/common/variables/environment.node'; +import { FileSystem } from '../../../../../platform/common/platform/fileSystem.node'; +import * as sinon from 'sinon'; +// import * as path from '../../../../../platform/vscode-path/path'; + +suite('Custom Environment Variables Provider', () => { + let customEnvVarsProvider: CustomEnvironmentVariablesProvider; + let envVarsService: IEnvironmentVariablesService; + const disposables: IDisposable[] = []; + let workspace: IWorkspaceService; + let pythonExtChecker: IPythonExtensionChecker; + const envFile = Uri.joinPath(Uri.file(EXTENSION_ROOT_DIR_FOR_TESTS), 'src', 'test', 'datascience', '.env'); + let contentsOfOldEnvFile: string; + let onDidChangeConfiguration: EventEmitter; + let customPythonEnvFile = Uri.joinPath( + Uri.file(EXTENSION_ROOT_DIR_FOR_TESTS), + 'src', + 'test', + 'datascience', + '.env.python' + ); + let onFSEvent: EventEmitter; + let fsWatcher: FileSystemWatcher; + const workspaceUri = Uri.joinPath(Uri.file(EXTENSION_ROOT_DIR_FOR_TESTS), 'src', 'test', 'datascience'); + const workspaceFolder = { index: 0, name: 'workspace', uri: workspaceUri }; + setup(async function () { + traceInfo(`Start Test ${this.currentTest?.title}`); + clearCache(); + envVarsService = new EnvironmentVariablesService( + new FileSystem(instance(mock()), instance(mock())) + ); + pythonExtChecker = mock(); + when(pythonExtChecker.isPythonExtensionInstalled).thenReturn(true); + contentsOfOldEnvFile = fs.readFileSync(envFile.fsPath).toString(); + onDidChangeConfiguration = new EventEmitter(); + disposables.push(onDidChangeConfiguration); + workspace = mock(); + onFSEvent = new EventEmitter(); + disposables.push(onFSEvent); + fsWatcher = mock(); + when(fsWatcher.dispose()).thenReturn(); + when(fsWatcher.onDidChange).thenReturn(onFSEvent.event); + when(fsWatcher.onDidCreate).thenReturn(onFSEvent.event); + when(fsWatcher.onDidDelete).thenReturn(onFSEvent.event); + when(workspace.onDidChangeConfiguration).thenReturn(onDidChangeConfiguration.event); + when(workspace.workspaceFolders).thenReturn([workspaceFolder]); + when(workspace.getWorkspaceFolder(anything())).thenCall(() => workspaceFolder); + when(workspace.getConfiguration(anything(), anything())).thenCall(() => { + const workspaceConfig = mock(); + when(workspaceConfig.get('envFile')).thenReturn('${workspaceFolder}/.env.python'); + return instance(workspaceConfig); + }); + when(workspace.getWorkspaceFolderIdentifier(anything())).thenCall(() => workspaceFolder.uri.fsPath); + when(workspace.createFileSystemWatcher(anything(), anything(), anything(), anything())).thenReturn( + instance(fsWatcher) + ); + }); + teardown(async function () { + traceInfo(`Ended Test ${this.currentTest?.title}`); + if (this.currentTest?.isFailed()) { + await captureScreenShot(this); + } + + disposeAllDisposables(disposables); + if (fs.existsSync(customPythonEnvFile.fsPath)) { + fs.unlinkSync(customPythonEnvFile.fsPath); + } + fs.writeFileSync(envFile.fsPath, contentsOfOldEnvFile); + sinon.restore(); + traceInfo(`Ended Test (completed) ${this.currentTest?.title}`); + }); + + function createProvider(cacheDuration?: number) { + customEnvVarsProvider = new CustomEnvironmentVariablesProvider( + envVarsService, + disposables, + instance(workspace), + pythonExtChecker, + cacheDuration + ); + } + test('Loads .env file', async () => { + const fsSpy = sinon.spy(FileSystem.prototype, 'readFile'); + const envVars = dedent` + VSCODE_JUPYTER_ENV_TEST_VAR1=FOO + VSCODE_JUPYTER_ENV_TEST_VAR2=BAR + `; + traceInfo('Write to env file', envFile.fsPath); + fs.writeFileSync(envFile.fsPath, envVars); + createProvider(); + const vars = await customEnvVarsProvider.getCustomEnvironmentVariables(undefined, 'RunNonPythonCode'); + + assert.deepEqual(vars, { + VSCODE_JUPYTER_ENV_TEST_VAR1: 'FOO', + VSCODE_JUPYTER_ENV_TEST_VAR2: 'BAR' + }); + + // Reading again doesn't require a new read of the file. + const originalCalLCount = fsSpy.callCount; + const vars2 = await customEnvVarsProvider.getCustomEnvironmentVariables(undefined, 'RunNonPythonCode'); + + assert.strictEqual(fsSpy.callCount, originalCalLCount); + assert.deepEqual(vars2, { + VSCODE_JUPYTER_ENV_TEST_VAR1: 'FOO', + VSCODE_JUPYTER_ENV_TEST_VAR2: 'BAR' + }); + }); + test('Detects changes to .env file', async () => { + let envVars = dedent` + VSCODE_JUPYTER_ENV_TEST_VAR1=FOO + VSCODE_JUPYTER_ENV_TEST_VAR2=BAR + `; + traceInfo('Write to env file1', envFile.fsPath); + fs.writeFileSync(envFile.fsPath, envVars); + createProvider(); + let vars = await customEnvVarsProvider.getCustomEnvironmentVariables(undefined, 'RunNonPythonCode'); + + assert.deepEqual(vars, { + VSCODE_JUPYTER_ENV_TEST_VAR1: 'FOO', + VSCODE_JUPYTER_ENV_TEST_VAR2: 'BAR' + }); + + // Change the .env file. + const changeDetected = createEventHandler( + customEnvVarsProvider, + 'onDidEnvironmentVariablesChange', + disposables + ); + envVars = dedent` + VSCODE_JUPYTER_ENV_TEST_VAR1=FOO2 + VSCODE_JUPYTER_ENV_TEST_VAR2=BAR2 + `; + traceInfo('Write to env file2', envFile.fsPath); + fs.writeFileSync(envFile.fsPath, envVars); + onFSEvent.fire(envFile); + + // Detect the change. + await changeDetected.assertFired(5_000); + + // Ensure the new vars are different. + vars = await customEnvVarsProvider.getCustomEnvironmentVariables(undefined, 'RunNonPythonCode'); + assert.deepEqual(vars, { + VSCODE_JUPYTER_ENV_TEST_VAR1: 'FOO2', + VSCODE_JUPYTER_ENV_TEST_VAR2: 'BAR2' + }); + }); + test('Detects creation of the .env file', async () => { + traceInfo('Delete to env file', envFile.fsPath); + fs.unlinkSync(envFile.fsPath); + createProvider(); + let vars = await customEnvVarsProvider.getCustomEnvironmentVariables(undefined, 'RunNonPythonCode'); + assert.isEmpty(vars || {}); + + // Create the .env file. + const changeDetected = createEventHandler( + customEnvVarsProvider, + 'onDidEnvironmentVariablesChange', + disposables + ); + const envVars = dedent` + VSCODE_JUPYTER_ENV_TEST_VAR1=FOO2 + VSCODE_JUPYTER_ENV_TEST_VAR2=BAR2 + `; + traceInfo('Create env file', envFile.fsPath); + fs.writeFileSync(envFile.fsPath, envVars); + onFSEvent.fire(envFile); + + // Detect the change. + await changeDetected.assertFired(5_000); + + // Ensure the new vars are different. + vars = await customEnvVarsProvider.getCustomEnvironmentVariables(undefined, 'RunNonPythonCode'); + assert.deepEqual(vars, { + VSCODE_JUPYTER_ENV_TEST_VAR1: 'FOO2', + VSCODE_JUPYTER_ENV_TEST_VAR2: 'BAR2' + }); + }); + test('Loads python.env file', async () => { + const envVars = dedent` + VSCODE_JUPYTER_ENV_TEST_VAR1=FOO + VSCODE_JUPYTER_ENV_TEST_VAR2=BAR + `; + traceInfo('Write to env file', envFile.fsPath); + fs.writeFileSync(envFile.fsPath, envVars); + const pythonEnvVars = dedent` + VSCODE_JUPYTER_ENV_TEST_VAR1=PYTHON_FOO + VSCODE_JUPYTER_ENV_TEST_VAR2=PYTHON_BAR + `; + traceInfo('Write to python env file', customPythonEnvFile.fsPath); + fs.writeFileSync(customPythonEnvFile.fsPath, pythonEnvVars); + + createProvider(); + const vars = await customEnvVarsProvider.getCustomEnvironmentVariables(undefined, 'RunNonPythonCode'); + const pythonVars = await customEnvVarsProvider.getCustomEnvironmentVariables(undefined, 'RunPythonCode'); + + assert.deepEqual(vars, { + VSCODE_JUPYTER_ENV_TEST_VAR1: 'FOO', + VSCODE_JUPYTER_ENV_TEST_VAR2: 'BAR' + }); + assert.deepEqual(pythonVars, { + VSCODE_JUPYTER_ENV_TEST_VAR1: 'PYTHON_FOO', + VSCODE_JUPYTER_ENV_TEST_VAR2: 'PYTHON_BAR' + }); + }); + test('Detects changes to python.env file', async () => { + let envVars = dedent` + VSCODE_JUPYTER_ENV_TEST_VAR1=FOO + VSCODE_JUPYTER_ENV_TEST_VAR2=BAR + `; + traceInfo('Write to env file', customPythonEnvFile.fsPath); + fs.writeFileSync(customPythonEnvFile.fsPath, envVars); + + createProvider(); + let vars = await customEnvVarsProvider.getCustomEnvironmentVariables(undefined, 'RunPythonCode'); + + assert.deepEqual(vars, { + VSCODE_JUPYTER_ENV_TEST_VAR1: 'FOO', + VSCODE_JUPYTER_ENV_TEST_VAR2: 'BAR' + }); + + // Change the .env file. + const changeDetected = createEventHandler( + customEnvVarsProvider, + 'onDidEnvironmentVariablesChange', + disposables + ); + envVars = dedent` + VSCODE_JUPYTER_ENV_TEST_VAR1=FOO2 + VSCODE_JUPYTER_ENV_TEST_VAR2=BAR2 + `; + traceInfo('Write to env file 2', customPythonEnvFile.fsPath); + fs.writeFileSync(customPythonEnvFile.fsPath, envVars); + onFSEvent.fire(customPythonEnvFile); + + // Detect the change. + await changeDetected.assertFired(5_000); + + // Ensure the new vars are different. + vars = await customEnvVarsProvider.getCustomEnvironmentVariables(undefined, 'RunPythonCode'); + assert.deepEqual(vars, { + VSCODE_JUPYTER_ENV_TEST_VAR1: 'FOO2', + VSCODE_JUPYTER_ENV_TEST_VAR2: 'BAR2' + }); + }); + test('Detects creation of the python.env file', async () => { + createProvider(); + + let vars = await customEnvVarsProvider.getCustomEnvironmentVariables(undefined, 'RunPythonCode'); + assert.isEmpty(vars || {}); + + // Create the .env file. + const changeDetected = createEventHandler( + customEnvVarsProvider, + 'onDidEnvironmentVariablesChange', + disposables + ); + const envVars = dedent` + VSCODE_JUPYTER_ENV_TEST_VAR1=FOO2 + VSCODE_JUPYTER_ENV_TEST_VAR2=BAR2 + `; + traceInfo('Write to Python env file', customPythonEnvFile.fsPath); + fs.writeFileSync(customPythonEnvFile.fsPath, envVars); + onFSEvent.fire(customPythonEnvFile); + + // Detect the change. + await changeDetected.assertFired(5_000); + + // Ensure the new vars are different. + vars = await customEnvVarsProvider.getCustomEnvironmentVariables(undefined, 'RunPythonCode'); + assert.deepEqual(vars, { + VSCODE_JUPYTER_ENV_TEST_VAR1: 'FOO2', + VSCODE_JUPYTER_ENV_TEST_VAR2: 'BAR2' + }); + }); +});