Fix two intermittently failing library tests (closes #90)

Library.close() now returns a promise that resolves once the work it
started has finished: an in-flight refresh with its cache write, the ML
data fetch, and running precache sweeps. The interval test could see a
refresh's new state, close, and remove the directory while the write was
still running. Every library test now awaits close(), and a new test
holds a cache write open to prove close() waits for it.

The precache test waited for its stub source to be called, but the cache
records a file only after checking it on disk, so status() could lag. It
now waits for both fills to report "done".

Model: opus-5-5

test/library/content-library.test.ts

@@ -116,7 +116,7 @@ describe("Library content wiring", () => {
         expect(lib.photos.byID({ fileID: 1 })!.record().thumbnailPath).toBe(
             result.path,
         );
-        lib.close();
+        await lib.close();
     });
 
     it("drives thumbnails.ensure through the cache", async () => {
@@ -139,7 +139,7 @@ describe("Library content wiring", () => {
         expect(results).toEqual([
             { fileID: 1, path: join(root, "cache", "thumbnails", "1.jpg") },
         ]);
-        lib.close();
+        await lib.close();
     });
 
     it("throws from content methods when opened without a content source", async () => {
@@ -155,6 +155,6 @@ describe("Library content wiring", () => {
         await expect(
             lib.thumbnails.ensure({ fileIDs: [1], priority: "visible" }),
         ).rejects.toThrow(/content cache/i);
-        lib.close();
+        await lib.close();
     });
 });

test/library/fresh.test.ts

@@ -179,7 +179,7 @@ describe("Library.fresh", () => {
             // And the change is now live for the default namespaces too.
             expect(lib.photos.byID({ fileID: 1002 })?.fileID).toBe(1002);
         } finally {
-            lib.close();
+            await lib.close();
         }
     });
 
@@ -228,7 +228,7 @@ describe("Library.fresh", () => {
                 expect(reads.photos.byID({ fileID: 1002 })?.fileID).toBe(1002);
             }
         } finally {
-            lib.close();
+            await lib.close();
         }
     });
 
@@ -266,7 +266,7 @@ describe("Library.fresh", () => {
             );
             expect(lib.status().lastError).toBeUndefined();
         } finally {
-            lib.close();
+            await lib.close();
         }
     });
 });

test/library/library.test.ts

@@ -17,7 +17,8 @@
  *    failure surfaces via `onProgress` ("failed") and `status()`, and a later
  *    success clears the error. `open()` itself resolves even when the first
  *    refresh fails (offline start from cache).
- * 5. `close()` stops the timer and is idempotent.
+ * 5. `close()` stops the timer and is idempotent, and its promise resolves
+ *    only once an in-flight refresh has written the cache file.
  * 6. `cacheDirectory` defaults to the env-paths cache dir plus the user id.
  * 7. `open()` branches on the cache: an empty cache awaits the first refresh
  *    (it has nothing to serve yet); an existing cache serves its copy at once
@@ -37,6 +38,11 @@
  * short interval and `vi.waitFor`: a fake clock cannot settle the real
  * fsync-and-rename cache write, and empty diffs never write, so the eventual
  * state is stable to poll for.
+ *
+ * A refresh changes RAM before it writes the cache file, so a polled state can
+ * be visible while that write is still running. Every test therefore awaits
+ * `close()`, which waits for the in-flight refresh, before `afterEach` removes
+ * the directory.
  */
 
 import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
@@ -193,7 +199,7 @@ describe("Library.open and background refresh", () => {
             expect(reloaded.getFile(1, 1001)?.id).toBe(1001);
             expect(reloaded.collectionsSinceTime).toBe(100);
         } finally {
-            lib.close();
+            await lib.close();
         }
     });
 
@@ -224,7 +230,7 @@ describe("Library.open and background refresh", () => {
             expect(client.collectionsSinceTimes.length).toBe(collectionCalls);
             expect(client.filesCalls.length).toBe(fileCalls);
         } finally {
-            lib.close();
+            await lib.close();
         }
     });
 
@@ -271,7 +277,7 @@ describe("Library.open and background refresh", () => {
                 { timeout: 2000, interval: 5 },
             );
         } finally {
-            lib.close();
+            await lib.close();
         }
     });
 
@@ -303,7 +309,7 @@ describe("Library.open and background refresh", () => {
             // never re-fetched.
             expect(client.filesCalls).toEqual([]);
         } finally {
-            lib.close();
+            await lib.close();
         }
     });
 
@@ -357,7 +363,7 @@ describe("Library.open and background refresh", () => {
                 { timeout: 2000, interval: 5 },
             );
         } finally {
-            lib.close();
+            await lib.close();
         }
     });
 
