Skip to content

Commit 100954f

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 100954f

3 files changed

Lines changed: 697 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: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,4 +356,172 @@ 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: { name: 'get_agent_details', arguments: { agentName: 'invalid' } },
427+
})) as { isError: boolean; content: { text: string }[] };
428+
429+
expect(result.isError).toBe(true);
430+
expect(result.content[0].text).toContain("Agent 'invalid' not found");
431+
});
432+
433+
it('should return error response when parse_mode fails', async () => {
434+
vi.mocked(mockKeywordService.parseMode).mockRejectedValue(
435+
new Error('Parse error'),
436+
);
437+
438+
const handler = handlers.get('tools/call');
439+
expect(handler).toBeDefined();
440+
441+
const result = (await handler!({
442+
params: { name: 'parse_mode', arguments: { prompt: 'INVALID test' } },
443+
})) as { isError: boolean; content: { text: string }[] };
444+
445+
expect(result.isError).toBe(true);
446+
expect(result.content[0].text).toContain('Failed to parse mode');
447+
});
448+
449+
it('should return error response when get_project_config fails', async () => {
450+
handlers.clear();
451+
const failingConfigService = createMockConfigService({});
452+
vi.mocked(failingConfigService.getSettings).mockRejectedValue(
453+
new Error('Settings error'),
454+
);
455+
456+
const service = new McpService(
457+
mockRulesService as RulesService,
458+
mockKeywordService as KeywordService,
459+
failingConfigService as ConfigService,
460+
);
461+
service.onModuleInit();
462+
463+
const handler = handlers.get('tools/call');
464+
expect(handler).toBeDefined();
465+
466+
const result = (await handler!({
467+
params: { name: 'get_project_config', arguments: {} },
468+
})) as { isError: boolean; content: { text: string }[] };
469+
470+
expect(result.isError).toBe(true);
471+
expect(result.content[0].text).toContain('Failed to get project config');
472+
});
473+
});
474+
475+
describe('Prompts errors', () => {
476+
it('should throw error when agent not found in activate_agent', async () => {
477+
vi.mocked(mockRulesService.getAgent).mockRejectedValue(
478+
new Error('Agent not found'),
479+
);
480+
481+
const handler = handlers.get('prompts/get');
482+
expect(handler).toBeDefined();
483+
484+
await expect(
485+
handler!({
486+
params: { name: 'activate_agent', arguments: { role: 'invalid-agent' } },
487+
}),
488+
).rejects.toThrow("Agent 'invalid-agent' not found");
489+
});
490+
491+
it('should throw error for unknown prompt', async () => {
492+
const handler = handlers.get('prompts/get');
493+
expect(handler).toBeDefined();
494+
495+
await expect(
496+
handler!({ params: { name: 'unknown_prompt', arguments: {} } }),
497+
).rejects.toThrow('Prompt not found');
498+
});
499+
});
500+
});
501+
502+
describe('startStdio', () => {
503+
it('should connect server with StdioServerTransport', async () => {
504+
const service = new McpService(
505+
mockRulesService as RulesService,
506+
mockKeywordService as KeywordService,
507+
mockConfigService as ConfigService,
508+
);
509+
510+
// Should not throw
511+
await expect(service.startStdio()).resolves.not.toThrow();
512+
});
513+
});
514+
515+
describe('getServer', () => {
516+
it('should return the MCP server instance', () => {
517+
const service = new McpService(
518+
mockRulesService as RulesService,
519+
mockKeywordService as KeywordService,
520+
mockConfigService as ConfigService,
521+
);
522+
523+
const server = service.getServer();
524+
expect(server).toBeDefined();
525+
});
526+
});
359527
});

0 commit comments

Comments
 (0)