fix edge case where changes are partially merged
This commit is contained in:
parent
ce190a9972
commit
a0f85c82ff
3 changed files with 189 additions and 9 deletions
|
@ -657,6 +657,76 @@ describe('create-or-update-branch tests', () => {
|
||||||
).toBeTruthy()
|
).toBeTruthy()
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('tests create, commit with partial changes on the base, and update', async () => {
|
||||||
|
// This is an edge case where the changes for a single commit are partially merged to the base
|
||||||
|
|
||||||
|
// Create tracked and untracked file changes
|
||||||
|
const changes = await createChanges()
|
||||||
|
const commitMessage = uuidv4()
|
||||||
|
const result = await createOrUpdateBranch(
|
||||||
|
git,
|
||||||
|
commitMessage,
|
||||||
|
'',
|
||||||
|
BRANCH,
|
||||||
|
REMOTE_NAME,
|
||||||
|
false,
|
||||||
|
ADD_PATHS_DEFAULT
|
||||||
|
)
|
||||||
|
await git.checkout(BRANCH)
|
||||||
|
expect(result.action).toEqual('created')
|
||||||
|
expect(await getFileContent(TRACKED_FILE)).toEqual(changes.tracked)
|
||||||
|
expect(await getFileContent(UNTRACKED_FILE)).toEqual(changes.untracked)
|
||||||
|
expect(
|
||||||
|
await gitLogMatches([commitMessage, INIT_COMMIT_MESSAGE])
|
||||||
|
).toBeTruthy()
|
||||||
|
|
||||||
|
// Push pull request branch to remote
|
||||||
|
await git.push([
|
||||||
|
'--force-with-lease',
|
||||||
|
REMOTE_NAME,
|
||||||
|
`HEAD:refs/heads/${BRANCH}`
|
||||||
|
])
|
||||||
|
|
||||||
|
await afterTest(false)
|
||||||
|
await beforeTest()
|
||||||
|
|
||||||
|
// Create a commit on the base with a partial merge of the changes
|
||||||
|
await createFile(TRACKED_FILE, changes.tracked)
|
||||||
|
const baseCommitMessage = uuidv4()
|
||||||
|
await git.exec(['add', '-A'])
|
||||||
|
await git.commit(['-m', baseCommitMessage])
|
||||||
|
await git.push([
|
||||||
|
'--force',
|
||||||
|
REMOTE_NAME,
|
||||||
|
`HEAD:refs/heads/${DEFAULT_BRANCH}`
|
||||||
|
])
|
||||||
|
|
||||||
|
// Create the same tracked and untracked file changes
|
||||||
|
const _changes = await createChanges(changes.tracked, changes.untracked)
|
||||||
|
const _commitMessage = uuidv4()
|
||||||
|
const _result = await createOrUpdateBranch(
|
||||||
|
git,
|
||||||
|
_commitMessage,
|
||||||
|
'',
|
||||||
|
BRANCH,
|
||||||
|
REMOTE_NAME,
|
||||||
|
false,
|
||||||
|
ADD_PATHS_DEFAULT
|
||||||
|
)
|
||||||
|
await git.checkout(BRANCH)
|
||||||
|
expect(_result.action).toEqual('updated')
|
||||||
|
expect(_result.hasDiffWithBase).toBeTruthy()
|
||||||
|
expect(await getFileContent(TRACKED_FILE)).toEqual(_changes.tracked)
|
||||||
|
expect(await getFileContent(UNTRACKED_FILE)).toEqual(_changes.untracked)
|
||||||
|
expect(
|
||||||
|
await gitLogMatches([
|
||||||
|
_commitMessage,
|
||||||
|
baseCommitMessage,
|
||||||
|
INIT_COMMIT_MESSAGE
|
||||||
|
])
|
||||||
|
).toBeTruthy()
|
||||||
|
})
|
||||||
|
|
||||||
it('tests create, squash merge, and update with identical changes', async () => {
|
it('tests create, squash merge, and update with identical changes', async () => {
|
||||||
// Branches that have been squash merged appear to have a diff with the base due to
|
// Branches that have been squash merged appear to have a diff with the base due to
|
||||||
// different commits for the same changes. To prevent creating pull requests
|
// different commits for the same changes. To prevent creating pull requests
|
||||||
|
@ -1679,6 +1749,81 @@ describe('create-or-update-branch tests', () => {
|
||||||
).toBeTruthy()
|
).toBeTruthy()
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('tests create, commit with partial changes on the base, and update (WBNB)', async () => {
|
||||||
|
// This is an edge case where the changes for a single commit are partially merged to the base
|
||||||
|
|
||||||
|
// Set the working base to a branch that is not the pull request base
|
||||||
|
await git.checkout(NOT_BASE_BRANCH)
|
||||||
|
|
||||||
|
// Create tracked and untracked file changes
|
||||||
|
const changes = await createChanges()
|
||||||
|
const commitMessage = uuidv4()
|
||||||
|
const result = await createOrUpdateBranch(
|
||||||
|
git,
|
||||||
|
commitMessage,
|
||||||
|
BASE,
|
||||||
|
BRANCH,
|
||||||
|
REMOTE_NAME,
|
||||||
|
false,
|
||||||
|
ADD_PATHS_DEFAULT
|
||||||
|
)
|
||||||
|
await git.checkout(BRANCH)
|
||||||
|
expect(result.action).toEqual('created')
|
||||||
|
expect(await getFileContent(TRACKED_FILE)).toEqual(changes.tracked)
|
||||||
|
expect(await getFileContent(UNTRACKED_FILE)).toEqual(changes.untracked)
|
||||||
|
expect(
|
||||||
|
await gitLogMatches([commitMessage, INIT_COMMIT_MESSAGE])
|
||||||
|
).toBeTruthy()
|
||||||
|
|
||||||
|
// Push pull request branch to remote
|
||||||
|
await git.push([
|
||||||
|
'--force-with-lease',
|
||||||
|
REMOTE_NAME,
|
||||||
|
`HEAD:refs/heads/${BRANCH}`
|
||||||
|
])
|
||||||
|
|
||||||
|
await afterTest(false)
|
||||||
|
await beforeTest()
|
||||||
|
|
||||||
|
// Create a commit on the base with a partial merge of the changes
|
||||||
|
await createFile(TRACKED_FILE, changes.tracked)
|
||||||
|
const baseCommitMessage = uuidv4()
|
||||||
|
await git.exec(['add', '-A'])
|
||||||
|
await git.commit(['-m', baseCommitMessage])
|
||||||
|
await git.push([
|
||||||
|
'--force',
|
||||||
|
REMOTE_NAME,
|
||||||
|
`HEAD:refs/heads/${DEFAULT_BRANCH}`
|
||||||
|
])
|
||||||
|
|
||||||
|
// Set the working base to a branch that is not the pull request base
|
||||||
|
await git.checkout(NOT_BASE_BRANCH)
|
||||||
|
|
||||||
|
// Create the same tracked and untracked file changes
|
||||||
|
const _changes = await createChanges(changes.tracked, changes.untracked)
|
||||||
|
const _commitMessage = uuidv4()
|
||||||
|
const _result = await createOrUpdateBranch(
|
||||||
|
git,
|
||||||
|
_commitMessage,
|
||||||
|
BASE,
|
||||||
|
BRANCH,
|
||||||
|
REMOTE_NAME,
|
||||||
|
false,
|
||||||
|
ADD_PATHS_DEFAULT
|
||||||
|
)
|
||||||
|
await git.checkout(BRANCH)
|
||||||
|
expect(_result.action).toEqual('updated')
|
||||||
|
expect(_result.hasDiffWithBase).toBeTruthy()
|
||||||
|
expect(await getFileContent(TRACKED_FILE)).toEqual(_changes.tracked)
|
||||||
|
expect(await getFileContent(UNTRACKED_FILE)).toEqual(_changes.untracked)
|
||||||
|
expect(
|
||||||
|
await gitLogMatches([
|
||||||
|
_commitMessage,
|
||||||
|
baseCommitMessage // fetch depth of base is 1
|
||||||
|
])
|
||||||
|
).toBeTruthy()
|
||||||
|
})
|
||||||
|
|
||||||
it('tests create, squash merge, and update with identical changes (WBNB)', async () => {
|
it('tests create, squash merge, and update with identical changes (WBNB)', async () => {
|
||||||
// Branches that have been squash merged appear to have a diff with the base due to
|
// Branches that have been squash merged appear to have a diff with the base due to
|
||||||
// different commits for the same changes. To prevent creating pull requests
|
// different commits for the same changes. To prevent creating pull requests
|
||||||
|
|
23
dist/index.js
vendored
23
dist/index.js
vendored
|
@ -133,6 +133,14 @@ function isEven(git, branch1, branch2) {
|
||||||
!(yield isBehind(git, branch1, branch2)));
|
!(yield isBehind(git, branch1, branch2)));
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
// Return true if the specified number of commits on branch1 and branch2 have a diff
|
||||||
|
function commitsHaveDiff(git, branch1, branch2, depth) {
|
||||||
|
return __awaiter(this, void 0, void 0, function* () {
|
||||||
|
const diff1 = (yield git.exec(['diff', '--stat', `${branch1}..${branch1}~${depth}`])).stdout.trim();
|
||||||
|
const diff2 = (yield git.exec(['diff', '--stat', `${branch2}..${branch2}~${depth}`])).stdout.trim();
|
||||||
|
return diff1 !== diff2;
|
||||||
|
});
|
||||||
|
}
|
||||||
function splitLines(multilineString) {
|
function splitLines(multilineString) {
|
||||||
return multilineString
|
return multilineString
|
||||||
.split('\n')
|
.split('\n')
|
||||||
|
@ -241,20 +249,25 @@ function createOrUpdateBranch(git, commitMessage, base, branch, branchRemoteName
|
||||||
yield git.checkout(branch);
|
yield git.checkout(branch);
|
||||||
// Reset the branch if one of the following conditions is true.
|
// Reset the branch if one of the following conditions is true.
|
||||||
// - If the branch differs from the recreated temp branch.
|
// - If the branch differs from the recreated temp branch.
|
||||||
|
// - If the number of commits ahead of the base branch differs between the branch and
|
||||||
|
// temp branch. This catches a case where the base branch has been force pushed to
|
||||||
|
// a new commit.
|
||||||
// - If the recreated temp branch is not ahead of the base. This means there will be
|
// - If the recreated temp branch is not ahead of the base. This means there will be
|
||||||
// no pull request diff after the branch is reset. This will reset any undeleted
|
// no pull request diff after the branch is reset. This will reset any undeleted
|
||||||
// branches after merging. In particular, it catches a case where the branch was
|
// branches after merging. In particular, it catches a case where the branch was
|
||||||
// squash merged but not deleted. We need to reset to make sure it doesn't appear
|
// squash merged but not deleted. We need to reset to make sure it doesn't appear
|
||||||
// to have a diff with the base due to different commits for the same changes.
|
// to have a diff with the base due to different commits for the same changes.
|
||||||
// - If the number of commits ahead of the base branch differs between the branch and
|
// - If the diff of the commits ahead of the base branch differs between the branch and
|
||||||
// temp branch. This catches a case where the base branch has been force pushed to
|
// temp branch. This catches a case where changes have been partially merged to the
|
||||||
// a new commit.
|
// base. The overall diff is the same, but the branch needs to be rebased to show
|
||||||
|
// the correct diff.
|
||||||
|
//
|
||||||
// For changes on base this reset is equivalent to a rebase of the pull request branch.
|
// For changes on base this reset is equivalent to a rebase of the pull request branch.
|
||||||
const branchCommitsAhead = yield commitsAhead(git, base, branch);
|
const branchCommitsAhead = yield commitsAhead(git, base, branch);
|
||||||
if ((yield git.hasDiff([`${branch}..${tempBranch}`])) ||
|
if ((yield git.hasDiff([`${branch}..${tempBranch}`])) ||
|
||||||
branchCommitsAhead != tempBranchCommitsAhead ||
|
branchCommitsAhead != tempBranchCommitsAhead ||
|
||||||
!(tempBranchCommitsAhead > 0) // !isAhead
|
!(tempBranchCommitsAhead > 0) || // !isAhead
|
||||||
) {
|
(yield commitsHaveDiff(git, branch, tempBranch, tempBranchCommitsAhead))) {
|
||||||
core.info(`Resetting '${branch}'`);
|
core.info(`Resetting '${branch}'`);
|
||||||
// Alternatively, git switch -C branch tempBranch
|
// Alternatively, git switch -C branch tempBranch
|
||||||
yield git.checkout(branch, tempBranch);
|
yield git.checkout(branch, tempBranch);
|
||||||
|
|
|
@ -124,6 +124,22 @@ async function isEven(
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Return true if the specified number of commits on branch1 and branch2 have a diff
|
||||||
|
async function commitsHaveDiff(
|
||||||
|
git: GitCommandManager,
|
||||||
|
branch1: string,
|
||||||
|
branch2: string,
|
||||||
|
depth: number
|
||||||
|
): Promise<boolean> {
|
||||||
|
const diff1 = (
|
||||||
|
await git.exec(['diff', '--stat', `${branch1}..${branch1}~${depth}`])
|
||||||
|
).stdout.trim()
|
||||||
|
const diff2 = (
|
||||||
|
await git.exec(['diff', '--stat', `${branch2}..${branch2}~${depth}`])
|
||||||
|
).stdout.trim()
|
||||||
|
return diff1 !== diff2
|
||||||
|
}
|
||||||
|
|
||||||
function splitLines(multilineString: string): string[] {
|
function splitLines(multilineString: string): string[] {
|
||||||
return multilineString
|
return multilineString
|
||||||
.split('\n')
|
.split('\n')
|
||||||
|
@ -270,20 +286,26 @@ export async function createOrUpdateBranch(
|
||||||
|
|
||||||
// Reset the branch if one of the following conditions is true.
|
// Reset the branch if one of the following conditions is true.
|
||||||
// - If the branch differs from the recreated temp branch.
|
// - If the branch differs from the recreated temp branch.
|
||||||
|
// - If the number of commits ahead of the base branch differs between the branch and
|
||||||
|
// temp branch. This catches a case where the base branch has been force pushed to
|
||||||
|
// a new commit.
|
||||||
// - If the recreated temp branch is not ahead of the base. This means there will be
|
// - If the recreated temp branch is not ahead of the base. This means there will be
|
||||||
// no pull request diff after the branch is reset. This will reset any undeleted
|
// no pull request diff after the branch is reset. This will reset any undeleted
|
||||||
// branches after merging. In particular, it catches a case where the branch was
|
// branches after merging. In particular, it catches a case where the branch was
|
||||||
// squash merged but not deleted. We need to reset to make sure it doesn't appear
|
// squash merged but not deleted. We need to reset to make sure it doesn't appear
|
||||||
// to have a diff with the base due to different commits for the same changes.
|
// to have a diff with the base due to different commits for the same changes.
|
||||||
// - If the number of commits ahead of the base branch differs between the branch and
|
// - If the diff of the commits ahead of the base branch differs between the branch and
|
||||||
// temp branch. This catches a case where the base branch has been force pushed to
|
// temp branch. This catches a case where changes have been partially merged to the
|
||||||
// a new commit.
|
// base. The overall diff is the same, but the branch needs to be rebased to show
|
||||||
|
// the correct diff.
|
||||||
|
//
|
||||||
// For changes on base this reset is equivalent to a rebase of the pull request branch.
|
// For changes on base this reset is equivalent to a rebase of the pull request branch.
|
||||||
const branchCommitsAhead = await commitsAhead(git, base, branch)
|
const branchCommitsAhead = await commitsAhead(git, base, branch)
|
||||||
if (
|
if (
|
||||||
(await git.hasDiff([`${branch}..${tempBranch}`])) ||
|
(await git.hasDiff([`${branch}..${tempBranch}`])) ||
|
||||||
branchCommitsAhead != tempBranchCommitsAhead ||
|
branchCommitsAhead != tempBranchCommitsAhead ||
|
||||||
!(tempBranchCommitsAhead > 0) // !isAhead
|
!(tempBranchCommitsAhead > 0) || // !isAhead
|
||||||
|
(await commitsHaveDiff(git, branch, tempBranch, tempBranchCommitsAhead))
|
||||||
) {
|
) {
|
||||||
core.info(`Resetting '${branch}'`)
|
core.info(`Resetting '${branch}'`)
|
||||||
// Alternatively, git switch -C branch tempBranch
|
// Alternatively, git switch -C branch tempBranch
|
||||||
|
|
Loading…
Reference in a new issue