@@ -402,7 +408,7 @@ describe("Library.open and background refresh", () => {
             await new Promise((r) => setTimeout(r, FAST_INTERVAL * 1000 * 4));
             expect(saveSpy).toHaveBeenCalledTimes(2);
         } finally {
-            lib.close();
+            await lib.close();
             saveSpy.mockRestore();
         }
     });
@@ -462,7 +468,7 @@ describe("Library.open and background refresh", () => {
                 { timeout: 2000, interval: 5 },
             );
         } finally {
-            lib.close();
+            await lib.close();
         }
     });
 
@@ -488,7 +494,7 @@ describe("Library.open and background refresh", () => {
                 ),
             ).toBe(true);
         } finally {
-            lib.close();
+            await lib.close();
         }
     });
 
@@ -502,9 +508,14 @@ describe("Library.open and background refresh", () => {
         seed.putFile(file(1001, 1, 400));
         await seed.save();
 
-        // The server never answers this run's first refresh.
+        // The server does not answer this run's first refresh until the test
+        // is done with it.
+        let answerFirstFetch: (page: CollectionsPage) => void = () => {};
         const client = new MockClient();
-        client.collectionsSince = () => new Promise<CollectionsPage>(() => {});
+        client.collectionsSince = () =>
+            new Promise<CollectionsPage>((resolve) => {
+                answerFirstFetch = resolve;
+            });
 
         // open() must resolve from the cache without blocking on the network,
         // and reads must serve the seeded copy.
@@ -517,7 +528,9 @@ describe("Library.open and background refresh", () => {
             expect(lib.status().lastRefreshAt).toBeUndefined();
             expect(lib.status().lastError).toBeUndefined();
         } finally {
-            lib.close();
+            // close() waits for the outstanding refresh, so let it finish.
+            answerFirstFetch({ collections: [], deleted: [], cursor: 500 });
+            await lib.close();
         }
     });
 
@@ -564,7 +577,7 @@ describe("Library.open and background refresh", () => {
             expect(lib.listFiles(1).map((f) => f.id)).toEqual([1001]);
             expect(lib.status().lastRefreshAt).toBeGreaterThan(0);
         } finally {
-            lib.close();
+            await lib.close();
         }
     });
 
@@ -619,7 +632,7 @@ describe("Library.open and background refresh", () => {
             );
             expect(reloaded.getFile(1, 1001)?.id).toBe(1001);
         } finally {
-            lib.close();
+            await lib.close();
             saveSpy.mockRestore();
         }
     });
@@ -634,8 +647,8 @@ describe("Library.open and background refresh", () => {
         });
         const callsAfterOpen = client.collectionsSinceTimes.length;
 
-        lib.close();
-        lib.close(); // second close must not throw
+        await lib.close();
+        await lib.close(); // second close must not throw
         expect(lib.status().closed).toBe(true);
 
         // No further refreshes fire once closed.
@@ -643,6 +656,65 @@ describe("Library.open and background refresh", () => {
         expect(client.collectionsSinceTimes.length).toBe(callsAfterOpen);
     });
 
+    it("close() resolves only after an in-flight refresh has written the cache", async () => {
+        const path = join(cacheDirectory, "metadata.json");
+        const seed = await MetadataStore.load(path);
+        seed.userID = USER_ID;
+        seed.collectionsSinceTime = 500;
+        seed.putCollection(collection(1, 400));
+        seed.putFile(file(1001, 1, 400));
+        await seed.save();
+
+        const client = new MockClient();
+        client.collectionsQueue.push({
+            collections: [collection(1, 600)],
+            deleted: [],
+            cursor: 600,
+        });
+        client.filesFor(1, {
+            files: [file(1002, 1, 600)],
+            deleted: [],
+            cursor: 600,
+        });
+
+        // Hold the refresh's cache write until the test releases it.
+        const realSave = MetadataStore.prototype.save;
+        let releaseSave: () => void = () => {};
+        const saveHeld = new Promise<void>((resolve) => {
+            releaseSave = resolve;
+        });
+        const saveSpy = vi
+            .spyOn(MetadataStore.prototype, "save")
+            .mockImplementation(async function (this: MetadataStore) {
+                await saveHeld;
+                return realSave.call(this);
+            });
+
+        const lib = await Library.open({ client, cacheDirectory });
+        try {
+            await vi.waitFor(() => expect(saveSpy).toHaveBeenCalled(), {
+                timeout: 2000,
+                interval: 5,
+            });
+
+            let closed = false;
+            const closing = lib.close().then(() => {
+                closed = true;
+            });
+            await new Promise((r) => setTimeout(r, 50));
+            expect(closed).toBe(false);
+
+            releaseSave();
+            await closing;
+            const reloaded = await MetadataStore.load(path);
+            expect(reloaded.getFile(1, 1002)?.id).toBe(1002);
+        } finally {
+            releaseSave();
+            await lib.close();
+            saveSpy.mockRestore();
+        }
+    });
+
     it("defaults cacheDirectory to the env-paths cache dir plus user id", async () => {
         const xdg = join(dir, "xdg-cache");
         const prev = process.env.XDG_CACHE_HOME;
@@ -659,7 +731,7 @@ describe("Library.open and background refresh", () => {
                 expect(lib.cacheDirectory.startsWith(xdg)).toBe(true);
                 expect(lib.cacheDirectory.endsWith(String(USER_ID))).toBe(true);
             } finally {
-                lib.close();
+                await lib.close();
             }
         } finally {
             if (prev === undefined) delete process.env.XDG_CACHE_HOME;

test/library/mldata.test.ts

@@ -374,7 +374,7 @@ describe("Library ML-data fetch on refresh", () => {
             await new Promise((r) => setTimeout(r, FAST_INTERVAL * 1000 * 5));
             expect(client.mlFetchCalls.length).toBe(callsAfterFirst);
         } finally {
-            lib.close();
+            await lib.close();
         }
     });
 
@@ -443,7 +443,7 @@ describe("Library ML-data fetch on refresh", () => {
             expect(client.mlFetchCalls.length).toBeGreaterThan(callsBefore);
             expect(client.mlFetchCalls.flat()).toContain(1001);
         } finally {
-            lib.close();
+            await lib.close();
         }
     });
 });

