Skip to content

Consolidate external dependency wrappers for improved testability #1239

@karthiknadig

Description

@karthiknadig

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

  • Refactor src/managers/poetry/poetryUtils.ts to use childProcess.apis wrapper
  • Audit all src/managers/** files for direct child_process imports
  • Audit all src/managers/** files for direct fs imports
  • Document wrapper patterns in testing workflow instructions
  • Add unit tests demonstrating the mocking pattern for each wrapper

Benefits

  1. Consistent stubbing: All external calls go through known wrapper modules
  2. Easier testing: Tests can stub childProcess.apis module instead of complex process mocking
  3. Better isolation: Managers become pure business logic testable without real processes/files

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions