mirror of
https://github.com/tiennm99/ccs.git
synced 2026-10-13 12:14:19 +00:00
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:
1 parent
8ccf193fd4
commit
a7871283d4
5 files changed
+73
-85
No files matched your search
@@ -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;
|
||||||
|
|
||||||
|
|||||||
@@ -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,
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -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', () => ({
|
|
||||||
ensureCLIProxyBinary: async (_verbose = false, options?: unknown) => {
|
|
||||||
ensureBinaryCalls.push(options);
|
|
||||||
throw new Error(
|
|
||||||
'CLIProxy Plus binary is not installed locally. Run "ccs cliproxy install" when you have network access.'
|
|
||||||
);
|
|
||||||
},
|
|
||||||
}));
|
|
||||||
|
|
||||||
mock.module('../../../src/cliproxy/config-generator', () => ({
|
|
||||||
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', () => {
|
describe('ensureCliproxyService', () => {
|
||||||
it('fails fast without attempting a runtime install when the local binary is missing', async () => {
|
it('fails fast without attempting a runtime install when the local binary is missing', async () => {
|
||||||
ensureBinaryCalls.length = 0;
|
// 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);
|
const result = await ensureCliproxyService(8317, false, {
|
||||||
|
ensureBinaryFn: async (_verbose, options) => {
|
||||||
|
ensureBinaryCalls.push(options);
|
||||||
|
throw new Error(
|
||||||
|
'CLIProxy Plus binary is not installed locally. Run "ccs cliproxy install" when you have network access.'
|
||||||
|
);
|
||||||
|
},
|
||||||
|
detectRunningProxyFn: async () => ({ running: false, verified: false }),
|
||||||
|
configNeedsRegenerationFn: () => false,
|
||||||
|
withStartupLockFn: async (fn) => await fn(),
|
||||||
|
});
|
||||||
|
|
||||||
expect(result).toEqual({
|
expect(result).toEqual({
|
||||||
started: false,
|
started: false,
|
||||||
|
|||||||
@@ -91,33 +91,27 @@ 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', () => ({
|
checkForUpdatesCalls += 1;
|
||||||
checkForUpdates: async () => {
|
return {
|
||||||
checkForUpdatesCalls += 1;
|
hasUpdate: false,
|
||||||
return {
|
currentVersion: '6.8.2-0',
|
||||||
hasUpdate: false,
|
latestVersion: '6.8.2-0',
|
||||||
currentVersion: '6.8.2-0',
|
fromCache: false,
|
||||||
latestVersion: '6.8.2-0',
|
checkedAt: Date.now(),
|
||||||
fromCache: false,
|
};
|
||||||
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')));
|
||||||
|
|||||||
Reference in new issue
Block a user