Skip to content

Commit c38177f

Browse files
test(scripts): cover commit-msg footer append behavior
Address review feedback: extract a pure appendTicketFooter() from main() so the footer-writing logic is unit-testable, and add tests for footer append, existing-footer dedup (TICKET/ISSUE, any case), null-ticket skip, branch-start anchoring, and global-regex lastIndex safety. TICKET: WEB-000
1 parent 6c602b7 commit c38177f

2 files changed

Lines changed: 56 additions & 9 deletions

File tree

scripts/prepare-commit-msg.js

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,31 +17,46 @@ const escapedPrefixes = issuePrefixes
1717
.filter((p) => p !== '#')
1818
.map((p) => p.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'));
1919
// Anchored at branch start; case-insensitive so "web-000-desc" matches "WEB-"
20-
const branchRegex = new RegExp(`^(${escapedPrefixes.join('|')})(\\d+)`, 'i');
20+
// null when no prefixes are configured — avoids ^()(\d+) matching bare-digit branches.
21+
const branchRegex =
22+
escapedPrefixes.length > 0 ? new RegExp(`^(${escapedPrefixes.join('|')})(\\d+)`, 'i') : null;
2123
// ex TICKET: WP-1234
2224
const commitRegex = /(ticket|issue):\s(\S+)/gim;
2325

2426
function extractTicket(branch) {
27+
if (!branchRegex) return null;
2528
const found = branch.match(branchRegex);
2629
return found ? found[0].toUpperCase() : null;
2730
}
2831

32+
// Given the existing commit message and a ticket, return the message with a
33+
// TICKET footer appended, or null when no change is needed (no ticket, or the
34+
// message already contains a ticket/issue footer). Pure so it is unit-testable.
35+
function appendTicketFooter(data, ticket) {
36+
if (!ticket) {
37+
return null;
38+
}
39+
// Reset lastIndex: commitRegex is global, so .test() is stateful across calls.
40+
commitRegex.lastIndex = 0;
41+
if (commitRegex.test(data)) {
42+
return null;
43+
}
44+
return `${data}\nTICKET: ${ticket}\n`;
45+
}
46+
2947
async function main() {
3048
const commitMsgFilepath = process.argv[2];
3149
try {
3250
const branch = (await exec(`git branch --show-current`)).stdout.trim();
3351
const ticket = extractTicket(branch);
34-
if (!ticket) {
35-
return;
36-
}
3752
const data = fs.readFileSync(commitMsgFilepath, 'utf8');
38-
// Exit if ticket is already in commit footer
39-
if (data.match(commitRegex)) {
53+
const updated = appendTicketFooter(data, ticket);
54+
if (updated === null) {
4055
return;
4156
}
42-
fs.writeFileSync(commitMsgFilepath, `${data}\nTICKET: ${ticket}\n`);
57+
fs.writeFileSync(commitMsgFilepath, updated);
4358
} catch (e) {}
4459
}
4560

46-
module.exports = { extractTicket };
61+
module.exports = { extractTicket, appendTicketFooter };
4762
if (require.main === module) main();

scripts/tests/prepare-commit-msg.test.js

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
const assert = require('assert');
2-
const { extractTicket } = require('../prepare-commit-msg');
2+
const { extractTicket, appendTicketFooter } = require('../prepare-commit-msg');
33

44
describe('extractTicket', () => {
55
it('extracts uppercase ticket from uppercase branch', () => {
@@ -20,7 +20,39 @@ describe('extractTicket', () => {
2020
assert.strictEqual(extractTicket('feature-no-ticket'), null);
2121
});
2222

23+
it('is anchored at branch start (ignores prefixes mid-name)', () => {
24+
assert.strictEqual(extractTicket('feature/WEB-000-x'), null);
25+
});
26+
27+
it('returns null for prefix without digits (WEB-foo)', () => {
28+
assert.strictEqual(extractTicket('WEB-foo'), null);
29+
});
30+
2331
it('returns null for empty branch', () => {
2432
assert.strictEqual(extractTicket(''), null);
2533
});
2634
});
35+
36+
describe('appendTicketFooter', () => {
37+
it('appends a TICKET footer when none is present', () => {
38+
assert.strictEqual(appendTicketFooter('feat: add thing', 'WEB-000'), 'feat: add thing\nTICKET: WEB-000\n');
39+
});
40+
41+
it('returns null when ticket is null (branch had no match)', () => {
42+
assert.strictEqual(appendTicketFooter('feat: add thing', null), null);
43+
});
44+
45+
it('returns null when a TICKET footer already exists', () => {
46+
assert.strictEqual(appendTicketFooter('feat: add thing\n\nTICKET: WEB-000', 'WEB-000'), null);
47+
});
48+
49+
it('returns null when an ISSUE footer already exists (any case)', () => {
50+
assert.strictEqual(appendTicketFooter('feat: add thing\n\nissue: WEB-111', 'WEB-000'), null);
51+
});
52+
53+
it('is not stateful across repeated calls (global-regex lastIndex safe)', () => {
54+
// Guards against RegExp.lastIndex leaking between .test() invocations.
55+
assert.strictEqual(appendTicketFooter('feat: a', 'WEB-000'), 'feat: a\nTICKET: WEB-000\n');
56+
assert.strictEqual(appendTicketFooter('feat: b', 'WEB-000'), 'feat: b\nTICKET: WEB-000\n');
57+
});
58+
});

0 commit comments

Comments
 (0)