Skip to content

Commit 087ee69

Browse files
wishket-pjwwishket-pjw
committed
test: expand test coverage for MCP services
- Add tests for mcp.controller (SSE transport handling) - Add error handling tests for mcp.service - Add comprehensive tests for rules.service close #41
1 parent e4122b3 commit 087ee69

3 files changed

Lines changed: 705 additions & 0 deletions

File tree

‎mcp-server/src/mcp/mcp.controller.spec.ts‎

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
import { describe, it, expect, vi, beforeEach } from 'vitest';
2+
import { Response, Request } from 'express';
3+
4+
// Hoist mock variables and class
5+
const { mockHandlePostMessage, MockSSEServerTransport } = vi.hoisted(() => {
6+
const mockHandlePostMessage = vi.fn();
7+
8+
class MockSSEServerTransport {
9+
constructor(
10+
public endpoint: string,
11+
public response: unknown,
12+
) {}
13+
handlePostMessage = mockHandlePostMessage;
14+
}
15+
16+
return { mockHandlePostMessage, MockSSEServerTransport };
17+
});
18+
19+
vi.mock('@modelcontextprotocol/sdk/server/sse.js', () => ({
20+
SSEServerTransport: MockSSEServerTransport,
21+
}));
22+
23+
// Import after mocks
24+
import { McpController } from './mcp.controller';
25+
import { McpService } from './mcp.service';
26+
27+
describe('McpController', () => {
28+
let controller: McpController;
29+
let mockMcpService: Partial<McpService>;
30+
let mockResponse: Partial<Response>;
31+
let mockRequest: Partial<Request>;
32+
let mockConnect: ReturnType<typeof vi.fn>;
33+
34+
beforeEach(() => {
35+
vi.clearAllMocks();
36+
37+
mockConnect = vi.fn().mockResolvedValue(undefined);
38+
mockMcpService = {
39+
getServer: vi.fn().mockReturnValue({
40+
connect: mockConnect,
41+
}),
42+
};
43+
44+
mockResponse = {
45+
status: vi.fn().mockReturnThis(),
46+
send: vi.fn().mockReturnThis(),
47+
};
48+
49+
mockRequest = {
50+
body: {},
51+
headers: {},
52+
};
53+
54+
controller = new McpController(mockMcpService as McpService);
55+
});
56+
57+
describe('handleSse', () => {
58+
it('should create SSEServerTransport with correct endpoint', async () => {
59+
await controller.handleSse(mockResponse as Response);
60+
61+
// Access private transport via type assertion
62+
const transport = (
63+
controller as unknown as {
64+
transport: InstanceType<typeof MockSSEServerTransport>;
65+
}
66+
).transport;
67+
expect(transport).toBeInstanceOf(MockSSEServerTransport);
68+
expect(transport.endpoint).toBe('/messages');
69+
expect(transport.response).toBe(mockResponse);
70+
});
71+
72+
it('should connect transport to MCP server', async () => {
73+
await controller.handleSse(mockResponse as Response);
74+
75+
expect(mockMcpService.getServer).toHaveBeenCalled();
76+
expect(mockConnect).toHaveBeenCalled();
77+
});
78+
79+
it('should replace existing transport on new connection', async () => {
80+
// First connection
81+
await controller.handleSse(mockResponse as Response);
82+
const firstTransport = (
83+
controller as unknown as {
84+
transport: InstanceType<typeof MockSSEServerTransport>;
85+
}
86+
).transport;
87+
88+
// Second connection (should replace)
89+
const newMockResponse: Partial<Response> = {
90+
status: vi.fn().mockReturnThis(),
91+
send: vi.fn().mockReturnThis(),
92+
};
93+
await controller.handleSse(newMockResponse as Response);
94+
95+
const secondTransport = (
96+
controller as unknown as {
97+
transport: InstanceType<typeof MockSSEServerTransport>;
98+
}
99+
).transport;
100+
101+
expect(secondTransport).not.toBe(firstTransport);
102+
expect(secondTransport.response).toBe(newMockResponse);
103+
});
104+
});
105+
106+
describe('handleMessages', () => {
107+
it('should return 400 when no active SSE connection', async () => {
108+
// No SSE connection established
109+
await controller.handleMessages(
110+
mockRequest as Request,
111+
mockResponse as Response,
112+
);
113+
114+
expect(mockResponse.status).toHaveBeenCalledWith(400);
115+
expect(mockResponse.send).toHaveBeenCalledWith(
116+
'No active SSE connection',
117+
);
118+
});
119+
120+
it('should forward message to transport when connection exists', async () => {
121+
// First establish SSE connection
122+
await controller.handleSse(mockResponse as Response);
123+
124+
// Then send message
125+
const messageRequest: Partial<Request> = {
126+
body: { method: 'tools/list' },
127+
headers: {},
128+
};
129+
const messageResponse: Partial<Response> = {
130+
status: vi.fn().mockReturnThis(),
131+
send: vi.fn().mockReturnThis(),
132+
};
133+
134+
await controller.handleMessages(
135+
messageRequest as Request,
136+
messageResponse as Response,
137+
);
138+
139+
expect(mockHandlePostMessage).toHaveBeenCalledWith(
140+
messageRequest,
141+
messageResponse,
142+
);
143+
});
144+
145+
it('should use the latest transport after reconnection', async () => {
146+
// First connection
147+
await controller.handleSse(mockResponse as Response);
148+
149+
// Reconnect
150+
const newSseResponse: Partial<Response> = {
151+
status: vi.fn().mockReturnThis(),
152+
send: vi.fn().mockReturnThis(),
153+
};
154+
await controller.handleSse(newSseResponse as Response);
155+
156+
// Send message - should use the new transport
157+
const messageRequest: Partial<Request> = {
158+
body: { method: 'resources/list' },
159+
headers: {},
160+
};
161+
162+
await controller.handleMessages(
163+
messageRequest as Request,
164+
mockResponse as Response,
165+
);
166+
167+
expect(mockHandlePostMessage).toHaveBeenCalled();
168+
});
169+
170+
it('should not call handlePostMessage when transport is null', async () => {
171+
await controller.handleMessages(
172+
mockRequest as Request,
173+
mockResponse as Response,
174+
);
175+
176+
expect(mockHandlePostMessage).not.toHaveBeenCalled();
177+
});
178+
});
179+
});

