Skip to content

Commit d619de6

Browse files
Kartik Rajkarthiknadig
Kartik Raj
authored and
karthiknadig
committed
Ensure environment variables provider can be created using standard classes (microsoft#15377)
* Ensure environment variables provider can be created using standard classes * Fix unit tests
1 parent 6db5c5e commit d619de6

5 files changed

Lines changed: 29 additions & 36 deletions

File tree

‎src/client/common/variables/environmentVariablesProvider.ts‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,12 @@ import { inject, injectable, optional } from 'inversify';
55
import { ConfigurationChangeEvent, Disposable, Event, EventEmitter, FileSystemWatcher, Uri } from 'vscode';
66
import { sendFileCreationTelemetry } from '../../telemetry/envFileTelemetry';
77
import { IWorkspaceService } from '../application/types';
8+
import { PythonSettings } from '../configSettings';
89
import { traceVerbose } from '../logger';
910
import { IPlatformService } from '../platform/types';
10-
import { IConfigurationService, ICurrentProcess, IDisposableRegistry } from '../types';
11+
import { ICurrentProcess, IDisposableRegistry } from '../types';
1112
import { InMemoryCache } from '../utils/cacheUtils';
13+
import { SystemVariables } from './systemVariables';
1214
import { EnvironmentVariables, IEnvironmentVariablesProvider, IEnvironmentVariablesService } from './types';
1315

1416
const CACHE_DURATION = 60 * 60 * 1000;
@@ -25,7 +27,6 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid
2527
@inject(IDisposableRegistry) disposableRegistry: Disposable[],
2628
@inject(IPlatformService) private platformService: IPlatformService,
2729
@inject(IWorkspaceService) private workspaceService: IWorkspaceService,
28-
@inject(IConfigurationService) private readonly configurationService: IConfigurationService,
2930
@inject(ICurrentProcess) private process: ICurrentProcess,
3031
@optional() private cacheDuration: number = CACHE_DURATION,
3132
) {
@@ -86,11 +87,17 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid
8687
return mergedVars;
8788
}
8889
public async getCustomEnvironmentVariables(resource?: Uri): Promise<EnvironmentVariables | undefined> {
89-
const settings = this.configurationService.getSettings(resource);
90+
const systemVariables: SystemVariables = new SystemVariables(
91+
undefined,
92+
PythonSettings.getSettingsUriAndTarget(resource, this.workspaceService).uri?.fsPath,
93+
this.workspaceService,
94+
);
95+
const envFileSetting = this.workspaceService.getConfiguration('python', resource).get<string>('envFile');
96+
const envFile = systemVariables.resolveAny(envFileSetting)!;
9097
const workspaceFolderUri = this.getWorkspaceFolderUri(resource);
9198
this.trackedWorkspaceFolders.add(workspaceFolderUri ? workspaceFolderUri.fsPath : '');
92-
this.createFileWatcher(settings.envFile, workspaceFolderUri);
93-
return this.envVarsService.parseFile(settings.envFile, this.process.env);
99+
this.createFileWatcher(envFile, workspaceFolderUri);
100+
return this.envVarsService.parseFile(envFile, this.process.env);
94101
}
95102
public configurationChanged(e: ConfigurationChangeEvent) {
96103
this.trackedWorkspaceFolders.forEach((item) => {

‎src/client/extensionActivation.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import {
3030
IOutputChannel,
3131
} from './common/types';
3232
import { noop } from './common/utils/misc';
33-
import { registerTypes as variableRegisterTypes } from './common/variables/serviceRegistry';
3433
import { DebuggerTypeName } from './debugger/constants';
3534
import { DebugSessionEventDispatcher } from './debugger/extension/hooks/eventHandlerDispatcher';
3635
import { IDebugSessionEventHandlers } from './debugger/extension/hooks/types';
@@ -112,7 +111,6 @@ async function activateLegacy(ext: ExtensionState): Promise<ActivationResult> {
112111
const { enableProposedApi } = applicationEnv.packageJson;
113112
serviceManager.addSingletonInstance<boolean>(UseProposedApi, enableProposedApi);
114113
// Feature specific registrations.
115-
variableRegisterTypes(serviceManager);
116114
unitTestsRegisterTypes(serviceManager);
117115
lintersRegisterTypes(serviceManager);
118116
interpretersRegisterTypes(serviceManager);

‎src/client/extensionInit.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
IOutputChannel,
1818
WORKSPACE_MEMENTO,
1919
} from './common/types';
20+
import { registerTypes as variableRegisterTypes } from './common/variables/serviceRegistry';
2021
import { OutputChannelNames } from './common/utils/localize';
2122
import { ExtensionState } from './components';
2223
import { ServiceContainer } from './ioc/container';
@@ -69,6 +70,7 @@ export function initializeStandard(ext: ExtensionState): void {
6970
const { serviceManager } = ext.legacyIOC;
7071
// Core registrations (non-feature specific).
7172
commonRegisterTypes(serviceManager);
73+
variableRegisterTypes(serviceManager);
7274
platformRegisterTypes(serviceManager);
7375
processRegisterTypes(serviceManager);
7476

‎src/test/common/variables/envVarsProvider.multiroot.test.ts‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import * as path from 'path';
77
import { anything, instance, mock, when } from 'ts-mockito';
88
import { ConfigurationTarget, Disposable, Uri, workspace } from 'vscode';
99
import { WorkspaceService } from '../../../client/common/application/workspace';
10-
import { ConfigurationService } from '../../../client/common/configuration/service';
1110
import { PlatformService } from '../../../client/common/platform/platformService';
1211
import { IFileSystem } from '../../../client/common/platform/types';
1312
import { IDisposableRegistry, IPathUtils } from '../../../client/common/types';
@@ -76,14 +75,12 @@ suite('Multiroot Environment Variables Provider', () => {
7675
const variablesService = new EnvironmentVariablesService(pathUtils, fs);
7776
const disposables = ioc.serviceContainer.get<Disposable[]>(IDisposableRegistry);
7877
ioc.serviceManager.addSingletonInstance(IInterpreterAutoSelectionService, new MockAutoSelectionService());
79-
const cfgService = new ConfigurationService(ioc.serviceContainer);
8078
const workspaceService = new WorkspaceService();
8179
return new EnvironmentVariablesProvider(
8280
variablesService,
8381
disposables,
8482
new PlatformService(),
8583
workspaceService,
86-
cfgService,
8784
mockProcess,
8885
);
8986
}

‎src/test/common/variables/environmentVariablesProvider.unit.test.ts‎

Lines changed: 15 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,10 @@ import * as typemoq from 'typemoq';
1111
import { ConfigurationChangeEvent, FileSystemWatcher, Uri } from 'vscode';
1212
import { IWorkspaceService } from '../../../client/common/application/types';
1313
import { WorkspaceService } from '../../../client/common/application/workspace';
14-
import { PythonSettings } from '../../../client/common/configSettings';
15-
import { ConfigurationService } from '../../../client/common/configuration/service';
1614
import { PlatformService } from '../../../client/common/platform/platformService';
1715
import { IPlatformService } from '../../../client/common/platform/types';
1816
import { CurrentProcess } from '../../../client/common/process/currentProcess';
19-
import { IConfigurationService, ICurrentProcess, IPythonSettings } from '../../../client/common/types';
17+
import { ICurrentProcess } from '../../../client/common/types';
2018
import { sleep } from '../../../client/common/utils/async';
2119
import { EnvironmentVariablesService } from '../../../client/common/variables/environment';
2220
import { EnvironmentVariablesProvider } from '../../../client/common/variables/environmentVariablesProvider';
@@ -29,26 +27,27 @@ suite('Multiroot Environment Variables Provider', () => {
2927
let envVarsService: IEnvironmentVariablesService;
3028
let platform: IPlatformService;
3129
let workspace: IWorkspaceService;
32-
let configuration: IConfigurationService;
3330
let currentProcess: ICurrentProcess;
34-
let settings: IPythonSettings;
31+
let envFile: string;
3532

3633
setup(() => {
34+
envFile = '';
3735
envVarsService = mock(EnvironmentVariablesService);
3836
platform = mock(PlatformService);
3937
workspace = mock(WorkspaceService);
40-
configuration = mock(ConfigurationService);
4138
currentProcess = mock(CurrentProcess);
42-
settings = mock(PythonSettings);
4339

44-
when(configuration.getSettings(anything())).thenReturn(instance(settings));
4540
when(workspace.onDidChangeConfiguration).thenReturn(noop as any);
41+
when(workspace.getConfiguration('python', anything())).thenReturn({
42+
get: (settingName: string) => {
43+
return settingName === 'envFile' ? envFile : '';
44+
},
45+
} as any);
4646
provider = new EnvironmentVariablesProvider(
4747
instance(envVarsService),
4848
[],
4949
instance(platform),
5050
instance(workspace),
51-
instance(configuration),
5251
instance(currentProcess),
5352
);
5453

@@ -198,62 +197,56 @@ suite('Multiroot Environment Variables Provider', () => {
198197
});
199198

200199
test(`Getting environment variables (without an envfile, without PATH in current env, without PYTHONPATH in current env) & ${workspaceTitle}`, async () => {
201-
const envFile = path.join('a', 'b', 'env.file');
200+
envFile = path.join('a', 'b', 'env.file');
202201
const workspaceFolder = workspaceUri ? { name: '', index: 0, uri: workspaceUri } : undefined;
203202
const currentProcEnv = { SOMETHING: 'wow' };
204203

205204
when(currentProcess.env).thenReturn(currentProcEnv);
206-
when(settings.envFile).thenReturn(envFile);
207205
when(workspace.getWorkspaceFolder(workspaceUri)).thenReturn(workspaceFolder);
208206
when(envVarsService.parseFile(envFile, currentProcEnv)).thenResolve(undefined);
209207
when(platform.pathVariableName).thenReturn('PATH');
210208

211209
const vars = await provider.getEnvironmentVariables(workspaceUri);
212210

213211
verify(currentProcess.env).atLeast(1);
214-
verify(settings.envFile).atLeast(1);
215212
verify(envVarsService.parseFile(envFile, currentProcEnv)).atLeast(1);
216213
verify(envVarsService.mergeVariables(deepEqual(currentProcEnv), deepEqual({}))).once();
217214
verify(platform.pathVariableName).atLeast(1);
218215
assert.deepEqual(vars, {});
219216
});
220217
test(`Getting environment variables (with an envfile, without PATH in current env, without PYTHONPATH in current env) & ${workspaceTitle}`, async () => {
221-
const envFile = path.join('a', 'b', 'env.file');
218+
envFile = path.join('a', 'b', 'env.file');
222219
const workspaceFolder = workspaceUri ? { name: '', index: 0, uri: workspaceUri } : undefined;
223220
const currentProcEnv = { SOMETHING: 'wow' };
224221
const envFileVars = { MY_FILE: '1234' };
225222

226223
when(currentProcess.env).thenReturn(currentProcEnv);
227-
when(settings.envFile).thenReturn(envFile);
228224
when(workspace.getWorkspaceFolder(workspaceUri)).thenReturn(workspaceFolder);
229225
when(envVarsService.parseFile(envFile, currentProcEnv)).thenCall(async () => ({ ...envFileVars }));
230226
when(platform.pathVariableName).thenReturn('PATH');
231227

232228
const vars = await provider.getEnvironmentVariables(workspaceUri);
233229

234230
verify(currentProcess.env).atLeast(1);
235-
verify(settings.envFile).atLeast(1);
236231
verify(envVarsService.parseFile(envFile, currentProcEnv)).atLeast(1);
237232
verify(envVarsService.mergeVariables(deepEqual(currentProcEnv), deepEqual(envFileVars))).once();
238233
verify(platform.pathVariableName).atLeast(1);
239234
assert.deepEqual(vars, envFileVars);
240235
});
241236
test(`Getting environment variables (with an envfile, with PATH in current env, with PYTHONPATH in current env) & ${workspaceTitle}`, async () => {
242-
const envFile = path.join('a', 'b', 'env.file');
237+
envFile = path.join('a', 'b', 'env.file');
243238
const workspaceFolder = workspaceUri ? { name: '', index: 0, uri: workspaceUri } : undefined;
244239
const currentProcEnv = { SOMETHING: 'wow', PATH: 'some path value', PYTHONPATH: 'some python path value' };
245240
const envFileVars = { MY_FILE: '1234' };
246241

247242
when(currentProcess.env).thenReturn(currentProcEnv);
248-
when(settings.envFile).thenReturn(envFile);
249243
when(workspace.getWorkspaceFolder(workspaceUri)).thenReturn(workspaceFolder);
250244
when(envVarsService.parseFile(envFile, currentProcEnv)).thenCall(async () => ({ ...envFileVars }));
251245
when(platform.pathVariableName).thenReturn('PATH');
252246

253247
const vars = await provider.getEnvironmentVariables(workspaceUri);
254248

255249
verify(currentProcess.env).atLeast(1);
256-
verify(settings.envFile).atLeast(1);
257250
verify(envVarsService.parseFile(envFile, currentProcEnv)).atLeast(1);
258251
verify(envVarsService.mergeVariables(deepEqual(currentProcEnv), deepEqual(envFileVars))).once();
259252
verify(envVarsService.appendPath(deepEqual(envFileVars), currentProcEnv.PATH)).once();
@@ -263,12 +256,11 @@ suite('Multiroot Environment Variables Provider', () => {
263256
});
264257

265258
test(`Getting environment variables which are already cached does not reinvoke the method ${workspaceTitle}`, async () => {
266-
const envFile = path.join('a', 'b', 'env.file');
259+
envFile = path.join('a', 'b', 'env.file');
267260
const workspaceFolder = workspaceUri ? { name: '', index: 0, uri: workspaceUri } : undefined;
268261
const currentProcEnv = { SOMETHING: 'wow' };
269262

270263
when(currentProcess.env).thenReturn(currentProcEnv);
271-
when(settings.envFile).thenReturn(envFile);
272264
when(workspace.getWorkspaceFolder(workspaceUri)).thenReturn(workspaceFolder);
273265
when(envVarsService.parseFile(envFile, currentProcEnv)).thenResolve(undefined);
274266
when(platform.pathVariableName).thenReturn('PATH');
@@ -280,7 +272,7 @@ suite('Multiroot Environment Variables Provider', () => {
280272
await provider.getEnvironmentVariables(workspaceUri);
281273

282274
// Verify that the contents of `_getEnvironmentVariables()` method are only invoked once
283-
verify(configuration.getSettings(anything())).once();
275+
verify(workspace.getConfiguration('python', anything())).once();
284276
assert.deepEqual(vars, {});
285277
});
286278

@@ -290,7 +282,6 @@ suite('Multiroot Environment Variables Provider', () => {
290282
const currentProcEnv = { SOMETHING: 'wow' };
291283

292284
when(currentProcess.env).thenReturn(currentProcEnv);
293-
when(settings.envFile).thenReturn(envFile);
294285
when(workspace.getWorkspaceFolder(workspaceUri)).thenReturn(workspaceFolder);
295286
when(envVarsService.parseFile(envFile, currentProcEnv)).thenResolve(undefined);
296287
when(platform.pathVariableName).thenReturn('PATH');
@@ -300,7 +291,6 @@ suite('Multiroot Environment Variables Provider', () => {
300291
[],
301292
instance(platform),
302293
instance(workspace),
303-
instance(configuration),
304294
instance(currentProcess),
305295
100,
306296
);
@@ -312,14 +302,14 @@ suite('Multiroot Environment Variables Provider', () => {
312302
await provider.getEnvironmentVariables(workspaceUri);
313303

314304
// Verify that the contents of `_getEnvironmentVariables()` method are invoked twice
315-
verify(configuration.getSettings(anything())).twice();
305+
verify(workspace.getConfiguration('python', anything())).twice();
316306
assert.deepEqual(vars, {});
317307
});
318308

319309
test(`Environment variables are updated when env file changes ${workspaceTitle}`, async () => {
320310
const root = workspaceUri?.fsPath ?? '';
321311
const sourceDir = path.join(root, 'a', 'b');
322-
const envFile = path.join(sourceDir, 'env.file');
312+
envFile = path.join(sourceDir, 'env.file');
323313
const sourceFile = path.join(sourceDir, 'main.py');
324314

325315
const workspaceFolder = workspaceUri ? { name: '', index: 0, uri: workspaceUri } : undefined;
@@ -339,7 +329,6 @@ suite('Multiroot Environment Variables Provider', () => {
339329
when(workspace.createFileSystemWatcher(envFile)).thenReturn(fileSystemWatcher.object);
340330

341331
when(currentProcess.env).thenReturn(currentProcEnv);
342-
when(settings.envFile).thenReturn(envFile);
343332
when(workspace.getWorkspaceFolder(anything())).thenReturn(workspaceFolder);
344333
when(envVarsService.parseFile(envFile, currentProcEnv)).thenCall(async () => ({ ...envFileVars }));
345334
when(platform.pathVariableName).thenReturn('PATH');

0 commit comments

Comments
 (0)