From 983773a0e3c51cb87e315ca66728c85be5ba68fa Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 27 Jan 2026 13:47:47 +0000 Subject: [PATCH 01/13] Initial plan From 2f7e57ba9ec42ce5518c16135afef60423d8391f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 27 Jan 2026 13:50:20 +0000 Subject: [PATCH 02/13] Add legal boilerplate validation for external contributors Co-authored-by: vaind <6349682+vaind@users.noreply.github.com> --- danger/README.md | 1 + danger/dangerfile.js | 131 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 132 insertions(+) diff --git a/danger/README.md b/danger/README.md index e979ab4..d904254 100644 --- a/danger/README.md +++ b/danger/README.md @@ -61,6 +61,7 @@ The Danger action runs the following checks: - **Action pinning**: Verifies GitHub Actions are pinned to specific commits for security - **Conventional commits**: Validates commit message format and PR title conventions - **Cross-repo links**: Checks for proper formatting of links in changelog entries +- **Legal boilerplate validation**: For external contributors (non-organization members), verifies the presence of required legal notices in PR descriptions when the repository's PR template includes a "Legal Boilerplate" section For detailed rule implementations, see [dangerfile.js](dangerfile.js). diff --git a/danger/dangerfile.js b/danger/dangerfile.js index d5feaa4..5eb2fc6 100644 --- a/danger/dangerfile.js +++ b/danger/dangerfile.js @@ -186,6 +186,136 @@ async function checkActionsArePinned() { } } +async function checkLegalBoilerplate() { + console.log('::debug:: Checking legal boilerplate requirements...'); + + // Check if the PR author is an external contributor + const prAuthor = danger.github.pr.user.login; + const repoOwner = danger.github.pr.base.repo.owner.login; + + // Check if author is a member of the getsentry organization + let isExternalContributor = false; + try { + // Check organization membership + await danger.github.api.orgs.checkMembershipForUser({ + org: repoOwner, + username: prAuthor + }); + console.log(`::debug:: ${prAuthor} is a member of ${repoOwner} organization`); + isExternalContributor = false; + } catch (error) { + // If the API call fails with 404, the user is not a member + if (error.status === 404) { + console.log(`::debug:: ${prAuthor} is NOT a member of ${repoOwner} organization`); + isExternalContributor = true; + } else { + // For other errors (like permission issues), log and skip the check + console.log(`::warning:: Could not check organization membership for ${prAuthor}: ${error.message}`); + return; + } + } + + // If not an external contributor, skip the check + if (!isExternalContributor) { + console.log('::debug:: Skipping legal boilerplate check for organization member'); + return; + } + + // Check if the PR template contains a legal boilerplate section + let prTemplateContent = null; + const possibleTemplatePaths = [ + '.github/PULL_REQUEST_TEMPLATE.md', + '.github/pull_request_template.md', + 'PULL_REQUEST_TEMPLATE.md', + 'pull_request_template.md', + '.github/PULL_REQUEST_TEMPLATE/pull_request_template.md' + ]; + + for (const templatePath of possibleTemplatePaths) { + try { + prTemplateContent = await danger.github.utils.fileContents(templatePath); + console.log(`::debug:: Found PR template at ${templatePath}`); + break; + } catch (error) { + // Template not found at this path, try next + continue; + } + } + + // If no template found, skip the check + if (!prTemplateContent) { + console.log('::debug:: No PR template found, skipping legal boilerplate check'); + return; + } + + // Check if the template contains a legal boilerplate section + // Look for headers like "### Legal Boilerplate", "## Legal Boilerplate", etc. + const legalBoilerplateHeaderRegex = /^#{1,6}\s+Legal\s+Boilerplate/im; + if (!legalBoilerplateHeaderRegex.test(prTemplateContent)) { + console.log('::debug:: PR template does not contain a Legal Boilerplate section'); + return; + } + + console.log('::debug:: PR template contains Legal Boilerplate section, checking PR description...'); + + // Check if the PR description contains the legal boilerplate + const prBody = danger.github.pr.body || ''; + + // Look for the legal boilerplate header in the PR description + if (!legalBoilerplateHeaderRegex.test(prBody)) { + fail( + 'This PR is missing the required legal boilerplate. As an external contributor, please include the "Legal Boilerplate" section from the PR template in your PR description.', + undefined, + undefined + ); + + markdown(` +### ⚖️ Legal Boilerplate Required + +As an external contributor, your PR must include the legal boilerplate from the PR template. + +Please add the following section to your PR description: + +${extractLegalBoilerplateSection(prTemplateContent)} + +This is required to ensure proper intellectual property rights for your contributions. + `.trim()); + return; + } + + console.log('::debug:: Legal boilerplate found in PR description ✓'); +} + +/// Extract the legal boilerplate section from the PR template +function extractLegalBoilerplateSection(templateContent) { + // Find the legal boilerplate section and extract it + const lines = templateContent.split('\n'); + let inLegalSection = false; + let legalSection = []; + + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + + // Check if this line is the legal boilerplate header + if (/^#{1,6}\s+Legal\s+Boilerplate/i.test(line)) { + inLegalSection = true; + legalSection.push(line); + continue; + } + + // If we're in the legal section + if (inLegalSection) { + // Check if we've reached another header (end of legal section) + if (/^#{1,6}\s+/.test(line)) { + break; + } + legalSection.push(line); + } + } + + return legalSection.join('\n').trim(); +} + async function checkFromExternalChecks() { // Get the external dangerfile path from environment variable (passed via workflow input) // Priority: EXTRA_DANGERFILE (absolute path) -> EXTRA_DANGERFILE_INPUT (relative path) @@ -231,6 +361,7 @@ async function checkAll() { await checkDocs(); await checkChangelog(); await checkActionsArePinned(); + await checkLegalBoilerplate(); await checkFromExternalChecks(); } From 294196e5e82d83148acec715b611f8fdf36568aa Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 27 Jan 2026 13:52:18 +0000 Subject: [PATCH 03/13] Address code review feedback: improve comments, docs, and security Co-authored-by: vaind <6349682+vaind@users.noreply.github.com> --- danger/dangerfile.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/danger/dangerfile.js b/danger/dangerfile.js index 5eb2fc6..1693cda 100644 --- a/danger/dangerfile.js +++ b/danger/dangerfile.js @@ -193,7 +193,7 @@ async function checkLegalBoilerplate() { const prAuthor = danger.github.pr.user.login; const repoOwner = danger.github.pr.base.repo.owner.login; - // Check if author is a member of the getsentry organization + // Check if author is a member of the repository's organization let isExternalContributor = false; try { // Check organization membership @@ -269,6 +269,7 @@ async function checkLegalBoilerplate() { undefined ); + const legalBoilerplateContent = extractLegalBoilerplateSection(prTemplateContent); markdown(` ### ⚖️ Legal Boilerplate Required @@ -276,7 +277,9 @@ As an external contributor, your PR must include the legal boilerplate from the Please add the following section to your PR description: -${extractLegalBoilerplateSection(prTemplateContent)} +\`\`\`markdown +${legalBoilerplateContent} +\`\`\` This is required to ensure proper intellectual property rights for your contributions. `.trim()); @@ -286,7 +289,11 @@ This is required to ensure proper intellectual property rights for your contribu console.log('::debug:: Legal boilerplate found in PR description ✓'); } -/// Extract the legal boilerplate section from the PR template +/** + * Extract the legal boilerplate section from the PR template + * @param {string} templateContent - The PR template content + * @returns {string} The extracted legal boilerplate section + */ function extractLegalBoilerplateSection(templateContent) { // Find the legal boilerplate section and extract it const lines = templateContent.split('\n'); From 24ffc9f8125886ad379442c7deac655c76626417 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 27 Jan 2026 19:07:22 +0000 Subject: [PATCH 04/13] Add comprehensive tests for legal boilerplate validation Co-authored-by: vaind <6349682+vaind@users.noreply.github.com> --- danger/dangerfile-legal-boilerplate.test.js | 251 ++++++++++++++++++++ danger/dangerfile-utils.js | 37 ++- danger/dangerfile.js | 36 +-- 3 files changed, 288 insertions(+), 36 deletions(-) create mode 100644 danger/dangerfile-legal-boilerplate.test.js diff --git a/danger/dangerfile-legal-boilerplate.test.js b/danger/dangerfile-legal-boilerplate.test.js new file mode 100644 index 0000000..39f1f01 --- /dev/null +++ b/danger/dangerfile-legal-boilerplate.test.js @@ -0,0 +1,251 @@ +const { describe, it } = require('node:test'); +const assert = require('node:assert'); +const { extractLegalBoilerplateSection } = require('./dangerfile-utils.js'); + +describe('Legal Boilerplate Validation', () => { + describe('extractLegalBoilerplateSection', () => { + it('should extract legal boilerplate section with ### header', () => { + const template = ` +# Pull Request Template + +## Description +Please describe your changes + +### Legal Boilerplate +Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms. + +## Checklist +- [ ] Tests added +`; + + const result = extractLegalBoilerplateSection(template); + + assert.ok(result.includes('### Legal Boilerplate'), 'Should include the header'); + assert.ok(result.includes('Functional Software, Inc.'), 'Should include the legal text'); + assert.ok(!result.includes('## Checklist'), 'Should not include the next section'); + }); + + it('should extract legal boilerplate section with ## header', () => { + const template = ` +# Pull Request Template + +## Legal Boilerplate + +This is a legal notice. + +## Other Section +More content +`; + + const result = extractLegalBoilerplateSection(template); + + assert.strictEqual(result.trim(), '## Legal Boilerplate\n\nThis is a legal notice.'); + }); + + it('should extract legal boilerplate section with different heading levels', () => { + const testCases = [ + { header: '# Legal Boilerplate', text: 'Level 1 header' }, + { header: '## Legal Boilerplate', text: 'Level 2 header' }, + { header: '### Legal Boilerplate', text: 'Level 3 header' }, + { header: '#### Legal Boilerplate', text: 'Level 4 header' }, + { header: '##### Legal Boilerplate', text: 'Level 5 header' }, + { header: '###### Legal Boilerplate', text: 'Level 6 header' } + ]; + + testCases.forEach(({ header, text }) => { + const template = `${header}\n${text}\n## Next Section`; + const result = extractLegalBoilerplateSection(template); + + assert.ok(result.includes(header), `Should extract section with ${header}`); + assert.ok(result.includes(text), `Should include text for ${header}`); + assert.ok(!result.includes('Next Section'), `Should not include next section for ${header}`); + }); + }); + + it('should handle case-insensitive matching', () => { + const templates = [ + '### Legal Boilerplate\nContent', + '### legal boilerplate\nContent', + '### LEGAL BOILERPLATE\nContent', + '### Legal BOILERPLATE\nContent' + ]; + + templates.forEach(template => { + const result = extractLegalBoilerplateSection(template); + assert.ok(result.length > 0, `Should extract from: ${template.split('\n')[0]}`); + assert.ok(result.includes('Content'), `Should include content from: ${template.split('\n')[0]}`); + }); + }); + + it('should handle legal boilerplate with multiple paragraphs', () => { + const template = ` +### Legal Boilerplate + +First paragraph of legal text. + +Second paragraph of legal text. + +Third paragraph of legal text. + +## Next Section +`; + + const result = extractLegalBoilerplateSection(template); + + assert.ok(result.includes('First paragraph'), 'Should include first paragraph'); + assert.ok(result.includes('Second paragraph'), 'Should include second paragraph'); + assert.ok(result.includes('Third paragraph'), 'Should include third paragraph'); + assert.ok(!result.includes('Next Section'), 'Should not include next section'); + }); + + it('should handle legal boilerplate at end of template', () => { + const template = ` +# PR Template + +## Description +Content + +### Legal Boilerplate +Legal text at the end. +`; + + const result = extractLegalBoilerplateSection(template); + + assert.ok(result.includes('### Legal Boilerplate'), 'Should include header'); + assert.ok(result.includes('Legal text at the end.'), 'Should include text'); + }); + + it('should return empty string when no legal boilerplate section exists', () => { + const template = ` +# Pull Request Template + +## Description +Please describe your changes + +## Checklist +- [ ] Tests added +`; + + const result = extractLegalBoilerplateSection(template); + + assert.strictEqual(result, '', 'Should return empty string when no legal section found'); + }); + + it('should handle empty template', () => { + const result = extractLegalBoilerplateSection(''); + assert.strictEqual(result, '', 'Should return empty string for empty template'); + }); + + it('should handle template with only legal boilerplate section', () => { + const template = '### Legal Boilerplate\nThis is the only content.'; + const result = extractLegalBoilerplateSection(template); + + assert.ok(result.includes('### Legal Boilerplate'), 'Should include header'); + assert.ok(result.includes('This is the only content.'), 'Should include content'); + }); + + it('should handle legal boilerplate with special characters', () => { + const template = ` +### Legal Boilerplate +Text with special chars: @#$%^&*()_+-={}[]|\\:";'<>?,./ +And some unicode: 你好世界 🎉 + +## Next +`; + + const result = extractLegalBoilerplateSection(template); + + assert.ok(result.includes('special chars'), 'Should handle special characters'); + assert.ok(result.includes('你好世界'), 'Should handle unicode'); + assert.ok(result.includes('🎉'), 'Should handle emoji'); + }); + + it('should handle legal boilerplate with code blocks', () => { + const template = ` +### Legal Boilerplate + +Some text with code: + +\`\`\`javascript +const legal = true; +\`\`\` + +More text. + +## Next Section +`; + + const result = extractLegalBoilerplateSection(template); + + assert.ok(result.includes('const legal = true;'), 'Should include code blocks'); + assert.ok(result.includes('More text.'), 'Should include text after code block'); + assert.ok(!result.includes('Next Section'), 'Should not include next section'); + }); + + it('should handle legal boilerplate with lists', () => { + const template = ` +### Legal Boilerplate + +You agree to: +- Item 1 +- Item 2 +- Item 3 + +## Other +`; + + const result = extractLegalBoilerplateSection(template); + + assert.ok(result.includes('- Item 1'), 'Should include list items'); + assert.ok(result.includes('- Item 2'), 'Should include list items'); + assert.ok(result.includes('- Item 3'), 'Should include list items'); + }); + + it('should handle legal boilerplate with extra whitespace', () => { + const template = ` +### Legal Boilerplate +Content with spaces. +## Next +`; + + const result = extractLegalBoilerplateSection(template); + + assert.ok(result.includes('Content with spaces.'), 'Should handle extra whitespace in header'); + }); + + it('should stop at first subsequent header', () => { + const template = ` +### Legal Boilerplate +First section content. +### Another Legal Boilerplate +This should not be included. +`; + + const result = extractLegalBoilerplateSection(template); + + assert.ok(result.includes('First section content.'), 'Should include first section'); + assert.ok(!result.includes('This should not be included.'), 'Should stop at next header'); + }); + + it('should handle blank lines within legal section', () => { + const template = ` +### Legal Boilerplate + +First paragraph. + + +Second paragraph with blank lines above. + +## Next +`; + + const result = extractLegalBoilerplateSection(template); + + assert.ok(result.includes('First paragraph.'), 'Should include first paragraph'); + assert.ok(result.includes('Second paragraph'), 'Should include second paragraph'); + // Should preserve blank lines + const blankLineCount = (result.match(/\n\n/g) || []).length; + assert.ok(blankLineCount >= 1, 'Should preserve blank lines'); + }); + }); +}); diff --git a/danger/dangerfile-utils.js b/danger/dangerfile-utils.js index daaed77..bbdba27 100644 --- a/danger/dangerfile-utils.js +++ b/danger/dangerfile-utils.js @@ -86,8 +86,43 @@ function extractPRFlavor(prTitle, prBranchRef) { return ""; } +/** + * Extract the legal boilerplate section from the PR template + * @param {string} templateContent - The PR template content + * @returns {string} The extracted legal boilerplate section + */ +function extractLegalBoilerplateSection(templateContent) { + // Find the legal boilerplate section and extract it + const lines = templateContent.split('\n'); + let inLegalSection = false; + let legalSection = []; + + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + + // Check if this line is the legal boilerplate header + if (/^#{1,6}\s+Legal\s+Boilerplate/i.test(line)) { + inLegalSection = true; + legalSection.push(line); + continue; + } + + // If we're in the legal section + if (inLegalSection) { + // Check if we've reached another header (end of legal section) + if (/^#{1,6}\s+/.test(line)) { + break; + } + legalSection.push(line); + } + } + + return legalSection.join('\n').trim(); +} + module.exports = { FLAVOR_CONFIG, getFlavorConfig, - extractPRFlavor + extractPRFlavor, + extractLegalBoilerplateSection }; diff --git a/danger/dangerfile.js b/danger/dangerfile.js index 1693cda..4dbb688 100644 --- a/danger/dangerfile.js +++ b/danger/dangerfile.js @@ -1,4 +1,4 @@ -const { getFlavorConfig, extractPRFlavor } = require('./dangerfile-utils.js'); +const { getFlavorConfig, extractPRFlavor, extractLegalBoilerplateSection } = require('./dangerfile-utils.js'); const headRepoName = danger.github.pr.head.repo.git_url; const baseRepoName = danger.github.pr.base.repo.git_url; @@ -289,40 +289,6 @@ This is required to ensure proper intellectual property rights for your contribu console.log('::debug:: Legal boilerplate found in PR description ✓'); } -/** - * Extract the legal boilerplate section from the PR template - * @param {string} templateContent - The PR template content - * @returns {string} The extracted legal boilerplate section - */ -function extractLegalBoilerplateSection(templateContent) { - // Find the legal boilerplate section and extract it - const lines = templateContent.split('\n'); - let inLegalSection = false; - let legalSection = []; - - for (let i = 0; i < lines.length; i++) { - const line = lines[i]; - - // Check if this line is the legal boilerplate header - if (/^#{1,6}\s+Legal\s+Boilerplate/i.test(line)) { - inLegalSection = true; - legalSection.push(line); - continue; - } - - // If we're in the legal section - if (inLegalSection) { - // Check if we've reached another header (end of legal section) - if (/^#{1,6}\s+/.test(line)) { - break; - } - legalSection.push(line); - } - } - - return legalSection.join('\n').trim(); -} - async function checkFromExternalChecks() { // Get the external dangerfile path from environment variable (passed via workflow input) // Priority: EXTRA_DANGERFILE (absolute path) -> EXTRA_DANGERFILE_INPUT (relative path) From 71f5985240b2d027440f5a2206f671b1d96f8202 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 29 Jan 2026 08:26:00 +0000 Subject: [PATCH 05/13] Fix bugs in legal boilerplate validation per code review - Fix fileContents bug: check for non-empty string instead of catching exceptions - Use author_association field instead of org membership API for reliability - Validate boilerplate content (min 50 chars) not just header presence - Merge tests into dangerfile-utils.test.js following existing patterns - Remove unnecessary undefined arguments from fail() calls All 35 tests pass (20 existing + 15 legal boilerplate tests) Co-authored-by: vaind <6349682+vaind@users.noreply.github.com> --- danger/dangerfile-legal-boilerplate.test.js | 251 -------------------- danger/dangerfile-utils.test.js | 248 ++++++++++++++++++- danger/dangerfile.js | 75 +++--- 3 files changed, 287 insertions(+), 287 deletions(-) delete mode 100644 danger/dangerfile-legal-boilerplate.test.js diff --git a/danger/dangerfile-legal-boilerplate.test.js b/danger/dangerfile-legal-boilerplate.test.js deleted file mode 100644 index 39f1f01..0000000 --- a/danger/dangerfile-legal-boilerplate.test.js +++ /dev/null @@ -1,251 +0,0 @@ -const { describe, it } = require('node:test'); -const assert = require('node:assert'); -const { extractLegalBoilerplateSection } = require('./dangerfile-utils.js'); - -describe('Legal Boilerplate Validation', () => { - describe('extractLegalBoilerplateSection', () => { - it('should extract legal boilerplate section with ### header', () => { - const template = ` -# Pull Request Template - -## Description -Please describe your changes - -### Legal Boilerplate -Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms. - -## Checklist -- [ ] Tests added -`; - - const result = extractLegalBoilerplateSection(template); - - assert.ok(result.includes('### Legal Boilerplate'), 'Should include the header'); - assert.ok(result.includes('Functional Software, Inc.'), 'Should include the legal text'); - assert.ok(!result.includes('## Checklist'), 'Should not include the next section'); - }); - - it('should extract legal boilerplate section with ## header', () => { - const template = ` -# Pull Request Template - -## Legal Boilerplate - -This is a legal notice. - -## Other Section -More content -`; - - const result = extractLegalBoilerplateSection(template); - - assert.strictEqual(result.trim(), '## Legal Boilerplate\n\nThis is a legal notice.'); - }); - - it('should extract legal boilerplate section with different heading levels', () => { - const testCases = [ - { header: '# Legal Boilerplate', text: 'Level 1 header' }, - { header: '## Legal Boilerplate', text: 'Level 2 header' }, - { header: '### Legal Boilerplate', text: 'Level 3 header' }, - { header: '#### Legal Boilerplate', text: 'Level 4 header' }, - { header: '##### Legal Boilerplate', text: 'Level 5 header' }, - { header: '###### Legal Boilerplate', text: 'Level 6 header' } - ]; - - testCases.forEach(({ header, text }) => { - const template = `${header}\n${text}\n## Next Section`; - const result = extractLegalBoilerplateSection(template); - - assert.ok(result.includes(header), `Should extract section with ${header}`); - assert.ok(result.includes(text), `Should include text for ${header}`); - assert.ok(!result.includes('Next Section'), `Should not include next section for ${header}`); - }); - }); - - it('should handle case-insensitive matching', () => { - const templates = [ - '### Legal Boilerplate\nContent', - '### legal boilerplate\nContent', - '### LEGAL BOILERPLATE\nContent', - '### Legal BOILERPLATE\nContent' - ]; - - templates.forEach(template => { - const result = extractLegalBoilerplateSection(template); - assert.ok(result.length > 0, `Should extract from: ${template.split('\n')[0]}`); - assert.ok(result.includes('Content'), `Should include content from: ${template.split('\n')[0]}`); - }); - }); - - it('should handle legal boilerplate with multiple paragraphs', () => { - const template = ` -### Legal Boilerplate - -First paragraph of legal text. - -Second paragraph of legal text. - -Third paragraph of legal text. - -## Next Section -`; - - const result = extractLegalBoilerplateSection(template); - - assert.ok(result.includes('First paragraph'), 'Should include first paragraph'); - assert.ok(result.includes('Second paragraph'), 'Should include second paragraph'); - assert.ok(result.includes('Third paragraph'), 'Should include third paragraph'); - assert.ok(!result.includes('Next Section'), 'Should not include next section'); - }); - - it('should handle legal boilerplate at end of template', () => { - const template = ` -# PR Template - -## Description -Content - -### Legal Boilerplate -Legal text at the end. -`; - - const result = extractLegalBoilerplateSection(template); - - assert.ok(result.includes('### Legal Boilerplate'), 'Should include header'); - assert.ok(result.includes('Legal text at the end.'), 'Should include text'); - }); - - it('should return empty string when no legal boilerplate section exists', () => { - const template = ` -# Pull Request Template - -## Description -Please describe your changes - -## Checklist -- [ ] Tests added -`; - - const result = extractLegalBoilerplateSection(template); - - assert.strictEqual(result, '', 'Should return empty string when no legal section found'); - }); - - it('should handle empty template', () => { - const result = extractLegalBoilerplateSection(''); - assert.strictEqual(result, '', 'Should return empty string for empty template'); - }); - - it('should handle template with only legal boilerplate section', () => { - const template = '### Legal Boilerplate\nThis is the only content.'; - const result = extractLegalBoilerplateSection(template); - - assert.ok(result.includes('### Legal Boilerplate'), 'Should include header'); - assert.ok(result.includes('This is the only content.'), 'Should include content'); - }); - - it('should handle legal boilerplate with special characters', () => { - const template = ` -### Legal Boilerplate -Text with special chars: @#$%^&*()_+-={}[]|\\:";'<>?,./ -And some unicode: 你好世界 🎉 - -## Next -`; - - const result = extractLegalBoilerplateSection(template); - - assert.ok(result.includes('special chars'), 'Should handle special characters'); - assert.ok(result.includes('你好世界'), 'Should handle unicode'); - assert.ok(result.includes('🎉'), 'Should handle emoji'); - }); - - it('should handle legal boilerplate with code blocks', () => { - const template = ` -### Legal Boilerplate - -Some text with code: - -\`\`\`javascript -const legal = true; -\`\`\` - -More text. - -## Next Section -`; - - const result = extractLegalBoilerplateSection(template); - - assert.ok(result.includes('const legal = true;'), 'Should include code blocks'); - assert.ok(result.includes('More text.'), 'Should include text after code block'); - assert.ok(!result.includes('Next Section'), 'Should not include next section'); - }); - - it('should handle legal boilerplate with lists', () => { - const template = ` -### Legal Boilerplate - -You agree to: -- Item 1 -- Item 2 -- Item 3 - -## Other -`; - - const result = extractLegalBoilerplateSection(template); - - assert.ok(result.includes('- Item 1'), 'Should include list items'); - assert.ok(result.includes('- Item 2'), 'Should include list items'); - assert.ok(result.includes('- Item 3'), 'Should include list items'); - }); - - it('should handle legal boilerplate with extra whitespace', () => { - const template = ` -### Legal Boilerplate -Content with spaces. -## Next -`; - - const result = extractLegalBoilerplateSection(template); - - assert.ok(result.includes('Content with spaces.'), 'Should handle extra whitespace in header'); - }); - - it('should stop at first subsequent header', () => { - const template = ` -### Legal Boilerplate -First section content. -### Another Legal Boilerplate -This should not be included. -`; - - const result = extractLegalBoilerplateSection(template); - - assert.ok(result.includes('First section content.'), 'Should include first section'); - assert.ok(!result.includes('This should not be included.'), 'Should stop at next header'); - }); - - it('should handle blank lines within legal section', () => { - const template = ` -### Legal Boilerplate - -First paragraph. - - -Second paragraph with blank lines above. - -## Next -`; - - const result = extractLegalBoilerplateSection(template); - - assert.ok(result.includes('First paragraph.'), 'Should include first paragraph'); - assert.ok(result.includes('Second paragraph'), 'Should include second paragraph'); - // Should preserve blank lines - const blankLineCount = (result.match(/\n\n/g) || []).length; - assert.ok(blankLineCount >= 1, 'Should preserve blank lines'); - }); - }); -}); diff --git a/danger/dangerfile-utils.test.js b/danger/dangerfile-utils.test.js index cfd1fe1..3abf2e8 100644 --- a/danger/dangerfile-utils.test.js +++ b/danger/dangerfile-utils.test.js @@ -1,6 +1,6 @@ const { describe, it } = require('node:test'); const assert = require('node:assert'); -const { getFlavorConfig, extractPRFlavor, FLAVOR_CONFIG } = require('./dangerfile-utils.js'); +const { getFlavorConfig, extractPRFlavor, extractLegalBoilerplateSection, FLAVOR_CONFIG } = require('./dangerfile-utils.js'); describe('dangerfile-utils', () => { describe('getFlavorConfig', () => { @@ -275,4 +275,250 @@ describe('dangerfile-utils', () => { }); }); }); + + describe('extractLegalBoilerplateSection', () => { + it('should extract legal boilerplate section with ### header', () => { + const template = ` +# Pull Request Template + +## Description +Please describe your changes + +### Legal Boilerplate +Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms. + +## Checklist +- [ ] Tests added +`; + + const result = extractLegalBoilerplateSection(template); + + assert.ok(result.includes('### Legal Boilerplate'), 'Should include the header'); + assert.ok(result.includes('Functional Software, Inc.'), 'Should include the legal text'); + assert.ok(!result.includes('## Checklist'), 'Should not include the next section'); + }); + + it('should extract legal boilerplate section with ## header', () => { + const template = ` +# Pull Request Template + +## Legal Boilerplate + +This is a legal notice. + +## Other Section +More content +`; + + const result = extractLegalBoilerplateSection(template); + + assert.strictEqual(result.trim(), '## Legal Boilerplate\n\nThis is a legal notice.'); + }); + + it('should extract legal boilerplate section with different heading levels', () => { + const testCases = [ + { header: '# Legal Boilerplate', text: 'Level 1 header' }, + { header: '## Legal Boilerplate', text: 'Level 2 header' }, + { header: '### Legal Boilerplate', text: 'Level 3 header' }, + { header: '#### Legal Boilerplate', text: 'Level 4 header' }, + { header: '##### Legal Boilerplate', text: 'Level 5 header' }, + { header: '###### Legal Boilerplate', text: 'Level 6 header' } + ]; + + testCases.forEach(({ header, text }) => { + const template = `${header}\n${text}\n## Next Section`; + const result = extractLegalBoilerplateSection(template); + + assert.ok(result.includes(header), `Should extract section with ${header}`); + assert.ok(result.includes(text), `Should include text for ${header}`); + assert.ok(!result.includes('Next Section'), `Should not include next section for ${header}`); + }); + }); + + it('should handle case-insensitive matching', () => { + const templates = [ + '### Legal Boilerplate\nContent', + '### legal boilerplate\nContent', + '### LEGAL BOILERPLATE\nContent', + '### Legal BOILERPLATE\nContent' + ]; + + templates.forEach(template => { + const result = extractLegalBoilerplateSection(template); + assert.ok(result.length > 0, `Should extract from: ${template.split('\n')[0]}`); + assert.ok(result.includes('Content'), `Should include content from: ${template.split('\n')[0]}`); + }); + }); + + it('should handle legal boilerplate with multiple paragraphs', () => { + const template = ` +### Legal Boilerplate + +First paragraph of legal text. + +Second paragraph of legal text. + +Third paragraph of legal text. + +## Next Section +`; + + const result = extractLegalBoilerplateSection(template); + + assert.ok(result.includes('First paragraph'), 'Should include first paragraph'); + assert.ok(result.includes('Second paragraph'), 'Should include second paragraph'); + assert.ok(result.includes('Third paragraph'), 'Should include third paragraph'); + assert.ok(!result.includes('Next Section'), 'Should not include next section'); + }); + + it('should handle legal boilerplate at end of template', () => { + const template = ` +# PR Template + +## Description +Content + +### Legal Boilerplate +Legal text at the end. +`; + + const result = extractLegalBoilerplateSection(template); + + assert.ok(result.includes('### Legal Boilerplate'), 'Should include header'); + assert.ok(result.includes('Legal text at the end.'), 'Should include text'); + }); + + it('should return empty string when no legal boilerplate section exists', () => { + const template = ` +# Pull Request Template + +## Description +Please describe your changes + +## Checklist +- [ ] Tests added +`; + + const result = extractLegalBoilerplateSection(template); + + assert.strictEqual(result, '', 'Should return empty string when no legal section found'); + }); + + it('should handle empty template', () => { + const result = extractLegalBoilerplateSection(''); + assert.strictEqual(result, '', 'Should return empty string for empty template'); + }); + + it('should handle template with only legal boilerplate section', () => { + const template = '### Legal Boilerplate\nThis is the only content.'; + const result = extractLegalBoilerplateSection(template); + + assert.ok(result.includes('### Legal Boilerplate'), 'Should include header'); + assert.ok(result.includes('This is the only content.'), 'Should include content'); + }); + + it('should handle legal boilerplate with special characters', () => { + const template = ` +### Legal Boilerplate +Text with special chars: @#$%^&*()_+-={}[]|\\:";'<>?,./ +And some unicode: 你好世界 🎉 + +## Next +`; + + const result = extractLegalBoilerplateSection(template); + + assert.ok(result.includes('special chars'), 'Should handle special characters'); + assert.ok(result.includes('你好世界'), 'Should handle unicode'); + assert.ok(result.includes('🎉'), 'Should handle emoji'); + }); + + it('should handle legal boilerplate with code blocks', () => { + const template = ` +### Legal Boilerplate + +Some text with code: + +\`\`\`javascript +const legal = true; +\`\`\` + +More text. + +## Next Section +`; + + const result = extractLegalBoilerplateSection(template); + + assert.ok(result.includes('const legal = true;'), 'Should include code blocks'); + assert.ok(result.includes('More text.'), 'Should include text after code block'); + assert.ok(!result.includes('Next Section'), 'Should not include next section'); + }); + + it('should handle legal boilerplate with lists', () => { + const template = ` +### Legal Boilerplate + +You agree to: +- Item 1 +- Item 2 +- Item 3 + +## Other +`; + + const result = extractLegalBoilerplateSection(template); + + assert.ok(result.includes('- Item 1'), 'Should include list items'); + assert.ok(result.includes('- Item 2'), 'Should include list items'); + assert.ok(result.includes('- Item 3'), 'Should include list items'); + }); + + it('should handle legal boilerplate with extra whitespace', () => { + const template = ` +### Legal Boilerplate +Content with spaces. +## Next +`; + + const result = extractLegalBoilerplateSection(template); + + assert.ok(result.includes('Content with spaces.'), 'Should handle extra whitespace in header'); + }); + + it('should stop at first subsequent header', () => { + const template = ` +### Legal Boilerplate +First section content. +### Another Legal Boilerplate +This should not be included. +`; + + const result = extractLegalBoilerplateSection(template); + + assert.ok(result.includes('First section content.'), 'Should include first section'); + assert.ok(!result.includes('This should not be included.'), 'Should stop at next header'); + }); + + it('should handle blank lines within legal section', () => { + const template = ` +### Legal Boilerplate + +First paragraph. + + +Second paragraph with blank lines above. + +## Next +`; + + const result = extractLegalBoilerplateSection(template); + + assert.ok(result.includes('First paragraph.'), 'Should include first paragraph'); + assert.ok(result.includes('Second paragraph'), 'Should include second paragraph'); + // Should preserve blank lines + const blankLineCount = (result.match(/\n\n/g) || []).length; + assert.ok(blankLineCount >= 1, 'Should preserve blank lines'); + }); + }); }); \ No newline at end of file diff --git a/danger/dangerfile.js b/danger/dangerfile.js index 4dbb688..b6ad723 100644 --- a/danger/dangerfile.js +++ b/danger/dangerfile.js @@ -189,35 +189,16 @@ async function checkActionsArePinned() { async function checkLegalBoilerplate() { console.log('::debug:: Checking legal boilerplate requirements...'); - // Check if the PR author is an external contributor - const prAuthor = danger.github.pr.user.login; - const repoOwner = danger.github.pr.base.repo.owner.login; + // Check if the PR author is an external contributor using author_association + // Values: OWNER, MEMBER, COLLABORATOR, CONTRIBUTOR, FIRST_TIME_CONTRIBUTOR, FIRST_TIMER, NONE + const authorAssociation = danger.github.pr.author_association; + const isExternalContributor = !['OWNER', 'MEMBER', 'COLLABORATOR'].includes(authorAssociation); - // Check if author is a member of the repository's organization - let isExternalContributor = false; - try { - // Check organization membership - await danger.github.api.orgs.checkMembershipForUser({ - org: repoOwner, - username: prAuthor - }); - console.log(`::debug:: ${prAuthor} is a member of ${repoOwner} organization`); - isExternalContributor = false; - } catch (error) { - // If the API call fails with 404, the user is not a member - if (error.status === 404) { - console.log(`::debug:: ${prAuthor} is NOT a member of ${repoOwner} organization`); - isExternalContributor = true; - } else { - // For other errors (like permission issues), log and skip the check - console.log(`::warning:: Could not check organization membership for ${prAuthor}: ${error.message}`); - return; - } - } + console.log(`::debug:: Author association: ${authorAssociation}, isExternal: ${isExternalContributor}`); // If not an external contributor, skip the check if (!isExternalContributor) { - console.log('::debug:: Skipping legal boilerplate check for organization member'); + console.log('::debug:: Skipping legal boilerplate check for organization member/collaborator'); return; } @@ -232,13 +213,11 @@ async function checkLegalBoilerplate() { ]; for (const templatePath of possibleTemplatePaths) { - try { - prTemplateContent = await danger.github.utils.fileContents(templatePath); + const content = await danger.github.utils.fileContents(templatePath); + if (content) { + prTemplateContent = content; console.log(`::debug:: Found PR template at ${templatePath}`); break; - } catch (error) { - // Template not found at this path, try next - continue; } } @@ -258,18 +237,18 @@ async function checkLegalBoilerplate() { console.log('::debug:: PR template contains Legal Boilerplate section, checking PR description...'); + // Extract the legal boilerplate content from the template + const expectedBoilerplate = extractLegalBoilerplateSection(prTemplateContent); + // Check if the PR description contains the legal boilerplate const prBody = danger.github.pr.body || ''; // Look for the legal boilerplate header in the PR description if (!legalBoilerplateHeaderRegex.test(prBody)) { fail( - 'This PR is missing the required legal boilerplate. As an external contributor, please include the "Legal Boilerplate" section from the PR template in your PR description.', - undefined, - undefined + 'This PR is missing the required legal boilerplate. As an external contributor, please include the "Legal Boilerplate" section from the PR template in your PR description.' ); - const legalBoilerplateContent = extractLegalBoilerplateSection(prTemplateContent); markdown(` ### ⚖️ Legal Boilerplate Required @@ -278,7 +257,33 @@ As an external contributor, your PR must include the legal boilerplate from the Please add the following section to your PR description: \`\`\`markdown -${legalBoilerplateContent} +${expectedBoilerplate} +\`\`\` + +This is required to ensure proper intellectual property rights for your contributions. + `.trim()); + return; + } + + // Extract the actual boilerplate from PR body and verify it has meaningful content + const actualBoilerplate = extractLegalBoilerplateSection(prBody); + + // Check if the boilerplate has substantial content (at least 50 characters beyond the header) + const contentWithoutHeader = actualBoilerplate.replace(/^#{1,6}\s+Legal\s+Boilerplate\s*/i, '').trim(); + if (contentWithoutHeader.length < 50) { + fail( + 'The legal boilerplate section in your PR description appears to be incomplete. Please include the full legal text from the PR template.' + ); + + markdown(` +### ⚖️ Incomplete Legal Boilerplate + +Your PR contains a "Legal Boilerplate" header but the content appears incomplete. + +Please ensure you include the complete legal text from the template: + +\`\`\`markdown +${expectedBoilerplate} \`\`\` This is required to ensure proper intellectual property rights for your contributions. From 791297a3727ce4bd8bb2292ba3c89d42a5ead5ab Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 29 Jan 2026 08:39:14 +0000 Subject: [PATCH 06/13] Simplify validation: check exact boilerplate match instead of arbitrary length Replace arbitrary 50-character check with actual content comparison. Now validates that the exact legal boilerplate from the template is present in the PR description by normalizing whitespace and comparing content. Simplified logic removes unnecessary complexity and ensures the actual legal text stays intact, not just that something exists. Co-authored-by: vaind <6349682+vaind@users.noreply.github.com> --- danger/dangerfile.js | 50 ++++++++++++++++++-------------------------- 1 file changed, 20 insertions(+), 30 deletions(-) diff --git a/danger/dangerfile.js b/danger/dangerfile.js index b6ad723..1e6d132 100644 --- a/danger/dangerfile.js +++ b/danger/dangerfile.js @@ -190,19 +190,15 @@ async function checkLegalBoilerplate() { console.log('::debug:: Checking legal boilerplate requirements...'); // Check if the PR author is an external contributor using author_association - // Values: OWNER, MEMBER, COLLABORATOR, CONTRIBUTOR, FIRST_TIME_CONTRIBUTOR, FIRST_TIMER, NONE const authorAssociation = danger.github.pr.author_association; const isExternalContributor = !['OWNER', 'MEMBER', 'COLLABORATOR'].includes(authorAssociation); - console.log(`::debug:: Author association: ${authorAssociation}, isExternal: ${isExternalContributor}`); - - // If not an external contributor, skip the check if (!isExternalContributor) { console.log('::debug:: Skipping legal boilerplate check for organization member/collaborator'); return; } - // Check if the PR template contains a legal boilerplate section + // Find PR template let prTemplateContent = null; const possibleTemplatePaths = [ '.github/PULL_REQUEST_TEMPLATE.md', @@ -221,33 +217,28 @@ async function checkLegalBoilerplate() { } } - // If no template found, skip the check if (!prTemplateContent) { console.log('::debug:: No PR template found, skipping legal boilerplate check'); return; } - // Check if the template contains a legal boilerplate section - // Look for headers like "### Legal Boilerplate", "## Legal Boilerplate", etc. + // Check if template contains a Legal Boilerplate section const legalBoilerplateHeaderRegex = /^#{1,6}\s+Legal\s+Boilerplate/im; if (!legalBoilerplateHeaderRegex.test(prTemplateContent)) { console.log('::debug:: PR template does not contain a Legal Boilerplate section'); return; } - console.log('::debug:: PR template contains Legal Boilerplate section, checking PR description...'); - - // Extract the legal boilerplate content from the template + // Extract expected boilerplate from template const expectedBoilerplate = extractLegalBoilerplateSection(prTemplateContent); - - // Check if the PR description contains the legal boilerplate const prBody = danger.github.pr.body || ''; - // Look for the legal boilerplate header in the PR description - if (!legalBoilerplateHeaderRegex.test(prBody)) { - fail( - 'This PR is missing the required legal boilerplate. As an external contributor, please include the "Legal Boilerplate" section from the PR template in your PR description.' - ); + // Extract actual boilerplate from PR body + const actualBoilerplate = extractLegalBoilerplateSection(prBody); + + // Check if PR body contains the legal boilerplate section + if (!actualBoilerplate) { + fail('This PR is missing the required legal boilerplate. As an external contributor, please include the "Legal Boilerplate" section from the PR template in your PR description.'); markdown(` ### ⚖️ Legal Boilerplate Required @@ -265,22 +256,21 @@ This is required to ensure proper intellectual property rights for your contribu return; } - // Extract the actual boilerplate from PR body and verify it has meaningful content - const actualBoilerplate = extractLegalBoilerplateSection(prBody); + // Verify the actual boilerplate matches the expected one + // Normalize whitespace for comparison + const normalizeWhitespace = (str) => str.replace(/\s+/g, ' ').trim(); + const expectedNormalized = normalizeWhitespace(expectedBoilerplate); + const actualNormalized = normalizeWhitespace(actualBoilerplate); - // Check if the boilerplate has substantial content (at least 50 characters beyond the header) - const contentWithoutHeader = actualBoilerplate.replace(/^#{1,6}\s+Legal\s+Boilerplate\s*/i, '').trim(); - if (contentWithoutHeader.length < 50) { - fail( - 'The legal boilerplate section in your PR description appears to be incomplete. Please include the full legal text from the PR template.' - ); + if (expectedNormalized !== actualNormalized) { + fail('The legal boilerplate in your PR description does not match the template. Please ensure you include the complete, unmodified legal text from the PR template.'); markdown(` -### ⚖️ Incomplete Legal Boilerplate +### ⚖️ Legal Boilerplate Mismatch -Your PR contains a "Legal Boilerplate" header but the content appears incomplete. +Your PR contains a "Legal Boilerplate" section, but it doesn't match the required text from the template. -Please ensure you include the complete legal text from the template: +Please replace it with the exact text from the template: \`\`\`markdown ${expectedBoilerplate} @@ -291,7 +281,7 @@ This is required to ensure proper intellectual property rights for your contribu return; } - console.log('::debug:: Legal boilerplate found in PR description ✓'); + console.log('::debug:: Legal boilerplate validated successfully ✓'); } async function checkFromExternalChecks() { From 072b08a63e9b80a42199c7968c1794f22253c1ac Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Thu, 29 Jan 2026 11:28:17 +0100 Subject: [PATCH 07/13] Add integration tests for checkLegalBoilerplate orchestration function Extract checkLegalBoilerplate() from dangerfile.js into dangerfile-utils.js with dependency injection (accepts { danger, fail, markdown } as parameters) so the orchestration logic can be tested without the DangerJS runtime. Adds 19 tests covering author association filtering, template discovery, missing/mismatched boilerplate detection, success cases, and markdown output. Co-Authored-By: Claude Opus 4.5 --- danger/dangerfile-utils.js | 110 +++++++++++++- danger/dangerfile-utils.test.js | 245 +++++++++++++++++++++++++++++++- danger/dangerfile.js | 102 +------------ 3 files changed, 357 insertions(+), 100 deletions(-) diff --git a/danger/dangerfile-utils.js b/danger/dangerfile-utils.js index bbdba27..07fbae0 100644 --- a/danger/dangerfile-utils.js +++ b/danger/dangerfile-utils.js @@ -120,9 +120,117 @@ function extractLegalBoilerplateSection(templateContent) { return legalSection.join('\n').trim(); } +/** + * Check that external contributors include the required legal boilerplate in their PR body. + * Accepts danger context and reporting functions as parameters for testability. + * + * @param {object} options + * @param {object} options.danger - The DangerJS danger object + * @param {Function} options.fail - DangerJS fail function + * @param {Function} options.markdown - DangerJS markdown function + */ +async function checkLegalBoilerplate({ danger, fail, markdown }) { + console.log('::debug:: Checking legal boilerplate requirements...'); + + // Check if the PR author is an external contributor using author_association + const authorAssociation = danger.github.pr.author_association; + const isExternalContributor = !['OWNER', 'MEMBER', 'COLLABORATOR'].includes(authorAssociation); + + if (!isExternalContributor) { + console.log('::debug:: Skipping legal boilerplate check for organization member/collaborator'); + return; + } + + // Find PR template + let prTemplateContent = null; + const possibleTemplatePaths = [ + '.github/PULL_REQUEST_TEMPLATE.md', + '.github/pull_request_template.md', + 'PULL_REQUEST_TEMPLATE.md', + 'pull_request_template.md', + '.github/PULL_REQUEST_TEMPLATE/pull_request_template.md' + ]; + + for (const templatePath of possibleTemplatePaths) { + const content = await danger.github.utils.fileContents(templatePath); + if (content) { + prTemplateContent = content; + console.log(`::debug:: Found PR template at ${templatePath}`); + break; + } + } + + if (!prTemplateContent) { + console.log('::debug:: No PR template found, skipping legal boilerplate check'); + return; + } + + // Check if template contains a Legal Boilerplate section + const legalBoilerplateHeaderRegex = /^#{1,6}\s+Legal\s+Boilerplate/im; + if (!legalBoilerplateHeaderRegex.test(prTemplateContent)) { + console.log('::debug:: PR template does not contain a Legal Boilerplate section'); + return; + } + + // Extract expected boilerplate from template + const expectedBoilerplate = extractLegalBoilerplateSection(prTemplateContent); + const prBody = danger.github.pr.body || ''; + + // Extract actual boilerplate from PR body + const actualBoilerplate = extractLegalBoilerplateSection(prBody); + + // Check if PR body contains the legal boilerplate section + if (!actualBoilerplate) { + fail('This PR is missing the required legal boilerplate. As an external contributor, please include the "Legal Boilerplate" section from the PR template in your PR description.'); + + markdown(` +### ⚖️ Legal Boilerplate Required + +As an external contributor, your PR must include the legal boilerplate from the PR template. + +Please add the following section to your PR description: + +\`\`\`markdown +${expectedBoilerplate} +\`\`\` + +This is required to ensure proper intellectual property rights for your contributions. + `.trim()); + return; + } + + // Verify the actual boilerplate matches the expected one + // Normalize whitespace for comparison + const normalizeWhitespace = (str) => str.replace(/\s+/g, ' ').trim(); + const expectedNormalized = normalizeWhitespace(expectedBoilerplate); + const actualNormalized = normalizeWhitespace(actualBoilerplate); + + if (expectedNormalized !== actualNormalized) { + fail('The legal boilerplate in your PR description does not match the template. Please ensure you include the complete, unmodified legal text from the PR template.'); + + markdown(` +### ⚖️ Legal Boilerplate Mismatch + +Your PR contains a "Legal Boilerplate" section, but it doesn't match the required text from the template. + +Please replace it with the exact text from the template: + +\`\`\`markdown +${expectedBoilerplate} +\`\`\` + +This is required to ensure proper intellectual property rights for your contributions. + `.trim()); + return; + } + + console.log('::debug:: Legal boilerplate validated successfully ✓'); +} + module.exports = { FLAVOR_CONFIG, getFlavorConfig, extractPRFlavor, - extractLegalBoilerplateSection + extractLegalBoilerplateSection, + checkLegalBoilerplate }; diff --git a/danger/dangerfile-utils.test.js b/danger/dangerfile-utils.test.js index 3abf2e8..b0d92b3 100644 --- a/danger/dangerfile-utils.test.js +++ b/danger/dangerfile-utils.test.js @@ -1,6 +1,6 @@ const { describe, it } = require('node:test'); const assert = require('node:assert'); -const { getFlavorConfig, extractPRFlavor, extractLegalBoilerplateSection, FLAVOR_CONFIG } = require('./dangerfile-utils.js'); +const { getFlavorConfig, extractPRFlavor, extractLegalBoilerplateSection, checkLegalBoilerplate, FLAVOR_CONFIG } = require('./dangerfile-utils.js'); describe('dangerfile-utils', () => { describe('getFlavorConfig', () => { @@ -521,4 +521,247 @@ Second paragraph with blank lines above. assert.ok(blankLineCount >= 1, 'Should preserve blank lines'); }); }); +}); + +// --- Integration tests for checkLegalBoilerplate --- + +const PR_TEMPLATE_WITH_BOILERPLATE = `# Pull Request Template + +## Description +Please describe your changes + +### Legal Boilerplate +Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms. + +## Checklist +- [ ] Tests added`; + +const LEGAL_TEXT = 'Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here\'s the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry\'s choice of terms.'; + +/** + * Build a mock danger context for checkLegalBoilerplate tests. + * @param {object} overrides - Properties to override on danger.github.pr + * @param {string|null} templateContent - Content returned by fileContents for template paths (null = no template) + */ +function buildMockContext({ prOverrides = {}, templateContent = PR_TEMPLATE_WITH_BOILERPLATE } = {}) { + const failMessages = []; + const markdownMessages = []; + + const danger = { + github: { + pr: { + author_association: 'CONTRIBUTOR', + body: '', + ...prOverrides + }, + utils: { + fileContents: async (path) => { + // Only return content for the first standard template path + if (templateContent && path === '.github/PULL_REQUEST_TEMPLATE.md') { + return templateContent; + } + return ''; + } + } + } + }; + + return { + danger, + fail: (msg) => failMessages.push(msg), + markdown: (msg) => markdownMessages.push(msg), + failMessages, + markdownMessages + }; +} + +describe('checkLegalBoilerplate', () => { + // --- Skips for internal contributors --- + + it('should skip check for OWNER association', async () => { + const ctx = buildMockContext({ prOverrides: { author_association: 'OWNER' } }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 0); + assert.strictEqual(ctx.markdownMessages.length, 0); + }); + + it('should skip check for MEMBER association', async () => { + const ctx = buildMockContext({ prOverrides: { author_association: 'MEMBER' } }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 0); + }); + + it('should skip check for COLLABORATOR association', async () => { + const ctx = buildMockContext({ prOverrides: { author_association: 'COLLABORATOR' } }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 0); + }); + + // --- External contributor associations that should be checked --- + + it('should check for CONTRIBUTOR association', async () => { + const ctx = buildMockContext({ prOverrides: { author_association: 'CONTRIBUTOR', body: '' } }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 1, 'Should fail for external CONTRIBUTOR without boilerplate'); + }); + + it('should check for FIRST_TIME_CONTRIBUTOR association', async () => { + const ctx = buildMockContext({ prOverrides: { author_association: 'FIRST_TIME_CONTRIBUTOR', body: '' } }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 1); + }); + + it('should check for NONE association', async () => { + const ctx = buildMockContext({ prOverrides: { author_association: 'NONE', body: '' } }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 1); + }); + + // --- Template discovery --- + + it('should skip when no PR template is found', async () => { + const ctx = buildMockContext({ templateContent: null }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 0, 'Should not fail when no template exists'); + }); + + it('should skip when template has no Legal Boilerplate section', async () => { + const ctx = buildMockContext({ templateContent: '# Template\n\n## Description\nJust a normal template.' }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 0, 'Should not fail when template lacks legal section'); + }); + + it('should find template at the first matching path', async () => { + const calledPaths = []; + const ctx = buildMockContext(); + ctx.danger.github.utils.fileContents = async (path) => { + calledPaths.push(path); + // Return content for the second path to verify it tries multiple + if (path === '.github/pull_request_template.md') { + return PR_TEMPLATE_WITH_BOILERPLATE; + } + return ''; + }; + ctx.danger.github.pr.body = `## My PR\n\n### Legal Boilerplate\n${LEGAL_TEXT}`; + await checkLegalBoilerplate(ctx); + assert.ok(calledPaths.includes('.github/PULL_REQUEST_TEMPLATE.md'), 'Should try uppercase path first'); + assert.ok(calledPaths.includes('.github/pull_request_template.md'), 'Should try lowercase path second'); + // Should stop after finding the template (not try remaining paths) + assert.ok(!calledPaths.includes('PULL_REQUEST_TEMPLATE.md'), 'Should stop after finding template'); + }); + + // --- Missing boilerplate in PR body --- + + it('should fail when external contributor PR body is empty', async () => { + const ctx = buildMockContext({ prOverrides: { author_association: 'NONE', body: '' } }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 1); + assert.ok(ctx.failMessages[0].includes('missing the required legal boilerplate')); + assert.strictEqual(ctx.markdownMessages.length, 1); + assert.ok(ctx.markdownMessages[0].includes('Legal Boilerplate Required')); + }); + + it('should fail when external contributor PR body is null', async () => { + const ctx = buildMockContext({ prOverrides: { author_association: 'CONTRIBUTOR', body: null } }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 1); + assert.ok(ctx.failMessages[0].includes('missing the required legal boilerplate')); + }); + + it('should fail when PR body has no legal section', async () => { + const ctx = buildMockContext({ + prOverrides: { + author_association: 'FIRST_TIME_CONTRIBUTOR', + body: '## Description\nMy cool changes\n\n## Checklist\n- [x] Tests' + } + }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 1); + assert.ok(ctx.failMessages[0].includes('missing the required legal boilerplate')); + }); + + // --- Boilerplate mismatch --- + + it('should fail when boilerplate text is modified', async () => { + const ctx = buildMockContext({ + prOverrides: { + author_association: 'CONTRIBUTOR', + body: '### Legal Boilerplate\nI changed the legal text to something else entirely.' + } + }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 1); + assert.ok(ctx.failMessages[0].includes('does not match the template')); + assert.strictEqual(ctx.markdownMessages.length, 1); + assert.ok(ctx.markdownMessages[0].includes('Legal Boilerplate Mismatch')); + }); + + it('should fail when boilerplate is truncated', async () => { + const ctx = buildMockContext({ + prOverrides: { + author_association: 'CONTRIBUTOR', + body: '### Legal Boilerplate\nLook, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015.' + } + }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 1); + assert.ok(ctx.failMessages[0].includes('does not match the template')); + }); + + // --- Matching boilerplate (success cases) --- + + it('should pass when boilerplate matches exactly', async () => { + const ctx = buildMockContext({ + prOverrides: { + author_association: 'CONTRIBUTOR', + body: `## Description\nMy changes\n\n### Legal Boilerplate\n${LEGAL_TEXT}\n\n## Checklist\n- [x] Done` + } + }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 0, 'Should not fail when boilerplate matches'); + assert.strictEqual(ctx.markdownMessages.length, 0); + }); + + it('should pass when boilerplate matches with different whitespace', async () => { + const ctx = buildMockContext({ + prOverrides: { + author_association: 'CONTRIBUTOR', + body: `### Legal Boilerplate\n${LEGAL_TEXT.replace(/\. /g, '.\n')}` + } + }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 0, 'Should pass with normalized whitespace differences'); + }); + + it('should pass when boilerplate has extra surrounding whitespace', async () => { + const ctx = buildMockContext({ + prOverrides: { + author_association: 'CONTRIBUTOR', + body: `### Legal Boilerplate\n\n ${LEGAL_TEXT} \n\n## Next` + } + }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 0); + }); + + // --- Markdown message content --- + + it('should include expected boilerplate in the markdown hint when missing', async () => { + const ctx = buildMockContext({ prOverrides: { author_association: 'NONE', body: '' } }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.markdownMessages.length, 1); + assert.ok(ctx.markdownMessages[0].includes('Functional Software, Inc.'), 'Markdown should include the expected legal text'); + }); + + it('should include expected boilerplate in the markdown hint on mismatch', async () => { + const ctx = buildMockContext({ + prOverrides: { + author_association: 'CONTRIBUTOR', + body: '### Legal Boilerplate\nWrong text here.' + } + }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.markdownMessages.length, 1); + assert.ok(ctx.markdownMessages[0].includes('Functional Software, Inc.')); + }); }); \ No newline at end of file diff --git a/danger/dangerfile.js b/danger/dangerfile.js index 1e6d132..57410d5 100644 --- a/danger/dangerfile.js +++ b/danger/dangerfile.js @@ -1,4 +1,4 @@ -const { getFlavorConfig, extractPRFlavor, extractLegalBoilerplateSection } = require('./dangerfile-utils.js'); +const { getFlavorConfig, extractPRFlavor, extractLegalBoilerplateSection, checkLegalBoilerplate } = require('./dangerfile-utils.js'); const headRepoName = danger.github.pr.head.repo.git_url; const baseRepoName = danger.github.pr.base.repo.git_url; @@ -186,102 +186,8 @@ async function checkActionsArePinned() { } } -async function checkLegalBoilerplate() { - console.log('::debug:: Checking legal boilerplate requirements...'); - - // Check if the PR author is an external contributor using author_association - const authorAssociation = danger.github.pr.author_association; - const isExternalContributor = !['OWNER', 'MEMBER', 'COLLABORATOR'].includes(authorAssociation); - - if (!isExternalContributor) { - console.log('::debug:: Skipping legal boilerplate check for organization member/collaborator'); - return; - } - - // Find PR template - let prTemplateContent = null; - const possibleTemplatePaths = [ - '.github/PULL_REQUEST_TEMPLATE.md', - '.github/pull_request_template.md', - 'PULL_REQUEST_TEMPLATE.md', - 'pull_request_template.md', - '.github/PULL_REQUEST_TEMPLATE/pull_request_template.md' - ]; - - for (const templatePath of possibleTemplatePaths) { - const content = await danger.github.utils.fileContents(templatePath); - if (content) { - prTemplateContent = content; - console.log(`::debug:: Found PR template at ${templatePath}`); - break; - } - } - - if (!prTemplateContent) { - console.log('::debug:: No PR template found, skipping legal boilerplate check'); - return; - } - - // Check if template contains a Legal Boilerplate section - const legalBoilerplateHeaderRegex = /^#{1,6}\s+Legal\s+Boilerplate/im; - if (!legalBoilerplateHeaderRegex.test(prTemplateContent)) { - console.log('::debug:: PR template does not contain a Legal Boilerplate section'); - return; - } - - // Extract expected boilerplate from template - const expectedBoilerplate = extractLegalBoilerplateSection(prTemplateContent); - const prBody = danger.github.pr.body || ''; - - // Extract actual boilerplate from PR body - const actualBoilerplate = extractLegalBoilerplateSection(prBody); - - // Check if PR body contains the legal boilerplate section - if (!actualBoilerplate) { - fail('This PR is missing the required legal boilerplate. As an external contributor, please include the "Legal Boilerplate" section from the PR template in your PR description.'); - - markdown(` -### ⚖️ Legal Boilerplate Required - -As an external contributor, your PR must include the legal boilerplate from the PR template. - -Please add the following section to your PR description: - -\`\`\`markdown -${expectedBoilerplate} -\`\`\` - -This is required to ensure proper intellectual property rights for your contributions. - `.trim()); - return; - } - - // Verify the actual boilerplate matches the expected one - // Normalize whitespace for comparison - const normalizeWhitespace = (str) => str.replace(/\s+/g, ' ').trim(); - const expectedNormalized = normalizeWhitespace(expectedBoilerplate); - const actualNormalized = normalizeWhitespace(actualBoilerplate); - - if (expectedNormalized !== actualNormalized) { - fail('The legal boilerplate in your PR description does not match the template. Please ensure you include the complete, unmodified legal text from the PR template.'); - - markdown(` -### ⚖️ Legal Boilerplate Mismatch - -Your PR contains a "Legal Boilerplate" section, but it doesn't match the required text from the template. - -Please replace it with the exact text from the template: - -\`\`\`markdown -${expectedBoilerplate} -\`\`\` - -This is required to ensure proper intellectual property rights for your contributions. - `.trim()); - return; - } - - console.log('::debug:: Legal boilerplate validated successfully ✓'); +async function checkLegalBoilerplateRule() { + await checkLegalBoilerplate({ danger, fail, markdown }); } async function checkFromExternalChecks() { @@ -329,7 +235,7 @@ async function checkAll() { await checkDocs(); await checkChangelog(); await checkActionsArePinned(); - await checkLegalBoilerplate(); + await checkLegalBoilerplateRule(); await checkFromExternalChecks(); } From ce9bfaf35ad03147ea58c5ab5c6041f3e72d9582 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Fri, 30 Jan 2026 08:44:16 +0100 Subject: [PATCH 08/13] Simplify checkLegalBoilerplate and remove unnecessary indirection Extract findPRTemplate helper and named constants (INTERNAL_ASSOCIATIONS, PR_TEMPLATE_PATHS). Simplify extractLegalBoilerplateSection using findIndex. Inline checkLegalBoilerplate call in checkAll, removing the wrapper function. Derive test LEGAL_TEXT from the template constant to prevent drift. Co-Authored-By: Claude Opus 4.5 --- danger/dangerfile-utils.js | 119 ++++++++++++-------------------- danger/dangerfile-utils.test.js | 10 +-- danger/dangerfile.js | 8 +-- 3 files changed, 51 insertions(+), 86 deletions(-) diff --git a/danger/dangerfile-utils.js b/danger/dangerfile-utils.js index 07fbae0..9c5c975 100644 --- a/danger/dangerfile-utils.js +++ b/danger/dangerfile-utils.js @@ -86,100 +86,64 @@ function extractPRFlavor(prTitle, prBranchRef) { return ""; } -/** - * Extract the legal boilerplate section from the PR template - * @param {string} templateContent - The PR template content - * @returns {string} The extracted legal boilerplate section - */ -function extractLegalBoilerplateSection(templateContent) { - // Find the legal boilerplate section and extract it - const lines = templateContent.split('\n'); - let inLegalSection = false; - let legalSection = []; - - for (let i = 0; i < lines.length; i++) { - const line = lines[i]; - - // Check if this line is the legal boilerplate header - if (/^#{1,6}\s+Legal\s+Boilerplate/i.test(line)) { - inLegalSection = true; - legalSection.push(line); - continue; - } - - // If we're in the legal section - if (inLegalSection) { - // Check if we've reached another header (end of legal section) - if (/^#{1,6}\s+/.test(line)) { - break; - } - legalSection.push(line); +/** @returns {string} The legal boilerplate section extracted from the content, or empty string if none found */ +function extractLegalBoilerplateSection(content) { + const lines = content.split('\n'); + const legalHeaderIndex = lines.findIndex(line => /^#{1,6}\s+Legal\s+Boilerplate/i.test(line)); + + if (legalHeaderIndex === -1) { + return ''; + } + + const sectionLines = [lines[legalHeaderIndex]]; + + for (let i = legalHeaderIndex + 1; i < lines.length; i++) { + if (/^#{1,6}\s+/.test(lines[i])) { + break; } + sectionLines.push(lines[i]); } - - return legalSection.join('\n').trim(); + + return sectionLines.join('\n').trim(); } +const INTERNAL_ASSOCIATIONS = ['OWNER', 'MEMBER', 'COLLABORATOR']; + +const PR_TEMPLATE_PATHS = [ + '.github/PULL_REQUEST_TEMPLATE.md', + '.github/pull_request_template.md', + 'PULL_REQUEST_TEMPLATE.md', + 'pull_request_template.md', + '.github/PULL_REQUEST_TEMPLATE/pull_request_template.md' +]; + /** * Check that external contributors include the required legal boilerplate in their PR body. * Accepts danger context and reporting functions as parameters for testability. - * - * @param {object} options - * @param {object} options.danger - The DangerJS danger object - * @param {Function} options.fail - DangerJS fail function - * @param {Function} options.markdown - DangerJS markdown function */ async function checkLegalBoilerplate({ danger, fail, markdown }) { console.log('::debug:: Checking legal boilerplate requirements...'); - // Check if the PR author is an external contributor using author_association const authorAssociation = danger.github.pr.author_association; - const isExternalContributor = !['OWNER', 'MEMBER', 'COLLABORATOR'].includes(authorAssociation); - - if (!isExternalContributor) { + if (INTERNAL_ASSOCIATIONS.includes(authorAssociation)) { console.log('::debug:: Skipping legal boilerplate check for organization member/collaborator'); return; } - // Find PR template - let prTemplateContent = null; - const possibleTemplatePaths = [ - '.github/PULL_REQUEST_TEMPLATE.md', - '.github/pull_request_template.md', - 'PULL_REQUEST_TEMPLATE.md', - 'pull_request_template.md', - '.github/PULL_REQUEST_TEMPLATE/pull_request_template.md' - ]; - - for (const templatePath of possibleTemplatePaths) { - const content = await danger.github.utils.fileContents(templatePath); - if (content) { - prTemplateContent = content; - console.log(`::debug:: Found PR template at ${templatePath}`); - break; - } - } - + const prTemplateContent = await findPRTemplate(danger); if (!prTemplateContent) { console.log('::debug:: No PR template found, skipping legal boilerplate check'); return; } - // Check if template contains a Legal Boilerplate section - const legalBoilerplateHeaderRegex = /^#{1,6}\s+Legal\s+Boilerplate/im; - if (!legalBoilerplateHeaderRegex.test(prTemplateContent)) { + const expectedBoilerplate = extractLegalBoilerplateSection(prTemplateContent); + if (!expectedBoilerplate) { console.log('::debug:: PR template does not contain a Legal Boilerplate section'); return; } - // Extract expected boilerplate from template - const expectedBoilerplate = extractLegalBoilerplateSection(prTemplateContent); - const prBody = danger.github.pr.body || ''; - - // Extract actual boilerplate from PR body - const actualBoilerplate = extractLegalBoilerplateSection(prBody); + const actualBoilerplate = extractLegalBoilerplateSection(danger.github.pr.body || ''); - // Check if PR body contains the legal boilerplate section if (!actualBoilerplate) { fail('This PR is missing the required legal boilerplate. As an external contributor, please include the "Legal Boilerplate" section from the PR template in your PR description.'); @@ -199,13 +163,10 @@ This is required to ensure proper intellectual property rights for your contribu return; } - // Verify the actual boilerplate matches the expected one - // Normalize whitespace for comparison + // Normalize whitespace so minor formatting differences don't cause false negatives const normalizeWhitespace = (str) => str.replace(/\s+/g, ' ').trim(); - const expectedNormalized = normalizeWhitespace(expectedBoilerplate); - const actualNormalized = normalizeWhitespace(actualBoilerplate); - if (expectedNormalized !== actualNormalized) { + if (normalizeWhitespace(expectedBoilerplate) !== normalizeWhitespace(actualBoilerplate)) { fail('The legal boilerplate in your PR description does not match the template. Please ensure you include the complete, unmodified legal text from the PR template.'); markdown(` @@ -227,6 +188,18 @@ This is required to ensure proper intellectual property rights for your contribu console.log('::debug:: Legal boilerplate validated successfully ✓'); } +/** Try each known PR template path and return the first one with content. */ +async function findPRTemplate(danger) { + for (const templatePath of PR_TEMPLATE_PATHS) { + const content = await danger.github.utils.fileContents(templatePath); + if (content) { + console.log(`::debug:: Found PR template at ${templatePath}`); + return content; + } + } + return null; +} + module.exports = { FLAVOR_CONFIG, getFlavorConfig, diff --git a/danger/dangerfile-utils.test.js b/danger/dangerfile-utils.test.js index b0d92b3..613256f 100644 --- a/danger/dangerfile-utils.test.js +++ b/danger/dangerfile-utils.test.js @@ -536,13 +536,10 @@ Look, I get it. The entity doing business as "Sentry" was incorporated in the St ## Checklist - [ ] Tests added`; -const LEGAL_TEXT = 'Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here\'s the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry\'s choice of terms.'; +// Derived from the template to stay in sync automatically +const LEGAL_BOILERPLATE_SECTION = extractLegalBoilerplateSection(PR_TEMPLATE_WITH_BOILERPLATE); +const LEGAL_TEXT = LEGAL_BOILERPLATE_SECTION.replace('### Legal Boilerplate\n', ''); -/** - * Build a mock danger context for checkLegalBoilerplate tests. - * @param {object} overrides - Properties to override on danger.github.pr - * @param {string|null} templateContent - Content returned by fileContents for template paths (null = no template) - */ function buildMockContext({ prOverrides = {}, templateContent = PR_TEMPLATE_WITH_BOILERPLATE } = {}) { const failMessages = []; const markdownMessages = []; @@ -556,7 +553,6 @@ function buildMockContext({ prOverrides = {}, templateContent = PR_TEMPLATE_WITH }, utils: { fileContents: async (path) => { - // Only return content for the first standard template path if (templateContent && path === '.github/PULL_REQUEST_TEMPLATE.md') { return templateContent; } diff --git a/danger/dangerfile.js b/danger/dangerfile.js index 57410d5..84eaef5 100644 --- a/danger/dangerfile.js +++ b/danger/dangerfile.js @@ -1,4 +1,4 @@ -const { getFlavorConfig, extractPRFlavor, extractLegalBoilerplateSection, checkLegalBoilerplate } = require('./dangerfile-utils.js'); +const { getFlavorConfig, extractPRFlavor, checkLegalBoilerplate } = require('./dangerfile-utils.js'); const headRepoName = danger.github.pr.head.repo.git_url; const baseRepoName = danger.github.pr.base.repo.git_url; @@ -186,10 +186,6 @@ async function checkActionsArePinned() { } } -async function checkLegalBoilerplateRule() { - await checkLegalBoilerplate({ danger, fail, markdown }); -} - async function checkFromExternalChecks() { // Get the external dangerfile path from environment variable (passed via workflow input) // Priority: EXTRA_DANGERFILE (absolute path) -> EXTRA_DANGERFILE_INPUT (relative path) @@ -235,7 +231,7 @@ async function checkAll() { await checkDocs(); await checkChangelog(); await checkActionsArePinned(); - await checkLegalBoilerplateRule(); + await checkLegalBoilerplate({ danger, fail, markdown }); await checkFromExternalChecks(); } From 8bc7876028797bddbd8fd5032e48a0f26f6f853f Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Fri, 30 Jan 2026 09:00:51 +0100 Subject: [PATCH 09/13] Refactor: extract helpers and consolidate test structure Extract normalizeWhitespace, findPRTemplate, and formatBoilerplateHint to module scope for reuse and top-down readability. Deduplicate markdown hint construction across missing/mismatch error paths. Nest integration tests and their helpers inside the top-level describe block. Co-Authored-By: Claude Opus 4.5 --- danger/dangerfile-utils.js | 85 ++++--- danger/dangerfile-utils.test.js | 392 ++++++++++++++++---------------- 2 files changed, 235 insertions(+), 242 deletions(-) diff --git a/danger/dangerfile-utils.js b/danger/dangerfile-utils.js index 9c5c975..e067084 100644 --- a/danger/dangerfile-utils.js +++ b/danger/dangerfile-utils.js @@ -117,6 +117,36 @@ const PR_TEMPLATE_PATHS = [ '.github/PULL_REQUEST_TEMPLATE/pull_request_template.md' ]; +/** Collapse all whitespace runs into single spaces for comparison. */ +function normalizeWhitespace(str) { + return str.replace(/\s+/g, ' ').trim(); +} + +/** Try each known PR template path and return the first one with content. */ +async function findPRTemplate(danger) { + for (const templatePath of PR_TEMPLATE_PATHS) { + const content = await danger.github.utils.fileContents(templatePath); + if (content) { + console.log(`::debug:: Found PR template at ${templatePath}`); + return content; + } + } + return null; +} + +/** Build a markdown hint showing the expected boilerplate text. */ +function formatBoilerplateHint(title, description, expectedBoilerplate) { + return `### ⚖️ ${title} + +${description} + +\`\`\`markdown +${expectedBoilerplate} +\`\`\` + +This is required to ensure proper intellectual property rights for your contributions.`; +} + /** * Check that external contributors include the required legal boilerplate in their PR body. * Accepts danger context and reporting functions as parameters for testability. @@ -146,58 +176,25 @@ async function checkLegalBoilerplate({ danger, fail, markdown }) { if (!actualBoilerplate) { fail('This PR is missing the required legal boilerplate. As an external contributor, please include the "Legal Boilerplate" section from the PR template in your PR description.'); - - markdown(` -### ⚖️ Legal Boilerplate Required - -As an external contributor, your PR must include the legal boilerplate from the PR template. - -Please add the following section to your PR description: - -\`\`\`markdown -${expectedBoilerplate} -\`\`\` - -This is required to ensure proper intellectual property rights for your contributions. - `.trim()); + markdown(formatBoilerplateHint( + 'Legal Boilerplate Required', + 'As an external contributor, your PR must include the legal boilerplate from the PR template.\n\nPlease add the following section to your PR description:', + expectedBoilerplate + )); return; } - // Normalize whitespace so minor formatting differences don't cause false negatives - const normalizeWhitespace = (str) => str.replace(/\s+/g, ' ').trim(); - if (normalizeWhitespace(expectedBoilerplate) !== normalizeWhitespace(actualBoilerplate)) { fail('The legal boilerplate in your PR description does not match the template. Please ensure you include the complete, unmodified legal text from the PR template.'); - - markdown(` -### ⚖️ Legal Boilerplate Mismatch - -Your PR contains a "Legal Boilerplate" section, but it doesn't match the required text from the template. - -Please replace it with the exact text from the template: - -\`\`\`markdown -${expectedBoilerplate} -\`\`\` - -This is required to ensure proper intellectual property rights for your contributions. - `.trim()); + markdown(formatBoilerplateHint( + 'Legal Boilerplate Mismatch', + 'Your PR contains a "Legal Boilerplate" section, but it doesn\'t match the required text from the template.\n\nPlease replace it with the exact text from the template:', + expectedBoilerplate + )); return; } - console.log('::debug:: Legal boilerplate validated successfully ✓'); -} - -/** Try each known PR template path and return the first one with content. */ -async function findPRTemplate(danger) { - for (const templatePath of PR_TEMPLATE_PATHS) { - const content = await danger.github.utils.fileContents(templatePath); - if (content) { - console.log(`::debug:: Found PR template at ${templatePath}`); - return content; - } - } - return null; + console.log('::debug:: Legal boilerplate validated successfully'); } module.exports = { diff --git a/danger/dangerfile-utils.test.js b/danger/dangerfile-utils.test.js index 613256f..31ca66c 100644 --- a/danger/dangerfile-utils.test.js +++ b/danger/dangerfile-utils.test.js @@ -513,7 +513,7 @@ Second paragraph with blank lines above. `; const result = extractLegalBoilerplateSection(template); - + assert.ok(result.includes('First paragraph.'), 'Should include first paragraph'); assert.ok(result.includes('Second paragraph'), 'Should include second paragraph'); // Should preserve blank lines @@ -521,11 +521,9 @@ Second paragraph with blank lines above. assert.ok(blankLineCount >= 1, 'Should preserve blank lines'); }); }); -}); -// --- Integration tests for checkLegalBoilerplate --- - -const PR_TEMPLATE_WITH_BOILERPLATE = `# Pull Request Template + describe('checkLegalBoilerplate', () => { + const PR_TEMPLATE_WITH_BOILERPLATE = `# Pull Request Template ## Description Please describe your changes @@ -536,228 +534,226 @@ Look, I get it. The entity doing business as "Sentry" was incorporated in the St ## Checklist - [ ] Tests added`; -// Derived from the template to stay in sync automatically -const LEGAL_BOILERPLATE_SECTION = extractLegalBoilerplateSection(PR_TEMPLATE_WITH_BOILERPLATE); -const LEGAL_TEXT = LEGAL_BOILERPLATE_SECTION.replace('### Legal Boilerplate\n', ''); - -function buildMockContext({ prOverrides = {}, templateContent = PR_TEMPLATE_WITH_BOILERPLATE } = {}) { - const failMessages = []; - const markdownMessages = []; - - const danger = { - github: { - pr: { - author_association: 'CONTRIBUTOR', - body: '', - ...prOverrides - }, - utils: { - fileContents: async (path) => { - if (templateContent && path === '.github/PULL_REQUEST_TEMPLATE.md') { - return templateContent; + // Derived from the template to stay in sync automatically + const LEGAL_BOILERPLATE_SECTION = extractLegalBoilerplateSection(PR_TEMPLATE_WITH_BOILERPLATE); + const LEGAL_TEXT = LEGAL_BOILERPLATE_SECTION.replace('### Legal Boilerplate\n', ''); + + function buildMockContext({ prOverrides = {}, templateContent = PR_TEMPLATE_WITH_BOILERPLATE } = {}) { + const failMessages = []; + const markdownMessages = []; + + const danger = { + github: { + pr: { + author_association: 'CONTRIBUTOR', + body: '', + ...prOverrides + }, + utils: { + fileContents: async (path) => { + if (templateContent && path === '.github/PULL_REQUEST_TEMPLATE.md') { + return templateContent; + } + return ''; + } } - return ''; } - } + }; + + return { + danger, + fail: (msg) => failMessages.push(msg), + markdown: (msg) => markdownMessages.push(msg), + failMessages, + markdownMessages + }; } - }; - - return { - danger, - fail: (msg) => failMessages.push(msg), - markdown: (msg) => markdownMessages.push(msg), - failMessages, - markdownMessages - }; -} - -describe('checkLegalBoilerplate', () => { - // --- Skips for internal contributors --- - - it('should skip check for OWNER association', async () => { - const ctx = buildMockContext({ prOverrides: { author_association: 'OWNER' } }); - await checkLegalBoilerplate(ctx); - assert.strictEqual(ctx.failMessages.length, 0); - assert.strictEqual(ctx.markdownMessages.length, 0); - }); - it('should skip check for MEMBER association', async () => { - const ctx = buildMockContext({ prOverrides: { author_association: 'MEMBER' } }); - await checkLegalBoilerplate(ctx); - assert.strictEqual(ctx.failMessages.length, 0); - }); - - it('should skip check for COLLABORATOR association', async () => { - const ctx = buildMockContext({ prOverrides: { author_association: 'COLLABORATOR' } }); - await checkLegalBoilerplate(ctx); - assert.strictEqual(ctx.failMessages.length, 0); - }); + // --- Skips for internal contributors --- - // --- External contributor associations that should be checked --- - - it('should check for CONTRIBUTOR association', async () => { - const ctx = buildMockContext({ prOverrides: { author_association: 'CONTRIBUTOR', body: '' } }); - await checkLegalBoilerplate(ctx); - assert.strictEqual(ctx.failMessages.length, 1, 'Should fail for external CONTRIBUTOR without boilerplate'); - }); - - it('should check for FIRST_TIME_CONTRIBUTOR association', async () => { - const ctx = buildMockContext({ prOverrides: { author_association: 'FIRST_TIME_CONTRIBUTOR', body: '' } }); - await checkLegalBoilerplate(ctx); - assert.strictEqual(ctx.failMessages.length, 1); - }); - - it('should check for NONE association', async () => { - const ctx = buildMockContext({ prOverrides: { author_association: 'NONE', body: '' } }); - await checkLegalBoilerplate(ctx); - assert.strictEqual(ctx.failMessages.length, 1); - }); + it('should skip check for OWNER association', async () => { + const ctx = buildMockContext({ prOverrides: { author_association: 'OWNER' } }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 0); + assert.strictEqual(ctx.markdownMessages.length, 0); + }); - // --- Template discovery --- + it('should skip check for MEMBER association', async () => { + const ctx = buildMockContext({ prOverrides: { author_association: 'MEMBER' } }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 0); + }); - it('should skip when no PR template is found', async () => { - const ctx = buildMockContext({ templateContent: null }); - await checkLegalBoilerplate(ctx); - assert.strictEqual(ctx.failMessages.length, 0, 'Should not fail when no template exists'); - }); + it('should skip check for COLLABORATOR association', async () => { + const ctx = buildMockContext({ prOverrides: { author_association: 'COLLABORATOR' } }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 0); + }); - it('should skip when template has no Legal Boilerplate section', async () => { - const ctx = buildMockContext({ templateContent: '# Template\n\n## Description\nJust a normal template.' }); - await checkLegalBoilerplate(ctx); - assert.strictEqual(ctx.failMessages.length, 0, 'Should not fail when template lacks legal section'); - }); + // --- External contributor associations that should be checked --- - it('should find template at the first matching path', async () => { - const calledPaths = []; - const ctx = buildMockContext(); - ctx.danger.github.utils.fileContents = async (path) => { - calledPaths.push(path); - // Return content for the second path to verify it tries multiple - if (path === '.github/pull_request_template.md') { - return PR_TEMPLATE_WITH_BOILERPLATE; - } - return ''; - }; - ctx.danger.github.pr.body = `## My PR\n\n### Legal Boilerplate\n${LEGAL_TEXT}`; - await checkLegalBoilerplate(ctx); - assert.ok(calledPaths.includes('.github/PULL_REQUEST_TEMPLATE.md'), 'Should try uppercase path first'); - assert.ok(calledPaths.includes('.github/pull_request_template.md'), 'Should try lowercase path second'); - // Should stop after finding the template (not try remaining paths) - assert.ok(!calledPaths.includes('PULL_REQUEST_TEMPLATE.md'), 'Should stop after finding template'); - }); + it('should check for CONTRIBUTOR association', async () => { + const ctx = buildMockContext({ prOverrides: { author_association: 'CONTRIBUTOR', body: '' } }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 1, 'Should fail for external CONTRIBUTOR without boilerplate'); + }); - // --- Missing boilerplate in PR body --- + it('should check for FIRST_TIME_CONTRIBUTOR association', async () => { + const ctx = buildMockContext({ prOverrides: { author_association: 'FIRST_TIME_CONTRIBUTOR', body: '' } }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 1); + }); - it('should fail when external contributor PR body is empty', async () => { - const ctx = buildMockContext({ prOverrides: { author_association: 'NONE', body: '' } }); - await checkLegalBoilerplate(ctx); - assert.strictEqual(ctx.failMessages.length, 1); - assert.ok(ctx.failMessages[0].includes('missing the required legal boilerplate')); - assert.strictEqual(ctx.markdownMessages.length, 1); - assert.ok(ctx.markdownMessages[0].includes('Legal Boilerplate Required')); - }); + it('should check for NONE association', async () => { + const ctx = buildMockContext({ prOverrides: { author_association: 'NONE', body: '' } }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 1); + }); - it('should fail when external contributor PR body is null', async () => { - const ctx = buildMockContext({ prOverrides: { author_association: 'CONTRIBUTOR', body: null } }); - await checkLegalBoilerplate(ctx); - assert.strictEqual(ctx.failMessages.length, 1); - assert.ok(ctx.failMessages[0].includes('missing the required legal boilerplate')); - }); + // --- Template discovery --- - it('should fail when PR body has no legal section', async () => { - const ctx = buildMockContext({ - prOverrides: { - author_association: 'FIRST_TIME_CONTRIBUTOR', - body: '## Description\nMy cool changes\n\n## Checklist\n- [x] Tests' - } + it('should skip when no PR template is found', async () => { + const ctx = buildMockContext({ templateContent: null }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 0, 'Should not fail when no template exists'); }); - await checkLegalBoilerplate(ctx); - assert.strictEqual(ctx.failMessages.length, 1); - assert.ok(ctx.failMessages[0].includes('missing the required legal boilerplate')); - }); - // --- Boilerplate mismatch --- + it('should skip when template has no Legal Boilerplate section', async () => { + const ctx = buildMockContext({ templateContent: '# Template\n\n## Description\nJust a normal template.' }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 0, 'Should not fail when template lacks legal section'); + }); - it('should fail when boilerplate text is modified', async () => { - const ctx = buildMockContext({ - prOverrides: { - author_association: 'CONTRIBUTOR', - body: '### Legal Boilerplate\nI changed the legal text to something else entirely.' - } + it('should find template at the first matching path', async () => { + const calledPaths = []; + const ctx = buildMockContext(); + ctx.danger.github.utils.fileContents = async (path) => { + calledPaths.push(path); + if (path === '.github/pull_request_template.md') { + return PR_TEMPLATE_WITH_BOILERPLATE; + } + return ''; + }; + ctx.danger.github.pr.body = `## My PR\n\n### Legal Boilerplate\n${LEGAL_TEXT}`; + await checkLegalBoilerplate(ctx); + assert.ok(calledPaths.includes('.github/PULL_REQUEST_TEMPLATE.md'), 'Should try uppercase path first'); + assert.ok(calledPaths.includes('.github/pull_request_template.md'), 'Should try lowercase path second'); + assert.ok(!calledPaths.includes('PULL_REQUEST_TEMPLATE.md'), 'Should stop after finding template'); + }); + + // --- Missing boilerplate in PR body --- + + it('should fail when external contributor PR body is empty', async () => { + const ctx = buildMockContext({ prOverrides: { author_association: 'NONE', body: '' } }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 1); + assert.ok(ctx.failMessages[0].includes('missing the required legal boilerplate')); + assert.strictEqual(ctx.markdownMessages.length, 1); + assert.ok(ctx.markdownMessages[0].includes('Legal Boilerplate Required')); + }); + + it('should fail when external contributor PR body is null', async () => { + const ctx = buildMockContext({ prOverrides: { author_association: 'CONTRIBUTOR', body: null } }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 1); + assert.ok(ctx.failMessages[0].includes('missing the required legal boilerplate')); + }); + + it('should fail when PR body has no legal section', async () => { + const ctx = buildMockContext({ + prOverrides: { + author_association: 'FIRST_TIME_CONTRIBUTOR', + body: '## Description\nMy cool changes\n\n## Checklist\n- [x] Tests' + } + }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 1); + assert.ok(ctx.failMessages[0].includes('missing the required legal boilerplate')); }); - await checkLegalBoilerplate(ctx); - assert.strictEqual(ctx.failMessages.length, 1); - assert.ok(ctx.failMessages[0].includes('does not match the template')); - assert.strictEqual(ctx.markdownMessages.length, 1); - assert.ok(ctx.markdownMessages[0].includes('Legal Boilerplate Mismatch')); - }); - it('should fail when boilerplate is truncated', async () => { - const ctx = buildMockContext({ - prOverrides: { - author_association: 'CONTRIBUTOR', - body: '### Legal Boilerplate\nLook, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015.' - } + // --- Boilerplate mismatch --- + + it('should fail when boilerplate text is modified', async () => { + const ctx = buildMockContext({ + prOverrides: { + author_association: 'CONTRIBUTOR', + body: '### Legal Boilerplate\nI changed the legal text to something else entirely.' + } + }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 1); + assert.ok(ctx.failMessages[0].includes('does not match the template')); + assert.strictEqual(ctx.markdownMessages.length, 1); + assert.ok(ctx.markdownMessages[0].includes('Legal Boilerplate Mismatch')); + }); + + it('should fail when boilerplate is truncated', async () => { + const ctx = buildMockContext({ + prOverrides: { + author_association: 'CONTRIBUTOR', + body: '### Legal Boilerplate\nLook, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015.' + } + }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 1); + assert.ok(ctx.failMessages[0].includes('does not match the template')); }); - await checkLegalBoilerplate(ctx); - assert.strictEqual(ctx.failMessages.length, 1); - assert.ok(ctx.failMessages[0].includes('does not match the template')); - }); - // --- Matching boilerplate (success cases) --- + // --- Matching boilerplate (success cases) --- - it('should pass when boilerplate matches exactly', async () => { - const ctx = buildMockContext({ - prOverrides: { - author_association: 'CONTRIBUTOR', - body: `## Description\nMy changes\n\n### Legal Boilerplate\n${LEGAL_TEXT}\n\n## Checklist\n- [x] Done` - } + it('should pass when boilerplate matches exactly', async () => { + const ctx = buildMockContext({ + prOverrides: { + author_association: 'CONTRIBUTOR', + body: `## Description\nMy changes\n\n### Legal Boilerplate\n${LEGAL_TEXT}\n\n## Checklist\n- [x] Done` + } + }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 0, 'Should not fail when boilerplate matches'); + assert.strictEqual(ctx.markdownMessages.length, 0); }); - await checkLegalBoilerplate(ctx); - assert.strictEqual(ctx.failMessages.length, 0, 'Should not fail when boilerplate matches'); - assert.strictEqual(ctx.markdownMessages.length, 0); - }); - it('should pass when boilerplate matches with different whitespace', async () => { - const ctx = buildMockContext({ - prOverrides: { - author_association: 'CONTRIBUTOR', - body: `### Legal Boilerplate\n${LEGAL_TEXT.replace(/\. /g, '.\n')}` - } + it('should pass when boilerplate matches with different whitespace', async () => { + const ctx = buildMockContext({ + prOverrides: { + author_association: 'CONTRIBUTOR', + body: `### Legal Boilerplate\n${LEGAL_TEXT.replace(/\. /g, '.\n')}` + } + }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 0, 'Should pass with normalized whitespace differences'); }); - await checkLegalBoilerplate(ctx); - assert.strictEqual(ctx.failMessages.length, 0, 'Should pass with normalized whitespace differences'); - }); - it('should pass when boilerplate has extra surrounding whitespace', async () => { - const ctx = buildMockContext({ - prOverrides: { - author_association: 'CONTRIBUTOR', - body: `### Legal Boilerplate\n\n ${LEGAL_TEXT} \n\n## Next` - } + it('should pass when boilerplate has extra surrounding whitespace', async () => { + const ctx = buildMockContext({ + prOverrides: { + author_association: 'CONTRIBUTOR', + body: `### Legal Boilerplate\n\n ${LEGAL_TEXT} \n\n## Next` + } + }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.failMessages.length, 0); }); - await checkLegalBoilerplate(ctx); - assert.strictEqual(ctx.failMessages.length, 0); - }); - // --- Markdown message content --- + // --- Markdown message content --- - it('should include expected boilerplate in the markdown hint when missing', async () => { - const ctx = buildMockContext({ prOverrides: { author_association: 'NONE', body: '' } }); - await checkLegalBoilerplate(ctx); - assert.strictEqual(ctx.markdownMessages.length, 1); - assert.ok(ctx.markdownMessages[0].includes('Functional Software, Inc.'), 'Markdown should include the expected legal text'); - }); + it('should include expected boilerplate in the markdown hint when missing', async () => { + const ctx = buildMockContext({ prOverrides: { author_association: 'NONE', body: '' } }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.markdownMessages.length, 1); + assert.ok(ctx.markdownMessages[0].includes('Functional Software, Inc.'), 'Markdown should include the expected legal text'); + }); - it('should include expected boilerplate in the markdown hint on mismatch', async () => { - const ctx = buildMockContext({ - prOverrides: { - author_association: 'CONTRIBUTOR', - body: '### Legal Boilerplate\nWrong text here.' - } + it('should include expected boilerplate in the markdown hint on mismatch', async () => { + const ctx = buildMockContext({ + prOverrides: { + author_association: 'CONTRIBUTOR', + body: '### Legal Boilerplate\nWrong text here.' + } + }); + await checkLegalBoilerplate(ctx); + assert.strictEqual(ctx.markdownMessages.length, 1); + assert.ok(ctx.markdownMessages[0].includes('Functional Software, Inc.')); }); - await checkLegalBoilerplate(ctx); - assert.strictEqual(ctx.markdownMessages.length, 1); - assert.ok(ctx.markdownMessages[0].includes('Functional Software, Inc.')); }); }); \ No newline at end of file From 6288ebbe78b6b91a566b566caa2dac4bdc6f7851 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Fri, 30 Jan 2026 09:09:11 +0100 Subject: [PATCH 10/13] Add test runner instructions to CONTRIBUTING.md The test command and Node version requirement were only documented in CI workflow YAML, making it hard for new contributors to discover how to run tests locally. Co-Authored-By: Claude Opus 4.5 --- danger/CONTRIBUTING.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/danger/CONTRIBUTING.md b/danger/CONTRIBUTING.md index 5bd26ee..1fd5e14 100644 --- a/danger/CONTRIBUTING.md +++ b/danger/CONTRIBUTING.md @@ -1,5 +1,16 @@ # Contributing +## Running tests + +The test suite uses Node.js's built-in test runner (requires Node 18+): + +```sh +cd danger +node --test +``` + +No dependencies to install — the tests use only `node:test` and `node:assert`. + ## How to run dangerfile locally - [Working on your Dangerfile](https://danger.systems/js/guides/the_dangerfile.html#working-on-your-dangerfile) From 0d3dbfbdeedff386fb8803a967be2bf085edeb7d Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Fri, 30 Jan 2026 09:10:27 +0100 Subject: [PATCH 11/13] Add unit tests for normalizeWhitespace and formatBoilerplateHint These helpers were extracted in the previous refactor but lacked direct unit tests, relying only on indirect coverage through the integration tests. Co-Authored-By: Claude Opus 4.5 --- danger/dangerfile-utils.js | 2 + danger/dangerfile-utils.test.js | 74 ++++++++++++++++++++++++++------- 2 files changed, 61 insertions(+), 15 deletions(-) diff --git a/danger/dangerfile-utils.js b/danger/dangerfile-utils.js index e067084..f23e252 100644 --- a/danger/dangerfile-utils.js +++ b/danger/dangerfile-utils.js @@ -202,5 +202,7 @@ module.exports = { getFlavorConfig, extractPRFlavor, extractLegalBoilerplateSection, + normalizeWhitespace, + formatBoilerplateHint, checkLegalBoilerplate }; diff --git a/danger/dangerfile-utils.test.js b/danger/dangerfile-utils.test.js index 31ca66c..2ee97aa 100644 --- a/danger/dangerfile-utils.test.js +++ b/danger/dangerfile-utils.test.js @@ -1,6 +1,6 @@ const { describe, it } = require('node:test'); const assert = require('node:assert'); -const { getFlavorConfig, extractPRFlavor, extractLegalBoilerplateSection, checkLegalBoilerplate, FLAVOR_CONFIG } = require('./dangerfile-utils.js'); +const { getFlavorConfig, extractPRFlavor, extractLegalBoilerplateSection, normalizeWhitespace, formatBoilerplateHint, checkLegalBoilerplate, FLAVOR_CONFIG } = require('./dangerfile-utils.js'); describe('dangerfile-utils', () => { describe('getFlavorConfig', () => { @@ -292,7 +292,7 @@ Look, I get it. The entity doing business as "Sentry" was incorporated in the St `; const result = extractLegalBoilerplateSection(template); - + assert.ok(result.includes('### Legal Boilerplate'), 'Should include the header'); assert.ok(result.includes('Functional Software, Inc.'), 'Should include the legal text'); assert.ok(!result.includes('## Checklist'), 'Should not include the next section'); @@ -311,7 +311,7 @@ More content `; const result = extractLegalBoilerplateSection(template); - + assert.strictEqual(result.trim(), '## Legal Boilerplate\n\nThis is a legal notice.'); }); @@ -328,7 +328,7 @@ More content testCases.forEach(({ header, text }) => { const template = `${header}\n${text}\n## Next Section`; const result = extractLegalBoilerplateSection(template); - + assert.ok(result.includes(header), `Should extract section with ${header}`); assert.ok(result.includes(text), `Should include text for ${header}`); assert.ok(!result.includes('Next Section'), `Should not include next section for ${header}`); @@ -364,7 +364,7 @@ Third paragraph of legal text. `; const result = extractLegalBoilerplateSection(template); - + assert.ok(result.includes('First paragraph'), 'Should include first paragraph'); assert.ok(result.includes('Second paragraph'), 'Should include second paragraph'); assert.ok(result.includes('Third paragraph'), 'Should include third paragraph'); @@ -383,7 +383,7 @@ Legal text at the end. `; const result = extractLegalBoilerplateSection(template); - + assert.ok(result.includes('### Legal Boilerplate'), 'Should include header'); assert.ok(result.includes('Legal text at the end.'), 'Should include text'); }); @@ -400,7 +400,7 @@ Please describe your changes `; const result = extractLegalBoilerplateSection(template); - + assert.strictEqual(result, '', 'Should return empty string when no legal section found'); }); @@ -412,7 +412,7 @@ Please describe your changes it('should handle template with only legal boilerplate section', () => { const template = '### Legal Boilerplate\nThis is the only content.'; const result = extractLegalBoilerplateSection(template); - + assert.ok(result.includes('### Legal Boilerplate'), 'Should include header'); assert.ok(result.includes('This is the only content.'), 'Should include content'); }); @@ -427,7 +427,7 @@ And some unicode: 你好世界 🎉 `; const result = extractLegalBoilerplateSection(template); - + assert.ok(result.includes('special chars'), 'Should handle special characters'); assert.ok(result.includes('你好世界'), 'Should handle unicode'); assert.ok(result.includes('🎉'), 'Should handle emoji'); @@ -449,7 +449,7 @@ More text. `; const result = extractLegalBoilerplateSection(template); - + assert.ok(result.includes('const legal = true;'), 'Should include code blocks'); assert.ok(result.includes('More text.'), 'Should include text after code block'); assert.ok(!result.includes('Next Section'), 'Should not include next section'); @@ -468,7 +468,7 @@ You agree to: `; const result = extractLegalBoilerplateSection(template); - + assert.ok(result.includes('- Item 1'), 'Should include list items'); assert.ok(result.includes('- Item 2'), 'Should include list items'); assert.ok(result.includes('- Item 3'), 'Should include list items'); @@ -476,13 +476,13 @@ You agree to: it('should handle legal boilerplate with extra whitespace', () => { const template = ` -### Legal Boilerplate +### Legal Boilerplate Content with spaces. ## Next `; const result = extractLegalBoilerplateSection(template); - + assert.ok(result.includes('Content with spaces.'), 'Should handle extra whitespace in header'); }); @@ -495,7 +495,7 @@ This should not be included. `; const result = extractLegalBoilerplateSection(template); - + assert.ok(result.includes('First section content.'), 'Should include first section'); assert.ok(!result.includes('This should not be included.'), 'Should stop at next header'); }); @@ -522,6 +522,50 @@ Second paragraph with blank lines above. }); }); + describe('normalizeWhitespace', () => { + it('should collapse multiple spaces into one', () => { + assert.strictEqual(normalizeWhitespace('hello world'), 'hello world'); + }); + + it('should collapse tabs and newlines into spaces', () => { + assert.strictEqual(normalizeWhitespace("hello\t\nworld"), 'hello world'); + }); + + it('should trim leading and trailing whitespace', () => { + assert.strictEqual(normalizeWhitespace(' hello world '), 'hello world'); + }); + + it('should return empty string for whitespace-only input', () => { + assert.strictEqual(normalizeWhitespace(' \t\n '), ''); + }); + + it('should leave single-spaced text unchanged', () => { + assert.strictEqual(normalizeWhitespace('already clean'), 'already clean'); + }); + }); + + describe('formatBoilerplateHint', () => { + it('should include the title with emoji prefix', () => { + const result = formatBoilerplateHint('My Title', 'Some description.', 'boilerplate text'); + assert.ok(result.includes('### ⚖️ My Title')); + }); + + it('should include the description', () => { + const result = formatBoilerplateHint('Title', 'Please fix this.', 'boilerplate'); + assert.ok(result.includes('Please fix this.')); + }); + + it('should include the boilerplate in a markdown code block', () => { + const result = formatBoilerplateHint('Title', 'Desc', 'expected legal text'); + assert.ok(result.includes('```markdown\nexpected legal text\n```')); + }); + + it('should include the IP rights notice', () => { + const result = formatBoilerplateHint('Title', 'Desc', 'text'); + assert.ok(result.includes('intellectual property rights')); + }); + }); + describe('checkLegalBoilerplate', () => { const PR_TEMPLATE_WITH_BOILERPLATE = `# Pull Request Template @@ -756,4 +800,4 @@ Look, I get it. The entity doing business as "Sentry" was incorporated in the St assert.ok(ctx.markdownMessages[0].includes('Functional Software, Inc.')); }); }); -}); \ No newline at end of file +}); From 58c7162625205faa2123352ccd45592b3e5e6189 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Fri, 30 Jan 2026 09:11:24 +0100 Subject: [PATCH 12/13] Log author_association and use consistent comment style Add debug log of the actual author_association value before the internal/external check, making it easier to troubleshoot unexpected association values. Also normalize comment style to /// to match the rest of the file. Co-Authored-By: Claude Opus 4.5 --- danger/dangerfile-utils.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/danger/dangerfile-utils.js b/danger/dangerfile-utils.js index f23e252..7689705 100644 --- a/danger/dangerfile-utils.js +++ b/danger/dangerfile-utils.js @@ -117,12 +117,12 @@ const PR_TEMPLATE_PATHS = [ '.github/PULL_REQUEST_TEMPLATE/pull_request_template.md' ]; -/** Collapse all whitespace runs into single spaces for comparison. */ +/// Collapse all whitespace runs into single spaces for comparison. function normalizeWhitespace(str) { return str.replace(/\s+/g, ' ').trim(); } -/** Try each known PR template path and return the first one with content. */ +/// Try each known PR template path and return the first one with content. async function findPRTemplate(danger) { for (const templatePath of PR_TEMPLATE_PATHS) { const content = await danger.github.utils.fileContents(templatePath); @@ -134,7 +134,7 @@ async function findPRTemplate(danger) { return null; } -/** Build a markdown hint showing the expected boilerplate text. */ +/// Build a markdown hint showing the expected boilerplate text. function formatBoilerplateHint(title, description, expectedBoilerplate) { return `### ⚖️ ${title} @@ -147,14 +147,13 @@ ${expectedBoilerplate} This is required to ensure proper intellectual property rights for your contributions.`; } -/** - * Check that external contributors include the required legal boilerplate in their PR body. - * Accepts danger context and reporting functions as parameters for testability. - */ +/// Check that external contributors include the required legal boilerplate in their PR body. +/// Accepts danger context and reporting functions as parameters for testability. async function checkLegalBoilerplate({ danger, fail, markdown }) { console.log('::debug:: Checking legal boilerplate requirements...'); const authorAssociation = danger.github.pr.author_association; + console.log(`::debug:: PR author_association: ${authorAssociation}`); if (INTERNAL_ASSOCIATIONS.includes(authorAssociation)) { console.log('::debug:: Skipping legal boilerplate check for organization member/collaborator'); return; From 8ffb2c9e230122590c7d0db8777eef9be39ab73f Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Fri, 30 Jan 2026 09:21:35 +0100 Subject: [PATCH 13/13] Add changelog entry for legal boilerplate validation Co-Authored-By: Claude Opus 4.5 --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fa6c5bf..2fb96d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ ## Unreleased +### Features + +- Danger - Add legal boilerplate validation for external contributors ([#145](https://github.com/getsentry/github-workflows/pull/145)) + - Verify that PRs from non-organization members include the required legal boilerplate from the PR template + - Show actionable markdown hints when the boilerplate is missing or doesn't match + ### Fixes - Sentry-CLI integration test action - Accept chunked ProGuard uploads for compatibility with Sentry CLI 3.x ([#140](https://github.com/getsentry/github-workflows/pull/140))