Skip to content

Commit

Permalink
fix(pr): ensure PR update after branch commit (#18839)
Browse files Browse the repository at this point in the history
Co-authored-by: Rhys Arkins <rhys@arkins.net>
Co-authored-by: Michael Kriese <michael.kriese@visualon.de>
Closes #17388
  • Loading branch information
PhilipAbed committed Nov 16, 2022
1 parent 9562cf4 commit 6c48643
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 10 deletions.
54 changes: 54 additions & 0 deletions lib/workers/repository/update/branch/index.spec.ts
Expand Up @@ -1955,5 +1955,59 @@ describe('workers/repository/update/branch/index', () => {
result: 'done',
});
});

it('continues branch, skips automerge if there are artifact errors', async () => {
jest.spyOn(getUpdated, 'getUpdatedPackageFiles').mockResolvedValueOnce({
updatedPackageFiles: [{}],
artifactErrors: [{}],
} as PackageFilesResult);
npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({
artifactErrors: [],
updatedArtifacts: [],
});
git.branchExists.mockReturnValueOnce(true);
git.isBranchModified.mockResolvedValueOnce(true);
git.getBranchCommit.mockReturnValueOnce('123test');
platform.findPr.mockResolvedValueOnce({ sha: '123test' } as any);
const res = await branchWorker.processBranch(config);
expect(automerge.tryBranchAutomerge).not.toHaveBeenCalled();
expect(prAutomerge.checkAutoMerge).not.toHaveBeenCalled();
expect(res).toEqual({
branchExists: true,
commitSha: '123test',
prNo: undefined,
result: 'done',
updatesVerified: true,
});
});

it('continues to update PR, if branch got updated, even when prCreation!==immediate', async () => {
git.branchExists.mockReturnValueOnce(true);
git.isBranchModified.mockResolvedValueOnce(false);
git.getBranchCommit.mockReturnValueOnce('123test');
npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({
artifactErrors: [],
updatedArtifacts: [partial<FileChange>({})],
} as WriteExistingFilesResult);
platform.getBranchPr.mockResolvedValueOnce({
state: 'open',
} as Pr);
jest.spyOn(getUpdated, 'getUpdatedPackageFiles').mockResolvedValueOnce({
updatedPackageFiles: [{}],
} as PackageFilesResult);
const inconfig = {
...config,
prCreation: 'not-pending',
} as BranchConfig;
expect(await branchWorker.processBranch(inconfig)).toEqual({
branchExists: true,
updatesVerified: true,
prNo: undefined,
result: 'done',
commitSha: '123test',
});
expect(automerge.tryBranchAutomerge).not.toHaveBeenCalled();
expect(prWorker.ensurePr).toHaveBeenCalledTimes(1);
});
});
});
27 changes: 17 additions & 10 deletions lib/workers/repository/update/branch/index.ts
Expand Up @@ -553,9 +553,11 @@ export async function processBranch(
await setStability(config);
await setConfidence(config);

// break if we pushed a new commit because status check are pretty sure pending but maybe not reported yet
// new commit means status check are pretty sure pending but maybe not reported yet
// if PR has not been created + new commit + prCreation !== immediate skip
// but do not break when there are artifact errors
if (
!branchPr &&
!config.artifactErrors?.length &&
!userRebaseRequested &&
commitSha &&
Expand All @@ -565,14 +567,15 @@ export async function processBranch(
return {
branchExists: true,
updatesVerified,
prNo: branchPr?.number,
result: BranchResult.Pending,
commitSha,
};
}

// Try to automerge branch and finish if successful, but only if branch already existed before this run
if (branchExists || config.ignoreTests) {
// skip if we have a non-immediate pr and there is an existing PR,
// we want to update the PR and skip the Auto merge since status checks aren't done yet
if (!config.artifactErrors?.length && (!commitSha || config.ignoreTests)) {
const mergeStatus = await tryBranchAutomerge(config);
logger.debug(`mergeStatus=${mergeStatus}`);
if (mergeStatus === 'automerged') {
Expand Down Expand Up @@ -811,13 +814,17 @@ export async function processBranch(
}
} else if (config.automerge) {
logger.debug('PR is configured for automerge');
const prAutomergeResult = await checkAutoMerge(pr, config);
if (prAutomergeResult?.automerged) {
return {
branchExists,
result: BranchResult.Automerged,
commitSha,
};
// skip automerge if there is a new commit since status checks aren't done yet
if (!commitSha || config.ignoreTests) {
logger.debug('checking auto-merge');
const prAutomergeResult = await checkAutoMerge(pr, config);
if (prAutomergeResult?.automerged) {
return {
branchExists,
result: BranchResult.Automerged,
commitSha,
};
}
}
} else {
logger.debug('PR is not configured for automerge');
Expand Down

0 comments on commit 6c48643

Please sign in to comment.