test(cliproxy): replace process-wide mock.module with DI seams

Bun's `mock.module()` is process-wide and is NOT undone by `mock.restore()`.
Two test files used module-level mocks that leaked across test runs,
silently breaking unrelated suites that imported the mocked modules
transitively:

- tests/unit/cliproxy/version-checker-stale-cache.test.ts mocked
  binary/version-checker, contaminating cliproxy-stats-routes-install
  and cliproxy-stats-routes-model-update suites that import a route
  module which transitively imports version-checker.
- tests/unit/cliproxy/service-manager-startup.test.ts mocked 8 modules
  at top level (binary-manager, stats-fetcher, etc.), contaminating any
  later file that imports them.

CI happens to be green because file ordering on the GitHub runner places
victim files BEFORE contaminators in `bun test`. On Linux locally the
order is reversed, producing 6 reproducible test failures on every
`bun run test:fast` run. A change in OS, bun version, or new test files
that import the mocked modules could silently flip the CI runner's
order and surface these failures unexpectedly.

Replaced module-level mocks with explicit dependency-injection seams in
production code, following the existing pattern in version-checker.ts
and version-cache.ts (`fetchJsonFn?`, `fetchLatestVersionFn?`):

- src/cliproxy/types.ts: `BinaryManagerConfig.checkForUpdatesFn` (optional)
- src/cliproxy/binary/lifecycle.ts: route through new fn with default
- src/cliproxy/service-manager.ts: `ensureCliproxyService(deps?)` with
  optional `ensureBinaryFn`, `detectRunningProxyFn`,
  `configNeedsRegenerationFn`, `withStartupLockFn`

All deps fields are optional with real-implementation defaults, so
production callers are unchanged. Tests now inject deterministic stubs
through call-site parameters instead of module-level mocks.

