diff --git a/.github/scripts/package.json b/.github/scripts/package.json index 161ba425a60..b3ef3607692 100644 --- a/.github/scripts/package.json +++ b/.github/scripts/package.json @@ -9,6 +9,7 @@ "conventional-changelog": "7.2.0", "debug": "4.4.3", "glob": "13.0.6", + "minimatch": "10.2.4", "semver": "7.7.4", "tempfile": "6.0.1" }, diff --git a/.github/scripts/pnpm-lock.yaml b/.github/scripts/pnpm-lock.yaml index b547faa90a8..ebb2db3ba38 100644 --- a/.github/scripts/pnpm-lock.yaml +++ b/.github/scripts/pnpm-lock.yaml @@ -23,6 +23,9 @@ importers: glob: specifier: 13.0.6 version: 13.0.6 + minimatch: + specifier: 10.2.4 + version: 10.2.4 semver: specifier: 7.7.4 version: 7.7.4 diff --git a/.github/scripts/quality/check-pr-size.mjs b/.github/scripts/quality/check-pr-size.mjs index 775e233853f..554b7214d5c 100644 --- a/.github/scripts/quality/check-pr-size.mjs +++ b/.github/scripts/quality/check-pr-size.mjs @@ -1,6 +1,9 @@ /** * Checks that the PR does not exceed the line addition limit. * + * Files matching any pattern in EXCLUDE_PATTERNS are not counted toward the + * limit (e.g. test files, snapshots). + * * A maintainer (write access or above) can override by commenting `/size-limit-override` * on the PR. The override takes effect on the next pull_request event (push, reopen, etc.). * @@ -9,18 +12,43 @@ * 1 – PR exceeds the limit with no valid override */ +import { minimatch } from 'minimatch'; import { initGithub, getEventFromGithubEventPath } from '../github-helpers.mjs'; export const SIZE_LIMIT = 1000; export const OVERRIDE_COMMAND = '/size-limit-override'; +export const EXCLUDE_PATTERNS = [ + // Test files (by extension) + '**/*.test.ts', + '**/*.test.js', + '**/*.test.mjs', + '**/*.spec.ts', + '**/*.spec.js', + '**/*.spec.mjs', + // Test directories + '**/test/**', + '**/tests/**', + '**/__tests__/**', + // Snapshots + '**/__snapshots__/**', + '**/*.snap', + // Fixtures and mocks + '**/fixtures/**', + '**/__mocks__/**', + // Dedicated testing package + 'packages/testing/**', + // Lock file (can produce massive diffs on dependency changes) + 'pnpm-lock.yaml', +]; + const BOT_MARKER = ''; /** * Returns true if any comment in the list is a valid `/size-limit-override` from a * user with write access or above. * - * @param {Array<{ body?: string, user: { login: string } }>} comments + * @param {Array<{ body?: string, user: { login: string } | null }>} comments * @param {(username: string) => Promise} getPermission - returns the permission level string * @returns {Promise} */ @@ -30,6 +58,10 @@ export async function hasValidOverride(comments, getPermission) { continue; } + if (!comment.user) { + return false; + } + const perm = await getPermission(comment.user.login); if (['admin', 'write', 'maintain'].includes(perm)) { return true; @@ -38,12 +70,32 @@ export async function hasValidOverride(comments, getPermission) { return false; } +/** + * Returns the total additions across all files, excluding those matching any exclude pattern. + * + * @param {Array<{ filename: string, additions: number }>} files + * @param {string[]} excludePatterns + * @returns {number} + */ +export function countFilteredAdditions(files, excludePatterns) { + return files + .filter((file) => !excludePatterns.some((pattern) => minimatch(file.filename, pattern))) + .reduce((sum, file) => sum + file.additions, 0); +} + async function main() { const event = getEventFromGithubEventPath(); const pr = event.pull_request; const { octokit, owner, repo } = initGithub(); - const additions = pr.additions; + const files = await octokit.paginate(octokit.rest.pulls.listFiles, { + owner, + repo, + pull_number: pr.number, + per_page: 100, + }); + + const additions = countFilteredAdditions(files, EXCLUDE_PATTERNS); const { data: comments } = await octokit.rest.issues.listComments({ owner, @@ -63,14 +115,14 @@ async function main() { return perm.permission; }); - const botComment = comments.find((c) => c.body.includes(BOT_MARKER)); + const botComment = comments.find((c) => c.body?.includes(BOT_MARKER)); if (additions > SIZE_LIMIT && !overrideFound) { const message = [ BOT_MARKER, - `## ⚠️ PR exceeds size limit (${additions.toLocaleString()} lines added)`, + `## ! PR exceeds size limit (${additions.toLocaleString()} lines added)`, '', - `This PR adds **${additions.toLocaleString()} lines**, exceeding the ${SIZE_LIMIT.toLocaleString()}-line limit.`, + `This PR adds **${additions.toLocaleString()} lines**, exceeding the ${SIZE_LIMIT.toLocaleString()}-line limit (test files excluded).`, '', 'Large PRs are harder to review and increase the risk of bugs going unnoticed. Please consider:', '- Breaking this into smaller, logically separate PRs', @@ -96,7 +148,7 @@ async function main() { } console.log( - `::error::PR adds ${additions.toLocaleString()} lines, exceeding the ${SIZE_LIMIT.toLocaleString()}-line limit. Reduce PR size or ask a maintainer to comment \`${OVERRIDE_COMMAND}\`.`, + `::error::PR adds ${additions.toLocaleString()} lines (test files excluded), exceeding the ${SIZE_LIMIT.toLocaleString()}-line limit. Reduce PR size or ask a maintainer to comment \`${OVERRIDE_COMMAND}\`.`, ); process.exit(1); } else { @@ -109,7 +161,7 @@ async function main() { } if (overrideFound && additions > SIZE_LIMIT) { console.log( - `PR size limit overridden. ${additions.toLocaleString()} lines added (limit: ${SIZE_LIMIT.toLocaleString()}).`, + `PR size limit overridden. ${additions.toLocaleString()} lines added (limit: ${SIZE_LIMIT.toLocaleString()}, test files excluded).`, ); } } diff --git a/.github/scripts/quality/check-pr-size.test.mjs b/.github/scripts/quality/check-pr-size.test.mjs index f9cab62d30e..a52422808f6 100644 --- a/.github/scripts/quality/check-pr-size.test.mjs +++ b/.github/scripts/quality/check-pr-size.test.mjs @@ -13,9 +13,10 @@ mock.module('../github-helpers.mjs', { }, }); -let hasValidOverride, SIZE_LIMIT, OVERRIDE_COMMAND; +let hasValidOverride, countFilteredAdditions, SIZE_LIMIT, OVERRIDE_COMMAND, EXCLUDE_PATTERNS; before(async () => { - ({ hasValidOverride, SIZE_LIMIT, OVERRIDE_COMMAND } = await import('./check-pr-size.mjs')); + ({ hasValidOverride, countFilteredAdditions, SIZE_LIMIT, OVERRIDE_COMMAND, EXCLUDE_PATTERNS } = + await import('./check-pr-size.mjs')); }); /** @param {string} permission */ @@ -114,3 +115,92 @@ describe('hasValidOverride', () => { assert.ok(result); }); }); + +describe('countFilteredAdditions', () => { + it('sums additions across all files when no patterns are given', () => { + const files = [ + { filename: 'src/foo.ts', additions: 100 }, + { filename: 'src/bar.ts', additions: 200 }, + ]; + assert.equal(countFilteredAdditions(files, []), 300); + }); + + it('excludes files matching a glob pattern', () => { + const files = [ + { filename: 'src/foo.ts', additions: 100 }, + { filename: 'src/foo.test.ts', additions: 500 }, + ]; + assert.equal(countFilteredAdditions(files, ['**/*.test.ts']), 100); + }); + + it('excludes files matching any of multiple patterns', () => { + const files = [ + { filename: 'src/foo.ts', additions: 100 }, + { filename: 'src/foo.test.ts', additions: 200 }, + { filename: 'src/foo.spec.ts', additions: 300 }, + { filename: 'src/__tests__/bar.ts', additions: 400 }, + ]; + assert.equal( + countFilteredAdditions(files, ['**/*.test.ts', '**/*.spec.ts', '**/__tests__/**']), + 100, + ); + }); + + it('returns 0 when all files are excluded', () => { + const files = [ + { filename: 'src/foo.test.ts', additions: 100 }, + { filename: 'src/bar.test.ts', additions: 200 }, + ]; + assert.equal(countFilteredAdditions(files, ['**/*.test.ts']), 0); + }); + + it('returns 0 for an empty file list', () => { + assert.equal(countFilteredAdditions([], EXCLUDE_PATTERNS), 0); + }); + + it('applies EXCLUDE_PATTERNS to common test file extensions', () => { + const files = [ + { filename: 'src/service.ts', additions: 50 }, + { filename: 'src/service.test.ts', additions: 100 }, + { filename: 'src/service.spec.ts', additions: 100 }, + { filename: 'src/service.test.mjs', additions: 100 }, + { filename: 'src/service.spec.mjs', additions: 100 }, + { filename: 'src/service.test.js', additions: 100 }, + { filename: 'src/service.spec.js', additions: 100 }, + { filename: 'src/__tests__/helper.ts', additions: 100 }, + { filename: 'src/component.snap', additions: 100 }, + ]; + assert.equal(countFilteredAdditions(files, EXCLUDE_PATTERNS), 50); + }); + + it('applies EXCLUDE_PATTERNS to test directories (test/, tests/, __tests__)', () => { + const files = [ + { filename: 'packages/cli/src/service.ts', additions: 50 }, + { filename: 'packages/cli/test/unit/service.test.ts', additions: 100 }, + { filename: 'packages/cli/test/integration/api.test.ts', additions: 100 }, + { filename: 'packages/nodes-base/nodes/Foo/tests/Foo.test.ts', additions: 100 }, + { filename: 'packages/core/src/__tests__/cipher.test.ts', additions: 100 }, + ]; + assert.equal(countFilteredAdditions(files, EXCLUDE_PATTERNS), 50); + }); + + it('applies EXCLUDE_PATTERNS to snapshots, fixtures, and mocks', () => { + const files = [ + { filename: 'packages/cli/src/service.ts', additions: 50 }, + { filename: 'packages/editor-ui/src/__snapshots__/Canvas.test.ts.snap', additions: 100 }, + { filename: 'packages/workflow/test/fixtures/workflow.json', additions: 100 }, + { filename: 'packages/core/src/__mocks__/fs.ts', additions: 100 }, + ]; + assert.equal(countFilteredAdditions(files, EXCLUDE_PATTERNS), 50); + }); + + it('applies EXCLUDE_PATTERNS to packages/testing and pnpm-lock.yaml', () => { + const files = [ + { filename: 'packages/cli/src/service.ts', additions: 50 }, + { filename: 'packages/testing/playwright/tests/workflow.spec.ts', additions: 100 }, + { filename: 'packages/testing/playwright/pages/CanvasPage.ts', additions: 100 }, + { filename: 'pnpm-lock.yaml', additions: 500 }, + ]; + assert.equal(countFilteredAdditions(files, EXCLUDE_PATTERNS), 50); + }); +});