test/library/precache.test.ts

@@ -402,35 +402,47 @@ describe("Precache through Library.open", () => {
     }
 
     it("starts both precaches from open() and reports them in status()", async () => {
-        const thumbFetched = new Set<number>();
-        const origFetched = new Set<number>();
         const source: ContentSource = {
-            original: async ({ file: f, destination }) => {
-                origFetched.add(f.id);
+            original: async ({ destination }) => {
                 await writeFile(destination, Buffer.alloc(10, 1));
                 return { bytesWritten: 10 };
             },
-            thumbnail: async ({ file: f, destination }) => {
-                thumbFetched.add(f.id);
+            thumbnail: async ({ destination }) => {
                 await writeFile(destination, Buffer.alloc(10, 1));
                 return { bytesWritten: 10 };
             },
         };
+        // Each fill reports "done" once the cache has recorded its files. The
+        // source returning is not enough: the cache records a file only after
+        // it has checked it on disk.
+        const finished = new Set<string>();
+        let bothFinished!: () => void;
+        const precached = new Promise<void>((r) => (bothFinished = r));
         const lib = await Library.open({
             client: new MockClient(),
             cacheDirectory: join(root, "cache"),
             contentSource: source,
             refreshIntervalSeconds: 3600,
+            onProgress: (e) => {
+                if (
+                    e.status === "done" &&
+                    (e.operation === "precacheThumbnails" ||
+                        e.operation === "precacheOriginals")
+                ) {
+                    finished.add(e.operation);
+                    if (finished.size === 2) bothFinished();
+                }
+            },
         });
 
         // Every file's thumbnail is precached; the favorite (file 3) and the
         // week's files (1, 2) all have their originals precached.
-        await until(() => thumbFetched.size === 3 && origFetched.size === 3);
+        await precached;
         const status = lib.status();
         expect(status.thumbnailsTotal).toBe(3);
         expect(status.thumbnailsCached).toBe(3);
         expect(status.originalsPinned).toBe(3);
         expect(status.originalsCached).toBe(3);
-        lib.close();
+        await lib.close();
     });
 });

test/library/read.test.ts

@@ -531,7 +531,7 @@ describe("Library exposes the read surface over its live store", () => {
             expect(client.collectionsCalls).toBe(collectionsBefore);
             expect(client.filesCalls).toBe(filesBefore);
         } finally {
-            lib.close();
+            await lib.close();
         }
     });
 });

test/library/snapshot.test.ts

@@ -159,7 +159,7 @@ describe("Library.snapshot and Library.subscribe", () => {
             for (const p of snap.photos) expect("key" in p).toBe(false);
             for (const a of snap.albums) expect("key" in a).toBe(false);
         } finally {
-            lib.close();
+            await lib.close();
         }
     });
 
@@ -219,7 +219,7 @@ describe("Library.snapshot and Library.subscribe", () => {
             expect(change.refreshedAt).toBeGreaterThan(0);
         } finally {
             unsubscribe();
-            lib.close();
+            await lib.close();
         }
     });
 
@@ -251,7 +251,7 @@ describe("Library.snapshot and Library.subscribe", () => {
             expect(changes).toEqual([]);
         } finally {
             unsubscribe();
-            lib.close();
+            await lib.close();
         }
     });
 
@@ -297,7 +297,7 @@ describe("Library.snapshot and Library.subscribe", () => {
             );
             expect(changes).toEqual([]);
         } finally {
-            lib.close();
+            await lib.close();
         }
     });
 });