From ceb9776dd96615633f908ee86ed2d60d8d8904b2 Mon Sep 17 00:00:00 2001 From: nollymarlonga Date: Wed, 9 Sep 2026 15:52:53 -0500 Subject: [PATCH] fix(cicd): exclude spec- and docs-only PRs from the Release QA report MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since Spec-Kit landed, every feature ships two PRs and PR 1 carries spec.md alone under specs/-/. Those PRs contain nothing runnable, so QA has nothing to exercise and they never earn a QA label — yet the release QA report counted them as un-QA'd, inflating the Slack warning and @-mentioning their authors on every release. Pure documentation PRs had the same problem. The report had no idea what files a PR touched: classifyExclusion only ever read authorType, author, labels and title. So: - github.ts: new fetchChangedFiles, batched GraphQL (20 PRs per query) rather than one REST listFiles per PR. Unlike fetchClosingIssueRefs it swallows errors instead of re-throwing — an unknown file list just means no path-based exclusion, i.e. the previous behaviour, so failing the whole QA section over it would be the worse trade. - exclusions.ts: isDocumentationPath + a 4th rule, applied after the title heuristics so a bot-authored spec PR still reads bot-author. Documentation is specs/**, docs/** and loose *.md / *.mdx. Agent tooling is checked first and stays in QA scope (.claude/, .agents/, .cursor/, .specify/, any CLAUDE.md / AGENTS.md) — in this repo that markdown *is* the deliverable, and without the exception .claude/skills/x/SKILL.md would match the markdown pattern and drop a real feature out of the report. - types.ts: spec-only and docs-only reasons; changedFiles?: string[] where undefined means unknown, deliberately distinct from []. Fail safe throughout: a PR is excluded only on a file list fetched in full whose every path is documentation. An absent or truncated list keeps the PR in QA scope — over-reporting beats silently hiding a code change. Excluded PRs also now render on a clean release. Both renderers previously returned before that section when nothing was flagged, and after this change the common case is exactly that — nothing flagged *because* the gaps were spec PRs — which would leave the drop in counts unexplained. Slack is untouched and still silent when nothing needs review. Verified against v26.09.03-01...v26.09.09-01 (38 PRs): missing 25 -> 17, excluded 0 -> 9. All 9 re-checked independently via gh — every one genuinely spec/docs-only, and no docs-only PR was left behind. The truncation guard fired for real on #37423 (>100 files), which correctly stayed in QA scope. Co-Authored-By: Claude Opus 5 --- .../release-qa-status/src/exclusions.test.ts | 111 +++++++++++++- .../release-qa-status/src/exclusions.ts | 55 ++++++- .../release-qa-status/src/format.test.ts | 45 +++++- .../scripts/release-qa-status/src/format.ts | 19 ++- .../release-qa-status/src/github.test.ts | 36 ++++- .../scripts/release-qa-status/src/github.ts | 145 +++++++++++++++++- .../scripts/release-qa-status/src/qa.test.ts | 36 +++++ .../scripts/release-qa-status/src/types.ts | 11 +- 8 files changed, 442 insertions(+), 16 deletions(-) diff --git a/.github/scripts/release-qa-status/src/exclusions.test.ts b/.github/scripts/release-qa-status/src/exclusions.test.ts index c4e87caa4a2b..8745bb2ec1cd 100644 --- a/.github/scripts/release-qa-status/src/exclusions.test.ts +++ b/.github/scripts/release-qa-status/src/exclusions.test.ts @@ -1,4 +1,4 @@ -import { classifyExclusion } from './exclusions'; +import { classifyExclusion, isDocumentationPath } from './exclusions'; import { PRDetails } from './types'; function pr(overrides: Partial = {}): PRDetails { @@ -77,3 +77,112 @@ describe('classifyExclusion', () => { }); }); }); + +describe('isDocumentationPath', () => { + it.each([ + 'specs/37376-uve-edit-pencil-permission/spec.md', + 'specs/36985-block-editor-selection-guard/contracts/report.schema.json', + 'docs/backend/JAVA_STANDARDS.md', + 'docs/images/architecture.png', + 'README.md', + 'CONTRIBUTING.md', + 'core-web/libs/sdk/react/CHANGELOG.MD', + 'examples/nextjs/docs.mdx', + ])('treats %s as documentation', (path) => { + expect(isDocumentationPath(path)).toBe(true); + }); + + it.each([ + 'dotCMS/src/main/java/com/dotcms/rest/Foo.java', + 'core-web/libs/dotcms-ui/src/lib/foo.component.html', + 'bom/application/pom.xml', + '.github/workflows/cicd_6-release.yml', + ])('treats %s as implementation', (path) => { + expect(isDocumentationPath(path)).toBe(false); + }); + + // Agent/dev tooling ships as markdown in this repo but is the deliverable, + // not prose about one — PR #37309 added a whole Claude skill in markdown alone. + it.each([ + '.claude/skills/dot-test-plan/SKILL.md', + '.claude/commands/create-issue.md', + '.agents/skills/angular-developer/references/signals.md', + '.cursor/rules/java.mdc', + '.specify/memory/constitution.md', + '.specify/templates/tasks-template.md', + 'CLAUDE.md', + 'core-web/CLAUDE.md', + 'dotCMS/src/main/java/com/dotcms/rest/CLAUDE.md', + 'core-web/apps/dotcms-ui/AGENTS.md', + ])('keeps agent tooling %s in QA scope', (path) => { + expect(isDocumentationPath(path)).toBe(false); + }); +}); + +describe('classifyExclusion — path heuristics', () => { + it('excludes a Spec-Kit PR 1 as spec-only', () => { + expect( + classifyExclusion( + pr({ + title: 'Spec: UVE contentlet permission gating', + changedFiles: [ + 'specs/37376-uve-edit-pencil-permission/spec.md', + 'specs/37376-uve-edit-pencil-permission/data-model.md', + ], + }) + ) + ).toEqual({ excluded: true, reason: 'spec-only' }); + }); + + it('excludes a pure docs PR as docs-only', () => { + expect( + classifyExclusion(pr({ changedFiles: ['docs/core/SPEC_KIT_QUICK_START.md', 'README.md'] })) + ).toEqual({ excluded: true, reason: 'docs-only' }); + }); + + it('reports specs mixed with other docs as docs-only, not spec-only', () => { + expect( + classifyExclusion( + pr({ changedFiles: ['specs/37376-uve-edit-pencil-permission/spec.md', 'README.md'] }) + ) + ).toEqual({ excluded: true, reason: 'docs-only' }); + }); + + it('keeps a PR that ships a spec alongside implementation', () => { + expect( + classifyExclusion( + pr({ + changedFiles: [ + 'specs/37376-uve-edit-pencil-permission/spec.md', + 'dotCMS/src/main/java/com/dotcms/rest/ContentResource.java', + ], + }) + ) + ).toEqual({ excluded: false }); + }); + + it('keeps a markdown-only PR that ships agent tooling', () => { + // PR #37309: `feat(skills): add dot-pr-spec-summary` — markdown, but a feature. + expect( + classifyExclusion( + pr({ changedFiles: ['.claude/skills/dot-pr-spec-summary/SKILL.md', 'CLAUDE.md'] }) + ) + ).toEqual({ excluded: false }); + }); + + it('keeps a PR whose file list is unknown', () => { + // Fetch failed or the PR has more files than one page holds. Over-report + // rather than silently hide what might be a code change. + expect(classifyExclusion(pr({ changedFiles: undefined }))).toEqual({ excluded: false }); + expect(classifyExclusion(pr({ changedFiles: [] }))).toEqual({ excluded: false }); + }); + + it('prefers an earlier reason over the path check', () => { + // A bot that only writes specs is still excluded as a bot. + expect( + classifyExclusion( + pr({ authorType: 'Bot', changedFiles: ['specs/37376-foo/spec.md'] }) + ) + ).toEqual({ excluded: true, reason: 'bot-author' }); + }); +}); diff --git a/.github/scripts/release-qa-status/src/exclusions.ts b/.github/scripts/release-qa-status/src/exclusions.ts index 019eee6a3f38..e5cd2eb53c15 100644 --- a/.github/scripts/release-qa-status/src/exclusions.ts +++ b/.github/scripts/release-qa-status/src/exclusions.ts @@ -1,6 +1,7 @@ /** * Determine whether a PR should be excluded from QA evaluation because it's - * a bot, dependency bump, or release-machinery change. + * a bot, dependency bump, release-machinery change, or carries no + * implementation at all (spec / documentation only). */ import { ExclusionReason, PRDetails } from './types'; @@ -39,6 +40,42 @@ const RELEASE_MACHINERY_PATTERNS: RegExp[] = [ /^merge pull request/i, ]; +/** + * Agent/dev tooling that happens to ship as markdown. It reads like + * documentation but *is* the deliverable — PR #37309 added a whole Claude + * skill without touching a single non-markdown file — so it stays in QA scope. + * Checked before the documentation patterns, or `.claude/skills/x/SKILL.md` + * would match DOC_FILE_PATTERN and drop out of the report. + */ +const IMPLEMENTATION_PREFIXES = ['.claude/', '.agents/', '.cursor/', '.specify/']; +const IMPLEMENTATION_BASENAMES = ['claude.md', 'agents.md']; + +/** Spec-Kit feature directories: `specs/-/{spec,plan,tasks}.md`, contracts, etc. */ +const SPEC_PREFIX = 'specs/'; + +/** Documentation trees — every file under them is prose, whatever the extension. */ +const DOC_PREFIXES = ['docs/']; + +/** Loose prose anywhere else in the tree: README.md, CONTRIBUTING.md, a library's *.mdx. */ +const DOC_FILE_PATTERN = /\.mdx?$/i; + +/** + * True when a path carries no implementation — nothing QA could exercise. + * Case-insensitive: paths come straight from the GitHub API and casing varies + * (`CLAUDE.md`, `SKILL.md`, `README.MD`). + */ +export function isDocumentationPath(path: string): boolean { + const lower = path.toLowerCase(); + + if (IMPLEMENTATION_PREFIXES.some((p) => lower.startsWith(p))) return false; + const basename = lower.slice(lower.lastIndexOf('/') + 1); + if (IMPLEMENTATION_BASENAMES.includes(basename)) return false; + + if (lower.startsWith(SPEC_PREFIX)) return true; + if (DOC_PREFIXES.some((p) => lower.startsWith(p))) return true; + return DOC_FILE_PATTERN.test(lower); +} + export interface ExclusionResult { excluded: boolean; reason?: ExclusionReason; @@ -71,5 +108,21 @@ export function classifyExclusion(pr: PRDetails): ExclusionResult { return { excluded: true, reason: 'release-machinery' }; } + // 4) Path heuristics — last, so a bot-authored spec PR still reads + // `bot-author` rather than `spec-only`. + // + // Spec-Kit ships every feature as two PRs, and PR 1 carries spec.md alone. + // It has nothing runnable, so it never earns a QA label, and without this + // it lands in `missing`/`unlinked` and pages its author on every release. + // + // Fail safe: only exclude on a file list we actually have in full. An + // absent list (fetch failed, or too many files to page) leaves the PR in + // QA scope — over-reporting beats silently hiding a code change. + const files = pr.changedFiles; + if (files && files.length > 0 && files.every(isDocumentationPath)) { + const allSpecs = files.every((f) => f.toLowerCase().startsWith(SPEC_PREFIX)); + return { excluded: true, reason: allSpecs ? 'spec-only' : 'docs-only' }; + } + return { excluded: false }; } diff --git a/.github/scripts/release-qa-status/src/format.test.ts b/.github/scripts/release-qa-status/src/format.test.ts index 7dbb40050808..6bebb17f8b69 100644 --- a/.github/scripts/release-qa-status/src/format.test.ts +++ b/.github/scripts/release-qa-status/src/format.test.ts @@ -1,4 +1,4 @@ -import { renderMarkdown, renderSlack } from './format'; +import { renderMarkdown, renderSlack, renderText } from './format'; import { PRQAResult, ReleaseQAReport, SlackMapping } from './types'; function pr( @@ -219,4 +219,45 @@ describe('renderMarkdown', () => { const out = renderMarkdown(report()); expect(out).toContain('All non-excluded PRs have a recognized QA verdict'); }); -}); \ No newline at end of file +}); + +describe('spec/docs-only exclusions', () => { + // The whole point of the path heuristics: a release whose only "gaps" were + // Spec-Kit PR 1s must not page anyone. renderSlack keys off + // failed+missing+unlinked+external, so excluded PRs drop out for free — + // this locks that in against a future refactor folding excluded into flagged. + const specOnlyRelease = () => + report({ + summary: { failed: 0, missing: 0, unlinked: 0, external: 0, passed: 1, excluded: 2 }, + passed: [pr(37423, 'Dojo to Angular: dotAI Portlet', 'passed')], + excluded: [ + pr(37404, 'Spec: UVE contentlet permission gating', 'excluded', { + exclusionReason: 'spec-only', + }), + pr(37115, 'docs(speckit): link the quick start walkthrough video', 'excluded', { + exclusionReason: 'docs-only', + }), + ], + }); + + it('stays silent on Slack', () => { + expect(renderSlack(specOnlyRelease(), { mappings: [] })).toBe(''); + }); + + it('lists the reasons in the markdown excluded table', () => { + const out = renderMarkdown(specOnlyRelease()); + expect(out).toContain('## Excluded (2)'); + expect(out).toContain('`spec-only`'); + expect(out).toContain('`docs-only`'); + // The blurb above the table must name them, or a reader sees an + // unexplained reason in a section that only ever meant "bot / bump". + expect(out).toMatch(/spec-only.*docs-only/s); + }); + + it('lists the reasons in the text excluded section', () => { + const out = renderText(specOnlyRelease()); + expect(out).toContain('Excluded (2)'); + expect(out).toContain('#37404 Spec: UVE contentlet permission gating — @alice [spec-only]'); + expect(out).toContain('[docs-only]'); + }); +}); diff --git a/.github/scripts/release-qa-status/src/format.ts b/.github/scripts/release-qa-status/src/format.ts index 33fbb97047dc..f3ca8a246c32 100644 --- a/.github/scripts/release-qa-status/src/format.ts +++ b/.github/scripts/release-qa-status/src/format.ts @@ -81,9 +81,13 @@ export function renderText(report: ReleaseQAReport): string { report.summary.missing + report.summary.unlinked + report.summary.external; + // The Excluded section still renders below on a clean release: it is the only + // record of which PRs were skipped and why, and after the spec/docs-only rules + // the common case is exactly this one — nothing flagged *because* the gaps were + // spec PRs. Returning early here would leave that drop unexplained. if (totalFlagged === 0) { lines.push(':white_check_mark: All non-excluded PRs have a recognized QA verdict.'); - return lines.join('\n'); + lines.push(''); } for (const key of ['failed', 'missing', 'unlinked', 'external'] as const) { @@ -99,7 +103,8 @@ export function renderText(report: ReleaseQAReport): string { if (report.excluded.length > 0) { lines.push(`Excluded (${report.excluded.length})`); lines.push( - ' Bot / dependency-bump / version-bump / release-machinery PRs (skipped before QA check).' + ' Bot / dependency-bump / version-bump / release-machinery / spec-only / docs-only PRs\n' + + ' (skipped before QA check).' ); for (const pr of report.excluded) lines.push(renderExcludedTextLine(pr)); lines.push(''); @@ -161,10 +166,12 @@ export function renderMarkdown(report: ReleaseQAReport): string { out.push(`| Excluded | ${s.excluded} |`); out.push(''); + // See renderText: the Excluded section below must survive a clean release, + // so this reports the all-clear instead of returning on it. const flagged = s.failed + s.missing + s.unlinked + s.external; if (flagged === 0) { out.push(':white_check_mark: All non-excluded PRs have a recognized QA verdict.'); - return out.join('\n'); + out.push(''); } const sections: Array<{ bucket: PRQAResult[]; key: BucketKey }> = [ @@ -200,7 +207,11 @@ export function renderMarkdown(report: ReleaseQAReport): string { if (report.excluded.length > 0) { out.push(`## Excluded (${report.excluded.length})`); out.push(''); - out.push('Bot / dependency-bump / version-bump / release-machinery PRs.'); + out.push( + 'Bot / dependency-bump / version-bump / release-machinery PRs, plus PRs that ' + + 'change only specs or documentation (`spec-only` / `docs-only`) and so carry ' + + 'nothing for QA to exercise.' + ); out.push(''); out.push('| PR | Title | Author | Reason |'); out.push('|---|---|---|---|'); diff --git a/.github/scripts/release-qa-status/src/github.test.ts b/.github/scripts/release-qa-status/src/github.test.ts index 339dd7fd11e8..f189c3b16cb3 100644 --- a/.github/scripts/release-qa-status/src/github.test.ts +++ b/.github/scripts/release-qa-status/src/github.test.ts @@ -1,4 +1,4 @@ -import { findPreviousTag } from './github'; +import { findPreviousTag, parseChangedFilesResponse } from './github'; // Guards against drift from gather-release-data/src/github.ts, which resolves the // same release boundary. If these two disagree, the QA status and the changelog @@ -30,3 +30,37 @@ describe('findPreviousTag', () => { expect(findPreviousTag(releases, 'v26.08.19-01')).toBeUndefined(); }); }); + +describe('parseChangedFilesResponse', () => { + const files = (paths: string[], hasNextPage = false) => ({ + files: { nodes: paths.map((path) => ({ path })), pageInfo: { hasNextPage } }, + }); + + it('maps each alias to its paths', () => { + const data = { repository: { pr1: files(['specs/foo/spec.md', 'README.md']) } }; + expect(parseChangedFilesResponse(data, [1])).toEqual( + new Map([[1, ['specs/foo/spec.md', 'README.md']]]) + ); + }); + + // Everything below must yield `undefined`, never `[]`. An empty array reads as + // "changed nothing", which classifyExclusion would be free to exclude on; the + // point of these cases is that we do not know, so the PR stays in QA scope. + it('returns undefined when the list is truncated', () => { + const spy = jest.spyOn(process.stderr, 'write').mockImplementation(() => true); + const data = { repository: { pr2: files(['a.md'], true) } }; + expect(parseChangedFilesResponse(data, [2]).get(2)).toBeUndefined(); + expect(spy).toHaveBeenCalledWith(expect.stringContaining('#2')); + spy.mockRestore(); + }); + + it('returns undefined for a missing alias, a null PR, and a null files connection', () => { + const data = { repository: { pr4: null, pr5: { files: null } } }; + const out = parseChangedFilesResponse(data, [3, 4, 5]); + expect(out.get(3)).toBeUndefined(); + expect(out.get(4)).toBeUndefined(); + expect(out.get(5)).toBeUndefined(); + // Present-but-unknown, not absent: classifyExclusion reads every PR. + expect(out.size).toBe(3); + }); +}); diff --git a/.github/scripts/release-qa-status/src/github.ts b/.github/scripts/release-qa-status/src/github.ts index 16e996e41768..e29791991d1c 100644 --- a/.github/scripts/release-qa-status/src/github.ts +++ b/.github/scripts/release-qa-status/src/github.ts @@ -312,6 +312,136 @@ export async function fetchClosingIssueRefs( return results; } +interface GraphQLFilesResponse { + repository: Record< + string, + { + files: { + nodes: Array<{ path: string }>; + pageInfo: { hasNextPage: boolean }; + } | null; + } | null + >; +} + +/** Max paths one GraphQL page returns — GitHub caps `first` at 100. */ +const FILES_PAGE_SIZE = 100; + +/** + * Shape one GraphQL batch response into per-PR path lists. + * + * `undefined` means "we don't know what this PR changed" and is deliberately + * distinct from `[]`. A PR whose alias is missing, whose `files` connection is + * null, or that has more files than one page holds all resolve to `undefined` + * so the caller keeps it in QA scope. Not paginating past 100 is a judgement + * call, not an oversight: this list exists only to answer "is every file + * documentation?", and a 100+ file PR is never documentation-only. + * + * Exported for tests — pure, so it needs no network. + */ +export function parseChangedFilesResponse( + data: GraphQLFilesResponse, + prNumbers: number[] +): Map { + const results = new Map(); + + for (const n of prNumbers) { + const pr = data.repository?.[`pr${n}`]; + if (!pr || !pr.files) { + results.set(n, undefined); + continue; + } + if (pr.files.pageInfo.hasNextPage) { + process.stderr.write( + `Note: PR #${n} changed more than ${FILES_PAGE_SIZE} files — ` + + `treating its file list as unknown (stays in QA scope).\n` + ); + results.set(n, undefined); + continue; + } + results.set( + n, + pr.files.nodes.map((f) => f.path) + ); + } + + return results; +} + +/** + * Fetch the changed paths for a batch of PRs, so `classifyExclusion` can tell + * a spec/docs-only PR from one carrying implementation. + * + * Batched GraphQL rather than one REST `pulls.listFiles` per PR: the report + * already spends 3+ REST calls per PR, and this adds roughly one request per + * twenty instead of one per PR. + * + * Unlike `fetchClosingIssueRefs`, a GraphQL failure here is swallowed rather + * than re-thrown. There, an empty result silently demotes PRs to `unlinked` + * and floods Slack with bogus warnings, so failing loudly is right. Here the + * failure mode is the opposite and harmless: an unknown file list simply means + * no path-based exclusion, which is exactly how the report behaved before this + * existed. Killing the whole QA section over it would be the worse trade. + */ +export async function fetchChangedFiles( + octokit: Octokit, + owner: string, + repo: string, + prNumbers: number[] +): Promise> { + const results = new Map(); + const BATCH = 20; + + for (let i = 0; i < prNumbers.length; i += BATCH) { + const batch = prNumbers.slice(i, i + BATCH); + // GraphQL aliases must be static field names (no $variables), so PR numbers + // are interpolated. They come from the commits→pulls API and are already + // integers; re-check anyway before building the query. + const safeBatch = batch.filter((n) => Number.isInteger(n) && n > 0); + if (safeBatch.length === 0) continue; + + const aliases = safeBatch + .map( + (n) => + ` pr${n}: pullRequest(number: ${n}) {\n` + + ` files(first: ${FILES_PAGE_SIZE}) {\n` + + ` nodes { path }\n` + + ` pageInfo { hasNextPage }\n` + + ` }\n` + + ` }` + ) + .join('\n'); + + const query = + `query($owner: String!, $repo: String!) {\n` + + ` repository(owner: $owner, name: $repo) {\n` + + aliases + + `\n }\n}`; + + try { + const data = await octokit.graphql(query, { + owner, + repo, + }); + for (const [n, files] of parseChangedFilesResponse(data, safeBatch)) { + results.set(n, files); + } + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error); + process.stderr.write( + `Warning: could not fetch changed files for PR(s) ` + + `${safeBatch.map((n) => `#${n}`).join(', ')}: ${msg}. ` + + `They stay in QA scope (no path-based exclusion).\n` + ); + for (const n of safeBatch) results.set(n, undefined); + } + + if (i + BATCH < prNumbers.length) await sleep(500); + } + + return results; +} + export async function fetchPRDetails( octokit: Octokit, owner: string, @@ -366,12 +496,9 @@ export async function fetchPRDetails( ); } - const refs = await fetchClosingIssueRefs( - octokit, - owner, - repo, - Array.from(results.keys()) - ); + const prNumbersFetched = Array.from(results.keys()); + + const refs = await fetchClosingIssueRefs(octokit, owner, repo, prNumbersFetched); for (const [n, pr] of results) { const r = refs.get(n); if (r) { @@ -380,6 +507,12 @@ export async function fetchPRDetails( } } + const changed = await fetchChangedFiles(octokit, owner, repo, prNumbersFetched); + for (const [n, pr] of results) { + // Leave `changedFiles` undefined when unknown — see classifyExclusion. + pr.changedFiles = changed.get(n); + } + return results; } diff --git a/.github/scripts/release-qa-status/src/qa.test.ts b/.github/scripts/release-qa-status/src/qa.test.ts index 1f2360bc13c6..c6c223d882a7 100644 --- a/.github/scripts/release-qa-status/src/qa.test.ts +++ b/.github/scripts/release-qa-status/src/qa.test.ts @@ -39,6 +39,42 @@ describe('computePRQA', () => { expect(result.linkedIssues).toEqual([]); }); + it('excludes a spec-only PR even when it does link an issue', () => { + // A Spec-Kit PR 1 usually closes nothing, but when it does the linked issue + // is the *feature* issue, which QA will label once the implementation PR + // ships. Reading its label here would report the spec PR as un-QA'd. + const result = computePRQA( + pr({ + linkedIssues: [37376], + changedFiles: ['specs/37376-uve-edit-pencil-permission/spec.md'], + }), + new Map([[37376, issue(37376, [])]]) + ); + expect(result.status).toBe('excluded'); + expect(result.exclusionReason).toBe('spec-only'); + expect(result.linkedIssues).toEqual([]); + }); + + it('excludes a docs-only PR', () => { + const result = computePRQA( + pr({ changedFiles: ['docs/core/GIT_WORKFLOWS.md'] }), + new Map() + ); + expect(result.status).toBe('excluded'); + expect(result.exclusionReason).toBe('docs-only'); + }); + + it('still evaluates QA for a PR that ships a spec plus implementation', () => { + const result = computePRQA( + pr({ + linkedIssues: [37376], + changedFiles: ['specs/37376-foo/spec.md', 'dotCMS/src/main/java/Foo.java'], + }), + new Map([[37376, issue(37376, ['QA : Passed'])]]) + ); + expect(result.status).toBe('passed'); + }); + it('returns unlinked when no closing refs of either kind exist', () => { const result = computePRQA(pr(), new Map()); expect(result.status).toBe('unlinked'); diff --git a/.github/scripts/release-qa-status/src/types.ts b/.github/scripts/release-qa-status/src/types.ts index 7eb07dc8d75b..6d3168496a5b 100644 --- a/.github/scripts/release-qa-status/src/types.ts +++ b/.github/scripts/release-qa-status/src/types.ts @@ -16,7 +16,9 @@ export type ExclusionReason = | 'bot-author' | 'dependency-bump' | 'version-bump' - | 'release-machinery'; + | 'release-machinery' + | 'spec-only' + | 'docs-only'; /** Label resolution for a single linked issue. */ export interface LinkedIssueInfo { @@ -49,6 +51,13 @@ export interface PRDetails { linkedIssues: number[]; /** Cross-repo closing-issue references. */ externalRefs: ExternalRef[]; + /** + * Paths changed by the PR. `undefined` means the list is unknown — the API + * call failed, or the PR has more files than one page returns. Never exclude + * a PR on an unknown list: a missing list must degrade to today's behaviour, + * not silently hide a code change from QA. + */ + changedFiles?: string[]; } /** A single PR with its computed QA result. */