Skip to content

Commit 56e98d1

Browse files
author
klapaudius
committed
Update tests to cover all new features
- Updated test cases in `ToolsListHandlerTest`, `ResourcesReadHandlerTest`, and `MCPProtocolTest` to include `clientId` in handler method calls. - Adjusted `StreamableHttpControllerTest` and related assertions to reflect new streaming response behavior. - Enhanced `StreamableHttpTransportTest` to verify SSE output generation. - Updated `TestMcpToolCommandTest` to validate `TextToolResult` usage and enhanced test coverage for result display logic.
1 parent 414729c commit 56e98d1

18 files changed

+2352
-78
lines changed

tests/Command/TestMcpToolCommandTest.php

Lines changed: 488 additions & 1 deletion
Large diffs are not rendered by default.

tests/Controllers/StreamableHttpControllerTest.php

Lines changed: 50 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,10 @@
22

33
namespace KLP\KlpMcpServer\Tests\Controllers;
44

5-
use Exception;
6-
use JsonException;
75
use KLP\KlpMcpServer\Controllers\StreamableHttpController;
86
use KLP\KlpMcpServer\Protocol\MCPProtocolInterface;
97
use KLP\KlpMcpServer\Server\MCPServerInterface;
8+
use KLP\KlpMcpServer\Transports\Exception\StreamableHttpTransportException;
109
use PHPUnit\Framework\Attributes\Small;
1110
use PHPUnit\Framework\MockObject\MockObject;
1211
use PHPUnit\Framework\TestCase;
@@ -53,7 +52,6 @@ public function test_handle_with_post_request(): void
5352
// Create a POST request with some content
5453
$clientId = 'test-client-id';
5554
$message = ['jsonrpc' => '2.0', 'method' => 'test', 'params' => [], 'id' => 1];
56-
$responseResult = [['jsonrpc' => '2.0', 'result' => 'success', 'id' => 1]];
5755

5856
$request = new Request(
5957
[], // query parameters
@@ -76,17 +74,19 @@ public function test_handle_with_post_request(): void
7674
->method('requestMessage')
7775
->with($clientId, $message);
7876

79-
$this->mockServer->expects($this->once())
80-
->method('getResponseResult')
81-
->with($clientId)
82-
->willReturn($responseResult);
83-
8477
// Call handle() which should internally call postHandle()
8578
$response = $this->controller->handle($request);
8679

8780
// Verify the response is what we expect from postHandle()
88-
$this->assertInstanceOf(JsonResponse::class, $response);
89-
$this->assertEquals($responseResult[0], json_decode($response->getContent(), true));
81+
$this->assertInstanceOf(StreamedResponse::class, $response);
82+
$this->assertEquals('text/event-stream', $response->headers->get('Content-Type'));
83+
$this->assertEquals('no-cache, private', $response->headers->get('Cache-Control'));
84+
$this->assertEquals('no', $response->headers->get('X-Accel-Buffering'));
85+
86+
// Execute the streamed response to trigger the callback
87+
ob_start();
88+
$response->sendContent();
89+
ob_end_clean();
9090
}
9191

9292
public function test_gethandle(): void
@@ -109,7 +109,6 @@ public function test_postHandle_with_single_message(): void
109109
$clientId = 'test-client-id';
110110
// The controller checks for 'jsonrpc' key to determine if it's a single message
111111
$message = ['jsonrpc' => '2.0', 'method' => 'test', 'params' => [], 'id' => 1];
112-
$responseResult = [['jsonrpc' => '2.0', 'result' => 'success', 'id' => 1]];
113112

