From 00e7821fcb8b67e34df94debff51a58067e3ec03 Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@keylocation.sg>
Date: Wed, 8 Nov 2017 11:09:26 +0100
Subject: [PATCH] feat: use mostly markdown for pr bodies (#1118)
Existing solution uses HTML bodies for PR descriptions, as that was the easiest way to get consistency between GitHub and GitLab. However, VSTS supports only markdown so we needed to refactor how this is done. Now, GitHub PR bodies uses only minimal HTML (for summary/details) while GitLab PR bodies are converted to HTML using GitHub flavoured markdown for maximum compatibility. VSTS will be able to strip out the minimal markdown.
Closes #1018
---
lib/platform/gitlab/index.js | 18 +++---
lib/workers/pr/index.js | 31 ++++-----
.../pr/__snapshots__/index.spec.js.snap | 62 ++++++++++++++++--
test/workers/pr/index.spec.js | 64 +++++++++----------
4 files changed, 110 insertions(+), 65 deletions(-)
diff --git a/lib/platform/gitlab/index.js b/lib/platform/gitlab/index.js
index a951361ad4..78c7cccd7c 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 c86d96c8ed..42bac0ac8d 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 f4400485f0..91ae1f659f 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 057b42693a..7cf4faa232 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();
--
GitLab