Skip to content

Commit 922c55c

Browse files
Copilotstephentoub
andcommitted
Improve test comments for spec compliance clarity
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
1 parent 71692e3 commit 922c55c

1 file changed

Lines changed: 16 additions & 10 deletions

File tree

tests/ModelContextProtocol.Tests/Protocol/JsonRpcMessageConverterTests.cs

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -306,13 +306,15 @@ public static void RoundTrip_Error_PreservesData()
306306
[Fact]
307307
public static void Deserialize_ResponseWithExplicitNullError_TreatedAsSuccessResponse()
308308
{
309-
// Arrange - A valid response with explicit "error": null should be treated as success response
309+
// Arrange - Some implementations may include "error": null in success responses.
310+
// While JSON-RPC 2.0 spec says responses have either result OR error (not both),
311+
// this tests that we handle the lenient case gracefully.
310312
string json = """{"jsonrpc":"2.0","id":1,"result":{"data":"value"},"error":null}""";
311313

312314
// Act
313315
var message = JsonSerializer.Deserialize<JsonRpcMessage>(json, McpJsonUtilities.DefaultOptions);
314316

315-
// Assert - Should be a success response, not an error
317+
// Assert - Should be a success response since error is null
316318
Assert.NotNull(message);
317319
Assert.IsType<JsonRpcResponse>(message);
318320
var response = (JsonRpcResponse)message;
@@ -324,13 +326,14 @@ public static void Deserialize_ResponseWithExplicitNullError_TreatedAsSuccessRes
324326
[Fact]
325327
public static void Deserialize_ResponseWithNullResultAndNullError_TreatedAsSuccessWithNullResult()
326328
{
327-
// Arrange - Both result and error are explicitly null
329+
// Arrange - Both result and error are explicitly null.
330+
// Per JSON-RPC 2.0, result: null is a valid success response value.
328331
string json = """{"jsonrpc":"2.0","id":1,"result":null,"error":null}""";
329332

330333
// Act
331334
var message = JsonSerializer.Deserialize<JsonRpcMessage>(json, McpJsonUtilities.DefaultOptions);
332335

333-
// Assert - result: null is a valid success response
336+
// Assert - result: null is valid, error: null is ignored
334337
Assert.NotNull(message);
335338
Assert.IsType<JsonRpcResponse>(message);
336339
var response = (JsonRpcResponse)message;
@@ -339,15 +342,17 @@ public static void Deserialize_ResponseWithNullResultAndNullError_TreatedAsSucce
339342
}
340343

341344
[Fact]
342-
public static void Deserialize_ResponseWithErrorBeforeResult_ErrorTakesPrecedence()
345+
public static void Deserialize_ResponseWithBothErrorAndResult_ErrorTakesPrecedence()
343346
{
344-
// Arrange - Error property comes before result in JSON (error takes precedence)
347+
// Arrange - JSON-RPC 2.0 spec says a response should have either result OR error, not both.
348+
// However, if a non-compliant implementation sends both, we verify consistent behavior:
349+
// error takes precedence regardless of property order.
345350
string json = """{"jsonrpc":"2.0","id":1,"error":{"code":-32600,"message":"Invalid"},"result":{"data":"ignored"}}""";
346351

347352
// Act
348353
var message = JsonSerializer.Deserialize<JsonRpcMessage>(json, McpJsonUtilities.DefaultOptions);
349354

350-
// Assert - Should be an error response since error takes precedence
355+
// Assert - Error takes precedence
351356
Assert.NotNull(message);
352357
Assert.IsType<JsonRpcError>(message);
353358
var error = (JsonRpcError)message;
@@ -356,15 +361,16 @@ public static void Deserialize_ResponseWithErrorBeforeResult_ErrorTakesPrecedenc
356361
}
357362

358363
[Fact]
359-
public static void Deserialize_ResponseWithResultBeforeError_ErrorTakesPrecedence()
364+
public static void Deserialize_ResponseWithBothResultAndError_ErrorTakesPrecedenceRegardlessOfOrder()
360365
{
361-
// Arrange - Result property comes before error in JSON (error still takes precedence)
366+
// Arrange - Same as above but with result appearing before error in the JSON.
367+
// Validates that property order doesn't affect the precedence logic.
362368
string json = """{"jsonrpc":"2.0","id":1,"result":{"data":"ignored"},"error":{"code":-32600,"message":"Invalid"}}""";
363369

364370
// Act
365371
var message = JsonSerializer.Deserialize<JsonRpcMessage>(json, McpJsonUtilities.DefaultOptions);
366372

367-
// Assert - Should be an error response since error takes precedence regardless of order
373+
// Assert - Error still takes precedence
368374
Assert.NotNull(message);
369375
Assert.IsType<JsonRpcError>(message);
370376
var error = (JsonRpcError)message;

0 commit comments

Comments
 (0)