From db6ed3fd6029a17a7a4531ab3b0a6503dc303566 Mon Sep 17 00:00:00 2001 From: Navdeep Date: Fri, 28 Jun 2024 00:03:00 -0400 Subject: [PATCH 1/7] add prepare-commit-msg git hook --- .husky/prepare-commit-msg | 2 ++ .husky/prepare-commit-msg.js | 30 ++++++++++++++++++++++++++++++ package.json | 2 +- 3 files changed, 33 insertions(+), 1 deletion(-) create mode 100755 .husky/prepare-commit-msg create mode 100755 .husky/prepare-commit-msg.js diff --git a/.husky/prepare-commit-msg b/.husky/prepare-commit-msg new file mode 100755 index 0000000000..5c53a7f324 --- /dev/null +++ b/.husky/prepare-commit-msg @@ -0,0 +1,2 @@ +#!/bin/sh +node .husky/prepare-commit-msg.js $1 \ No newline at end of file diff --git a/.husky/prepare-commit-msg.js b/.husky/prepare-commit-msg.js new file mode 100755 index 0000000000..c5ecec2c67 --- /dev/null +++ b/.husky/prepare-commit-msg.js @@ -0,0 +1,30 @@ +const fs = require("fs"); +const { execSync } = require("child_process"); + +const ticketNumberRegex = /LF-\d+/; +const commitMessageFilePath = process.argv[2]; + +try { + const branchName = execSync("git rev-parse --abbrev-ref HEAD").toString(); + const match = branchName.match(ticketNumberRegex); + if (match) { + const commitMessage = fs.readFileSync(commitMessageFilePath, "utf8"); + appendTicketNumber(commitMessage, match[0]); + } else { + console.warn( + "JIRA ticket number not found in branch name. Proceeding without modification." + ); + } +} catch (error) { + console.error("Error processing commit message:", error); +} + +function appendTicketNumber(commitMessage, ticketNumber) { + if (!commitMessage.match(ticketNumberRegex)) { + fs.writeFileSync(commitMessageFilePath, ticketNumber + " " + commitMessage); + } else { + console.log( + "Commit message already contains a JIRA ticket number. Proceeding without modification." + ); + } +} diff --git a/package.json b/package.json index 87c6bce5a9..07d107f7de 100644 --- a/package.json +++ b/package.json @@ -10,7 +10,7 @@ "lint": "lerna run lint", "start": "lerna run start", "bootstrap": "lerna bootstrap", - "prepare": "husky install", + "prepare": "husky install && chmod u+x .husky/*", "ngrok": "ngrok start --config=./ngrok/ngrok.yml --all", "ngrok:setup": "node ./ngrok/ngrok-setup.js", "ngrok:api": "ngrok start --config=./ngrok/ngrok.yml api", From dc048830e5f8882b49ee97c8165f239ddb5e3b55 Mon Sep 17 00:00:00 2001 From: Navdeep Date: Sat, 6 Jul 2024 10:29:53 -0400 Subject: [PATCH 2/7] convert script to ESM --- .husky/prepare-commit-msg.js | 4 ++-- lint-staged.config.js | 4 ++-- package.json | 1 + 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.husky/prepare-commit-msg.js b/.husky/prepare-commit-msg.js index c5ecec2c67..332d6eefdf 100755 --- a/.husky/prepare-commit-msg.js +++ b/.husky/prepare-commit-msg.js @@ -1,5 +1,5 @@ -const fs = require("fs"); -const { execSync } = require("child_process"); +import fs from "node:fs"; +import { execSync } from "node:child_process"; const ticketNumberRegex = /LF-\d+/; const commitMessageFilePath = process.argv[2]; diff --git a/lint-staged.config.js b/lint-staged.config.js index d7425ba3bb..d5bbe81aa0 100644 --- a/lint-staged.config.js +++ b/lint-staged.config.js @@ -1,3 +1,3 @@ -module.exports = { - '*.{md,yml}': 'prettier --write' +export default { + "*.{md,yml,json,js}": "prettier --write", }; diff --git a/package.json b/package.json index 07d107f7de..a197324667 100644 --- a/package.json +++ b/package.json @@ -16,6 +16,7 @@ "ngrok:api": "ngrok start --config=./ngrok/ngrok.yml api", "ngrok:webapp": "ngrok start --config=./ngrok/ngrok.yml webapp" }, + "type": "module", "jest": { "testPathIgnorePatterns": [ "[/\\\\](node_modules|packages/webapp/scripts)[/\\\\]" From 48f2e51ba53077c654ce86813acc3fd392cf2e5d Mon Sep 17 00:00:00 2001 From: Navdeep Date: Sat, 6 Jul 2024 10:46:16 -0400 Subject: [PATCH 3/7] add tests for prepare-commit-msg hook --- tests/prepare-commit-msg.test.js | 64 ++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 tests/prepare-commit-msg.test.js diff --git a/tests/prepare-commit-msg.test.js b/tests/prepare-commit-msg.test.js new file mode 100644 index 0000000000..255beb98b0 --- /dev/null +++ b/tests/prepare-commit-msg.test.js @@ -0,0 +1,64 @@ +import { test, describe, before, after } from "node:test"; +import assert from "node:assert/strict"; +import { execSync } from "node:child_process"; + +const makeCommit = (commitMsg) => { + execSync(`git commit --allow-empty --no-verify -m "${commitMsg}"`); + return execSync("git log -1 --pretty=%B").toString().trim(); +}; + +describe("prepare-commit-msg - adding JIRA ticket number to commit messages", () => { + const ticketNumber = "LF-1212"; + const currentBranchName = execSync("git rev-parse --abbrev-ref HEAD") + .toString() + .trim(); + const testBranchName = `${ticketNumber}-dummy-branch`; + + let stashed = false; + + before(() => { + // Check if there are uncommitted changes in current branch + if (execSync("git status --porcelain").toString().trim().length > 0) { + execSync("git stash push -u"); + stashed = true; + } + + // Create test branch + execSync(`git checkout -b ${testBranchName}`); + }); + + // Checkout original branch and delete test branch; pop stash if necessary + after(() => { + execSync(`git checkout ${currentBranchName}`); + execSync(`git branch -D ${testBranchName}`); + if (stashed) { + execSync("git stash pop"); + stashed = false; + } + }); + + test("should automatically add ticket number when missing", () => { + const modifiedCommitMsg = makeCommit("random commit message"); + assert(modifiedCommitMsg.startsWith(ticketNumber)); + }); + + test("should not modify commit message if it already contains ticket number", () => { + const testCommit = (msg) => { + const latestCommitMsg = makeCommit(msg); + assert.equal(latestCommitMsg, msg); + }; + testCommit(`${ticketNumber} starting with ticket number`); + testCommit(`with ${ticketNumber} ticket number inside`); + testCommit(`with ticket number at end ${ticketNumber}`); + }); + + test("should not modify commit message if branch name does not contain ticket number", () => { + // Rename the branch to one that does not contain a ticket number + execSync('git branch -m "dummy-branch"'); + const latestCommitMsg = makeCommit("test commit"); + assert.equal(latestCommitMsg, "test commit"); + + // Rename the branch back to the test branch name + execSync(`git branch -m ${testBranchName}`); + }); +}); From d3ae32eac09dfbfec5133fdf9ae37950dd30f91c Mon Sep 17 00:00:00 2001 From: Navdeep Date: Wed, 10 Jul 2024 19:04:02 -0400 Subject: [PATCH 4/7] refactor to use mocks in tests --- .husky/prepare-commit-msg.js | 56 +++++++++++++-------- tests/prepare-commit-msg.test.js | 84 +++++++++++++++----------------- 2 files changed, 75 insertions(+), 65 deletions(-) diff --git a/.husky/prepare-commit-msg.js b/.husky/prepare-commit-msg.js index 332d6eefdf..7cef478213 100755 --- a/.husky/prepare-commit-msg.js +++ b/.husky/prepare-commit-msg.js @@ -1,30 +1,46 @@ import fs from "node:fs"; +import { fileURLToPath } from "node:url"; import { execSync } from "node:child_process"; const ticketNumberRegex = /LF-\d+/; const commitMessageFilePath = process.argv[2]; -try { - const branchName = execSync("git rev-parse --abbrev-ref HEAD").toString(); - const match = branchName.match(ticketNumberRegex); - if (match) { - const commitMessage = fs.readFileSync(commitMessageFilePath, "utf8"); - appendTicketNumber(commitMessage, match[0]); - } else { - console.warn( - "JIRA ticket number not found in branch name. Proceeding without modification." - ); +const git = { + getCurrentBranchName: () => + execSync("git rev-parse --abbrev-ref HEAD").toString().trim(), + readCommitMsg: () => fs.readFileSync(commitMessageFilePath, "utf8"), + writeCommitMsg: (msg) => fs.writeFileSync(commitMessageFilePath, msg), +}; + +function prepareCommitMsg() { + try { + const branchName = git.getCurrentBranchName(); + console.log(branchName); + const match = branchName.match(ticketNumberRegex); + if (!match) { + console.warn( + "JIRA ticket number not found in branch name. Proceeding without modification." + ); + process.exit(); + } + + const commitMessage = git.readCommitMsg(); + + if (!commitMessage.match(ticketNumberRegex)) { + git.writeCommitMsg(match[0] + " " + commitMessage); + } else { + console.log( + "Commit message already contains a JIRA ticket number. Proceeding without modification." + ); + } + } catch (error) { + console.error("Error processing commit message:", error); } -} catch (error) { - console.error("Error processing commit message:", error); } -function appendTicketNumber(commitMessage, ticketNumber) { - if (!commitMessage.match(ticketNumberRegex)) { - fs.writeFileSync(commitMessageFilePath, ticketNumber + " " + commitMessage); - } else { - console.log( - "Commit message already contains a JIRA ticket number. Proceeding without modification." - ); - } +// Call the hook only when this file is executed directly by node. +if (process.argv[1] === fileURLToPath(import.meta.url)) { + prepareCommitMsg(); } + +export { git, prepareCommitMsg }; diff --git a/tests/prepare-commit-msg.test.js b/tests/prepare-commit-msg.test.js index 255beb98b0..fc7d11c236 100644 --- a/tests/prepare-commit-msg.test.js +++ b/tests/prepare-commit-msg.test.js @@ -1,64 +1,58 @@ -import { test, describe, before, after } from "node:test"; +import { test, describe, mock } from "node:test"; import assert from "node:assert/strict"; -import { execSync } from "node:child_process"; +import { prepareCommitMsg, git } from "../.husky/prepare-commit-msg.js"; -const makeCommit = (commitMsg) => { - execSync(`git commit --allow-empty --no-verify -m "${commitMsg}"`); - return execSync("git log -1 --pretty=%B").toString().trim(); +const setUpMocks = ({ commitMsg, branchName }) => { + mock.method(git, "getCurrentBranchName", () => branchName); + mock.method(git, "readCommitMsg", () => commitMsg); + return mock.method(git, "writeCommitMsg", () => {}); }; describe("prepare-commit-msg - adding JIRA ticket number to commit messages", () => { - const ticketNumber = "LF-1212"; - const currentBranchName = execSync("git rev-parse --abbrev-ref HEAD") - .toString() - .trim(); - const testBranchName = `${ticketNumber}-dummy-branch`; + test("should automatically add ticket number when missing", () => { + const mockWriteCommitMsg = setUpMocks({ + branchName: `LF-1212-test-branch`, + commitMsg: "test commit", + }); - let stashed = false; + prepareCommitMsg(); - before(() => { - // Check if there are uncommitted changes in current branch - if (execSync("git status --porcelain").toString().trim().length > 0) { - execSync("git stash push -u"); - stashed = true; - } + // function to modifiy commit msg should be called called once + assert.equal(mockWriteCommitMsg.mock.callCount(), 1); - // Create test branch - execSync(`git checkout -b ${testBranchName}`); + // function to modifiy commit msg should be called called with modified commit message + assert.equal( + mockWriteCommitMsg.mock.calls[0].arguments[0], + "LF-1212 test commit" + ); }); - // Checkout original branch and delete test branch; pop stash if necessary - after(() => { - execSync(`git checkout ${currentBranchName}`); - execSync(`git branch -D ${testBranchName}`); - if (stashed) { - execSync("git stash pop"); - stashed = false; - } - }); + test("should not modify commit message if it already has ticket number in front", (t) => { + const mockWriteCommitMsg = setUpMocks({ + branchName: `LF-1212-test-branch`, + commitMsg: "LF-1212 test commit", + }); + prepareCommitMsg(); - test("should automatically add ticket number when missing", () => { - const modifiedCommitMsg = makeCommit("random commit message"); - assert(modifiedCommitMsg.startsWith(ticketNumber)); + assert.equal(mockWriteCommitMsg.mock.callCount(), 0); }); - test("should not modify commit message if it already contains ticket number", () => { - const testCommit = (msg) => { - const latestCommitMsg = makeCommit(msg); - assert.equal(latestCommitMsg, msg); - }; - testCommit(`${ticketNumber} starting with ticket number`); - testCommit(`with ${ticketNumber} ticket number inside`); - testCommit(`with ticket number at end ${ticketNumber}`); + test("should not modify commit message if it already has ticket number inside", () => { + const mockWriteCommitMsg = setUpMocks({ + branchName: `LF-1212-test-branch`, + commitMsg: "test LF-1212 commit", + }); + prepareCommitMsg(); + + assert.equal(mockWriteCommitMsg.mock.callCount(), 0); }); test("should not modify commit message if branch name does not contain ticket number", () => { - // Rename the branch to one that does not contain a ticket number - execSync('git branch -m "dummy-branch"'); - const latestCommitMsg = makeCommit("test commit"); - assert.equal(latestCommitMsg, "test commit"); + const mockWriteCommitMsg = setUpMocks({ + branchName: `test-branch`, + commitMsg: "test commit", + }); - // Rename the branch back to the test branch name - execSync(`git branch -m ${testBranchName}`); + assert.equal(mockWriteCommitMsg.mock.callCount(), 0); }); }); From 389ddbe09fe3f6de325da5f2efcab196dda22ce4 Mon Sep 17 00:00:00 2001 From: Navdeep Date: Wed, 10 Jul 2024 19:11:13 -0400 Subject: [PATCH 5/7] fix test --- tests/prepare-commit-msg.test.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/prepare-commit-msg.test.js b/tests/prepare-commit-msg.test.js index fc7d11c236..49c9e4718d 100644 --- a/tests/prepare-commit-msg.test.js +++ b/tests/prepare-commit-msg.test.js @@ -32,6 +32,7 @@ describe("prepare-commit-msg - adding JIRA ticket number to commit messages", () branchName: `LF-1212-test-branch`, commitMsg: "LF-1212 test commit", }); + prepareCommitMsg(); assert.equal(mockWriteCommitMsg.mock.callCount(), 0); @@ -42,6 +43,7 @@ describe("prepare-commit-msg - adding JIRA ticket number to commit messages", () branchName: `LF-1212-test-branch`, commitMsg: "test LF-1212 commit", }); + prepareCommitMsg(); assert.equal(mockWriteCommitMsg.mock.callCount(), 0); @@ -53,6 +55,8 @@ describe("prepare-commit-msg - adding JIRA ticket number to commit messages", () commitMsg: "test commit", }); + prepareCommitMsg(); + assert.equal(mockWriteCommitMsg.mock.callCount(), 0); }); }); From 7b490eafc7c5c3b8bb19ee876b0af1aea2cb12fd Mon Sep 17 00:00:00 2001 From: Navdeep Date: Wed, 10 Jul 2024 19:36:14 -0400 Subject: [PATCH 6/7] fix broken test by replacing process.exit with early return --- .husky/prepare-commit-msg.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/.husky/prepare-commit-msg.js b/.husky/prepare-commit-msg.js index 7cef478213..4b3deeb34a 100755 --- a/.husky/prepare-commit-msg.js +++ b/.husky/prepare-commit-msg.js @@ -15,24 +15,28 @@ const git = { function prepareCommitMsg() { try { const branchName = git.getCurrentBranchName(); - console.log(branchName); const match = branchName.match(ticketNumberRegex); + + // Branch name doesn't contain ticket number if (!match) { console.warn( "JIRA ticket number not found in branch name. Proceeding without modification." ); - process.exit(); + return; } const commitMessage = git.readCommitMsg(); - if (!commitMessage.match(ticketNumberRegex)) { - git.writeCommitMsg(match[0] + " " + commitMessage); - } else { + // Commit message already contains ticket number + if (commitMessage.match(ticketNumberRegex)) { console.log( "Commit message already contains a JIRA ticket number. Proceeding without modification." ); + return; } + + // Modify commit message + git.writeCommitMsg(match[0] + " " + commitMessage); } catch (error) { console.error("Error processing commit message:", error); } From f1d9eb8f50399ef19b0728e5580903ce6ff421ec Mon Sep 17 00:00:00 2001 From: Navdeep Date: Wed, 31 Jul 2024 12:37:37 -0400 Subject: [PATCH 7/7] change hook to commit-msg and filter comments when processing commit --- .husky/commit-msg | 2 + .../{prepare-commit-msg.js => commit-msg.js} | 21 +++++--- .husky/prepare-commit-msg | 2 - tests/prepare-commit-msg.test.js | 54 +++++++++++++++---- 4 files changed, 60 insertions(+), 19 deletions(-) create mode 100755 .husky/commit-msg rename .husky/{prepare-commit-msg.js => commit-msg.js} (70%) delete mode 100755 .husky/prepare-commit-msg diff --git a/.husky/commit-msg b/.husky/commit-msg new file mode 100755 index 0000000000..7fa70086f3 --- /dev/null +++ b/.husky/commit-msg @@ -0,0 +1,2 @@ +#!/bin/sh +node .husky/commit-msg.js $1 \ No newline at end of file diff --git a/.husky/prepare-commit-msg.js b/.husky/commit-msg.js similarity index 70% rename from .husky/prepare-commit-msg.js rename to .husky/commit-msg.js index 4b3deeb34a..9f8027190e 100755 --- a/.husky/prepare-commit-msg.js +++ b/.husky/commit-msg.js @@ -12,7 +12,7 @@ const git = { writeCommitMsg: (msg) => fs.writeFileSync(commitMessageFilePath, msg), }; -function prepareCommitMsg() { +function processCommitMsg() { try { const branchName = git.getCurrentBranchName(); const match = branchName.match(ticketNumberRegex); @@ -27,8 +27,16 @@ function prepareCommitMsg() { const commitMessage = git.readCommitMsg(); - // Commit message already contains ticket number - if (commitMessage.match(ticketNumberRegex)) { + const filteredCommitMessage = commitMessage + .split("\n") + .filter((line) => !line.trim().startsWith("#")) + .join("\n") + .trim(); + + if (filteredCommitMessage === "") return; + + // Check if commit already contains ticket number + if (filteredCommitMessage.match(ticketNumberRegex)) { console.log( "Commit message already contains a JIRA ticket number. Proceeding without modification." ); @@ -36,15 +44,16 @@ function prepareCommitMsg() { } // Modify commit message - git.writeCommitMsg(match[0] + " " + commitMessage); + git.writeCommitMsg(match[0] + " " + filteredCommitMessage); } catch (error) { console.error("Error processing commit message:", error); } } // Call the hook only when this file is executed directly by node. +// This is needed to ensure proper execution of tests. if (process.argv[1] === fileURLToPath(import.meta.url)) { - prepareCommitMsg(); + processCommitMsg(); } -export { git, prepareCommitMsg }; +export { git, processCommitMsg }; diff --git a/.husky/prepare-commit-msg b/.husky/prepare-commit-msg deleted file mode 100755 index 5c53a7f324..0000000000 --- a/.husky/prepare-commit-msg +++ /dev/null @@ -1,2 +0,0 @@ -#!/bin/sh -node .husky/prepare-commit-msg.js $1 \ No newline at end of file diff --git a/tests/prepare-commit-msg.test.js b/tests/prepare-commit-msg.test.js index 49c9e4718d..dc549ccbc6 100644 --- a/tests/prepare-commit-msg.test.js +++ b/tests/prepare-commit-msg.test.js @@ -1,8 +1,8 @@ import { test, describe, mock } from "node:test"; import assert from "node:assert/strict"; -import { prepareCommitMsg, git } from "../.husky/prepare-commit-msg.js"; +import { processCommitMsg, git } from "../.husky/commit-msg.js"; -const setUpMocks = ({ commitMsg, branchName }) => { +const setupMocks = ({ commitMsg, branchName }) => { mock.method(git, "getCurrentBranchName", () => branchName); mock.method(git, "readCommitMsg", () => commitMsg); return mock.method(git, "writeCommitMsg", () => {}); @@ -10,52 +10,84 @@ const setUpMocks = ({ commitMsg, branchName }) => { describe("prepare-commit-msg - adding JIRA ticket number to commit messages", () => { test("should automatically add ticket number when missing", () => { - const mockWriteCommitMsg = setUpMocks({ + const mockWriteCommitMsg = setupMocks({ branchName: `LF-1212-test-branch`, commitMsg: "test commit", }); - prepareCommitMsg(); + processCommitMsg(); // function to modifiy commit msg should be called called once assert.equal(mockWriteCommitMsg.mock.callCount(), 1); - // function to modifiy commit msg should be called called with modified commit message + // function to modifiy commit msg should be called with modified commit message assert.equal( mockWriteCommitMsg.mock.calls[0].arguments[0], "LF-1212 test commit" ); }); + test("should filter out comments starting with #", () => { + const mockWriteCommitMsg = setupMocks({ + branchName: `LF-1212-test-branch`, + commitMsg: `test commit + # Please enter the commit message for your changes. Lines starting + # with '#' will be ignored, and an empty message aborts the commit. + # + # On branch LF-1212-test-branch + # + `, + }); + + processCommitMsg(); + + assert.equal(mockWriteCommitMsg.mock.callCount(), 1); + assert.equal( + mockWriteCommitMsg.mock.calls[0].arguments[0], + "LF-1212 test commit" + ); + }); + + test("should not modify commit message if it is empty", (t) => { + const mockWriteCommitMsg = setupMocks({ + branchName: `LF-1212-test-branch`, + commitMsg: "", + }); + + processCommitMsg(); + + assert.equal(mockWriteCommitMsg.mock.callCount(), 0); + }); + test("should not modify commit message if it already has ticket number in front", (t) => { - const mockWriteCommitMsg = setUpMocks({ + const mockWriteCommitMsg = setupMocks({ branchName: `LF-1212-test-branch`, commitMsg: "LF-1212 test commit", }); - prepareCommitMsg(); + processCommitMsg(); assert.equal(mockWriteCommitMsg.mock.callCount(), 0); }); test("should not modify commit message if it already has ticket number inside", () => { - const mockWriteCommitMsg = setUpMocks({ + const mockWriteCommitMsg = setupMocks({ branchName: `LF-1212-test-branch`, commitMsg: "test LF-1212 commit", }); - prepareCommitMsg(); + processCommitMsg(); assert.equal(mockWriteCommitMsg.mock.callCount(), 0); }); test("should not modify commit message if branch name does not contain ticket number", () => { - const mockWriteCommitMsg = setUpMocks({ + const mockWriteCommitMsg = setupMocks({ branchName: `test-branch`, commitMsg: "test commit", }); - prepareCommitMsg(); + processCommitMsg(); assert.equal(mockWriteCommitMsg.mock.callCount(), 0); });