Ensure environment variables provider can be created using standard c…

…lasses (#15377) * Ensure environment variables provider can be created using standard classes * Fix unit tests
Karthik Nadig (karthiknadig) · Mar 8, 2021 · Feb 9, 2021 · Feb 9, 2021 · Feb 9, 2021 · Feb 9, 2021
commit d619de6ca23961b117ec04be81816023a20976bc
diff --git a/src/client/common/variables/environmentVariablesProvider.ts b/src/client/common/variables/environmentVariablesProvider.ts
@@ -5,10 +5,12 @@ import { inject, injectable, optional } from 'inversify';
 import { ConfigurationChangeEvent, Disposable, Event, EventEmitter, FileSystemWatcher, Uri } from 'vscode';
 import { sendFileCreationTelemetry } from '../../telemetry/envFileTelemetry';
 import { IWorkspaceService } from '../application/types';
+import { PythonSettings } from '../configSettings';
 import { traceVerbose } from '../logger';
 import { IPlatformService } from '../platform/types';
-import { IConfigurationService, ICurrentProcess, IDisposableRegistry } from '../types';
+import { ICurrentProcess, IDisposableRegistry } from '../types';
 import { InMemoryCache } from '../utils/cacheUtils';
+import { SystemVariables } from './systemVariables';
 import { EnvironmentVariables, IEnvironmentVariablesProvider, IEnvironmentVariablesService } from './types';
 
 const CACHE_DURATION = 60 * 60 * 1000;
@@ -25,7 +27,6 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid
         @inject(IDisposableRegistry) disposableRegistry: Disposable[],
         @inject(IPlatformService) private platformService: IPlatformService,
         @inject(IWorkspaceService) private workspaceService: IWorkspaceService,
-        @inject(IConfigurationService) private readonly configurationService: IConfigurationService,
         @inject(ICurrentProcess) private process: ICurrentProcess,
         @optional() private cacheDuration: number = CACHE_DURATION,
     ) {
@@ -86,11 +87,17 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid
         return mergedVars;
     }
     public async getCustomEnvironmentVariables(resource?: Uri): Promise<EnvironmentVariables | undefined> {
-        const settings = this.configurationService.getSettings(resource);
+        const systemVariables: SystemVariables = new SystemVariables(
+            undefined,
+            PythonSettings.getSettingsUriAndTarget(resource, this.workspaceService).uri?.fsPath,
+            this.workspaceService,
+        );
+        const envFileSetting = this.workspaceService.getConfiguration('python', resource).get<string>('envFile');
+        const envFile = systemVariables.resolveAny(envFileSetting)!;
         const workspaceFolderUri = this.getWorkspaceFolderUri(resource);
         this.trackedWorkspaceFolders.add(workspaceFolderUri ? workspaceFolderUri.fsPath : '');
-        this.createFileWatcher(settings.envFile, workspaceFolderUri);
-        return this.envVarsService.parseFile(settings.envFile, this.process.env);
+        this.createFileWatcher(envFile, workspaceFolderUri);
+        return this.envVarsService.parseFile(envFile, this.process.env);
     }
     public configurationChanged(e: ConfigurationChangeEvent) {
         this.trackedWorkspaceFolders.forEach((item) => {

diff --git a/src/client/extensionActivation.ts b/src/client/extensionActivation.ts
@@ -30,7 +30,6 @@ import {
     IOutputChannel,
 } from './common/types';
 import { noop } from './common/utils/misc';
-import { registerTypes as variableRegisterTypes } from './common/variables/serviceRegistry';
 import { DebuggerTypeName } from './debugger/constants';
 import { DebugSessionEventDispatcher } from './debugger/extension/hooks/eventHandlerDispatcher';
 import { IDebugSessionEventHandlers } from './debugger/extension/hooks/types';
@@ -112,7 +111,6 @@ async function activateLegacy(ext: ExtensionState): Promise<ActivationResult> {
     const { enableProposedApi } = applicationEnv.packageJson;
     serviceManager.addSingletonInstance<boolean>(UseProposedApi, enableProposedApi);
     // Feature specific registrations.
-    variableRegisterTypes(serviceManager);
     unitTestsRegisterTypes(serviceManager);
     lintersRegisterTypes(serviceManager);
     interpretersRegisterTypes(serviceManager);

diff --git a/src/client/extensionInit.ts b/src/client/extensionInit.ts
@@ -17,6 +17,7 @@ import {
     IOutputChannel,
     WORKSPACE_MEMENTO,
 } from './common/types';
+import { registerTypes as variableRegisterTypes } from './common/variables/serviceRegistry';
 import { OutputChannelNames } from './common/utils/localize';
 import { ExtensionState } from './components';
 import { ServiceContainer } from './ioc/container';
@@ -69,6 +70,7 @@ export function initializeStandard(ext: ExtensionState): void {
     const { serviceManager } = ext.legacyIOC;
     // Core registrations (non-feature specific).
     commonRegisterTypes(serviceManager);
+    variableRegisterTypes(serviceManager);
     platformRegisterTypes(serviceManager);
     processRegisterTypes(serviceManager);
 

diff --git a/src/test/common/variables/envVarsProvider.multiroot.test.ts b/src/test/common/variables/envVarsProvider.multiroot.test.ts
@@ -7,7 +7,6 @@ import * as path from 'path';
 import { anything, instance, mock, when } from 'ts-mockito';
 import { ConfigurationTarget, Disposable, Uri, workspace } from 'vscode';
 import { WorkspaceService } from '../../../client/common/application/workspace';
-import { ConfigurationService } from '../../../client/common/configuration/service';
 import { PlatformService } from '../../../client/common/platform/platformService';
 import { IFileSystem } from '../../../client/common/platform/types';
 import { IDisposableRegistry, IPathUtils } from '../../../client/common/types';
@@ -76,14 +75,12 @@ suite('Multiroot Environment Variables Provider', () => {
         const variablesService = new EnvironmentVariablesService(pathUtils, fs);
         const disposables = ioc.serviceContainer.get<Disposable[]>(IDisposableRegistry);
         ioc.serviceManager.addSingletonInstance(IInterpreterAutoSelectionService, new MockAutoSelectionService());
-        const cfgService = new ConfigurationService(ioc.serviceContainer);
         const workspaceService = new WorkspaceService();
         return new EnvironmentVariablesProvider(
             variablesService,
             disposables,
             new PlatformService(),
             workspaceService,
-            cfgService,
             mockProcess,
         );
     }

diff --git a/src/test/common/variables/environmentVariablesProvider.unit.test.ts b/src/test/common/variables/environmentVariablesProvider.unit.test.ts
@@ -11,12 +11,10 @@ import * as typemoq from 'typemoq';
 import { ConfigurationChangeEvent, FileSystemWatcher, Uri } from 'vscode';
 import { IWorkspaceService } from '../../../client/common/application/types';
 import { WorkspaceService } from '../../../client/common/application/workspace';
-import { PythonSettings } from '../../../client/common/configSettings';
-import { ConfigurationService } from '../../../client/common/configuration/service';
 import { PlatformService } from '../../../client/common/platform/platformService';
 import { IPlatformService } from '../../../client/common/platform/types';
 import { CurrentProcess } from '../../../client/common/process/currentProcess';
-import { IConfigurationService, ICurrentProcess, IPythonSettings } from '../../../client/common/types';
+import { ICurrentProcess } from '../../../client/common/types';
 import { sleep } from '../../../client/common/utils/async';
 import { EnvironmentVariablesService } from '../../../client/common/variables/environment';
 import { EnvironmentVariablesProvider } from '../../../client/common/variables/environmentVariablesProvider';
@@ -29,26 +27,27 @@ suite('Multiroot Environment Variables Provider', () => {
     let envVarsService: IEnvironmentVariablesService;
     let platform: IPlatformService;
     let workspace: IWorkspaceService;
-    let configuration: IConfigurationService;
     let currentProcess: ICurrentProcess;
-    let settings: IPythonSettings;
+    let envFile: string;
 
     setup(() => {
+        envFile = '';
         envVarsService = mock(EnvironmentVariablesService);
         platform = mock(PlatformService);
         workspace = mock(WorkspaceService);
-        configuration = mock(ConfigurationService);
         currentProcess = mock(CurrentProcess);
-        settings = mock(PythonSettings);
 
-        when(configuration.getSettings(anything())).thenReturn(instance(settings));
         when(workspace.onDidChangeConfiguration).thenReturn(noop as any);
+        when(workspace.getConfiguration('python', anything())).thenReturn({
+            get: (settingName: string) => {
+                return settingName === 'envFile' ? envFile : '';
+            },
+        } as any);
         provider = new EnvironmentVariablesProvider(
             instance(envVarsService),
             [],
             instance(platform),
             instance(workspace),
-            instance(configuration),
             instance(currentProcess),
         );
 
@@ -198,62 +197,56 @@ suite('Multiroot Environment Variables Provider', () => {
         });
 
         test(`Getting environment variables (without an envfile, without PATH in current env, without PYTHONPATH in current env) & ${workspaceTitle}`, async () => {
-            const envFile = path.join('a', 'b', 'env.file');
+            envFile = path.join('a', 'b', 'env.file');
             const workspaceFolder = workspaceUri ? { name: '', index: 0, uri: workspaceUri } : undefined;
             const currentProcEnv = { SOMETHING: 'wow' };
 
             when(currentProcess.env).thenReturn(currentProcEnv);
-            when(settings.envFile).thenReturn(envFile);
             when(workspace.getWorkspaceFolder(workspaceUri)).thenReturn(workspaceFolder);
             when(envVarsService.parseFile(envFile, currentProcEnv)).thenResolve(undefined);
             when(platform.pathVariableName).thenReturn('PATH');
 
             const vars = await provider.getEnvironmentVariables(workspaceUri);
 
             verify(currentProcess.env).atLeast(1);
-            verify(settings.envFile).atLeast(1);
             verify(envVarsService.parseFile(envFile, currentProcEnv)).atLeast(1);
             verify(envVarsService.mergeVariables(deepEqual(currentProcEnv), deepEqual({}))).once();
             verify(platform.pathVariableName).atLeast(1);
             assert.deepEqual(vars, {});
         });
         test(`Getting environment variables (with an envfile, without PATH in current env, without PYTHONPATH in current env) & ${workspaceTitle}`, async () => {
-            const envFile = path.join('a', 'b', 'env.file');
+            envFile = path.join('a', 'b', 'env.file');
             const workspaceFolder = workspaceUri ? { name: '', index: 0, uri: workspaceUri } : undefined;
             const currentProcEnv = { SOMETHING: 'wow' };
             const envFileVars = { MY_FILE: '1234' };
 
             when(currentProcess.env).thenReturn(currentProcEnv);
-            when(settings.envFile).thenReturn(envFile);
             when(workspace.getWorkspaceFolder(workspaceUri)).thenReturn(workspaceFolder);
             when(envVarsService.parseFile(envFile, currentProcEnv)).thenCall(async () => ({ ...envFileVars }));
             when(platform.pathVariableName).thenReturn('PATH');
 
             const vars = await provider.getEnvironmentVariables(workspaceUri);
 
             verify(currentProcess.env).atLeast(1);
-            verify(settings.envFile).atLeast(1);
             verify(envVarsService.parseFile(envFile, currentProcEnv)).atLeast(1);
             verify(envVarsService.mergeVariables(deepEqual(currentProcEnv), deepEqual(envFileVars))).once();
             verify(platform.pathVariableName).atLeast(1);
             assert.deepEqual(vars, envFileVars);
         });
         test(`Getting environment variables (with an envfile, with PATH in current env, with PYTHONPATH in current env) & ${workspaceTitle}`, async () => {
-            const envFile = path.join('a', 'b', 'env.file');
+            envFile = path.join('a', 'b', 'env.file');
             const workspaceFolder = workspaceUri ? { name: '', index: 0, uri: workspaceUri } : undefined;
             const currentProcEnv = { SOMETHING: 'wow', PATH: 'some path value', PYTHONPATH: 'some python path value' };
             const envFileVars = { MY_FILE: '1234' };
 
             when(currentProcess.env).thenReturn(currentProcEnv);
-            when(settings.envFile).thenReturn(envFile);
             when(workspace.getWorkspaceFolder(workspaceUri)).thenReturn(workspaceFolder);
             when(envVarsService.parseFile(envFile, currentProcEnv)).thenCall(async () => ({ ...envFileVars }));
             when(platform.pathVariableName).thenReturn('PATH');
 
             const vars = await provider.getEnvironmentVariables(workspaceUri);
 
             verify(currentProcess.env).atLeast(1);
-            verify(settings.envFile).atLeast(1);
             verify(envVarsService.parseFile(envFile, currentProcEnv)).atLeast(1);
             verify(envVarsService.mergeVariables(deepEqual(currentProcEnv), deepEqual(envFileVars))).once();
             verify(envVarsService.appendPath(deepEqual(envFileVars), currentProcEnv.PATH)).once();
@@ -263,12 +256,11 @@ suite('Multiroot Environment Variables Provider', () => {
         });
 
         test(`Getting environment variables which are already cached does not reinvoke the method ${workspaceTitle}`, async () => {
-            const envFile = path.join('a', 'b', 'env.file');
+            envFile = path.join('a', 'b', 'env.file');
             const workspaceFolder = workspaceUri ? { name: '', index: 0, uri: workspaceUri } : undefined;
             const currentProcEnv = { SOMETHING: 'wow' };
 
             when(currentProcess.env).thenReturn(currentProcEnv);
-            when(settings.envFile).thenReturn(envFile);
             when(workspace.getWorkspaceFolder(workspaceUri)).thenReturn(workspaceFolder);
             when(envVarsService.parseFile(envFile, currentProcEnv)).thenResolve(undefined);
             when(platform.pathVariableName).thenReturn('PATH');
@@ -280,7 +272,7 @@ suite('Multiroot Environment Variables Provider', () => {
             await provider.getEnvironmentVariables(workspaceUri);
 
             // Verify that the contents of `_getEnvironmentVariables()` method are only invoked once
-            verify(configuration.getSettings(anything())).once();
+            verify(workspace.getConfiguration('python', anything())).once();
             assert.deepEqual(vars, {});
         });
 
@@ -290,7 +282,6 @@ suite('Multiroot Environment Variables Provider', () => {
             const currentProcEnv = { SOMETHING: 'wow' };
 
             when(currentProcess.env).thenReturn(currentProcEnv);
-            when(settings.envFile).thenReturn(envFile);
             when(workspace.getWorkspaceFolder(workspaceUri)).thenReturn(workspaceFolder);
             when(envVarsService.parseFile(envFile, currentProcEnv)).thenResolve(undefined);
             when(platform.pathVariableName).thenReturn('PATH');
@@ -300,7 +291,6 @@ suite('Multiroot Environment Variables Provider', () => {
                 [],
                 instance(platform),
                 instance(workspace),
-                instance(configuration),
                 instance(currentProcess),
                 100,
             );
@@ -312,14 +302,14 @@ suite('Multiroot Environment Variables Provider', () => {
             await provider.getEnvironmentVariables(workspaceUri);
 
             // Verify that the contents of `_getEnvironmentVariables()` method are invoked twice
-            verify(configuration.getSettings(anything())).twice();
+            verify(workspace.getConfiguration('python', anything())).twice();
             assert.deepEqual(vars, {});
         });
 
         test(`Environment variables are updated when env file changes ${workspaceTitle}`, async () => {
             const root = workspaceUri?.fsPath ?? '';
             const sourceDir = path.join(root, 'a', 'b');
-            const envFile = path.join(sourceDir, 'env.file');
+            envFile = path.join(sourceDir, 'env.file');
             const sourceFile = path.join(sourceDir, 'main.py');
 
             const workspaceFolder = workspaceUri ? { name: '', index: 0, uri: workspaceUri } : undefined;
@@ -339,7 +329,6 @@ suite('Multiroot Environment Variables Provider', () => {
             when(workspace.createFileSystemWatcher(envFile)).thenReturn(fileSystemWatcher.object);
 
             when(currentProcess.env).thenReturn(currentProcEnv);
-            when(settings.envFile).thenReturn(envFile);
             when(workspace.getWorkspaceFolder(anything())).thenReturn(workspaceFolder);
             when(envVarsService.parseFile(envFile, currentProcEnv)).thenCall(async () => ({ ...envFileVars }));
             when(platform.pathVariableName).thenReturn('PATH');