fix(prd-7): M4 test coverage review — 28 new tests for comment operations and branch setup
- 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 <noreply@anthropic.com>
This commit is contained in:
Vlad MocanuandClaude Opus 4.6 committed
1 parent
6b0302d226
commit
bbfd630fc5
5 files changed
+1006
-1
No files matched your search
@@ -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
|
||||
|
||||
@@ -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).
|
||||
@@ -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<string, any> = {}) {
|
||||
// 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/<runId>
|
||||
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");
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -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<string, any> = {},
|
||||
): 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);
|
||||
});
|
||||
});
|
||||
@@ -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<string, any> = {}): 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"));
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user