From 68a8d2788d11ffed4178614701d3dd8cda31b314 Mon Sep 17 00:00:00 2001 From: Abi Raja Date: Fri, 8 Dec 2023 16:48:34 -0500 Subject: [PATCH] improve tests and catch history tree generation errors better --- frontend/src/App.tsx | 15 +++++++--- frontend/src/components/history/utils.test.ts | 30 +++++++++++++++---- frontend/src/components/history/utils.ts | 4 +-- 3 files changed, 37 insertions(+), 12 deletions(-) diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index f1301e0..95cd8d3 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -211,10 +211,17 @@ function App() { return; } - const updatedHistory = [ - ...extractHistoryTree(appHistory, currentVersion), - updateInstruction, - ]; + let historyTree; + try { + historyTree = extractHistoryTree(appHistory, currentVersion); + } catch { + toast.error( + "Version history is invalid. This shouldn't happen. Please contact support or open a Github issue." + ); + return; + } + + const updatedHistory = [...historyTree, updateInstruction]; if (shouldIncludeResultImage) { const resultImage = await takeScreenshot(); diff --git a/frontend/src/components/history/utils.test.ts b/frontend/src/components/history/utils.test.ts index 0dc26bd..2abaf90 100644 --- a/frontend/src/components/history/utils.test.ts +++ b/frontend/src/components/history/utils.test.ts @@ -53,7 +53,26 @@ const longerBranchingHistory: History = [ }, ]; -test("should only include history from this point onward", () => { +const basicBadHistory: History = [ + { + type: "ai_create", + parentIndex: null, + code: "1. create", + inputs: { + image_url: "", + }, + }, + { + type: "ai_edit", + parentIndex: 2, // <- Bad parent index + code: "2. edit with better icons", + inputs: { + prompt: "use better icons", + }, + }, +]; + +test("should correctly extract the history tree", () => { expect(extractHistoryTree(basicLinearHistory, 2)).toEqual([ "1. create", "use better icons", @@ -93,11 +112,12 @@ test("should only include history from this point onward", () => { "3. edit with better icons and red text", ]); - // Errors - TODO: Handle these + // Errors + // Bad index - // TODO: Throw an exception instead? - expect(extractHistoryTree(basicLinearHistory, 100)).toEqual([]); - expect(extractHistoryTree(basicLinearHistory, -2)).toEqual([]); + expect(() => extractHistoryTree(basicLinearHistory, 100)).toThrow(); + expect(() => extractHistoryTree(basicLinearHistory, -2)).toThrow(); // Bad tree + expect(() => extractHistoryTree(basicBadHistory, 1)).toThrow(); }); diff --git a/frontend/src/components/history/utils.ts b/frontend/src/components/history/utils.ts index cfdfbbc..ce7bf3c 100644 --- a/frontend/src/components/history/utils.ts +++ b/frontend/src/components/history/utils.ts @@ -22,9 +22,7 @@ export function extractHistoryTree( // Move to the parent of the current item currentIndex = item.parentIndex; } else { - // TODO: Throw an exception here? - // Break the loop if the item is not found (should not happen in a well-formed history) - break; + throw new Error("Malformed history: missing parent index"); } }