mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 05:29:44 +02:00
ci: Exclude test files from required reviews (no-changelog) (#40164)
This commit is contained in:
parent
69595f5821
commit
0ae0b6b3c2
6
.github/WORKFLOWS.md
vendored
6
.github/WORKFLOWS.md
vendored
|
|
@ -884,8 +884,10 @@ The file drives four workflows:
|
|||
### Required reviews
|
||||
|
||||
An entry with the `required` option makes team approval mandatory: when a PR
|
||||
changes a file whose winning entry carries `required`, a member of each listed
|
||||
team must approve the PR. `ci-owners-required-reviews.yml` evaluates this on
|
||||
changes a non-test file whose winning entry carries `required`, a member of each
|
||||
listed team must approve the PR. Test files match the shared patterns in
|
||||
`test-files.mjs` and do not trigger required reviews.
|
||||
`ci-owners-required-reviews.yml` evaluates this on
|
||||
PR changes and review events, and reports a commit status
|
||||
named **Required Reviews** on the head SHA. A missing approval reports
|
||||
`pending` ("Waiting for approval from: …"), not `failure`, so an unreviewed PR
|
||||
|
|
|
|||
6
.github/scripts/owners/owners.mjs
vendored
6
.github/scripts/owners/owners.mjs
vendored
|
|
@ -1,6 +1,8 @@
|
|||
import { readFileSync, statSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
|
||||
import { isTestFile } from "../test-files.mjs";
|
||||
|
||||
/**
|
||||
* @typedef OwnersEntry
|
||||
* @property { string } pattern
|
||||
|
|
@ -236,7 +238,7 @@ export function assignOwnership(files, entries) {
|
|||
|
||||
/**
|
||||
* Determine which teams must approve the changeset: a team is required when
|
||||
* its `required` entry wins (last-match) for a changed file.
|
||||
* its `required` entry wins (last-match) for a changed non-test file.
|
||||
*
|
||||
* @param { Set<string> } files
|
||||
* @param { OwnersEntry[] } entries
|
||||
|
|
@ -247,6 +249,8 @@ export function resolveRequiredTeams(files, entries) {
|
|||
const teamToFiles = new Map();
|
||||
|
||||
for (const file of [...files].sort()) {
|
||||
if (isTestFile(file)) continue;
|
||||
|
||||
const entry = findOwningEntry(file, entries);
|
||||
if (!entry?.required) continue;
|
||||
|
||||
|
|
|
|||
16
.github/scripts/owners/owners.test.mjs
vendored
16
.github/scripts/owners/owners.test.mjs
vendored
|
|
@ -293,6 +293,22 @@ describe('resolveRequiredTeams', () => {
|
|||
assert.equal(result.size, 0);
|
||||
});
|
||||
|
||||
it('ignores test files whose winning entry is required', () => {
|
||||
const result = resolveRequiredTeams(
|
||||
new Set([
|
||||
'.github/scripts/check.test.mjs',
|
||||
'.github/workflows/__tests__/check.ts',
|
||||
'.github/workflows/ci.yml',
|
||||
]),
|
||||
[
|
||||
entry({ pattern: '.github/scripts/', team: '@n8n-io/qa-dx', required: true }),
|
||||
entry({ pattern: '.github/workflows/', team: '@n8n-io/qa-dx', required: true }),
|
||||
],
|
||||
);
|
||||
|
||||
assert.deepEqual(result.get('@n8n-io/qa-dx'), ['.github/workflows/ci.yml']);
|
||||
});
|
||||
|
||||
it('a later non-required entry overrides an earlier required one', () => {
|
||||
const result = resolveRequiredTeams(new Set(['db/migrations/1-init.ts']), [
|
||||
entry({ pattern: 'db/', team: '@n8n-io/migrations-review', required: true }),
|
||||
|
|
|
|||
2
.github/scripts/owners/required-reviews.mjs
vendored
2
.github/scripts/owners/required-reviews.mjs
vendored
|
|
@ -1,7 +1,7 @@
|
|||
/**
|
||||
* Required-review enforcement for OWNERS entries marked `required`.
|
||||
*
|
||||
* Computes the teams whose approval the changeset needs (per OWNERS)
|
||||
* Computes the teams whose approval the non-test changes need (per OWNERS)
|
||||
* and reports the verdict as a "Required Reviews" commit status on the PR
|
||||
* head SHA. A ruleset that lists this status as a required check blocks the
|
||||
* merge until a member of each required team has approved the PR. Merge-queue
|
||||
|
|
|
|||
30
.github/scripts/quality/check-pr-size.mjs
vendored
30
.github/scripts/quality/check-pr-size.mjs
vendored
|
|
@ -14,35 +14,13 @@
|
|||
|
||||
import { initGithub, getEventFromGithubEventPath } from '../github-helpers.mjs';
|
||||
import { matchesGlob } from '../glob.mjs';
|
||||
import { isTestFile, TEST_PATTERNS } from '../test-files.mjs';
|
||||
|
||||
export { isTestFile, TEST_PATTERNS };
|
||||
|
||||
export const SIZE_LIMIT = 1000;
|
||||
export const OVERRIDE_COMMAND = '/size-limit-override';
|
||||
|
||||
export const TEST_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__/**',
|
||||
// Former packages/testing workspaces
|
||||
'packages/quality/policy/**',
|
||||
'packages/quality/testing/**',
|
||||
'packages/quality/environments/**',
|
||||
'packages/quality/efficiency/microbenchmarks/**',
|
||||
];
|
||||
|
||||
export const MISC_PATTERNS = [
|
||||
// Lock file (can produce massive diffs on dependency changes)
|
||||
'pnpm-lock.yaml',
|
||||
|
|
@ -59,7 +37,7 @@ export const EXCLUDE_PATTERNS = [...TEST_PATTERNS, ...MISC_PATTERNS];
|
|||
* @returns { 'testFiles' | 'misc' | 'sourceCode' }
|
||||
*/
|
||||
export function categorizeFile(filename) {
|
||||
if (TEST_PATTERNS.some((pattern) => matchesGlob(filename, pattern))) return 'testFiles';
|
||||
if (isTestFile(filename)) return 'testFiles';
|
||||
if (MISC_PATTERNS.some((pattern) => matchesGlob(filename, pattern))) return 'misc';
|
||||
return 'sourceCode';
|
||||
}
|
||||
|
|
|
|||
34
.github/scripts/test-files.mjs
vendored
Normal file
34
.github/scripts/test-files.mjs
vendored
Normal file
|
|
@ -0,0 +1,34 @@
|
|||
import { matchesGlob } from './glob.mjs';
|
||||
|
||||
export const TEST_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__/**',
|
||||
// Former packages/testing workspaces
|
||||
'packages/quality/policy/**',
|
||||
'packages/quality/testing/**',
|
||||
'packages/quality/environments/**',
|
||||
'packages/quality/efficiency/microbenchmarks/**',
|
||||
];
|
||||
|
||||
/**
|
||||
* @param { string } filename
|
||||
* @returns { boolean }
|
||||
*/
|
||||
export function isTestFile(filename) {
|
||||
return TEST_PATTERNS.some((pattern) => matchesGlob(filename, pattern));
|
||||
}
|
||||
4
OWNERS
4
OWNERS
|
|
@ -8,8 +8,8 @@
|
|||
#
|
||||
# Last-match-wins: specific rules MUST come AFTER general rules.
|
||||
#
|
||||
# The `required` option makes team approval mandatory: a member of the team
|
||||
# must approve the PR before merge. This is enforced by the
|
||||
# The `required` option makes team approval mandatory for non-test files: a
|
||||
# member of the team must approve the PR before merge. This is enforced by the
|
||||
# "Required Reviews" commit status (ci-owners-required-reviews.yml).
|
||||
# All other entries are advisory (reviewer recommendations and opt-in
|
||||
# reviewer auto-assignment).
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user