‎mcp-server/src/mcp/mcp.service.spec.ts‎

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,4 +356,180 @@ describe('McpService', () => {
356356
expect(text).toContain('airbnb');
357357
});
358358
});
359+
360+
describe('Error handling', () => {
361+
describe('Resources errors', () => {
362+
it('should throw error for invalid URI scheme', async () => {
363+
const handler = handlers.get('resources/read');
364+
expect(handler).toBeDefined();
365+
366+
await expect(
367+
handler!({ params: { uri: 'invalid://something' } }),
368+
).rejects.toThrow('Invalid URI scheme');
369+
});
370+
371+
it('should throw error when rules resource not found', async () => {
372+
vi.mocked(mockRulesService.getRuleContent!).mockRejectedValue(
373+
new Error('File not found'),
374+
);
375+
376+
const handler = handlers.get('resources/read');
377+
expect(handler).toBeDefined();
378+
379+
await expect(
380+
handler!({ params: { uri: 'rules://nonexistent.md' } }),
381+
).rejects.toThrow('Resource not found');
382+
});
383+
384+
it('should throw error when config://project fails to load', async () => {
385+
handlers.clear();
386+
const failingConfigService = createMockConfigService({});
387+
vi.mocked(failingConfigService.getProjectConfig!).mockRejectedValue(
388+
new Error('Config load error'),
389+
);
390+
391+
const service = new McpService(
392+
mockRulesService as RulesService,
393+
mockKeywordService as KeywordService,
394+
failingConfigService as ConfigService,
395+
);
396+
service.onModuleInit();
397+
398+
const handler = handlers.get('resources/read');
399+
expect(handler).toBeDefined();
400+
401+
await expect(
402+
handler!({ params: { uri: 'config://project' } }),
403+
).rejects.toThrow('Failed to load project configuration');
404+
});
405+
});
406+
407+
describe('Tools errors', () => {
408+
it('should throw error for unknown tool', async () => {
409+
const handler = handlers.get('tools/call');
410+
expect(handler).toBeDefined();
411+
412+
await expect(
413+
handler!({ params: { name: 'unknown_tool', arguments: {} } }),
414+
).rejects.toThrow('Tool not found: unknown_tool');
415+
});
416+
417+
it('should return error response when get_agent_details fails', async () => {
418+
vi.mocked(mockRulesService.getAgent!).mockRejectedValue(
419+
new Error('Agent not found'),
420+
);
421+
422+
const handler = handlers.get('tools/call');
423+
expect(handler).toBeDefined();
424+
425+
const result = (await handler!({
426+
params: {
427+
name: 'get_agent_details',
428+
arguments: { agentName: 'invalid' },
429+
},
430+
})) as { isError: boolean; content: { text: string }[] };
431+
432+
expect(result.isError).toBe(true);
433+
expect(result.content[0].text).toContain("Agent 'invalid' not found");
434+
});
435+
436+
it('should return error response when parse_mode fails', async () => {
437+
vi.mocked(mockKeywordService.parseMode!).mockRejectedValue(
438+
new Error('Parse error'),
439+
);
440+
441+
const handler = handlers.get('tools/call');
442+
expect(handler).toBeDefined();
443+
444+
const result = (await handler!({
445+
params: { name: 'parse_mode', arguments: { prompt: 'INVALID test' } },
446+
})) as { isError: boolean; content: { text: string }[] };
447+
448+
expect(result.isError).toBe(true);
449+
expect(result.content[0].text).toContain('Failed to parse mode');
450+
});
451+
452+
it('should return error response when get_project_config fails', async () => {
453+
handlers.clear();
454+
const failingConfigService = createMockConfigService({});
455+
vi.mocked(failingConfigService.getSettings!).mockRejectedValue(
456+
new Error('Settings error'),
457+
);
458+
459+
const service = new McpService(
460+
mockRulesService as RulesService,
461+
mockKeywordService as KeywordService,
462+
failingConfigService as ConfigService,
463+
);
464+
service.onModuleInit();
465+
466+
const handler = handlers.get('tools/call');
467+
expect(handler).toBeDefined();
468+
469+
const result = (await handler!({
470+
params: { name: 'get_project_config', arguments: {} },
471+
})) as { isError: boolean; content: { text: string }[] };
472+
473+
expect(result.isError).toBe(true);
474+
expect(result.content[0].text).toContain(
475+
'Failed to get project config',
476+
);
477+
});
478+
});
479+
480+
describe('Prompts errors', () => {
481+
it('should throw error when agent not found in activate_agent', async () => {
482+
vi.mocked(mockRulesService.getAgent!).mockRejectedValue(
483+
new Error('Agent not found'),
484+
);
485+
486+
const handler = handlers.get('prompts/get');
487+
expect(handler).toBeDefined();
488+
489+
await expect(
490+
handler!({
491+
params: {
492+
name: 'activate_agent',
493+
arguments: { role: 'invalid-agent' },
494+
},
495+
}),
496+
).rejects.toThrow("Agent 'invalid-agent' not found");
497+
});
498+
499+
it('should throw error for unknown prompt', async () => {
500+
const handler = handlers.get('prompts/get');
501+
expect(handler).toBeDefined();
502+
503+
await expect(
504+
handler!({ params: { name: 'unknown_prompt', arguments: {} } }),
505+
).rejects.toThrow('Prompt not found');
506+
});
507+
});
508+
});
509+
510+
describe('startStdio', () => {
511+
it('should connect server with StdioServerTransport', async () => {
512+
const service = new McpService(
513+
mockRulesService as RulesService,
514+
mockKeywordService as KeywordService,
515+
mockConfigService as ConfigService,
516+
);
517+
518+
// Should not throw
519+
await expect(service.startStdio()).resolves.not.toThrow();
520+
});
521+
});
522+
523+
describe('getServer', () => {
524+
it('should return the MCP server instance', () => {
525+
const service = new McpService(
526+
mockRulesService as RulesService,
527+
mockKeywordService as KeywordService,
528+
mockConfigService as ConfigService,
529+
);
530+
531+
const server = service.getServer();
532+
expect(server).toBeDefined();
533+
});
534+
});
359535
});

0 commit comments

Comments
 (0)