fix: rewrite branch.test.ts to match actual implementation

- Remove tests for non-existent cleanupBranch function
- Fix types: use Octokits instead of Octokit, ParsedGitHubContext instead of EntityContext
- Fix assertions: test actual BranchInfo return type (baseBranch, claudeBranch, currentBranch)
- Add comprehensive test coverage for:
  - Issue branch creation (default/custom base branch, commit signing)
  - PR branch handling (open/closed/merged states)
  - Error handling (branch not found, repository fetch failures)
  - Branch naming (kubernetes-compatible, custom prefixes)
- Use proper Bun test mocking patterns instead of Jest
- Match function signatures and behavior from actual setupBranch implementation

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-authored-by: kashyap murali <km-anthropic@users.noreply.github.com>
This commit is contained in:
claude[bot]
2025-09-10 04:42:56 +00:00
parent 266d8536dc
commit 6325e51611

View File

@@ -1,114 +1,178 @@
import { describe, test, expect, jest, beforeEach } from "@jest/globals"; import { describe, test, expect, beforeEach, afterEach } from "bun:test";
import { setupBranch, cleanupBranch } from "../branch"; import { mock } from "bun:test";
import type { Octokit } from "../../../utils/octokit"; import { setupBranch, type BranchInfo } from "../branch";
import type { Octokits } from "../../api/client";
import type { FetchDataResult } from "../../data/fetcher"; import type { FetchDataResult } from "../../data/fetcher";
import type { EntityContext } from "../../context"; import type { ParsedGitHubContext } from "../../context";
import type { GitHubPullRequest, GitHubIssue } from "../../types";
describe("Branch Operations", () => { // Mock process.exit to prevent tests from actually exiting
let mockOctokit: jest.Mocked<Octokit>; const mockExit = mock(() => {});
let mockContext: EntityContext; const originalExit = process.exit;
describe("setupBranch", () => {
let mockOctokits: Octokits;
let mockContext: ParsedGitHubContext;
let mockGithubData: FetchDataResult; let mockGithubData: FetchDataResult;
beforeEach(() => { beforeEach(() => {
// ISSUE 1: Not properly mocking all required methods // Replace process.exit temporarily
mockOctokit = { (process as any).exit = mockExit;
mockExit.mockClear();
// Simple mock objects
mockOctokits = {
rest: { rest: {
repos: { repos: {
getBranch: jest.fn(), get: mock(() => Promise.resolve({ data: { default_branch: "main" } })),
createRef: jest.fn(),
}, },
git: { git: {
getRef: jest.fn(), getRef: mock(() => Promise.resolve({
createRef: jest.fn(), data: { object: { sha: "abc123def456" } }
})),
}, },
}, },
graphql: mock(() => Promise.resolve({})),
} as any; } as any;
mockContext = { mockContext = {
repository: { repository: {
owner: "test-owner", owner: "test-owner",
repo: "test-repo", repo: "test-repo",
full_name: "test-owner/test-repo",
}, },
isPR: true, isPR: false,
entityNumber: 123, entityNumber: 123,
} as EntityContext; inputs: {
branchPrefix: "claude/",
useCommitSigning: false,
},
} as ParsedGitHubContext;
// Default mock data for issues
mockGithubData = { mockGithubData = {
prData: { contextData: {
title: "Test Issue",
body: "Test issue body",
state: "OPEN",
} as GitHubIssue,
comments: [],
changedFiles: [],
changedFilesWithSHA: [],
reviewData: null,
};
});
afterEach(() => {
// Restore original process.exit
process.exit = originalExit;
});
describe("Issue branch creation", () => {
test("should create new branch for issue using default branch as source", async () => {
const result = await setupBranch(mockOctokits, mockGithubData, mockContext);
expect(result.baseBranch).toBe("main");
expect(result.claudeBranch).toMatch(/^claude\/issue-123-\d{8}-\d{4}$/);
expect(result.currentBranch).toMatch(/^claude\/issue-123-\d{8}-\d{4}$/);
});
test("should use provided base branch as source", async () => {
mockContext.inputs.baseBranch = "develop";
const result = await setupBranch(mockOctokits, mockGithubData, mockContext);
expect(result.baseBranch).toBe("develop");
expect(result.claudeBranch).toMatch(/^claude\/issue-123-\d{8}-\d{4}$/);
});
test("should handle commit signing mode", async () => {
mockContext.inputs.useCommitSigning = true;
const result = await setupBranch(mockOctokits, mockGithubData, mockContext);
expect(result.baseBranch).toBe("main");
expect(result.currentBranch).toBe("main"); // Should stay on source branch
expect(result.claudeBranch).toMatch(/^claude\/issue-123-\d{8}-\d{4}$/);
});
});
describe("PR branch handling", () => {
beforeEach(() => {
mockContext.isPR = true;
mockGithubData.contextData = {
title: "Test PR", title: "Test PR",
baseBranch: "main", body: "Test PR body",
headBranch: "feature/test", state: "OPEN",
// ISSUE 2: Missing required fields from the actual type baseRefName: "main",
}, headRefName: "feature/test",
} as FetchDataResult; commits: { totalCount: 5 },
} as GitHubPullRequest;
}); });
describe("setupBranch", () => { test("should checkout existing PR branch for open PR", async () => {
test("should create a new branch for PR", async () => { const result = await setupBranch(mockOctokits, mockGithubData, mockContext);
// ISSUE 3: Wrong mock response structure
mockOctokit.rest.git.getRef.mockResolvedValue({ expect(result.baseBranch).toBe("main");
data: { expect(result.currentBranch).toBe("feature/test");
object: { expect(result.claudeBranch).toBeUndefined(); // No claude branch for open PRs
sha: "abc123",
},
},
}); });
mockOctokit.rest.git.createRef.mockResolvedValue({ test("should create new branch for closed PR", async () => {
data: { const closedPR = mockGithubData.contextData as GitHubPullRequest;
ref: "refs/heads/claude/pr-123", closedPR.state = "CLOSED";
},
const result = await setupBranch(mockOctokits, mockGithubData, mockContext);
expect(result.baseBranch).toBe("main");
expect(result.claudeBranch).toMatch(/^claude\/pr-123-\d{8}-\d{4}$/);
expect(result.currentBranch).toMatch(/^claude\/pr-123-\d{8}-\d{4}$/);
}); });
const result = await setupBranch(mockOctokit, mockGithubData, mockContext); test("should create new branch for merged PR", async () => {
const mergedPR = mockGithubData.contextData as GitHubPullRequest;
mergedPR.state = "MERGED";
// ISSUE 4: Testing wrong property names const result = await setupBranch(mockOctokits, mockGithubData, mockContext);
expect(result.branch).toBe("claude/pr-123");
expect(result.base).toBe("main");
// ISSUE 5: Not checking if the function was called with correct parameters expect(result.baseBranch).toBe("main");
expect(mockOctokit.rest.git.createRef).toHaveBeenCalled(); expect(result.claudeBranch).toMatch(/^claude\/pr-123-\d{8}-\d{4}$/);
});
test("should handle existing branch", async () => {
// ISSUE 6: Testing error case incorrectly
mockOctokit.rest.git.getRef.mockRejectedValue(new Error("Not found"));
// ISSUE 7: Not catching the error properly
const result = await setupBranch(mockOctokit, mockGithubData, mockContext);
expect(result).toBeNull();
});
// ISSUE 8: Missing important test case for issue context
test("should work for issues", async () => {
mockContext.isPR = false;
// This test doesn't actually check issue-specific logic
const result = await setupBranch(mockOctokit, mockGithubData, mockContext);
expect(result).toBeDefined();
}); });
}); });
describe("cleanupBranch", () => { describe("Error handling", () => {
test("should delete branch successfully", async () => { test("should exit with code 1 when source branch doesn't exist", async () => {
// ISSUE 9: Synchronous expectation for async operation mockOctokits.rest.git.getRef = mock(() => Promise.reject(new Error("Branch not found")));
mockOctokit.rest.git.deleteRef = jest.fn();
cleanupBranch(mockOctokit, "claude/pr-123", mockContext); await setupBranch(mockOctokits, mockGithubData, mockContext);
// ISSUE 10: Not awaiting the async function expect(mockExit).toHaveBeenCalledWith(1);
expect(mockOctokit.rest.git.deleteRef).toHaveBeenCalledWith({ });
owner: "test-owner",
repo: "test-repo", test("should exit with code 1 when repository fetch fails", async () => {
ref: "claude/pr-123", // ISSUE 11: Wrong ref format (should include 'heads/') mockOctokits.rest.repos.get = mock(() => Promise.reject(new Error("Repository not found")));
await setupBranch(mockOctokits, mockGithubData, mockContext);
expect(mockExit).toHaveBeenCalledWith(1);
}); });
}); });
// ISSUE 12: Missing error handling test describe("Branch naming", () => {
test("should generate kubernetes-compatible branch names", async () => {
const result = await setupBranch(mockOctokits, mockGithubData, mockContext);
// Branch name should be lowercase, use hyphens, and include timestamp
expect(result.claudeBranch).toMatch(/^claude\/issue-123-\d{8}-\d{4}$/);
expect(result.claudeBranch?.length).toBeLessThanOrEqual(50);
}); });
// ISSUE 13: Missing edge cases like branch name sanitization test("should use custom branch prefix", async () => {
mockContext.inputs.branchPrefix = "ai/";
// ISSUE 14: No integration test to verify the full flow const result = await setupBranch(mockOctokits, mockGithubData, mockContext);
expect(result.claudeBranch).toMatch(/^ai\/issue-123-\d{8}-\d{4}$/);
});
});
}); });