Fix two intermittently failing library tests (closes #90)
check / check (push) Successful in 27s

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 and CLI command now awaits close(), and
tests hold each of the three writes 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
This commit was merged in pull request #92.
This commit is contained in:
2026-09-23 04:38:00 +02:00
parent c75c4f987c
commit f1836ced57
12 changed files with 278 additions and 77 deletions
+89 -17
View File
@@ -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;