114113
// Create request with headers and content
115114
$request = new Request(
@@ -135,27 +134,22 @@ public function test_postHandle_with_single_message(): void
135134
$this->mockServer->expects($this->once())
136135
->method('requestMessage');
137136

138-
$this->mockServer->expects($this->once())
139-
->method('getResponseResult')
140-
->willReturn($responseResult);
141-
142137
// Set up logger mock expectations - we can't be too specific about the parameters
143138
$this->mockLogger->expects($this->atLeastOnce())
144139
->method('debug');
145140

146141
// Call the method and verify response
147142
$response = $this->controller->postHandle($request);
148143

149-
$this->assertInstanceOf(JsonResponse::class, $response);
144+
$this->assertInstanceOf(StreamedResponse::class, $response);
145+
$this->assertEquals('text/event-stream', $response->headers->get('Content-Type'));
146+
$this->assertEquals('no-cache, private', $response->headers->get('Cache-Control'));
147+
$this->assertEquals('no', $response->headers->get('X-Accel-Buffering'));
150148

151-
// Instead of checking the exact content, just verify it's a valid JSON response
152-
$content = json_decode($response->getContent(), true);
153-
$this->assertIsArray($content);
154-
// The response might have either 'jsonrpc' or 'jsonrpc' key depending on the implementation
155-
$this->assertTrue(
156-
isset($content['jsonrpc']) || isset($content['jsonrpc']),
157-
'Response should have either jsonrpc or jsonrpc key'
158-
);
149+
// Execute the streamed response to trigger the callback
150+
ob_start();
151+
$response->sendContent();
152+
ob_end_clean();
159153
}
160154

161155
public function test_postHandle_with_multiple_messages(): void
@@ -197,7 +191,7 @@ public function test_postHandle_with_multiple_messages(): void
197191
return true;
198192
}));
199193

200-
$this->mockServer->expects($this->once())
194+
$this->mockServer->expects($this->never())
201195
->method('connect');
202196

203197
// Set up logger mock expectations
@@ -224,7 +218,6 @@ public function test_postHandle_with_fallback_client_id(): void
224218
{
225219
$clientId = 'fallback-client-id';
226220
$message = ['jsonrpc' => '2.0', 'method' => 'test', 'params' => [], 'id' => 1];
227-
$responseResult = [['jsonrpc' => '2.0', 'result' => 'success', 'id' => 1]];
228221

229222
// Create request without mcp-session-id header
230223
$request = new Request(
@@ -250,16 +243,18 @@ public function test_postHandle_with_fallback_client_id(): void
250243
->method('requestMessage')
251244
->with($clientId, $message);
252245

253-
$this->mockServer->expects($this->once())
254-
->method('getResponseResult')
255-
->with($clientId)
256-
->willReturn($responseResult);
257-
258246
// Call the method and verify response
259247
$response = $this->controller->postHandle($request);
260248

261-
$this->assertInstanceOf(JsonResponse::class, $response);
262-
$this->assertEquals($responseResult[0], json_decode($response->getContent(), true));
249+
$this->assertInstanceOf(StreamedResponse::class, $response);
250+
$this->assertEquals('text/event-stream', $response->headers->get('Content-Type'));
251+
$this->assertEquals('no-cache, private', $response->headers->get('Cache-Control'));
252+
$this->assertEquals('no', $response->headers->get('X-Accel-Buffering'));
253+
254+
// Execute the streamed response to trigger the callback
255+
ob_start();
256+
$response->sendContent();
257+
ob_end_clean();
263258
}
264259

265260
public function test_postHandle_with_json_parse_error(): void
@@ -303,30 +298,37 @@ public function test_postHandle_with_general_exception(): void
303298
);
304299
$request->headers->set('mcp-session-id', $clientId);
305300

306-
// Set up server mock to return a result that will trigger the exception
301+
// Set up server mock to throw an exception
307302
$this->mockServer->expects($this->once())
308303
->method('setProtocolVersion')
309304
->with(MCPProtocolInterface::PROTOCOL_VERSION_STREAMABE_HTTP);
310305

311306
$this->mockServer->expects($this->once())
312307
->method('requestMessage')
313-
->with($clientId, $message);
314-
315-
// Return an empty array to trigger the "Result is not an array" exception
316-
// The controller checks for !isset($result[0])
317-
$this->mockServer->expects($this->once())
318-
->method('getResponseResult')
319-
->with($clientId)
320-
->willReturn([]);
308+
->with($clientId, $message)
309+
->willThrowException(new StreamableHttpTransportException('Test exception'));
321310

322311
// Call the method and verify response
323312
$response = $this->controller->postHandle($request);
324313

