Skip to content

Fix ESLint errors for environment variable provider #15383

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
karrtikr merged 1 commit into from
Feb 11, 2021
Merged

Fix ESLint errors for environment variable provider #15383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions .eslintignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,6 @@ src/test/common/featureDeprecationManager.unit.test.ts
src/test/common/serviceRegistry.unit.test.ts
src/test/common/extensions.unit.test.ts
src/test/common/variables/envVarsService.unit.test.ts
src/test/common/variables/environmentVariablesProvider.unit.test.ts
src/test/common/variables/envVarsProvider.multiroot.test.ts
src/test/common/nuget/azureBobStoreRepository.unit.test.ts
src/test/common/helpers.test.ts
src/test/common/application/commands/reloadCommand.unit.test.ts
Expand Down Expand Up @@ -575,7 +573,6 @@ src/client/common/constants.ts
src/client/common/variables/serviceRegistry.ts
src/client/common/variables/environment.ts
src/client/common/variables/types.ts
src/client/common/variables/environmentVariablesProvider.ts
src/client/common/variables/sysTypes.ts
src/client/common/variables/systemVariables.ts
src/client/common/nuget/azureBlobStoreNugetRepository.ts
Expand Down
17 changes: 13 additions & 4 deletions src/client/common/variables/environmentVariablesProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,13 @@ const CACHE_DURATION = 60 * 60 * 1000;
@injectable()
export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvider, Disposable {
public trackedWorkspaceFolders = new Set<string>();

private fileWatchers = new Map<string, FileSystemWatcher>();

private disposables: Disposable[] = [];

private changeEventEmitter: EventEmitter<Uri | undefined>;

private readonly envVarCaches = new Map<string, InMemoryCache<EnvironmentVariables>>();

constructor(
Expand All @@ -40,7 +44,7 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid
return this.changeEventEmitter.event;
}

public dispose() {
public dispose(): void {
this.changeEventEmitter.dispose();
this.fileWatchers.forEach((watcher) => {
if (watcher) {
Expand Down Expand Up @@ -70,6 +74,7 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid
cache.data = { ...vars };
return vars;
}

public async _getEnvironmentVariables(resource?: Uri): Promise<EnvironmentVariables> {
let mergedVars = await this.getCustomEnvironmentVariables(resource);
if (!mergedVars) {
Expand All @@ -86,6 +91,7 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid
}
return mergedVars;
}

public async getCustomEnvironmentVariables(resource?: Uri): Promise<EnvironmentVariables | undefined> {
const systemVariables: SystemVariables = new SystemVariables(
undefined,
Expand All @@ -99,15 +105,17 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid
this.createFileWatcher(envFile, workspaceFolderUri);
return this.envVarsService.parseFile(envFile, this.process.env);
}
public configurationChanged(e: ConfigurationChangeEvent) {

public configurationChanged(e: ConfigurationChangeEvent): void {
this.trackedWorkspaceFolders.forEach((item) => {
const uri = item && item.length > 0 ? Uri.file(item) : undefined;
if (e.affectsConfiguration('python.envFile', uri)) {
this.onEnvironmentFileChanged(uri);
}
});
}
public createFileWatcher(envFile: string, workspaceFolderUri?: Uri) {

public createFileWatcher(envFile: string, workspaceFolderUri?: Uri): void {
if (this.fileWatchers.has(envFile)) {
return;
}
Expand All @@ -119,9 +127,10 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid
this.disposables.push(envFileWatcher.onDidDelete(() => this.onEnvironmentFileChanged(workspaceFolderUri)));
}
}

private getWorkspaceFolderUri(resource?: Uri): Uri | undefined {
if (!resource) {
return;
return undefined;
}
const workspaceFolder = this.workspaceService.getWorkspaceFolder(resource!);
return workspaceFolder ? workspaceFolder.uri : undefined;
Expand Down
4 changes: 2 additions & 2 deletions src/test/common/variables/envVarsProvider.multiroot.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ suite('Multiroot Environment Variables Provider', () => {
const pathVariableName = getSearchPathEnvVarNames()[0];
suiteSetup(async function () {
if (!IS_MULTI_ROOT_TEST) {
return this.skip();
this.skip();
}
await clearPythonPathInWorkspaceFolder(workspace4Path);
await updateSetting('envFile', undefined, workspace4PyFile, ConfigurationTarget.WorkspaceFolder);
Expand Down Expand Up @@ -178,7 +178,7 @@ suite('Multiroot Environment Variables Provider', () => {
// this test is flaky on windows (likely the value of the path property
// has incorrect path separator chars). Tracked by GH #4756
if (isOs(OSType.Windows)) {
return this.skip();
this.skip();
}

await updateSetting('envFile', '${workspaceRoot}/.env', workspace4PyFile, ConfigurationTarget.WorkspaceFolder);
Expand Down
59 changes: 42 additions & 17 deletions src/test/common/variables/environmentVariablesProvider.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

Expand Down Expand Up @@ -65,7 +66,9 @@ suite('Multiroot Environment Variables Provider', () => {

provider.trackedWorkspaceFolders.add(workspaceFolder1Uri.fsPath);
provider.trackedWorkspaceFolders.add(workspaceFolder2Uri.fsPath);
provider.onDidEnvironmentVariablesChange((uri) => (affectedWorkspace = uri));
provider.onDidEnvironmentVariablesChange((uri) => {
affectedWorkspace = uri;
});
const changedEvent: ConfigurationChangeEvent = {
affectsConfiguration(setting: string, uri?: Uri) {
return setting === 'python.envFile' && uri!.fsPath === workspaceFolder1Uri.fsPath;
Expand All @@ -82,9 +85,11 @@ suite('Multiroot Environment Variables Provider', () => {
const workspaceFolderUri = Uri.file('workspace1');

provider.trackedWorkspaceFolders.add(workspaceFolderUri.fsPath);
provider.onDidEnvironmentVariablesChange((uri) => (affectedWorkspace = uri));
provider.onDidEnvironmentVariablesChange((uri) => {
affectedWorkspace = uri;
});
const changedEvent: ConfigurationChangeEvent = {
affectsConfiguration(_setting: string, _uri?: Uri) {
affectsConfiguration() {
return false;
},
};
Expand All @@ -95,9 +100,11 @@ suite('Multiroot Environment Variables Provider', () => {
});
test('Event is not fired when workspace is not tracked', () => {
let affectedWorkspace: Uri | undefined;
provider.onDidEnvironmentVariablesChange((uri) => (affectedWorkspace = uri));
provider.onDidEnvironmentVariablesChange((uri) => {
affectedWorkspace = uri;
});
const changedEvent: ConfigurationChangeEvent = {
affectsConfiguration(_setting: string, _uri?: Uri) {
affectsConfiguration() {
return true;
},
};
Expand All @@ -110,17 +117,22 @@ suite('Multiroot Environment Variables Provider', () => {
const workspaceTitle = workspaceUri ? '(with a workspace)' : '(without a workspace)';
test(`Event is fired when the environment file is modified ${workspaceTitle}`, () => {
let affectedWorkspace: Uri | undefined = Uri.file('dummy value');
const envFile = path.join('a', 'b', 'env.file');
envFile = path.join('a', 'b', 'env.file');
const fileSystemWatcher = typemoq.Mock.ofType<FileSystemWatcher>();

// eslint-disable-next-line @typescript-eslint/ban-types
let onChangeHandler: undefined | ((resource?: Uri) => Function);

fileSystemWatcher
.setup((fs) => fs.onDidChange(typemoq.It.isAny()))
.callback((cb) => (onChangeHandler = cb))
.callback((cb) => {
onChangeHandler = cb;
})
.verifiable(typemoq.Times.once());
when(workspace.createFileSystemWatcher(envFile)).thenReturn(fileSystemWatcher.object);
provider.onDidEnvironmentVariablesChange((uri) => (affectedWorkspace = uri));
provider.onDidEnvironmentVariablesChange((uri) => {
affectedWorkspace = uri;
});

provider.createFileWatcher(envFile, workspaceUri);

Expand All @@ -133,17 +145,22 @@ suite('Multiroot Environment Variables Provider', () => {
});
test(`Event is fired when the environment file is deleted ${workspaceTitle}`, () => {
let affectedWorkspace: Uri | undefined = Uri.file('dummy value');
const envFile = path.join('a', 'b', 'env.file');
envFile = path.join('a', 'b', 'env.file');
const fileSystemWatcher = typemoq.Mock.ofType<FileSystemWatcher>();

// eslint-disable-next-line @typescript-eslint/ban-types
let onDeleted: undefined | ((resource?: Uri) => Function);

fileSystemWatcher
.setup((fs) => fs.onDidDelete(typemoq.It.isAny()))
.callback((cb) => (onDeleted = cb))
.callback((cb) => {
onDeleted = cb;
})
.verifiable(typemoq.Times.once());
when(workspace.createFileSystemWatcher(envFile)).thenReturn(fileSystemWatcher.object);
provider.onDidEnvironmentVariablesChange((uri) => (affectedWorkspace = uri));
provider.onDidEnvironmentVariablesChange((uri) => {
affectedWorkspace = uri;
});

provider.createFileWatcher(envFile, workspaceUri);

Expand All @@ -156,17 +173,22 @@ suite('Multiroot Environment Variables Provider', () => {
});
test(`Event is fired when the environment file is created ${workspaceTitle}`, () => {
let affectedWorkspace: Uri | undefined = Uri.file('dummy value');
const envFile = path.join('a', 'b', 'env.file');
envFile = path.join('a', 'b', 'env.file');
const fileSystemWatcher = typemoq.Mock.ofType<FileSystemWatcher>();

// eslint-disable-next-line @typescript-eslint/ban-types
let onCreated: undefined | ((resource?: Uri) => Function);

fileSystemWatcher
.setup((fs) => fs.onDidCreate(typemoq.It.isAny()))
.callback((cb) => (onCreated = cb))
.callback((cb) => {
onCreated = cb;
})
.verifiable(typemoq.Times.once());
when(workspace.createFileSystemWatcher(envFile)).thenReturn(fileSystemWatcher.object);
provider.onDidEnvironmentVariablesChange((uri) => (affectedWorkspace = uri));
provider.onDidEnvironmentVariablesChange((uri) => {
affectedWorkspace = uri;
});

provider.createFileWatcher(envFile, workspaceUri);

Expand All @@ -178,7 +200,7 @@ suite('Multiroot Environment Variables Provider', () => {
assert.equal(affectedWorkspace, workspaceUri);
});
test(`File system watcher event handlers are added once ${workspaceTitle}`, () => {
const envFile = path.join('a', 'b', 'env.file');
envFile = path.join('a', 'b', 'env.file');
const fileSystemWatcher = typemoq.Mock.ofType<FileSystemWatcher>();

fileSystemWatcher.setup((fs) => fs.onDidChange(typemoq.It.isAny())).verifiable(typemoq.Times.once());
Expand Down Expand Up @@ -277,7 +299,7 @@ suite('Multiroot Environment Variables Provider', () => {
});

test(`Cache result must be cleared when cache expires ${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' };

Expand Down Expand Up @@ -319,12 +341,15 @@ suite('Multiroot Environment Variables Provider', () => {
};
const envFileVars = { MY_FILE: '1234', PYTHONPATH: `./foo${path.delimiter}./bar` };

// eslint-disable-next-line @typescript-eslint/ban-types
let onChangeHandler: undefined | ((resource?: Uri) => Function);
const fileSystemWatcher = typemoq.Mock.ofType<FileSystemWatcher>();

fileSystemWatcher
.setup((fs) => fs.onDidChange(typemoq.It.isAny()))
.callback((cb) => (onChangeHandler = cb))
.callback((cb) => {
onChangeHandler = cb;
})
.verifiable(typemoq.Times.once());
when(workspace.createFileSystemWatcher(envFile)).thenReturn(fileSystemWatcher.object);

Expand Down