Skip to content

Commit dec559b

Browse files
ctecte
committed
One more optimization, and more tests
1 parent 2c9f77f commit dec559b

File tree

4 files changed

+361
-38
lines changed

4 files changed

+361
-38
lines changed

‎src/integrations/terminal/__tests__/TerminalProcessExec.pwsh.test.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,8 +225,6 @@ describePlatform("TerminalProcess with PowerShell Command Output", () => {
225225
beforeAll(() => {
226226
// Initialize TerminalRegistry event handlers
227227
TerminalRegistry.initialize()
228-
// Log environment info
229-
console.log(`Running PowerShell tests with PowerShell Core available: ${hasPwsh}`)
230228
})
231229

232230
beforeEach(() => {

‎src/integrations/terminal/__tests__/setupTerminalTests.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ function isPowerShellCoreAvailable() {
1919
const hasPwsh = isPowerShellCoreAvailable()
2020

2121
// Log environment information
22-
console.log(`Test environment: ${process.platform} ${process.arch}`)
23-
console.log(`PowerShell Core available: ${hasPwsh}`)
22+
// console.log(`Test environment: ${process.platform} ${process.arch}`)
23+
// console.log(`PowerShell Core available: ${hasPwsh}`)
2424

2525
// Define interface for global test environment
2626
declare global {

‎src/shared/__tests__/combineApiRequests.test.ts‎

Lines changed: 312 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
// npx jest src/shared/__tests__/combineApiRequests.test.ts
2+
13
import { combineApiRequests } from "../combineApiRequests"
24
import { ClineMessage } from "../ExtensionMessage"
35

@@ -136,6 +138,21 @@ describe("combineApiRequests", () => {
136138
expect(result).toEqual([])
137139
})
138140

141+
it("should return original array when no API request messages exist", () => {
142+
const messages: ClineMessage[] = [
143+
createOtherMessage("text", "Message 1", 999),
144+
createOtherMessage("task", "Task message", 1000),
145+
createOtherMessage("error", "Error message", 1001),
146+
]
147+
148+
const result = combineApiRequests(messages)
149+
150+
// Should return the original array unchanged
151+
expect(result).toEqual(messages)
152+
// Verify the optimization path was taken (by reference equality)
153+
expect(result).toBe(messages)
154+
})
155+
139156
it("should keep api_req_started message if no matching api_req_finished is found", () => {
140157
const startMessage = createStartMessage()
141158
const messages: ClineMessage[] = [startMessage]
@@ -269,6 +286,228 @@ describe("combineApiRequests", () => {
269286
expect(result[0].reasoning).toBe("This is a test")
270287
expect(result[0].partial).toBe(false)
271288
})
289+
290+
it("should handle invalid JSON in api_req_started message", () => {
291+
const startMessage: ClineMessage = {
292+
type: "say",
293+
say: "api_req_started",
294+
text: "This is not valid JSON",
295+
ts: 1000,
296+
}
297+
const finishMessage = createFinishMessage('{"cost":0.005}', 1001)
298+
299+
const messages: ClineMessage[] = [startMessage, finishMessage]
300+
301+
const result = combineApiRequests(messages)
302+
303+
// Should have one message (the combined one)
304+
expect(result).toHaveLength(1)
305+
306+
// The text should be a JSON string with only the finish message properties
307+
const parsedText = JSON.parse(result[0].text || "{}")
308+
expect(parsedText).toEqual({
309+
cost: 0.005,
310+
})
311+
})
312+
313+
it("should handle invalid JSON in api_req_finished message", () => {
314+
const startMessage = createStartMessage('{"request":"GET /api/data"}', 1000)
315+
const finishMessage: ClineMessage = {
316+
type: "say",
317+
say: "api_req_finished",
318+
text: "This is not valid JSON",
319+
ts: 1001,
320+
}
321+
322+
const messages: ClineMessage[] = [startMessage, finishMessage]
323+
324+
const result = combineApiRequests(messages)
325+
326+
// Should have one message (the combined one)
327+
expect(result).toHaveLength(1)
328+
329+
// The text should be a JSON string with only the start message properties
330+
const parsedText = JSON.parse(result[0].text || "{}")
331+
expect(parsedText).toEqual({
332+
request: "GET /api/data",
333+
})
334+
})
335+
336+
it("should handle non-object JSON in api_req_started message", () => {
337+
const startMessage: ClineMessage = {
338+
type: "say",
339+
say: "api_req_started",
340+
text: '"just a string"', // Valid JSON, but not an object
341+
ts: 1000,
342+
}
343+
const finishMessage = createFinishMessage('{"cost":0.005}', 1001)
344+
345+
const messages: ClineMessage[] = [startMessage, finishMessage]
346+
347+
const result = combineApiRequests(messages)
348+
349+
// Should have one message (the combined one)
350+
expect(result).toHaveLength(1)
351+
352+
// The current implementation spreads string characters into the object
353+
// This test validates the actual behavior
354+
const parsedText = JSON.parse(result[0].text || "{}")
355+
// Check that the cost property exists (from finish message)
356+
expect(parsedText.cost).toBe(0.005)
357+
// Check that string characters got spread (actual implementation behavior)
358+
expect(typeof parsedText["0"]).toBe("string")
359+
})
360+
361+
it("should handle non-object JSON in api_req_finished message", () => {
362+
const startMessage = createStartMessage('{"request":"GET /api/data"}', 1000)
363+
const finishMessage: ClineMessage = {
364+
type: "say",
365+
say: "api_req_finished",
366+
text: '"just a string"', // Valid JSON, but not an object
367+
ts: 1001,
368+
}
369+
370+
const messages: ClineMessage[] = [startMessage, finishMessage]
371+
372+
const result = combineApiRequests(messages)
373+
374+
// Should have one message (the combined one)
375+
expect(result).toHaveLength(1)
376+
377+
// The current implementation spreads string characters into the object
378+
// This test validates the actual behavior
379+
const parsedText = JSON.parse(result[0].text || "{}")
380+
// Check that request property exists (from start message)
381+
expect(parsedText.request).toBe("GET /api/data")
382+
// Check that string characters got spread (actual implementation behavior)
383+
expect(typeof parsedText["0"]).toBe("string")
384+
})
385+
386+
it("should properly merge nested JSON objects", () => {
387+
const startMessage = createStartMessage(
388+
'{"request":"GET /api/data", "metadata": {"source": "user", "priority": "high"}}',
389+
1000,
390+
)
391+
const finishMessage = createFinishMessage(
392+
'{"cost":0.005, "metadata": {"duration": 200, "priority": "override"}}',
393+
1001,
394+
)
395+
396+
const messages: ClineMessage[] = [startMessage, finishMessage]
397+
398+
const result = combineApiRequests(messages)
399+
400+
// Should have one message (the combined one)
401+
expect(result).toHaveLength(1)
402+
403+
// Using shallow merge, nested objects are completely replaced rather than merged
404+
const parsedText = JSON.parse(result[0].text || "{}")
405+
expect(parsedText).toEqual({
406+
request: "GET /api/data",
407+
cost: 0.005,
408+
metadata: {
409+
// 'source' property from start message is lost in shallow merge
410+
priority: "override",
411+
duration: 200,
412+
},
413+
})
414+
})
415+
416+
it("should handle complex JSON objects with multiple properties", () => {
417+
const startMessage = createStartMessage(
418+
'{"request":"GET /api/data", "user": "john", "timestamp": 1616161616, "params": {"page": 1, "limit": 10}}',
419+
1000,
420+
)
421+
const finishMessage = createFinishMessage(
422+
'{"cost":0.005, "duration": 150, "cache": false, "results": {"count": 42, "status": "success"}}',
423+
1001,
424+
)
425+
426+
const messages: ClineMessage[] = [startMessage, finishMessage]
427+
428+
const result = combineApiRequests(messages)
429+
430+
// Should have one message (the combined one)
431+
expect(result).toHaveLength(1)
432+
433+
// All properties should be properly merged
434+
const parsedText = JSON.parse(result[0].text || "{}")
435+
expect(parsedText).toEqual({
436+
request: "GET /api/data",
437+
user: "john",
438+
timestamp: 1616161616,
439+
params: {
440+
page: 1,
441+
limit: 10,
442+
},
443+
cost: 0.005,
444+
duration: 150,
445+
cache: false,
446+
results: {
447+
count: 42,
448+
status: "success",
449+
},
450+
})
451+
})
452+
453+
it("should handle api_req_started and api_req_finished messages that are out of order", () => {
454+
// The finish message appears before the start message in the array
455+
const messages: ClineMessage[] = [
456+
createFinishMessage('{"cost":0.005}', 1001),
457+
createStartMessage('{"request":"GET /api/data"}', 1000),
458+
]
459+
460+
const result = combineApiRequests(messages)
461+
462+
// Should have one message (the original start message)
463+
expect(result).toHaveLength(1)
464+
465+
// The start message should remain unchanged since the finish message appears before it
466+
expect(result[0].say).toBe("api_req_started")
467+
const parsedText = JSON.parse(result[0].text || "{}")
468+
expect(parsedText).toEqual({
469+
request: "GET /api/data",
470+
})
471+
})
472+
473+
it("should handle empty text fields (empty JSON objects)", () => {
474+
const startMessage = createStartMessage("{}", 1000)
475+
const finishMessage = createFinishMessage("{}", 1001)
476+
477+
const messages: ClineMessage[] = [startMessage, finishMessage]
478+
479+
const result = combineApiRequests(messages)
480+
481+
// Should have one message (the combined one)
482+
expect(result).toHaveLength(1)
483+
484+
// The combined text should be an empty object
485+
const parsedText = JSON.parse(result[0].text || "{}")
486+
expect(parsedText).toEqual({})
487+
})
488+
489+
it("should handle undefined text field", () => {
490+
const startMessage = createStartMessage('{"request":"GET /api/data"}', 1000)
491+
const finishMessage: ClineMessage = {
492+
type: "say",
493+
say: "api_req_finished",
494+
text: undefined, // undefined text field
495+
ts: 1001,
496+
}
497+
498+
const messages: ClineMessage[] = [startMessage, finishMessage]
499+
500+
const result = combineApiRequests(messages)
501+
502+
// Should have one message (the combined one)
503+
expect(result).toHaveLength(1)
504+
505+
// The text should be a JSON string with only the start message properties
506+
const parsedText = JSON.parse(result[0].text || "{}")
507+
expect(parsedText).toEqual({
508+
request: "GET /api/data",
509+
})
510+
})
272511
})
273512

274513
describe("Bug verification", () => {
@@ -319,5 +558,78 @@ describe("combineApiRequests", () => {
319558
// No message should have say="api_req_finished"
320559
expect(result.some((msg) => msg.say === "api_req_finished")).toBe(false)
321560
})
561+
562+
it("should handle multiple finish messages for each start message correctly", () => {
563+
const messages: ClineMessage[] = [
564+
createStartMessage('{"request":"GET /api/data1"}', 1000),
565+
createFinishMessage('{"cost":0.005}', 1001),
566+
createFinishMessage('{"duration":150}', 1002), // Should be ignored
567+
createStartMessage('{"request":"GET /api/data2"}', 2000),
568+
createFinishMessage('{"cost":0.007}', 2001),
569+
createFinishMessage('{"duration":200}', 2002), // Should be ignored
570+
]
571+
572+
const result = combineApiRequests(messages)
573+
574+
// Should have two messages (the combined ones)
575+
expect(result).toHaveLength(2)
576+
577+
// Check first combined message
578+
const parsedText1 = JSON.parse(result[0].text || "{}")
579+
expect(parsedText1).toEqual({
580+
request: "GET /api/data1",
581+
cost: 0.005,
582+
})
583+
584+
// Check second combined message
585+
const parsedText2 = JSON.parse(result[1].text || "{}")
586+
expect(parsedText2).toEqual({
587+
request: "GET /api/data2",
588+
cost: 0.007,
589+
})
590+
591+
// No message should have say="api_req_finished"
592+
expect(result.some((msg) => msg.say === "api_req_finished")).toBe(false)
593+
})
594+
595+
it("should handle property overwrites correctly", () => {
596+
const startMessage = createStartMessage('{"request":"GET /api/data", "cost": 0.001}', 1000)
597+
const finishMessage = createFinishMessage('{"cost":0.005, "request": "OVERWRITTEN"}', 1001)
598+
599+
const messages: ClineMessage[] = [startMessage, finishMessage]
600+
601+
const result = combineApiRequests(messages)
602+
603+
// Should have one message (the combined one)
604+
expect(result).toHaveLength(1)
605+
606+
// The finish message properties should overwrite start message properties with the same name
607+
const parsedText = JSON.parse(result[0].text || "{}")
608+
expect(parsedText).toEqual({
609+
request: "OVERWRITTEN", // This was in both messages, finish value wins
610+
cost: 0.005, // This was in both messages, finish value wins
611+
})
612+
})
613+
614+
it("should handle array values in JSON properly", () => {
615+
const startMessage = createStartMessage('{"request":"GET /api/data", "tags": ["api", "get"]}', 1000)
616+
const finishMessage = createFinishMessage('{"cost":0.005, "results": [1, 2, 3]}', 1001)
617+
618+
const messages: ClineMessage[] = [startMessage, finishMessage]
619+
620+
const result = combineApiRequests(messages)
621+
622+
// Should have one message (the combined one)
623+
expect(result).toHaveLength(1)
624+
625+
// Array values should be preserved
626+
const parsedText = JSON.parse(result[0].text || "{}")
627+
expect(parsedText).toEqual({
628+
request: "GET /api/data",
629+
tags: ["api", "get"],
630+
cost: 0.005,
631+
results: [1, 2, 3],
632+
})
633+
})
322634
})
323635
})

0 commit comments

Comments
 (0)