Skip to content

Commit 7a66814

Browse files
levibostianlevibostian
authored
feat: gracefully handle when a merge conflict happens with a simulated merge (#154)
## Related GitHub Issues <!-- Link to any related GitHub issues that this pull request addresses or closes. --> ## Problem <!-- A clear description of the problem that this pull request is solving. --> When we perform a simulated merge, we use many different merge types, and it can be common for there to be a merge conflict when trying to rebase, for example. This causes decaf to crash on the CI, which feels not right. A crash shouldn't occur for something that is possible to occur. ## Solution <!-- Describe the approach you took to solve the problem and the changes made in this pull request. --> In the GitHub UI, you will also see that merge conflict, and they won't even allow you to click the merge button because of the merge conflict. We should gracefully handle it just the same, knowing the user can't actually merge the pull request anyway. So, instead of crashing the app, we should gracefully handle it and exit with a zero code, knowing the user can't merge anyway using rebase. They could use a different method instead. ## Testing <!-- Choose one of the below options for how you tested the code change. Include any specific setup or instructions for testing. --> - [X] Added automated tests. - [ ] Manually tested. If you check this box, provide instructions for others to test, too. ## Notes for reviewers <!-- If there is any additional information you would like to share with the person reviewing this pull request, please provide it here. -->
1 parent 55241c9 commit 7a66814

8 files changed

Lines changed: 267 additions & 77 deletions

File tree

‎index.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { run } from "./deploy.ts"
22
import { GetCommitsSinceLatestReleaseStepImpl } from "./lib/steps/get-commits-since-latest-release.ts"
33
import { exec } from "./lib/exec.ts"
44
import { logger } from "./lib/log.ts"
5+
import { MergeConflictError } from "./lib/git.ts"
56
import { SimulateMergeImpl } from "./lib/simulate-merge.ts"
67
import { PrepareTestModeEnvStepImpl } from "./lib/steps/prepare-testmode-env.ts"
78
import { StepRunnerImpl } from "./lib/step-runner.ts"
@@ -139,8 +140,14 @@ export async function main() {
139140
})
140141
}
141142

