Skip to content

Commit 0963a07

Browse files
author
Kartik Raj
committed
Code reviews
1 parent dab9ee5 commit 0963a07

File tree

5 files changed

+39
-38
lines changed

5 files changed

+39
-38
lines changed

src/client/pythonEnvironments/base/info/interpreter.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ type ShellExecFunc = (command: string, timeout: number) => Promise<ShellExecResu
4646

4747
type Logger = {
4848
info(msg: string): void;
49+
4950
error(msg: string): void;
5051
};
5152

src/client/pythonEnvironments/base/locator.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ export type PythonLocatorQuery = BasicPythonLocatorQuery & {
9292
searchLocations?: Uri[];
9393
};
9494

95-
export type QueryForEvent<E> = E extends PythonEnvsChangedEvent ? PythonLocatorQuery : BasicPythonLocatorQuery;
95+
type QueryForEvent<E> = E extends PythonEnvsChangedEvent ? PythonLocatorQuery : BasicPythonLocatorQuery;
9696

9797
/**
9898
* A single Python environment locator.

src/client/pythonEnvironments/collection/environmentsReducer.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { traceVerbose } from '../../common/logger';
77
import { createDeferred } from '../../common/utils/async';
88
import { areSameEnvironment, PythonEnvInfo, PythonEnvKind } from '../base/info';
99
import {
10-
ILocator, IPythonEnvsIterator, PythonEnvUpdatedEvent, QueryForEvent,
10+
ILocator, IPythonEnvsIterator, PythonEnvUpdatedEvent, PythonLocatorQuery,
1111
} from '../base/locator';
1212
import { PythonEnvsChangedEvent } from '../base/watcher';
1313

@@ -47,7 +47,7 @@ export class PythonEnvsReducer implements ILocator {
4747
return this.parentLocator.resolveEnv(environment);
4848
}
4949

50-
public iterEnvs(query?: QueryForEvent<PythonEnvsChangedEvent>): IPythonEnvsIterator {
50+
public iterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator {
5151
const didUpdate = new EventEmitter<PythonEnvUpdatedEvent | null>();
5252
const incomingIterator = this.parentLocator.iterEnvs(query);
5353
const iterator: IPythonEnvsIterator = iterEnvsIterator(incomingIterator, didUpdate);

src/client/pythonEnvironments/collection/environmentsResolver.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,23 +7,23 @@ import { traceVerbose } from '../../common/logger';
77
import { areSameEnvironment, PythonEnvInfo } from '../base/info';
88
import { InterpreterInformation } from '../base/info/interpreter';
99
import {
10-
ILocator, IPythonEnvsIterator, PythonEnvUpdatedEvent, QueryForEvent,
10+
ILocator, IPythonEnvsIterator, PythonEnvUpdatedEvent, PythonLocatorQuery,
1111
} from '../base/locator';
1212
import { PythonEnvsChangedEvent } from '../base/watcher';
1313
import { IEnvironmentInfoService } from '../info/environmentInfoService';
1414

1515
export class PythonEnvsResolver implements ILocator {
1616
public get onChanged(): Event<PythonEnvsChangedEvent> {
17-
return this.pythonEnvsReducer.onChanged;
17+
return this.parentLocator.onChanged;
1818
}
1919

2020
constructor(
21-
private readonly pythonEnvsReducer: ILocator,
21+
private readonly parentLocator: ILocator,
2222
private readonly environmentInfoService: IEnvironmentInfoService,
2323
) {}
2424

2525
public async resolveEnv(env: string | PythonEnvInfo): Promise<PythonEnvInfo | undefined> {
26-
const environment = await this.pythonEnvsReducer.resolveEnv(env);
26+
const environment = await this.parentLocator.resolveEnv(env);
2727
if (!environment) {
2828
return undefined;
2929
}
@@ -34,9 +34,9 @@ export class PythonEnvsResolver implements ILocator {
3434
return getResolvedEnv(interpreterInfo, environment);
3535
}
3636

37-
public iterEnvs(query?: QueryForEvent<PythonEnvsChangedEvent>): IPythonEnvsIterator {
37+
public iterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator {
3838
const didUpdate = new EventEmitter<PythonEnvUpdatedEvent | null>();
39-
const incomingIterator = this.pythonEnvsReducer.iterEnvs(query);
39+
const incomingIterator = this.parentLocator.iterEnvs(query);
4040
const iterator: IPythonEnvsIterator = this.iterEnvsIterator(incomingIterator, didUpdate);
4141
iterator.onUpdated = didUpdate.event;
4242
return iterator;

src/test/pythonEnvironments/collection/environmentsResolver.unit.test.ts

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,10 @@ suite('Environments Resolver', () => {
5858
const env3 = createEnv('env3', '2.7', PythonEnvKind.System, path.join('path', 'to', 'exec3'));
5959
const env4 = createEnv('env4', '3.9.0rc2', PythonEnvKind.Unknown, path.join('path', 'to', 'exec2'));
6060
const environmentsToBeIterated = [env1, env2, env3, env4];
61-
const pythonEnvReducer = new SimpleLocator(environmentsToBeIterated);
62-
const reducer = new PythonEnvsResolver(pythonEnvReducer, new EnvironmentInfoService());
61+
const parentLocator = new SimpleLocator(environmentsToBeIterated);
62+
const resolver = new PythonEnvsResolver(parentLocator, new EnvironmentInfoService());
6363

64-
const iterator = reducer.iterEnvs();
64+
const iterator = resolver.iterEnvs();
6565
const envs = await getEnvs(iterator);
6666

6767
assert.deepEqual(envs, environmentsToBeIterated);
@@ -72,11 +72,11 @@ suite('Environments Resolver', () => {
7272
const env1 = createEnv('env1', '3.5.12b1', PythonEnvKind.Unknown, path.join('path', 'to', 'exec1'));
7373
const env2 = createEnv('env2', '3.8.1', PythonEnvKind.Unknown, path.join('path', 'to', 'exec2'));
7474
const environmentsToBeIterated = [env1, env2];
75-
const pythonEnvReducer = new SimpleLocator(environmentsToBeIterated);
75+
const parentLocator = new SimpleLocator(environmentsToBeIterated);
7676
const onUpdatedEvents: (PythonEnvUpdatedEvent | null)[] = [];
77-
const reducer = new PythonEnvsResolver(pythonEnvReducer, new EnvironmentInfoService());
77+
const resolver = new PythonEnvsResolver(parentLocator, new EnvironmentInfoService());
7878

79-
const iterator = reducer.iterEnvs(); // Act
79+
const iterator = resolver.iterEnvs(); // Act
8080

8181
// Assert
8282
let { onUpdated } = iterator;
@@ -107,11 +107,11 @@ suite('Environments Resolver', () => {
107107
const updatedEnv = createEnv('env1', '3.8.1', PythonEnvKind.System, path.join('path', 'to', 'exec'));
108108
const environmentsToBeIterated = [env];
109109
const didUpdate = new EventEmitter<PythonEnvUpdatedEvent | null>();
110-
const pythonEnvReducer = new SimpleLocator(environmentsToBeIterated, { onUpdated: didUpdate.event });
110+
const parentLocator = new SimpleLocator(environmentsToBeIterated, { onUpdated: didUpdate.event });
111111
const onUpdatedEvents: (PythonEnvUpdatedEvent | null)[] = [];
112-
const reducer = new PythonEnvsResolver(pythonEnvReducer, new EnvironmentInfoService());
112+
const resolver = new PythonEnvsResolver(parentLocator, new EnvironmentInfoService());
113113

114-
const iterator = reducer.iterEnvs(); // Act
114+
const iterator = resolver.iterEnvs(); // Act
115115

116116
// Assert
117117
let { onUpdated } = iterator;
@@ -143,18 +143,18 @@ suite('Environments Resolver', () => {
143143
});
144144
});
145145

146-
test('onChanged fires iff onChanged from reducer fires', () => {
147-
const pythonEnvReducer = new SimpleLocator([]);
146+
test('onChanged fires iff onChanged from resolver fires', () => {
147+
const parentLocator = new SimpleLocator([]);
148148
const event1: PythonEnvsChangedEvent = {};
149149
const event2: PythonEnvsChangedEvent = { kind: PythonEnvKind.Unknown };
150150
const expected = [event1, event2];
151-
const reducer = new PythonEnvsResolver(pythonEnvReducer, new EnvironmentInfoService());
151+
const resolver = new PythonEnvsResolver(parentLocator, new EnvironmentInfoService());
152152

153153
const events: PythonEnvsChangedEvent[] = [];
154-
reducer.onChanged((e) => events.push(e));
154+
resolver.onChanged((e) => events.push(e));
155155

156-
pythonEnvReducer.fire(event1);
157-
pythonEnvReducer.fire(event2);
156+
parentLocator.fire(event1);
157+
parentLocator.fire(event2);
158158

159159
assert.deepEqual(events, expected);
160160
});
@@ -178,30 +178,30 @@ suite('Environments Resolver', () => {
178178
stubShellExec.restore();
179179
});
180180

181-
test('Calls into reducer to get resolved environment, then calls environnment service to resolve environment further and return it', async () => {
181+
test('Calls into parent locator to get resolved environment, then calls environnment service to resolve environment further and return it', async () => {
182182
const env = createEnv('env1', '3.8', PythonEnvKind.Unknown, path.join('path', 'to', 'exec'));
183183
const resolvedEnvReturnedByReducer = createEnv(
184184
'env1',
185185
'3.8.1',
186186
PythonEnvKind.Conda,
187187
'resolved/path/to/exec',
188188
);
189-
const pythonEnvReducer = new SimpleLocator([], {
189+
const parentLocator = new SimpleLocator([], {
190190
resolve: async (e: PythonEnvInfo) => {
191191
if (e === env) {
192192
return resolvedEnvReturnedByReducer;
193193
}
194-
throw new Error('Incorrect environment sent to the reducer');
194+
throw new Error('Incorrect environment sent to the resolver');
195195
},
196196
});
197-
const reducer = new PythonEnvsResolver(pythonEnvReducer, new EnvironmentInfoService());
197+
const resolver = new PythonEnvsResolver(parentLocator, new EnvironmentInfoService());
198198

199-
const expected = await reducer.resolveEnv(env);
199+
const expected = await resolver.resolveEnv(env);
200200

201201
assert.deepEqual(expected, createExpectedEnvInfo(resolvedEnvReturnedByReducer));
202202
});
203203

204-
test('If the reducer resolves environment, but fetching interpreter info returns undefined, return undefined', async () => {
204+
test('If the parent locator resolves environment, but fetching interpreter info returns undefined, return undefined', async () => {
205205
stubShellExec.returns(
206206
new Promise<ExecutionResult<string>>((_resolve, reject) => {
207207
reject();
@@ -214,29 +214,29 @@ suite('Environments Resolver', () => {
214214
PythonEnvKind.Conda,
215215
'resolved/path/to/exec',
216216
);
217-
const pythonEnvReducer = new SimpleLocator([], {
217+
const parentLocator = new SimpleLocator([], {
218218
resolve: async (e: PythonEnvInfo) => {
219219
if (e === env) {
220220
return resolvedEnvReturnedByReducer;
221221
}
222-
throw new Error('Incorrect environment sent to the reducer');
222+
throw new Error('Incorrect environment sent to the resolver');
223223
},
224224
});
225-
const reducer = new PythonEnvsResolver(pythonEnvReducer, new EnvironmentInfoService());
225+
const resolver = new PythonEnvsResolver(parentLocator, new EnvironmentInfoService());
226226

227-
const expected = await reducer.resolveEnv(env);
227+
const expected = await resolver.resolveEnv(env);
228228

229229
assert.deepEqual(expected, undefined);
230230
});
231231

232-
test("If the reducer isn't able to resolve environment, return undefined", async () => {
232+
test("If the parent locator isn't able to resolve environment, return undefined", async () => {
233233
const env = createEnv('env', '3.8', PythonEnvKind.Unknown, path.join('path', 'to', 'exec'));
234-
const pythonEnvReducer = new SimpleLocator([], {
234+
const parentLocator = new SimpleLocator([], {
235235
resolve: async () => undefined,
236236
});
237-
const reducer = new PythonEnvsResolver(pythonEnvReducer, new EnvironmentInfoService());
237+
const resolver = new PythonEnvsResolver(parentLocator, new EnvironmentInfoService());
238238

239-
const expected = await reducer.resolveEnv(env);
239+
const expected = await resolver.resolveEnv(env);
240240

241241
assert.deepEqual(expected, undefined);
242242
});

0 commit comments

Comments
 (0)