From 6b91d5d2ae9a949303aef503bb94f08ba71bf894 Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Sun, 15 Feb 2026 19:55:26 -0800 Subject: [PATCH] chore: add execProcess wrapper and refactor poetryUtils (Fixes #1239) --- src/common/childProcess.apis.ts | 31 +++++++++++ src/managers/poetry/poetryUtils.ts | 13 ++--- .../managers/poetry/poetryUtils.unit.test.ts | 55 ++++++++++++++++++- 3 files changed, 88 insertions(+), 11 deletions(-) diff --git a/src/common/childProcess.apis.ts b/src/common/childProcess.apis.ts index 7e7d1f4e..4decffea 100644 --- a/src/common/childProcess.apis.ts +++ b/src/common/childProcess.apis.ts @@ -1,4 +1,35 @@ import * as cp from 'child_process'; +import { promisify } from 'util'; + +const cpExec = promisify(cp.exec); + +/** + * Result of execProcess - contains stdout and stderr as strings. + */ +export interface ExecResult { + stdout: string; + stderr: string; +} + +/** + * Executes a command and returns the result as a promise. + * This function abstracts cp.exec to make it easier to mock in tests. + * + * @param command The command to execute (can include arguments). + * @param options Optional execution options. + * @returns A promise that resolves with { stdout, stderr } strings. + */ +export async function execProcess(command: string, options?: cp.ExecOptions): Promise { + const env = { + PYTHONUTF8: '1', + ...(options?.env ?? process.env), + }; + const result = await cpExec(command, { ...options, env }); + return { + stdout: result.stdout ?? '', + stderr: result.stderr ?? '', + }; +} /** * Spawns a new process using the specified command and arguments. diff --git a/src/managers/poetry/poetryUtils.ts b/src/managers/poetry/poetryUtils.ts index 217f5885..bac3d40e 100644 --- a/src/managers/poetry/poetryUtils.ts +++ b/src/managers/poetry/poetryUtils.ts @@ -3,6 +3,7 @@ import * as path from 'path'; import { Uri } from 'vscode'; import which from 'which'; import { EnvironmentManager, PythonEnvironment, PythonEnvironmentApi, PythonEnvironmentInfo } from '../../api'; +import { execProcess } from '../../common/childProcess.apis'; import { ENVS_EXTENSION_ID } from '../../common/constants'; import { traceError, traceInfo } from '../../common/logging'; import { getWorkspacePersistentState } from '../../common/persistentState'; @@ -190,7 +191,7 @@ export async function getPoetryVirtualenvsPath(poetryExe?: string): Promise { try { - const { stdout } = await exec(`"${poetry}" --version`); + const { stdout } = await execProcess(`"${poetry}" --version`); // Handle both formats: // Old: "Poetry version 1.5.1" // New: "Poetry (version 2.1.3)" traceInfo(`Poetry version output: ${stdout.trim()}`); - const match = stdout.match(/Poetry (?:version|[\(\s]+version[\s\)]+)([0-9]+\.[0-9]+\.[0-9]+)/i); + const match = stdout.match(/Poetry (?:version |[\(\s]+version[\s\)]+)([0-9]+\.[0-9]+\.[0-9]+)/i); return match ? match[1] : undefined; } catch { return undefined; diff --git a/src/test/managers/poetry/poetryUtils.unit.test.ts b/src/test/managers/poetry/poetryUtils.unit.test.ts index c57bf557..e1bd44e0 100644 --- a/src/test/managers/poetry/poetryUtils.unit.test.ts +++ b/src/test/managers/poetry/poetryUtils.unit.test.ts @@ -1,9 +1,14 @@ import assert from 'node:assert'; import * as sinon from 'sinon'; -import { isPoetryVirtualenvsInProject, nativeToPythonEnv } from '../../../managers/poetry/poetryUtils'; -import * as utils from '../../../managers/common/utils'; import { EnvironmentManager, PythonEnvironment, PythonEnvironmentApi, PythonEnvironmentInfo } from '../../../api'; +import * as childProcessApis from '../../../common/childProcess.apis'; import { NativeEnvInfo } from '../../../managers/common/nativePythonFinder'; +import * as utils from '../../../managers/common/utils'; +import { + getPoetryVersion, + isPoetryVirtualenvsInProject, + nativeToPythonEnv, +} from '../../../managers/poetry/poetryUtils'; suite('isPoetryVirtualenvsInProject', () => { test('should return false when env var is not set', () => { @@ -157,3 +162,49 @@ suite('nativeToPythonEnv - POETRY_VIRTUALENVS_IN_PROJECT integration', () => { assert.strictEqual(capturedInfo!.group, undefined, 'Non-global path should not be global'); }); }); + +suite('getPoetryVersion - childProcess.apis mocking pattern', () => { + let execProcessStub: sinon.SinonStub; + + setup(() => { + execProcessStub = sinon.stub(childProcessApis, 'execProcess'); + }); + + teardown(() => { + sinon.restore(); + }); + + test('should parse Poetry 1.x version format', async () => { + execProcessStub.resolves({ stdout: 'Poetry version 1.5.1\n', stderr: '' }); + + const version = await getPoetryVersion('/usr/bin/poetry'); + + assert.strictEqual(version, '1.5.1'); + assert.ok(execProcessStub.calledOnce); + assert.ok(execProcessStub.calledWith('"/usr/bin/poetry" --version')); + }); + + test('should parse Poetry 2.x version format', async () => { + execProcessStub.resolves({ stdout: 'Poetry (version 2.1.3)\n', stderr: '' }); + + const version = await getPoetryVersion('/usr/bin/poetry'); + + assert.strictEqual(version, '2.1.3'); + }); + + test('should return undefined when command fails', async () => { + execProcessStub.rejects(new Error('Command not found')); + + const version = await getPoetryVersion('/nonexistent/poetry'); + + assert.strictEqual(version, undefined); + }); + + test('should return undefined when output does not match expected format', async () => { + execProcessStub.resolves({ stdout: 'unexpected output', stderr: '' }); + + const version = await getPoetryVersion('/usr/bin/poetry'); + + assert.strictEqual(version, undefined); + }); +});