Merge pull request #1131 from RooVetGit/cte/debug-tests

Debug test flakes

src/core/Cline.ts

@@ -129,8 +129,9 @@ export class Cline {
 		images?: string[] | undefined,
 		historyItem?: HistoryItem | undefined,
 		experiments?: Record<string, boolean>,
+		startTask = true,
 	) {
-		if (!task && !images && !historyItem) {
+		if (startTask && !task && !images && !historyItem) {
 			throw new Error("Either historyItem or task/images must be provided")
 		}
 
@@ -153,13 +154,34 @@ export class Cline {
 		// Initialize diffStrategy based on current state
 		this.updateDiffStrategy(Experiments.isEnabled(experiments ?? {}, EXPERIMENT_IDS.DIFF_STRATEGY))
 
-		if (task || images) {
-			this.startTask(task, images)
-		} else if (historyItem) {
-			this.resumeTaskFromHistory()
+		if (startTask) {
+			if (task || images) {
+				this.startTask(task, images)
+			} else if (historyItem) {
+				this.resumeTaskFromHistory()
+			} else {
+				throw new Error("Either historyItem or task/images must be provided")
+			}
 		}
 	}
 
+	static create(...args: ConstructorParameters<typeof Cline>): [Cline, Promise<void>] {
+		args[10] = false // startTask
+		const instance = new Cline(...args)
+
+		let task
+
+		if (args[6] || args[7]) {
+			task = instance.startTask(args[6], args[7])
+		} else if (args[8]) {
+			task = instance.resumeTaskFromHistory()
+		} else {
+			throw new Error("Either historyItem or task/images must be provided")
+		}
+
+		return [instance, task]
+	}
+
 	// Add method to update diffStrategy
 	async updateDiffStrategy(experimentalDiffStrategy?: boolean) {
 		// If not provided, get from current state
@@ -745,8 +767,12 @@ export class Cline {
 		}
 	}
 
-	async abortTask() {
+	async abortTask(isAbandoned = false) {
 		// Will stop any autonomously running promises.
+		if (isAbandoned) {
+			this.abandoned = true
+		}
+
 		this.abort = true
 
 		this.terminalManager.disposeAll()
@@ -2967,7 +2993,7 @@ export class Cline {
 			}
 
 			// need to call here in case the stream was aborted
-			if (this.abort) {
+			if (this.abort || this.abandoned) {
 				throw new Error("Roo Code instance aborted")
 			}
 

src/core/__tests__/Cline.test.ts

@@ -1,3 +1,5 @@
+// npx jest src/core/__tests__/Cline.test.ts
+
 import { Cline } from "../Cline"
 import { ClineProvider } from "../webview/ClineProvider"
 import { ApiConfiguration, ModelInfo } from "../../shared/api"
@@ -324,8 +326,8 @@ describe("Cline", () => {
 	})
 
 	describe("constructor", () => {
-		it("should respect provided settings", () => {
-			const cline = new Cline(
+		it("should respect provided settings", async () => {
+			const [cline, task] = Cline.create(
 				mockProvider,
 				mockApiConfig,
 				"custom instructions",
@@ -337,10 +339,13 @@ describe("Cline", () => {
 
 			expect(cline.customInstructions).toBe("custom instructions")
 			expect(cline.diffEnabled).toBe(false)
+
+			await cline.abortTask(true)
+			await task.catch(() => {})
 		})
 
-		it("should use default fuzzy match threshold when not provided", () => {
-			const cline = new Cline(
+		it("should use default fuzzy match threshold when not provided", async () => {
+			const [cline, task] = await Cline.create(
 				mockProvider,
 				mockApiConfig,
 				"custom instructions",
@@ -353,12 +358,15 @@ describe("Cline", () => {
 			expect(cline.diffEnabled).toBe(true)
 			// The diff strategy should be created with default threshold (1.0)
 			expect(cline.diffStrategy).toBeDefined()
+
+			await cline.abortTask(true)
+			await task.catch(() => {})
 		})
 
-		it("should use provided fuzzy match threshold", () => {
+		it("should use provided fuzzy match threshold", async () => {
 			const getDiffStrategySpy = jest.spyOn(require("../diff/DiffStrategy"), "getDiffStrategy")
 
-			const cline = new Cline(
+			const [cline, task] = Cline.create(
 				mockProvider,
 				mockApiConfig,
 				"custom instructions",
@@ -373,12 +381,15 @@ describe("Cline", () => {
 			expect(getDiffStrategySpy).toHaveBeenCalledWith("claude-3-5-sonnet-20241022", 0.9, false)
 
 			getDiffStrategySpy.mockRestore()
+
+			await cline.abortTask(true)
+			await task.catch(() => {})
 		})
 
-		it("should pass default threshold to diff strategy when not provided", () => {
+		it("should pass default threshold to diff strategy when not provided", async () => {
 			const getDiffStrategySpy = jest.spyOn(require("../diff/DiffStrategy"), "getDiffStrategy")
 
-			const cline = new Cline(
+			const [cline, task] = Cline.create(
 				mockProvider,
 				mockApiConfig,
 				"custom instructions",
@@ -393,6 +404,9 @@ describe("Cline", () => {
 			expect(getDiffStrategySpy).toHaveBeenCalledWith("claude-3-5-sonnet-20241022", 1.0, false)
 
 			getDiffStrategySpy.mockRestore()
+
+			await cline.abortTask(true)
+			await task.catch(() => {})
 		})
 
 		it("should require either task or historyItem", () => {
@@ -455,7 +469,15 @@ describe("Cline", () => {
 		})
 
 		it("should include timezone information in environment details", async () => {
-			const cline = new Cline(mockProvider, mockApiConfig, undefined, false, false, undefined, "test task")
+			const [cline, task] = Cline.create(
+				mockProvider,
+				mockApiConfig,
+				undefined,
+				false,
+				false,
+				undefined,
+				"test task",
+			)
 
 			const details = await cline["getEnvironmentDetails"](false)
 
@@ -464,11 +486,24 @@ describe("Cline", () => {
 			expect(details).toMatch(/UTC-7:00/) // Fixed offset for America/Los_Angeles
 			expect(details).toContain("# Current Time")
 			expect(details).toMatch(/1\/1\/2024.*5:00:00 AM.*\(America\/Los_Angeles, UTC-7:00\)/) // Full time string format
+
+			await cline.abortTask(true)
+			await task.catch(() => {})
 		})
 
 		describe("API conversation handling", () => {
 			it("should clean conversation history before sending to API", async () => {
-				const cline = new Cline(mockProvider, mockApiConfig, undefined, false, false, undefined, "test task")
+				const [cline, task] = Cline.create(
+					mockProvider,
+					mockApiConfig,
+					undefined,
+					false,
+					false,
+					undefined,
+					"test task",
+				)
+				cline.abandoned = true
+				await task
 
 				// Mock the API's createMessage method to capture the conversation history
 				const createMessageSpy = jest.fn()
@@ -576,7 +611,7 @@ describe("Cline", () => {
 				]
 
 				// Test with model that supports images
-				const clineWithImages = new Cline(
+				const [clineWithImages, taskWithImages] = Cline.create(
 					mockProvider,
 					configWithImages,
 					undefined,
@@ -585,6 +620,7 @@ describe("Cline", () => {
 					undefined,
 					"test task",
 				)
+
 				// Mock the model info to indicate image support
 				jest.spyOn(clineWithImages.api, "getModel").mockReturnValue({
 					id: "claude-3-sonnet",
@@ -598,10 +634,11 @@ describe("Cline", () => {
 						outputPrice: 0.75,
 					} as ModelInfo,
 				})
+
 				clineWithImages.apiConversationHistory = conversationHistory
 
 				// Test with model that doesn't support images
-				const clineWithoutImages = new Cline(
+				const [clineWithoutImages, taskWithoutImages] = Cline.create(
 					mockProvider,
 					configWithoutImages,
 					undefined,
@@ -610,6 +647,7 @@ describe("Cline", () => {
 					undefined,
 					"test task",
 				)
+
 				// Mock the model info to indicate no image support
 				jest.spyOn(clineWithoutImages.api, "getModel").mockReturnValue({
 					id: "gpt-3.5-turbo",
@@ -623,6 +661,7 @@ describe("Cline", () => {
 						outputPrice: 0.2,
 					} as ModelInfo,
 				})
+
 				clineWithoutImages.apiConversationHistory = conversationHistory
 
 				// Mock abort state for both instances
@@ -631,6 +670,7 @@ describe("Cline", () => {
 					set: () => {},
 					configurable: true,
 				})
+
 				Object.defineProperty(clineWithoutImages, "abort", {
 					get: () => false,
 					set: () => {},
@@ -645,6 +685,7 @@ describe("Cline", () => {
 					content,
 					"",
 				])
+
 				// Set up mock streams
 				const mockStreamWithImages = (async function* () {
 					yield { type: "text", text: "test response" }
@@ -672,6 +713,12 @@ describe("Cline", () => {
 					},
 				]
 
+				clineWithImages.abandoned = true
+				await taskWithImages.catch(() => {})
+
+				clineWithoutImages.abandoned = true
+				await taskWithoutImages.catch(() => {})
+
 				// Trigger API requests
 				await clineWithImages.recursivelyMakeClineRequests([{ type: "text", text: "test request" }])
 				await clineWithoutImages.recursivelyMakeClineRequests([{ type: "text", text: "test request" }])
@@ -695,7 +742,15 @@ describe("Cline", () => {
 			})
 
 			it.skip("should handle API retry with countdown", async () => {
-				const cline = new Cline(mockProvider, mockApiConfig, undefined, false, false, undefined, "test task")
+				const [cline, task] = Cline.create(
+					mockProvider,
+					mockApiConfig,
+					undefined,
+					false,
+					false,
+					undefined,
+					"test task",
+				)
 
 				// Mock delay to track countdown timing
 				const mockDelay = jest.fn().mockResolvedValue(undefined)
@@ -809,10 +864,21 @@ describe("Cline", () => {
 				expect(errorMessage).toBe(
 					`${mockError.message}\n\nRetry attempt 1\nRetrying in ${baseDelay} seconds...`,
 				)
+
+				await cline.abortTask(true)
+				await task.catch(() => {})
 			})
 
 			it.skip("should not apply retry delay twice", async () => {
-				const cline = new Cline(mockProvider, mockApiConfig, undefined, false, false, undefined, "test task")
+				const [cline, task] = Cline.create(
+					mockProvider,
+					mockApiConfig,
+					undefined,
+					false,
+					false,
+					undefined,
+					"test task",
+				)
 
 				// Mock delay to track countdown timing
 				const mockDelay = jest.fn().mockResolvedValue(undefined)
@@ -925,11 +991,14 @@ describe("Cline", () => {
 					undefined,
 					false,
 				)
+
+				await cline.abortTask(true)
+				await task.catch(() => {})
 			})
 
 			describe("loadContext", () => {
 				it("should process mentions in task and feedback tags", async () => {
-					const cline = new Cline(
+					const [cline, task] = Cline.create(
 						mockProvider,
 						mockApiConfig,
 						undefined,
@@ -1002,6 +1071,9 @@ describe("Cline", () => {
 					const toolResult2 = processedContent[3] as Anthropic.ToolResultBlockParam
 					const content2 = Array.isArray(toolResult2.content) ? toolResult2.content[0] : toolResult2.content
 					expect((content2 as Anthropic.TextBlockParam).text).toBe("Regular tool result with @/path")
+
+					await cline.abortTask(true)
+					await task.catch(() => {})
 				})
 			})
 		})

src/core/diff/strategies/new-unified/__tests__/edit-strategies.test.ts

@@ -1,5 +1,3 @@
-/// <reference types="jest" />
-
 import { applyContextMatching, applyDMP, applyGitFallback } from "../edit-strategies"
 import { Hunk } from "../types"
 
@@ -277,7 +275,7 @@ describe("applyGitFallback", () => {
 		expect(result.result.join("\n")).toEqual("line1\nnew line2\nline3")
 		expect(result.confidence).toBe(1)
 		expect(result.strategy).toBe("git-fallback")
-	}, 10_000)
+	})
 
 	it("should return original content with 0 confidence when changes cannot be applied", async () => {
 		const hunk = {
@@ -293,5 +291,5 @@ describe("applyGitFallback", () => {
 		expect(result.result).toEqual(content)
 		expect(result.confidence).toBe(0)
 		expect(result.strategy).toBe("git-fallback")
-	}, 10_000)
+	})
 })