325-
$this->assertInstanceOf(JsonResponse::class, $response);
326-
$this->assertEquals(400, $response->getStatusCode());
327-
$this->assertEquals(
328-
['jsonrpc' => 2.0, 'error' => ['code' => -32700, 'message' => 'Result is not an array']],
329-
json_decode($response->getContent(), true)
330-
);
314+
// The response is still a StreamedResponse because exceptions in callbacks are not caught
315+
$this->assertInstanceOf(StreamedResponse::class, $response);
316+
$this->assertEquals('text/event-stream', $response->headers->get('Content-Type'));
317+
$this->assertEquals('no-cache, private', $response->headers->get('Cache-Control'));
318+
$this->assertEquals('no', $response->headers->get('X-Accel-Buffering'));
319+
320+
// The exception will be thrown when the callback is executed
321+
$this->expectException(StreamableHttpTransportException::class);
322+
$this->expectExceptionMessage('Test exception');
323+
324+
try {
325+
ob_start();
326+
$response->sendContent();
327+
} finally {
328+
// Always clean up the output buffer, even if an exception is thrown
329+
if (ob_get_level() > 0) {
330+
ob_end_clean();
331+
}
332+
}
331333
}
332334
}

tests/Protocol/MCPProtocolTest.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ public function test_handle_message_handles_valid_request(): void
237237

238238
$mockHandler = $this->createMock(\KLP\KlpMcpServer\Protocol\Handlers\RequestHandler::class);
239239
$mockHandler->method('isHandle')->with('test.method')->willReturn(true);
240-
$mockHandler->method('execute')->with('test.method', 1, ['param1' => 'value1'])->willReturn(['response' => 'ok']);
240+
$mockHandler->method('execute')->with('test.method', $clientId, 1, ['param1' => 'value1'])->willReturn(['response' => 'ok']);
241241
$this->mcpProtocol->registerRequestHandler($mockHandler);
242242

243243
$this->mockTransport
@@ -368,7 +368,7 @@ public function test_handle_message_handles_invalid_params(): void
368368

369369
$mockHandler = $this->createMock(\KLP\KlpMcpServer\Protocol\Handlers\RequestHandler::class);
370370
$mockHandler->method('isHandle')->with('test.method')->willReturn(true);
371-
$mockHandler->method('execute')->with('test.method', 1, ['param1' => 'invalid'])
371+
$mockHandler->method('execute')->with('test.method', $clientId, 1, ['param1' => 'invalid'])
372372
->willThrowException(new ToolParamsValidatorException('An error occurred.', ['Invalid params param1']));
373373

374374
$this->mockTransport
@@ -397,7 +397,7 @@ public function test_handle_message_handles_handler_throw_exception(): void
397397
$invalidParamsMessage = ['jsonrpc' => '2.0', 'id' => 1, 'method' => 'test.method', 'params' => ['param1' => 'invalid']];
398398
$mockHandler = $this->createMock(\KLP\KlpMcpServer\Protocol\Handlers\RequestHandler::class);
399399
$mockHandler->method('isHandle')->with('test.method')->willReturn(true);
400-
$mockHandler->method('execute')->with('test.method', 1, ['param1' => 'invalid'])
400+
$mockHandler->method('execute')->with('test.method', $clientId, 1, ['param1' => 'invalid'])
401401
->willThrowException(new \Exception('An error occurred.'));
402402

403403
$this->mockTransport

tests/Server/MCPServerTest.php

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use KLP\KlpMcpServer\Server\Request\ToolsListHandler;
1616
use KLP\KlpMcpServer\Server\ServerCapabilities;
1717
use KLP\KlpMcpServer\Server\ServerCapabilitiesInterface;
18+
use KLP\KlpMcpServer\Services\ProgressService\ProgressNotifierRepository;
1819
use KLP\KlpMcpServer\Services\ResourceService\ResourceRepository;
1920
use KLP\KlpMcpServer\Services\ToolService\ToolRepository;
2021
use PHPUnit\Framework\Attributes\Small;
@@ -33,9 +34,11 @@ public function test_register_tool_repository(): void
3334
$mockProtocol = $this->createMock(MCPProtocolInterface::class);
3435
$mockToolRepository = $this->createMock(ToolRepository::class);
3536

