From bbfd630fc5158829894a2c53170d1fb9ca50a7bc Mon Sep 17 00:00:00 2001 From: Vlad Mocanu Date: Thu, 26 Feb 2026 12:51:08 +0200 Subject: [PATCH] =?UTF-8?q?fix(prd-7):=20M4=20test=20coverage=20review=20?= =?UTF-8?q?=E2=80=94=2028=20new=20tests=20for=20comment=20operations=20and?= =?UTF-8?q?=20branch=20setup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - test/create-initial-comment.test.ts: 15 tests covering GitHub/Forgejo platform branches, sticky comment matching, review comment fallback, error handling with GITHUB_OUTPUT verification - test/update-tracking-comment.test.ts: 11 tests covering body construction, branch link logic (issue vs PR), comment type routing, Forgejo URLs - test/setup-branch-forgejo.test.ts: 2 tests verifying Forgejo uses repos.getBranch() while GitHub uses git.getRef() for source branch SHA - test/COVERAGE-NOTES.md: documents all coverage status and accepted gaps (tag mode orchestrator, entrypoint) with rationale - Notable finding: CLAUDE_APP_BOT_ID discrepancy flagged for M5 Progress: 67% complete (4/6 milestones) — M5 code quality next Co-Authored-By: Claude Opus 4.6 --- prds/7-senior-review-forgejo.md | 2 +- test/COVERAGE-NOTES.md | 51 +++ test/create-initial-comment.test.ts | 527 +++++++++++++++++++++++++++ test/setup-branch-forgejo.test.ts | 150 ++++++++ test/update-tracking-comment.test.ts | 277 ++++++++++++++ 5 files changed, 1006 insertions(+), 1 deletion(-) create mode 100644 test/COVERAGE-NOTES.md create mode 100644 test/create-initial-comment.test.ts create mode 100644 test/setup-branch-forgejo.test.ts create mode 100644 test/update-tracking-comment.test.ts diff --git a/prds/7-senior-review-forgejo.md b/prds/7-senior-review-forgejo.md index 27d38cf..73128f8 100644 --- a/prds/7-senior-review-forgejo.md +++ b/prds/7-senior-review-forgejo.md @@ -84,7 +84,7 @@ Perform a thorough, documented code review of all Forgejo-related changes. The r - Secrets in logs: review all `core.info()` / `core.debug()` calls - **Deliverable**: Security findings with severity ratings -- [ ] **M4: Test coverage review & improvements** +- [x] **M4: Test coverage review & improvements** - Assess existing unit test coverage for Forgejo code paths - Identify critical paths without test coverage diff --git a/test/COVERAGE-NOTES.md b/test/COVERAGE-NOTES.md new file mode 100644 index 0000000..249d880 --- /dev/null +++ b/test/COVERAGE-NOTES.md @@ -0,0 +1,51 @@ +# Test Coverage Notes — Forgejo Platform Support + +## Well Covered (unit tested) + +| Module | Tests | File | +| -------------------------------- | --------------- | ------------------------------------ | +| `src/forgejo/token.ts` | 8 tests | `test/forgejo-token.test.ts` | +| `src/platform/detector.ts` | 17 tests | `test/platform-detector.test.ts` | +| `src/forgejo/comment-logic.ts` | 30+ tests | `test/forgejo-comment-logic.test.ts` | +| `src/forgejo/data/fetcher.ts` | 50+ tests | `test/forgejo-data-fetcher.test.ts` | +| `src/github/validation/actor.ts` | 6 Forgejo tests | `test/actor.test.ts` | +| `src/mcp/install-mcp-server.ts` | 8 Forgejo tests | `test/install-mcp-server.test.ts` | + +## Added in M4 + +| Module | Tests | File | +| ------------------------------------------------------ | -------- | -------------------------------------- | +| `src/github/operations/comments/create-initial.ts` | 15 tests | `test/create-initial-comment.test.ts` | +| `src/github/operations/comments/update-with-branch.ts` | 11 tests | `test/update-tracking-comment.test.ts` | +| `src/github/operations/branch.ts` (API divergence) | 2 tests | `test/setup-branch-forgejo.test.ts` | + +## Accepted Gaps (orchestrators) + +### `src/modes/tag/index.ts` — `prepareTagMode()` + +**Why not unit tested**: Calls 8+ modules (checkHumanActor, createInitialComment, +fetchForgejoData/fetchGitHubData, setupBranch, configureGitAuth, createPrompt, +prepareMcpConfig). Each is individually unit tested. + +**Risk**: Low. The Forgejo branch is a simple `if (platform === "forgejo")` +choosing `fetchForgejoData` vs `fetchGitHubData`. Both are thoroughly tested. + +**Coverage path**: Integration tests in the separate `install-test` repo. + +### `src/entrypoints/run.ts` — main orchestrator + +**Why not unit tested**: Token setup path is a trivial conditional (1 line). Testing +requires mocking the entire execution environment (GitHub Actions context, CLI +installation, process lifecycle). + +**Risk**: Very low. The Forgejo token path calls `setupForgejoToken()` which +has 8 dedicated tests. + +**Coverage path**: Integration tests in the separate `install-test` repo. + +## Known Issue: CLAUDE_APP_BOT_ID Discrepancy + +`src/github/operations/comments/create-initial.ts` (line 23) defines a local +`CLAUDE_APP_BOT_ID = 209825114`, while `src/github/constants.ts` exports +`CLAUDE_APP_BOT_ID = 41898282`. These are different values with the same name. +This should be investigated in M5 (code quality review). diff --git a/test/create-initial-comment.test.ts b/test/create-initial-comment.test.ts new file mode 100644 index 0000000..c1386d2 --- /dev/null +++ b/test/create-initial-comment.test.ts @@ -0,0 +1,527 @@ +#!/usr/bin/env bun + +import { + describe, + test, + expect, + beforeEach, + afterEach, + spyOn, + type Mock, +} from "bun:test"; +import { tmpdir } from "os"; +import { join } from "path"; +import { readFileSync, unlinkSync, writeFileSync } from "fs"; +import { createInitialComment } from "../src/github/operations/comments/create-initial"; +import { _resetPlatformCache } from "../src/platform/detector"; +import { createMockContext } from "./mockContext"; +import type { Octokit } from "@octokit/rest"; + +// Note: create-initial.ts defines its own CLAUDE_APP_BOT_ID = 209825114 (line 23), +// which differs from src/github/constants.ts CLAUDE_APP_BOT_ID = 41898282. +// Tests use 209825114 to match the actual code in create-initial.ts. +const LOCAL_CLAUDE_APP_BOT_ID = 209825114; + +function createMockOctokit(overrides: Record = {}) { + // createInitialComment receives an Octokit instance and accesses octokit.rest.issues.* + return { + rest: { + issues: { + listComments: overrides.listComments ?? (async () => ({ data: [] })), + createComment: + overrides.createComment ?? (async () => ({ data: { id: 100 } })), + updateComment: + overrides.updateComment ?? (async () => ({ data: { id: 200 } })), + }, + pulls: { + createReplyForReviewComment: + overrides.createReplyForReviewComment ?? + (async () => ({ data: { id: 300 } })), + }, + }, + } as unknown as Octokit; +} + +describe("createInitialComment", () => { + let savedPlatform: string | undefined; + let savedServerUrl: string | undefined; + let savedRunNumber: string | undefined; + let savedRunId: string | undefined; + let savedGithubOutput: string | undefined; + let tempOutputFile: string; + let consoleLogSpy: Mock<(...args: any[]) => void>; + let consoleErrorSpy: Mock<(...args: any[]) => void>; + + beforeEach(() => { + _resetPlatformCache(); + savedPlatform = process.env.PLATFORM; + savedServerUrl = process.env.GITHUB_SERVER_URL; + savedRunNumber = process.env.GITHUB_RUN_NUMBER; + savedRunId = process.env.GITHUB_RUN_ID; + savedGithubOutput = process.env.GITHUB_OUTPUT; + + // Create a real temp file for GITHUB_OUTPUT + tempOutputFile = join( + tmpdir(), + `test-gh-output-${Date.now()}-${Math.random().toString(36).slice(2)}`, + ); + writeFileSync(tempOutputFile, ""); + process.env.GITHUB_OUTPUT = tempOutputFile; + + // Default to GitHub platform + delete process.env.PLATFORM; + process.env.GITHUB_SERVER_URL = "https://github.com"; + process.env.GITHUB_RUN_ID = "9876543"; + + consoleLogSpy = spyOn(console, "log").mockImplementation(() => {}); + consoleErrorSpy = spyOn(console, "error").mockImplementation(() => {}); + }); + + afterEach(() => { + if (savedPlatform !== undefined) process.env.PLATFORM = savedPlatform; + else delete process.env.PLATFORM; + if (savedServerUrl !== undefined) + process.env.GITHUB_SERVER_URL = savedServerUrl; + else delete process.env.GITHUB_SERVER_URL; + if (savedRunNumber !== undefined) + process.env.GITHUB_RUN_NUMBER = savedRunNumber; + else delete process.env.GITHUB_RUN_NUMBER; + if (savedRunId !== undefined) process.env.GITHUB_RUN_ID = savedRunId; + else delete process.env.GITHUB_RUN_ID; + if (savedGithubOutput !== undefined) + process.env.GITHUB_OUTPUT = savedGithubOutput; + else delete process.env.GITHUB_OUTPUT; + + try { + unlinkSync(tempOutputFile); + } catch {} + + consoleLogSpy.mockRestore(); + consoleErrorSpy.mockRestore(); + _resetPlatformCache(); + }); + + describe("GitHub platform — basic comment creation", () => { + test("creates issue comment for issue_comment event", async () => { + const createComment = async () => ({ data: { id: 42 } }); + const createCommentSpy = spyOn({ createComment }, "createComment"); + const mockOctokit = createMockOctokit({ + createComment: createCommentSpy, + }); + + const context = createMockContext({ entityNumber: 10 }); + const result = await createInitialComment(mockOctokit, context); + + expect(result.id).toBe(42); + expect(createCommentSpy).toHaveBeenCalled(); + const callArgs = (createCommentSpy.mock.calls as any[][])[0]![0]; + expect(callArgs.owner).toBe("test-owner"); + expect(callArgs.repo).toBe("test-repo"); + expect(callArgs.issue_number).toBe(10); + // GitHub body uses CDN spinner image + expect(callArgs.body).toContain("user-attachments/assets"); + expect(callArgs.body).toContain("Claude Code is working"); + }); + + test("writes comment ID to GITHUB_OUTPUT file", async () => { + const mockOctokit = createMockOctokit({ + createComment: async () => ({ data: { id: 777 } }), + }); + const context = createMockContext(); + + await createInitialComment(mockOctokit, context); + + const output = readFileSync(tempOutputFile, "utf-8"); + expect(output).toContain("claude_comment_id=777"); + }); + + test("uses GitHub CDN spinner and actions/runs URL in body", async () => { + let capturedBody = ""; + const mockOctokit = createMockOctokit({ + createComment: async (args: any) => { + capturedBody = args.body; + return { data: { id: 1 } }; + }, + }); + const context = createMockContext({ runId: "12345" }); + + await createInitialComment(mockOctokit, context); + + // GitHub spinner is a CDN image + expect(capturedBody).toContain("user-attachments/assets"); + expect(capturedBody).not.toContain("⏳"); + // GitHub URL uses GITHUB_SERVER_URL + actions/runs/ + expect(capturedBody).toContain( + "https://github.com/test-owner/test-repo/actions/runs/12345", + ); + }); + }); + + describe("GitHub platform — sticky comment on PR", () => { + test("creates new comment when no existing comment matches", async () => { + const createComment = async () => ({ data: { id: 50 } }); + const updateComment = async () => ({ data: { id: 99 } }); + const createSpy = spyOn({ createComment }, "createComment"); + const updateSpy = spyOn({ updateComment }, "updateComment"); + const mockOctokit = createMockOctokit({ + listComments: async () => ({ data: [] }), + createComment: createSpy, + updateComment: updateSpy, + }); + + const context = createMockContext({ + eventName: "pull_request", + isPR: true, + entityNumber: 5, + inputs: { useStickyComment: true }, + payload: { + action: "opened", + number: 5, + pull_request: { number: 5 }, + } as any, + }); + + const result = await createInitialComment(mockOctokit, context); + expect(result.id).toBe(50); + expect(createSpy).toHaveBeenCalled(); + expect(updateSpy).not.toHaveBeenCalled(); + }); + + test("updates existing comment matched by CLAUDE_APP_BOT_ID", async () => { + const updateComment = async () => ({ data: { id: 200 } }); + const updateSpy = spyOn({ updateComment }, "updateComment"); + const createComment = async () => ({ data: { id: 100 } }); + const createSpy = spyOn({ createComment }, "createComment"); + const mockOctokit = createMockOctokit({ + listComments: async () => ({ + data: [ + { + id: 200, + user: { + id: LOCAL_CLAUDE_APP_BOT_ID, + type: "User", + login: "someone", + }, + body: "old body", + }, + ], + }), + createComment: createSpy, + updateComment: updateSpy, + }); + + const context = createMockContext({ + eventName: "pull_request", + isPR: true, + entityNumber: 5, + inputs: { useStickyComment: true }, + payload: { + action: "opened", + number: 5, + pull_request: { number: 5 }, + } as any, + }); + + const result = await createInitialComment(mockOctokit, context); + expect(result.id).toBe(200); + expect(updateSpy).toHaveBeenCalled(); + expect(createSpy).not.toHaveBeenCalled(); + }); + + test("updates existing comment matched by Bot type with 'claude' in login", async () => { + const updateComment = async () => ({ data: { id: 201 } }); + const updateSpy = spyOn({ updateComment }, "updateComment"); + const mockOctokit = createMockOctokit({ + listComments: async () => ({ + data: [ + { + id: 201, + user: { id: 999, type: "Bot", login: "claude[bot]" }, + body: "old", + }, + ], + }), + updateComment: updateSpy, + }); + + const context = createMockContext({ + eventName: "pull_request", + isPR: true, + inputs: { useStickyComment: true }, + payload: { + action: "opened", + number: 1, + pull_request: { number: 1 }, + } as any, + }); + + const result = await createInitialComment(mockOctokit, context); + expect(result.id).toBe(201); + expect(updateSpy).toHaveBeenCalled(); + }); + + test("updates existing comment matched by exact body content", async () => { + // To test body matching, we need the mock comment's body to exactly equal + // the generated initialBody. Import the GitHub comment builder to construct + // the matching body with the same runId as the mock context. + const { createCommentBody, createJobRunLink } = await import( + "../src/github/operations/comments/common" + ); + const expectedBody = createCommentBody( + createJobRunLink("test-owner", "test-repo", "1234567890"), + ); + + const updateComment = async () => ({ data: { id: 202 } }); + const updateSpy = spyOn({ updateComment }, "updateComment"); + const mockOctokit = createMockOctokit({ + listComments: async () => ({ + data: [ + { + id: 202, + user: { id: 1, type: "User", login: "random-user" }, + body: expectedBody, + }, + ], + }), + updateComment: updateSpy, + }); + + const context = createMockContext({ + eventName: "pull_request", + isPR: true, + inputs: { useStickyComment: true }, + payload: { + action: "opened", + number: 1, + pull_request: { number: 1 }, + } as any, + }); + + const result = await createInitialComment(mockOctokit, context); + expect(result.id).toBe(202); + expect(updateSpy).toHaveBeenCalled(); + }); + }); + + describe("GitHub platform — PR review comment reply", () => { + test("creates reply via createReplyForReviewComment when payload has comment.id", async () => { + const createReply = async () => ({ data: { id: 301 } }); + const replySpy = spyOn({ createReply }, "createReply"); + const mockOctokit = createMockOctokit({ + createReplyForReviewComment: replySpy, + }); + + const context = createMockContext({ + eventName: "pull_request_review_comment", + isPR: true, + entityNumber: 10, + payload: { + action: "created", + comment: { id: 555 }, + pull_request: { number: 10 }, + } as any, + }); + + const result = await createInitialComment(mockOctokit, context); + expect(result.id).toBe(301); + expect(replySpy).toHaveBeenCalled(); + const callArgs = (replySpy.mock.calls as any[][])[0]![0]; + expect(callArgs.comment_id).toBe(555); + expect(callArgs.pull_number).toBe(10); + }); + + test("falls back to createComment when payload lacks comment.id", async () => { + const createComment = async () => ({ data: { id: 102 } }); + const createSpy = spyOn({ createComment }, "createComment"); + const createReply = async () => ({ data: { id: 301 } }); + const replySpy = spyOn({ createReply }, "createReply"); + const mockOctokit = createMockOctokit({ + createComment: createSpy, + createReplyForReviewComment: replySpy, + }); + + const context = createMockContext({ + eventName: "pull_request_review_comment", + isPR: true, + entityNumber: 10, + // No comment.id in payload (Forgejo behavior) + payload: { + action: "created", + pull_request: { number: 10 }, + } as any, + }); + + const result = await createInitialComment(mockOctokit, context); + expect(result.id).toBe(102); + expect(createSpy).toHaveBeenCalled(); + expect(replySpy).not.toHaveBeenCalled(); + }); + }); + + describe("Forgejo platform", () => { + beforeEach(() => { + _resetPlatformCache(); + process.env.PLATFORM = "forgejo"; + process.env.GITHUB_SERVER_URL = "https://git.example.com"; + process.env.GITHUB_RUN_NUMBER = "42"; + }); + + test("uses Unicode spinner and Forgejo URL format in body", async () => { + let capturedBody = ""; + const mockOctokit = createMockOctokit({ + createComment: async (args: any) => { + capturedBody = args.body; + return { data: { id: 1 } }; + }, + }); + const context = createMockContext(); + + await createInitialComment(mockOctokit, context); + + // Forgejo uses Unicode hourglass, not CDN image + expect(capturedBody).toContain("⏳"); + expect(capturedBody).not.toContain("user-attachments/assets"); + // Forgejo URL uses GITHUB_RUN_NUMBER + expect(capturedBody).toContain( + "https://git.example.com/test-owner/test-repo/actions/runs/42", + ); + }); + + test("sticky comment matches by botName input on Forgejo", async () => { + const updateComment = async () => ({ data: { id: 203 } }); + const updateSpy = spyOn({ updateComment }, "updateComment"); + const mockOctokit = createMockOctokit({ + listComments: async () => ({ + data: [ + { + id: 203, + user: { id: 999, type: "User", login: "claude" }, + body: "some old body", + }, + ], + }), + updateComment: updateSpy, + }); + + const context = createMockContext({ + eventName: "pull_request", + isPR: true, + inputs: { useStickyComment: true, botName: "claude" }, + payload: { + action: "opened", + number: 1, + pull_request: { number: 1 }, + } as any, + }); + + const result = await createInitialComment(mockOctokit, context); + expect(result.id).toBe(203); + expect(updateSpy).toHaveBeenCalled(); + }); + + test("review comment event falls back to createComment (no comment.id in Forgejo payload)", async () => { + const createComment = async () => ({ data: { id: 103 } }); + const createSpy = spyOn({ createComment }, "createComment"); + const mockOctokit = createMockOctokit({ + createComment: createSpy, + }); + + const context = createMockContext({ + eventName: "pull_request_review_comment", + isPR: true, + entityNumber: 7, + payload: { + action: "created", + pull_request: { number: 7 }, + // Forgejo payload lacks top-level comment object + } as any, + }); + + const result = await createInitialComment(mockOctokit, context); + expect(result.id).toBe(103); + expect(createSpy).toHaveBeenCalled(); + }); + }); + + describe("error handling", () => { + test("falls back to issues.createComment when primary path throws", async () => { + let fallbackCalled = false; + const mockOctokit = createMockOctokit({ + listComments: async () => { + throw new Error("API error"); + }, + createComment: async () => { + fallbackCalled = true; + return { data: { id: 999 } }; + }, + }); + + const context = createMockContext({ + eventName: "pull_request", + isPR: true, + inputs: { useStickyComment: true }, + payload: { + action: "opened", + number: 1, + pull_request: { number: 1 }, + } as any, + }); + + const result = await createInitialComment(mockOctokit, context); + expect(result.id).toBe(999); + expect(fallbackCalled).toBe(true); + expect(consoleErrorSpy).toHaveBeenCalled(); + }); + + test("throws when both primary and fallback fail", async () => { + const mockOctokit = createMockOctokit({ + listComments: async () => { + throw new Error("Primary error"); + }, + createComment: async () => { + throw new Error("Fallback error"); + }, + }); + + const context = createMockContext({ + eventName: "pull_request", + isPR: true, + inputs: { useStickyComment: true }, + payload: { + action: "opened", + number: 1, + pull_request: { number: 1 }, + } as any, + }); + + await expect(createInitialComment(mockOctokit, context)).rejects.toThrow( + "Fallback error", + ); + }); + + test("writes comment ID from fallback to GITHUB_OUTPUT", async () => { + const mockOctokit = createMockOctokit({ + listComments: async () => { + throw new Error("API error"); + }, + createComment: async () => ({ data: { id: 888 } }), + }); + + const context = createMockContext({ + eventName: "pull_request", + isPR: true, + inputs: { useStickyComment: true }, + payload: { + action: "opened", + number: 1, + pull_request: { number: 1 }, + } as any, + }); + + await createInitialComment(mockOctokit, context); + + const output = readFileSync(tempOutputFile, "utf-8"); + expect(output).toContain("claude_comment_id=888"); + }); + }); +}); diff --git a/test/setup-branch-forgejo.test.ts b/test/setup-branch-forgejo.test.ts new file mode 100644 index 0000000..7e4e492 --- /dev/null +++ b/test/setup-branch-forgejo.test.ts @@ -0,0 +1,150 @@ +#!/usr/bin/env bun + +/** + * Tests the Forgejo API divergence in setupBranch(). + * + * Forgejo uses repos.getBranch() to get source branch SHA because Forgejo's + * /git/refs (plural) API differs from GitHub's /git/ref (singular), causing + * git.getRef() to 404 on Forgejo. + * + * Approach: Call setupBranch() with mocked Octokit. The platform-specific + * API call records that it was called, then throws a sentinel error to stop + * execution before git shell commands run (avoiding real repo side effects). + */ + +import { + describe, + test, + expect, + beforeEach, + afterEach, + spyOn, + type Mock, +} from "bun:test"; +import { setupBranch } from "../src/github/operations/branch"; +import { _resetPlatformCache } from "../src/platform/detector"; +import { createMockContext } from "./mockContext"; +import type { Octokits } from "../src/github/api/client"; +import type { FetchDataResult } from "../src/github/data/fetcher"; + +const mockGithubData: FetchDataResult = { + contextData: { + title: "Test issue", + labels: { nodes: [{ name: "bug" }] }, + }, + comments: [], + changedFiles: [], + changedFilesWithSHA: [], + reviewData: null, + imageUrlMap: new Map(), +} as any; + +function createBranchMockOctokits( + overrides: Record = {}, +): Octokits { + return { + rest: { + repos: { + get: + overrides.reposGet ?? + (async () => ({ data: { default_branch: "main" } })), + getBranch: + overrides.getBranch ?? + (async () => ({ data: { commit: { sha: "abc123" } } })), + }, + git: { + getRef: + overrides.getRef ?? + (async () => ({ data: { object: { sha: "abc123" } } })), + }, + }, + graphql: {} as any, + } as unknown as Octokits; +} + +describe("setupBranch — Forgejo API divergence", () => { + let savedPlatform: string | undefined; + let savedServerUrl: string | undefined; + let consoleLogSpy: Mock<(...args: any[]) => void>; + let consoleErrorSpy: Mock<(...args: any[]) => void>; + + beforeEach(() => { + _resetPlatformCache(); + savedPlatform = process.env.PLATFORM; + savedServerUrl = process.env.GITHUB_SERVER_URL; + + consoleLogSpy = spyOn(console, "log").mockImplementation(() => {}); + consoleErrorSpy = spyOn(console, "error").mockImplementation(() => {}); + }); + + afterEach(() => { + if (savedPlatform !== undefined) process.env.PLATFORM = savedPlatform; + else delete process.env.PLATFORM; + if (savedServerUrl !== undefined) + process.env.GITHUB_SERVER_URL = savedServerUrl; + else delete process.env.GITHUB_SERVER_URL; + + consoleLogSpy.mockRestore(); + consoleErrorSpy.mockRestore(); + _resetPlatformCache(); + }); + + test("on Forgejo: calls repos.getBranch() instead of git.getRef()", async () => { + process.env.PLATFORM = "forgejo"; + process.env.GITHUB_SERVER_URL = "https://git.example.com"; + _resetPlatformCache(); + + let getBranchCalled = false; + let getRefCalled = false; + + const mockOctokits = createBranchMockOctokits({ + getBranch: async () => { + getBranchCalled = true; + // Throw sentinel error to stop before git commands execute + throw new Error("STOP_BEFORE_GIT_COMMANDS"); + }, + getRef: async () => { + getRefCalled = true; + throw new Error("STOP_BEFORE_GIT_COMMANDS"); + }, + }); + + const context = createMockContext({ isPR: false, entityNumber: 1 }); + + await expect( + setupBranch(mockOctokits, mockGithubData, context), + ).rejects.toThrow("STOP_BEFORE_GIT_COMMANDS"); + + expect(getBranchCalled).toBe(true); + expect(getRefCalled).toBe(false); + }); + + test("on GitHub: calls git.getRef() instead of repos.getBranch()", async () => { + delete process.env.PLATFORM; + process.env.GITHUB_SERVER_URL = "https://github.com"; + _resetPlatformCache(); + + let getBranchCalled = false; + let getRefCalled = false; + + const mockOctokits = createBranchMockOctokits({ + getBranch: async () => { + getBranchCalled = true; + throw new Error("STOP_BEFORE_GIT_COMMANDS"); + }, + getRef: async () => { + getRefCalled = true; + throw new Error("STOP_BEFORE_GIT_COMMANDS"); + }, + }); + + const context = createMockContext({ isPR: false, entityNumber: 1 }); + + await expect( + setupBranch(mockOctokits, mockGithubData, context), + ).rejects.toThrow("STOP_BEFORE_GIT_COMMANDS"); + + expect(getRefCalled).toBe(true); + expect(getBranchCalled).toBe(false); + }); +}); diff --git a/test/update-tracking-comment.test.ts b/test/update-tracking-comment.test.ts new file mode 100644 index 0000000..5555bb3 --- /dev/null +++ b/test/update-tracking-comment.test.ts @@ -0,0 +1,277 @@ +#!/usr/bin/env bun + +import { + describe, + test, + expect, + beforeEach, + afterEach, + spyOn, + type Mock, +} from "bun:test"; +import { updateTrackingComment } from "../src/github/operations/comments/update-with-branch"; +import { _resetPlatformCache } from "../src/platform/detector"; +import { createMockContext } from "./mockContext"; +import type { Octokits } from "../src/github/api/client"; + +function createMockOctokits(overrides: Record = {}): Octokits { + // updateTrackingComment calls updateClaudeComment(octokit.rest, ...), + // which accesses octokit.rest.issues.updateComment / octokit.rest.pulls.updateReviewComment + return { + rest: { + rest: { + issues: { + updateComment: + overrides.updateComment ?? + (async () => ({ data: { id: 1, html_url: "", updated_at: "" } })), + }, + pulls: { + updateReviewComment: + overrides.updateReviewComment ?? + (async () => ({ data: { id: 1, html_url: "", updated_at: "" } })), + }, + }, + }, + graphql: {} as any, + } as unknown as Octokits; +} + +describe("updateTrackingComment", () => { + let savedPlatform: string | undefined; + let savedServerUrl: string | undefined; + let savedRunNumber: string | undefined; + let savedRunId: string | undefined; + let consoleLogSpy: Mock<(...args: any[]) => void>; + let consoleErrorSpy: Mock<(...args: any[]) => void>; + + beforeEach(() => { + _resetPlatformCache(); + savedPlatform = process.env.PLATFORM; + savedServerUrl = process.env.GITHUB_SERVER_URL; + savedRunNumber = process.env.GITHUB_RUN_NUMBER; + savedRunId = process.env.GITHUB_RUN_ID; + + // Default to GitHub platform + delete process.env.PLATFORM; + process.env.GITHUB_SERVER_URL = "https://github.com"; + + consoleLogSpy = spyOn(console, "log").mockImplementation(() => {}); + consoleErrorSpy = spyOn(console, "error").mockImplementation(() => {}); + }); + + afterEach(() => { + if (savedPlatform !== undefined) process.env.PLATFORM = savedPlatform; + else delete process.env.PLATFORM; + if (savedServerUrl !== undefined) + process.env.GITHUB_SERVER_URL = savedServerUrl; + else delete process.env.GITHUB_SERVER_URL; + if (savedRunNumber !== undefined) + process.env.GITHUB_RUN_NUMBER = savedRunNumber; + else delete process.env.GITHUB_RUN_NUMBER; + if (savedRunId !== undefined) process.env.GITHUB_RUN_ID = savedRunId; + else delete process.env.GITHUB_RUN_ID; + + consoleLogSpy.mockRestore(); + consoleErrorSpy.mockRestore(); + _resetPlatformCache(); + }); + + describe("GitHub platform — body construction", () => { + test("includes GitHub job run link using context.runId", async () => { + let capturedBody = ""; + const mockOctokits = createMockOctokits({ + updateComment: async (args: any) => { + capturedBody = args.body; + return { data: { id: 1, html_url: "", updated_at: "" } }; + }, + }); + + const context = createMockContext({ runId: "55555" }); + await updateTrackingComment(mockOctokits, context, 1); + + expect(capturedBody).toContain("actions/runs/55555"); + expect(capturedBody).toContain("https://github.com/test-owner/test-repo"); + }); + + test("includes branch link for issue context", async () => { + let capturedBody = ""; + const mockOctokits = createMockOctokits({ + updateComment: async (args: any) => { + capturedBody = args.body; + return { data: { id: 1, html_url: "", updated_at: "" } }; + }, + }); + + const context = createMockContext({ isPR: false }); + await updateTrackingComment(mockOctokits, context, 1, "claude/fix-123"); + + expect(capturedBody).toContain("View branch"); + expect(capturedBody).toContain("tree/claude/fix-123"); + }); + + test("omits branch link for PR context even when branch provided", async () => { + let capturedBody = ""; + const mockOctokits = createMockOctokits({ + updateComment: async (args: any) => { + capturedBody = args.body; + return { data: { id: 1, html_url: "", updated_at: "" } }; + }, + }); + + const context = createMockContext({ isPR: true }); + await updateTrackingComment(mockOctokits, context, 1, "claude/fix-123"); + + expect(capturedBody).not.toContain("View branch"); + expect(capturedBody).not.toContain("tree/"); + }); + + test("omits branch link when branch is undefined", async () => { + let capturedBody = ""; + const mockOctokits = createMockOctokits({ + updateComment: async (args: any) => { + capturedBody = args.body; + return { data: { id: 1, html_url: "", updated_at: "" } }; + }, + }); + + const context = createMockContext({ isPR: false }); + await updateTrackingComment(mockOctokits, context, 1); + + expect(capturedBody).not.toContain("View branch"); + }); + }); + + describe("Forgejo platform — body construction", () => { + beforeEach(() => { + _resetPlatformCache(); + process.env.PLATFORM = "forgejo"; + process.env.GITHUB_SERVER_URL = "https://git.example.com"; + process.env.GITHUB_RUN_NUMBER = "77"; + }); + + test("uses Forgejo job run link with GITHUB_RUN_NUMBER", async () => { + let capturedBody = ""; + const mockOctokits = createMockOctokits({ + updateComment: async (args: any) => { + capturedBody = args.body; + return { data: { id: 1, html_url: "", updated_at: "" } }; + }, + }); + + const context = createMockContext(); + await updateTrackingComment(mockOctokits, context, 1); + + expect(capturedBody).toContain( + "https://git.example.com/test-owner/test-repo/actions/runs/77", + ); + }); + + test("uses Forgejo branch link for issues", async () => { + let capturedBody = ""; + const mockOctokits = createMockOctokits({ + updateComment: async (args: any) => { + capturedBody = args.body; + return { data: { id: 1, html_url: "", updated_at: "" } }; + }, + }); + + const context = createMockContext({ isPR: false }); + await updateTrackingComment(mockOctokits, context, 1, "claude/fix-42"); + + expect(capturedBody).toContain( + "https://git.example.com/test-owner/test-repo/tree/claude/fix-42", + ); + }); + + test("uses Forgejo comment body with Unicode spinner", async () => { + let capturedBody = ""; + const mockOctokits = createMockOctokits({ + updateComment: async (args: any) => { + capturedBody = args.body; + return { data: { id: 1, html_url: "", updated_at: "" } }; + }, + }); + + const context = createMockContext(); + await updateTrackingComment(mockOctokits, context, 1); + + expect(capturedBody).toContain("⏳"); + expect(capturedBody).not.toContain("user-attachments/assets"); + }); + }); + + describe("comment type routing", () => { + test("uses issue comment update for non-review events", async () => { + let issueUpdateCalled = false; + let reviewUpdateCalled = false; + const mockOctokits = createMockOctokits({ + updateComment: async () => { + issueUpdateCalled = true; + return { data: { id: 1, html_url: "", updated_at: "" } }; + }, + updateReviewComment: async () => { + reviewUpdateCalled = true; + return { data: { id: 1, html_url: "", updated_at: "" } }; + }, + }); + + const context = createMockContext({ eventName: "issue_comment" }); + await updateTrackingComment(mockOctokits, context, 1); + + expect(issueUpdateCalled).toBe(true); + expect(reviewUpdateCalled).toBe(false); + }); + + test("uses PR review comment update for review comment events", async () => { + let issueUpdateCalled = false; + let reviewUpdateCalled = false; + const mockOctokits = createMockOctokits({ + updateComment: async () => { + issueUpdateCalled = true; + return { data: { id: 1, html_url: "", updated_at: "" } }; + }, + updateReviewComment: async () => { + reviewUpdateCalled = true; + return { data: { id: 1, html_url: "", updated_at: "" } }; + }, + }); + + const context = createMockContext({ + eventName: "pull_request_review_comment", + isPR: true, + }); + await updateTrackingComment(mockOctokits, context, 1); + + expect(reviewUpdateCalled).toBe(true); + expect(issueUpdateCalled).toBe(false); + }); + }); + + describe("error handling", () => { + test("throws and logs error when update fails", async () => { + const mockOctokits = createMockOctokits({ + updateComment: async () => { + throw new Error("Update failed"); + }, + }); + + const context = createMockContext(); + await expect( + updateTrackingComment(mockOctokits, context, 1), + ).rejects.toThrow("Update failed"); + expect(consoleErrorSpy).toHaveBeenCalled(); + }); + + test("logs correct comment type label on success", async () => { + const mockOctokits = createMockOctokits(); + + const issueContext = createMockContext({ eventName: "issue_comment" }); + await updateTrackingComment(mockOctokits, issueContext, 42); + + expect(consoleLogSpy).toHaveBeenCalledWith( + expect.stringContaining("issue"), + ); + expect(consoleLogSpy).toHaveBeenCalledWith(expect.stringContaining("42")); + }); + }); +});