diff --git a/lib/platform/gitlab/index.js b/lib/platform/gitlab/index.js index a951361ad44fae586d565e69b07c49179b4a759f..78c7cccd7c8a103a8235f7f15b9c7cd17c84f6e1 100644 --- a/lib/platform/gitlab/index.js +++ b/lib/platform/gitlab/index.js @@ -347,15 +347,17 @@ async function findPr(branchName, prTitle, state = 'all') { // Pull Request -async function createPr(branchName, title, body, labels, useDefaultBranch) { +async function createPr( + branchName, + title, + description, + labels, + useDefaultBranch +) { const targetBranch = useDefaultBranch ? config.defaultBranch : config.baseBranch; logger.debug(`Creating Merge Request: ${title}`); - const description = body - .replace('</h4>', ' </h4>') // See #954 - .replace(/Pull Request/g, 'Merge Request') - .replace(/PR/g, 'MR'); const res = await get.post(`projects/${config.repoName}/merge_requests`, { body: { source_branch: branchName, @@ -400,11 +402,7 @@ async function getPr(prNo) { return pr; } -async function updatePr(prNo, title, body) { - const description = body - .replace('</h4>', ' </h4>') // See #954 - .replace(/Pull Request/g, 'Merge Request') - .replace(/PR/g, 'MR'); +async function updatePr(prNo, title, description) { await get.put(`projects/${config.repoName}/merge_requests/${prNo}`, { body: { title, diff --git a/lib/workers/pr/index.js b/lib/workers/pr/index.js index c86d96c8edc7a69a5ee33a67955bcb6ff04e5e06..42bac0ac8de4fb2c19ce7419d3845af0fedf9e0a 100644 --- a/lib/workers/pr/index.js +++ b/lib/workers/pr/index.js @@ -129,22 +129,23 @@ async function ensurePr(prConfig) { } const prTitle = handlebars.compile(config.prTitle)(config); - let prBody; - function trimPrBody() { - let prBodyMarkdown = handlebars.compile(config.prBody)(config); - prBodyMarkdown = prBodyMarkdown.replace('@', '@​'); - prBody = converter.makeHtml(prBodyMarkdown); - // Public GitHub repos need links prevented - see #489 - prBody = prBody.replace(issueRe, '$1#​$2$3'); - const backTickRe = /`([^/]*?)`/g; - prBody = prBody.replace(backTickRe, '<code>$1</code>'); - // istanbul ignore if - if (prBody.length > 250000) { - config.upgrades.pop(); - trimPrBody(); - } + let prBody = handlebars.compile(config.prBody)(config); + // Put a zero width space after every @ symbol to prevent unintended hyperlinking + prBody = prBody.replace('@', '@​'); + // Public GitHub repos need links prevented - see #489 + prBody = prBody.replace(issueRe, '$1#​$2$3'); + // convert escaped backticks back to ` + const backTickRe = /`([^/]*?)`/g; + prBody = prBody.replace(backTickRe, '`$1`'); + // It would be nice to abstract this to the platform layer but made difficult due to our create/update check + if (config.platform === 'gitlab') { + // Convert to HTML using GitHub-flavoured markdown as it is more feature-rich than GitLab's flavour + prBody = converter + .makeHtml(prBody) + .replace('</h4>', ' </h4>') // See #954 + .replace(/Pull Request/g, 'Merge Request') + .replace(/PR/g, 'MR'); } - trimPrBody(); try { // Check if existing PR exists diff --git a/test/workers/pr/__snapshots__/index.spec.js.snap b/test/workers/pr/__snapshots__/index.spec.js.snap index f4400485f0217ee2ef2c65292d5eb8bd0a30d093..91ae1f659feb866c73b216cd2340cc0494231ae7 100644 --- a/test/workers/pr/__snapshots__/index.spec.js.snap +++ b/test/workers/pr/__snapshots__/index.spec.js.snap @@ -26,19 +26,71 @@ Array [ ] `; -exports[`workers/pr ensurePr should return modified existing PR 1`] = ` -Object { - "body": "<p>This Pull Request updates dependency <a href=\\"https://github.com/renovateapp/dummy\\">dummy</a> from <code>v1.0.0</code> to <code>v1.1.0</code></p> +exports[`workers/pr ensurePr should convert to HTML PR for gitlab 1`] = ` +Array [ + "renovate/dummy-1.x", + "Update dependency dummy to v1.1.0", + "<p>This Merge Request updates dependency <a href=\\"https://github.com/renovateapp/dummy\\">dummy</a> from <code>v1.0.0</code> to <code>v1.1.0</code></p> <h3 id=\\"commits\\">Commits</h3> <p><details><br /> <summary>renovateapp/dummy</summary></p> -<h4 id=\\"110\\">1.1.0</h4> +<h4 id=\\"110\\">1.1.0 </h4> <ul> <li><a href=\\"https://github.com/renovateapp/dummy/commit/abcdefghijklmnopqrstuvwxyz\\"><code>abcdefg</code></a> foo <a href=\\"https://github.com/renovateapp/dummy/issues/3\\">#3</a></li> </ul> <p></details></p> <hr /> -<p>This PR has been generated by <a href=\\"https://renovateapp.com\\">Renovate Bot</a>.</p>", +<p>This MR has been generated by <a href=\\"https://renovateapp.com\\">Renovate Bot</a>.</p>", + Array [], +] +`; + +exports[`workers/pr ensurePr should create PR if success 1`] = ` +Array [ + "renovate/dummy-1.x", + "Update dependency dummy to v1.1.0", + "This Pull Request updates dependency [dummy](https://github.com/renovateapp/dummy) from \`v1.0.0\` to \`v1.1.0\` + + +### Commits + +<details> +<summary>renovateapp/dummy</summary> + +#### 1.1.0 +- [\`abcdefg\`](https://github.com/renovateapp/dummy/commit/abcdefghijklmnopqrstuvwxyz) foo [#3](https://github.com/renovateapp/dummy/issues/3) + +</details> + + + +--- + +This PR has been generated by [Renovate Bot](https://renovateapp.com).", + Array [], +] +`; + +exports[`workers/pr ensurePr should return modified existing PR 1`] = ` +Object { + "body": "This Pull Request updates dependency [dummy](https://github.com/renovateapp/dummy) from \`v1.0.0\` to \`v1.1.0\` + + +### Commits + +<details> +<summary>renovateapp/dummy</summary> + +#### 1.1.0 +- [\`abcdefg\`](https://github.com/renovateapp/dummy/commit/abcdefghijklmnopqrstuvwxyz) foo [#3](https://github.com/renovateapp/dummy/issues/3) + +</details> + + + +--- + +This PR has been generated by [Renovate Bot](https://renovateapp.com).", "displayNumber": "Existing PR", "title": "Update dependency dummy to v1.1.0", } diff --git a/test/workers/pr/index.spec.js b/test/workers/pr/index.spec.js index 057b42693ad5fecd16fa152ed73537385be2b304..7cf4faa232d87ce6e710710d1dc65f5ce353bc49 100644 --- a/test/workers/pr/index.spec.js +++ b/test/workers/pr/index.spec.js @@ -86,28 +86,24 @@ describe('workers/pr', () => { }); describe('ensurePr', () => { let config; - let existingPr; + const existingPr = { + displayNumber: 'Existing PR', + title: 'Update dependency dummy to v1.1.0', + }; beforeEach(() => { config = { ...defaultConfig, }; + config.branchName = 'renovate/dummy-1.x'; + config.prTitle = 'Update dependency dummy to v1.1.0'; + config.depName = 'dummy'; + config.isGitHub = true; + config.privateRepo = true; + config.currentVersion = '1.0.0'; + config.newVersion = '1.1.0'; + config.repositoryUrl = 'https://github.com/renovateapp/dummy'; platform.createPr.mockReturnValue({ displayNumber: 'New Pull Request' }); config.upgrades = [config]; - existingPr = { - title: 'Update dependency dummy to v1.1.0', - body: `<p>This Pull Request updates dependency <a href="https://github.com/renovateapp/dummy">dummy</a> from <code>v1.0.0</code> to <code>v1.1.0</code></p> -<h3 id="commits">Commits</h3> -<p><details><br /> -<summary>renovateapp/dummy</summary></p> -<h4 id="110">1.1.0</h4> -<ul> -<li><a href="https://github.com/renovateapp/dummy/commit/abcdefghijklmnopqrstuvwxyz"><code>abcdefg</code></a> foo <a href="https://github.com/renovateapp/dummy/issues/3">#3</a></li> -</ul> -<p></details></p> -<hr /> -<p>This PR has been generated by <a href="https://renovateapp.com">Renovate Bot</a>.</p>`, - displayNumber: 'Existing PR', - }; }); afterEach(() => { jest.clearAllMocks(); @@ -130,6 +126,19 @@ describe('workers/pr', () => { config.prCreation = 'status-success'; const pr = await prWorker.ensurePr(config); expect(pr).toMatchObject({ displayNumber: 'New Pull Request' }); + expect(platform.createPr.mock.calls[0]).toMatchSnapshot(); + existingPr.body = platform.createPr.mock.calls[0][2]; + }); + it('should convert to HTML PR for gitlab', async () => { + platform.getBranchStatus.mockReturnValueOnce('success'); + config.prCreation = 'status-success'; + config.platform = 'gitlab'; + const pr = await prWorker.ensurePr(config); + expect(pr).toMatchObject({ displayNumber: 'New Pull Request' }); + expect(platform.createPr.mock.calls[0]).toMatchSnapshot(); + expect(platform.createPr.mock.calls[0][2].indexOf('<p>This MR')).not.toBe( + -1 + ); }); it('should delete branch and return null if creating PR fails', async () => { platform.getBranchStatus.mockReturnValueOnce('success'); @@ -210,10 +219,10 @@ describe('workers/pr', () => { config.warnings = [{}]; const pr = await prWorker.ensurePr(config); expect( - platform.createPr.mock.calls[0][2].indexOf('Errors</h3>') + platform.createPr.mock.calls[0][2].indexOf('### Errors') ).not.toEqual(-1); expect( - platform.createPr.mock.calls[0][2].indexOf('Warnings</h3>') + platform.createPr.mock.calls[0][2].indexOf('### Warnings') ).not.toEqual(-1); expect(pr).toMatchObject({ displayNumber: 'New Pull Request' }); }); @@ -227,15 +236,9 @@ describe('workers/pr', () => { expect(platform.addReviewers.mock.calls.length).toBe(0); }); it('should add assignees and reviewers to existing PR', async () => { - config.depName = 'dummy'; - config.automerge = true; config.assignees = ['bar']; config.reviewers = ['baz']; - config.isGitHub = true; - config.privateRepo = true; - config.currentVersion = '1.0.0'; - config.newVersion = '1.1.0'; - config.repositoryUrl = 'https://github.com/renovateapp/dummy'; + config.automerge = true; platform.getBranchPr.mockReturnValueOnce(existingPr); platform.getBranchStatus.mockReturnValueOnce('failure'); config.semanticPrefix = ''; @@ -247,24 +250,15 @@ describe('workers/pr', () => { expect(pr).toMatchObject(existingPr); }); it('should return unmodified existing PR', async () => { - config.depName = 'dummy'; - config.isGitHub = true; - config.privateRepo = true; - config.currentVersion = '1.0.0'; - config.newVersion = '1.1.0'; - config.repositoryUrl = 'https://github.com/renovateapp/dummy'; platform.getBranchPr.mockReturnValueOnce(existingPr); config.semanticPrefix = ''; const pr = await prWorker.ensurePr(config); expect(platform.updatePr.mock.calls).toMatchSnapshot(); - expect(platform.updatePr.mock.calls.length).toBe(0); + expect(platform.updatePr.mock.calls).toHaveLength(0); expect(pr).toMatchObject(existingPr); }); it('should return modified existing PR', async () => { - config.depName = 'dummy'; - config.currentVersion = '1.0.0'; config.newVersion = '1.2.0'; - config.isGitHub = true; platform.getBranchPr.mockReturnValueOnce(existingPr); const pr = await prWorker.ensurePr(config); expect(pr).toMatchSnapshot();