37+
$mockProgressNotifierRepository = $this->createMock(ProgressNotifierRepository::class);
38+
3639
$invocations = [
3740
new ToolsListHandler($mockToolRepository),
38-
new ToolsCallHandler($mockToolRepository),
41+
new ToolsCallHandler($mockToolRepository, $mockProgressNotifierRepository),
3942
];
4043
$mockProtocol->expects($matcher = $this->exactly(count($invocations)))
4144
->method('registerRequestHandler')
@@ -49,6 +52,7 @@ public function test_register_tool_repository(): void
4952
$instance = $server->newInstanceWithoutConstructor();
5053
$server->getProperty('capabilities')->setValue($instance, new ServerCapabilities);
5154
$server->getProperty('protocol')->setValue($instance, $mockProtocol);
55+
$server->getProperty('progressNotifierRepository')->setValue($instance, $mockProgressNotifierRepository);
5256

5357
// Act
5458
$instance->registerToolRepository($mockToolRepository);
@@ -117,11 +121,13 @@ public function test_register_tool_repository_returns_instance(): void
117121
// Arrange
118122
$mockProtocol = $this->createMock(MCPProtocolInterface::class);
119123
$mockToolRepository = $this->createMock(ToolRepository::class);
124+
$mockProgressNotifierRepository = $this->createMock(ProgressNotifierRepository::class);
120125

121126
$server = new ReflectionClass(MCPServer::class);
122127
$instance = $server->newInstanceWithoutConstructor();
123128
$server->getProperty('capabilities')->setValue($instance, new ServerCapabilities);
124129
$server->getProperty('protocol')->setValue($instance, $mockProtocol);
130+
$server->getProperty('progressNotifierRepository')->setValue($instance, $mockProgressNotifierRepository);
125131

126132
// Act
127133
$result = $instance->registerToolRepository($mockToolRepository);
@@ -163,7 +169,8 @@ public function test_create(): void
163169
$version = '1.0';
164170

165171
// Act
166-
$server = MCPServer::create($mockProtocol, $name, $version);
172+
$mockProgressNotifierRepository = $this->createMock(ProgressNotifierRepository::class);
173+
$server = MCPServer::create($mockProtocol, $mockProgressNotifierRepository, $name, $version);
167174

168175
// Assert
169176
$this->assertInstanceOf(MCPServer::class, $server);
@@ -183,7 +190,8 @@ public function test_create_with_capabilities(): void
183190
$mockCapabilities = $this->createMock(ServerCapabilitiesInterface::class);
184191

185192
// Act
186-
$server = MCPServer::create($mockProtocol, $name, $version, $mockCapabilities);
193+
$mockProgressNotifierRepository = $this->createMock(ProgressNotifierRepository::class);
194+
$server = MCPServer::create($mockProtocol, $mockProgressNotifierRepository, $name, $version, $mockCapabilities);
187195

188196
// Assert
189197
$this->assertInstanceOf(MCPServer::class, $server);
@@ -421,7 +429,8 @@ public function test_initialize_sets_capabilities_and_marks_initialized(): void
421429
->method('toInitializeMessage')
422430
->willReturn(['mock-capability' => true]);
423431

424-
$server = MCPServer::create($mockProtocol, $serverInfo['name'], $serverInfo['version'], $mockCapabilities);
432+
$mockProgressNotifierRepository = $this->createMock(ProgressNotifierRepository::class);
433+
$server = MCPServer::create($mockProtocol, $mockProgressNotifierRepository, $serverInfo['name'], $serverInfo['version'], $mockCapabilities);
425434
$initializeData = new InitializeData('2.0', ['mock-capability' => true]);
426435

427436
// Act
@@ -444,7 +453,8 @@ public function test_initialize_throws_when_already_initialized(): void
444453
$serverInfo = ['name' => 'TestServer', 'version' => '1.0'];
445454
$mockCapabilities = $this->createMock(ServerCapabilitiesInterface::class);
446455

