Problem
The codebase has inconsistent patterns for external dependencies (child process spawning, file system operations, settings access), making unit testing difficult. While some files use wrapper APIs from common/, others directly import from Node.js or VS Code, which requires complex stubbing in tests.
This issue is extracted from the "Additional Simplifications" section of #1159 to keep that issue focused on mock infrastructure.
Current State
Child Process Spawning
- Using wrapper (
childProcess.apis): Most managers (e.g., condaUtils.ts, nativePythonFinder.ts, helpers.ts)
- Direct import:
poetryUtils.ts still uses import * as cp from 'child_process' and promisify(cp.exec)
File System Operations
- Using wrapper (
workspace.fs.apis): Only envVarUtils.ts
- Direct import: Many files likely use direct
fs or minimal wrappers
Settings Access
- Using wrapper (
workspace.apis): Most production code (good adoption)
- Direct calls: Documentation/skills files reference
vscode.workspace.getConfiguration() directly, but production code is generally consistent
Proposed Solution
1. Abstract Child Process Spawning in poetryUtils.ts
Refactor poetryUtils.ts to use spawnProcess from childProcess.apis instead of direct child_process imports:
// Before
import * as cp from 'child_process';
const exec = promisify(cp.exec);
const { stdout } = await exec(`"${poetry}" config virtualenvs.path`);
// After
import { spawnProcess } from '../../common/childProcess.apis';
const result = await spawnProcess(poetry, ['config', 'virtualenvs.path']);
2. Audit and Consolidate File System Usage
- Audit all managers for direct
fs imports
- Migrate to
workspace.fs.apis wrappers for consistency
- Document the pattern in testing workflow instructions
3. Verify Settings Access Consistency
- Audit for any remaining direct
vscode.workspace.getConfiguration() calls in production code
- Ensure all settings access uses
workspace.apis wrappers
Acceptance Criteria
Benefits
- Consistent stubbing: All external calls go through known wrapper modules
- Easier testing: Tests can stub
childProcess.apis module instead of complex process mocking
- Better isolation: Managers become pure business logic testable without real processes/files
Related
Problem
The codebase has inconsistent patterns for external dependencies (child process spawning, file system operations, settings access), making unit testing difficult. While some files use wrapper APIs from
common/, others directly import from Node.js or VS Code, which requires complex stubbing in tests.This issue is extracted from the "Additional Simplifications" section of #1159 to keep that issue focused on mock infrastructure.
Current State
Child Process Spawning
childProcess.apis): Most managers (e.g.,condaUtils.ts,nativePythonFinder.ts,helpers.ts)poetryUtils.tsstill usesimport * as cp from 'child_process'andpromisify(cp.exec)File System Operations
workspace.fs.apis): OnlyenvVarUtils.tsfsor minimal wrappersSettings Access
workspace.apis): Most production code (good adoption)vscode.workspace.getConfiguration()directly, but production code is generally consistentProposed Solution
1. Abstract Child Process Spawning in poetryUtils.ts
Refactor
poetryUtils.tsto usespawnProcessfromchildProcess.apisinstead of directchild_processimports:2. Audit and Consolidate File System Usage
fsimportsworkspace.fs.apiswrappers for consistency3. Verify Settings Access Consistency
vscode.workspace.getConfiguration()calls in production codeworkspace.apiswrappersAcceptance Criteria
src/managers/poetry/poetryUtils.tsto usechildProcess.apiswrappersrc/managers/**files for directchild_processimportssrc/managers/**files for directfsimportsBenefits
childProcess.apismodule instead of complex process mockingRelated
src/common/childProcess.apis.ts- Child process wrappersrc/common/workspace.fs.apis.ts- File system wrappersrc/common/workspace.apis.ts- Settings/workspace wrapper