Verified locally: \`bun run validate\` is now 2540 pass / 0 fail
(previously 2534 pass / 6 fail at the same upstream/main HEAD).

Built [OnSteroids](https://onsteroids.ai)
This commit is contained in:
Sergey Galuza committed 2026-04-26 09:43:09 +02:00
1 parent 8ccf193fd4
commit a7871283d4
5 files changed
+63 -75

No files matched your search

+2 -1
View File
@@ -57,7 +57,8 @@ function clampToMaxStable(version: string | undefined, verbose: boolean): string
async function handleAutoUpdate(config: BinaryManagerConfig, verbose: boolean): Promise<void> { async function handleAutoUpdate(config: BinaryManagerConfig, verbose: boolean): Promise<void> {
const backend: CLIProxyBackend = config.backend ?? DEFAULT_BACKEND; const backend: CLIProxyBackend = config.backend ?? DEFAULT_BACKEND;
const backendLabel = getBackendLabel(backend); const backendLabel = getBackendLabel(backend);
const updateResult = await checkForUpdates(config.binPath, config.version, verbose, backend); const checkFn = config.checkForUpdatesFn ?? checkForUpdates;
const updateResult = await checkFn(config.binPath, config.version, verbose, backend);
const currentVersion = updateResult.currentVersion; const currentVersion = updateResult.currentVersion;
const latestVersion = updateResult.latestVersion; const latestVersion = updateResult.latestVersion;
+23 -5
View File
@@ -138,6 +138,18 @@ export interface ServiceStartResult {
error?: string; error?: string;
} }
/**
* Test-only seams for ensureCliproxyService. Production callers omit this and
* get the real implementations. Tests inject stubs to avoid bun's
* `mock.module()`, which is process-wide and leaks across test files.
*/
export interface EnsureCliproxyServiceDeps {
ensureBinaryFn?: typeof ensureCLIProxyBinary;
detectRunningProxyFn?: typeof detectRunningProxy;
configNeedsRegenerationFn?: typeof configNeedsRegeneration;
withStartupLockFn?: typeof withStartupLock;
}
/** /**
* Ensure CLIProxy service is running * Ensure CLIProxy service is running
* *
@@ -146,12 +158,18 @@ export interface ServiceStartResult {
* *
* @param port CLIProxy port (default: 8317) * @param port CLIProxy port (default: 8317)
* @param verbose Show debug output * @param verbose Show debug output
* @param deps Test-only dependency overrides (see EnsureCliproxyServiceDeps)
* @returns Result indicating success and whether it was already running * @returns Result indicating success and whether it was already running
*/ */
export async function ensureCliproxyService( export async function ensureCliproxyService(
port: number = CLIPROXY_DEFAULT_PORT, port: number = CLIPROXY_DEFAULT_PORT,
verbose: boolean = false verbose: boolean = false,
deps: EnsureCliproxyServiceDeps = {}
): Promise<ServiceStartResult> { ): Promise<ServiceStartResult> {
const ensureBinaryFn = deps.ensureBinaryFn ?? ensureCLIProxyBinary;
const detectRunningProxyFn = deps.detectRunningProxyFn ?? detectRunningProxy;
const configNeedsRegenerationFn = deps.configNeedsRegenerationFn ?? configNeedsRegeneration;
const withStartupLockFn = deps.withStartupLockFn ?? withStartupLock;
const log = (msg: string) => { const log = (msg: string) => {
if (verbose) { if (verbose) {
console.error(`[cliproxy-service] ${msg}`); console.error(`[cliproxy-service] ${msg}`);
@@ -160,17 +178,17 @@ export async function ensureCliproxyService(
// Check if config needs update (even if running) // Check if config needs update (even if running)
let configRegenerated = false; let configRegenerated = false;
if (configNeedsRegeneration()) { if (configNeedsRegenerationFn()) {
log('Config outdated, regenerating...'); log('Config outdated, regenerating...');
regenerateConfig(port); regenerateConfig(port);
configRegenerated = true; configRegenerated = true;
} }
// Use startup lock to coordinate with other CCS processes (ccs agy, ccs config, etc.) // Use startup lock to coordinate with other CCS processes (ccs agy, ccs config, etc.)
return await withStartupLock(async () => { return await withStartupLockFn(async () => {
// Use unified detection (HTTP check + session-lock + port-process) // Use unified detection (HTTP check + session-lock + port-process)
log(`Checking if CLIProxy is running on port ${port}...`); log(`Checking if CLIProxy is running on port ${port}...`);
const proxyStatus = await detectRunningProxy(port); const proxyStatus = await detectRunningProxyFn(port);
log(`Proxy detection: ${JSON.stringify(proxyStatus)}`); log(`Proxy detection: ${JSON.stringify(proxyStatus)}`);
if (proxyStatus.running && proxyStatus.verified) { if (proxyStatus.running && proxyStatus.verified) {
@@ -216,7 +234,7 @@ export async function ensureCliproxyService(
// 1. Ensure binary exists // 1. Ensure binary exists
let binaryPath: string; let binaryPath: string;
try { try {
binaryPath = await ensureCLIProxyBinary(verbose, { binaryPath = await ensureBinaryFn(verbose, {
allowInstall: false, allowInstall: false,
skipAutoUpdate: true, skipAutoUpdate: true,
}); });
+13
View File
@@ -4,6 +4,7 @@
*/ */
import type { CompositeTierConfig } from '../config/unified-config-types'; import type { CompositeTierConfig } from '../config/unified-config-types';
import type { UpdateCheckResult } from './binary/types';
/** /**
* Supported operating systems * Supported operating systems
@@ -56,6 +57,18 @@ export interface BinaryManagerConfig {
allowInstall: boolean; allowInstall: boolean;
/** Backend variant (original vs plus) */ /** Backend variant (original vs plus) */
backend?: CLIProxyBackend; backend?: CLIProxyBackend;
/**
* Test-only seam: override the auto-update check. When omitted, the real
* `checkForUpdates` from ./binary/version-checker is used. Provided so tests
* can verify "skipAutoUpdate respected" without resorting to bun's
* `mock.module()`, which leaks across test files in the same process.
*/
checkForUpdatesFn?: (
binPath: string,
configVersion: string,
verbose?: boolean,
backend?: CLIProxyBackend
) => Promise<UpdateCheckResult>;
} }
/** /**
@@ -1,65 +1,26 @@
import { describe, expect, it, mock } from 'bun:test'; import { describe, expect, it } from 'bun:test';
const ensureBinaryCalls: Array<unknown> = []; import { ensureCliproxyService } from '../../../src/cliproxy/service-manager';
mock.module('../../../src/cliproxy/binary-manager', () => ({ describe('ensureCliproxyService', () => {
ensureCLIProxyBinary: async (_verbose = false, options?: unknown) => { it('fails fast without attempting a runtime install when the local binary is missing', async () => {
// Inject stubs via the deps seam instead of bun's `mock.module()`. The
// module-level mock approach is process-wide and was previously leaking
// stubbed binary-manager / stats-fetcher modules into unrelated test
// suites (notably cliproxy-stats-routes-*) that import them transitively.
const ensureBinaryCalls: Array<unknown> = [];
const result = await ensureCliproxyService(8317, false, {
ensureBinaryFn: async (_verbose, options) => {
ensureBinaryCalls.push(options); ensureBinaryCalls.push(options);
throw new Error( throw new Error(
'CLIProxy Plus binary is not installed locally. Run "ccs cliproxy install" when you have network access.' 'CLIProxy Plus binary is not installed locally. Run "ccs cliproxy install" when you have network access.'
); );
}, },
})); detectRunningProxyFn: async () => ({ running: false, verified: false }),
configNeedsRegenerationFn: () => false,
mock.module('../../../src/cliproxy/config-generator', () => ({ withStartupLockFn: async (fn) => await fn(),
ensureConfigDir: () => undefined, });
generateConfig: () => '/tmp/cliproxy-config.yaml',
regenerateConfig: () => '/tmp/cliproxy-config.yaml',
configNeedsRegeneration: () => false,
CLIPROXY_DEFAULT_PORT: 8317,
getCliproxyWritablePath: () => '/tmp',
}));
mock.module('../../../src/cliproxy/proxy-detector', () => ({
detectRunningProxy: async () => ({ running: false, verified: false }),
waitForProxyHealthy: async () => false,
}));
mock.module('../../../src/cliproxy/startup-lock', () => ({
withStartupLock: async <T>(fn: () => Promise<T>) => await fn(),
}));
mock.module('../../../src/cliproxy/session-tracker', () => ({
registerSession: () => undefined,
}));
mock.module('../../../src/cliproxy/stats-fetcher', () => ({
isCliproxyRunning: async () => false,
}));
mock.module('../../../src/cliproxy/auth/token-refresh-config', () => ({
getTokenRefreshConfig: () => null,
}));
mock.module('../../../src/cliproxy/auth/token-refresh-worker', () => ({
TokenRefreshWorker: class {
isActive(): boolean {
return false;
}
start(): void {}
stop(): void {}
},
}));
const { ensureCliproxyService } = await import(
`../../../src/cliproxy/service-manager?service-manager-startup=${Date.now()}`
);
describe('ensureCliproxyService', () => {
it('fails fast without attempting a runtime install when the local binary is missing', async () => {
ensureBinaryCalls.length = 0;
const result = await ensureCliproxyService(8317, false);
expect(result).toEqual({ expect(result).toEqual({
started: false, started: false,
@@ -91,14 +91,18 @@ describe('version-checker stale cache fallback', () => {
it('skips update lookups when runtime startup prefers the installed binary', async () => { it('skips update lookups when runtime startup prefers the installed binary', async () => {
const { getExecutableName } = await import('../../../src/cliproxy/platform-detector'); const { getExecutableName } = await import('../../../src/cliproxy/platform-detector');
const { ensureBinary } = await import('../../../src/cliproxy/binary/lifecycle');
const plusBinDir = path.join(tempHome, '.ccs', 'cliproxy', 'bin', 'plus'); const plusBinDir = path.join(tempHome, '.ccs', 'cliproxy', 'bin', 'plus');
fs.mkdirSync(plusBinDir, { recursive: true }); fs.mkdirSync(plusBinDir, { recursive: true });
fs.writeFileSync(path.join(plusBinDir, getExecutableName('plus')), 'binary'); fs.writeFileSync(path.join(plusBinDir, getExecutableName('plus')), 'binary');
// Verify the contract via dependency injection rather than mock.module().
// bun's mock.module() is process-wide and is NOT undone by mock.restore(),
// which previously leaked a stubbed version-checker into unrelated test
// files (cliproxy-stats-routes-*) that transitively import it.
let checkForUpdatesCalls = 0; let checkForUpdatesCalls = 0;
const checkForUpdatesSpy = async () => {
mock.module('../../../src/cliproxy/binary/version-checker', () => ({
checkForUpdates: async () => {
checkForUpdatesCalls += 1; checkForUpdatesCalls += 1;
return { return {
hasUpdate: false, hasUpdate: false,
@@ -107,17 +111,7 @@ describe('version-checker stale cache fallback', () => {
fromCache: false, fromCache: false,
checkedAt: Date.now(), checkedAt: Date.now(),
}; };
}, };
fetchLatestVersion: async () => {
throw new Error('fetchLatestVersion should not run when skipAutoUpdate is enabled');
},
isNewerVersion: () => false,
isVersionFaulty: () => false,
}));
const { ensureBinary } = await import(
`../../../src/cliproxy/binary/lifecycle?skip-auto-update=${Date.now()}`
);
const binaryPath = await ensureBinary({ const binaryPath = await ensureBinary({
version: '6.8.2-0', version: '6.8.2-0',
@@ -129,6 +123,7 @@ describe('version-checker stale cache fallback', () => {
skipAutoUpdate: true, skipAutoUpdate: true,
allowInstall: true, allowInstall: true,
backend: 'plus', backend: 'plus',
checkForUpdatesFn: checkForUpdatesSpy,
}); });
expect(binaryPath).toBe(path.join(plusBinDir, getExecutableName('plus'))); expect(binaryPath).toBe(path.join(plusBinDir, getExecutableName('plus')));