From ae7d6af71711fb101897406f26d236cbbd782971 Mon Sep 17 00:00:00 2001 From: "mingholy.lmh" Date: Mon, 1 Dec 2025 13:41:46 +0800 Subject: [PATCH] fix(test): remove unused test cases --- .../test/unit/ProcessTransport.test.ts | 9 +- .../sdk-typescript/test/unit/Query.test.ts | 60 +--- .../unit/SdkControlServerTransport.test.ts | 259 ------------------ 3 files changed, 18 insertions(+), 310 deletions(-) delete mode 100644 packages/sdk-typescript/test/unit/SdkControlServerTransport.test.ts diff --git a/packages/sdk-typescript/test/unit/ProcessTransport.test.ts b/packages/sdk-typescript/test/unit/ProcessTransport.test.ts index 0854a02d..b8602654 100644 --- a/packages/sdk-typescript/test/unit/ProcessTransport.test.ts +++ b/packages/sdk-typescript/test/unit/ProcessTransport.test.ts @@ -1187,13 +1187,20 @@ describe('ProcessTransport', () => { const options: TransportOptions = { pathToQwenExecutable: 'qwen', stderr: stderrCallback, + debug: true, // Enable debug to ensure stderr data is logged }; new ProcessTransport(options); + // Clear previous calls from logger.info during initialization + stderrCallback.mockClear(); + mockStderr.emit('data', Buffer.from('error message')); - expect(stderrCallback).toHaveBeenCalledWith('error message'); + // The stderr data is passed through logger.debug, which formats it + // So we check that the callback was called with a message containing 'error message' + expect(stderrCallback).toHaveBeenCalled(); + expect(stderrCallback.mock.calls[0][0]).toContain('error message'); }); }); diff --git a/packages/sdk-typescript/test/unit/Query.test.ts b/packages/sdk-typescript/test/unit/Query.test.ts index b7309a19..2b89ca51 100644 --- a/packages/sdk-typescript/test/unit/Query.test.ts +++ b/packages/sdk-typescript/test/unit/Query.test.ts @@ -312,50 +312,6 @@ describe('Query', () => { await transport2.close(); }); - it('should validate MCP server name conflicts', async () => { - const mockServer = { - connect: vi.fn(), - }; - - await expect(async () => { - const query = new Query(transport, { - cwd: '/test', - mcpServers: { server1: { command: 'test' } }, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sdkMcpServers: { server1: mockServer as any }, - }); - await query.initialized; - }).rejects.toThrow(/name conflicts/); - }); - - it('should initialize with SDK MCP servers', async () => { - const mockServer = { - connect: vi.fn().mockResolvedValue(undefined), - }; - - const query = new Query(transport, { - cwd: '/test', - // eslint-disable-next-line @typescript-eslint/no-explicit-any - sdkMcpServers: { testServer: mockServer as any }, - }); - - // Respond to initialize request - await vi.waitFor(() => { - expect(transport.writtenMessages.length).toBeGreaterThan(0); - }); - - const initRequest = - transport.getLastWrittenMessage() as CLIControlRequest; - transport.simulateMessage( - createControlResponse(initRequest.request_id, true, {}), - ); - - await query.initialized; - expect(mockServer.connect).toHaveBeenCalled(); - - await query.close(); - }); - it('should handle initialization errors', async () => { const query = new Query(transport, { cwd: '/test', @@ -483,7 +439,7 @@ describe('Query', () => { describe('Control Plane - Permission Control', () => { it('should handle can_use_tool control requests', async () => { - const canUseTool = vi.fn().mockResolvedValue(true); + const canUseTool = vi.fn().mockResolvedValue({ behavior: 'allow' }); const query = new Query(transport, { cwd: '/test', canUseTool, @@ -507,7 +463,7 @@ describe('Query', () => { }); it('should send control response with permission result - allow', async () => { - const canUseTool = vi.fn().mockResolvedValue(true); + const canUseTool = vi.fn().mockResolvedValue({ behavior: 'allow' }); const query = new Query(transport, { cwd: '/test', canUseTool, @@ -533,7 +489,7 @@ describe('Query', () => { }); it('should send control response with permission result - deny', async () => { - const canUseTool = vi.fn().mockResolvedValue(false); + const canUseTool = vi.fn().mockResolvedValue({ behavior: 'deny' }); const query = new Query(transport, { cwd: '/test', canUseTool, @@ -586,7 +542,7 @@ describe('Query', () => { const canUseTool = vi.fn().mockImplementation( () => new Promise((resolve) => { - setTimeout(() => resolve(true), 35000); // Exceeds 30s timeout + setTimeout(() => resolve({ behavior: 'allow' }), 35000); // Exceeds 30s timeout }), ); @@ -709,10 +665,14 @@ describe('Query', () => { describe('Control Plane - Control Cancel', () => { it('should handle control cancel requests', async () => { const canUseTool = vi.fn().mockImplementation( - ({ signal }: { signal: AbortSignal }) => + ( + _toolName: string, + _toolInput: unknown, + { signal }: { signal: AbortSignal }, + ) => new Promise((resolve, reject) => { signal.addEventListener('abort', () => reject(new AbortError())); - setTimeout(() => resolve(true), 5000); + setTimeout(() => resolve({ behavior: 'allow' }), 5000); }), ); diff --git a/packages/sdk-typescript/test/unit/SdkControlServerTransport.test.ts b/packages/sdk-typescript/test/unit/SdkControlServerTransport.test.ts deleted file mode 100644 index 6bfd61a0..00000000 --- a/packages/sdk-typescript/test/unit/SdkControlServerTransport.test.ts +++ /dev/null @@ -1,259 +0,0 @@ -/** - * Unit tests for SdkControlServerTransport - * - * Tests MCP message proxying between MCP Server and Query's control plane. - */ - -import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { SdkControlServerTransport } from '../../src/mcp/SdkControlServerTransport.js'; - -describe('SdkControlServerTransport', () => { - let sendToQuery: ReturnType; - let transport: SdkControlServerTransport; - - beforeEach(() => { - sendToQuery = vi.fn().mockResolvedValue({ result: 'success' }); - transport = new SdkControlServerTransport({ - serverName: 'test-server', - sendToQuery, - }); - }); - - describe('Lifecycle', () => { - it('should start successfully', async () => { - await transport.start(); - expect(transport.isStarted()).toBe(true); - }); - - it('should close successfully', async () => { - await transport.start(); - await transport.close(); - expect(transport.isStarted()).toBe(false); - }); - - it('should handle close callback', async () => { - const onclose = vi.fn(); - transport.onclose = onclose; - - await transport.start(); - await transport.close(); - - expect(onclose).toHaveBeenCalled(); - }); - }); - - describe('Message Sending', () => { - it('should send message to Query', async () => { - await transport.start(); - - const message = { - jsonrpc: '2.0' as const, - id: 1, - method: 'tools/list', - params: {}, - }; - - await transport.send(message); - - expect(sendToQuery).toHaveBeenCalledWith(message); - }); - - it('should throw error when sending before start', async () => { - const message = { - jsonrpc: '2.0' as const, - id: 1, - method: 'tools/list', - }; - - await expect(transport.send(message)).rejects.toThrow('not started'); - }); - - it('should handle send errors', async () => { - const error = new Error('Network error'); - sendToQuery.mockRejectedValue(error); - - const onerror = vi.fn(); - transport.onerror = onerror; - - await transport.start(); - - const message = { - jsonrpc: '2.0' as const, - id: 1, - method: 'tools/list', - }; - - await expect(transport.send(message)).rejects.toThrow('Network error'); - expect(onerror).toHaveBeenCalledWith(error); - }); - }); - - describe('Message Receiving', () => { - it('should deliver message to MCP Server via onmessage', async () => { - const onmessage = vi.fn(); - transport.onmessage = onmessage; - - await transport.start(); - - const message = { - jsonrpc: '2.0' as const, - id: 1, - result: { tools: [] }, - }; - - transport.handleMessage(message); - - expect(onmessage).toHaveBeenCalledWith(message); - }); - - it('should warn when receiving message without onmessage handler', async () => { - const consoleWarnSpy = vi - .spyOn(console, 'warn') - .mockImplementation(() => {}); - - await transport.start(); - - const message = { - jsonrpc: '2.0' as const, - id: 1, - result: {}, - }; - - transport.handleMessage(message); - - expect(consoleWarnSpy).toHaveBeenCalled(); - - consoleWarnSpy.mockRestore(); - }); - - it('should warn when receiving message for closed transport', async () => { - const consoleWarnSpy = vi - .spyOn(console, 'warn') - .mockImplementation(() => {}); - const onmessage = vi.fn(); - transport.onmessage = onmessage; - - await transport.start(); - await transport.close(); - - const message = { - jsonrpc: '2.0' as const, - id: 1, - result: {}, - }; - - transport.handleMessage(message); - - expect(consoleWarnSpy).toHaveBeenCalled(); - expect(onmessage).not.toHaveBeenCalled(); - - consoleWarnSpy.mockRestore(); - }); - }); - - describe('Error Handling', () => { - it('should deliver error to MCP Server via onerror', async () => { - const onerror = vi.fn(); - transport.onerror = onerror; - - await transport.start(); - - const error = new Error('Test error'); - transport.handleError(error); - - expect(onerror).toHaveBeenCalledWith(error); - }); - - it('should log error when no onerror handler set', async () => { - const consoleErrorSpy = vi - .spyOn(console, 'error') - .mockImplementation(() => {}); - - await transport.start(); - - const error = new Error('Test error'); - transport.handleError(error); - - expect(consoleErrorSpy).toHaveBeenCalled(); - - consoleErrorSpy.mockRestore(); - }); - }); - - describe('Server Name', () => { - it('should return server name', () => { - expect(transport.getServerName()).toBe('test-server'); - }); - }); - - describe('Bidirectional Communication', () => { - it('should support full message round-trip', async () => { - const onmessage = vi.fn(); - transport.onmessage = onmessage; - - await transport.start(); - - // Send request from MCP Server to CLI - const request = { - jsonrpc: '2.0' as const, - id: 1, - method: 'tools/list', - params: {}, - }; - - await transport.send(request); - expect(sendToQuery).toHaveBeenCalledWith(request); - - // Receive response from CLI to MCP Server - const response = { - jsonrpc: '2.0' as const, - id: 1, - result: { - tools: [ - { - name: 'test_tool', - description: 'A test tool', - inputSchema: { type: 'object' }, - }, - ], - }, - }; - - transport.handleMessage(response); - expect(onmessage).toHaveBeenCalledWith(response); - }); - - it('should handle multiple messages in sequence', async () => { - const onmessage = vi.fn(); - transport.onmessage = onmessage; - - await transport.start(); - - // Send multiple requests - for (let i = 0; i < 5; i++) { - const message = { - jsonrpc: '2.0' as const, - id: i, - method: 'test', - }; - - await transport.send(message); - } - - expect(sendToQuery).toHaveBeenCalledTimes(5); - - // Receive multiple responses - for (let i = 0; i < 5; i++) { - const message = { - jsonrpc: '2.0' as const, - id: i, - result: {}, - }; - - transport.handleMessage(message); - } - - expect(onmessage).toHaveBeenCalledTimes(5); - }); - }); -});