mirror of
https://github.com/n8n-io/n8n.git
synced 2026-06-19 07:36:52 +00:00
ci: Post file stats on PRs (#32053)
This commit is contained in:
@@ -407,6 +407,60 @@ export async function getChangedFiles(pullRequestNumber) {
|
||||
]);
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns all files changed in a PR with full metadata including line counts.
|
||||
*
|
||||
* @param { number } pullRequestNumber
|
||||
* @returns { Promise<Array<{ filename: string, additions: number, deletions: number, previous_filename?: string }>> }
|
||||
* */
|
||||
export async function getPrFiles(pullRequestNumber) {
|
||||
const { octokit, owner, repo } = initGithub();
|
||||
|
||||
return await octokit.paginate(octokit.rest.pulls.listFiles, {
|
||||
owner,
|
||||
repo,
|
||||
pull_number: pullRequestNumber,
|
||||
per_page: 100,
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Post a PR comment, or update the existing one if a previous run already
|
||||
* left one identified by the provided bot marker.
|
||||
*
|
||||
* @param { number } pullRequestNumber
|
||||
* @param { string } body
|
||||
* @param { string } botMarker
|
||||
*/
|
||||
export async function postOrUpdateComment(pullRequestNumber, body, botMarker) {
|
||||
const { octokit, owner, repo } = initGithub();
|
||||
|
||||
const comments = await octokit.paginate(octokit.rest.issues.listComments, {
|
||||
owner,
|
||||
repo,
|
||||
issue_number: pullRequestNumber,
|
||||
per_page: 100,
|
||||
});
|
||||
|
||||
const existing = comments.find((c) => c.body?.includes(botMarker));
|
||||
|
||||
if (existing) {
|
||||
await octokit.rest.issues.updateComment({
|
||||
owner,
|
||||
repo,
|
||||
comment_id: existing.id,
|
||||
body,
|
||||
});
|
||||
} else {
|
||||
await octokit.rest.issues.createComment({
|
||||
owner,
|
||||
repo,
|
||||
issue_number: pullRequestNumber,
|
||||
body,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @param {string} tag
|
||||
*/
|
||||
|
||||
@@ -0,0 +1,90 @@
|
||||
import { afterEach, beforeEach, describe, it, mock } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
|
||||
/**
|
||||
* Run these tests by running
|
||||
*
|
||||
* node --test --experimental-test-module-mocks ./.github/scripts/github-helpers.test.mjs
|
||||
* */
|
||||
|
||||
let octokitImpl;
|
||||
|
||||
mock.module('@actions/github', {
|
||||
namedExports: {
|
||||
getOctokit: () => octokitImpl(),
|
||||
},
|
||||
});
|
||||
|
||||
const { postOrUpdateComment } = await import('./github-helpers.mjs');
|
||||
|
||||
const ORIGINAL_ENV = { ...process.env };
|
||||
|
||||
describe('postOrUpdateComment', () => {
|
||||
beforeEach(() => {
|
||||
process.env.GITHUB_TOKEN = 'token';
|
||||
process.env.GITHUB_REPOSITORY = 'n8n-io/n8n';
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
process.env = { ...ORIGINAL_ENV };
|
||||
});
|
||||
|
||||
it('creates a new comment when no existing bot-marker comment is found', async () => {
|
||||
const createComment = mock.fn(async () => {});
|
||||
const updateComment = mock.fn(async () => {});
|
||||
const paginate = mock.fn(async () => [{ id: 1, body: 'unrelated comment' }]);
|
||||
octokitImpl = () => ({
|
||||
paginate,
|
||||
rest: {
|
||||
issues: {
|
||||
listComments: {},
|
||||
createComment,
|
||||
updateComment,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
await postOrUpdateComment(123, 'new body', '<!-- marker -->');
|
||||
|
||||
assert.equal(updateComment.mock.calls.length, 0);
|
||||
assert.equal(createComment.mock.calls.length, 1);
|
||||
assert.deepEqual(createComment.mock.calls[0].arguments[0], {
|
||||
owner: 'n8n-io',
|
||||
repo: 'n8n',
|
||||
issue_number: 123,
|
||||
body: 'new body',
|
||||
});
|
||||
assert.deepEqual(paginate.mock.calls[0].arguments[1], {
|
||||
owner: 'n8n-io',
|
||||
repo: 'n8n',
|
||||
issue_number: 123,
|
||||
per_page: 100,
|
||||
});
|
||||
});
|
||||
|
||||
it('updates the existing comment when a bot-marker comment is found', async () => {
|
||||
const createComment = mock.fn(async () => {});
|
||||
const updateComment = mock.fn(async () => {});
|
||||
octokitImpl = () => ({
|
||||
paginate: mock.fn(async () => [{ id: 42, body: '<!-- marker -->\nold body' }]),
|
||||
rest: {
|
||||
issues: {
|
||||
listComments: {},
|
||||
createComment,
|
||||
updateComment,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
await postOrUpdateComment(123, 'updated body', '<!-- marker -->');
|
||||
|
||||
assert.equal(createComment.mock.calls.length, 0);
|
||||
assert.equal(updateComment.mock.calls.length, 1);
|
||||
assert.deepEqual(updateComment.mock.calls[0].arguments[0], {
|
||||
owner: 'n8n-io',
|
||||
repo: 'n8n',
|
||||
comment_id: 42,
|
||||
body: 'updated body',
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -1,104 +1,254 @@
|
||||
import { ensureEnvVar, getChangedFiles, initGithub } from "./github-helpers.mjs";
|
||||
import { assignOwnership, ownershipsToAllocations, parseOwnersFile } from "./owners.mjs";
|
||||
/**
|
||||
* PR recommendations entry point.
|
||||
*
|
||||
* Posts (or updates) a single PR comment that combines:
|
||||
* - Recommended reviewer teams based on file ownership
|
||||
* - A breakdown of changed lines by category (source code, test files, misc)
|
||||
*
|
||||
* Advisory only — does not gate merging.
|
||||
*/
|
||||
|
||||
import { minimatch } from 'minimatch';
|
||||
import { ensureEnvVar, getPrFiles, postOrUpdateComment } from './github-helpers.mjs';
|
||||
import { assignOwnership, ownershipsToAllocations, parseOwnersFile } from './owners.mjs';
|
||||
import { MISC_PATTERNS, SIZE_LIMIT, TEST_PATTERNS } from './quality/check-pr-size.mjs';
|
||||
|
||||
/** @typedef {import('./owners.mjs').Allocation} Allocation */
|
||||
|
||||
/**
|
||||
* @param { number } pullRequestNumber
|
||||
* */
|
||||
export async function getReviewRecommendations(pullRequestNumber) {
|
||||
const changedFiles = await getChangedFiles(pullRequestNumber);
|
||||
const owners = parseOwnersFile();
|
||||
* @typedef {{
|
||||
* sourceCodeAdded: number, sourceCodeRemoved: number,
|
||||
* testFilesAdded: number, testFilesRemoved: number,
|
||||
* miscAdded: number, miscRemoved: number,
|
||||
* }} LineStats
|
||||
*/
|
||||
|
||||
const ownerships = assignOwnership(changedFiles, owners);
|
||||
const allocations = ownershipsToAllocations(ownerships);
|
||||
const BOT_MARKER = '<!-- pr-recommendations -->';
|
||||
|
||||
const topAllocations = allocations.sort((a, b) => b.fileCount - a.fileCount).slice(0, 3);
|
||||
|
||||
await commentOnPrWithRecommendations(pullRequestNumber, topAllocations, changedFiles);
|
||||
function createEmptyLineStats() {
|
||||
return {
|
||||
sourceCodeAdded: 0,
|
||||
sourceCodeRemoved: 0,
|
||||
testFilesAdded: 0,
|
||||
testFilesRemoved: 0,
|
||||
miscAdded: 0,
|
||||
miscRemoved: 0,
|
||||
};
|
||||
}
|
||||
|
||||
const BOT_MARKER = "<!-- owners-review-recommendations -->";
|
||||
/**
|
||||
* @param { LineStats } stats
|
||||
* @param {{ filename: string, additions: number, deletions: number }} file
|
||||
*/
|
||||
function addFileToLineStats(stats, file) {
|
||||
const isTest = TEST_PATTERNS.some((p) => minimatch(file.filename, p));
|
||||
const isMisc = !isTest && MISC_PATTERNS.some((p) => minimatch(file.filename, p));
|
||||
|
||||
if (isTest) {
|
||||
stats.testFilesAdded += file.additions;
|
||||
stats.testFilesRemoved += file.deletions;
|
||||
} else if (isMisc) {
|
||||
stats.miscAdded += file.additions;
|
||||
stats.miscRemoved += file.deletions;
|
||||
} else {
|
||||
stats.sourceCodeAdded += file.additions;
|
||||
stats.sourceCodeRemoved += file.deletions;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the PR comment body listing the top reviewer teams with their share
|
||||
* of ownership over the changed files.
|
||||
* Compute line addition and deletion counts categorised as source code,
|
||||
* test files, or misc.
|
||||
*
|
||||
* @param { Array<{ filename: string, additions: number, deletions: number }> } files
|
||||
* @returns { LineStats }
|
||||
*/
|
||||
export function computeLineStats(files) {
|
||||
const stats = createEmptyLineStats();
|
||||
|
||||
for (const file of files) {
|
||||
addFileToLineStats(stats, file);
|
||||
}
|
||||
|
||||
return stats;
|
||||
}
|
||||
|
||||
/**
|
||||
* Compute line stats for each owner allocation. Renamed files can be owned by
|
||||
* either their previous or current filename, but are counted only once per team.
|
||||
*
|
||||
* @param { Allocation[] } allocations
|
||||
* @param { Map<string, string[]> } ownerships
|
||||
* @param { Array<{ filename: string, previous_filename?: string, additions: number, deletions: number }> } files
|
||||
* @returns { Map<string, LineStats> }
|
||||
*/
|
||||
export function computeAllocationLineStats(allocations, ownerships, files) {
|
||||
const statsByTeam = new Map();
|
||||
|
||||
for (const { team } of allocations) {
|
||||
const stats = createEmptyLineStats();
|
||||
const ownedFiles = new Set(ownerships.get(team) ?? []);
|
||||
|
||||
for (const file of files) {
|
||||
if (ownedFiles.has(file.filename) || ownedFiles.has(file.previous_filename)) {
|
||||
addFileToLineStats(stats, file);
|
||||
}
|
||||
}
|
||||
|
||||
statsByTeam.set(team, stats);
|
||||
}
|
||||
|
||||
return statsByTeam;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param { LineStats } lineStats
|
||||
* @param {'sourceCode' | 'testFiles' | 'misc'} key
|
||||
* @returns { string }
|
||||
*/
|
||||
function formatLineStatsCell(lineStats, key) {
|
||||
return `+${lineStats[`${key}Added`].toLocaleString()} / -${lineStats[`${key}Removed`].toLocaleString()}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param { LineStats } lineStats
|
||||
* @returns { number }
|
||||
*/
|
||||
function totalLineChanges(lineStats) {
|
||||
return (
|
||||
lineStats.sourceCodeAdded +
|
||||
lineStats.sourceCodeRemoved +
|
||||
lineStats.testFilesAdded +
|
||||
lineStats.testFilesRemoved +
|
||||
lineStats.miscAdded +
|
||||
lineStats.miscRemoved
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* @param { LineStats } target
|
||||
* @param { LineStats } source
|
||||
*/
|
||||
function addLineStats(target, source) {
|
||||
target.sourceCodeAdded += source.sourceCodeAdded;
|
||||
target.sourceCodeRemoved += source.sourceCodeRemoved;
|
||||
target.testFilesAdded += source.testFilesAdded;
|
||||
target.testFilesRemoved += source.testFilesRemoved;
|
||||
target.miscAdded += source.miscAdded;
|
||||
target.miscRemoved += source.miscRemoved;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param { Allocation[] } allocations
|
||||
* @param { Map<string, LineStats> } lineStatsByTeam
|
||||
* @returns { LineStats }
|
||||
*/
|
||||
function aggregateLineStats(allocations, lineStatsByTeam) {
|
||||
const stats = createEmptyLineStats();
|
||||
|
||||
for (const { team } of allocations) {
|
||||
addLineStats(stats, lineStatsByTeam.get(team) ?? createEmptyLineStats());
|
||||
}
|
||||
|
||||
return stats;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build an ownership-first overview table with line stats grouped by team.
|
||||
*
|
||||
* @param { Allocation[] } allocations
|
||||
* @param { Set<string> } changedFiles
|
||||
* @param { LineStats } totalLineStats
|
||||
* @param { Map<string, LineStats> } lineStatsByTeam
|
||||
* @param { Allocation[] } [otherAllocations]
|
||||
* @returns { string }
|
||||
* */
|
||||
export function buildRecommendationsBody(allocations, changedFiles) {
|
||||
*/
|
||||
export function buildOverviewTable(allocations, changedFiles, totalLineStats, lineStatsByTeam, otherAllocations = []) {
|
||||
const total = changedFiles.size;
|
||||
const rows = allocations.length > 0 && total > 0
|
||||
? allocations.map(({ team, fileCount }) => {
|
||||
const pct = Math.round((fileCount / total) * 100);
|
||||
const teamLineStats = lineStatsByTeam.get(team) ?? createEmptyLineStats();
|
||||
return `| ${team} | ${fileCount} | ${pct}% | ${formatLineStatsCell(teamLineStats, 'sourceCode')} | ${formatLineStatsCell(teamLineStats, 'testFiles')} | ${formatLineStatsCell(teamLineStats, 'misc')} |`;
|
||||
})
|
||||
: [`| _No owning teams matched_ | 0 | 0% | ${formatLineStatsCell(totalLineStats, 'sourceCode')} | ${formatLineStatsCell(totalLineStats, 'testFiles')} | ${formatLineStatsCell(totalLineStats, 'misc')} |`];
|
||||
|
||||
if (allocations.length === 0 || total === 0) {
|
||||
return [
|
||||
BOT_MARKER,
|
||||
"## Recommended reviewers",
|
||||
"",
|
||||
"_No owning teams matched the files changed in this PR._",
|
||||
].join("\n");
|
||||
if (otherAllocations.length > 0 && total > 0) {
|
||||
const otherFileCount = otherAllocations.reduce((sum, { fileCount }) => sum + fileCount, 0);
|
||||
const otherPct = Math.round((otherFileCount / total) * 100);
|
||||
const otherLineStats = aggregateLineStats(otherAllocations, lineStatsByTeam);
|
||||
rows.push(`| Other teams | ${otherFileCount} | ${otherPct}% | ${formatLineStatsCell(otherLineStats, 'sourceCode')} | ${formatLineStatsCell(otherLineStats, 'testFiles')} | ${formatLineStatsCell(otherLineStats, 'misc')} |`);
|
||||
}
|
||||
|
||||
const rows = allocations.map(({ team, fileCount }) => {
|
||||
const pct = Math.round((fileCount / total) * 100);
|
||||
return `| ${team} | ${fileCount} | ${pct}% |`;
|
||||
});
|
||||
|
||||
return [
|
||||
BOT_MARKER,
|
||||
"## Recommended reviewers",
|
||||
"",
|
||||
`Based on ownership of the ${total} changed file${total === 1 ? "" : "s"} in this PR:`,
|
||||
"",
|
||||
"| Team | Files owned | Share |",
|
||||
"| --- | ---: | ---: |",
|
||||
'## PR review overview',
|
||||
'',
|
||||
`Based on ownership of the ${total} changed file${total === 1 ? '' : 's'} in this PR:`,
|
||||
'',
|
||||
'| Ownership | Files owned | Share | Source code | Test files | Misc |',
|
||||
'| --- | ---: | ---: | ---: | ---: | ---: |',
|
||||
...rows,
|
||||
].join("\n");
|
||||
`| **Total** | **${total.toLocaleString()}** | **${total === 0 ? 0 : 100}%** | **${formatLineStatsCell(totalLineStats, 'sourceCode')}** | **${formatLineStatsCell(totalLineStats, 'testFiles')}** | **${formatLineStatsCell(totalLineStats, 'misc')}** |`,
|
||||
].join('\n');
|
||||
}
|
||||
|
||||
/**
|
||||
* Post the recommendations as a PR comment, or update the existing one if a
|
||||
* previous run already left one (identified by BOT_MARKER).
|
||||
* Construct the full PR comment body from reviewer allocations and line stats.
|
||||
*
|
||||
* @param { number } pullRequestNumber
|
||||
* @param { Allocation[] } allocations
|
||||
* @param { Set<string> } changedFiles
|
||||
* */
|
||||
export async function commentOnPrWithRecommendations(pullRequestNumber, allocations, changedFiles) {
|
||||
const { octokit, owner, repo } = initGithub();
|
||||
* @param { LineStats } lineStats
|
||||
* @param { Map<string, LineStats> } lineStatsByTeam
|
||||
* @param { Allocation[] } [otherAllocations]
|
||||
* @returns { string }
|
||||
*/
|
||||
export function buildComment(allocations, changedFiles, lineStats, lineStatsByTeam = new Map(), otherAllocations = []) {
|
||||
const body = [
|
||||
BOT_MARKER,
|
||||
buildOverviewTable(allocations, changedFiles, lineStats, lineStatsByTeam, otherAllocations),
|
||||
];
|
||||
|
||||
const body = buildRecommendationsBody(allocations, changedFiles);
|
||||
|
||||
const comments = await octokit.paginate(octokit.rest.issues.listComments, {
|
||||
owner,
|
||||
repo,
|
||||
issue_number: pullRequestNumber,
|
||||
per_page: 100,
|
||||
});
|
||||
|
||||
const existing = comments.find(c => c.body?.includes(BOT_MARKER));
|
||||
|
||||
if (existing) {
|
||||
await octokit.rest.issues.updateComment({
|
||||
owner,
|
||||
repo,
|
||||
comment_id: existing.id,
|
||||
body,
|
||||
});
|
||||
} else {
|
||||
await octokit.rest.issues.createComment({
|
||||
owner,
|
||||
repo,
|
||||
issue_number: pullRequestNumber,
|
||||
body,
|
||||
});
|
||||
if (lineStats.sourceCodeAdded > SIZE_LIMIT) {
|
||||
body.push('', `❗ Source code additions (${lineStats.sourceCodeAdded.toLocaleString()}) exceed the ${SIZE_LIMIT.toLocaleString()}-line limit.`);
|
||||
}
|
||||
|
||||
return body.join('\n');
|
||||
}
|
||||
|
||||
/**
|
||||
* @param { number } pullRequestNumber
|
||||
*/
|
||||
export async function run(pullRequestNumber) {
|
||||
const files = await getPrFiles(pullRequestNumber);
|
||||
|
||||
const changedFiles = new Set([
|
||||
...files.map((f) => f.filename),
|
||||
...files.filter((f) => f.previous_filename).map((f) => f.previous_filename),
|
||||
]);
|
||||
|
||||
const lineStats = computeLineStats(files);
|
||||
|
||||
const owners = parseOwnersFile();
|
||||
const ownerships = assignOwnership(changedFiles, owners);
|
||||
const allocations = ownershipsToAllocations(ownerships);
|
||||
const lineStatsByTeam = computeAllocationLineStats(allocations, ownerships, files);
|
||||
const sortedAllocations = allocations
|
||||
.toSorted((a, b) => {
|
||||
const lineChangeDiff =
|
||||
totalLineChanges(lineStatsByTeam.get(b.team) ?? createEmptyLineStats()) -
|
||||
totalLineChanges(lineStatsByTeam.get(a.team) ?? createEmptyLineStats());
|
||||
|
||||
return lineChangeDiff || b.fileCount - a.fileCount;
|
||||
});
|
||||
const topAllocations = sortedAllocations.slice(0, 3);
|
||||
const otherAllocations = sortedAllocations.slice(3);
|
||||
|
||||
const body = buildComment(topAllocations, changedFiles, lineStats, lineStatsByTeam, otherAllocations);
|
||||
|
||||
await postOrUpdateComment(pullRequestNumber, body, BOT_MARKER);
|
||||
}
|
||||
|
||||
// only run when executed directly, not when imported by tests
|
||||
if (import.meta.url === `file://${process.argv[1]}`) {
|
||||
const pullRequestNumber = parseInt(ensureEnvVar("PULL_REQUEST_NUMBER"));
|
||||
|
||||
await getReviewRecommendations(pullRequestNumber);
|
||||
const pullRequestNumber = parseInt(ensureEnvVar('PULL_REQUEST_NUMBER'));
|
||||
await run(pullRequestNumber);
|
||||
}
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { describe, it, mock } from 'node:test';
|
||||
import { beforeEach, describe, it, mock } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
|
||||
/**
|
||||
@@ -7,71 +7,382 @@ import assert from 'node:assert/strict';
|
||||
* node --test --experimental-test-module-mocks ./.github/scripts/owners-review-recommendations.test.mjs
|
||||
* */
|
||||
|
||||
// mock.module must be called before the module under test is imported,
|
||||
// because static imports are hoisted and resolve before any code runs.
|
||||
/** @type {(pullRequestNumber: number) => Promise<Array<{ filename: string, additions: number, deletions: number, previous_filename?: string }>>} */
|
||||
let getPrFilesImpl = async () => [];
|
||||
/** @type {(pullRequestNumber: number, body: string, botMarker: string) => Promise<void>} */
|
||||
let postOrUpdateCommentImpl = async () => {};
|
||||
|
||||
mock.module('./github-helpers.mjs', {
|
||||
namedExports: {
|
||||
ensureEnvVar: () => {}, // no-op in tests
|
||||
initGithub: () => {}, // no-op in tests
|
||||
getChangedFiles: () => Promise.resolve(new Set()), // no-op in tests
|
||||
getChangedFiles: () => Promise.resolve(new Set()),
|
||||
getEventFromGithubEventPath: () => ({}),
|
||||
getPrFiles: (n) => getPrFilesImpl(n),
|
||||
postOrUpdateComment: (pullRequestNumber, body, botMarker) =>
|
||||
postOrUpdateCommentImpl(pullRequestNumber, body, botMarker),
|
||||
},
|
||||
});
|
||||
|
||||
/** @type { (allocations: { team: string, fileCount: number }[], changedFiles: Set<string>) => string } */
|
||||
let buildRecommendationsBody;
|
||||
const { buildRecommendationsBody: imported } = await import('./owners-review-recommendations.mjs');
|
||||
buildRecommendationsBody = imported;
|
||||
/** @type {() => Array<{ filepath: string, team: string }>} */
|
||||
let parseOwnersFileImpl = () => [];
|
||||
/** @type {(files: Set<string>, owners: Array<{ filepath: string, team: string }>) => Map<string, string[]>} */
|
||||
let assignOwnershipImpl = () => new Map();
|
||||
/** @type {(ownerships: Map<string, string[]>) => Array<{ team: string, fileCount: number }>} */
|
||||
let ownershipsToAllocationsImpl = () => [];
|
||||
|
||||
describe('buildRecommendationsBody', () => {
|
||||
const marker = '<!-- owners-review-recommendations -->';
|
||||
mock.module('./owners.mjs', {
|
||||
namedExports: {
|
||||
parseOwnersFile: () => parseOwnersFileImpl(),
|
||||
assignOwnership: (files, owners) => assignOwnershipImpl(files, owners),
|
||||
ownershipsToAllocations: (ownerships) => ownershipsToAllocationsImpl(ownerships),
|
||||
},
|
||||
});
|
||||
|
||||
it('returns the fallback message when there are no allocations', () => {
|
||||
const body = buildRecommendationsBody([], new Set(['a.ts']));
|
||||
const { buildOverviewTable, buildComment, computeAllocationLineStats, computeLineStats, run } =
|
||||
await import('./owners-review-recommendations.mjs');
|
||||
|
||||
assert.ok(body.startsWith(marker), 'body must start with the bot marker');
|
||||
assert.match(body, /No owning teams matched/);
|
||||
});
|
||||
const BOT_MARKER = '<!-- pr-recommendations -->';
|
||||
const EMPTY_LINE_STATS = {
|
||||
sourceCodeAdded: 0,
|
||||
sourceCodeRemoved: 0,
|
||||
testFilesAdded: 0,
|
||||
testFilesRemoved: 0,
|
||||
miscAdded: 0,
|
||||
miscRemoved: 0,
|
||||
};
|
||||
|
||||
it('returns the fallback message when no files changed', () => {
|
||||
const body = buildRecommendationsBody(
|
||||
[{ team: '@n8n-io/cli-team', fileCount: 0 }],
|
||||
new Set(),
|
||||
);
|
||||
|
||||
assert.match(body, /No owning teams matched/);
|
||||
});
|
||||
|
||||
it('renders a table with team name, file count, and rounded percentage', () => {
|
||||
const body = buildRecommendationsBody(
|
||||
describe('buildOverviewTable', () => {
|
||||
it('renders an ownership-first table with per-team line stats', () => {
|
||||
const table = buildOverviewTable(
|
||||
[
|
||||
{ team: '@n8n-io/cli-team', fileCount: 6 },
|
||||
{ team: '@n8n-io/catalysts', fileCount: 3 },
|
||||
{ team: '@n8n-io/ai-team', fileCount: 1 },
|
||||
{ team: '@n8n-io/cli-team', fileCount: 2 },
|
||||
{ team: '@n8n-io/editor-ui-team', fileCount: 1 },
|
||||
],
|
||||
new Set(['1', '2', '3', '4', '5', '6', '7', '8', '9', '10']),
|
||||
new Set(['a.ts', 'b.test.ts', 'README.md']),
|
||||
{
|
||||
sourceCodeAdded: 10,
|
||||
sourceCodeRemoved: 2,
|
||||
testFilesAdded: 5,
|
||||
testFilesRemoved: 1,
|
||||
miscAdded: 3,
|
||||
miscRemoved: 0,
|
||||
},
|
||||
new Map([
|
||||
[
|
||||
'@n8n-io/cli-team',
|
||||
{
|
||||
sourceCodeAdded: 10,
|
||||
sourceCodeRemoved: 2,
|
||||
testFilesAdded: 5,
|
||||
testFilesRemoved: 1,
|
||||
miscAdded: 0,
|
||||
miscRemoved: 0,
|
||||
},
|
||||
],
|
||||
[
|
||||
'@n8n-io/editor-ui-team',
|
||||
{
|
||||
sourceCodeAdded: 0,
|
||||
sourceCodeRemoved: 0,
|
||||
testFilesAdded: 0,
|
||||
testFilesRemoved: 0,
|
||||
miscAdded: 3,
|
||||
miscRemoved: 0,
|
||||
},
|
||||
],
|
||||
]),
|
||||
);
|
||||
|
||||
assert.ok(body.startsWith(marker));
|
||||
assert.match(body, /\| @n8n-io\/cli-team \| 6 \| 60% \|/);
|
||||
assert.match(body, /\| @n8n-io\/catalysts \| 3 \| 30% \|/);
|
||||
assert.match(body, /\| @n8n-io\/ai-team \| 1 \| 10% \|/);
|
||||
assert.match(table, /## PR review overview/);
|
||||
assert.match(table, /\| Ownership \| Files owned \| Share \| Source code \| Test files \| Misc \|/);
|
||||
assert.match(table, /\| @n8n-io\/cli-team \| 2 \| 67% \| \+10 \/ -2 \| \+5 \/ -1 \| \+0 \/ -0 \|/);
|
||||
assert.match(table, /\| @n8n-io\/editor-ui-team \| 1 \| 33% \| \+0 \/ -0 \| \+0 \/ -0 \| \+3 \/ -0 \|/);
|
||||
assert.match(table, /\| \*\*Total\*\* \| \*\*3\*\* \| \*\*100%\*\* \| \*\*\+10 \/ -2\*\* \| \*\*\+5 \/ -1\*\* \| \*\*\+3 \/ -0\*\* \|/);
|
||||
});
|
||||
|
||||
it('renders a no-owner row when no owners matched', () => {
|
||||
const table = buildOverviewTable(
|
||||
[],
|
||||
new Set(['a.ts']),
|
||||
{ ...EMPTY_LINE_STATS, sourceCodeAdded: 12, sourceCodeRemoved: 1 },
|
||||
new Map(),
|
||||
);
|
||||
|
||||
assert.match(table, /\| _No owning teams matched_ \| 0 \| 0% \| \+12 \/ -1 \| \+0 \/ -0 \| \+0 \/ -0 \|/);
|
||||
});
|
||||
|
||||
it('uses singular "file" when exactly one file changed', () => {
|
||||
const body = buildRecommendationsBody(
|
||||
const table = buildOverviewTable(
|
||||
[{ team: '@n8n-io/cli-team', fileCount: 1 }],
|
||||
new Set(['only.ts']),
|
||||
EMPTY_LINE_STATS,
|
||||
new Map(),
|
||||
);
|
||||
|
||||
assert.match(body, /ownership of the 1 changed file in this PR/);
|
||||
assert.match(table, /ownership of the 1 changed file in this PR/);
|
||||
});
|
||||
|
||||
it('uses plural "files" for more than one changed file', () => {
|
||||
const body = buildRecommendationsBody(
|
||||
[{ team: '@n8n-io/cli-team', fileCount: 2 }],
|
||||
it('uses plural "files" for more than one file changed', () => {
|
||||
const table = buildOverviewTable(
|
||||
[{ team: '@n8n-io/cli-team', fileCount: 1 }],
|
||||
new Set(['a.ts', 'b.ts']),
|
||||
EMPTY_LINE_STATS,
|
||||
new Map(),
|
||||
);
|
||||
|
||||
assert.match(body, /ownership of the 2 changed files in this PR/);
|
||||
assert.match(table, /ownership of the 2 changed files in this PR/);
|
||||
});
|
||||
|
||||
it('renders an aggregate row for allocations outside the displayed teams', () => {
|
||||
const table = buildOverviewTable(
|
||||
[{ team: '@n8n-io/cli-team', fileCount: 2 }],
|
||||
new Set(['a.ts', 'b.ts', 'c.ts']),
|
||||
{ ...EMPTY_LINE_STATS, sourceCodeAdded: 13, sourceCodeRemoved: 3 },
|
||||
new Map([
|
||||
['@n8n-io/cli-team', { ...EMPTY_LINE_STATS, sourceCodeAdded: 10, sourceCodeRemoved: 1 }],
|
||||
['@n8n-io/team-a', { ...EMPTY_LINE_STATS, sourceCodeAdded: 2, sourceCodeRemoved: 1 }],
|
||||
['@n8n-io/team-b', { ...EMPTY_LINE_STATS, sourceCodeAdded: 1, sourceCodeRemoved: 1 }],
|
||||
]),
|
||||
[
|
||||
{ team: '@n8n-io/team-a', fileCount: 1 },
|
||||
{ team: '@n8n-io/team-b', fileCount: 1 },
|
||||
],
|
||||
);
|
||||
|
||||
assert.match(table, /\| Other teams \| 2 \| 67% \| \+3 \/ -2 \| \+0 \/ -0 \| \+0 \/ -0 \|/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildComment', () => {
|
||||
it('starts with the bot marker', () => {
|
||||
const body = buildComment([], new Set(['a.ts']), EMPTY_LINE_STATS);
|
||||
|
||||
assert.ok(body.startsWith(BOT_MARKER), 'body must start with the bot marker');
|
||||
});
|
||||
|
||||
it('includes the overview table', () => {
|
||||
const body = buildComment(
|
||||
[{ team: '@n8n-io/cli-team', fileCount: 1 }],
|
||||
new Set(['a.ts']),
|
||||
{ ...EMPTY_LINE_STATS, sourceCodeAdded: 42 },
|
||||
);
|
||||
|
||||
assert.match(body, /## PR review overview/);
|
||||
assert.match(body, /\| @n8n-io\/cli-team \| 1 \| 100% \|/);
|
||||
});
|
||||
|
||||
it('shows a warning below the table when source code additions exceed the size limit', () => {
|
||||
const body = buildComment([], new Set(['a.ts']), { ...EMPTY_LINE_STATS, sourceCodeAdded: 1001 });
|
||||
|
||||
assert.match(body, /❗ Source code additions \(1[,.]001\) exceed the 1[,.]000-line limit\./);
|
||||
});
|
||||
});
|
||||
|
||||
describe('computeLineStats', () => {
|
||||
it('returns all zeros for an empty file list', () => {
|
||||
assert.deepEqual(computeLineStats([]), EMPTY_LINE_STATS);
|
||||
});
|
||||
|
||||
it('categorises a plain source file as source code', () => {
|
||||
const stats = computeLineStats([{ filename: 'src/foo.ts', additions: 10, deletions: 3 }]);
|
||||
|
||||
assert.equal(stats.sourceCodeAdded, 10);
|
||||
assert.equal(stats.sourceCodeRemoved, 3);
|
||||
assert.equal(stats.testFilesAdded, 0);
|
||||
assert.equal(stats.miscAdded, 0);
|
||||
});
|
||||
|
||||
describe('test file patterns', () => {
|
||||
for (const [label, filename] of [
|
||||
['*.test.ts', 'src/foo.test.ts'],
|
||||
['*.test.js', 'src/foo.test.js'],
|
||||
['*.test.mjs', 'src/foo.test.mjs'],
|
||||
['*.spec.ts', 'src/foo.spec.ts'],
|
||||
['*.spec.js', 'src/foo.spec.js'],
|
||||
['*.spec.mjs', 'src/foo.spec.mjs'],
|
||||
['test/ directory', 'packages/cli/test/helpers.ts'],
|
||||
['tests/ directory', 'packages/cli/tests/helpers.ts'],
|
||||
['__tests__/ directory', 'src/__tests__/foo.ts'],
|
||||
['__snapshots__/ directory', 'src/__snapshots__/foo.snap'],
|
||||
['*.snap extension', 'src/foo.snap'],
|
||||
['fixtures/ directory', 'src/fixtures/data.json'],
|
||||
['__mocks__/ directory', 'src/__mocks__/axios.ts'],
|
||||
['packages/testing/**', 'packages/testing/playwright/spec.ts'],
|
||||
]) {
|
||||
it(`classifies ${label} as testFiles`, () => {
|
||||
const stats = computeLineStats([{ filename, additions: 7, deletions: 2 }]);
|
||||
|
||||
assert.equal(stats.testFilesAdded, 7, `expected ${filename} to be a test file`);
|
||||
assert.equal(stats.testFilesRemoved, 2);
|
||||
assert.equal(stats.sourceCodeAdded, 0);
|
||||
assert.equal(stats.miscAdded, 0);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
describe('misc file patterns', () => {
|
||||
for (const [label, filename] of [
|
||||
['pnpm-lock.yaml', 'pnpm-lock.yaml'],
|
||||
['*.md', 'README.md'],
|
||||
['nested *.md', 'packages/cli/CHANGELOG.md'],
|
||||
['*.mdx', 'docs/guide.mdx'],
|
||||
['nested *.mdx', 'packages/editor-ui/docs/page.mdx'],
|
||||
]) {
|
||||
it(`classifies ${label} as misc`, () => {
|
||||
const stats = computeLineStats([{ filename, additions: 3, deletions: 1 }]);
|
||||
|
||||
assert.equal(stats.miscAdded, 3, `expected ${filename} to be misc`);
|
||||
assert.equal(stats.miscRemoved, 1);
|
||||
assert.equal(stats.sourceCodeAdded, 0);
|
||||
assert.equal(stats.testFilesAdded, 0);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
it('sums additions and deletions across multiple files per category', () => {
|
||||
const stats = computeLineStats([
|
||||
{ filename: 'src/a.ts', additions: 10, deletions: 5 },
|
||||
{ filename: 'src/b.ts', additions: 15, deletions: 3 },
|
||||
{ filename: 'src/a.test.ts', additions: 5, deletions: 1 },
|
||||
{ filename: 'src/b.spec.ts', additions: 8, deletions: 2 },
|
||||
{ filename: 'pnpm-lock.yaml', additions: 100, deletions: 50 },
|
||||
{ filename: 'README.md', additions: 20, deletions: 10 },
|
||||
]);
|
||||
|
||||
assert.equal(stats.sourceCodeAdded, 25);
|
||||
assert.equal(stats.sourceCodeRemoved, 8);
|
||||
assert.equal(stats.testFilesAdded, 13);
|
||||
assert.equal(stats.testFilesRemoved, 3);
|
||||
assert.equal(stats.miscAdded, 120);
|
||||
assert.equal(stats.miscRemoved, 60);
|
||||
});
|
||||
});
|
||||
|
||||
describe('computeAllocationLineStats', () => {
|
||||
it('computes line stats per owner allocation', () => {
|
||||
const statsByTeam = computeAllocationLineStats(
|
||||
[
|
||||
{ team: '@n8n-io/cli-team', fileCount: 2 },
|
||||
{ team: '@n8n-io/docs-team', fileCount: 1 },
|
||||
],
|
||||
new Map([
|
||||
['@n8n-io/cli-team', ['src/a.ts', 'src/a.test.ts']],
|
||||
['@n8n-io/docs-team', ['README.md']],
|
||||
]),
|
||||
[
|
||||
{ filename: 'src/a.ts', additions: 10, deletions: 1 },
|
||||
{ filename: 'src/a.test.ts', additions: 5, deletions: 2 },
|
||||
{ filename: 'README.md', additions: 3, deletions: 1 },
|
||||
],
|
||||
);
|
||||
|
||||
assert.deepEqual(statsByTeam.get('@n8n-io/cli-team'), {
|
||||
sourceCodeAdded: 10,
|
||||
sourceCodeRemoved: 1,
|
||||
testFilesAdded: 5,
|
||||
testFilesRemoved: 2,
|
||||
miscAdded: 0,
|
||||
miscRemoved: 0,
|
||||
});
|
||||
assert.deepEqual(statsByTeam.get('@n8n-io/docs-team'), {
|
||||
sourceCodeAdded: 0,
|
||||
sourceCodeRemoved: 0,
|
||||
testFilesAdded: 0,
|
||||
testFilesRemoved: 0,
|
||||
miscAdded: 3,
|
||||
miscRemoved: 1,
|
||||
});
|
||||
});
|
||||
|
||||
it('matches renamed files by previous filename', () => {
|
||||
const statsByTeam = computeAllocationLineStats(
|
||||
[{ team: '@n8n-io/cli-team', fileCount: 1 }],
|
||||
new Map([['@n8n-io/cli-team', ['old-name.ts']]]),
|
||||
[{ filename: 'new-name.ts', previous_filename: 'old-name.ts', additions: 7, deletions: 3 }],
|
||||
);
|
||||
|
||||
assert.equal(statsByTeam.get('@n8n-io/cli-team').sourceCodeAdded, 7);
|
||||
assert.equal(statsByTeam.get('@n8n-io/cli-team').sourceCodeRemoved, 3);
|
||||
});
|
||||
});
|
||||
|
||||
describe('run', () => {
|
||||
let postOrUpdateComment;
|
||||
|
||||
beforeEach(() => {
|
||||
getPrFilesImpl = async () => [];
|
||||
postOrUpdateComment = mock.fn(async () => {});
|
||||
postOrUpdateCommentImpl = postOrUpdateComment;
|
||||
parseOwnersFileImpl = () => [];
|
||||
assignOwnershipImpl = () => new Map();
|
||||
ownershipsToAllocationsImpl = () => [];
|
||||
});
|
||||
|
||||
it('calls postOrUpdateComment with the PR number, generated body, and bot marker', async () => {
|
||||
await run(42);
|
||||
|
||||
assert.equal(postOrUpdateComment.mock.calls.length, 1, 'postOrUpdateComment should be called once');
|
||||
assert.equal(postOrUpdateComment.mock.calls[0].arguments[0], 42);
|
||||
assert.ok(postOrUpdateComment.mock.calls[0].arguments[1].includes(BOT_MARKER));
|
||||
assert.equal(postOrUpdateComment.mock.calls[0].arguments[2], BOT_MARKER);
|
||||
});
|
||||
|
||||
it('includes renamed files in the changed files set', async () => {
|
||||
getPrFilesImpl = async () => [
|
||||
{ filename: 'new-name.ts', additions: 5, deletions: 0, previous_filename: 'old-name.ts' },
|
||||
];
|
||||
|
||||
let capturedChangedFiles = new Set();
|
||||
assignOwnershipImpl = (files) => {
|
||||
capturedChangedFiles = files;
|
||||
return new Map();
|
||||
};
|
||||
|
||||
await run(42);
|
||||
|
||||
assert.ok(capturedChangedFiles.has('new-name.ts'), 'new filename must be included');
|
||||
assert.ok(capturedChangedFiles.has('old-name.ts'), 'previous filename must be included');
|
||||
});
|
||||
|
||||
it('limits recommendations to the top 3 allocations by total line changes', async () => {
|
||||
getPrFilesImpl = async () => [
|
||||
{ filename: 'a.ts', additions: 1, deletions: 0 },
|
||||
{ filename: 'b.ts', additions: 50, deletions: 50 },
|
||||
{ filename: 'c.ts', additions: 20, deletions: 20 },
|
||||
{ filename: 'd.ts', additions: 10, deletions: 10 },
|
||||
];
|
||||
assignOwnershipImpl = () =>
|
||||
new Map([
|
||||
['@n8n-io/team-a', ['a.ts']],
|
||||
['@n8n-io/team-b', ['b.ts']],
|
||||
['@n8n-io/team-c', ['c.ts']],
|
||||
['@n8n-io/team-d', ['d.ts']],
|
||||
]);
|
||||
ownershipsToAllocationsImpl = () => [
|
||||
{ team: '@n8n-io/team-a', fileCount: 1 },
|
||||
{ team: '@n8n-io/team-b', fileCount: 1 },
|
||||
{ team: '@n8n-io/team-c', fileCount: 1 },
|
||||
{ team: '@n8n-io/team-d', fileCount: 1 },
|
||||
];
|
||||
|
||||
await run(42);
|
||||
|
||||
const body = postOrUpdateComment.mock.calls[0].arguments[1];
|
||||
assert.match(body, /@n8n-io\/team-b[\s\S]*@n8n-io\/team-c[\s\S]*@n8n-io\/team-d/);
|
||||
assert.doesNotMatch(body, /@n8n-io\/team-a/);
|
||||
assert.match(body, /\| Other teams \| 1 \| 25% \| \+1 \/ -0 \| \+0 \/ -0 \| \+0 \/ -0 \|/);
|
||||
});
|
||||
|
||||
it('posted comment body includes the ownership-first overview table', async () => {
|
||||
getPrFilesImpl = async () => [{ filename: 'src/foo.ts', additions: 50, deletions: 5 }];
|
||||
ownershipsToAllocationsImpl = () => [{ team: '@n8n-io/cli-team', fileCount: 1 }];
|
||||
assignOwnershipImpl = () => new Map([['@n8n-io/cli-team', ['src/foo.ts']]]);
|
||||
|
||||
await run(42);
|
||||
|
||||
const body = postOrUpdateComment.mock.calls[0].arguments[1];
|
||||
assert.match(body, /## PR review overview/);
|
||||
assert.match(body, /\| @n8n-io\/cli-team \| 1 \| 100% \| \+50 \/ -5 \|/);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -18,7 +18,7 @@ import { initGithub, getEventFromGithubEventPath } from '../github-helpers.mjs';
|
||||
export const SIZE_LIMIT = 1000;
|
||||
export const OVERRIDE_COMMAND = '/size-limit-override';
|
||||
|
||||
export const EXCLUDE_PATTERNS = [
|
||||
export const TEST_PATTERNS = [
|
||||
// Test files (by extension)
|
||||
'**/*.test.ts',
|
||||
'**/*.test.js',
|
||||
@@ -38,12 +38,17 @@ export const EXCLUDE_PATTERNS = [
|
||||
'**/__mocks__/**',
|
||||
// Dedicated testing package
|
||||
'packages/testing/**',
|
||||
];
|
||||
|
||||
export const MISC_PATTERNS = [
|
||||
// Lock file (can produce massive diffs on dependency changes)
|
||||
'pnpm-lock.yaml',
|
||||
'**/*.md',
|
||||
'**/*.mdx'
|
||||
'**/*.mdx',
|
||||
];
|
||||
|
||||
export const EXCLUDE_PATTERNS = [...TEST_PATTERNS, ...MISC_PATTERNS];
|
||||
|
||||
const BOT_MARKER = '<!-- pr-size-check -->';
|
||||
|
||||
/**
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
name: 'CI: Owners Review Recommendations'
|
||||
|
||||
# Posts (or updates) a PR comment recommending which @n8n-io teams should
|
||||
# review the PR, based on file ownership defined in .github/OWNERS.
|
||||
# Posts (or updates) a PR comment with recommended reviewer teams (based on
|
||||
# file ownership in .github/OWNERS) and a breakdown of changed lines by
|
||||
# category (source code, test files, misc).
|
||||
#
|
||||
# Advisory only — does not gate merging.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user