Update: commit-message enforces issue ref style (fixes #56) (#70)

eslint · Aug 5, 2018 · 9de0b7b · 9de0b7b
1 parent 7bf5291
commit 9de0b7b
Show file tree

Hide file tree

Showing 2 changed files with 90 additions and 1 deletion.
diff --git a/src/plugins/commit-message/index.js b/src/plugins/commit-message/index.js
@@ -9,6 +9,12 @@
 const { getCommitMessageForPR } = require("../utils");
 
 const TAG_REGEX = /^((?:Breaking|Build|Chore|Docs|Fix|New|Update|Upgrade):|Revert )/;
+
+const POTENTIAL_ISSUE_REF_REGEX = /#\d+/;
+
+const VALID_ISSUE_REF = "(?:(?:fixes|refs) #\\d+)";
+const CORRECT_ISSUE_REF_REGEX = new RegExp(` \\(${VALID_ISSUE_REF}(?:, ${VALID_ISSUE_REF})*\\)$`);
+
 const MESSAGE_LENGTH_LIMIT = 72;
 
 const EXCLUDED_REPOSITORY_NAMES = new Set([
@@ -23,7 +29,21 @@ const EXCLUDED_REPOSITORY_NAMES = new Set([
  * @private
  */
 function checkCommitMessage(message) {
-    return TAG_REGEX.test(message) && message.split(/\r?\n/)[0].length <= MESSAGE_LENGTH_LIMIT;
+
+    // First, check tag and summary length
+    let isValid = TAG_REGEX.test(message) && message.split(/\r?\n/)[0].length <= MESSAGE_LENGTH_LIMIT;
+
+    // Then, if there appears to be an issue reference, test for correctness
+    if (isValid && POTENTIAL_ISSUE_REF_REGEX.test(message)) {
+        const issueSuffixMatch = CORRECT_ISSUE_REF_REGEX.exec(message);
+
+        // If no suffix, or issue ref occurs before suffix, message is invalid
+        if (!issueSuffixMatch || POTENTIAL_ISSUE_REF_REGEX.test(message.slice(0, issueSuffixMatch.index))) {
+            isValid = false;
+        }
+    }
+
+    return isValid;
 }
 
 /**

diff --git a/tests/plugins/commit-message/index.js b/tests/plugins/commit-message/index.js
@@ -190,6 +190,75 @@ describe("commit-message", () => {
                 expect(nockScope.isDone()).toBeTruthy();
             });
 
+            // Tests for malformed issue references
+            [
+                "#1",
+                "fixes #1",
+                "(fix #1)",
+                "(ref #1)",
+                "(closes #1)",
+                "(fixes #1, #2)",
+
+                // Unexpected issue number with valid suffix should fail
+                "#1 (fixes #1)"
+            ].forEach(suffix => {
+                test(`Posts failure status if the commit message references issue improperly: ${suffix}`, async() => {
+                    mockSingleCommitWithMessage(
+                        `New: foo ${suffix}`
+                    );
+
+                    const nockScope = nock("https://api.github.com")
+                        .post("/repos/test/repo-test/statuses/first-sha", req => req.state === "failure")
+                        .reply(201);
+
+                    await emitBotEvent(bot, { action });
+                    expect(nockScope.isDone()).toBeTruthy();
+                });
+
+                test(`Posts failure status if multiple commits and PR title references issue improperly: ${suffix}`, async() => {
+                    mockMultipleCommits();
+
+                    const nockScope = nock("https://api.github.com")
+                        .post("/repos/test/repo-test/statuses/second-sha", req => req.state === "failure")
+                        .reply(201);
+
+                    await emitBotEvent(bot, { action, pull_request: { number: 1, title: `New: foo ${suffix}` } });
+                    expect(nockScope.isDone()).toBeTruthy();
+                });
+            });
+
+            // Tests for correct issue references
+            [
+                "(fixes #1)",
+                "(refs #1)",
+                "(fixes #1, fixes #2)",
+                "(fixes #1, refs #2, fixes #3)"
+            ].forEach(suffix => {
+                test(`Posts success status if the commit message references issue correctly: ${suffix}`, async() => {
+                    mockSingleCommitWithMessage(
+                        `New: foo ${suffix}`
+                    );
+
+                    const nockScope = nock("https://api.github.com")
+                        .post("/repos/test/repo-test/statuses/first-sha", req => req.state === "success")
+                        .reply(201);
+
+                    await emitBotEvent(bot, { action });
+                    expect(nockScope.isDone()).toBeTruthy();
+                });
+
+                test(`Posts success status if multiple commits and PR title references issue correctly: ${suffix}`, async() => {
+                    mockMultipleCommits();
+
+                    const nockScope = nock("https://api.github.com")
+                        .post("/repos/test/repo-test/statuses/second-sha", req => req.state === "success")
+                        .reply(201);
+
+                    await emitBotEvent(bot, { action, pull_request: { number: 1, title: `New: foo ${suffix}` } });
+                    expect(nockScope.isDone()).toBeTruthy();
+                });
+            });
+
             test("Does not post a status if the repository is excluded", async() => {
                 await emitBotEvent(bot, {
                     action: "opened",