Skip to content

Commit 99f9e7c

Browse files
committed
test: Improve http server mock and assertion matching
1 parent 795780e commit 99f9e7c

File tree

3 files changed

+56
-45
lines changed

3 files changed

+56
-45
lines changed

src/lib/server.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1690,7 +1690,7 @@ export async function startProxyServer(
16901690
// const tempServerForPortCheck = http.createServer(); // Not needed if findFreePort creates its own
16911691
proxyListenPort = await findFreePort(initialPortForProxySearch); // findFreePort internally creates server for checks
16921692
logger.info(`[PROXY_DEBUG] startProxyServer: Found free port ${proxyListenPort} for proxy.`);
1693-
} catch (error: unknown) { // Ensure error is typed as unknown or any
1693+
} catch (error) { // Keep error as any or unknown for broader catch
16941694
const err = error instanceof Error ? error : new Error(String(error));
16951695
logger.error(`[ProxyServer] Failed to find free port for proxy: ${err.message}`, { errorDetails: err });
16961696
return null; // Return null if findFreePort fails
@@ -1744,7 +1744,7 @@ export async function startProxyServer(
17441744
Object.keys(error.response.headers).forEach(key => {
17451745
const headerValue = error.response!.headers[key];
17461746
// Forward common headers, especially content-type for JSON errors
1747-
if (headerValue !== undefined && ['content-type', 'content-length', 'mcp-session-id', 'cache-control', 'connection'].includes(key.toLowerCase())) {
1747+
if (headerValue !== undefined && ['content-type', 'content-length', 'mcp-session-id', 'cache-control', 'connection', 'transfer-encoding'].includes(key.toLowerCase())) {
17481748
res.setHeader(key, headerValue as string | string[]);
17491749
}
17501750
});

src/tests/index.test.ts

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -566,12 +566,9 @@ describe('CLI with yargs (index.ts)', () => {
566566
it('should handle invalid JSON parameters for client command (stdio) and log via yargs .fail()', async () => {
567567
process.env.VITEST_TESTING_FAIL_HANDLER = "true";
568568
mockConsoleError.mockClear(); // Clear before run
569-
const expectedError = new Error("Invalid JSON parameters: Expected ',' or '}' after property value in JSON at position 16");
570-
(expectedError as any).details = "Expected ',' or '}' after property value in JSON at position 16";
571569

572570
await expect(runMainWithArgs(['agent_query', '{"query": "test"'])).rejects.toThrowError(
573-
// Match the exact error message that handleClientCommand throws
574-
"Invalid JSON parameters: Expected ',' or '}' after property value in JSON at position 16"
571+
new Error("Invalid JSON parameters: Expected ',' or '}' after property value in JSON at position 16")
575572
);
576573

577574
// yargs .fail() handler logs to console.error in test mode
@@ -662,13 +659,12 @@ describe('CLI with yargs (index.ts)', () => {
662659
expect(mockStartServerHandler).toHaveBeenCalled();
663660

664661
// Check if the SUT's yargs middleware logged the port being observed.
665-
const portMiddlewareLogFound = currentMockLoggerInstance.info.mock.calls.some(call =>
666-
typeof call[0] === 'string' && call[0].includes(`[SUT_INDEX_TS_YARGS_MW] --port option value at middleware: ${customPort}`)
662+
expect(currentMockLoggerInstance.info.mock.calls).toEqual(
663+
expect.arrayContaining([
664+
[expect.stringContaining(`[SUT_INDEX_TS_YARGS_MW] --port option value at middleware: ${customPort}`)]
665+
])
667666
);
668-
expect(portMiddlewareLogFound,
669-
`Expected SUT log indicating --port ${customPort} was processed by middleware. Log calls: ${JSON.stringify(currentMockLoggerInstance.info.mock.calls)}`
670-
).toBe(true);
671-
667+
672668
// Restore original
673669
if (originalHttpPort === undefined) delete process.env.HTTP_PORT;
674670
else process.env.HTTP_PORT = originalHttpPort;
@@ -782,11 +778,13 @@ describe('CLI with yargs (index.ts)', () => {
782778
mockConsoleError.mockClear(); // Clear before run
783779
mockProcessExit.mockClear();
784780

785-
const expectedErrorPattern = /Unknown command: unknowncommand|You must provide a command to run|Not enough non-option arguments|Unknown argument: unknowncommand/;
781+
const expectedErrorPattern = /Unknown command: unknowncommand|You must provide a command to run|Not enough non-option arguments|Unknown argument: unknowncommand/; // Added Unknown argument
786782
await expect(runMainWithArgs(['unknowncommand'])).rejects.toThrowError(expectedErrorPattern);
787783

788784
const consoleErrorCalls = mockConsoleError.mock.calls.map(call => call.join(' ')).join('\n');
785+
// Check that help output was logged
789786
expect(consoleErrorCalls).toMatch(/Usage: codecompass <command> \[options\]/);
787+
// Check that the specific error message from yargs was logged by the .fail() handler's test mode path
790788
expect(consoleErrorCalls).toMatch(/YARGS_FAIL_TEST_MODE_ERROR_OUTPUT:.*(Unknown command: unknowncommand|You must provide a command to run|Not enough non-option arguments|Unknown argument: unknowncommand)/s);
791789

792790
expect(mockStartServerHandler).not.toHaveBeenCalled();
@@ -804,6 +802,7 @@ describe('CLI with yargs (index.ts)', () => {
804802

805803
const consoleErrorCalls = mockConsoleError.mock.calls.map(call => call.join(' ')).join('\n');
806804
expect(consoleErrorCalls).toMatch(/Usage: codecompass <command> \[options\]/);
805+
// Check that the specific error message from yargs was logged by the .fail() handler's test mode path
807806
expect(consoleErrorCalls).toMatch(/YARGS_FAIL_TEST_MODE_ERROR_OUTPUT:.*(Unknown argument: --unknown-option|Unknown arguments: unknown-option, unknownOption)/s);
808807

809808
expect(mockStartServerHandler).not.toHaveBeenCalled();
@@ -882,7 +881,10 @@ describe('CLI with yargs (index.ts)', () => {
882881

883882
process.env.VITEST_TESTING_FAIL_HANDLER = "true";
884883
await expect(runMainWithArgs(['agent_query', '{"query":"test_json_rpc_error"}', '--json']))
885-
.rejects.toThrow(errorThrownByHandleClient);
884+
.rejects.toThrow(expect.objectContaining({ // Check for an object that contains the jsonRpcError
885+
message: `Tool 'agent_query' failed: ${rpcErrorObject.message}`,
886+
jsonRpcError: rpcErrorObject
887+
}));
886888

887889
// SUT's handleClientCommand logs the JSON error to console.error when --json is active
888890
// It logs the error object passed to it, which now includes jsonRpcError
@@ -902,7 +904,7 @@ describe('CLI with yargs (index.ts)', () => {
902904
// yargs .fail() handler also logs
903905
const yargsFailOutputLogged = mockConsoleError.mock.calls.some(call =>
904906
call[0] === 'YARGS_FAIL_TEST_MODE_ERROR_OUTPUT:' &&
905-
call[1] === errorThrownByHandleClient.message
907+
typeof call[1] === 'string' && call[1].includes(`Tool 'agent_query' failed: ${rpcErrorObject.message}`)
906908
);
907909
expect(yargsFailOutputLogged).toBe(true);
908910
expect(mockProcessExit).not.toHaveBeenCalled();
@@ -931,7 +933,7 @@ describe('CLI with yargs (index.ts)', () => {
931933
// yargs .fail() handler also logs
932934
const yargsFailOutputLogged = mockConsoleError.mock.calls.some(call =>
933935
call[0] === 'YARGS_FAIL_TEST_MODE_ERROR_OUTPUT:' &&
934-
call[1] === genericError.message
936+
typeof call[1] === 'string' && call[1] === genericError.message
935937
);
936938
expect(yargsFailOutputLogged).toBe(true);
937939
expect(mockProcessExit).not.toHaveBeenCalled();

src/tests/server.test.ts

Lines changed: 38 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1355,14 +1355,15 @@ describe('startProxyServer', () => {
13551355
nock.enableNetConnect((host) => host.startsWith('127.0.0.1') || host.startsWith('localhost'));
13561356

13571357
// Ensure the http.createServer mock used by startProxyServer behaves asynchronously for listen and close
1358+
// This mock is specific to the 'startProxyServer' describe block.
13581359
vi.mocked(http.createServer).mockImplementation(() => {
13591360
const serverInstance = {
1360-
_listeners: {} as Record<string, ((...args: any[]) => void) | undefined>,
1361-
_port: null as number | null, // Add _port to store the listening port
1362-
_host: null as string | null, // Add _host to store the listening host
1361+
_listeners: {} as Record<string, ((...args: any[]) => void) | ((...args: any[]) => void)[] | undefined>,
1362+
_port: null as number | null,
1363+
_host: null as string | null,
13631364
listen: vi.fn(function(this: any, portOrPathOrOptions: any, hostOrCb?: any, backlogOrCb?: any, cb?: any) {
13641365
let portToListen: number | undefined;
1365-
let hostToListen: string | undefined = 'localhost'; // Default host
1366+
let hostToListen: string | undefined = '127.0.0.1'; // Default host for listen
13661367
let actualCallback: (() => void) | undefined;
13671368

13681369
if (typeof portOrPathOrOptions === 'number') {
@@ -1374,7 +1375,7 @@ describe('startProxyServer', () => {
13741375
} else if (typeof hostOrCb === 'function') {
13751376
actualCallback = hostOrCb;
13761377
}
1377-
} else if (typeof portOrPathOrOptions === 'object' && portOrPathOrOptions !== null) { // Options object
1378+
} else if (typeof portOrPathOrOptions === 'object' && portOrPathOrOptions !== null) {
13781379
portToListen = (portOrPathOrOptions as import('net').ListenOptions).port;
13791380
hostToListen = (portOrPathOrOptions as import('net').ListenOptions).host || hostToListen;
13801381
if (typeof hostOrCb === 'function') actualCallback = hostOrCb;
@@ -1383,59 +1384,67 @@ describe('startProxyServer', () => {
13831384
this._port = portToListen ?? null;
13841385
this._host = hostToListen ?? null;
13851386

1386-
process.nextTick(() => { // Simulate async listen
1387+
process.nextTick(() => {
13871388
if (this._listeners && typeof this._listeners.listening === 'function') {
1388-
this._listeners.listening();
1389-
}
1390-
if (actualCallback) {
1391-
actualCallback();
1389+
(this._listeners.listening as () => void)();
1390+
} else if (Array.isArray(this._listeners?.listening)) {
1391+
(this._listeners.listening as ((...args: any[]) => void)[]).forEach(fn => fn());
13921392
}
1393+
if (actualCallback) actualCallback();
13931394
});
13941395
return this;
13951396
}),
13961397
on: vi.fn(function(this: any, event: string, callback: (...args: any[]) => void) {
1397-
if (!this._listeners[event]) this._listeners[event] = [];
1398-
this._listeners[event].push(callback); // Support multiple listeners if needed, though emit below is simple
1399-
// For simplicity, if 'error' or 'listening' is set, we'll just use the last one for emit.
1400-
// A full EventEmitter mock would handle arrays of listeners.
1401-
// For this test's purpose, storing the last one for 'error' and 'listening' is likely sufficient.
1402-
if (event === 'error' || event === 'listening' || event === 'close') {
1403-
this._listeners[event] = callback; // Overwrite for simplicity for these key events
1398+
if (!this._listeners[event]) {
1399+
this._listeners[event] = [];
1400+
}
1401+
if (Array.isArray(this._listeners[event])) {
1402+
(this._listeners[event] as ((...args: any[]) => void)[]).push(callback);
1403+
} else { // If it was a single function (e.g. from 'once'), make it an array
1404+
this._listeners[event] = [this._listeners[event] as (...args: any[]) => void, callback];
14041405
}
14051406
return this;
14061407
}),
14071408
once: vi.fn(function(this: any, event: string, callback: (...args: any[]) => void) {
1408-
// A simple once: store it, and if emit is called, it will be invoked then cleared.
1409-
// For findFreePort, it uses 'once' for 'error' and 'listening'.
1410-
this._listeners[event] = (...args: any[]) => {
1411-
delete this._listeners[event]; // Remove after one call
1409+
const onceWrapper = (...args: any[]) => {
1410+
if (Array.isArray(this._listeners[event])) {
1411+
this._listeners[event] = (this._listeners[event] as ((...args: any[]) => void)[]).filter(fn => fn !== onceWrapper);
1412+
} else if (this._listeners[event] === onceWrapper) {
1413+
delete this._listeners[event];
1414+
}
14121415
callback(...args);
14131416
};
1417+
this.on(event, onceWrapper); // Use the 'on' method to add it
14141418
return this;
14151419
}),
1416-
address: vi.fn(function(this: any) { // Use function for 'this'
1420+
address: vi.fn(function(this: any) {
14171421
if (this._port !== null) {
14181422
return { port: this._port, address: this._host || '127.0.0.1', family: 'IPv4' };
14191423
}
14201424
return null;
14211425
}),
14221426
close: vi.fn(function(this: any, cb?: (err?: Error) => void) {
1423-
process.nextTick(() => { // Simulate async close
1427+
process.nextTick(() => {
14241428
if (this._listeners && typeof this._listeners.close === 'function') {
1425-
this._listeners.close();
1429+
(this._listeners.close as () => void)();
1430+
} else if (Array.isArray(this._listeners?.close)) {
1431+
(this._listeners.close as ((...args: any[]) => void)[]).forEach(fn => fn());
14261432
}
14271433
if (cb) cb();
14281434
});
14291435
return this;
14301436
}),
14311437
removeAllListeners: vi.fn().mockReturnThis(),
1432-
emit: vi.fn(function(this: any, event: string, ...args: any[]) { // Basic emit for testing
1433-
if (this._listeners && typeof this._listeners[event] === 'function') {
1434-
this._listeners[event](...args);
1438+
emit: vi.fn(function(this: any, event: string, ...args: any[]) {
1439+
const listeners = this._listeners?.[event];
1440+
if (typeof listeners === 'function') {
1441+
listeners(...args);
1442+
} else if (Array.isArray(listeners)) {
1443+
listeners.forEach(fn => fn(...args));
14351444
}
14361445
}),
14371446
};
1438-
return serverInstance as unknown as http.Server; // Cast to http.Server
1447+
return serverInstance as unknown as http.Server;
14391448
});
14401449
});
14411450

@@ -1501,7 +1510,7 @@ describe('startProxyServer', () => {
15011510
expect(actualProxyListenPort).toBe(proxyListenPort);
15021511

15031512
// Check for key log messages with more flexibility
1504-
const infoCalls = ml.info.mock.calls.map(call => call[0]);
1513+
const infoCalls = ml.info.mock.calls.map(call => String(call[0])); // Ensure string for matching
15051514
expect(infoCalls).toEqual(expect.arrayContaining([
15061515
expect.stringContaining(`Original CodeCompass server (v1.0.0-existing) is running on port ${targetExistingServerPort}`),
15071516
expect.stringContaining(`This instance (CodeCompass Proxy) is listening on port ${actualProxyListenPort}`),

0 commit comments

Comments
 (0)