diff --git a/src/__tests__/server-app.test.ts b/src/__tests__/server-app.test.ts index 200a36e2..52b34e55 100644 --- a/src/__tests__/server-app.test.ts +++ b/src/__tests__/server-app.test.ts @@ -955,52 +955,22 @@ function parsedFeed(url: string, hubUrl?: string): ParsedFeed { }; } -// The fan-out below is about to move out of the route into a named service. -// Its contract has never been asserted, only its auth and its failure modes. -test("find marks each candidate with whether it advertises a WebSub hub", async () => { +test("find marks push feeds and preserves plain or failed candidates", async () => { const dependencies = createDependencies(); authenticated(dependencies); dependencies.httpClient.get = async () => ({ data: ` - `, - }); - dependencies.feedParser.parseUrl = async (url: string) => - parsedFeed(url, url.includes("push") ? "https://hub.example/" : undefined); - const app = await appFor(dependencies); - - const response = await app.handle( - new Request( - "http://localhost/api/find?link=https%3A%2F%2Fsite.example%2F", - { - headers: { cookie: "sid=test" }, - }, - ), - ); - - expect(response.status).toBe(200); - expect(await response.json()).toEqual([ - { title: "Push", url: "https://site.example/push.xml", websub: true }, - { title: "Plain", url: "https://site.example/plain.xml", websub: false }, - ]); -}); - -// A dead candidate is worth showing: the user finds out when they preview it, -// and dropping it silently would make the page look like the feed never -// existed. -test("find keeps a candidate whose parse throws, merely unmarked", async () => { - const dependencies = createDependencies(); - authenticated(dependencies); - dependencies.httpClient.get = async () => ({ - data: ` - `, }); dependencies.feedParser.parseUrl = async (url: string) => { if (url.includes("dead")) throw new Error("404"); - return parsedFeed(url, "https://hub.example/"); + return parsedFeed( + url, + url.includes("push") ? "https://hub.example/" : undefined, + ); }; const app = await appFor(dependencies); @@ -1013,9 +983,11 @@ test("find keeps a candidate whose parse throws, merely unmarked", async () => { ), ); + expect(response.status).toBe(200); expect(await response.json()).toEqual([ + { title: "Push", url: "https://site.example/push.xml", websub: true }, + { title: "Plain", url: "https://site.example/plain.xml", websub: false }, { title: "Dead", url: "https://site.example/dead.xml", websub: false }, - { title: "Live", url: "https://site.example/live.xml", websub: true }, ]); }); diff --git a/src/extension/__tests__/badge.test.ts b/src/extension/__tests__/badge.test.ts index 482b83a1..afacf0b9 100644 --- a/src/extension/__tests__/badge.test.ts +++ b/src/extension/__tests__/badge.test.ts @@ -6,10 +6,8 @@ describe("formatBadgeCount", () => { test.each([ [0, ""], [1, "1"], - [3, "3"], [99, "99"], [100, "99+"], - [1234, "99+"], ])("formats %i as %s", (count, expected) => { expect(formatBadgeCount(count)).toBe(expected); }); diff --git a/src/features/auth/__tests__/mail-sender.test.ts b/src/features/auth/__tests__/mail-sender.test.ts index 9bd34232..eadbd0a4 100644 --- a/src/features/auth/__tests__/mail-sender.test.ts +++ b/src/features/auth/__tests__/mail-sender.test.ts @@ -32,7 +32,7 @@ describe("MailSender", () => { }), async (input, init) => { captured = { init, input }; - return new Response(null, { status: 200 }); + return new Response(null, { status: 202 }); }, ); @@ -122,20 +122,6 @@ describe("MailSender", () => { expect(calls).toBe(0); }); - test("resolves for successful Mailjet responses", async () => { - const sender = new MailSender( - config({ - MAILJET_API_KEY: "mailjet-key", - MAILJET_API_SECRET: "mailjet-secret", - }), - async () => new Response("accepted", { status: 202 }), - ); - - await expect( - sender.sendActivationEmail("reader@example.com", "activation-token"), - ).resolves.toBeUndefined(); - }); - test("throws a bounded diagnostic for non-success responses", async () => { const sender = new MailSender( config({ diff --git a/src/features/feeds/__tests__/favicon-selection.test.ts b/src/features/feeds/__tests__/favicon-selection.test.ts index 7d85ea39..2016a3b7 100644 --- a/src/features/feeds/__tests__/favicon-selection.test.ts +++ b/src/features/feeds/__tests__/favicon-selection.test.ts @@ -1,9 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { - imageDimensions, - isBetterFavicon, - targetFaviconSize, -} from "../favicon-selection.ts"; +import { imageDimensions, isBetterFavicon } from "../favicon-selection.ts"; function png(width: number, height: number): Buffer { const buffer = Buffer.alloc(24); @@ -68,8 +64,7 @@ describe("imageDimensions", () => { expect(imageDimensions(bmp(64, 64))).toEqual({ height: 64, width: 64 }); }); - // A bottom-up BMP stores a negative height. - test("reads a bottom-up BMP as a positive height", () => { + test("reads a negative BMP height as a positive dimension", () => { expect(imageDimensions(bmp(64, -64))).toEqual({ height: 64, width: 64 }); }); @@ -142,9 +137,3 @@ describe("favicon size preference", () => { expect(isBetterFavicon(16, 32, 64)).toBe(false); }); }); - -describe("targetFaviconSize", () => { - test("is the size providers are asked for and candidates judged against", () => { - expect(targetFaviconSize).toBe(64); - }); -}); diff --git a/src/features/feeds/__tests__/feed-discovery.test.ts b/src/features/feeds/__tests__/feed-discovery.test.ts index 9b39796c..155d81c9 100644 --- a/src/features/feeds/__tests__/feed-discovery.test.ts +++ b/src/features/feeds/__tests__/feed-discovery.test.ts @@ -12,16 +12,6 @@ const probe = ( }); describe("markWebSubAvailability", () => { - test("marks a candidate that advertises a hub", async () => { - const result = await markWebSubAvailability( - [feed("Push", "https://a.example/feed")], - probe(() => ({ websub: { hubUrl: "https://hub.example/" } })), - ); - expect(result).toEqual([ - { title: "Push", url: "https://a.example/feed", websub: true }, - ]); - }); - test("leaves a candidate without a hub unmarked", async () => { const result = await markWebSubAvailability( [feed("Plain", "https://a.example/feed")], @@ -38,21 +28,6 @@ describe("markWebSubAvailability", () => { expect(result[0]?.websub).toBe(false); }); - // A dead candidate is still worth showing: the failure surfaces when the - // user previews it, and dropping it would make the page look like the feed - // never existed. - test("keeps a candidate whose parse throws, merely unmarked", async () => { - const result = await markWebSubAvailability( - [feed("Dead", "https://a.example/gone")], - probe(() => { - throw new Error("404"); - }), - ); - expect(result).toEqual([ - { title: "Dead", url: "https://a.example/gone", websub: false }, - ]); - }); - // One bad candidate must not take the rest of the list with it, which is // what a bare Promise.all without the per-candidate catch would do. test("one failing candidate does not lose the others", async () => { @@ -72,40 +47,39 @@ describe("markWebSubAvailability", () => { ]); }); - // The list is rendered in the order the scanner found the feeds, so a slow - // probe must not reorder it. - test("preserves candidate order regardless of probe latency", async () => { - const result = await markWebSubAvailability( + test("probes concurrently and preserves order when probes finish in reverse", async () => { + const slow = Promise.withResolvers<{ websub?: unknown }>(); + const fast = Promise.withResolvers<{ websub?: unknown }>(); + const started: string[] = []; + const result = markWebSubAvailability( [ feed("Slow", "https://a.example/slow"), feed("Fast", "https://a.example/fast"), ], { - parseUrl: async (url: string) => { - if (url.includes("slow")) await Bun.sleep(20); - return { websub: undefined }; - }, - }, - ); - expect(result.map((entry) => entry.title)).toEqual(["Slow", "Fast"]); - }); - - test("probes candidates concurrently rather than one after another", async () => { - const started = Date.now(); - await markWebSubAvailability( - [ - feed("A", "https://a.example/1"), - feed("B", "https://a.example/2"), - feed("C", "https://a.example/3"), - ], - { - parseUrl: async () => { - await Bun.sleep(30); - return { websub: undefined }; + parseUrl: (url) => { + started.push(url); + return url.endsWith("slow") ? slow.promise : fast.promise; }, }, ); - expect(Date.now() - started).toBeLessThan(80); + try { + expect(started).toEqual([ + "https://a.example/slow", + "https://a.example/fast", + ]); + fast.resolve({}); + await fast.promise; + slow.resolve({ websub: { hubUrl: "https://hub.example/" } }); + expect(await result).toEqual([ + { title: "Slow", url: "https://a.example/slow", websub: true }, + { title: "Fast", url: "https://a.example/fast", websub: false }, + ]); + } finally { + slow.resolve({}); + fast.resolve({}); + await result; + } }); test("an empty candidate list yields an empty result", async () => { diff --git a/src/features/feeds/__tests__/feed-mapper.test.ts b/src/features/feeds/__tests__/feed-mapper.test.ts index 55dd04f4..d1b2d3b9 100644 --- a/src/features/feeds/__tests__/feed-mapper.test.ts +++ b/src/features/feeds/__tests__/feed-mapper.test.ts @@ -160,7 +160,7 @@ describe("mapFeedItemToArticle", () => { expect(result.publishedAt).toEqual(publishedDate); }); - test("should generate consistent GUID for same content", () => { + test("generates stable GUIDs that distinguish different content", () => { const content = { content: "Same content", description: "Same desc", @@ -184,6 +184,13 @@ describe("mapFeedItemToArticle", () => { ); expect(result1.guid).toBe(result2.guid); + const changed = mapFeedItemToArticle( + createMockFeedItem({ ...content, content: "Different content" }), + mockFeed, + mockSource, + mockRewriteLinks, + ); + expect(changed.guid).not.toBe(result1.guid); }); test("should handle null content with description fallback", () => { @@ -248,27 +255,6 @@ describe("mapFeedToPreview", () => { }); }); - test("should handle partially undefined feed data", () => { - const mockFeed = createMockFeed({ - description: "description", - title: null, - }); - - const result = mapFeedToPreview( - mockFeed, - "https://example.com/feed.xml", - mockRewriteLinks, - ); - - expect(result).toEqual({ - articles: [], - description: "description", - feedUrl: "https://example.com/feed.xml", - link: undefined, - title: "https://example.com/feed.xml", - }); - }); - test("preserves preview article persistence fields", () => { const publishedAt = new Date("2024-03-06T12:00:00Z"); const mockFeed = createMockFeed({ diff --git a/src/features/feeds/__tests__/feed-preview-cache.test.ts b/src/features/feeds/__tests__/feed-preview-cache.test.ts index 72759ff1..cb79f1c8 100644 --- a/src/features/feeds/__tests__/feed-preview-cache.test.ts +++ b/src/features/feeds/__tests__/feed-preview-cache.test.ts @@ -86,60 +86,33 @@ describe("FeedPreviewCache", () => { expect(await cache.get(7, `${feedUrl}#other`)).toBeUndefined(); }); - test("deletes malformed cached entries", async () => { - const redis = new FakeRedis(); - const cache = new FeedPreviewCache(redis); - await cache.save(7, feedUrl, preview); - const key = redis.setCalls[0]?.[0]; - if (!key) throw new Error("Preview was not cached"); - redis.values.set(key, "not-json"); - - expect(await cache.get(7, feedUrl)).toBeUndefined(); - expect(redis.deleted).toEqual([key]); - }); - - test("deletes entries with non-finite timestamps or extra fields", async () => { - const malformedEntries = [ - { + test("deletes malformed or mismatched cached previews", async () => { + const entries = [ + "not-json", + JSON.stringify({ ...previewWire, articles: [{ ...previewWire.articles[0], publishedAt: null }], - }, - { ...previewWire, extra: true }, + }), + JSON.stringify({ ...previewWire, extra: true }), + JSON.stringify({ + ...previewWire, + feedUrl: "https://example.com/other.xml", + }), ]; - await Promise.all( - malformedEntries.map(async (entry) => { + entries.map(async (entry) => { const redis = new FakeRedis(); const cache = new FeedPreviewCache(redis); await cache.save(7, feedUrl, preview); const key = redis.setCalls[0]?.[0]; if (!key) throw new Error("Preview was not cached"); - redis.values.set(key, JSON.stringify(entry)); - + redis.values.set(key, entry); expect(await cache.get(7, feedUrl)).toBeUndefined(); expect(redis.deleted).toEqual([key]); }), ); }); - test("deletes entries with invalid fields or a different URL", async () => { - const redis = new FakeRedis(); - const cache = new FeedPreviewCache(redis); - await cache.save(7, feedUrl, preview); - const key = redis.setCalls[0]?.[0]; - if (!key) throw new Error("Preview was not cached"); - redis.values.set( - key, - JSON.stringify({ - ...previewWire, - feedUrl: "https://example.com/other.xml", - }), - ); - - expect(await cache.get(7, feedUrl)).toBeUndefined(); - expect(redis.deleted).toEqual([key]); - }); - test("treats Redis errors as cache misses", async () => { const redis = new FakeRedis(); const cache = new FeedPreviewCache(redis); diff --git a/src/features/feeds/__tests__/opml-parser-cases/expected/parse-opml/single-feed.ts b/src/features/feeds/__tests__/opml-parser-cases/expected/parse-opml/single-feed.ts deleted file mode 100644 index b4ff1be3..00000000 --- a/src/features/feeds/__tests__/opml-parser-cases/expected/parse-opml/single-feed.ts +++ /dev/null @@ -1,10 +0,0 @@ -import type { OpmlSource } from "#shared/types/opml-types.ts"; - -export const expected: OpmlSource[] = [ - { - homeUrl: "https://example.com", - name: "Example Feed", - type: "source", - xmlUrl: "https://example.com/feed.xml", - }, -]; diff --git a/src/features/feeds/__tests__/opml-parser-cases/expected/process-outline/invalid-xml-url.ts b/src/features/feeds/__tests__/opml-parser-cases/expected/process-outline/invalid-xml-url.ts deleted file mode 100644 index 28c170cb..00000000 --- a/src/features/feeds/__tests__/opml-parser-cases/expected/process-outline/invalid-xml-url.ts +++ /dev/null @@ -1,8 +0,0 @@ -import type { OpmlSource } from "#shared/types/opml-types.ts"; - -export const expected: OpmlSource = { - homeUrl: "https://example.com", - name: "Invalid XML URL Feed", - type: "source", - xmlUrl: "not-a-valid-url", -}; diff --git a/src/features/feeds/__tests__/opml-parser-cases/inputs/parse-opml/single-feed.ts b/src/features/feeds/__tests__/opml-parser-cases/inputs/parse-opml/single-feed.ts deleted file mode 100644 index 3e728700..00000000 --- a/src/features/feeds/__tests__/opml-parser-cases/inputs/parse-opml/single-feed.ts +++ /dev/null @@ -1,15 +0,0 @@ -export const input = ` - - - Feed Subscriptions - - - - - `; diff --git a/src/features/feeds/__tests__/opml-parser.test.ts b/src/features/feeds/__tests__/opml-parser.test.ts index 9cb363ea..782fe860 100644 --- a/src/features/feeds/__tests__/opml-parser.test.ts +++ b/src/features/feeds/__tests__/opml-parser.test.ts @@ -180,33 +180,15 @@ describe("OpmlParser", () => { path.join(process.cwd(), inputDirectory, `${testFile}.ts`) ); - // Only import expected result for non-error test cases - let expected; - if (!testFile.includes("invalid")) { - const expectedModule = await import( - path.join(process.cwd(), expectedDirectory, `${testFile}.ts`) - ); - expected = expectedModule.expected; + if (testFile === "invalid-xml") { + expect(() => parser.parseOpml(input)).toThrow(SyntaxError); + return; } - try { - const result = await parser.parseOpml(input); - // Only compare results for non-error test cases - if (testFile.includes("invalid")) { - // For invalid test cases, we expect the parser to succeed - // but we don't care about the exact result - expect(result).toBeTruthy(); - } else { - expect(result).toEqual(expected); - } - } catch (error) { - // If this is an error test case, we expect an error - if (testFile.includes("invalid")) { - expect(error).toBeTruthy(); - } else { - throw error; - } - } + const { expected } = await import( + path.join(process.cwd(), expectedDirectory, `${testFile}.ts`) + ); + expect(parser.parseOpml(input)).toEqual(expected); }); } }); diff --git a/src/features/feeds/__tests__/rewrite-links-cases/basic-absolute-01-already-absolute.json b/src/features/feeds/__tests__/rewrite-links-cases/basic-absolute-01-already-absolute.json index 5abebbf9..ef8a764b 100644 --- a/src/features/feeds/__tests__/rewrite-links-cases/basic-absolute-01-already-absolute.json +++ b/src/features/feeds/__tests__/rewrite-links-cases/basic-absolute-01-already-absolute.json @@ -1,5 +1,5 @@ { - "description": "absolute URL should not be changed", - "content": "Link", - "expected": "Link" + "description": "preserves absolute HTTP and HTTPS links and adds safe link attributes", + "content": "LinkLinkLink", + "expected": "LinkLinkLink" } diff --git a/src/features/feeds/__tests__/rewrite-links-cases/basic-absolute-02-http-protocol.json b/src/features/feeds/__tests__/rewrite-links-cases/basic-absolute-02-http-protocol.json deleted file mode 100644 index 2e71e70c..00000000 --- a/src/features/feeds/__tests__/rewrite-links-cases/basic-absolute-02-http-protocol.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "description": "absolute URL with http protocol", - "content": "Link", - "expected": "Link" -} diff --git a/src/features/feeds/__tests__/rewrite-links-cases/basic-absolute-03-with-path.json b/src/features/feeds/__tests__/rewrite-links-cases/basic-absolute-03-with-path.json deleted file mode 100644 index 957a8330..00000000 --- a/src/features/feeds/__tests__/rewrite-links-cases/basic-absolute-03-with-path.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "description": "absolute URL with path", - "content": "Link", - "expected": "Link" -} diff --git a/src/features/feeds/__tests__/rewrite-links-cases/basic-absolute-04-multiple-urls.json b/src/features/feeds/__tests__/rewrite-links-cases/basic-absolute-04-multiple-urls.json deleted file mode 100644 index 97268106..00000000 --- a/src/features/feeds/__tests__/rewrite-links-cases/basic-absolute-04-multiple-urls.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "description": "multiple absolute URLs", - "content": "Link1Link2", - "expected": "Link1Link2" -} diff --git a/src/features/jobs/__tests__/main.test.ts b/src/features/jobs/__tests__/main.test.ts index 8f96fee9..b98668a7 100644 --- a/src/features/jobs/__tests__/main.test.ts +++ b/src/features/jobs/__tests__/main.test.ts @@ -268,7 +268,8 @@ async function createMainWorker( return new MainWorker(); } -test("initialize schedules configured intervals and starts the worker", async () => { +test("initializes scheduled jobs and closes the worker on cleanup", async () => { + let closeCalls = 0; const repeatIntervals = new Map(); let workerOptions: Parameters[1] | undefined; const queue: MainWorkerQueue = { @@ -280,7 +281,12 @@ test("initialize schedules configured intervals and starts the worker", async () }; const createWorker: MainWorkerFactory = (_processor, options) => { workerOptions = options; - return noopWorkerFactory(_processor, options); + return { + ...noopWorkerFactory(_processor, options), + async close() { + closeCalls++; + }, + }; }; const worker = await createMainWorker( config, @@ -302,6 +308,9 @@ test("initialize schedules configured intervals and starts the worker", async () expect(repeatIntervals.get(JobName.GatherFaviconJobs)).toBe(86_400_000); expect(repeatIntervals.get(JobName.WebSubRenewal)).toBe(86_400_000); expect(workerOptions).toEqual({ concurrency: 2, lockDuration: 40_000 }); + expect(closeCalls).toBe(0); + await worker.cleanup(); + expect(closeCalls).toBe(1); }); test("captured processor parses the queued source", async () => { @@ -684,37 +693,6 @@ test("rejects malformed and unknown jobs before downstream calls", async () => { expect(downstreamCalls).toEqual([]); }); -test("cleanup delegates to the worker", async () => { - let closeCalls = 0; - const queue: MainWorkerQueue = { - async add() {}, - async addBulk() {}, - }; - const createWorker: MainWorkerFactory = () => ({ - async close() { - closeCalls++; - }, - onFailed() {}, - }); - const worker = await createMainWorker( - config, - queue, - idleParser, - idleFaviconRefresher, - idleSources, - idleSources, - idleCleanupOrphanedData, - idleJobFailures, - createWorker, - idleHubPoster, - ); - await worker.initialize(); - - await worker.cleanup(); - - expect(closeCalls).toBe(1); -}); - test("records a durable failure for non-ParseSource job errors", async () => { let processor: ((job: MainWorkerJob) => Promise) | undefined; const recorded: [string, string][] = []; diff --git a/src/platform/__tests__/config.test.ts b/src/platform/__tests__/config.test.ts index e85213c9..b22684f2 100644 --- a/src/platform/__tests__/config.test.ts +++ b/src/platform/__tests__/config.test.ts @@ -110,16 +110,6 @@ describe("loadConfig", () => { }, ); - test("allows mail when the relay secret is present", () => { - expect( - loadConfig({ - DATABASE_URL: databaseUrl, - MAIL_ENABLED: "true", - MAIL_RELAY_SECRET: "relay-secret", - }).MAIL_RELAY_SECRET, - ).toBe("relay-secret"); - }); - test("allows blank Turnstile keys to disable Turnstile", () => { expect( loadConfig({ diff --git a/src/platform/http/__tests__/http-cache-policy.test.ts b/src/platform/http/__tests__/http-cache-policy.test.ts index 1b7d16aa..4646f071 100644 --- a/src/platform/http/__tests__/http-cache-policy.test.ts +++ b/src/platform/http/__tests__/http-cache-policy.test.ts @@ -289,10 +289,18 @@ describe("refresh", () => { url: "https://example.test/feed", }; - test("keeps the stored body and status", () => { + test("refreshes freshness after a 304 while preserving the stored response", () => { + const before = Math.floor(Date.now() / second) * second; const next = refresh(stored, headers({ "cache-control": "max-age=60" })); + const nextHeaders = new Headers(next.headers); + const stamped = Date.parse(nextHeaders.get("date") ?? ""); + expect(next.body).toBe(stored.body); - expect(next.status).toBe(200); + expect(next.status).toBe(stored.status); + expect(nextHeaders.get("age")).toBeNull(); + expect(stamped).toBeGreaterThanOrEqual(before); + expect(stamped).toBeLessThanOrEqual(Date.now()); + expect(next.expiresAt).toBe(stamped + 60 * second); }); test("overwrites stored headers the 304 restates", () => { @@ -300,28 +308,8 @@ describe("refresh", () => { expect(new Headers(next.headers).get("etag")).toBe('"v2"'); }); - // Otherwise the stored copy's original Age would keep counting against a - // response the origin has just confirmed is current. - test("drops a stored Age the 304 did not restate", () => { - const next = refresh(stored, headers({ "cache-control": "max-age=60" })); - expect(new Headers(next.headers).get("age")).toBeNull(); - }); - test("keeps an Age the 304 did restate", () => { const next = refresh(stored, headers({ age: "5" })); expect(new Headers(next.headers).get("age")).toBe("5"); }); - - // Same reason: the stored Date would make the refreshed entry look as old as - // the response it replaced. - test("stamps Date to now when the 304 omitted it", () => { - const next = refresh(stored, headers({ "cache-control": "max-age=60" })); - const stamped = Date.parse(new Headers(next.headers).get("date") ?? ""); - expect(Math.abs(stamped - Date.now())).toBeLessThan(5 * second); - }); - - test("recomputes expiry from the merged headers", () => { - const next = refresh(stored, headers({ "cache-control": "max-age=60" })); - expect(next.expiresAt).toBeGreaterThan(Date.now() + 55 * second); - }); }); diff --git a/src/platform/http/__tests__/http-client.test.ts b/src/platform/http/__tests__/http-client.test.ts index 1a2a57c3..2c4dcad3 100644 --- a/src/platform/http/__tests__/http-client.test.ts +++ b/src/platform/http/__tests__/http-client.test.ts @@ -128,37 +128,50 @@ test("caches fresh responses, retries transient failures, and defers background ).rejects.toBeInstanceOf(HttpDeferredError); }); -test("skipCache bypasses the local TTL short-circuit but still revalidates conditionally", async () => { - let requests = 0; +test("skipCache sends validators and a 304 prevents another request while fresh", async () => { + const sentHeaders: Headers[] = []; const store = redis(); const client = new HttpClient(store, { - transport: queuedTransport([nativeResponse("", { status: 304 })], () => { - requests++; - }), + transport: async (_url, headers) => { + sentHeaders.push(new Headers(headers)); + return nativeResponse("", { + headers: { "cache-control": "max-age=60" }, + status: 304, + }); + }, }); - // Seed a still-fresh cached entry directly, as if an earlier fetch had - // already populated it -- avoids this test's own network call tripping - // the per-hostname reservation interval before the skipCache request runs. const url = "https://1.1.1.1/feed"; + const initialExpiry = Date.now() + 30_000; + const modified = "Thu, 01 Jan 2026 12:00:00 GMT"; const cacheKey = `http-cache:${Buffer.from(url).toString("base64url")}`; store.values.set( cacheKey, JSON.stringify({ - body: Buffer.from("stale-cached-feed").toString("base64"), - expiresAt: Date.now() + 60_000, - headers: [["etag", '"v1"']], + body: Buffer.from("cached-feed").toString("base64"), + expiresAt: initialExpiry, + headers: [ + ["etag", '"v1"'], + ["last-modified", modified], + ], status: 200, url, }), ); - // Still within the cached entry's TTL, so a plain get() would return it - // without any network request -- skipCache forces revalidation instead. const revalidated = await client.get(url, { skipCache: true }); - expect(revalidated.data).toBe("stale-cached-feed"); + expect(revalidated.data).toBe("cached-feed"); expect(revalidated.cached).toBe(true); - expect(requests).toBe(1); + expect(revalidated.freshUntil).toBeGreaterThan(initialExpiry); + expect(sentHeaders).toHaveLength(1); + expect(sentHeaders[0]!.get("if-none-match")).toBe('"v1"'); + expect(sentHeaders[0]!.get("if-modified-since")).toBe(modified); + + const reused = await client.get(url); + expect(reused.data).toBe("cached-feed"); + expect(reused.cached).toBe(true); + expect(reused.freshUntil).toBe(revalidated.freshUntil); + expect(sentHeaders).toHaveLength(1); }); test("uses X-RateLimit-Reset as an absolute epoch timestamp", async () => { @@ -456,21 +469,6 @@ test("honours Retry-After on a 503 instead of retrying it", async () => { expect(store.values.get("http-blocked:1.1.1.1")).toBe(String(error.retryAt)); }); -// Without the header a 503 is still just a transient failure to retry. -test("still retries a 503 that carries no Retry-After", async () => { - let requests = 0; - const client = new HttpClient(redis(), { - intervalMs: shortInterval, - transport: queuedTransport( - [nativeResponse("overloaded", { status: 503 }), nativeResponse("feed")], - () => requests++, - ), - }); - - expect((await client.get("https://1.1.1.1/feed")).data).toBe("feed"); - expect(requests).toBe(2); -}); - // The host slot used to be reserved once per call, so the three attempts of // one retry sequence landed 200ms and 400ms apart -- three requests to one // host inside an interval that exists to allow one. diff --git a/src/platform/http/__tests__/http-rate-limiter.test.ts b/src/platform/http/__tests__/http-rate-limiter.test.ts index 2ba93b51..2a78d0d1 100644 --- a/src/platform/http/__tests__/http-rate-limiter.test.ts +++ b/src/platform/http/__tests__/http-rate-limiter.test.ts @@ -100,8 +100,7 @@ describe("rateLimitBlockUntil", () => { }); describe("rateLimitBlockUntil header spellings and reset units", () => { - // RFC 9331 standardised the un-prefixed names; the X- forms predate it. - test("reads the un-prefixed RFC 9331 names", () => { + test("reads an epoch reset with un-prefixed header names", () => { const reset = (now + 60_000) / 1_000; expect( rateLimitBlockUntil( @@ -140,19 +139,6 @@ describe("rateLimitBlockUntil header spellings and reset units", () => { ).toBe(now + 60_000); }); - test("still reads a large reset as an epoch timestamp", () => { - const reset = (now + 60_000) / 1_000; - expect( - rateLimitBlockUntil( - headers({ - "ratelimit-remaining": "0", - "ratelimit-reset": String(reset), - }), - now, - ), - ).toBe(reset * 1_000); - }); - test("ignores a delta of zero and a negative reset", () => { expect( rateLimitBlockUntil( diff --git a/src/platform/http/__tests__/request-deadline.test.ts b/src/platform/http/__tests__/request-deadline.test.ts index 3a63d381..6de803f3 100644 --- a/src/platform/http/__tests__/request-deadline.test.ts +++ b/src/platform/http/__tests__/request-deadline.test.ts @@ -4,18 +4,6 @@ import { HttpDeadlineError, RequestDeadline } from "../request-deadline.ts"; const never = new Promise(() => {}); describe("RequestDeadline", () => { - test("passes a result through while the budget holds", async () => { - const deadline = new RequestDeadline(1_000); - expect(await deadline.run(Promise.resolve("ok"))).toBe("ok"); - deadline.dispose(); - }); - - test("rejects an operation that outlives the budget", async () => { - const deadline = new RequestDeadline(10); - await expect(deadline.run(never)).rejects.toBeInstanceOf(HttpDeadlineError); - deadline.dispose(); - }); - // The transport is handed this signal, so expiry has to cancel an in-flight // read rather than wait for it to return on its own. test("aborts its controller when the budget runs out", async () => { @@ -36,20 +24,12 @@ describe("RequestDeadline", () => { deadline.dispose(); }); - // A retry or redirect hop starting after expiry must not be dispatched at - // all, which is the whole point of a budget shared across steps. - test("refuses to start a new operation after expiry", async () => { + test("rejects an already-resolved operation after expiry", async () => { const deadline = new RequestDeadline(10); await expect(deadline.run(never)).rejects.toBeInstanceOf(HttpDeadlineError); - let started = false; - const operation = (async () => { - started = true; - return "late"; - })(); - await expect(deadline.run(operation)).rejects.toBeInstanceOf( + await expect(deadline.run(Promise.resolve("late"))).rejects.toBeInstanceOf( HttpDeadlineError, ); - expect(started).toBe(true); deadline.dispose(); }); @@ -72,10 +52,11 @@ describe("RequestDeadline", () => { }); // Without this the timer keeps the process alive after a fast request. - test("dispose clears the pending timer", async () => { + test("passes a result through and disposal prevents later abort", async () => { const deadline = new RequestDeadline(50); expect(await deadline.run(Promise.resolve(1))).toBe(1); deadline.dispose(); + await Bun.sleep(75); expect(deadline.controller.signal.aborted).toBe(false); }); }); diff --git a/src/shared/net/__tests__/private-network-guard.test.ts b/src/shared/net/__tests__/private-network-guard.test.ts index c40c4689..8656e351 100644 --- a/src/shared/net/__tests__/private-network-guard.test.ts +++ b/src/shared/net/__tests__/private-network-guard.test.ts @@ -64,7 +64,6 @@ describe("isBlockedHostname", () => { "0x7f.1", // hex 2-part -> 127.0.0.1 "0177.0.0.1", // octal -> 127.0.0.1 "0x7f.0.0.1", // hex dotted quad - "0177.0.0.1", // octal -> 127.0.0.1 "0:0:0:0:0:0:0:1", // full-form loopback "0:0:0:0:0:0:0:0", // full-form unspecified "::ffff:127.0.0.1", // IPv4-mapped loopback, dotted tail diff --git a/src/shared/net/private-network-guard.ts b/src/shared/net/private-network-guard.ts index b6fa97aa..6757eca7 100644 --- a/src/shared/net/private-network-guard.ts +++ b/src/shared/net/private-network-guard.ts @@ -1,13 +1,7 @@ -// Shared by the extension's reader-fetch (src/extension/reader-fetch.ts) and -// server-side outbound requests whose target URL comes from untrusted, -// attacker-influenced content rather than something a user directly typed -// (a WebSub hub URL discovered inside fetched feed content, for example). -// Checks the hostname string itself, not a DNS-resolved address -- same -// limitation as the extension's original version, so this does not defend -// against DNS rebinding (a hostname that resolves to a private IP at -// request time). Closing that gap would mean resolving DNS ourselves and -// connecting to the resolved address directly, which neither call site -// does today. +// Hostname-only guard shared by extension reader fetching and WebSub URL +// validation. It parses IP literals and applies our block rules without DNS +// lookup. Server requests also validate and pin DNS results in +// platform/http/http-native-transport.ts; the extension cannot do that. // // Because real HTTP clients accept non-canonical IP literal forms // (inet_aton-style "2130706433" or "127.1", hex "0x7f.1", octal "017.0.0.1", diff --git a/src/shared/util/__tests__/is-plain-text.test.ts b/src/shared/util/__tests__/is-plain-text.test.ts index aa912721..fe3a256f 100644 --- a/src/shared/util/__tests__/is-plain-text.test.ts +++ b/src/shared/util/__tests__/is-plain-text.test.ts @@ -1,38 +1,16 @@ -import { describe, expect, test } from "bun:test"; +import { expect, test } from "bun:test"; import { isPlainText } from "#shared/util/is-plain-text.ts"; -describe("isPlainText", () => { - test("should return true for empty string", () => { - expect(isPlainText("")).toBe(true); - }); - - test("should return true for plain text content", () => { - expect(isPlainText("This is a plain text.")).toBe(true); - }); - - test("should return true for HTML content", () => { - expect(isPlainText("

