Skip to content

Commit f328f6b

Browse files
Improve test coverage for rpc_formatter.go edge cases
Add missing test cases to TestFormatRPCMessage, TestFormatRPCMessageMarkdown, and TestFormatJSONWithoutFields that cover untested branches in rpc_formatter.go: - formatRPCMessage: empty ServerID (no server prefix), empty Payload, and response with error but no payload - formatRPCMessageMarkdown: empty ServerID, empty Payload (no code block), and tools/call with a non-string 'name' field in params (no tool name appended) - formatJSONWithoutFields: nil fieldsToRemove slice and removing non-existent fields Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 8f964e0 commit f328f6b

1 file changed

Lines changed: 94 additions & 0 deletions

File tree

internal/logger/rpc_logger_test.go

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,42 @@ func TestFormatRPCMessage(t *testing.T) {
5555
},
5656
want: []string{"client←tools/call", "200b"},
5757
},
58+
{
59+
name: "empty server ID omits server prefix",
60+
info: &RPCMessageInfo{
61+
Direction: RPCDirectionOutbound,
62+
MessageType: RPCMessageRequest,
63+
ServerID: "",
64+
Method: "tools/list",
65+
PayloadSize: 30,
66+
Payload: `{"method":"tools/list"}`,
67+
},
68+
want: []string{"30b", `{"method":"tools/list"}`},
69+
},
70+
{
71+
name: "empty payload omits payload from output",
72+
info: &RPCMessageInfo{
73+
Direction: RPCDirectionOutbound,
74+
MessageType: RPCMessageRequest,
75+
ServerID: "github",
76+
Method: "tools/list",
77+
PayloadSize: 0,
78+
Payload: "",
79+
},
80+
want: []string{"github→tools/list", "0b"},
81+
},
82+
{
83+
name: "response with error and no payload",
84+
info: &RPCMessageInfo{
85+
Direction: RPCDirectionInbound,
86+
MessageType: RPCMessageResponse,
87+
ServerID: "backend",
88+
PayloadSize: 0,
89+
Payload: "",
90+
Error: "connection reset",
91+
},
92+
want: []string{"backend←resp", "0b", "err:connection reset"},
93+
},
5894
}
5995

6096
for _, tt := range tests {
@@ -177,6 +213,46 @@ func TestFormatRPCMessageMarkdown(t *testing.T) {
177213
want: []string{"**github**→`tools/call`", "```json", `"params"`},
178214
notWant: []string{`"jsonrpc"`, `"method"`},
179215
},
216+
{
217+
name: "tools/call with non-string name in params does not append tool name",
218+
info: &RPCMessageInfo{
219+
Direction: RPCDirectionOutbound,
220+
MessageType: RPCMessageRequest,
221+
ServerID: "github",
222+
Method: "tools/call",
223+
PayloadSize: 60,
224+
Payload: `{"jsonrpc":"2.0","method":"tools/call","params":{"name":42,"arguments":{}}}`,
225+
},
226+
// name is not a string so tool name extraction should not add backtick-wrapped name
227+
want: []string{"**github**→`tools/call`"},
228+
notWant: []string{"**github**→`tools/call` `"},
229+
},
230+
{
231+
name: "empty server ID produces no server prefix",
232+
info: &RPCMessageInfo{
233+
Direction: RPCDirectionOutbound,
234+
MessageType: RPCMessageRequest,
235+
ServerID: "",
236+
Method: "tools/list",
237+
PayloadSize: 40,
238+
Payload: `{"jsonrpc":"2.0","method":"tools/list","params":{"key":"val"}}`,
239+
},
240+
want: []string{"```json", `"params"`},
241+
notWant: []string{"**"},
242+
},
243+
{
244+
name: "empty payload produces no code block",
245+
info: &RPCMessageInfo{
246+
Direction: RPCDirectionOutbound,
247+
MessageType: RPCMessageRequest,
248+
ServerID: "github",
249+
Method: "tools/list",
250+
PayloadSize: 0,
251+
Payload: "",
252+
},
253+
want: []string{"**github**→`tools/list`"},
254+
notWant: []string{"```json"},
255+
},
180256
}
181257

182258
for _, tt := range tests {
@@ -258,6 +334,24 @@ func TestFormatJSONWithoutFields(t *testing.T) {
258334
wantValid: true,
259335
wantEmpty: false,
260336
},
337+
{
338+
name: "nil fields to remove leaves all fields",
339+
input: `{"jsonrpc":"2.0","method":"tools/list","id":1}`,
340+
fieldsToRemove: nil,
341+
wantContains: []string{`"jsonrpc"`, `"method"`, `"id"`},
342+
wantNotContain: []string{},
343+
wantValid: true,
344+
wantEmpty: false,
345+
},
346+
{
347+
name: "removing non-existent field is a no-op",
348+
input: `{"id":1,"result":{}}`,
349+
fieldsToRemove: []string{"jsonrpc", "method"},
350+
wantContains: []string{`"id"`, `"result"`},
351+
wantNotContain: []string{`"jsonrpc"`, `"method"`},
352+
wantValid: true,
353+
wantEmpty: false,
354+
},
261355
}
262356

263357
for _, tt := range tests {

0 commit comments

Comments
 (0)