-
Notifications
You must be signed in to change notification settings - Fork 38
Open
Labels
Description
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.tsstill usesimport * as cp from 'child_process'andpromisify(cp.exec)
File System Operations
- Using wrapper (
workspace.fs.apis): OnlyenvVarUtils.ts - Direct import: Many files likely use direct
fsor 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
fsimports - Migrate to
workspace.fs.apiswrappers 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.apiswrappers
Acceptance Criteria
- Refactor
src/managers/poetry/poetryUtils.tsto usechildProcess.apiswrapper - Audit all
src/managers/**files for directchild_processimports - Audit all
src/managers/**files for directfsimports - Document wrapper patterns in testing workflow instructions
- Add unit tests demonstrating the mocking pattern for each wrapper
Benefits
- Consistent stubbing: All external calls go through known wrapper modules
- Easier testing: Tests can stub
childProcess.apismodule instead of complex process mocking - Better isolation: Managers become pure business logic testable without real processes/files
Related
- Parent issue: Improve testability of environment managers by creating a mock/fake NativePythonFinder #1159 (mock NativePythonFinder infrastructure)
src/common/childProcess.apis.ts- Child process wrappersrc/common/workspace.fs.apis.ts- File system wrappersrc/common/workspace.apis.ts- Settings/workspace wrapper
Reactions are currently unavailable