diff --git a/lib/links.js b/lib/links.js
index 9bbbe0a8..d21cd9b1 100644
--- a/lib/links.js
+++ b/lib/links.js
@@ -32,27 +32,27 @@ class LinkParser {
}
getRefsUrlsFromArray(arr) {
- const result = [];
+ const result = new Set();
for (const item of arr) {
const m = item.match(REF_RE);
if (!m) continue;
const ref = m[1];
const url = this.getUrlFromOP(ref);
- if (url) result.push(url);
+ if (url) result.add(url);
}
- return result;
+ return Array.from(result);
}
getPRUrlsFromArray(arr) {
- const result = [];
+ const result = new Set();
for (const item of arr) {
const m = item.match(PR_RE);
if (!m) continue;
const prUrl = m[1];
const url = this.getUrlFromOP(prUrl);
- if (url) result.push(url);
+ if (url) result.add(url);
}
- return result;
+ return Array.from(result);
}
// Do this so we can reliably get the correct url.
diff --git a/lib/metadata_gen.js b/lib/metadata_gen.js
index e2a02d78..3214adb5 100644
--- a/lib/metadata_gen.js
+++ b/lib/metadata_gen.js
@@ -32,7 +32,7 @@ class MetadataGenerator {
const parser = new LinkParser(owner, repo, op);
const fixes = parser.getFixes();
- const refs = parser.getRefs();
+ const refs = parser.getRefs().filter(f => f !== prUrl);
const altPrUrl = parser.getAltPrUrl();
const meta = [];
diff --git a/test/fixtures/data.js b/test/fixtures/data.js
index 8c6f17ed..115c56a9 100644
--- a/test/fixtures/data.js
+++ b/test/fixtures/data.js
@@ -77,6 +77,8 @@ const firstTimerPrivatePR = readJSON('first_timer_pr_with_private_email.json');
const semverMajorPR = readJSON('semver_major_pr.json');
const fixAndRefPR = readJSON('pr_with_fixes_and_refs.json');
const fixCrossPR = readJSON('pr_with_fixes_cross.json');
+const duplicateRefPR = readJSON('pr_with_duplicate_refs.json');
+const selfRefPR = readJSON('pr_with_self_ref.json');
const backportPR = readJSON('pr_with_backport.json');
const conflictingPR = readJSON('conflicting_pr.json');
const emptyProfilePR = readJSON('empty_profile_pr.json');
@@ -137,5 +139,7 @@ module.exports = {
readmeNoCollaboratorE,
readmeUnordered,
closedPR,
- mergedPR
+ mergedPR,
+ selfRefPR,
+ duplicateRefPR
};
diff --git a/test/fixtures/pr_with_duplicate_refs.json b/test/fixtures/pr_with_duplicate_refs.json
new file mode 100644
index 00000000..a38272f5
--- /dev/null
+++ b/test/fixtures/pr_with_duplicate_refs.json
@@ -0,0 +1,20 @@
+{
+ "createdAt": "2017-10-24T11:13:43Z",
+ "url": "https://github.com/nodejs/node/pull/16438",
+ "bodyHTML":
+ "Refs: #16439\nRefs: #16439
",
+ "bodyText": "Refs: https://github.com/nodejs/node/pull/16439\nRefs: https://github.com/nodejs/node/pull/16439",
+ "labels": {
+ "nodes": [
+ {
+ "name": "build"
+ },
+ {
+ "name": "npm"
+ },
+ {
+ "name": "python"
+ }
+ ]
+ }
+}
diff --git a/test/fixtures/pr_with_self_ref.json b/test/fixtures/pr_with_self_ref.json
new file mode 100644
index 00000000..b075aaf5
--- /dev/null
+++ b/test/fixtures/pr_with_self_ref.json
@@ -0,0 +1,20 @@
+{
+ "createdAt": "2017-10-24T11:13:43Z",
+ "url": "https://github.com/nodejs/node/pull/16438",
+ "bodyHTML":
+ "Refs: #16438
",
+ "bodyText": "Refs: https://github.com/nodejs/node/pull/16438",
+ "labels": {
+ "nodes": [
+ {
+ "name": "build"
+ },
+ {
+ "name": "npm"
+ },
+ {
+ "name": "python"
+ }
+ ]
+ }
+}
diff --git a/test/unit/metadata_gen.test.js b/test/unit/metadata_gen.test.js
index d8dbede9..07520c54 100644
--- a/test/unit/metadata_gen.test.js
+++ b/test/unit/metadata_gen.test.js
@@ -3,19 +3,30 @@
const MetadataGenerator = require('../../lib/metadata_gen');
const {
fixAndRefPR,
+ selfRefPR,
+ duplicateRefPR,
fixCrossPR,
backportPR,
allGreenReviewers
} = require('../fixtures/data');
const assert = require('assert');
+
const data = {
owner: 'nodejs',
repo: 'node',
pr: fixAndRefPR,
reviewers: allGreenReviewers
};
-const crossData = Object.assign({}, data, { pr: fixCrossPR });
+const expected = `PR-URL: https://github.com/nodejs/node/pull/16438
+Fixes: https://github.com/nodejs/node/issues/16437
+Refs: https://github.com/nodejs/node/pull/15148
+Reviewed-By: Foo User
+Reviewed-By: Quux User
+Reviewed-By: Baz User
+Reviewed-By: Bar User
+`;
+
const backportArgv = {
argv: {
owner: 'nodejs',
@@ -29,17 +40,31 @@ const backportArgv = {
backport: true
}
};
-
const backportData = Object.assign({}, data, { pr: backportPR }, backportArgv);
+const backportExpected = `PR-URL: https://github.com/nodejs/node/pull/29995
+Backport-PR-URL: https://github.com/nodejs/node/pull/30072
+Fixes: https://github.com/nodejs/build/issues/1961
+Refs: https://github.com/nodejs/node/commit/53ca0b9ae145c430842bf78e553e3b6cbd2823aa#commitcomment-35494896
+`;
-const expected = `PR-URL: https://github.com/nodejs/node/pull/16438
-Fixes: https://github.com/nodejs/node/issues/16437
-Refs: https://github.com/nodejs/node/pull/15148
+const selfRefData = Object.assign({}, data, { pr: selfRefPR });
+const selfRefExpected = `PR-URL: https://github.com/nodejs/node/pull/16438
+Reviewed-By: Foo User
+Reviewed-By: Quux User
+Reviewed-By: Baz User
+Reviewed-By: Bar User
+`;
+
+const duplicateRefData = Object.assign({}, data, { pr: duplicateRefPR });
+const duplicateRefExpected = `PR-URL: https://github.com/nodejs/node/pull/16438
+Refs: https://github.com/nodejs/node/pull/16439
Reviewed-By: Foo User
Reviewed-By: Quux User
Reviewed-By: Baz User
Reviewed-By: Bar User
`;
+
+const crossData = Object.assign({}, data, { pr: fixCrossPR });
const crossExpected = `PR-URL: https://github.com/nodejs/node/pull/16438
Fixes: https://github.com/joyeecheung/node-core-utils/issues/123
Reviewed-By: Foo User
@@ -47,11 +72,7 @@ Reviewed-By: Quux User
Reviewed-By: Baz User
Reviewed-By: Bar User
`;
-const backportExpected = `PR-URL: https://github.com/nodejs/node/pull/29995
-Backport-PR-URL: https://github.com/nodejs/node/pull/30072
-Fixes: https://github.com/nodejs/build/issues/1961
-Refs: https://github.com/nodejs/node/commit/53ca0b9ae145c430842bf78e553e3b6cbd2823aa#commitcomment-35494896
-`;
+
const skipRefsExpected = `PR-URL: https://github.com/nodejs/node/pull/16438
Reviewed-By: Foo User
Reviewed-By: Quux User
@@ -65,6 +86,16 @@ describe('MetadataGenerator', () => {
assert.strictEqual(expected, results);
});
+ it('should prevent duplicate references', () => {
+ const results = new MetadataGenerator(duplicateRefData).getMetadata();
+ assert.strictEqual(duplicateRefExpected, results);
+ });
+
+ it('should prevent self-referential references', () => {
+ const results = new MetadataGenerator(selfRefData).getMetadata();
+ assert.strictEqual(selfRefExpected, results);
+ });
+
it('should handle cross-owner and cross-repo fixes properly', () => {
const results = new MetadataGenerator(crossData).getMetadata();
assert.strictEqual(crossExpected, results);