Skip to content

Handle venv creation when running in no-workspace case #145

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.

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
merged 3 commits into from
Jan 29, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions src/common/pickers/projects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ export async function pickProjectMany(
}
} else if (projects.length === 1) {
return [...projects];
} else if (projects.length === 0) {
return [];
}
return undefined;
}
63 changes: 33 additions & 30 deletions src/features/envCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,44 +111,47 @@ export async function createAnyEnvironmentCommand(
options?: any,
): Promise<PythonEnvironment | undefined> {
const select = options?.selectEnvironment;
const projects = await pickProjectMany(pm.getProjects());
if (projects && projects.length > 0) {
const defaultManagers: InternalEnvironmentManager[] = [];

projects.forEach((p) => {
const manager = em.getEnvironmentManager(p.uri);
if (manager && manager.supportsCreate && !defaultManagers.includes(manager)) {
defaultManagers.push(manager);
}
});

const managerId = await pickEnvironmentManager(
em.managers.filter((m) => m.supportsCreate),
defaultManagers,
);

const manager = em.managers.find((m) => m.id === managerId);
if (manager) {
const env = await manager.create(projects.map((p) => p.uri));
if (select) {
await em.setEnvironments(
projects.map((p) => p.uri),
env,
);
}
return env;
}
} else if (projects && projects.length === 0) {
const projects = pm.getProjects();
if (projects.length === 0) {
const managerId = await pickEnvironmentManager(em.managers.filter((m) => m.supportsCreate));

const manager = em.managers.find((m) => m.id === managerId);
if (manager) {
const env = await manager.create('global');
if (select) {
if (select && env) {
await manager.set(undefined, env);
}
return env;
}
} else if (projects.length > 0) {
const selected = await pickProjectMany(projects);

if (selected && selected.length > 0) {
const defaultManagers: InternalEnvironmentManager[] = [];

selected.forEach((p) => {
const manager = em.getEnvironmentManager(p.uri);
if (manager && manager.supportsCreate && !defaultManagers.includes(manager)) {
defaultManagers.push(manager);
}
});

const managerId = await pickEnvironmentManager(
em.managers.filter((m) => m.supportsCreate),
defaultManagers,
);

const manager = em.managers.find((m) => m.id === managerId);
if (manager) {
const env = await manager.create(selected.map((p) => p.uri));
if (select && env) {
await em.setEnvironments(
selected.map((p) => p.uri),
env,
);
}
return env;
}
}
}
}

Expand Down
21 changes: 3 additions & 18 deletions src/test/copilotTools.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ suite('GetPackagesTool Tests', () => {
mockApi = typeMoq.Mock.ofType<PythonProjectEnvironmentApi & PythonPackageGetterApi>();
mockEnvironment = typeMoq.Mock.ofType<PythonEnvironment>();

// eslint-disable-next-line @typescript-eslint/no-explicit-any
mockEnvironment.setup((x: any) => x.then).returns(() => undefined);

// refresh will always return a resolved promise
mockApi.setup((x) => x.refreshPackages(typeMoq.It.isAny())).returns(() => Promise.resolve());

Expand All @@ -30,9 +33,6 @@ suite('GetPackagesTool Tests', () => {
});

test('should throw error if filePath is undefined', async () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
mockEnvironment.setup((x: any) => x.then).returns(() => undefined);

const testFile: IGetActiveFile = {
filePath: '',
};
Expand Down Expand Up @@ -61,9 +61,6 @@ suite('GetPackagesTool Tests', () => {
});

test('should throw error for notebook cells', async () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
mockEnvironment.setup((x: any) => x.then).returns(() => undefined);

const testFile: IGetActiveFile = {
filePath: 'test.ipynb#123',
};
Expand All @@ -84,9 +81,6 @@ suite('GetPackagesTool Tests', () => {
filePath: 'test.py',
};

// eslint-disable-next-line @typescript-eslint/no-explicit-any
mockEnvironment.setup((x: any) => x.then).returns(() => undefined);

mockApi
.setup((x) => x.getEnvironment(typeMoq.It.isAny()))
.returns(() => {
Expand All @@ -109,9 +103,6 @@ suite('GetPackagesTool Tests', () => {
filePath: 'test.py',
};

// eslint-disable-next-line @typescript-eslint/no-explicit-any
mockEnvironment.setup((x: any) => x.then).returns(() => undefined);

mockApi
.setup((x) => x.getEnvironment(typeMoq.It.isAny()))
.returns(() => {
Expand Down Expand Up @@ -152,9 +143,6 @@ suite('GetPackagesTool Tests', () => {
filePath: 'test.py',
};

// eslint-disable-next-line @typescript-eslint/no-explicit-any
mockEnvironment.setup((x: any) => x.then).returns(() => undefined);

mockApi
.setup((x) => x.getEnvironment(typeMoq.It.isAny()))
.returns(() => {
Expand Down Expand Up @@ -197,9 +185,6 @@ suite('GetPackagesTool Tests', () => {
filePath: 'test.py',
};

// eslint-disable-next-line @typescript-eslint/no-explicit-any
mockEnvironment.setup((x: any) => x.then).returns(() => undefined);

mockApi
.setup((x) => x.getEnvironment(typeMoq.It.isAny()))
.returns(async () => {
Expand Down
177 changes: 177 additions & 0 deletions src/test/features/envCommands.unit.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
import * as assert from 'assert';
import * as typeMoq from 'typemoq';
import * as sinon from 'sinon';
import { EnvironmentManagers, InternalEnvironmentManager, PythonProjectManager } from '../../internal.api';
import * as projectApi from '../../common/pickers/projects';
import * as managerApi from '../../common/pickers/managers';
import { PythonEnvironment, PythonProject } from '../../api';
import { createAnyEnvironmentCommand } from '../../features/envCommands';
import { Uri } from 'vscode';

suite('Create Any Environment Command Tests', () => {
let em: typeMoq.IMock<EnvironmentManagers>;
let pm: typeMoq.IMock<PythonProjectManager>;
let manager: typeMoq.IMock<InternalEnvironmentManager>;
let env: typeMoq.IMock<PythonEnvironment>;
let pickProjectManyStub: sinon.SinonStub;
let pickEnvironmentManagerStub: sinon.SinonStub;
let project: PythonProject = {
uri: Uri.file('/some/test/workspace/folder'),
name: 'test-folder',
};
let project2: PythonProject = {
uri: Uri.file('/some/test/workspace/folder2'),
name: 'test-folder2',
};
let project3: PythonProject = {
uri: Uri.file('/some/test/workspace/folder3'),
name: 'test-folder3',
};

setup(() => {
manager = typeMoq.Mock.ofType<InternalEnvironmentManager>();
manager.setup((m) => m.id).returns(() => 'test');
manager.setup((m) => m.displayName).returns(() => 'Test Manager');
manager.setup((m) => m.description).returns(() => 'Test Manager Description');
manager.setup((m) => m.supportsCreate).returns(() => true);

env = typeMoq.Mock.ofType<PythonEnvironment>();
env.setup((e) => e.envId).returns(() => ({ id: 'env1', managerId: 'test' }));
// eslint-disable-next-line @typescript-eslint/no-explicit-any
env.setup((e: any) => e.then).returns(() => undefined);

em = typeMoq.Mock.ofType<EnvironmentManagers>();
em.setup((e) => e.managers).returns(() => [manager.object]);
em.setup((e) => e.getEnvironmentManager(typeMoq.It.isAnyString())).returns(() => manager.object);

pm = typeMoq.Mock.ofType<PythonProjectManager>();

pickEnvironmentManagerStub = sinon.stub(managerApi, 'pickEnvironmentManager');
pickProjectManyStub = sinon.stub(projectApi, 'pickProjectMany');
});

teardown(() => {
sinon.restore();
});

test('Create global venv (no-workspace): no-select', async () => {
pm.setup((p) => p.getProjects()).returns(() => []);
manager
.setup((m) => m.create('global'))
.returns(() => Promise.resolve(env.object))
.verifiable(typeMoq.Times.once());

manager.setup((m) => m.set(typeMoq.It.isAny(), typeMoq.It.isAny())).verifiable(typeMoq.Times.never());

pickEnvironmentManagerStub.resolves(manager.object.id);
pickProjectManyStub.resolves([]);

const result = await createAnyEnvironmentCommand(em.object, pm.object, { selectEnvironment: false });
// Add assertions to verify the result
assert.strictEqual(result, env.object, 'Expected the created environment to match the mocked environment.');
manager.verifyAll();
});

test('Create global venv (no-workspace): select', async () => {
pm.setup((p) => p.getProjects()).returns(() => []);
manager
.setup((m) => m.create('global'))
.returns(() => Promise.resolve(env.object))
.verifiable(typeMoq.Times.once());

manager.setup((m) => m.set(undefined, env.object)).verifiable(typeMoq.Times.once());

pickEnvironmentManagerStub.resolves(manager.object.id);
pickProjectManyStub.resolves([]);

const result = await createAnyEnvironmentCommand(em.object, pm.object, { selectEnvironment: true });
// Add assertions to verify the result
assert.strictEqual(result, env.object, 'Expected the created environment to match the mocked environment.');
manager.verifyAll();
});

test('Create workspace venv: no-select', async () => {
pm.setup((p) => p.getProjects()).returns(() => [project]);
manager
.setup((m) => m.create([project.uri]))
.returns(() => Promise.resolve(env.object))
.verifiable(typeMoq.Times.once());

manager.setup((m) => m.set(typeMoq.It.isAny(), typeMoq.It.isAny())).verifiable(typeMoq.Times.never());
em.setup((e) => e.setEnvironments(typeMoq.It.isAny(), typeMoq.It.isAny())).verifiable(typeMoq.Times.never());

pickEnvironmentManagerStub.resolves(manager.object.id);
pickProjectManyStub.resolves([project]);

const result = await createAnyEnvironmentCommand(em.object, pm.object, { selectEnvironment: false });

assert.strictEqual(result, env.object, 'Expected the created environment to match the mocked environment.');
manager.verifyAll();
em.verifyAll();
});

test('Create workspace venv: select', async () => {
pm.setup((p) => p.getProjects()).returns(() => [project]);
manager
.setup((m) => m.create([project.uri]))
.returns(() => Promise.resolve(env.object))
.verifiable(typeMoq.Times.once());

// This is a case where env managers handler does this in batch to avoid writing to files for each case
manager.setup((m) => m.set(typeMoq.It.isAny(), typeMoq.It.isAny())).verifiable(typeMoq.Times.never());
em.setup((e) => e.setEnvironments([project.uri], env.object)).verifiable(typeMoq.Times.once());

pickEnvironmentManagerStub.resolves(manager.object.id);
pickProjectManyStub.resolves([project]);

const result = await createAnyEnvironmentCommand(em.object, pm.object, { selectEnvironment: true });

assert.strictEqual(result, env.object, 'Expected the created environment to match the mocked environment.');
manager.verifyAll();
em.verifyAll();
});

test('Create multi-workspace venv: select all', async () => {
pm.setup((p) => p.getProjects()).returns(() => [project, project2, project3]);
manager
.setup((m) => m.create([project.uri, project2.uri, project3.uri]))
.returns(() => Promise.resolve(env.object))
.verifiable(typeMoq.Times.once());

// This is a case where env managers handler does this in batch to avoid writing to files for each case
manager.setup((m) => m.set(typeMoq.It.isAny(), typeMoq.It.isAny())).verifiable(typeMoq.Times.never());
em.setup((e) => e.setEnvironments([project.uri, project2.uri, project3.uri], env.object)).verifiable(
typeMoq.Times.once(),
);

pickEnvironmentManagerStub.resolves(manager.object.id);
pickProjectManyStub.resolves([project, project2, project3]);

const result = await createAnyEnvironmentCommand(em.object, pm.object, { selectEnvironment: true });

assert.strictEqual(result, env.object, 'Expected the created environment to match the mocked environment.');
manager.verifyAll();
em.verifyAll();
});

test('Create multi-workspace venv: select some', async () => {
pm.setup((p) => p.getProjects()).returns(() => [project, project2, project3]);
manager
.setup((m) => m.create([project.uri, project3.uri]))
.returns(() => Promise.resolve(env.object))
.verifiable(typeMoq.Times.once());

// This is a case where env managers handler does this in batch to avoid writing to files for each case
manager.setup((m) => m.set(typeMoq.It.isAny(), typeMoq.It.isAny())).verifiable(typeMoq.Times.never());
em.setup((e) => e.setEnvironments([project.uri, project3.uri], env.object)).verifiable(typeMoq.Times.once());

pickEnvironmentManagerStub.resolves(manager.object.id);
pickProjectManyStub.resolves([project, project3]);

const result = await createAnyEnvironmentCommand(em.object, pm.object, { selectEnvironment: true });

assert.strictEqual(result, env.object, 'Expected the created environment to match the mocked environment.');
manager.verifyAll();
em.verifyAll();
});
});