Skip to content

Commit

Permalink
refactor: separate automerge function in pr
Browse files Browse the repository at this point in the history
  • Loading branch information
rarkins committed Apr 14, 2021
1 parent 27a3513 commit 966e141
Show file tree
Hide file tree
Showing 6 changed files with 201 additions and 111 deletions.
27 changes: 15 additions & 12 deletions lib/workers/branch/index.spec.ts
Expand Up @@ -20,6 +20,7 @@ import * as _sanitize from '../../util/sanitize';
import * as _limits from '../global/limits';
import * as _prWorker from '../pr';
import type { EnsurePrResult } from '../pr';
import * as _prAutomerge from '../pr/automerge';
import type { Pr } from '../repository/onboarding/branch/check';
import type { BranchConfig, BranchUpgradeConfig } from '../types';
import { PrResult, ProcessBranchResult } from '../types';
Expand All @@ -40,6 +41,7 @@ jest.mock('../../manager/npm/post-update');
jest.mock('./automerge');
jest.mock('./commit');
jest.mock('../pr');
jest.mock('../pr/automerge');
jest.mock('../../util/exec');
jest.mock('../../util/sanitize');
jest.mock('../../util/git');
Expand All @@ -53,6 +55,7 @@ const reuse = mocked(_reuse);
const npmPostExtract = mocked(_npmPostExtract);
const automerge = mocked(_automerge);
const commit = mocked(_commit);
const prAutomerge = mocked(_prAutomerge);
const prWorker = mocked(_prWorker);
const exec = mocked(_exec);
const sanitize = mocked(_sanitize);
Expand All @@ -69,7 +72,7 @@ describe(getName(__filename), () => {
let config: BranchConfig;
beforeEach(() => {
prWorker.ensurePr = jest.fn();
prWorker.checkAutoMerge = jest.fn();
prAutomerge.checkAutoMerge = jest.fn();
config = {
...defaultConfig,
branchName: 'renovate/some-branch',
Expand Down Expand Up @@ -448,12 +451,12 @@ describe(getName(__filename), () => {
prResult: PrResult.Created,
pr: {},
} as EnsurePrResult);
prWorker.checkAutoMerge.mockResolvedValueOnce(true);
prAutomerge.checkAutoMerge.mockResolvedValueOnce(true);
commit.commitFilesToBranch.mockResolvedValueOnce(null);
await branchWorker.processBranch(config);
expect(prWorker.ensurePr).toHaveBeenCalledTimes(1);
expect(platform.ensureCommentRemoval).toHaveBeenCalledTimes(0);
expect(prWorker.checkAutoMerge).toHaveBeenCalledTimes(1);
expect(prAutomerge.checkAutoMerge).toHaveBeenCalledTimes(1);
});
it('ensures PR and adds lock file error comment if no releaseTimestamp', async () => {
getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce({
Expand All @@ -469,12 +472,12 @@ describe(getName(__filename), () => {
prResult: PrResult.Created,
pr: {},
} as EnsurePrResult);
prWorker.checkAutoMerge.mockResolvedValueOnce(true);
prAutomerge.checkAutoMerge.mockResolvedValueOnce(true);
commit.commitFilesToBranch.mockResolvedValueOnce(null);
await branchWorker.processBranch(config);
expect(platform.ensureComment).toHaveBeenCalledTimes(1);
expect(prWorker.ensurePr).toHaveBeenCalledTimes(1);
expect(prWorker.checkAutoMerge).toHaveBeenCalledTimes(0);
expect(prAutomerge.checkAutoMerge).toHaveBeenCalledTimes(0);
});
it('ensures PR and adds lock file error comment if old releaseTimestamp', async () => {
getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce({
Expand All @@ -490,13 +493,13 @@ describe(getName(__filename), () => {
prResult: PrResult.Created,
pr: {},
} as EnsurePrResult);
prWorker.checkAutoMerge.mockResolvedValueOnce(true);
prAutomerge.checkAutoMerge.mockResolvedValueOnce(true);
config.releaseTimestamp = '2018-04-26T05:15:51.877Z';
commit.commitFilesToBranch.mockResolvedValueOnce(null);
await branchWorker.processBranch(config);
expect(platform.ensureComment).toHaveBeenCalledTimes(1);
expect(prWorker.ensurePr).toHaveBeenCalledTimes(1);
expect(prWorker.checkAutoMerge).toHaveBeenCalledTimes(0);
expect(prAutomerge.checkAutoMerge).toHaveBeenCalledTimes(0);
});
it('ensures PR and adds lock file error comment if new releaseTimestamp and branch exists', async () => {
getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce({
Expand All @@ -512,13 +515,13 @@ describe(getName(__filename), () => {
prResult: PrResult.Created,
pr: {},
} as EnsurePrResult);
prWorker.checkAutoMerge.mockResolvedValueOnce(true);
prAutomerge.checkAutoMerge.mockResolvedValueOnce(true);
config.releaseTimestamp = new Date().toISOString();
commit.commitFilesToBranch.mockResolvedValueOnce(null);
await branchWorker.processBranch(config);
expect(platform.ensureComment).toHaveBeenCalledTimes(1);
expect(prWorker.ensurePr).toHaveBeenCalledTimes(1);
expect(prWorker.checkAutoMerge).toHaveBeenCalledTimes(0);
expect(prAutomerge.checkAutoMerge).toHaveBeenCalledTimes(0);
});
it('throws error if lock file errors and new releaseTimestamp', async () => {
getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce({
Expand All @@ -534,7 +537,7 @@ describe(getName(__filename), () => {
prResult: PrResult.Created,
pr: {},
} as EnsurePrResult);
prWorker.checkAutoMerge.mockResolvedValueOnce(true);
prAutomerge.checkAutoMerge.mockResolvedValueOnce(true);
config.releaseTimestamp = new Date().toISOString();
await expect(branchWorker.processBranch(config)).rejects.toThrow(
Error(MANAGER_LOCKFILE_ERROR)
Expand All @@ -555,12 +558,12 @@ describe(getName(__filename), () => {
prResult: PrResult.Created,
pr: {},
} as EnsurePrResult);
prWorker.checkAutoMerge.mockResolvedValueOnce(true);
prAutomerge.checkAutoMerge.mockResolvedValueOnce(true);
commit.commitFilesToBranch.mockResolvedValueOnce(null);
await branchWorker.processBranch(config);
expect(platform.ensureComment).toHaveBeenCalledTimes(1);
expect(prWorker.ensurePr).toHaveBeenCalledTimes(1);
expect(prWorker.checkAutoMerge).toHaveBeenCalledTimes(0);
expect(prAutomerge.checkAutoMerge).toHaveBeenCalledTimes(0);
});
it('swallows branch errors', async () => {
getUpdated.getUpdatedPackageFiles.mockImplementationOnce(() => {
Expand Down
3 changes: 2 additions & 1 deletion lib/workers/branch/index.ts
Expand Up @@ -27,7 +27,8 @@ import {
isBranchModified,
} from '../../util/git';
import { Limit, isLimitReached } from '../global/limits';
import { checkAutoMerge, ensurePr, getPlatformPrOptions } from '../pr';
import { ensurePr, getPlatformPrOptions } from '../pr';
import { checkAutoMerge } from '../pr/automerge';
import { BranchConfig, PrResult, ProcessBranchResult } from '../types';
import { tryBranchAutomerge } from './automerge';
import { prAlreadyExisted } from './check-existing';
Expand Down
83 changes: 83 additions & 0 deletions lib/workers/pr/automerge.spec.ts
@@ -0,0 +1,83 @@
import { getConfig, getName, git, mocked, partial } from '../../../test/util';
import { Pr, platform as _platform } from '../../platform';
import { BranchStatus } from '../../types';
import { BranchConfig } from '../types';
import * as prAutomerge from './automerge';

jest.mock('../../util/git');

const platform = mocked(_platform);
const defaultConfig = getConfig();

describe(getName(__filename), () => {
describe('checkAutoMerge(pr, config)', () => {
let config: BranchConfig;
let pr: Pr;
beforeEach(() => {
config = partial<BranchConfig>({
...defaultConfig,
});
pr = partial<Pr>({
canMerge: true,
});
});
afterEach(() => {
jest.clearAllMocks();
});
it('should not automerge if not configured', async () => {
await prAutomerge.checkAutoMerge(pr, config);
expect(platform.mergePr).toHaveBeenCalledTimes(0);
});
it('should automerge if enabled and pr is mergeable', async () => {
config.automerge = true;
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.green);
platform.mergePr.mockResolvedValueOnce(true);
await prAutomerge.checkAutoMerge(pr, config);
expect(platform.mergePr).toHaveBeenCalledTimes(1);
});
it('should automerge comment', async () => {
config.automerge = true;
config.automergeType = 'pr-comment';
config.automergeComment = '!merge';
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.green);
await prAutomerge.checkAutoMerge(pr, config);
expect(platform.ensureCommentRemoval).toHaveBeenCalledTimes(0);
expect(platform.ensureComment).toHaveBeenCalledTimes(1);
});
it('should remove previous automerge comment when rebasing', async () => {
config.automerge = true;
config.automergeType = 'pr-comment';
config.automergeComment = '!merge';
config.rebaseRequested = true;
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.green);
await prAutomerge.checkAutoMerge(pr, config);
expect(platform.ensureCommentRemoval).toHaveBeenCalledTimes(1);
expect(platform.ensureComment).toHaveBeenCalledTimes(1);
});
it('should not automerge if enabled and pr is mergeable but cannot rebase', async () => {
config.automerge = true;
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.green);
git.isBranchModified.mockResolvedValueOnce(true);
await prAutomerge.checkAutoMerge(pr, config);
expect(platform.mergePr).toHaveBeenCalledTimes(0);
});
it('should not automerge if enabled and pr is mergeable but branch status is not success', async () => {
config.automerge = true;
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.yellow);
await prAutomerge.checkAutoMerge(pr, config);
expect(platform.mergePr).toHaveBeenCalledTimes(0);
});
it('should not automerge if enabled and pr is mergeable but unstable', async () => {
config.automerge = true;
pr.canMerge = undefined;
await prAutomerge.checkAutoMerge(pr, config);
expect(platform.mergePr).toHaveBeenCalledTimes(0);
});
it('should not automerge if enabled and pr is unmergeable', async () => {
config.automerge = true;
pr.isConflicted = true;
await prAutomerge.checkAutoMerge(pr, config);
expect(platform.mergePr).toHaveBeenCalledTimes(0);
});
});
});
91 changes: 91 additions & 0 deletions lib/workers/pr/automerge.ts
@@ -0,0 +1,91 @@
import { getAdminConfig } from '../../config/admin';
import { logger } from '../../logger';
import { Pr, platform } from '../../platform';
import { BranchStatus } from '../../types';
import { isBranchModified } from '../../util/git';
import { BranchConfig } from '../types';

export async function checkAutoMerge(
pr: Pr,
config: BranchConfig
): Promise<boolean> {
logger.trace({ config }, 'checkAutoMerge');
const {
branchName,
automerge,
automergeType,
automergeComment,
requiredStatusChecks,
rebaseRequested,
} = config;
logger.debug(
{ automerge, automergeType, automergeComment },
`Checking #${pr.number} for automerge`
);
if (automerge) {
logger.debug('PR is configured for automerge');
// Return if PR not ready for automerge
if (pr.isConflicted) {
logger.debug('PR is conflicted');
logger.debug({ pr });
return false;
}
if (requiredStatusChecks && pr.canMerge !== true) {
logger.debug(
{ canMergeReason: pr.canMergeReason },
'PR is not ready for merge'
);
return false;
}
const branchStatus = await platform.getBranchStatus(
branchName,
requiredStatusChecks
);
if (branchStatus !== BranchStatus.green) {
logger.debug(
`PR is not ready for merge (branch status is ${branchStatus})`
);
return false;
}
// Check if it's been touched
if (await isBranchModified(branchName)) {
logger.debug('PR is ready for automerge but has been modified');
return false;
}
if (automergeType === 'pr-comment') {
logger.debug(`Applying automerge comment: ${automergeComment}`);
// istanbul ignore if
if (getAdminConfig().dryRun) {
logger.info(
`DRY-RUN: Would add PR automerge comment to PR #${pr.number}`
);
return false;
}
if (rebaseRequested) {
await platform.ensureCommentRemoval({
number: pr.number,
content: automergeComment,
});
}
return platform.ensureComment({
number: pr.number,
topic: null,
content: automergeComment,
});
}
// Let's merge this
logger.debug(`Automerging #${pr.number}`);
// istanbul ignore if
if (getAdminConfig().dryRun) {
logger.info(`DRY-RUN: Would merge PR #${pr.number}`);
return false;
}
const res = await platform.mergePr(pr.number, branchName);
if (res) {
logger.info({ pr: pr.number, prTitle: pr.title }, 'PR automerged');
}
return res;
}
logger.debug('No automerge');
return false;
}
17 changes: 9 additions & 8 deletions lib/workers/pr/index.spec.ts
Expand Up @@ -5,6 +5,7 @@ import { Pr, platform as _platform } from '../../platform';
import { BranchStatus } from '../../types';
import * as _limits from '../global/limits';
import { BranchConfig, PrResult } from '../types';
import * as prAutomerge from './automerge';
import * as _changelogHelper from './changelog';
import { getChangeLogJSON } from './changelog';
import * as codeOwners from './code-owners';
Expand Down Expand Up @@ -114,22 +115,22 @@ describe(getName(__filename), () => {
jest.clearAllMocks();
});
it('should not automerge if not configured', async () => {
await prWorker.checkAutoMerge(pr, config);
await prAutomerge.checkAutoMerge(pr, config);
expect(platform.mergePr).toHaveBeenCalledTimes(0);
});
it('should automerge if enabled and pr is mergeable', async () => {
config.automerge = true;
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.green);
platform.mergePr.mockResolvedValueOnce(true);
await prWorker.checkAutoMerge(pr, config);
await prAutomerge.checkAutoMerge(pr, config);
expect(platform.mergePr).toHaveBeenCalledTimes(1);
});
it('should automerge comment', async () => {
config.automerge = true;
config.automergeType = 'pr-comment';
config.automergeComment = '!merge';
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.green);
await prWorker.checkAutoMerge(pr, config);
await prAutomerge.checkAutoMerge(pr, config);
expect(platform.ensureCommentRemoval).toHaveBeenCalledTimes(0);
expect(platform.ensureComment).toHaveBeenCalledTimes(1);
});
Expand All @@ -139,33 +140,33 @@ describe(getName(__filename), () => {
config.automergeComment = '!merge';
config.rebaseRequested = true;
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.green);
await prWorker.checkAutoMerge(pr, config);
await prAutomerge.checkAutoMerge(pr, config);
expect(platform.ensureCommentRemoval).toHaveBeenCalledTimes(1);
expect(platform.ensureComment).toHaveBeenCalledTimes(1);
});
it('should not automerge if enabled and pr is mergeable but cannot rebase', async () => {
config.automerge = true;
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.green);
git.isBranchModified.mockResolvedValueOnce(true);
await prWorker.checkAutoMerge(pr, config);
await prAutomerge.checkAutoMerge(pr, config);
expect(platform.mergePr).toHaveBeenCalledTimes(0);
});
it('should not automerge if enabled and pr is mergeable but branch status is not success', async () => {
config.automerge = true;
platform.getBranchStatus.mockResolvedValueOnce(BranchStatus.yellow);
await prWorker.checkAutoMerge(pr, config);
await prAutomerge.checkAutoMerge(pr, config);
expect(platform.mergePr).toHaveBeenCalledTimes(0);
});
it('should not automerge if enabled and pr is mergeable but unstable', async () => {
config.automerge = true;
pr.canMerge = undefined;
await prWorker.checkAutoMerge(pr, config);
await prAutomerge.checkAutoMerge(pr, config);
expect(platform.mergePr).toHaveBeenCalledTimes(0);
});
it('should not automerge if enabled and pr is unmergeable', async () => {
config.automerge = true;
pr.isConflicted = true;
await prWorker.checkAutoMerge(pr, config);
await prAutomerge.checkAutoMerge(pr, config);
expect(platform.mergePr).toHaveBeenCalledTimes(0);
});
});
Expand Down

0 comments on commit 966e141

Please sign in to comment.