Skip to content

Commit 37ef9a9

Browse files
committed
test(mcp-server): Replace direct tracing module calls with spies for improved test isolation
1 parent cac9bd0 commit 37ef9a9

File tree

1 file changed

+28
-60
lines changed

1 file changed

+28
-60
lines changed

packages/core/test/lib/mcp-server.test.ts

Lines changed: 28 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,11 @@ import { beforeEach, describe, expect, it, vi } from 'vitest';
22
import { wrapMcpServerWithSentry } from '../../src/mcp-server';
33
import * as tracingModule from '../../src/tracing';
44

5-
vi.mock('../../src/tracing');
6-
75
describe('wrapMcpServerWithSentry', () => {
6+
const startSpanSpy = vi.spyOn(tracingModule, 'startSpan');
7+
88
beforeEach(() => {
99
vi.clearAllMocks();
10-
// @ts-expect-error mocking span is annoying
11-
vi.mocked(tracingModule.startSpan).mockImplementation((_, cb) => cb());
1210
});
1311

1412
it('should return the same instance (modified) if it is a valid MCP server instance', () => {
@@ -33,7 +31,7 @@ describe('wrapMcpServerWithSentry', () => {
3331
expect(result.tool).toBe(invalidMcpServer.tool);
3432

3533
// No calls to startSpan
36-
expect(tracingModule.startSpan).not.toHaveBeenCalled();
34+
expect(startSpanSpy).not.toHaveBeenCalled();
3735
});
3836

3937
it('should not wrap the same instance twice', () => {
@@ -114,7 +112,7 @@ describe('wrapMcpServerWithSentry', () => {
114112
// Simulate incoming message
115113
mockTransport.onmessage?.(jsonRpcRequest, {});
116114

117-
expect(tracingModule.startSpan).toHaveBeenCalledWith(
115+
expect(startSpanSpy).toHaveBeenCalledWith(
118116
expect.objectContaining({
119117
name: 'tools/call get-weather',
120118
forceTransaction: true,
@@ -135,7 +133,7 @@ describe('wrapMcpServerWithSentry', () => {
135133
// Simulate incoming notification
136134
mockTransport.onmessage?.(jsonRpcNotification, {});
137135

138-
expect(tracingModule.startSpan).toHaveBeenCalledWith(
136+
expect(startSpanSpy).toHaveBeenCalledWith(
139137
expect.objectContaining({
140138
name: 'notifications/initialized',
141139
forceTransaction: true,
@@ -156,7 +154,7 @@ describe('wrapMcpServerWithSentry', () => {
156154
// Simulate outgoing notification
157155
await mockTransport.send?.(outgoingNotification);
158156

159-
expect(tracingModule.startSpan).toHaveBeenCalledWith(
157+
expect(startSpanSpy).toHaveBeenCalledWith(
160158
expect.objectContaining({
161159
name: 'notifications/tools/list_changed',
162160
forceTransaction: true,
@@ -171,7 +169,7 @@ describe('wrapMcpServerWithSentry', () => {
171169
// Simulate non-JSON-RPC message
172170
mockTransport.onmessage?.({ some: 'data' }, {});
173171

174-
expect(tracingModule.startSpan).not.toHaveBeenCalled();
172+
expect(startSpanSpy).not.toHaveBeenCalled();
175173
});
176174

177175
it('should handle transport onclose events', async () => {
@@ -203,13 +201,7 @@ describe('wrapMcpServerWithSentry', () => {
203201
jsonrpc: '2.0',
204202
method: 'tools/call',
205203
id: 'req-1',
206-
params: {
207-
name: 'get-weather',
208-
arguments: {
209-
location: 'Seattle, WA',
210-
units: 'metric'
211-
}
212-
}
204+
params: { name: 'get-weather', arguments: { location: 'Seattle, WA' }}
213205
};
214206

215207
const extraWithClientInfo = {
@@ -221,33 +213,26 @@ describe('wrapMcpServerWithSentry', () => {
221213

222214
mockTransport.onmessage?.(jsonRpcRequest, extraWithClientInfo);
223215

224-
expect(tracingModule.startSpan).toHaveBeenCalledWith(
225-
expect.objectContaining({
216+
expect(startSpanSpy).toHaveBeenCalledWith(
217+
{
226218
name: 'tools/call get-weather',
227219
forceTransaction: true,
228-
attributes: expect.objectContaining({
229-
// Required
220+
attributes: {
230221
'mcp.method.name': 'tools/call',
231-
// Conditionally Required (tool operation)
232222
'mcp.tool.name': 'get-weather',
233223
'mcp.request.id': 'req-1',
234-
// Recommended
235224
'mcp.session.id': 'test-session-123',
236225
'client.address': '192.168.1.100',
237226
'client.port': 54321,
238-
// Transport attributes
239227
'mcp.transport': 'http',
240228
'network.transport': 'tcp',
241229
'network.protocol.version': '2.0',
242-
// Tool arguments (JSON-stringified)
243230
'mcp.request.argument.location': '"Seattle, WA"',
244-
'mcp.request.argument.units': '"metric"',
245-
// Sentry-specific
246231
'sentry.op': 'mcp.server',
247232
'sentry.origin': 'auto.function.mcp_server',
248233
'sentry.source': 'route',
249-
}),
250-
}),
234+
},
235+
},
251236
expect.any(Function)
252237
);
253238
});
@@ -264,30 +249,24 @@ describe('wrapMcpServerWithSentry', () => {
264249

265250
mockTransport.onmessage?.(jsonRpcRequest, {});
266251

267-
expect(tracingModule.startSpan).toHaveBeenCalledWith(
268-
expect.objectContaining({
252+
expect(startSpanSpy).toHaveBeenCalledWith(
253+
{
269254
name: 'resources/read file:///docs/api.md',
270255
forceTransaction: true,
271-
attributes: expect.objectContaining({
272-
// Required
256+
attributes: {
273257
'mcp.method.name': 'resources/read',
274-
// Conditionally Required (resource operation)
275258
'mcp.resource.uri': 'file:///docs/api.md',
276259
'mcp.request.id': 'req-2',
277-
// Recommended
278260
'mcp.session.id': 'test-session-123',
279-
// Transport attributes
280261
'mcp.transport': 'http',
281262
'network.transport': 'tcp',
282263
'network.protocol.version': '2.0',
283-
// Request arguments (JSON-stringified)
284264
'mcp.request.argument.uri': '"file:///docs/api.md"',
285-
// Sentry-specific
286265
'sentry.op': 'mcp.server',
287266
'sentry.origin': 'auto.function.mcp_server',
288267
'sentry.source': 'route',
289-
}),
290-
}),
268+
},
269+
},
291270
expect.any(Function)
292271
);
293272
});
@@ -304,30 +283,24 @@ describe('wrapMcpServerWithSentry', () => {
304283

305284
mockTransport.onmessage?.(jsonRpcRequest, {});
306285

307-
expect(tracingModule.startSpan).toHaveBeenCalledWith(
308-
expect.objectContaining({
286+
expect(startSpanSpy).toHaveBeenCalledWith(
287+
{
309288
name: 'prompts/get analyze-code',
310289
forceTransaction: true,
311-
attributes: expect.objectContaining({
312-
// Required
290+
attributes: {
313291
'mcp.method.name': 'prompts/get',
314-
// Conditionally Required (prompt operation)
315292
'mcp.prompt.name': 'analyze-code',
316293
'mcp.request.id': 'req-3',
317-
// Recommended
318294
'mcp.session.id': 'test-session-123',
319-
// Transport attributes
320295
'mcp.transport': 'http',
321296
'network.transport': 'tcp',
322297
'network.protocol.version': '2.0',
323-
// Request arguments (JSON-stringified)
324298
'mcp.request.argument.name': '"analyze-code"',
325-
// Sentry-specific
326299
'sentry.op': 'mcp.server',
327300
'sentry.origin': 'auto.function.mcp_server',
328301
'sentry.source': 'route',
329-
}),
330-
}),
302+
},
303+
},
331304
expect.any(Function)
332305
);
333306
});
@@ -343,27 +316,22 @@ describe('wrapMcpServerWithSentry', () => {
343316

344317
mockTransport.onmessage?.(jsonRpcNotification, {});
345318

346-
expect(tracingModule.startSpan).toHaveBeenCalledWith(
347-
expect.objectContaining({
319+
expect(startSpanSpy).toHaveBeenCalledWith(
320+
{
348321
name: 'notifications/tools/list_changed',
349322
forceTransaction: true,
350-
attributes: expect.objectContaining({
351-
// Required
323+
attributes: {
352324
'mcp.method.name': 'notifications/tools/list_changed',
353-
// Recommended
354325
'mcp.session.id': 'test-session-123',
355-
// Notification-specific
356326
'mcp.notification.direction': 'client_to_server',
357-
// Transport attributes
358327
'mcp.transport': 'http',
359328
'network.transport': 'tcp',
360329
'network.protocol.version': '2.0',
361-
// Sentry-specific
362330
'sentry.op': 'mcp.server',
363331
'sentry.origin': 'auto.mcp.notification',
364332
'sentry.source': 'route',
365-
}),
366-
}),
333+
},
334+
},
367335
expect.any(Function)
368336
);
369337

0 commit comments

Comments
 (0)