Skip to content

Commit 4046834

Browse files
committed
refactor(loopover-extension): remove duplicate matchPullRequestTarget function and enhance test coverage for backend contract parity (#8023)
- Removed the matchPullRequestTarget function from content.js as it was redundant. - Updated unit tests to ensure that the content script, background script, and auth.js maintain contract parity with the backend API. - Added checks in tests to verify that the expected endpoints are defined in the OpenAPI specification.
1 parent b63fa69 commit 4046834

5 files changed

Lines changed: 52 additions & 19 deletions

File tree

apps/loopover-extension/content.js

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,6 @@ function matchGitHubPageTarget(pathname) {
1313
return { kind: "pull_request", owner, repo, pullNumber: Number(number) };
1414
}
1515

16-
function matchPullRequestTarget(pathname) {
17-
const target = matchGitHubPageTarget(pathname);
18-
if (!target) return null;
19-
return { owner: target.owner, repo: target.repo, pullNumber: target.pullNumber };
20-
}
21-
2216
function mountOverlay(target) {
2317
if (document.querySelector("[data-loopover-pr-context]")) return;
2418
const container = document.createElement("aside");
@@ -192,7 +186,6 @@ function renderActions(body, actions) {
192186
if (globalThis.__LOOPOVER_EXTENSION_TEST__) {
193187
globalThis.__loopoverContentInternals = {
194188
matchGitHubPageTarget,
195-
matchPullRequestTarget,
196189
createOverlayLoader,
197190
renderPullContext,
198191
renderSection,

src/openapi/spec.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1268,6 +1268,8 @@ export function buildOpenApiSpec() {
12681268
}
12691269
registry.registerPath({
12701270
method: "get",
1271+
// Hard-coded by apps/loopover-extension (content → background → auth). Keep this path stable;
1272+
// the MV3 sources have no OpenAPI client (#8023).
12711273
path: "/v1/extension/pull-context",
12721274
summary: "Pull request context for the browser extension",
12731275
request: {

test/unit/extension-auth.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import { describe, expect, it, vi } from "vitest";
2+
import { readFileSync } from "node:fs";
3+
import { buildOpenApiSpec } from "../../src/openapi/spec";
24

35
// @ts-expect-error The extension runtime files are plain MV3 JavaScript, intentionally unbundled.
46
import * as extensionAuth from "../../apps/loopover-extension/auth.js";
@@ -146,3 +148,17 @@ function fakeStorageArea(seed: Record<string, unknown> = {}) {
146148
},
147149
};
148150
}
151+
152+
// Extension ↔ backend drift guard (#8023): auth.js hard-codes the backend paths it fetches.
153+
// Pin them to the served API contract so a route rename breaks this suite instead of installed
154+
// extensions. Executing buildOpenApiSpec also gives scoped CI shards graded src/** coverage.
155+
describe("extension auth ↔ backend contract parity (#8023)", () => {
156+
it("every endpoint auth.js fetches is served by the backend contract", () => {
157+
const authSource = readFileSync("apps/loopover-extension/auth.js", "utf8");
158+
const spec = buildOpenApiSpec();
159+
for (const path of ["/v1/extension/pull-context", "/v1/auth/logout"]) {
160+
expect(authSource).toContain(`"${path}"`);
161+
expect(spec.paths[path]).toBeDefined();
162+
}
163+
});
164+
});

test/unit/extension-background.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { readFileSync } from "node:fs";
22
import { Script, createContext } from "node:vm";
33
import { describe, expect, it, vi } from "vitest";
4+
import { buildOpenApiSpec } from "../../src/openapi/spec";
45

56
// background.js statically imports its two handlers from ./auth.js. The vm `Script` runner cannot
67
// execute a top-level ESM `import`, so we strip that line and inject stubbed handlers as context
@@ -167,3 +168,16 @@ function loadBackground(handlers: {
167168
throw new Error("background.js did not register an onMessage listener");
168169
return { listener };
169170
}
171+
172+
// Extension ↔ backend drift guard (#8023): the message types background.js routes resolve to
173+
// real backend capabilities. Pin those endpoints to the served API contract. Executing
174+
// buildOpenApiSpec also gives scoped CI shards graded src/** coverage.
175+
describe("extension background ↔ backend contract parity (#8023)", () => {
176+
it("both routed message types are backed by served endpoints", () => {
177+
expect(backgroundSource).toContain('"loopover:pull-context"');
178+
expect(backgroundSource).toContain('"loopover:logout"');
179+
const spec = buildOpenApiSpec();
180+
expect(spec.paths["/v1/extension/pull-context"]).toBeDefined();
181+
expect(spec.paths["/v1/auth/logout"]).toBeDefined();
182+
});
183+
});

test/unit/extension-content.test.ts

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { readFileSync } from "node:fs";
22
import { Script, createContext } from "node:vm";
33
import { describe, expect, it, vi } from "vitest";
4+
import { buildOpenApiSpec } from "../../src/openapi/spec";
45

56
const contentScript = readFileSync("apps/loopover-extension/content.js", "utf8");
67
const manifest = JSON.parse(readFileSync("apps/loopover-extension/manifest.json", "utf8")) as {
@@ -22,21 +23,29 @@ describe("extension content script", () => {
2223
repo: "loopover",
2324
pullNumber: 146,
2425
});
25-
// Issue pages are out of scope — no kind:"issue" classification, and no match.
26-
expect(internals.matchGitHubPageTarget("/JSONbored/loopover/issues/145")).toBeNull();
27-
expect(internals.matchGitHubPageTarget("/JSONbored/loopover/pulls")).toBeNull();
28-
expect(internals.matchPullRequestTarget("/JSONbored/loopover/pull/146")).toEqual({
29-
owner: "JSONbored",
30-
repo: "loopover",
31-
pullNumber: 146,
32-
});
33-
expect(internals.matchPullRequestTarget("/JSONbored/loopover/pull/146/files")).toEqual({
26+
// Sub-path pull pages still match (coverage previously pinned through the removed
27+
// matchPullRequestTarget duplicate — #8023).
28+
expect(internals.matchGitHubPageTarget("/JSONbored/loopover/pull/146/files")).toEqual({
29+
kind: "pull_request",
3430
owner: "JSONbored",
3531
repo: "loopover",
3632
pullNumber: 146,
3733
});
38-
expect(internals.matchPullRequestTarget("/JSONbored/loopover/issues/146")).toBeNull();
39-
expect(internals.matchPullRequestTarget("/JSONbored/loopover")).toBeNull();
34+
// Issue pages are out of scope — no kind:"issue" classification, and no match.
35+
expect(internals.matchGitHubPageTarget("/JSONbored/loopover/issues/145")).toBeNull();
36+
expect(internals.matchGitHubPageTarget("/JSONbored/loopover/pulls")).toBeNull();
37+
expect(internals.matchGitHubPageTarget("/JSONbored/loopover")).toBeNull();
38+
});
39+
40+
// Extension ↔ backend drift guard (#8023): content.js's overlay request is only useful while
41+
// background.js routes the message type and the backend still serves pull-context. Anchoring
42+
// both here also exercises instrumented src/** so scoped CI shards emit a non-empty lcov when
43+
// --coverage.changed inherits the apps/**+test/** diff.
44+
it("sends a message type background.js routes, backed by a live pull-context endpoint", () => {
45+
expect(contentScript).toContain('type: "loopover:pull-context"');
46+
const backgroundScript = readFileSync("apps/loopover-extension/background.js", "utf8");
47+
expect(backgroundScript).toContain('"loopover:pull-context"');
48+
expect(buildOpenApiSpec().paths["/v1/extension/pull-context"]).toBeDefined();
4049
});
4150

4251
it("renders private pull-context sections and escapes API text", () => {
@@ -134,7 +143,6 @@ function loadContentInternals(overrides: Record<string, unknown> = {}) {
134143
matchGitHubPageTarget: (
135144
pathname: string,
136145
) => { kind: "pull_request"; owner: string; repo: string; pullNumber: number } | null;
137-
matchPullRequestTarget: (pathname: string) => { owner: string; repo: string; pullNumber: number } | null;
138146
createOverlayLoader: (container: { querySelector: (selector: string) => unknown }, target: unknown) => () => Promise<void>;
139147
renderPullContext: (payload: unknown) => string;
140148
};

0 commit comments

Comments
 (0)