This is HTML content.

")).toBe(true); - }); - - test("should return true for JSON content", () => { - expect(isPlainText('{"key": "value"}')).toBe(true); - }); - - test("should return true for XML content", () => { - expect(isPlainText("Tove")).toBe(true); - }); - - test("should return true for Markdown content", () => { - expect(isPlainText("# This is a header")).toBe(true); - }); +test("accepts empty text", () => { + expect(isPlainText("")).toBe(true); +}); - test("should return false for binary data", () => { - // Simulating binary data as a string - const binaryString = String.fromCodePoint(0, 1, 2, 3, 4, 5); - expect(isPlainText(binaryString)).toBe(false); - }); +test("accepts markup, Unicode, and text whitespace", () => { + expect(isPlainText('

Café 世界 😀

\t\r\n{"key": "value"}')).toBe(true); +}); - test("should return false for other binary-like strings", () => { - expect(isPlainText("This is a string with null byte \0")).toBe(false); - }); +test("rejects control characters embedded in text", () => { + for (const code of [0, 8, 11, 12, 14, 31, 127, 159]) { + expect(isPlainText(`before${String.fromCodePoint(code)}after`)).toBe(false); + } }); diff --git a/src/spa/__tests__/behavior.test.ts b/src/spa/__tests__/behavior.test.ts index e0995172..0ea374b3 100644 --- a/src/spa/__tests__/behavior.test.ts +++ b/src/spa/__tests__/behavior.test.ts @@ -64,12 +64,8 @@ describe("resolveRoute", () => { expect(resolveRoute("/preview")).toEqual({ name: "preview" }); }); - test("resolves named pages exactly and otherwise uses the dashboard", () => { - expect(resolveRoute("/login")).toEqual({ name: "login", next: "/" }); - expect(resolveRoute("/register")).toEqual({ - name: "register", - next: "/", - }); + test("requires an exact match for named pages", () => { + expect(resolveRoute("/options")).toEqual({ name: "options" }); expect(resolveRoute("/options/more")).toEqual({ name: "dashboard" }); }); }); diff --git a/src/spa/__tests__/dashboard-behavior.test.ts b/src/spa/__tests__/dashboard-behavior.test.ts index fd390f5e..734c1af3 100644 --- a/src/spa/__tests__/dashboard-behavior.test.ts +++ b/src/spa/__tests__/dashboard-behavior.test.ts @@ -4,7 +4,6 @@ import { faviconUrls, filterTree, folderOpenFromStored, - folderOpenStorageKey, folderOpenToStored, findNode, findParentFolderUid, @@ -12,12 +11,10 @@ import { isTodayNode, nextPollDelayMs, sourceIds, - snoozePresets, snoozeUntilIso, totalUnread, treeNodeKey, treeTabStopKey, - unreadCount, withDecrementedUnread, withTodayNode, } from "../dashboard-behavior.ts"; @@ -51,45 +48,29 @@ describe("treeNodeKey", () => { }); describe("sourceIds", () => { - test("returns a source's own numeric uid", () => { - expect(sourceIds(source("42"))).toEqual([42]); - }); - - test("collects every source beneath a folder, in order", () => { - expect(sourceIds(folder("f", [source("2"), source("9")]))).toEqual([2, 9]); - }); - - test("an empty folder contributes nothing", () => { - expect(sourceIds(folder("f", []))).toEqual([]); + test("collects source IDs in order through nested and empty folders", () => { + expect( + sourceIds( + folder("f", [ + source("2"), + folder("nested", [source("9")]), + folder("empty", []), + ]), + ), + ).toEqual([2, 9]); + expect(sourceIds(folder("empty", []))).toEqual([]); }); }); describe("faviconUrls", () => { - test("skips sources with no favicon", () => { + test("collects favicons and skips missing or empty values", () => { const tree = folder("f", [ source("1", { favicon: "https://a.example/i.png" }), source("2"), + source("3", { favicon: "" }), ]); expect(faviconUrls(tree)).toEqual(["https://a.example/i.png"]); }); - - test("an empty favicon string is treated as absent", () => { - expect(faviconUrls(source("1", { favicon: "" }))).toEqual([]); - }); -}); - -describe("unreadCount", () => { - test("sums a folder's sources", () => { - const tree = folder("f", [ - source("1", { unreadCount: 3 }), - source("2", { unreadCount: 4 }), - ]); - expect(unreadCount(tree)).toBe(7); - }); - - test("a source reports its own count", () => { - expect(unreadCount(source("1", { unreadCount: 5 }))).toBe(5); - }); }); describe("totalUnread", () => { @@ -123,22 +104,32 @@ describe("nextPollDelayMs", () => { }); describe("withDecrementedUnread", () => { - test("subtracts the delta for the named source", () => { - const nodes = [source("1", { unreadCount: 5 })]; - const next = withDecrementedUnread(nodes, new Map([["1", 2]])); - expect(unreadCount(next[0]!)).toBe(3); - }); - - test("clamps at zero rather than going negative", () => { - const nodes = [source("1", { unreadCount: 1 })]; - const next = withDecrementedUnread(nodes, new Map([["1", 9]])); - expect(unreadCount(next[0]!)).toBe(0); - }); - - test("reaches sources nested in a folder", () => { - const nodes = [folder("f", [source("1", { unreadCount: 4 })])]; - const next = withDecrementedUnread(nodes, new Map([["1", 1]])); - expect(unreadCount(next[0]!)).toBe(3); + test("decrements nested sources, clamps at zero, and preserves untouched branches", () => { + const untouched = folder("keep", [source("3", { unreadCount: 2 })]); + const nodes = [ + folder("f", [ + source("1", { unreadCount: 5 }), + source("2", { unreadCount: 1 }), + ]), + untouched, + ]; + const next = withDecrementedUnread( + nodes, + new Map([ + ["1", 2], + ["2", 9], + ]), + ); + expect(next).toEqual([ + folder("f", [ + source("1", { unreadCount: 3 }), + source("2", { unreadCount: 0 }), + ]), + untouched, + ]); + expect(next).not.toBe(nodes); + expect(next[1]).toBe(untouched); + expect(totalUnread(nodes)).toBe(8); }); // The component diffs on identity to decide whether to re-render, so an @@ -148,14 +139,6 @@ describe("withDecrementedUnread", () => { expect(withDecrementedUnread(nodes, new Map([["9", 1]]))).toBe(nodes); }); - test("leaves untouched branches identical while replacing changed ones", () => { - const untouched = folder("keep", [source("1", { unreadCount: 2 })]); - const nodes = [untouched, source("2", { unreadCount: 2 })]; - const next = withDecrementedUnread(nodes, new Map([["2", 1]])); - expect(next).not.toBe(nodes); - expect(next[0]).toBe(untouched); - }); - test("a zero delta counts as no change", () => { const nodes = [source("1", { unreadCount: 4 })]; expect(withDecrementedUnread(nodes, new Map([["1", 0]]))).toBe(nodes); @@ -192,27 +175,13 @@ describe("findParentFolderUid", () => { describe("folder open persistence", () => { test('only the literal "closed" collapses a folder', () => { expect(folderOpenFromStored("closed")).toBe(false); - expect(folderOpenFromStored("open")).toBe(true); - }); - - // A folder that has never been toggled has no stored entry at all. - test("an absent entry reads as open", () => { - expect(folderOpenFromStored(null)).toBe(true); - }); - - // A value from an older build or a corrupted one must not hide feeds. - test("an unrecognised value reads as open", () => { - expect(folderOpenFromStored("")).toBe(true); - expect(folderOpenFromStored("CLOSED")).toBe(true); + for (const value of ["open", null, "", "CLOSED"]) + expect(folderOpenFromStored(value)).toBe(true); }); - test("round-trips both states", () => { - expect(folderOpenFromStored(folderOpenToStored(true))).toBe(true); - expect(folderOpenFromStored(folderOpenToStored(false))).toBe(false); - }); - - test("namespaces the key by uid", () => { - expect(folderOpenStorageKey("inbox")).toBe("folder:inbox"); + test("serializes both folder states", () => { + expect(folderOpenToStored(true)).toBe("open"); + expect(folderOpenToStored(false)).toBe("closed"); }); }); @@ -294,14 +263,6 @@ describe("withTodayNode", () => { test("disabled drops the node entirely", () => { expect(withTodayNode([source("1")], false)).toHaveLength(1); }); - - test("the virtual node yields no usable source id", () => { - const node = withTodayNode([source("1")], true)[0]!; - expect(isTodayNode(node)).toBe(true); - // Number("today") is NaN -- which is exactly why select() must branch - // on isTodayNode before reaching for sourceIds(). - expect(Number.isNaN(sourceIds(node)[0])).toBe(true); - }); }); describe("source snooze", () => { @@ -327,15 +288,8 @@ describe("source snooze", () => { ).toBe(false); }); - test("presets resolve to future timestamps", () => { - for (const preset of snoozePresets) { - const until = new Date(snoozeUntilIso(preset.hours, now)).getTime(); - expect(until).toBeGreaterThan(now); - } - expect(snoozePresets.map((preset) => preset.label)).toEqual([ - "Snooze 1 day", - "Snooze 1 week", - "Snooze forever", - ]); + test("converts snooze durations from hours to absolute timestamps", () => { + expect(snoozeUntilIso(24, now)).toBe("2026-09-10T12:00:00.000Z"); + expect(snoozeUntilIso(168, now)).toBe("2026-09-16T12:00:00.000Z"); }); }); diff --git a/src/spa/__tests__/format-date.test.ts b/src/spa/__tests__/format-date.test.ts index 3b4e3ae5..2b09332e 100644 --- a/src/spa/__tests__/format-date.test.ts +++ b/src/spa/__tests__/format-date.test.ts @@ -21,11 +21,12 @@ test.each(["", "ISO", "unix", "system"])( test("locale mode renders an absolute localized string", () => { setDateFormat("locale"); - const rendered = formatDate("2026-09-07T12:34:00Z"); - expect(rendered).not.toBe(""); - expect(rendered).toMatch(/\d/); - // Never a machine-style ISO date: that is the other mode's job. - expect(rendered).not.toMatch(/^2026-09-07/); + const date = new Date("2026-09-07T12:34:00Z"); + const expected = new Intl.DateTimeFormat(undefined, { + dateStyle: "medium", + timeStyle: "short", + }).format(date); + expect(formatDate(date.toISOString())).toBe(expected); }); test("iso mode renders fixed UTC YYYY-MM-DD HH:mm", () => { diff --git a/src/spa/__tests__/reading-session.test.ts b/src/spa/__tests__/reading-session.test.ts index dc3b8bae..2c3ffecf 100644 --- a/src/spa/__tests__/reading-session.test.ts +++ b/src/spa/__tests__/reading-session.test.ts @@ -30,14 +30,7 @@ describe("parse/serialize", () => { test("round-trips a snapshot", () => { const parsed = parseReadingSession(serializeReadingSession(aSession())); expect(parsed).toEqual(aSession()); - expect(serializeReadingSession(parsed!)).toBe( - serializeReadingSession(aSession()), - ); - }); - - test("keeps the version out of the typed shape but in the blob", () => { - const blob = JSON.parse(serializeReadingSession(aSession())); - expect(blob.version).toBe(1); + expect(JSON.parse(serializeReadingSession(aSession())).version).toBe(1); }); test("drops a blob from a different version (the migration story)", () => { @@ -54,6 +47,11 @@ describe("parse/serialize", () => { expect(parseReadingSession("not json {")).toBeUndefined(); expect(parseReadingSession("42")).toBeUndefined(); expect(parseReadingSession(JSON.stringify({ version: 1 }))).toBeUndefined(); + for (const app of [null, {}, { ...aSession().app, nodeType: "invalid" }]) { + expect( + parseReadingSession(JSON.stringify({ app, reader: [], version: 1 })), + ).toBeUndefined(); + } }); test("drops entries with out-of-range ratios", () => { @@ -64,8 +62,10 @@ describe("parse/serialize", () => { }); test("tolerates an absent app snapshot", () => { - const raw = serializeReadingSession(aSession({ app: undefined })); - expect(parseReadingSession(raw)?.app).toBeUndefined(); + const session = aSession({ app: undefined }); + expect(parseReadingSession(serializeReadingSession(session))).toEqual( + session, + ); }); }); @@ -131,7 +131,7 @@ describe("scroll ratio helpers", () => { describe("throttled recorder", () => { test("writes the first value immediately, then at most once per interval", () => { - let time = 1000; + let time = 0; const writes: number[] = []; const scheduled: Array<() => void> = []; const record = createThrottledRecorder( @@ -139,7 +139,8 @@ describe("throttled recorder", () => { { intervalMs: 500, now: () => time, - schedule: (callback) => { + schedule: (callback, delayMs) => { + expect(delayMs).toBe(500); scheduled.push(callback); return scheduled.length; }, @@ -151,41 +152,17 @@ describe("throttled recorder", () => { record(2); record(3); expect(writes).toEqual([1]); - time = 1500; + expect(scheduled).toHaveLength(1); + time = 500; scheduled.shift()!(); expect(writes).toEqual([1, 3]); // The interval now dates from the trailing write. record(4); expect(writes).toEqual([1, 3]); - time = 2000; + time = 1000; scheduled.shift()!(); expect(writes).toEqual([1, 3, 4]); }); - - test("a burst coalesces into one trailing write with the latest value", () => { - let time = 0; - const writes: number[] = []; - const scheduled: Array<() => void> = []; - const record = createThrottledRecorder( - (value: number) => writes.push(value), - { - intervalMs: 500, - now: () => time, - schedule: (callback) => { - scheduled.push(callback); - return scheduled.length; - }, - }, - ); - record(1); - record(2); - record(3); - expect(writes).toEqual([1]); - expect(scheduled.length).toBe(1); - time = 400; - scheduled[0]!(); - expect(writes).toEqual([1, 3]); - }); }); class FakeStorage { @@ -236,14 +213,18 @@ describe("ReadingSessionStore", () => { }); test("records and reads reader scroll ratios", () => { - const store = new ReadingSessionStore({ + const options = { local: new FakeStorage(), now: () => 5, session: new FakeStorage(), - }); + }; + const store = new ReadingSessionStore(options); store.recordReaderScroll(11, 0.75); expect(store.readerScroll(11)).toBe(0.75); expect(store.readerScroll(12)).toBeUndefined(); + const restored = new ReadingSessionStore(options); + restored.load(); + expect(restored.readerScroll(11)).toBe(0.75); }); test("a corrupt blob boots as no snapshot", () => { diff --git a/src/spa/__tests__/supersession.test.ts b/src/spa/__tests__/supersession.test.ts index c52efa88..ded65727 100644 --- a/src/spa/__tests__/supersession.test.ts +++ b/src/spa/__tests__/supersession.test.ts @@ -2,15 +2,10 @@ import { describe, expect, test } from "bun:test"; import { createSupersessionGuard } from "../supersession.ts"; describe("supersession guard", () => { - test("a fresh token is current until a newer one starts", () => { - const guard = createSupersessionGuard(); - const token = guard.start(); - expect(guard.isCurrent(token)).toBe(true); - }); - test("starting a new token supersedes the previous one", () => { const guard = createSupersessionGuard(); const first = guard.start(); + expect(guard.isCurrent(first)).toBe(true); const second = guard.start(); expect(guard.isCurrent(first)).toBe(false); expect(guard.isCurrent(second)).toBe(true); diff --git a/src/spa/reading-session.ts b/src/spa/reading-session.ts index 6d3a8e36..b261d156 100644 --- a/src/spa/reading-session.ts +++ b/src/spa/reading-session.ts @@ -76,7 +76,7 @@ export function parseReadingSession( if (record["version"] !== SESSION_VERSION) return undefined; const reader = parseReaderScrolls(record["reader"]); const app = parseAppSnapshot(record["app"]); - if (!reader || !app) return undefined; + if (!reader || (record["app"] !== undefined && !app)) return undefined; return { app, reader }; } diff --git a/tests/browser/spa.spec.ts b/tests/browser/spa.spec.ts index f1c75f9d..9984bac5 100644 --- a/tests/browser/spa.spec.ts +++ b/tests/browser/spa.spec.ts @@ -1169,7 +1169,10 @@ test("surfaces a malformed Options session without crashing", async ({ test("loads articles and content from a selected source", async ({ page }) => { await installApiFixture(page); await page.goto("/"); + const deleteButton = page.getByRole("button", { name: "delete articles" }); + await expect(deleteButton).toBeDisabled(); await selectSource(page); + await expect(deleteButton).toBeEnabled(); const option = page.getByRole("option", { name: /First article/ }); await expect(option).toHaveAttribute("aria-selected", "true"); @@ -1179,20 +1182,24 @@ test("loads articles and content from a selected source", async ({ page }) => { await expect(page.getByText("Feed article content")).toBeVisible(); }); -test("select all moves focus into the list so Delete works immediately", async ({ +test("select all preserves scroll and moves focus so Delete works immediately", async ({ page, }) => { - // Regression test: clicking the toolbar's "select all" button natively - // focuses the button itself, which sits outside .article-list -- the - // element handleArticleKeys (Delete/arrow-key handling) is attached to. - // Without moving focus back into the list, a Delete keypress right after - // clicking select-all was silently a no-op. const state = await installApiFixture(page, { multipleArticles: true }); await page.goto("/"); + await page.addStyleTag({ content: ".article-list { max-height: 40px; }" }); await selectSource(page); await expect(articleOptions(page)).toHaveCount(3); + const list = page.locator(".article-list"); + await list.evaluate((element) => { + element.scrollTop = element.scrollHeight; + }); + const scrolledTop = await list.evaluate((element) => element.scrollTop); + expect(scrolledTop).toBeGreaterThan(0); + await page.getByRole("button", { name: "select all" }).click(); + await expect(list).toHaveJSProperty("scrollTop", scrolledTop); await expect( page.getByRole("option", { name: /First article/ }), ).toHaveAttribute("aria-selected", "true"); @@ -1213,27 +1220,6 @@ test("select all moves focus into the list so Delete works immediately", async ( .toEqual([11, 12, 13]); }); -test("select all does not scroll the article list", async ({ page }) => { - // Regression test: focusArticleAt(0) used to always scrollIntoView the - // first row, which yanked the list back to the top even when the user - // had scrolled down before clicking select-all. - await installApiFixture(page, { multipleArticles: true }); - await page.goto("/"); - await page.addStyleTag({ content: ".article-list { max-height: 40px; }" }); - await selectSource(page); - await expect(articleOptions(page)).toHaveCount(3); - - const list = page.locator(".article-list"); - await list.evaluate((element) => { - element.scrollTop = element.scrollHeight; - }); - const scrolledTop = await list.evaluate((element) => element.scrollTop); - expect(scrolledTop).toBeGreaterThan(0); - - await page.getByRole("button", { name: "select all" }).click(); - await expect(list).toHaveJSProperty("scrollTop", scrolledTop); -}); - test("select all then clicking Delete removes every article", async ({ page, }) => { @@ -1271,25 +1257,6 @@ test("selecting a single article then pressing Delete removes only it", async ({ expect(state.removedArticleIds).toEqual([12]); }); -test("disables the delete-articles button until something is selected", async ({ - page, -}) => { - await installApiFixture(page, { multipleArticles: true }); - await page.goto("/"); - - const deleteButton = page.getByRole("button", { name: "delete articles" }); - await expect(deleteButton).toBeDisabled(); - - // Selecting a source auto-selects its first article for immediate - // reading (see "loads articles and content from a selected source"), so - // the button already reflects a selection right after this. - await selectSource(page); - await expect( - page.getByRole("option", { name: /First article/ }), - ).toHaveAttribute("aria-selected", "true"); - await expect(deleteButton).toBeEnabled(); -}); - test("source properties reuses the discovery panel with feed/website locked", async ({ page, }) => { @@ -1409,9 +1376,6 @@ test("blocks deleting a non-empty folder", async ({ page }) => { }); test("every toolbar icon renders and is clickable", async ({ page }) => { - // Regression test for the Icon component swap (inline currentColor SVG - // instead of ) -- each button must still have a nonzero hit - // area and a visible icon, not just an empty/invisible span. await installApiFixture(page, { multipleArticles: true }); await page.goto("/"); await selectSource(page); @@ -1424,16 +1388,11 @@ test("every toolbar icon renders and is clickable", async ({ page }) => { "select all", "delete articles", ]; - await Promise.all( - toolbarButtons.map(async (name) => { - const button = page.getByRole("button", { exact: true, name }).first(); - await expect(button).toBeVisible(); - const box = await button.boundingBox(); - expect(box?.width).toBeGreaterThan(0); - expect(box?.height).toBeGreaterThan(0); - await expect(button.locator("svg")).toBeVisible(); - }), - ); + for (const name of toolbarButtons) { + const button = page.getByRole("button", { exact: true, name }).first(); + await button.click({ trial: true }); + await expect(button.locator("svg")).toBeVisible(); + } }); test("shows the generic RSS icon for a source with no favicon", async ({ @@ -1877,9 +1836,6 @@ test("typing j in the feed filter does not move the selection", async ({ await expect(filter).toHaveValue("j"); }); -// Session restoration (#718). Each spec scrolls with real scroll events and -// waits out the 500ms write throttle once -- everything after that is the -// app's own state machine, no timers. test.describe("session restoration", () => { test("a reload restores the selected article and the list scroll", async ({ page, @@ -1896,15 +1852,22 @@ test.describe("session restoration", () => { await list.evaluate((element) => { element.scrollTop = 900; }); - await page.waitForTimeout(700); + await expect + .poll(() => + page.evaluate( + () => + JSON.parse( + sessionStorage.getItem("feedfathom:reading-session:v1") ?? "null", + )?.app?.listScrollTop, + ), + ) + .toBe(900); await page.reload(); // The restored selection re-opens the article in the reader pane and // puts the list back at the stored offset -- all from one boot fetch. await expect(page.locator(".reader h1")).toHaveText("Article 130"); - await expect - .poll(() => list.evaluate((element) => element.scrollTop)) - .toBeGreaterThan(500); + await expect(list).toHaveJSProperty("scrollTop", 900); await expect(articleOptions(page).nth(30)).toHaveAttribute( "aria-selected", "true", @@ -1926,13 +1889,20 @@ test.describe("session restoration", () => { return element.scrollTop; }); expect(halfScroll).toBeGreaterThan(200); - await page.waitForTimeout(700); + await expect + .poll(() => + page.evaluate( + () => + JSON.parse( + sessionStorage.getItem("feedfathom:reading-session:v1") ?? "null", + )?.reader?.[0]?.ratio, + ), + ) + .toBeCloseTo(0.5, 2); await page.reload(); await expect(page.locator(".reader h1")).toHaveText("First article"); - await expect - .poll(() => reader.evaluate((element) => element.scrollTop)) - .toBeGreaterThan(200); + await expect(reader).toHaveJSProperty("scrollTop", halfScroll); }); test("with the preference off, no snapshot is written and nothing restores", async ({ diff --git a/tools/__tests__/oxlint-plugin.test.ts b/tools/__tests__/oxlint-plugin.test.ts index 4f2e6944..5a58e304 100644 --- a/tools/__tests__/oxlint-plugin.test.ts +++ b/tools/__tests__/oxlint-plugin.test.ts @@ -55,8 +55,8 @@ function diagnosticCodes(stdout: string, stderr: string): string[] { } /** Lint an already-written file, returning only this plugin's rule names. */ -function lintFile(relativePath: string): string[] { - const result = Bun.spawnSync({ +async function lintFile(relativePath: string): Promise { + const child = Bun.spawn({ cmd: [ oxlintBin, "-c", @@ -69,10 +69,15 @@ function lintFile(relativePath: string): string[] { stderr: "pipe", stdout: "pipe", }); - return diagnosticCodes( - result.stdout.toString(), - result.stderr.toString(), - ).filter((name) => name.includes("feedfathom")); + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(child.stdout).text(), + new Response(child.stderr).text(), + child.exited, + ]); + expect([0, 1]).toContain(exitCode); + return diagnosticCodes(stdout, stderr).filter((name) => + name.includes("feedfathom"), + ); } /** Lint one source string and return the rule names that fired. */ @@ -174,7 +179,7 @@ test("no-barrel-file honours the allow list", async () => { join(directory, file), 'import { a } from "../../../a.ts";\nexport { a };', ); - expect(lintFile(file)).toEqual([]); + expect(await lintFile(file)).toEqual([]); }); /** Write a fixture at a path the layer rule reads, and lint it there. */