From a324ac5d71a8fbfa165fb32782af9c5ef68ead68 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Mon, 4 May 2026 23:13:05 +0800 Subject: [PATCH] refactor(app): extract pr-pane derivations into pure utils and unit tests (#713) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move getStateLabel and getActivityVerb out of pr-pane.tsx into pr-pane-data.ts so they can be tested without JSDOM, React, or vi.mock. Add exhaustive unit tests for both helpers alongside the existing mapPrPaneData/formatAge/deriveAvatarColor coverage. Delete pr-pane.test.tsx — the component test was asserting on derived label/icon strings that are now covered by pure function tests. --- packages/app/src/components/pr-pane.test.tsx | 169 ------------------- packages/app/src/components/pr-pane.tsx | 15 +- packages/app/src/utils/pr-pane-data.test.ts | 43 ++++- packages/app/src/utils/pr-pane-data.ts | 14 ++ 4 files changed, 57 insertions(+), 184 deletions(-) delete mode 100644 packages/app/src/components/pr-pane.test.tsx diff --git a/packages/app/src/components/pr-pane.test.tsx b/packages/app/src/components/pr-pane.test.tsx deleted file mode 100644 index dc2908905..000000000 --- a/packages/app/src/components/pr-pane.test.tsx +++ /dev/null @@ -1,169 +0,0 @@ -import React from "react"; -import { act } from "react"; -import { createRoot, type Root } from "react-dom/client"; -import { JSDOM } from "jsdom"; -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import type { PrPaneData } from "@/utils/pr-pane-data"; -import { PrPane } from "./pr-pane"; - -const { theme } = vi.hoisted(() => ({ - theme: { - spacing: { 1: 4, 2: 8, 3: 12, 4: 16 }, - fontSize: { xs: 11, sm: 13, base: 15 }, - fontWeight: { normal: "400" }, - colors: { - surfaceSidebar: "#0b0b0b", - surfaceSidebarHover: "#151515", - border: "#222", - foreground: "#fff", - foregroundMuted: "#aaa", - statusSuccess: "#30d158", - statusDanger: "#ff453a", - statusWarning: "#f2c945", - statusMerged: "#a371f7", - }, - }, -})); - -vi.mock("react-native-unistyles", () => ({ - StyleSheet: { - create: (factory: unknown) => (typeof factory === "function" ? factory(theme) : factory), - }, - useUnistyles: () => ({ theme }), -})); - -vi.mock("lucide-react-native", () => { - const createIcon = (name: string) => (props: Record) => - React.createElement("span", { ...props, "data-icon": name }); - return { - ChevronDown: createIcon("ChevronDown"), - ChevronRight: createIcon("ChevronRight"), - CircleCheck: createIcon("CircleCheck"), - CircleDot: createIcon("CircleDot"), - CircleSlash: createIcon("CircleSlash"), - CircleX: createIcon("CircleX"), - ExternalLink: createIcon("ExternalLink"), - GitMerge: createIcon("GitMerge"), - GitPullRequest: createIcon("GitPullRequest"), - GitPullRequestClosed: createIcon("GitPullRequestClosed"), - GitPullRequestDraft: createIcon("GitPullRequestDraft"), - MessageSquare: createIcon("MessageSquare"), - }; -}); - -vi.mock("@/utils/open-external-url", () => ({ - openExternalUrl: vi.fn(), -})); - -let root: Root | null = null; -let container: HTMLElement | null = null; - -beforeEach(() => { - const dom = new JSDOM(""); - vi.stubGlobal("React", React); - vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); - vi.stubGlobal("window", dom.window); - vi.stubGlobal("document", dom.window.document); - vi.stubGlobal("HTMLElement", dom.window.HTMLElement); - vi.stubGlobal("Node", dom.window.Node); - vi.stubGlobal("navigator", dom.window.navigator); - - container = document.createElement("div"); - document.body.appendChild(container); - root = createRoot(container); -}); - -afterEach(() => { - if (root) { - act(() => { - root?.unmount(); - }); - } - root = null; - container = null; - vi.unstubAllGlobals(); -}); - -function makeData(overrides: Partial = {}): PrPaneData { - return { - number: 1284, - title: "fix(server,app): coalesce tool call stream events", - state: "open", - url: "https://github.com/getpaseo/paseo/pull/1284", - reviewDecision: "changes_requested", - awaitingReviewers: [], - checks: [ - { name: "typecheck", workflow: "CI", status: "success", duration: "1m", url: "#" }, - { name: "test", workflow: "CI", status: "failure", duration: "2m", url: "#" }, - ], - activity: [ - { - kind: "review", - author: "alicek", - avatarColor: "#8b5cf6", - reviewState: "approved", - body: "LGTM", - age: "2h ago", - url: "#", - }, - { - kind: "comment", - author: "bmartin", - avatarColor: "#f97316", - body: "Worth a benchmark before/after.", - age: "3h ago", - url: "#", - }, - ], - ...overrides, - }; -} - -function render(data: PrPaneData) { - act(() => { - root?.render(); - }); -} - -describe("PrPane", () => { - it("renders the PR title and 'Open' state label from the data prop", () => { - render(makeData()); - - expect(container?.textContent).toContain("fix(server,app): coalesce tool call stream events"); - expect(container?.textContent).toContain("Open"); - }); - - it("renders the correct state label and icon for each PR state", () => { - render(makeData({ state: "draft" })); - expect(container?.textContent).toContain("Draft"); - expect(container?.querySelector('[data-icon="GitPullRequestDraft"]')).not.toBeNull(); - - render(makeData({ state: "merged" })); - expect(container?.textContent).toContain("Merged"); - expect(container?.querySelector('[data-icon="GitMerge"]')).not.toBeNull(); - - render(makeData({ state: "closed" })); - expect(container?.textContent).toContain("Closed"); - expect(container?.querySelector('[data-icon="GitPullRequestClosed"]')).not.toBeNull(); - }); - - it("derives check summary counts from the data prop", () => { - render(makeData()); - const text = container?.textContent ?? ""; - - expect(text).toContain("typecheck"); - expect(text).toContain("test"); - // 1 passed, 1 failed, 0 pending — so pending pill is hidden. - expect(text).toMatch(/1.*1/); - }); - - it("derives review summary counts and renders activity rows", () => { - render(makeData()); - const text = container?.textContent ?? ""; - - expect(text).toContain("alicek"); - expect(text).toContain("Approved"); - expect(text).toContain("bmartin"); - expect(text).toContain("Commented"); - }); -}); diff --git a/packages/app/src/components/pr-pane.tsx b/packages/app/src/components/pr-pane.tsx index 945837cfa..4c7bdacc3 100644 --- a/packages/app/src/components/pr-pane.tsx +++ b/packages/app/src/components/pr-pane.tsx @@ -16,6 +16,7 @@ import { MessageSquare, } from "lucide-react-native"; import { openExternalUrl } from "@/utils/open-external-url"; +import { getActivityVerb, getStateLabel } from "@/utils/pr-pane-data"; import type { CheckStatus, PrPaneActivity, @@ -283,13 +284,6 @@ function ActivityRow({ item }: { item: PrPaneActivity }) { ); } -function getActivityVerb(item: PrPaneActivity): string { - if (item.kind === "comment") return "Commented"; - if (item.reviewState === "approved") return "Approved"; - if (item.reviewState === "changes_requested") return "Requested changes"; - return "Reviewed"; -} - function getStateColor(state: PrState, theme: ReturnType["theme"]): string { if (state === "open") return theme.colors.statusSuccess; if (state === "draft") return theme.colors.foregroundMuted; @@ -304,13 +298,6 @@ function getStateIcon(state: PrState) { return GitPullRequest; } -function getStateLabel(state: PrState): string { - if (state === "draft") return "Draft"; - if (state === "merged") return "Merged"; - if (state === "closed") return "Closed"; - return "Open"; -} - const styles = StyleSheet.create((theme) => ({ root: { flex: 1, diff --git a/packages/app/src/utils/pr-pane-data.test.ts b/packages/app/src/utils/pr-pane-data.test.ts index e16d69462..fa98b5b4d 100644 --- a/packages/app/src/utils/pr-pane-data.test.ts +++ b/packages/app/src/utils/pr-pane-data.test.ts @@ -3,7 +3,13 @@ import type { CheckoutPrStatusResponse, PullRequestTimelineResponse, } from "@server/shared/messages"; -import { deriveAvatarColor, formatAge, mapPrPaneData } from "./pr-pane-data"; +import { + deriveAvatarColor, + formatAge, + getActivityVerb, + getStateLabel, + mapPrPaneData, +} from "./pr-pane-data"; type CheckoutPrStatus = NonNullable; type PullRequestTimeline = PullRequestTimelineResponse["payload"]; @@ -335,3 +341,38 @@ describe("formatAge", () => { expect(formatAge(now - 365 * 24 * 60 * 60_000, now)).toBe("1y ago"); }); }); + +describe("getStateLabel", () => { + it.each([ + ["open", "Open"], + ["draft", "Draft"], + ["merged", "Merged"], + ["closed", "Closed"], + ] as const)("maps %s → %s", (state, expected) => { + expect(getStateLabel(state)).toBe(expected); + }); +}); + +describe("getActivityVerb", () => { + it("returns Commented for comment kind", () => { + expect(getActivityVerb({ kind: "comment" })).toBe("Commented"); + }); + + it("returns Approved for approved review", () => { + expect(getActivityVerb({ kind: "review", reviewState: "approved" })).toBe("Approved"); + }); + + it("returns Requested changes for changes_requested review", () => { + expect(getActivityVerb({ kind: "review", reviewState: "changes_requested" })).toBe( + "Requested changes", + ); + }); + + it("returns Reviewed for a commented review with body (generic case)", () => { + expect(getActivityVerb({ kind: "review", reviewState: "commented" })).toBe("Reviewed"); + }); + + it("returns Reviewed when reviewState is undefined", () => { + expect(getActivityVerb({ kind: "review" })).toBe("Reviewed"); + }); +}); diff --git a/packages/app/src/utils/pr-pane-data.ts b/packages/app/src/utils/pr-pane-data.ts index 7fcf424ef..7f1b6a003 100644 --- a/packages/app/src/utils/pr-pane-data.ts +++ b/packages/app/src/utils/pr-pane-data.ts @@ -225,3 +225,17 @@ function hashLogin(login: string): number { } return hash; } + +export function getStateLabel(state: PrState): string { + if (state === "draft") return "Draft"; + if (state === "merged") return "Merged"; + if (state === "closed") return "Closed"; + return "Open"; +} + +export function getActivityVerb(item: Pick): string { + if (item.kind === "comment") return "Commented"; + if (item.reviewState === "approved") return "Approved"; + if (item.reviewState === "changes_requested") return "Requested changes"; + return "Reviewed"; +}