142-
// rethrow the error to ensure the action fails
143-
throw error
143+
if (error instanceof MergeConflictError) {
144+
// Merge conflicts are expected — log a clear message and continue to the next merge type.
145+
// The process will exit with 0 after all types are processed; GitHub will block the PR from merging anyway.
146+
logger.warning(error.message)
147+
} else {
148+
// Unexpected error — rethrow to ensure the action fails visibly
149+
throw error
150+
}
144151
} finally {
145152
// Always clean up the isolated git clone, even if an error occurred
146153
// Only clean up if we created an isolated clone (test mode only)

‎lib/e2e/e2e-stubs.test.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { Exec } from "../exec.ts"
2-
import { Git, GitRepoManager } from "../git.ts"
2+
import { Git, GitRepoManager, MergeConflictError } from "../git.ts"
33
import { GitCommit } from "../types/git.ts"
44
import { Environment } from "../environment.ts"
55
import { AnyStepName } from "../steps/types/any-step.ts"
@@ -31,6 +31,8 @@ export class GitStub implements Git {
3131
remoteRepo: GitRemoteRepositoryMock
3232
/** Whether the repository is in a shallow state */
3333
isShallow: boolean
34+
/** When set, merge() and rebase() will throw this error to simulate a merge conflict */
35+
simulateMergeConflictError: MergeConflictError | undefined
3436

3537
constructor(
3638
{ currentBranch, remoteRepo, commits }: { currentBranch: string; remoteRepo: GitRemoteRepositoryMock; commits: Map<string, GitCommit[]> },
@@ -43,6 +45,7 @@ export class GitStub implements Git {
4345
this.commitsFetched = new Map()
4446
this.isShallow = true // Start as shallow repository
4547
this.remoteRepo = remoteRepo
48+
this.simulateMergeConflictError = undefined
4649

4750
// Initialize commitsFetched to only contain the local commits initially
4851
// After fetch() is called, this will be populated with complete remote history
@@ -125,6 +128,10 @@ export class GitStub implements Git {
125128
commitMessage: string
126129
fastForward?: "--no-ff" | "--ff-only"
127130
}): Promise<void> => {
131+
if (this.simulateMergeConflictError) {
132+
throw this.simulateMergeConflictError
133+
}
134+
128135
const currentBranchCommits = this.localBranchCommits.get(this.currentBranch) || []
129136
const branchToMergeCommits = this.localBranchCommits.get(args.branchToMergeIn) || []
130137

@@ -246,6 +253,10 @@ export class GitStub implements Git {
246253
}
247254

248255
rebase = async (args: { branchToRebaseOnto: string }): Promise<void> => {
256+
if (this.simulateMergeConflictError) {
257+
throw this.simulateMergeConflictError
258+
}
259+
249260
const currentBranchCommits = this.localBranchCommits.get(this.currentBranch) || []
250261
const targetBranchCommits = this.localBranchCommits.get(args.branchToRebaseOnto) || []
251262

‎lib/e2e/e2e.test.ts‎

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { GitHubApi, GitHubPullRequest } from "../github-api.ts"
1515
import { assertObjectMatch } from "@std/assert"
1616
import { assertSnapshot } from "@std/testing/snapshot"
1717
import * as di from "../di.ts"
18+
import { MergeConflictError } from "../git.ts"
1819

1920
type PrCommentCall = {
2021
message: string
@@ -698,6 +699,55 @@ Deno.test("NON-TEST MODE: when deployment verification fails with failOnDeployVe
698699
assertEquals(didThrow, true, "Should throw error when verification fails in non-test mode with failOnDeployVerification=true")
699700
})
700701

702+
Deno.test("when a simulated merge fails with a conflict, expect no crash, error result in PR comment, and remaining merge types still run", async () => {
703+
const mainBranchCommits = [new GitCommitFake({ message: "commit on main", sha: "main-1" })]
704+
const featureCommits = [new GitCommitFake({ message: "commit on feature", sha: "feature-1" })]
705+
706+
const givenLocalCommits = new Map<string, GitCommit[]>([
707+
["main", mainBranchCommits],
708+
["feature", featureCommits],
709+
])
710+
711+
const givenRemoteCommits = new Map<string, GitCommit[]>([
712+
["main", mainBranchCommits],
713+
["feature", featureCommits],
714+
])
715+
716+
const { gitStub } = setupGitRepo({
717+
checkedOutBranch: "main",
718+
localCommits: givenLocalCommits,
719+
remoteCommits: givenRemoteCommits,
720+
remotePullRequests: [
721+
{
722+
prNumber: 123,
723+
targetBranchName: "main",
724+
sourceBranchName: "feature",
725+
title: "PR for feature",
726+
description: "Description for feature",
727+
},
728+
],
729+
})
730+
731+
// Make the git stub throw a conflict error on merge/rebase
732+
gitStub.simulateMergeConflictError = new MergeConflictError("git rebase onto 'main' failed due to merge conflicts.")
733+
734+
e2eStepScript.setGetLatestReleaseStepOutput(null)
735+
736+
// Should NOT throw — merge conflict is handled gracefully
737+
let didThrow = false
738+
try {
739+
await run({
740+
testMode: true,
741+
simulatedMergeTypes: ["rebase", "merge"],
742+
makePullRequestComment: true,
743+
})
744+
} catch (_error) {
745+
didThrow = true
746+
}
747+
748+
assertEquals(didThrow, false, "Should NOT throw when a simulated merge fails with a conflict")
749+
})
750+
701751
// helper functions
702752

703753
let diGraph: typeof di.productionDiGraph
@@ -724,7 +774,7 @@ const setupGitRepo = (
724774
remoteCommits: Map<string, GitCommit[]>
725775
remotePullRequests: GitHubPullRequest[]
726776
},
727-
): { remoteRepository: GitRemoteRepositoryMock; gitRepo: GitRepoManagerStub; prCommentCalls: PrCommentCall[] } => {
777+
): { remoteRepository: GitRemoteRepositoryMock; gitRepo: GitRepoManagerStub; gitStub: GitStub; prCommentCalls: PrCommentCall[] } => {
728778
currentBranchWhenTestStarts = checkedOutBranch
729779

730780
const remoteRepository: GitRemoteRepositoryMock = {
@@ -747,7 +797,7 @@ const setupGitRepo = (
747797
})
748798
diGraph = diGraph.override("github", () => githubApiMock)
749799

750-
return { remoteRepository, gitRepo, prCommentCalls }
800+
return { remoteRepository, gitRepo, gitStub, prCommentCalls }
751801
}
752802

753803
let currentBranchWhenTestStarts: string

‎lib/exec.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { AnyStepInput } from "./types/environment.ts"
44
export interface RunResult {
55
exitCode: number
66
stdout: string
7+
stderr: string
78
output: Record<string, unknown> | undefined
89
}
910

@@ -159,6 +160,7 @@ const run = async (
159160
return {
160161
exitCode: code,
161162
stdout: capturedStdout,
163+
stderr: capturedStderr,
162164
output: commandOutput,
163165
}
164166
}

0 commit comments

Comments
 (0)