Skip to content

Remove redundant tests and strengthen behavioral checks #832

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zakius merged 2 commits into from
Sep 15, 2026
Merged

Remove redundant tests and strengthen behavioral checks #832

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7485286
Merge pull request #827 from SmartRSS/staging
zakius Sep 12, 2026
b024c8a
Remove redundant tests and strengthen behavioral checks
zakius Sep 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 8 additions & 36 deletions src/__tests__/server-app.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: `<html><head>
<link rel="alternate" type="application/rss+xml" title="Push" href="https://site.example/push.xml">
<link rel="alternate" type="application/rss+xml" title="Plain" href="https://site.example/plain.xml">
</head></html>`,
});
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: `<html><head>
<link rel="alternate" type="application/rss+xml" title="Dead" href="https://site.example/dead.xml">
<link rel="alternate" type="application/rss+xml" title="Live" href="https://site.example/live.xml">
</head></html>`,
});
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);

Expand All @@ -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 },
]);
});

Expand Down
2 changes: 0 additions & 2 deletions src/extension/__tests__/badge.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down
16 changes: 1 addition & 15 deletions src/features/auth/__tests__/mail-sender.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ describe("MailSender", () => {
}),
async (input, init) => {
captured = { init, input };
return new Response(null, { status: 200 });
return new Response(null, { status: 202 });
},
);

Expand Down Expand Up @@ -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({
Expand Down
15 changes: 2 additions & 13 deletions src/features/feeds/__tests__/favicon-selection.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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);
Expand Down Expand Up @@ -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 });
});

Expand Down Expand Up @@ -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);
});
});
76 changes: 25 additions & 51 deletions src/features/feeds/__tests__/feed-discovery.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")],
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand Down
30 changes: 8 additions & 22 deletions src/features/feeds/__tests__/feed-mapper.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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", () => {
Expand Down Expand Up @@ -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({
Expand Down
51 changes: 12 additions & 39 deletions src/features/feeds/__tests__/feed-preview-cache.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
10 changes: 0 additions & 10 deletions src/features/feeds/__tests__/opml-parser-cases/expected/parse-opml/single-feed.ts
View file
Open in desktop

This file was deleted.

8 changes: 0 additions & 8 deletions src/features/feeds/__tests__/opml-parser-cases/expected/process-outline/invalid-xml-url.ts
View file
Open in desktop

This file was deleted.

Loading
Loading