From 1b8775657f86bf6935212639391079eecf88d575 Mon Sep 17 00:00:00 2001
From: Matthew Mone <mmone@vernier.com>
Date: Mon, 22 Dec 2025 14:25:32 -0800
Subject: [PATCH] Refactor Persistence tests to improve mock handling of
 indexedDB. Introduce createMockRequest function for better request management
 and ensure proper triggering of success and upgrade events during database
 initialization. Update PostProcessing tests to clarify grid setup for
 floodFill functionality.

---
 test/core/Persistence.test.js          | 69 +++++++++++++++++---------
 test/generation/PostProcessing.test.js |  5 +-
 2 files changed, 49 insertions(+), 25 deletions(-)

diff --git a/test/core/Persistence.test.js b/test/core/Persistence.test.js
index c332beb..fdf0c2e 100644
--- a/test/core/Persistence.test.js
+++ b/test/core/Persistence.test.js
@@ -32,7 +32,12 @@ describe("Core: Persistence", () => {
       transaction: sinon.stub().returns(mockTransaction),
     };
 
-    // Mock indexedDB.open
+    // Use window or self for browser environment
+    globalObj = typeof window !== 'undefined' ? window : (typeof self !== 'undefined' ? self : globalThis);
+  });
+
+  const createMockRequest = () => {
+    // Mock indexedDB.open - create a new request each time
     mockRequest = {
       onerror: null,
       onsuccess: null,
@@ -40,30 +45,35 @@ describe("Core: Persistence", () => {
       result: mockDB,
     };
 
-    // Use window or self for browser environment
-    globalObj = typeof window !== 'undefined' ? window : (typeof self !== 'undefined' ? self : globalThis);
-    globalObj.indexedDB = {
-      open: sinon.stub().returns(mockRequest),
-    };
-  });
+    // Mock indexedDB using defineProperty since it's read-only
+    Object.defineProperty(globalObj, 'indexedDB', {
+      value: {
+        open: sinon.stub().returns(mockRequest),
+      },
+      writable: true,
+      configurable: true,
+    });
 
-  const triggerSuccess = () => {
-    if (mockRequest.onsuccess) {
-      mockRequest.onsuccess({ target: { result: mockDB } });
-    }
-  };
-
-  const triggerUpgrade = () => {
-    if (mockRequest.onupgradeneeded) {
-      mockRequest.onupgradeneeded({ target: { result: mockDB } });
-    }
+    return mockRequest;
   };
 
   it("CoA 1: init should create database and object stores", async () => {
-    triggerUpgrade();
-    triggerSuccess();
+    const request = createMockRequest();
+    
+    // Start init, which will call indexedDB.open
+    const initPromise = persistence.init();
+    
+    // Trigger upgrade first (happens synchronously during open)
+    if (request.onupgradeneeded) {
+      request.onupgradeneeded({ target: { result: mockDB } });
+    }
+    
+    // Then trigger success
+    if (request.onsuccess) {
+      request.onsuccess({ target: { result: mockDB } });
+    }
 
-    await persistence.init();
+    await initPromise;
 
     expect(globalObj.indexedDB.open.calledWith("AetherShardsDB", 2)).to.be.true;
     expect(mockDB.createObjectStore.calledWith("Runs", { keyPath: "id" })).to.be.true;
@@ -194,17 +204,28 @@ describe("Core: Persistence", () => {
   });
 
   it("CoA 8: saveRun should auto-init if db not initialized", async () => {
-    triggerUpgrade();
-    triggerSuccess();
-
+    const request = createMockRequest();
+    
     const runData = { seed: 12345 };
     const mockPutRequest = {
-      onsuccess: null,
+      onsuccess: sinon.stub(),
       onerror: null,
     };
     mockStore.put.returns(mockPutRequest);
 
+    // Start saveRun, which will trigger init
     const savePromise = persistence.saveRun(runData);
+    
+    // Trigger upgrade and success for init
+    if (request.onupgradeneeded) {
+      request.onupgradeneeded({ target: { result: mockDB } });
+    }
+    if (request.onsuccess) {
+      request.onsuccess({ target: { result: mockDB } });
+    }
+    
+    // Wait a bit for init to complete, then trigger put success
+    await new Promise(resolve => setTimeout(resolve, 10));
     mockPutRequest.onsuccess();
 
     await savePromise;
diff --git a/test/generation/PostProcessing.test.js b/test/generation/PostProcessing.test.js
index f13cb74..6ca5199 100644
--- a/test/generation/PostProcessing.test.js
+++ b/test/generation/PostProcessing.test.js
@@ -82,6 +82,9 @@ describe("Generation: PostProcessor", () => {
   });
 
   it("CoA 4: floodFill should not include disconnected tiles", () => {
+    // Fill entire grid with solid first to isolate regions
+    grid.fill(1);
+    
     // Create two separate regions with proper floor setup
     // Region 1: connected tiles
     grid.setCell(1, 0, 1, 1); // Floor
@@ -89,7 +92,7 @@ describe("Generation: PostProcessor", () => {
     grid.setCell(2, 0, 1, 1); // Floor
     grid.setCell(2, 1, 1, 0); // Air (connected)
 
-    // Region 2: disconnected (no floor connection)
+    // Region 2: disconnected (surrounded by solid, no path to region 1)
     grid.setCell(10, 0, 10, 1); // Floor
     grid.setCell(10, 1, 10, 0); // Air (disconnected)