From 4046834269e6f33314fe27d159cdc2dbb230bc9b Mon Sep 17 00:00:00 2001 From: milosde111 Date: Wed, 22 Jul 2026 10:43:41 -0700 Subject: [PATCH] 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. --- apps/loopover-extension/content.js | 7 ------ src/openapi/spec.ts | 2 ++ test/unit/extension-auth.test.ts | 16 +++++++++++++ test/unit/extension-background.test.ts | 14 +++++++++++ test/unit/extension-content.test.ts | 32 ++++++++++++++++---------- 5 files changed, 52 insertions(+), 19 deletions(-) diff --git a/apps/loopover-extension/content.js b/apps/loopover-extension/content.js index 29f94aed4a..f5e58b5306 100644 --- a/apps/loopover-extension/content.js +++ b/apps/loopover-extension/content.js @@ -13,12 +13,6 @@ function matchGitHubPageTarget(pathname) { return { kind: "pull_request", owner, repo, pullNumber: Number(number) }; } -function matchPullRequestTarget(pathname) { - const target = matchGitHubPageTarget(pathname); - if (!target) return null; - return { owner: target.owner, repo: target.repo, pullNumber: target.pullNumber }; -} - function mountOverlay(target) { if (document.querySelector("[data-loopover-pr-context]")) return; const container = document.createElement("aside"); @@ -192,7 +186,6 @@ function renderActions(body, actions) { if (globalThis.__LOOPOVER_EXTENSION_TEST__) { globalThis.__loopoverContentInternals = { matchGitHubPageTarget, - matchPullRequestTarget, createOverlayLoader, renderPullContext, renderSection, diff --git a/src/openapi/spec.ts b/src/openapi/spec.ts index 53acd2ca76..ebc0f792b1 100644 --- a/src/openapi/spec.ts +++ b/src/openapi/spec.ts @@ -1268,6 +1268,8 @@ export function buildOpenApiSpec() { } registry.registerPath({ method: "get", + // Hard-coded by apps/loopover-extension (content → background → auth). Keep this path stable; + // the MV3 sources have no OpenAPI client (#8023). path: "/v1/extension/pull-context", summary: "Pull request context for the browser extension", request: { diff --git a/test/unit/extension-auth.test.ts b/test/unit/extension-auth.test.ts index 80338e43e0..a2a058a642 100644 --- a/test/unit/extension-auth.test.ts +++ b/test/unit/extension-auth.test.ts @@ -1,4 +1,6 @@ import { describe, expect, it, vi } from "vitest"; +import { readFileSync } from "node:fs"; +import { buildOpenApiSpec } from "../../src/openapi/spec"; // @ts-expect-error The extension runtime files are plain MV3 JavaScript, intentionally unbundled. import * as extensionAuth from "../../apps/loopover-extension/auth.js"; @@ -146,3 +148,17 @@ function fakeStorageArea(seed: Record = {}) { }, }; } + +// Extension ↔ backend drift guard (#8023): auth.js hard-codes the backend paths it fetches. +// Pin them to the served API contract so a route rename breaks this suite instead of installed +// extensions. Executing buildOpenApiSpec also gives scoped CI shards graded src/** coverage. +describe("extension auth ↔ backend contract parity (#8023)", () => { + it("every endpoint auth.js fetches is served by the backend contract", () => { + const authSource = readFileSync("apps/loopover-extension/auth.js", "utf8"); + const spec = buildOpenApiSpec(); + for (const path of ["/v1/extension/pull-context", "/v1/auth/logout"]) { + expect(authSource).toContain(`"${path}"`); + expect(spec.paths[path]).toBeDefined(); + } + }); +}); diff --git a/test/unit/extension-background.test.ts b/test/unit/extension-background.test.ts index 1d35d77c6c..00fad7adbf 100644 --- a/test/unit/extension-background.test.ts +++ b/test/unit/extension-background.test.ts @@ -1,6 +1,7 @@ import { readFileSync } from "node:fs"; import { Script, createContext } from "node:vm"; import { describe, expect, it, vi } from "vitest"; +import { buildOpenApiSpec } from "../../src/openapi/spec"; // background.js statically imports its two handlers from ./auth.js. The vm `Script` runner cannot // execute a top-level ESM `import`, so we strip that line and inject stubbed handlers as context @@ -167,3 +168,16 @@ function loadBackground(handlers: { throw new Error("background.js did not register an onMessage listener"); return { listener }; } + +// Extension ↔ backend drift guard (#8023): the message types background.js routes resolve to +// real backend capabilities. Pin those endpoints to the served API contract. Executing +// buildOpenApiSpec also gives scoped CI shards graded src/** coverage. +describe("extension background ↔ backend contract parity (#8023)", () => { + it("both routed message types are backed by served endpoints", () => { + expect(backgroundSource).toContain('"loopover:pull-context"'); + expect(backgroundSource).toContain('"loopover:logout"'); + const spec = buildOpenApiSpec(); + expect(spec.paths["/v1/extension/pull-context"]).toBeDefined(); + expect(spec.paths["/v1/auth/logout"]).toBeDefined(); + }); +}); diff --git a/test/unit/extension-content.test.ts b/test/unit/extension-content.test.ts index e8b9957503..1ad05b6aca 100644 --- a/test/unit/extension-content.test.ts +++ b/test/unit/extension-content.test.ts @@ -1,6 +1,7 @@ import { readFileSync } from "node:fs"; import { Script, createContext } from "node:vm"; import { describe, expect, it, vi } from "vitest"; +import { buildOpenApiSpec } from "../../src/openapi/spec"; const contentScript = readFileSync("apps/loopover-extension/content.js", "utf8"); const manifest = JSON.parse(readFileSync("apps/loopover-extension/manifest.json", "utf8")) as { @@ -22,21 +23,29 @@ describe("extension content script", () => { repo: "loopover", pullNumber: 146, }); - // Issue pages are out of scope — no kind:"issue" classification, and no match. - expect(internals.matchGitHubPageTarget("/JSONbored/loopover/issues/145")).toBeNull(); - expect(internals.matchGitHubPageTarget("/JSONbored/loopover/pulls")).toBeNull(); - expect(internals.matchPullRequestTarget("/JSONbored/loopover/pull/146")).toEqual({ - owner: "JSONbored", - repo: "loopover", - pullNumber: 146, - }); - expect(internals.matchPullRequestTarget("/JSONbored/loopover/pull/146/files")).toEqual({ + // Sub-path pull pages still match (coverage previously pinned through the removed + // matchPullRequestTarget duplicate — #8023). + expect(internals.matchGitHubPageTarget("/JSONbored/loopover/pull/146/files")).toEqual({ + kind: "pull_request", owner: "JSONbored", repo: "loopover", pullNumber: 146, }); - expect(internals.matchPullRequestTarget("/JSONbored/loopover/issues/146")).toBeNull(); - expect(internals.matchPullRequestTarget("/JSONbored/loopover")).toBeNull(); + // Issue pages are out of scope — no kind:"issue" classification, and no match. + expect(internals.matchGitHubPageTarget("/JSONbored/loopover/issues/145")).toBeNull(); + expect(internals.matchGitHubPageTarget("/JSONbored/loopover/pulls")).toBeNull(); + expect(internals.matchGitHubPageTarget("/JSONbored/loopover")).toBeNull(); + }); + + // Extension ↔ backend drift guard (#8023): content.js's overlay request is only useful while + // background.js routes the message type and the backend still serves pull-context. Anchoring + // both here also exercises instrumented src/** so scoped CI shards emit a non-empty lcov when + // --coverage.changed inherits the apps/**+test/** diff. + it("sends a message type background.js routes, backed by a live pull-context endpoint", () => { + expect(contentScript).toContain('type: "loopover:pull-context"'); + const backgroundScript = readFileSync("apps/loopover-extension/background.js", "utf8"); + expect(backgroundScript).toContain('"loopover:pull-context"'); + expect(buildOpenApiSpec().paths["/v1/extension/pull-context"]).toBeDefined(); }); it("renders private pull-context sections and escapes API text", () => { @@ -134,7 +143,6 @@ function loadContentInternals(overrides: Record = {}) { matchGitHubPageTarget: ( pathname: string, ) => { kind: "pull_request"; owner: string; repo: string; pullNumber: number } | null; - matchPullRequestTarget: (pathname: string) => { owner: string; repo: string; pullNumber: number } | null; createOverlayLoader: (container: { querySelector: (selector: string) => unknown }, target: unknown) => () => Promise; renderPullContext: (payload: unknown) => string; };