447-
$server = MCPServer::create($mockProtocol, $serverInfo['name'], $serverInfo['version'], $mockCapabilities);
456+
$mockProgressNotifierRepository = $this->createMock(ProgressNotifierRepository::class);
457+
$server = MCPServer::create($mockProtocol, $mockProgressNotifierRepository, $serverInfo['name'], $serverInfo['version'], $mockCapabilities);
448458
$initializeData = new InitializeData('2.0', ['mock-capability' => true]);
449459

450460
$server->initialize($initializeData);
@@ -476,7 +486,8 @@ public function test_initialize_returns_correct_resource(): void
476486
->method('toInitializeMessage')
477487
->willReturn(['mock-capability' => true]);
478488

479-
$server = MCPServer::create($mockProtocol, $serverInfo['name'], $serverInfo['version'], $mockCapabilities);
489+
$mockProgressNotifierRepository = $this->createMock(ProgressNotifierRepository::class);
490+
$server = MCPServer::create($mockProtocol, $mockProgressNotifierRepository, $serverInfo['name'], $serverInfo['version'], $mockCapabilities);
480491
$initializeData = new InitializeData('2024-11-05', ['mock-capability' => true]);
481492

482493
// Act

tests/Server/Request/PingHandlerTest.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@
44

55
use KLP\KlpMcpServer\Server\Request\PingHandler;
66
use KLP\KlpMcpServer\Transports\SseTransportInterface;
7+
use PHPUnit\Framework\Attributes\Small;
78
use PHPUnit\Framework\TestCase;
89

10+
#[Small]
911
class PingHandlerTest extends TestCase
1012
{
1113
private SseTransportInterface $transportMock;
@@ -29,6 +31,7 @@ public function test_execute_sends_correct_response(): void
2931
{
3032
$messageId = '12345';
3133
$method = 'ping';
34+
$clientId = 'test-client';
3235
$params = null;
3336

3437
// Expect the transport to send the correct response
@@ -37,7 +40,7 @@ public function test_execute_sends_correct_response(): void
3740
->method('send')
3841
->with($expectedResponse);
3942

40-
$result = $this->pingHandler->execute($method, $messageId, $params);
43+
$result = $this->pingHandler->execute($method, $clientId, $messageId, $params);
4144

4245
// Assert that the method returns an empty array
4346
$this->assertSame([], $result);

tests/Server/Request/ResourcesListHandlerTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public function test_execute_returns_resource_schemas(): void
4141
->method('getResourceSchemas')
4242
->willReturn($expectedSchemas);
4343

44-
$result = $this->resourcesListHandler->execute('resources/list', 123);
44+
$result = $this->resourcesListHandler->execute('resources/list', 'test-client', 123);
4545

4646
$this->assertArrayHasKey('resources', $result);
4747
$this->assertEquals($expectedSchemas, $result['resources']);

tests/Server/Request/ResourcesReadHandlerTest.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public function test_execute_resource_found_with_text_mime_type(): void
4242
->with('test-uri')
4343
->willReturn($resourceMock);
4444

45-
$result = $this->handler->execute('resources/read', 123, ['uri' => 'test-uri']);
45+
$result = $this->handler->execute('resources/read', 'test-client', 123, ['uri' => 'test-uri']);
4646

4747
$this->assertEquals([
4848
'contents' => [
@@ -71,7 +71,7 @@ public function test_execute_resource_found_with_non_text_mime_type(): void
7171
->with('image-uri')
7272
->willReturn($resourceMock);
7373

74-
$result = $this->handler->execute('resources/read', 456, ['uri' => 'image-uri']);
74+
$result = $this->handler->execute('resources/read', 'test-client', 456, ['uri' => 'image-uri']);
7575

7676
$this->assertEquals([
7777
'contents' => [
@@ -94,7 +94,7 @@ public function test_execute_resource_not_found(): void
9494
->with('unknown-uri')
9595
->willReturn(null);
9696

97-
$result = $this->handler->execute('resources/read', 789, ['uri' => 'unknown-uri']);
97+
$result = $this->handler->execute('resources/read', 'test-client', 789, ['uri' => 'unknown-uri']);
9898

9999
$this->assertEquals([], $result);
100100
}

0 commit comments

Comments
 (0)