From af394d7f3daa64f578749fb30682e8706115a6a7 Mon Sep 17 00:00:00 2001 From: jgarec <jgarec@users.noreply.github.com> Date: Thu, 4 Jul 2019 16:00:00 +0200 Subject: [PATCH] fix(master-issue): Respect dry-run setting. (#3974) --- lib/workers/repository/master-issue.js | 32 +- .../master-issue_with_2_PR_closed_ignored.txt | 9 + .../master-issue_with_2_PR_edited.txt | 9 + .../master-issue_with_3_PR_in_progress.txt | 11 + .../_fixtures/master-issue_with_8_PR.txt | 29 ++ test/workers/repository/master-issue.spec.js | 330 ++++++++++++++++++ 6 files changed, 412 insertions(+), 8 deletions(-) create mode 100644 test/workers/repository/_fixtures/master-issue_with_2_PR_closed_ignored.txt create mode 100644 test/workers/repository/_fixtures/master-issue_with_2_PR_edited.txt create mode 100644 test/workers/repository/_fixtures/master-issue_with_3_PR_in_progress.txt create mode 100644 test/workers/repository/_fixtures/master-issue_with_8_PR.txt create mode 100644 test/workers/repository/master-issue.spec.js diff --git a/lib/workers/repository/master-issue.js b/lib/workers/repository/master-issue.js index 39b04c70ef..6a036b6635 100644 --- a/lib/workers/repository/master-issue.js +++ b/lib/workers/repository/master-issue.js @@ -4,7 +4,6 @@ module.exports = { ensureMasterIssue, }; -// istanbul ignore next function getListItem(branch, type, pr) { let item = ' - [ ] '; item += `<!-- ${type}-branch=${branch.branchName} -->`; @@ -22,7 +21,6 @@ function getListItem(branch, type, pr) { return item + ' (' + uniquePackages.join(', ') + ')\n'; } -// istanbul ignore next async function ensureMasterIssue(config, branches) { if ( !(config.masterIssue || branches.some(branch => branch.masterIssueApproval)) @@ -36,13 +34,25 @@ async function ensureMasterIssue(config, branches) { ) { if (config.masterIssueAutoclose) { logger.debug('Closing master issue'); - await platform.ensureIssueClosing(config.masterIssueTitle); + if (config.dryRun) { + logger.info( + 'DRY-RUN: Would close Master Issue ' + config.masterIssueTitle + ); + } else { + await platform.ensureIssueClosing(config.masterIssueTitle); + } return; } - await platform.ensureIssue( - config.masterIssueTitle, - 'This repository is up-to-date and has no outstanding updates open or pending.' - ); + if (config.dryRun) { + logger.info( + 'DRY-RUN: Would ensure Master Issue ' + config.masterIssueTitle + ); + } else { + await platform.ensureIssue( + config.masterIssueTitle, + 'This repository is up-to-date and has no outstanding updates open or pending.' + ); + } return; } let issueBody = `This [master issue](https://renovatebot.com/blog/master-issue) contains a list of ${appName} updates and their statuses.\n\n`; @@ -148,5 +158,11 @@ async function ensureMasterIssue(config, branches) { issueBody += '\n'; } - await platform.ensureIssue(config.masterIssueTitle, issueBody); + if (config.dryRun) { + logger.info( + 'DRY-RUN: Would ensure Master Issue ' + config.masterIssueTitle + ); + } else { + await platform.ensureIssue(config.masterIssueTitle, issueBody); + } } diff --git a/test/workers/repository/_fixtures/master-issue_with_2_PR_closed_ignored.txt b/test/workers/repository/_fixtures/master-issue_with_2_PR_closed_ignored.txt new file mode 100644 index 0000000000..e0eba00ed7 --- /dev/null +++ b/test/workers/repository/_fixtures/master-issue_with_2_PR_closed_ignored.txt @@ -0,0 +1,9 @@ +This [master issue](https://renovatebot.com/blog/master-issue) contains a list of Renovate updates and their statuses. + +## Closed/Ignored + +These updates were closed unmerged and will not be recreated unless you click a checkbox below. + + - [ ] <!-- recreate-branch=branchName1 -->pr1 + - [ ] <!-- recreate-branch=branchName2 -->pr2 (`dep2`, `dep3`) + diff --git a/test/workers/repository/_fixtures/master-issue_with_2_PR_edited.txt b/test/workers/repository/_fixtures/master-issue_with_2_PR_edited.txt new file mode 100644 index 0000000000..8a6f9567a2 --- /dev/null +++ b/test/workers/repository/_fixtures/master-issue_with_2_PR_edited.txt @@ -0,0 +1,9 @@ +This [master issue](https://renovatebot.com/blog/master-issue) contains a list of Renovate updates and their statuses. + +## Edited/Blocked + +These updates have been manually edited so Renovate will no longer make changes. To discard all commits and start over, check the box below. + + - [ ] <!-- rebase-branch=branchName1 -->[pr1](../pull/1) + - [ ] <!-- rebase-branch=branchName2 -->pr2 (`dep2`, `dep3`) + diff --git a/test/workers/repository/_fixtures/master-issue_with_3_PR_in_progress.txt b/test/workers/repository/_fixtures/master-issue_with_3_PR_in_progress.txt new file mode 100644 index 0000000000..301dea48ea --- /dev/null +++ b/test/workers/repository/_fixtures/master-issue_with_3_PR_in_progress.txt @@ -0,0 +1,11 @@ +This [master issue](https://renovatebot.com/blog/master-issue) contains a list of Renovate updates and their statuses. + +## Open + +These updates have all been created already. Click a checkbox below to force a retry/rebase of any. + + - [ ] <!-- rebase-branch=branchName1 -->[pr1](../pull/1) + - [ ] <!-- rebase-branch=branchName2 -->pr2 (`dep2`, `dep3`) + - [ ] <!-- rebase-branch=branchName3 -->[pr3](../pull/3) + - [ ] <!-- rebase-all-open-prs -->**Check this option to rebase all the above open PRs at once** + diff --git a/test/workers/repository/_fixtures/master-issue_with_8_PR.txt b/test/workers/repository/_fixtures/master-issue_with_8_PR.txt new file mode 100644 index 0000000000..e7906b8b08 --- /dev/null +++ b/test/workers/repository/_fixtures/master-issue_with_8_PR.txt @@ -0,0 +1,29 @@ +This [master issue](https://renovatebot.com/blog/master-issue) contains a list of Renovate updates and their statuses. + +## Pending Approval + +These PRs will be created by Renovate only once you click their checkbox below. + + - [ ] <!-- approve-branch=branchName1 -->pr1 + - [ ] <!-- approve-branch=branchName2 -->pr2 + +## Awaiting Schedule + +These updates are awaiting their schedule. Click on a checkbox to ignore the schedule. + - [ ] <!-- unschedule-branch=branchName3 -->pr3 + - [ ] <!-- unschedule-branch=branchName4 -->pr4 + +## Rate Limited + +These updates are currently rate limited. Click on a checkbox below to override. + + - [ ] <!-- unlimit-branch=branchName5 -->pr5 + - [ ] <!-- unlimit-branch=branchName6 -->pr6 + +## Errored + +These updates encountered an error and will be retried. Click a checkbox below to force a retry now. + + - [ ] <!-- retry-branch=branchName7 -->pr7 + - [ ] <!-- retry-branch=branchName8 -->pr8 + diff --git a/test/workers/repository/master-issue.spec.js b/test/workers/repository/master-issue.spec.js new file mode 100644 index 0000000000..a2ab057fed --- /dev/null +++ b/test/workers/repository/master-issue.spec.js @@ -0,0 +1,330 @@ +const fs = require('fs'); +const masterIssue = require('../../../lib/workers/repository/master-issue'); + +let config; +beforeEach(() => { + jest.resetAllMocks(); + config = { ...require('../../config/config/_fixtures') }; + config.platform = 'github'; + config.errors = []; + config.warnings = []; +}); + +async function dryRun( + branches, + platform, + ensureIssueClosingCalls = 0, + ensureIssueCalls = 0, + getBranchPrCalls = 0, + findPrCalls = 0 +) { + jest.resetAllMocks(); + config.dryRun = true; + await masterIssue.ensureMasterIssue(config, branches); + expect(platform.ensureIssueClosing).toHaveBeenCalledTimes( + ensureIssueClosingCalls + ); + expect(platform.ensureIssue).toHaveBeenCalledTimes(ensureIssueCalls); + expect(platform.getBranchPr).toHaveBeenCalledTimes(getBranchPrCalls); + expect(platform.findPr).toHaveBeenCalledTimes(findPrCalls); +} + +describe('workers/repository/master-issue', () => { + describe('ensureMasterIssue()', () => { + it('do nothing if masterissue is disable', async () => { + const branches = []; + await masterIssue.ensureMasterIssue(config, branches); + expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(0); + expect(platform.ensureIssue).toHaveBeenCalledTimes(0); + expect(platform.getBranchPr).toHaveBeenCalledTimes(0); + expect(platform.findPr).toHaveBeenCalledTimes(0); + + // same with dry run + await dryRun(branches, platform); + }); + + it('do nothing if it has no masterissueapproval branches', async () => { + const branches = [ + { + prTitle: 'pr1', + }, + { + prTitle: 'pr2', + masterIssueApproval: false, + }, + ]; + await masterIssue.ensureMasterIssue(config, branches); + expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(0); + expect(platform.ensureIssue).toHaveBeenCalledTimes(0); + expect(platform.getBranchPr).toHaveBeenCalledTimes(0); + expect(platform.findPr).toHaveBeenCalledTimes(0); + + // same with dry run + await dryRun(branches, platform); + }); + + it('closes master issue when there is 0 PR opened and masterIssueAutoclose is true', async () => { + const branches = []; + config.masterIssue = true; + config.masterIssueAutoclose = true; + await masterIssue.ensureMasterIssue(config, branches); + expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(1); + expect(platform.ensureIssueClosing.mock.calls[0][0]).toBe( + config.masterIssueTitle + ); + expect(platform.ensureIssue).toHaveBeenCalledTimes(0); + expect(platform.getBranchPr).toHaveBeenCalledTimes(0); + expect(platform.findPr).toHaveBeenCalledTimes(0); + + // same with dry run + await dryRun(branches, platform); + }); + + it('closes master issue when all branches are automerged and masterIssueAutoclose is true', async () => { + const branches = [ + { + prTitle: 'pr1', + res: 'automerged', + }, + { + prTitle: 'pr2', + res: 'automerged', + masterIssueApproval: false, + }, + ]; + config.masterIssue = true; + config.masterIssueAutoclose = true; + await masterIssue.ensureMasterIssue(config, branches); + expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(1); + expect(platform.ensureIssueClosing.mock.calls[0][0]).toBe( + config.masterIssueTitle + ); + expect(platform.ensureIssue).toHaveBeenCalledTimes(0); + expect(platform.getBranchPr).toHaveBeenCalledTimes(0); + expect(platform.findPr).toHaveBeenCalledTimes(0); + + // same with dry run + await dryRun(branches, platform); + }); + + it('open or update master issue when all branches are closed and masterIssueAutoclose is false', async () => { + const branches = []; + config.masterIssue = true; + await masterIssue.ensureMasterIssue(config, branches); + expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(0); + expect(platform.ensureIssue).toHaveBeenCalledTimes(1); + expect(platform.ensureIssue.mock.calls[0][0]).toBe( + config.masterIssueTitle + ); + expect(platform.ensureIssue.mock.calls[0][1]).toBe( + 'This repository is up-to-date and has no outstanding updates open or pending.' + ); + expect(platform.getBranchPr).toHaveBeenCalledTimes(0); + expect(platform.findPr).toHaveBeenCalledTimes(0); + + // same with dry run + await dryRun(branches, platform); + }); + + it('checks an issue with 2 Pending Approvals, 2 not scheduled, 2 pr-hourly-limit-reached and 2 in error', async () => { + const branches = [ + { + prTitle: 'pr1', + upgrades: [{ depName: 'dep1' }], + res: 'needs-approval', + branchName: 'branchName1', + }, + { + prTitle: 'pr2', + upgrades: [{ depName: 'dep2' }], + res: 'needs-approval', + branchName: 'branchName2', + }, + { + prTitle: 'pr3', + upgrades: [{ depName: 'dep3' }], + res: 'not-scheduled', + branchName: 'branchName3', + }, + { + prTitle: 'pr4', + upgrades: [{ depName: 'dep4' }], + res: 'not-scheduled', + branchName: 'branchName4', + }, + { + prTitle: 'pr5', + upgrades: [{ depName: 'dep5' }], + res: 'pr-hourly-limit-reached', + branchName: 'branchName5', + }, + { + prTitle: 'pr6', + upgrades: [{ depName: 'dep6' }], + res: 'pr-hourly-limit-reached', + branchName: 'branchName6', + }, + { + prTitle: 'pr7', + upgrades: [{ depName: 'dep7' }], + res: 'error', + branchName: 'branchName7', + }, + { + prTitle: 'pr8', + upgrades: [{ depName: 'dep8' }], + res: 'error', + branchName: 'branchName8', + }, + ]; + config.masterIssue = true; + await masterIssue.ensureMasterIssue(config, branches); + expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(0); + expect(platform.ensureIssue).toHaveBeenCalledTimes(1); + expect(platform.ensureIssue.mock.calls[0][0]).toBe( + config.masterIssueTitle + ); + expect(platform.ensureIssue.mock.calls[0][1]).toBe( + fs.readFileSync( + 'test/workers/repository/_fixtures/master-issue_with_8_PR.txt', + 'utf8' + ) + ); + expect(platform.getBranchPr).toHaveBeenCalledTimes(0); + expect(platform.findPr).toHaveBeenCalledTimes(0); + + // same with dry run + await dryRun(branches, platform); + }); + + it('checks an issue with 2 PR pr-edited', async () => { + const branches = [ + { + prTitle: 'pr1', + upgrades: [{ depName: 'dep1' }], + res: 'pr-edited', + branchName: 'branchName1', + }, + { + prTitle: 'pr2', + upgrades: [{ depName: 'dep2' }, { depName: 'dep3' }], + res: 'pr-edited', + branchName: 'branchName2', + }, + ]; + config.masterIssue = true; + platform.getBranchPr.mockReturnValueOnce({ number: 1 }); + platform.getBranchPr.mockReturnValueOnce(undefined); + await masterIssue.ensureMasterIssue(config, branches); + expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(0); + expect(platform.ensureIssue).toHaveBeenCalledTimes(1); + expect(platform.ensureIssue.mock.calls[0][0]).toBe( + config.masterIssueTitle + ); + expect(platform.ensureIssue.mock.calls[0][1]).toBe( + fs.readFileSync( + 'test/workers/repository/_fixtures/master-issue_with_2_PR_edited.txt', + 'utf8' + ) + ); + expect(platform.getBranchPr).toHaveBeenCalledTimes(2); + expect(platform.getBranchPr.mock.calls[0][0]).toBe('branchName1'); + expect(platform.getBranchPr.mock.calls[1][0]).toBe('branchName2'); + expect(platform.findPr).toHaveBeenCalledTimes(0); + + // same with dry run + await dryRun(branches, platform, 0, 0, 2, 0); + }); + + it('checks an issue with 3 PR in progress and rebase all option', async () => { + const branches = [ + { + prTitle: 'pr1', + upgrades: [{ depName: 'dep1' }], + res: 'rebase', + branchName: 'branchName1', + }, + { + prTitle: 'pr2', + upgrades: [{ depName: 'dep2' }, { depName: 'dep3' }], + res: 'rebase', + branchName: 'branchName2', + }, + { + prTitle: 'pr3', + upgrades: [{ depName: 'dep3' }], + res: 'rebase', + branchName: 'branchName3', + }, + ]; + config.masterIssue = true; + platform.getBranchPr.mockReturnValueOnce({ number: 1 }); + platform.getBranchPr.mockReturnValueOnce(undefined); + platform.getBranchPr.mockReturnValueOnce({ number: 3 }); + await masterIssue.ensureMasterIssue(config, branches); + expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(0); + expect(platform.ensureIssue).toHaveBeenCalledTimes(1); + expect(platform.ensureIssue.mock.calls[0][0]).toBe( + config.masterIssueTitle + ); + expect(platform.ensureIssue.mock.calls[0][1]).toBe( + fs.readFileSync( + 'test/workers/repository/_fixtures/master-issue_with_3_PR_in_progress.txt', + 'utf8' + ) + ); + expect(platform.getBranchPr).toHaveBeenCalledTimes(3); + expect(platform.getBranchPr.mock.calls[0][0]).toBe('branchName1'); + expect(platform.getBranchPr.mock.calls[1][0]).toBe('branchName2'); + expect(platform.getBranchPr.mock.calls[2][0]).toBe('branchName3'); + expect(platform.findPr).toHaveBeenCalledTimes(0); + + // same with dry run + await dryRun(branches, platform, 0, 0, 3, 0); + }); + + it('checks an issue with 2 PR closed / ignored', async () => { + const branches = [ + { + prTitle: 'pr1', + upgrades: [{ depName: 'dep1' }], + res: 'already-existed', + branchName: 'branchName1', + }, + { + prTitle: 'pr2', + upgrades: [{ depName: 'dep2' }, { depName: 'dep3' }], + res: 'already-existed', + branchName: 'branchName2', + }, + ]; + config.masterIssue = true; + platform.getBranchPr.mockReturnValueOnce({ number: 1 }); + platform.getBranchPr.mockReturnValueOnce(undefined); + platform.getBranchPr.mockReturnValueOnce({ number: 3 }); + await masterIssue.ensureMasterIssue(config, branches); + expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(0); + expect(platform.ensureIssue).toHaveBeenCalledTimes(1); + expect(platform.ensureIssue.mock.calls[0][0]).toBe( + config.masterIssueTitle + ); + expect(platform.ensureIssue.mock.calls[0][1]).toBe( + fs.readFileSync( + 'test/workers/repository/_fixtures/master-issue_with_2_PR_closed_ignored.txt', + 'utf8' + ) + ); + expect(platform.getBranchPr).toHaveBeenCalledTimes(0); + expect(platform.findPr).toHaveBeenCalledTimes(2); + expect(platform.findPr.mock.calls[0][0]).toBe('branchName1'); + expect(platform.findPr.mock.calls[0][1]).toBe('pr1'); + expect(platform.findPr.mock.calls[0][2]).toBe('!open'); + expect(platform.findPr.mock.calls[1][0]).toBe('branchName2'); + expect(platform.findPr.mock.calls[1][1]).toBe('pr2'); + expect(platform.findPr.mock.calls[1][2]).toBe('!open'); + + // same with dry run + await dryRun(branches, platform, 0, 0, 0, 2); + }); + }); +}); -- GitLab