diff --git a/README.md b/README.md index 2fd00d2..5636d90 100644 --- a/README.md +++ b/README.md @@ -73,7 +73,7 @@ if (photo) { console.log(`original at ${path}`); } -lib.close(); +await lib.close(); ``` The lower-level `Client` (login, session serialization, and the raw @@ -563,7 +563,10 @@ and pass it. The three pools default to 10 / 5 / 25 (see Request pools below). `lib.status()` returns a `LibraryStatus` (collection/file counts, last refresh/ML times and errors, originals usage and effective limit, precache progress, and `closed`). `lib.close()` stops the background timer; it is -idempotent, and an in-flight refresh is left to finish. +idempotent, and an in-flight refresh is left to finish. The promise it returns +resolves once that refresh, including its cache write, has finished, so the +cache directory can then be removed; precache fetches already running are not +waited for. ### Default reads vs. fresh reads diff --git a/TODO.md b/TODO.md index e39ebf3..48f0a78 100644 --- a/TODO.md +++ b/TODO.md @@ -18,6 +18,13 @@ Tag v1.0.0. # Completed Steps +- 2026-09-23: Fixed two intermittently failing library tests (issue 90). + `Library.close()` now returns a promise that resolves once an in-flight + refresh, including its cache write, has finished; the library tests await it, + so `afterEach` no longer removes the cache directory while a refresh is still + writing into it. The precache test waits for both fills to report "done" + instead of for its stub source to be called, which happened before the cache + recorded the file. - 2026-09-23: Made the download deadline an idle deadline (issue 24). `downloadTimeoutMs` now aborts a file or thumbnail download only after no bytes have arrived for that long, default 60 seconds, instead of bounding the diff --git a/src/library/index.ts b/src/library/index.ts index 3b184cb..d4427e9 100644 --- a/src/library/index.ts +++ b/src/library/index.ts @@ -560,14 +560,19 @@ export class Library { } // Stop the background timer. Idempotent. An in-flight refresh is left to - // finish; it will not schedule another cycle once closed. - close(): void { + // finish; it will not schedule another cycle once closed. The returned + // promise resolves once that refresh, including its cache write, has + // finished, so a caller can then remove the cache directory. A refresh + // failure is reported through `status()`, not thrown here. Precache + // fetches already running are not waited for. + async close(): Promise { this.closed = true; this.precache?.close(); if (this.timer !== undefined) { clearTimeout(this.timer); this.timer = undefined; } + await this.cycle?.catch(() => {}); } private scheduleNext(): void { diff --git a/test/library/library.test.ts b/test/library/library.test.ts index fe951c3..ff60e25 100644 --- a/test/library/library.test.ts +++ b/test/library/library.test.ts @@ -37,6 +37,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 +198,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 +229,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 +276,7 @@ describe("Library.open and background refresh", () => { { timeout: 2000, interval: 5 }, ); } finally { - lib.close(); + await lib.close(); } }); @@ -303,7 +308,7 @@ describe("Library.open and background refresh", () => { // never re-fetched. expect(client.filesCalls).toEqual([]); } finally { - lib.close(); + await lib.close(); } }); @@ -357,7 +362,7 @@ describe("Library.open and background refresh", () => { { timeout: 2000, interval: 5 }, ); } finally { - lib.close(); + await lib.close(); } }); @@ -402,7 +407,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 +467,7 @@ describe("Library.open and background refresh", () => { { timeout: 2000, interval: 5 }, ); } finally { - lib.close(); + await lib.close(); } }); @@ -488,7 +493,7 @@ describe("Library.open and background refresh", () => { ), ).toBe(true); } finally { - lib.close(); + await lib.close(); } }); @@ -502,9 +507,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(() => {}); + client.collectionsSince = () => + new Promise((resolve) => { + answerFirstFetch = resolve; + }); // open() must resolve from the cache without blocking on the network, // and reads must serve the seeded copy. @@ -517,7 +527,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 +576,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 +631,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 +646,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. @@ -659,7 +671,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; diff --git a/test/library/precache.test.ts b/test/library/precache.test.ts index 2580579..923bffb 100644 --- a/test/library/precache.test.ts +++ b/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(); - const origFetched = new Set(); 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(); + let bothFinished!: () => void; + const precached = new Promise((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(); }); });