diff --git a/.github/scripts/generate-pr-comment.js b/.github/scripts/generate-pr-comment.js index 428cca610b882..dfcbf268f63e3 100644 --- a/.github/scripts/generate-pr-comment.js +++ b/.github/scripts/generate-pr-comment.js @@ -2,18 +2,23 @@ // Copyright (c) Meta Platforms, Inc. and affiliates. /** - * @description Generates and posts PR comment with analysis results - * @input --analysis --base-stats --package-size --a11y --visual --storybook-url --sandbox-url --preview-state --source-conclusion --run-url - * @output Formatted markdown comment body to stdout + * @description Generates the PR comment from the run's analysis results + * @input --analysis --base-stats --package-size --a11y --visual --preview --jobs --source-conclusion --run-url + * @output Formatted markdown comment body to stdout, compact by design: this + * head's preview first, the modified components (preview links, size, lines + * of code, a mark per browser suite that ran; three rows visible, the rest + * collapsed), the changed shots collapsed, one line for package size and + * one for suites that did not run. Accessibility violations add one + * collapsed section. GitHub shows + * the CI checks and CodeBunny posts its own review; neither is repeated. */ const fs = require('node:fs'); const {buildA11ySection} = require('./lib/a11y-format'); -const {buildVisualSection} = require('./lib/visual-format'); -const {renderPackageSizeSection} = require('./lib/package-size'); +const {buildCompactReport} = require('./lib/pr-status'); // Report-file strings render as literal inline text, report numbers as // numbers — the comment's structure comes only from this file's literals. -const {inline, num, sizeText} = require('./lib/report-text'); +const {inline, num} = require('./lib/report-text'); const args = process.argv.slice(2); const getArg = name => { @@ -23,335 +28,86 @@ const getArg = name => { const analysisFile = getArg('analysis') || 'analysis.json'; const baseStatsFile = getArg('base-stats'); -const packageSizeFile = getArg('package-size'); const a11yFile = getArg('a11y') || 'a11y-report.json'; -const visualFile = getArg('visual'); const runUrl = getArg('run-url') || ''; -const prNumber = getArg('pr-number') || ''; -const storybookUrl = getArg('storybook-url') || ''; -const sandboxUrl = getArg('sandbox-url') || ''; -const previewState = getArg('preview-state') || ''; const sourceConclusion = getArg('source-conclusion') || ''; -// Read analysis results -let analysis = { - newComponents: [], - modifiedComponents: [], - componentStats: {}, - totalBundle: {}, -}; -let a11yReport = null; // stays null when the audit did not run +// Optional JSON inputs. A missing or unreadable file leaves its part out. +function readOptionalJson(name, file = getArg(name)) { + if (!file) return null; + try { + return JSON.parse(fs.readFileSync(file, 'utf8')); + } catch (e) { + console.error(`Warning: Could not read --${name}:`, e.message); + return null; + } +} + +let analysis = {newComponents: [], modifiedComponents: [], componentStats: {}}; try { analysis = JSON.parse(fs.readFileSync(analysisFile, 'utf8')); } catch (e) { console.error('Warning: Could not read analysis file:', e.message); } -// Base measurements from a built checkout of the merge base, written by -// `analyze-pr.js --base-only`. The caller has already matched their head. -if (baseStatsFile) { - try { - const base = JSON.parse(fs.readFileSync(baseStatsFile, 'utf8')); - analysis.baseComponentStats = base.baseComponentStats || undefined; - analysis.baseBundlePackages = base.baseBundlePackages || undefined; - analysis.baseComponentSizes = base.baseComponentSizes || undefined; - } catch (e) { - console.error('Warning: Could not read base stats:', e.message); - } -} - +// Base measurements of the merge base, when the caller has one for this +// exact head (it has matched the head). +const baseStats = readOptionalJson('base-stats', baseStatsFile); +if (baseStats) { + analysis.baseComponentStats = baseStats.baseComponentStats || undefined; + analysis.baseBundlePackages = baseStats.baseBundlePackages || undefined; + analysis.baseComponentSizes = baseStats.baseComponentSizes || undefined; +} + +// This head's preview ({state, url, sha, aliasOrigin}), the source run's +// jobs, the package-size report, and the +// browser suites' results. The accessibility report is absent when the +// audit did not run; the visual verdict when the visual job did not. +const preview = readOptionalJson('preview'); +const jobs = readOptionalJson('jobs'); +const packageSize = readOptionalJson('package-size'); +const visualVerdict = readOptionalJson('visual'); +let a11yReport = null; try { a11yReport = JSON.parse(fs.readFileSync(a11yFile, 'utf8')); } catch (e) { console.error('Warning: Could not read a11y report:', e.message); } -// Build a Storybook deep link URL for a component -// Story titles like "Core/XDSButton" become path "/docs/core-xdsbutton--docs" -function getStorybookLink(storybookBaseUrl, storyTitle) { - if (!storybookBaseUrl || !storyTitle) return null; - // storyTitle may be a single string or an array when multiple story files match - const title = Array.isArray(storyTitle) ? storyTitle[0] : storyTitle; - if (!title) return null; - // Storybook converts "Core/XDSButton" to "core-xdsbutton" as the story ID - // prefix. Story ids only ever contain [a-z0-9_-]; the title came from a - // report file, so drop anything outside that alphabet before it lands in - // the href. - const storyPath = title - .toLowerCase() - .replace(/\//g, '-') - .replace(/[^a-z0-9_-]/g, ''); - return `${storybookBaseUrl}?path=/docs/${storyPath}--docs`; -} - -// Build an external link that opens in a new tab function extLink(text, url) { return `${text}`; } -// Difference of two measured byte counts, or null when either side was not -// measured. A missing size is never treated as 0. -function bytesDelta(after, before) { - return typeof after === 'number' && - Number.isFinite(after) && - typeof before === 'number' && - Number.isFinite(before) - ? after - before - : null; -} - -function signedSizeText(delta) { - if (delta === null) return 'N/A'; - return delta > 0 ? `+${sizeText(delta)}` : sizeText(delta); -} - -// Touch-only budget per component (architecture:component-style-authoring -// INV6, spec:AST-080 FR8): 2 KB gzip JavaScript, 1 KB gzip CSS. The report -// cannot tell whether growth runs only on a coarse pointer, so it flags any -// growth over budget and asks the author. -const JS_BUDGET_BYTES = 2 * 1024; -const CSS_BUDGET_BYTES = 1024; -const budgetFlags = []; - -// Build component stats section -let componentSection = ''; -if (analysis.newComponents && analysis.newComponents.length > 0) { - componentSection += `### New Components\n\n`; - for (const comp of analysis.newComponents) { - const stats = analysis.componentStats[comp] || {}; - const sbLink = getStorybookLink(storybookUrl, stats.storyTitle); - const sbBadge = sbLink ? ` · ${extLink('View in Storybook', sbLink)}` : ''; - const pkgBadge = stats.package - ? ` (${inline(stats.package)})` - : ''; - componentSection += `
\n${inline(comp)}${pkgBadge}${sbBadge}\n\n`; - componentSection += `| Metric | Value |\n|--------|-------|\n`; - componentSection += `| Entry File (ESM) | ${inline(stats.esmSize) || 'N/A'} |\n`; - if (stats.cjsSize) - componentSection += `| Entry File (CJS) | ${inline(stats.cjsSize)} |\n`; - componentSection += `| Lines of Code | ${inline(stats.linesOfCode) || 'N/A'} |\n`; - componentSection += `| Source Files | ${inline(stats.fileCount) || 'N/A'} |\n`; - componentSection += `| Complexity | ${inline(stats.complexityRating) || 'N/A'} (${num(stats.complexity)}) |\n`; - componentSection += `| Exports | ${stats.exports?.map(inline).join(', ') || 'N/A'} |\n`; - componentSection += `| Props Count | ${inline(stats.propsCount) || 'N/A'} |\n`; - componentSection += `| Has Tests | ${stats.hasTests ? 'Yes' : 'No'} |\n`; - componentSection += `| Has Stories | ${stats.hasStories ? 'Yes' : 'No'} |\n`; - componentSection += `\n
\n\n`; - } -} - -if (analysis.modifiedComponents && analysis.modifiedComponents.length > 0) { - componentSection += `### Modified Components\n\n`; - for (const comp of analysis.modifiedComponents) { - const stats = analysis.componentStats[comp] || {}; - const baseStats = analysis.baseComponentStats?.[comp] || {}; - const sbLink = getStorybookLink(storybookUrl, stats.storyTitle); - const sbBadge = sbLink ? ` · ${extLink('View in Storybook', sbLink)}` : ''; - - // Check if this modified directory has new exports - const newExports = analysis.newExports || []; - const compExports = stats.exports || []; - const newInComp = compExports.filter(e => newExports.includes(e)); - const newBadge = - newInComp.length > 0 ? ` · 🆕 ${newInComp.map(inline).join(', ')}` : ''; - const pkgBadge = stats.package - ? ` (${inline(stats.package)})` - : ''; - - componentSection += `
\n${inline(comp)}${pkgBadge}${sbBadge}${newBadge}\n\n`; - componentSection += `| Metric | Before | After | Delta |\n|--------|--------|-------|-------|\n`; - - const esmDelta = bytesDelta(stats.esmBytes, baseStats.esmBytes); - const esmDeltaStr = - esmDelta !== null - ? esmDelta > 0 - ? `+${esmDelta}B` - : `${esmDelta}B` - : 'N/A'; - - componentSection += `| Entry File (ESM) | ${inline(baseStats.esmSize) || 'N/A'} | ${inline(stats.esmSize) || 'N/A'} | ${esmDeltaStr} |\n`; - componentSection += `| Lines of Code | ${inline(baseStats.linesOfCode) || 'N/A'} | ${inline(stats.linesOfCode) || 'N/A'} | - |\n`; - componentSection += `| Complexity | ${inline(baseStats.complexityRating) || 'N/A'} | ${inline(stats.complexityRating) || 'N/A'} (${num(stats.complexity)}) | - |\n`; - componentSection += `\n
\n\n`; - } -} - -// Import and CSS size of every public component whose implementation changed, -// family folders included, before and after. -let sizeSection = ''; -const componentSizes = analysis.componentSizes || {}; -const baseComponentSizes = analysis.baseComponentSizes || {}; -const sizeKeys = [ - ...new Set([ - ...Object.keys(componentSizes), - ...Object.keys(baseComponentSizes), - ]), -].sort(); -if (sizeKeys.length > 0) { - sizeSection += '### Component Size\n\n'; - sizeSection += - '| Component | Import before | Import after | Delta | CSS before | CSS after | Delta |\n'; - sizeSection += - '|-----------|---------------|--------------|-------|------------|-----------|-------|\n'; - for (const key of sizeKeys) { - const head = componentSizes[key] || {}; - const base = baseComponentSizes[key] || {}; - const importDelta = bytesDelta(head.importGzipBytes, base.importGzipBytes); - const cssDelta = bytesDelta(head.cssGzipBytes, base.cssGzipBytes); - const jsOver = importDelta !== null && importDelta > JS_BUDGET_BYTES; - const cssOver = cssDelta !== null && cssDelta > CSS_BUDGET_BYTES; - if (jsOver || cssOver) - budgetFlags.push({comp: key, importDelta, cssDelta, jsOver, cssOver}); - sizeSection += `| ${inline(key)} | ${sizeText(base.importGzipBytes)} | ${sizeText(head.importGzipBytes)} | ${signedSizeText(importDelta)}${jsOver ? ' ⚠️' : ''} | ${sizeText(base.cssGzipBytes)} | ${sizeText(head.cssGzipBytes)} | ${signedSizeText(cssDelta)}${cssOver ? ' ⚠️' : ''} |\n`; - } - sizeSection += - '\nSizes are gzip. Import counts the JavaScript the import loads eagerly; CSS counts the package stylesheet rules referenced by the modules it can load from its own package, lazily loaded ones included.\n\n'; -} - -// Components whose growth passes the touch-only budget. -let budgetSection = ''; -if (budgetFlags.length > 0) { - budgetSection += '### ⚠️ Touch-only size budget\n\n'; - budgetSection += - 'These components grew past the touch-only budget (2 KB gzip JavaScript or 1 KB gzip CSS per component). If the growth runs only on a coarse pointer, load its JavaScript lazily so the import does not carry it, or justify the cost in the pull request description; CSS stays in the package stylesheet, so justify it.\n\n'; - budgetSection += - '| Component | JavaScript | CSS |\n|-----------|------------|-----|\n'; - for (const flag of budgetFlags) { - budgetSection += `| ${inline(flag.comp)} | ${signedSizeText(flag.importDelta)}${flag.jsOver ? ' ⚠️' : ''} | ${signedSizeText(flag.cssDelta)}${flag.cssOver ? ' ⚠️' : ''} |\n`; - } - budgetSection += '\n'; -} - -// Build accessibility section using shared module -const a11ySection = buildA11ySection(a11yReport); +const details = (summary, content) => + content + ? `
\n${summary}\n\n${content}\n
\n\n` + : ''; -// Visual regression is optional: the pr-visual job is skipped when no -// components changed, and absent entirely on older runs. -let visualVerdict = null; -if (visualFile) { - try { - visualVerdict = JSON.parse(fs.readFileSync(visualFile, 'utf8')); - } catch { - visualVerdict = null; - } -} -const visualSection = buildVisualSection( - visualVerdict, - getArg('visual-report-url'), - getArg('visual-image-url'), +// The audit's findings, collapsed: the component table carries each verdict. +const violations = Object.values(a11yReport?.components || {}).reduce( + (sum, component) => sum + (component.violations?.length || 0), + 0, ); +const a11yDetails = + violations > 0 + ? details( + `Accessibility: ${num(violations)} violation(s)`, + buildA11ySection(a11yReport), + ) + : ''; -// Build bundle size section — one row per package the PR actually touched. -let bundleSection = '### Bundle Size Summary\n\n'; -// Prefer the multi-package list; fall back to the legacy single-core shape so -// an older analysis.json still renders. -const bundlePackages = - analysis.bundlePackages && analysis.bundlePackages.length > 0 - ? analysis.bundlePackages - : analysis.totalBundle - ? [{package: '@astryxdesign/core', ...analysis.totalBundle}] - : []; - -const baseBundles = analysis.baseBundlePackages; -const signedBytes = delta => - delta === null ? 'N/A' : delta > 0 ? `+${delta}B` : `${delta}B`; -if (bundlePackages.length > 0 && Array.isArray(baseBundles)) { - bundleSection += `Gzip figures measure each package's entry file, not the code an import pulls in.\n\n`; - bundleSection += `| Package | Entry (ESM) | Entry gzip before | Entry gzip after | Delta |\n`; - bundleSection += `|---------|-------------|-------------------|------------------|-------|\n`; - for (const b of bundlePackages) { - const base = baseBundles.find(entry => entry.package === b.package) || {}; - const delta = bytesDelta(b.gzipBytes, base.gzipBytes); - bundleSection += `| ${inline(b.package)} | ${inline(b.esmSize) || 'N/A'} | ${inline(base.gzipSize) || 'N/A'} | ${inline(b.gzipSize) || 'N/A'} | ${signedBytes(delta)} |\n`; - } - bundleSection += `\n`; - const sheets = bundlePackages.filter( - b => typeof b.stylesheetGzipBytes === 'number', - ); - if (sheets.length > 0) { - bundleSection += `| Package stylesheet | Gzip before | Gzip after | Delta |\n`; - bundleSection += `|--------------------|-------------|------------|-------|\n`; - for (const b of sheets) { - const base = baseBundles.find(entry => entry.package === b.package) || {}; - const delta = bytesDelta(b.stylesheetGzipBytes, base.stylesheetGzipBytes); - bundleSection += `| ${inline(b.package)} | ${sizeText(base.stylesheetGzipBytes)} | ${sizeText(b.stylesheetGzipBytes)} | ${signedSizeText(delta)} |\n`; - } - bundleSection += `\n`; - } -} else if (bundlePackages.length > 0) { - bundleSection += `| Package | Size (ESM) | Size (CJS) | Gzipped |\n`; - bundleSection += `|---------|------------|------------|----------|\n`; - for (const b of bundlePackages) { - bundleSection += `| ${inline(b.package)} | ${inline(b.esmSize) || 'N/A'} | ${inline(b.cjsSize) || 'N/A'} | ${inline(b.gzipSize) || 'N/A'} |\n`; - } - bundleSection += `\n`; -} else { - bundleSection += '_No component packages changed._\n\n'; -} - -if (analysis.bundleDelta) { - const delta = num(analysis.bundleDelta); - const direction = - delta > 0 ? 'increased' : delta < 0 ? 'decreased' : 'unchanged'; - bundleSection += `**Bundle size ${direction}:** ${delta > 0 ? '+' : ''}${delta} bytes\n\n`; -} - -// Published package sizes from build-storybook's package-size check: the -// download and unpacked size of every published package, base against head, -// and any runtime dependency added or removed. The caller has already matched -// its head. -let packageSizeSection = ''; -if (packageSizeFile) { - try { - packageSizeSection = renderPackageSizeSection( - JSON.parse(fs.readFileSync(packageSizeFile, 'utf8')), - inline, - ); - } catch (e) { - console.error('Warning: Could not read package size report:', e.message); - } -} - -// Build storybook link section -let storybookSection = ''; -if (storybookUrl) { - storybookSection = `### 📚 Storybook Preview - -**${extLink('View Storybook for this PR', storybookUrl)}** - -`; -} - -// Build sandbox link section -let sandboxSection = ''; -if (sandboxUrl) { - sandboxSection = `### 🧪 Sandbox Preview - -**${extLink('View Sandbox for this PR', sandboxUrl)}** - -`; -} - -// Both previews come from the branch's Vercel alias, which serves the -// branch's latest deployment; until a push's deployment is ready it still -// shows the previous head. Vercel shortens a long alias with a hash, and then there are -// no links. -let previewAvailabilitySection = ''; -if (previewState) { - if (!storybookUrl || !sandboxUrl) - previewAvailabilitySection = `> **Preview availability:** this branch's preview address cannot be known in advance; open the preview from Vercel's comment on this pull request.${sourceConclusion && sourceConclusion !== 'success' ? ` CI concluded ${sourceConclusion}.` : ''}\n\n`; - else - previewAvailabilitySection = - "> **Preview:** the links below open this branch's latest Vercel deployment, which can trail a new push until that push's deployment is ready.\n\n"; -} +// Shots the visual job could not capture: one line; changed shots are listed +// in the report's visual section. +const visualFailed = + visualVerdict?.status === 'failed' + ? `> **Visual:** ${num(visualVerdict.counts?.failed)} shot(s) could not be captured.\n\n` + : ''; -// Build footer with links -let footerLinks = []; -if (storybookUrl) footerLinks.push(extLink('Storybook', storybookUrl)); -if (sandboxUrl) footerLinks.push(extLink('Sandbox', sandboxUrl)); -if (runUrl) footerLinks.push(extLink('View full report', runUrl)); -const footerLinksStr = - footerLinks.length > 0 ? ` | ${footerLinks.join(' | ')}` : ''; +// A run that did not succeed may have published only part of this report. +const conclusionNote = + sourceConclusion && sourceConclusion !== 'success' + ? `> **Current run:** CI concluded ${inline(sourceConclusion)}; parts of this report may be missing.\n\n` + : ''; // A one-line caveat shown when the analysis fell back to the approximate // two-dot diff. The file list may include base-only churn and, worse, may @@ -362,17 +118,24 @@ const diffModeCaveat = ? '> ⚠️ **Approximate analysis** - the diff could not be resolved to a merge base, so a two-dot fallback was used. The component list may include base-only changes or miss a component whose change also landed on base.\n\n' : ''; -// Build the full comment +const report = buildCompactReport({ + analysis, + preview, + jobs, + a11yReport, + visualVerdict, + packageSize, +}); +const footerLinksStr = runUrl + ? ` | ${extLink('View full report', runUrl)}` + : ''; + const body = `## PR Analysis Report -${diffModeCaveat}${previewAvailabilitySection}${storybookSection}${sandboxSection}${componentSection || (sizeSection ? '' : '_No new or modified components detected._\n\n')} -${sizeSection}${budgetSection}${bundleSection}${packageSizeSection} -${a11ySection} -${visualSection}--- +${report || '_No component or package changed._\n\n'}${conclusionNote}${diffModeCaveat}${visualFailed}${a11yDetails}--- Generated by PR Enrichment workflow${footerLinksStr} `; -// Output to stdout console.log(body); diff --git a/.github/scripts/generate-pr-comment.test.mjs b/.github/scripts/generate-pr-comment.test.mjs index c96e933971354..eae54940b8dfb 100644 --- a/.github/scripts/generate-pr-comment.test.mjs +++ b/.github/scripts/generate-pr-comment.test.mjs @@ -66,321 +66,131 @@ describe('generate-pr-comment diffMode caveat', () => { }); }); -describe('generate-pr-comment preview availability', () => { - it.each([ - ['none', [], false, false], - [ - 'storybook', - ['--storybook-url', 'https://example.test/pr/1/'], - true, - false, - ], - [ - 'sandbox', - ['--sandbox-url', 'https://example.test/pr/1/sandbox/'], - false, - true, - ], - [ - 'both', - [ - '--storybook-url', - 'https://example.test/pr/1/', - '--sandbox-url', - 'https://example.test/pr/1/sandbox/', - ], - true, - true, - ], - ])( - 'renders independent %s availability', - (state, urls, storybook, sandbox) => { - const stdout = runComment(baseAnalysis, [ - ...urls, - '--preview-state', - state, - '--source-conclusion', - 'success', - ]); - - expect(stdout.includes('View Storybook for this PR')).toBe(storybook); - expect(stdout.includes('View Sandbox for this PR')).toBe(sandbox); - expect(stdout.includes('> **Preview availability:**')).toBe( - !storybook || !sandbox, - ); - expect(stdout).toContain(''); - }, - ); - - it('explains that failed CI published no previews', () => { - const stdout = runComment(baseAnalysis, [ - '--preview-state', - 'none', - '--source-conclusion', - 'failure', - ]); - - expect(stdout).toContain('CI concluded failure'); - expect(stdout).not.toContain('View Storybook for this PR'); - expect(stdout).not.toContain('View Sandbox for this PR'); - }); -}); - -describe('generate-pr-comment base sizes', () => { - const sized = { - ...baseAnalysis, - componentStats: { - Card: {package: '@astryxdesign/core', esmSize: '160B', esmBytes: 160}, - }, - bundlePackages: [ - { - package: '@astryxdesign/core', - esmSize: '900B', - gzipSize: '300B', - gzipBytes: 300, - }, - ], - }; - - it('fills Before and Delta from base stats', () => { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'astryx-base-stats-')); +describe('generate-pr-comment compact report', () => { + function withInputs(inputs, analysis = baseAnalysis, extra = []) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'astryx-report-')); try { - const baseFile = path.join(dir, 'size-base.json'); - fs.writeFileSync( - baseFile, - JSON.stringify({ - baseComponentStats: { - Card: { - package: '@astryxdesign/core', - esmSize: '100B', - esmBytes: 100, - }, - }, - baseBundlePackages: [ - {package: '@astryxdesign/core', gzipSize: '280B', gzipBytes: 280}, - ], - }), - ); - const out = runComment(sized, ['--base-stats', baseFile]); - expect(out).toContain('| Entry File (ESM) | 100B | 160B | +60B |'); - expect(out).toContain( - '| @astryxdesign/core | 900B | 280B | 300B | +20B |', - ); + const args = []; + for (const [name, value] of Object.entries(inputs)) { + const file = path.join(dir, `${name}.json`); + fs.writeFileSync(file, JSON.stringify(value)); + args.push(`--${name}`, file); + } + return runComment(analysis, [...args, ...extra]); } finally { fs.rmSync(dir, {recursive: true, force: true}); } + } + const HEAD = 'a'.repeat(40); + const READY = 'https://astryx-abc123-fbopensource.vercel.app'; + const size = (importGzipBytes, cssGzipBytes = 1000) => ({ + package: '@astryxdesign/core', + importGzipBytes, + cssGzipBytes, + }); + const sized = names => ({ + ...baseAnalysis, + modifiedComponents: [], + componentSizes: Object.fromEntries( + names.map(([key, after]) => [key, size(after)]), + ), + baseComponentSizes: Object.fromEntries( + names.map(([key, , before]) => [key, size(before)]), + ), + }); + + it("opens with this head's preview, before anything else", () => { + const out = withInputs({preview: {state: 'ready', sha: HEAD, url: READY}}); + const body = out.slice(out.indexOf('')); + expect(body.split('\n')[2]).toBe('### 🔎 Preview · `aaaaaaa`'); + expect(body.split('\n')[4]).toBe( + `**[Storybook](${READY}/storybook/) · [Sandbox](${READY}/sandbox/)** · built from this PR's head`, + ); + }); + + it('shows three components and collapses the rest, the biggest growth first', () => { + const out = withInputs( + {preview: {state: 'ready', sha: HEAD, url: READY}}, + sized([ + ['core/A', 5000, 5000], + ['core/B', 9000, 8000], + ['core/C', 7000, 6900], + ['core/D', 4000, 4100], + ['core/E', 3000, 2990], + ]), + ); + const visible = out.slice(0, out.indexOf('
')); + expect(visible).toContain('### Modified components'); + expect(visible.match(/^\| \[[A-E]\]/gm)).toHaveLength(3); + expect(visible.indexOf('[B]')).toBeLessThan(visible.indexOf('[C]')); + expect(visible.indexOf('[C]')).toBeLessThan(visible.indexOf('[E]')); + expect(out).toContain('2 more components'); + expect(out.slice(out.indexOf('
'))).toContain( + `| [D](${READY}/components/D) |`, + ); + expect(out).toContain(`| [B](${READY}/components/B) |`); }); it('never reports a delta for an unmeasured size', () => { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'astryx-base-null-')); + const out = runComment({ + ...baseAnalysis, + componentSizes: {'core/Card': size(null), 'core/New': size(3000)}, + baseComponentSizes: {'core/Card': size(2000)}, + }); + expect(out).toContain('| Card | 1000 B CSS (0) |'); + expect(out).toContain('| New | 2.9 kB (new) · 1000 B CSS (new) |'); + }); + + it('fills the base from base stats', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'astryx-base-')); try { - const baseFile = path.join(dir, 'size-base.json'); + const baseFile = path.join(dir, 'base.json'); fs.writeFileSync( baseFile, - JSON.stringify({ - baseComponentStats: { - Card: { - package: '@astryxdesign/charts', - esmSize: null, - esmBytes: null, - }, - }, - baseBundlePackages: [ - {package: '@astryxdesign/core', gzipSize: null, gzipBytes: null}, - ], - }), + JSON.stringify({baseComponentSizes: {'core/Card': size(2000)}}), ); const out = runComment( - { - ...sized, - componentStats: { - Card: { - package: '@astryxdesign/charts', - esmSize: null, - esmBytes: null, - }, - }, - bundlePackages: [ - { - package: '@astryxdesign/core', - esmSize: '900B', - gzipSize: '300B', - gzipBytes: 300, - }, - ], - }, + {...baseAnalysis, componentSizes: {'core/Card': size(1500)}}, ['--base-stats', baseFile], ); - expect(out).toContain('| Entry File (ESM) | N/A | N/A | N/A |'); - expect(out).toContain('| @astryxdesign/core | 900B | N/A | 300B | N/A |'); - expect(out).not.toMatch(/\| [+-]?0B \|/); + expect(out).toContain('| Card | 1.5 kB (−500 B) · 1000 B CSS (0) |'); } finally { fs.rmSync(dir, {recursive: true, force: true}); } }); - it('keeps N/A without base stats', () => { - const out = runComment(sized); - expect(out).toContain('| Entry File (ESM) | N/A | 160B | N/A |'); - expect(out).toContain('| Package | Size (ESM) | Size (CJS) | Gzipped |'); - }); -}); - -describe('generate-pr-comment touch-only budget', () => { - function withBase(head, base) { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'astryx-budget-')); + it('collapses accessibility violations', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'astryx-a11y-')); try { - const baseFile = path.join(dir, 'size-base.json'); + const file = path.join(dir, 'a11y.json'); fs.writeFileSync( - baseFile, + file, JSON.stringify({ - baseComponentSizes: {'core/BottomSheet': base}, - baseBundlePackages: [], + components: { + Card: { + violations: [{id: 'label', impact: 'serious', description: 'x'}], + }, + }, }), ); - return runComment( - { - ...baseAnalysis, - modifiedComponents: [], - componentSizes: {'core/BottomSheet': head}, - bundlePackages: [], - }, - ['--base-stats', baseFile], - ); + const out = runComment(baseAnalysis, ['--a11y', file]); + expect(out).toContain('Accessibility: 1 violation(s)'); + expect(out).toContain('| Card | — | — | — · ✗ · — |'); } finally { fs.rmSync(dir, {recursive: true, force: true}); } - } - - it('flags JavaScript growth over 2 KB and CSS growth over 1 KB, family folders included', () => { - const out = withBase( - { - package: '@astryxdesign/core', - importGzipBytes: 10000, - cssGzipBytes: 3000, - }, - { - package: '@astryxdesign/core', - importGzipBytes: 7000, - cssGzipBytes: 1900, - }, - ); - expect(out).toContain('### ⚠️ Touch-only size budget'); - expect(out).toContain( - '| core/BottomSheet | 6.8KB | 9.8KB | +2.9KB ⚠️ | 1.9KB | 2.9KB | +1.1KB ⚠️ |', - ); }); - it('does not flag growth within budget', () => { - const out = withBase( - { - package: '@astryxdesign/core', - importGzipBytes: 9000, - cssGzipBytes: 2900, - }, - { - package: '@astryxdesign/core', - importGzipBytes: 7000, - cssGzipBytes: 1900, - }, - ); - expect(out).not.toContain('Touch-only size budget'); - expect(out).toContain( - '| core/BottomSheet | 6.8KB | 8.8KB | +2.0KB | 1.9KB | 2.8KB | +1000B |', - ); - }); - - it('does not flag a component that is new at head', () => { - const out = withBase( - { - package: '@astryxdesign/core', - importGzipBytes: 90000, - cssGzipBytes: 9000, - }, - { - package: '@astryxdesign/core', - importGzipBytes: null, - cssGzipBytes: null, - }, - ); - expect(out).not.toContain('Touch-only size budget'); - }); -}); - -describe('published package size', () => { - function withPackageSize(report) { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'astryx-package-size-')); - try { - const file = path.join(dir, 'package-size.json'); - fs.writeFileSync(file, JSON.stringify(report)); - return runComment({newComponents: [], modifiedComponents: []}, [ - '--package-size', - file, - ]); - } finally { - fs.rmSync(dir, {recursive: true, force: true}); - } - } - - const sizes = (packedBytes, files = 10) => ({ - packedBytes, - unpackedBytes: packedBytes * 4, - files, - }); - - it('renders the section after the bundle summary, with new dependencies', () => { - const out = withPackageSize({ - headSha: 'a'.repeat(40), - budget: {packedPercent: 5, packedMinBytes: 1024}, - packages: [ - { - name: '@astryxdesign/core', - base: sizes(100000, 10), - head: sizes(110000, 12), - overBudget: true, - }, - ], - addedDependencies: [ - { - package: '@astryxdesign/core', - field: 'dependencies', - name: 'left-pad', - range: '^1.3.0', - }, - ], - removedDependencies: [], - verdict: 'over-budget', - }); - expect(out.indexOf('### Bundle Size Summary')).toBeLessThan( - out.indexOf('### Published Package Size'), - ); - expect(out).toContain( - '| @astryxdesign/core | 97.7KB | 107.4KB | +9.8KB (+10.0%) ⚠️ |', - ); - expect(out).toContain('| 10 → 12 |'); - expect(out).toContain('left-pad'); - expect(out).toContain('**⚠️ Over the package budget:**'); + it('notes a run that did not succeed', () => { + const out = runComment(baseAnalysis, ['--source-conclusion', 'failure']); + expect(out).toContain('CI concluded failure'); }); it('renders report strings as literal text', () => { - const out = withPackageSize({ - packages: [ - { - name: '', - base: sizes(1000), - head: sizes(2000), - overBudget: false, - }, - ], - addedDependencies: [], - removedDependencies: [], - verdict: 'pass', + const out = runComment({ + ...baseAnalysis, + componentSizes: {'core/': size(1000)}, }); - expect(out).not.toContain(' { - const out = runComment({newComponents: [], modifiedComponents: []}); - expect(out).not.toContain('Published Package Size'); + expect(out).not.toContain(''); }); }); diff --git a/.github/scripts/lib/package-size.test.mjs b/.github/scripts/lib/package-size.test.mjs index 6c4235ed25fa2..bef6930a1d2a6 100644 --- a/.github/scripts/lib/package-size.test.mjs +++ b/.github/scripts/lib/package-size.test.mjs @@ -263,7 +263,7 @@ describe('renderPackageSizeSection', () => { ); expect(markdown).toContain('### Published Package Size'); expect(markdown).toContain( - '| @astryxdesign/core | 97.7KB | 98.6KB | +1000B (+1.0%) |', + '| @astryxdesign/core | 97.7 kB | 98.6 kB | +1000 B (+1.0%) |', ); expect(markdown).not.toContain('theme-neutral'); expect(markdown).toContain('2 other published packages are unchanged.'); @@ -279,7 +279,7 @@ describe('renderPackageSizeSection', () => { }), ); expect(markdown).toContain( - '| @astryxdesign/core | 8.58MB | 8.68MB | +97.7KB (+1.1%) |', + '| @astryxdesign/core | 8.58 MB | 8.68 MB | +97.7 kB (+1.1%) |', ); }); diff --git a/.github/scripts/lib/pr-preview.mjs b/.github/scripts/lib/pr-preview.mjs index 2c1880a2fd1a7..41b69970a06bb 100644 --- a/.github/scripts/lib/pr-preview.mjs +++ b/.github/scripts/lib/pr-preview.mjs @@ -2,10 +2,14 @@ import {execFileSync} from 'node:child_process'; import fs from 'node:fs'; +import os from 'node:os'; import path from 'node:path'; import process from 'node:process'; +import {createRequire} from 'node:module'; import {fileURLToPath} from 'node:url'; +const {previewLine} = createRequire(import.meta.url)('./pr-status.js'); + export const PR_ANALYSIS_MARKER = ''; function headMarker(sha) { @@ -49,6 +53,97 @@ function refuse(message) { throw new Error(`PR preview refused: ${message}`); } +const VERCEL_DEPLOYMENT_URL = /^https:\/\/[a-z0-9-]+\.vercel\.app\/?$/; + +/** + * This head's Vercel preview, from the deployment Vercel reports to GitHub for + * the exact commit: `ready` (that deployment's own address, so every link + * shows this head), `failed`, or `building` until Vercel reports it (the + * branch alias then serves the previous ready deployment). A fork's head is + * looked up the same way. Two reads in the comment job; no workflow runs on a + * deployment event. + */ +export async function headPreview({ + github, + owner, + repo, + identity, + aliasOrigin, +}) { + const sha = identity.headSha; + try { + const {data: deployments} = await github.rest.repos.listDeployments({ + owner, + repo, + sha, + per_page: 20, + }); + const deployment = deployments + .filter( + candidate => + candidate.creator?.login === 'vercel[bot]' && + String(candidate.environment).toLowerCase() === 'preview', + ) + .sort((a, b) => b.id - a.id)[0]; + if (deployment) { + const {data: statuses} = await github.rest.repos.listDeploymentStatuses({ + owner, + repo, + deployment_id: deployment.id, + per_page: 1, + }); + const latest = statuses[0]; + if ( + latest?.state === 'success' && + VERCEL_DEPLOYMENT_URL.test(String(latest.environment_url ?? '')) + ) + return { + state: 'ready', + sha, + url: latest.environment_url.replace(/\/$/, ''), + }; + if (latest?.state === 'failure' || latest?.state === 'error') + return {state: 'failed', sha}; + } + } catch { + // An unreadable deployment list leaves the preview as still building. + } + return aliasOrigin + ? {state: 'building', sha, aliasOrigin} + : {state: 'unknown', sha}; +} + +/** + * The source run's jobs (latest attempt) with each step's conclusion, so the + * report can say which jobs ran and which skipped their work by path. Null + * when the list cannot be read. + */ +export async function sourceRunJobs({github, owner, repo, runId}) { + try { + const jobs = await github.paginate( + github.rest.actions.listJobsForWorkflowRun, + { + owner, + repo, + run_id: runId, + filter: 'latest', + per_page: 100, + }, + ); + return jobs.map(job => ({ + name: job.name, + status: job.status, + conclusion: job.conclusion, + steps: (job.steps || []).map(step => ({ + name: step.name, + conclusion: step.conclusion, + })), + })); + } catch { + return null; + } +} + /** * The pull request moved on from the source run: it closed, merged, or got a * newer head. That is ordinary churn, not a forged identity. Nothing is @@ -415,27 +510,13 @@ export function validateAnalysisMetadata(metadata, identity) { return metadata; } -function previewState(storybook, sandbox) { - if (storybook && sandbox) return 'both'; - if (storybook) return 'storybook'; - if (sandbox) return 'sandbox'; - return 'none'; -} - -// The same preview wording as the generator's report: the branch alias -// serves the branch's latest deployment, which can trail a new push. A draft -// or an update-only reconcile (no alias attempted) says nothing about previews. -const PREVIEW_NOTE = - "> **Preview:** the links below open this branch's latest Vercel deployment, which can trail a new push until that push's deployment is ready."; -const PREVIEW_POINTER = - "> **Preview availability:** this branch's preview address cannot be known in advance; open the preview from Vercel's comment on this pull request."; - +// Without a trusted analysis, the report keeps the same preview line as the +// full report (pr-status.js), then says why the analysis is missing. function safeCurrentBody({ identity, runUrl, message: overrideMessage, - previewOrigin, - aliasAttempted, + preview, }) { const conclusion = identity.sourceConclusion; const message = @@ -443,17 +524,7 @@ function safeCurrentBody({ (conclusion === 'success' ? 'The current CI run completed, but its trusted analysis is unavailable.' : `The current CI run concluded ${conclusion || 'without a result'}. Current analysis is unavailable.`); - const links = previewOrigin - ? [ - `[View Storybook for this PR](${previewOrigin}/storybook/)`, - `[View Sandbox for this PR](${previewOrigin}/sandbox/)`, - ] - : []; - let previewMessage = ''; - if (links.length) - previewMessage = `\n\n${PREVIEW_NOTE}\n\n${links.join(' · ')}`; - else if (aliasAttempted) previewMessage = `\n\n${PREVIEW_POINTER}`; - return `## PR Analysis Report\n${PR_ANALYSIS_MARKER}\n${headMarker(identity.headSha)}\n\n> **Current run:** ${message}${previewMessage}\n\n---\n\nGenerated by PR Enrichment workflow | View current CI run\n`; + return `## PR Analysis Report\n${PR_ANALYSIS_MARKER}\n${headMarker(identity.headSha)}\n\n${previewLine(preview)}> **Current run:** ${message}\n\n---\n\nGenerated by PR Enrichment workflow | View current CI run\n`; } function readJSON(file) { @@ -545,8 +616,6 @@ export async function reconcilePrComment({ // preview and says nothing about one. const aliasAttempted = createIfMissing && !identity.draft; const previewOrigin = aliasAttempted ? branchPreviewOrigin(identity) : null; - const storybook = previewOrigin !== null; - const sandbox = storybook; const comments = await allComments(github, owner, repo, identity.prNumber); const botComments = comments.filter(comment => comment.user?.type === 'Bot'); const botComment = @@ -559,8 +628,30 @@ export async function reconcilePrComment({ return {action: 'none', body: null, identity}; } + // This head's preview, for the full report and the fallback alike. + const preview = aliasAttempted + ? await headPreview({ + github, + owner, + repo, + identity, + aliasOrigin: previewOrigin, + }) + : null; let body; if (analysisReady) { + // The comment job's own reads, handed to the generator as files. + const inputs = fs.mkdtempSync(path.join(os.tmpdir(), 'pr-report-')); + const jobs = await sourceRunJobs({ + github, + owner, + repo, + runId: identity.sourceRunId, + }); + const previewPath = path.join(inputs, 'preview.json'); + const jobsPath = path.join(inputs, 'jobs.json'); + if (preview) fs.writeFileSync(previewPath, JSON.stringify(preview)); + if (jobs) fs.writeFileSync(jobsPath, JSON.stringify(jobs)); const args = [ generator, '--analysis', @@ -585,11 +676,8 @@ export async function reconcilePrComment({ // checks, or with the ci:run-a11y label); the comment says so. ...(fs.existsSync(a11yPath) ? ['--a11y', a11yPath] : []), ...(fs.existsSync(visualPath) ? ['--visual', visualPath] : []), - ...(storybook ? ['--storybook-url', `${previewOrigin}/storybook/`] : []), - ...(sandbox ? ['--sandbox-url', `${previewOrigin}/sandbox/`] : []), - ...(aliasAttempted - ? ['--preview-state', previewState(storybook, sandbox)] - : []), + ...(preview ? ['--preview', previewPath] : []), + ...(jobs ? ['--jobs', jobsPath] : []), '--source-conclusion', identity.sourceConclusion, '--run-url', @@ -608,8 +696,7 @@ export async function reconcilePrComment({ identity, runUrl, message: fallbackMessage, - previewOrigin, - aliasAttempted, + preview, }); } } else { @@ -617,8 +704,7 @@ export async function reconcilePrComment({ identity, runUrl, message: fallbackMessage, - previewOrigin, - aliasAttempted, + preview, }); } diff --git a/.github/scripts/lib/pr-preview.test.mjs b/.github/scripts/lib/pr-preview.test.mjs index c015c2b8bdaf8..4b86724a1f9c9 100644 --- a/.github/scripts/lib/pr-preview.test.mjs +++ b/.github/scripts/lib/pr-preview.test.mjs @@ -202,6 +202,7 @@ async function reconcile({ execute, baseStats, packageSize, + touched, } = {}) { const value = previewAvailable ? {...requested, headRepository: requested.baseRepository} @@ -219,6 +220,11 @@ async function reconcile({ ); if (packageSize) fs.writeFileSync(packageSizePath, JSON.stringify(packageSize)); + const touchedPath = path.join( + path.dirname(paths.analysis), + 'touched-components.json', + ); + if (touched) fs.writeFileSync(touchedPath, JSON.stringify(touched)); const {github, state} = githubFixture({ value: githubFixtureValue, comments, @@ -236,6 +242,7 @@ async function reconcile({ metadataPath: paths.metadata, baseStatsPath, packageSizePath, + touchedPath, a11yPath: paths.a11y, visualPath: paths.visual, createIfMissing, @@ -815,30 +822,147 @@ describe('PR comment preview reconciliation', () => { ['alias known', true], ])('shows both or neither branch links: %s', async (_label, ready) => { const {result} = await reconcile({previewAvailable: ready}); - expect(result.body.includes('View Storybook for this PR')).toBe(ready); - expect(result.body.includes('View Sandbox for this PR')).toBe(ready); - expect(result.body.includes('> **Preview availability:**')).toBe(!ready); - if (ready) { - expect(result.body).toContain(`${VERCEL_ORIGIN}/storybook/`); - expect(result.body).toContain(`${VERCEL_ORIGIN}/sandbox/`); - } + expect( + result.body.includes(`[Storybook](${VERCEL_ORIGIN}/storybook/)`), + ).toBe(ready); + expect(result.body.includes(`[Sandbox](${VERCEL_ORIGIN}/sandbox/)`)).toBe( + ready, + ); + // A fork's head is looked up like any other: Vercel builds fork previews. + expect(result.body).not.toContain('No preview'); expect(result.body).not.toContain('facebook.github.io/astryx/pr/'); }); + describe("this head's preview and the run's jobs", () => { + const deployment = (overrides = {}) => ({ + id: 7, + environment: 'Preview', + creator: {login: 'vercel[bot]'}, + ...overrides, + }); + async function withGithub({deployments = [], statuses = [], jobs = []}) { + const value = identity({headRepository: 'meta/astryx'}); + const harness = githubFixture({value}); + harness.github.rest.repos = { + listDeployments: vi.fn(async () => ({data: deployments})), + listDeploymentStatuses: vi.fn(async () => ({data: statuses})), + }; + harness.github.rest.actions.listJobsForWorkflowRun = vi.fn(async () => ({ + data: jobs, + })); + const paths = fixture(value); + const seen = []; + const result = await reconcilePrComment({ + github: harness.github, + core: {info: vi.fn(), warning: vi.fn()}, + context: { + repo: {owner: 'meta', repo: 'astryx'}, + serverUrl: 'https://github.com', + }, + expectedIdentity: value, + analysisPath: paths.analysis, + metadataPath: paths.metadata, + a11yPath: paths.a11y, + visualPath: paths.visual, + execute: (node, args, options) => { + seen.push(args); + return execFileSync(node, args, options); + }, + }); + return {result, harness, args: seen[0]}; + } + + it("links Vercel's ready deployment of this exact head", async () => { + const {result, harness} = await withGithub({ + deployments: [deployment()], + statuses: [ + { + state: 'success', + environment_url: 'https://astryx-abc123-fbopensource.vercel.app', + }, + ], + }); + expect(harness.github.rest.repos.listDeployments).toHaveBeenCalledWith( + expect.objectContaining({sha: HEAD}), + ); + expect(result.body).toContain( + '[Storybook](https://astryx-abc123-fbopensource.vercel.app/storybook/)', + ); + expect(result.body).toContain('### 🔎 Preview · `aaaaaaa`'); + expect(result.body).toContain("built from this PR's head"); + }); + + it("ignores a deployment Vercel did not create, and an address that is not Vercel's", async () => { + for (const setup of [ + { + deployments: [deployment({creator: {login: 'someone'}})], + statuses: [ + { + state: 'success', + environment_url: 'https://astryx-abc123-fbopensource.vercel.app', + }, + ], + }, + { + deployments: [deployment()], + statuses: [ + {state: 'success', environment_url: 'https://evil.example/'}, + ], + }, + ]) { + const {result} = await withGithub(setup); + expect(result.body).not.toContain('astryx-abc123'); + expect(result.body).not.toContain('evil.example'); + expect(result.body).toContain('⏳ Building…'); + } + }); + + it('says a failed preview failed', async () => { + const {result} = await withGithub({ + deployments: [deployment()], + statuses: [{state: 'failure'}], + }); + expect(result.body).toContain('failed to build'); + }); + + it("passes the run's jobs, so the report says which suites did not run", async () => { + const {result, args} = await withGithub({ + jobs: [ + { + name: 'test-node (1/4)', + status: 'completed', + conclusion: 'success', + steps: [], + }, + { + name: 'pr-rtl (core)', + status: 'completed', + conclusion: 'skipped', + steps: [], + }, + ], + }); + expect(args).toContain('--jobs'); + expect(result.body).toContain( + 'Not run on this PR: add `ci:run-rtl` to run it.', + ); + }); + }); + describe('the fallback report, when there is no trusted analysis', () => { const failRender = () => { throw new Error('malformed analysis'); }; - it('links the branch alias under the same preview note as the report', async () => { + it('keeps the same preview line as the full report', async () => { const {result} = await reconcile({ previewAvailable: true, createIfMissing: true, execute: failRender, }); expect(result.body).toContain('trusted analysis is unavailable'); - expect(result.body).toContain('> **Preview:** the links below open'); - expect(result.body).toContain('View Storybook for this PR'); + expect(result.body).toContain('### 🔎 Preview · '); + expect(result.body).toContain(`[Storybook](${VERCEL_ORIGIN}/storybook/)`); }); it('points to Vercel when the alias cannot be computed', async () => { @@ -848,8 +972,8 @@ describe('PR comment preview reconciliation', () => { createIfMissing: true, execute: failRender, }); - expect(result.body).toContain('> **Preview availability:**'); - expect(result.body).not.toContain('View Storybook for this PR'); + expect(result.body).toContain('cannot be known in advance'); + expect(result.body).not.toContain('/storybook/'); }); it('says nothing about previews on a draft', async () => { @@ -874,7 +998,7 @@ describe('PR comment preview reconciliation', () => { it('says the accessibility audit did not run when there is no report', async () => { const {result} = await reconcile({a11y: false}); - expect(result.body).toContain('did not run on this pull request'); + expect(result.body).toMatch(/Not run on this PR: add [^\n]*`ci:run-a11y`/); expect(result.body).not.toContain('No accessibility violations detected'); }); @@ -882,7 +1006,7 @@ describe('PR comment preview reconciliation', () => { const value = identity({sourceConclusion: 'failure'}); const {result} = await reconcile({value}); - expect(result.body).toContain('Modified Components'); + expect(result.body).toContain('Modified components'); expect(result.body).toContain('Card'); expect(result.body).toContain('CI concluded failure'); expect(result.body).not.toContain('View Storybook for this PR'); @@ -1095,10 +1219,10 @@ describe('PR comment preview reconciliation', () => { expect(harness.state.updated).toHaveLength(1); expect(harness.state.comments).toHaveLength(1); expect(harness.state.comments[0].body).toContain( - 'View Storybook for this PR', + `[Storybook](${VERCEL_ORIGIN}/storybook/)`, ); expect(harness.state.comments[0].body).toContain( - 'View Sandbox for this PR', + `[Sandbox](${VERCEL_ORIGIN}/sandbox/)`, ); expect(harness.state.listedIssues).toEqual([5697, 5697]); }); diff --git a/.github/scripts/lib/pr-status.js b/.github/scripts/lib/pr-status.js new file mode 100644 index 0000000000000..0111a9d5dcb27 --- /dev/null +++ b/.github/scripts/lib/pr-status.js @@ -0,0 +1,462 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * @file pr-status.js + * @input The analysis (changed components, their import sizes and source + * stats at head and at the merge base), this head's Vercel preview state, + * the package-size report, the accessibility report, the visual verdict, + * and the source CI run's jobs + * @output The PR report's compact body: this head's preview first; one table + * of modified components (preview links, JS and CSS size and lines of code + * against main, and a fixed-position mark per browser suite when any ran), + * three rows visible and the rest collapsed; the changed shots, collapsed, + * when any changed; one line for package size; one line for suites that + * did not run + * @position Used by generate-pr-comment.js. Everything here reads data the CI + * run and the comment job already have; nothing starts a job. GitHub shows + * the CI checks and CodeBunny posts its own review, so neither is repeated. + */ + +const {inline, num, sizeText} = require('./report-text'); + +/** + * The heavy suites a pull request runs only with its `ci:run-` label, + * and the job display names (any parenthesized suffix removed) of each. + */ +const SUITES = [ + { + id: 'visual', + label: 'Visual', + short: 'Visual', + jobs: /^(Stable visual|visual-scout|pr-visual)/i, + }, + { + id: 'a11y', + label: 'Accessibility', + short: 'A11y', + jobs: /^(pr-a11y|Accessibility spec-test)/i, + }, + {id: 'rtl', label: 'RTL', short: 'RTL', jobs: /^pr-rtl/i}, + {id: 'perf', label: 'Perf', short: 'Perf', jobs: /^(perf|pr-perf)/i}, +]; + +/** Rows shown before the rest of a table is collapsed. */ +const VISIBLE_ROWS = 3; +/** Changed shots listed in the visual section, largest change first. */ +const VISUAL_FRAME_LIMIT = 12; + +/** Touch-only budget per component (architecture:component-style-authoring INV6). */ +const JS_BUDGET_BYTES = 2 * 1024; +const CSS_BUDGET_BYTES = 1024; + +/** + * A job's name without its parenthesized suffix: a matrix leg's `(1/3)`, a + * variant's `(browser specs)`, or a skipped matrix's unexpanded expression, + * which can hold parentheses of its own. + */ +function baseJobName(name) { + return String(name ?? '').replace(/\s*\(.*\)\s*$/, ''); +} + +/** + * Whether a suite ran on this run: `ran`, or `not-run` when every one of its + * jobs was skipped by its run condition (the label was absent). A suite with + * no jobs in the run is unknown (null). + */ +function suiteRunState(suite, jobs) { + const own = (jobs || []).filter(job => + suite.jobs.test(baseJobName(job.name)), + ); + if (own.length === 0) return null; + return own.every(job => job.conclusion === 'skipped') ? 'not-run' : 'ran'; +} + +function isMeasured(value) { + return typeof value === 'number' && Number.isFinite(value); +} + +function signedSize(value) { + return `${value > 0 ? '+' : '−'}${sizeText(Math.abs(value))}`; +} + +/** + * A change against main; growth in bold, so it stands out. Without a number, + * the component is new when main was measured, and unmeasured when it was + * not (no record of the merge base). + */ +function delta(value, format, baseMeasured = true) { + if (value === null) return baseMeasured ? 'new' : 'not measured'; + if (value === 0) return '0'; + const text = format(value); + return value > 0 ? `**${text}**` : text; +} + +function change(after, before) { + return isMeasured(after) && isMeasured(before) ? after - before : null; +} + +/** JS and CSS growth, and whether either passes the touch-only budget. */ +function sizeChange(head = {}, base = {}) { + const js = change(head.importGzipBytes, base.importGzipBytes); + const css = change(head.cssGzipBytes, base.cssGzipBytes); + return { + js, + css, + total: js === null && css === null ? null : (js ?? 0) + (css ?? 0), + overBudget: + (js !== null && js > JS_BUDGET_BYTES) || + (css !== null && css > CSS_BUDGET_BYTES), + }; +} + +/** + * JS and CSS import cost (gzip), each with its change against main, and a 🔴 + * when the growth passes the touch-only budget (advisory). + */ +function sizeCell(head = {}, base = {}, baseMeasured = true) { + const js = head.importGzipBytes; + const css = head.cssGzipBytes; + if (!isMeasured(js) && !isMeasured(css)) return '—'; + const growth = sizeChange(head, base); + const part = (after, changed, label) => + isMeasured(after) + ? `${sizeText(after)}${label} (${delta(changed, signedSize, baseMeasured)})` + : null; + return `${growth.overBudget ? '🔴 ' : ''}${[ + part(js, growth.js, ''), + part(css, growth.css, ' CSS'), + ] + .filter(Boolean) + .join(' · ')}`; +} + +/** Source lines of code with their change against main. */ +function locCell(head, base, baseMeasured = true) { + if (!isMeasured(head)) return '—'; + const count = value => Math.abs(value).toLocaleString('en-US'); + return `${count(num(head))} (${delta( + change(head, base), + value => `${value > 0 ? '+' : '−'}${count(value)}`, + baseMeasured, + )})`; +} + +/** + * One mark per browser suite, always in the order Visual · A11y · RTL: ✓ + * passed, ✗ failed or changed (its shots are in the visual section), — did + * not run for this component. + */ +const CHECK_SUITES = ['visual', 'a11y', 'rtl']; +const MARK = {pass: '✓', fail: '✗', changed: '✗'}; + +function checksCell(results) { + return CHECK_SUITES.map(id => MARK[results[id]] ?? '—').join(' · '); +} + +/** A Storybook docs link from a story title (`Core/Button` → `core-button`). */ +function storybookLink(origin, storyTitle) { + const title = Array.isArray(storyTitle) ? storyTitle[0] : storyTitle; + if (!origin || !title) return null; + const id = String(title) + .toLowerCase() + .replace(/\//g, '-') + .replace(/[^a-z0-9_-]/g, ''); + return `${origin}/storybook/?path=/docs/${id}--docs`; +} + +/** + * One row per modified component: every component this change sized (its + * folder changed or its bundle reaches a changed module) and every new or + * modified component folder. Results come from the accessibility report and + * the visual verdict. + */ +function componentRows({analysis, a11yReport, visualVerdict}) { + const head = analysis.componentSizes || {}; + const base = analysis.baseComponentSizes || {}; + const stats = analysis.componentStats || {}; + const baseStats = analysis.baseComponentStats || {}; + const keys = new Set(Object.keys(head)); + for (const name of [ + ...(analysis.newComponents || []), + ...(analysis.modifiedComponents || []), + ]) { + const pkg = String(stats[name]?.package || '@astryxdesign/core').replace( + /^@astryxdesign\//, + '', + ); + keys.add(`${pkg}/${name}`); + } + const visualChanges = visualVerdict?.changes || []; + // Main's sizes of the merge base were read (a base record or build). + const baseMeasured = + Boolean(analysis.baseSha) || + Object.keys(base).length > 0 || + Object.keys(baseStats).length > 0; + return [...keys].map(key => { + const name = key.slice(key.indexOf('/') + 1); + // A component the audit did not cover has no accessibility result. + const audited = a11yReport?.components?.[name]; + const a11y = audited + ? (audited.violations?.length ?? 0) > 0 + ? 'fail' + : 'pass' + : null; + const visual = + visualVerdict && visualVerdict.status !== 'skipped' + ? // A run that failed compared nothing: never a pass. + visualVerdict.status === 'failed' + ? 'fail' + : visualChanges.some(entry => entry.component === name) + ? 'changed' + : 'pass' + : null; + return { + key, + name, + storyTitle: stats[name]?.storyTitle, + head: head[key], + base: base[key], + growth: sizeChange(head[key], base[key]), + loc: [stats[name]?.linesOfCode, baseStats[name]?.linesOfCode], + results: {visual, a11y, rtl: null}, + baseMeasured, + }; + }); +} + +/** Failed or changed results first, then budget overruns, then the biggest growth. */ +function importance(row) { + const attention = Object.values(row.results).some( + value => value === 'fail' || value === 'changed', + ); + return [ + attention ? 0 : 1, + row.growth.overBudget ? 0 : 1, + -(row.growth.total ?? -Infinity), + ]; +} + +function compareRows(a, b) { + const [x, y] = [importance(a), importance(b)]; + for (let i = 0; i < x.length; i++) if (x[i] !== y[i]) return x[i] - y[i]; + return a.key.localeCompare(b.key); +} + +function buildComponentTable({analysis, origin, a11yReport, visualVerdict}) { + const rows = componentRows({analysis, a11yReport, visualVerdict}).sort( + compareRows, + ); + if (rows.length === 0) return ''; + // The checks column appears only when a browser suite has a result here. + const checks = rows.some(row => + CHECK_SUITES.some(id => row.results[id] != null), + ); + const columns = [ + 'Component', + 'Size', + 'LoC', + ...(checks ? ['Visual · A11y · RTL'] : []), + ]; + const header = `| ${columns.join(' | ')} |\n|${columns.map(() => '---').join('|')}|\n`; + // The name links its docs page in this head's preview; Storybook follows. + const line = row => { + // Core components go by name; another package's carry its prefix. + const label = inline(row.key.startsWith('core/') ? row.name : row.key); + const name = origin + ? `[${label}](${origin}/components/${encodeURIComponent(row.name)})` + : label; + const stories = storybookLink(origin, row.storyTitle); + const cells = [ + stories ? `${name} · [Storybook](${stories})` : name, + sizeCell(row.head, row.base, row.baseMeasured), + locCell(...row.loc, row.baseMeasured), + ...(checks ? [checksCell(row.results)] : []), + ]; + return `| ${cells.join(' | ')} |`; + }; + const visible = rows.slice(0, VISIBLE_ROWS); + const rest = rows.slice(VISIBLE_ROWS); + let out = `### Modified components\n\n${header}${visible.map(line).join('\n')}\n\n`; + if (rest.length > 0) + out += `
\n${rest.length} more component${rest.length === 1 ? '' : 's'}\n\n${header}${rest.map(line).join('\n')}\n\n
\n\n`; + return out; +} + +/** + * The changed shots, largest change first: up to VISUAL_FRAME_LIMIT rows + * naming each shot and its change, inside one disclosure whose summary + * carries the counts. The run's visual report holds the frames. + */ +function buildVisualDeltas({verdict}) { + if (!verdict || verdict.status === 'skipped' || verdict.status === 'failed') + return ''; + const all = [ + ...[...(verdict.changes || [])] + .sort((a, b) => Number(b.diffPixels ?? 0) - Number(a.diffPixels ?? 0)) + .map(change => ({...change, kinds: ['before', 'after', 'diff']})), + ...(verdict.added || []).map(key => ({key, name: key, kinds: ['after']})), + ...(verdict.removed || []).map(key => ({ + key, + name: key, + kinds: ['before'], + })), + ]; + if (all.length === 0) return ''; + const rows = all.slice(0, VISUAL_FRAME_LIMIT); + const unshown = all.length - rows.length; + const changeText = row => + row.kinds.length === 3 + ? `changed (${num(row.diffPixels ?? 0).toLocaleString('en-US')} px)` + : row.kinds[0] === 'after' + ? 'added' + : 'removed'; + const label = row => { + const story = [row.component || row.title, row.name] + .filter(Boolean) + .map(inline) + .join(' · '); + const variant = [row.theme, row.mode] + .filter(Boolean) + .map(inline) + .join(', '); + return variant ? `${story} (${variant})` : story; + }; + const header = '| Component · story | Change |\n|---|---|\n'; + const line = row => `| ${label(row)} | ${changeText(row)} |`; + // A shot key is `---__-`. + const componentOf = row => + String( + row.component ?? + String(row.key) + .split('--')[0] + .replace(/^[a-z]+-/, ''), + ) + .toLowerCase() + .replace(/[^a-z0-9]/g, ''); + const components = new Set(all.map(componentOf)); + return `
\nVisual changes: ${num(all.length)} snapshot${all.length === 1 ? '' : 's'} in ${components.size} component${components.size === 1 ? '' : 's'}\n\nA change is a question for the reviewer, not a failure.\n\n${header}${rows.map(line).join('\n')}\n\n${unshown > 0 ? `${num(unshown)} more are in the run's visual report.\n\n` : ''}
\n\n`; +} + +function previewLine(preview) { + if (!preview || preview.state === 'none') return ''; + const sha = `\`${inline(String(preview.sha ?? '').slice(0, 7))}\``; + const links = origin => + `**[Storybook](${origin}/storybook/) · [Sandbox](${origin}/sandbox/)**`; + let text; + if (preview.state === 'unknown') + text = + "This branch's preview address cannot be known in advance; open it from Vercel's comment."; + else if (preview.state === 'ready') + text = `${links(preview.url)} · built from this PR's head`; + else if (preview.state === 'failed') + text = "❌ The preview failed to build; see Vercel's comment."; + else + text = `⏳ Building…${preview.aliasOrigin ? ` Until it's ready, ${links(preview.aliasOrigin)} open the branch's previous preview.` : ''}`; + return `### 🔎 Preview · ${sha}\n\n${text}\n\n`; +} + +/** + * Package size: each published package whose download changed, one line each + * (the first three, then the rest collapsed), with any new runtime dependency. + */ +function packagesLine(report) { + if (!report || !Array.isArray(report.packages)) return ''; + if (report.verdict === 'no-base') + return '**Package size:** measured; main has no measurement of the merge base to compare yet.\n\n'; + const added = report.addedDependencies || []; + const changed = report.packages.filter( + entry => + (isMeasured(entry.base?.packedBytes) && + isMeasured(entry.head?.packedBytes) && + entry.base.packedBytes !== entry.head.packedBytes) || + added.some(dep => dep.package === entry.name), + ); + if (changed.length === 0) + return '**Package size:** no published package changed; no new dependencies.\n\n'; + const packed = sizeText; + const describe = entry => { + const value = change(entry.head?.packedBytes, entry.base?.packedBytes); + const deps = added.filter(dep => dep.package === entry.name); + const size = isMeasured(entry.head?.packedBytes) + ? `${packed(entry.head.packedBytes)} packed (${delta(value, signedSize)})${entry.overBudget ? ' 🔴' : ''}` + : 'new package'; + return `${inline(entry.name)} ${size}; ${ + deps.length > 0 + ? `new dependenc${deps.length === 1 ? 'y' : 'ies'}: ${deps.map(dep => inline(dep.name)).join(', ')}` + : 'no new dependencies' + }`; + }; + if (changed.length === 1) + return `**Package size:** ${describe(changed[0])}.\n\n`; + const lines = changed.map(entry => `- ${describe(entry)}`); + const rest = lines.slice(VISIBLE_ROWS); + return `**Package size:**\n\n${lines.slice(0, VISIBLE_ROWS).join('\n')}\n\n${ + rest.length > 0 + ? `
\n${rest.length} more package${rest.length === 1 ? '' : 's'}\n\n${rest.join('\n')}\n\n
\n\n` + : '' + }`; +} + +/** + * One line for the browser suites that did not run, with the labels that run + * them. A failed or cancelled Storybook build skips them for another reason + * (GitHub shows that check), so the line is left out. + */ +function notRunLine({jobs, a11yReport, visualVerdict}) { + const build = (jobs || []).find( + job => baseJobName(job.name) === 'build-storybook', + ); + if (build && ['failure', 'cancelled'].includes(build.conclusion)) return ''; + const notRun = SUITES.filter(suite => { + const state = suiteRunState(suite, jobs); + if (state !== null) return state === 'not-run'; + if (jobs) return false; + if (suite.id === 'visual') return !visualVerdict; + if (suite.id === 'a11y') return !a11yReport; + return false; + }); + if (notRun.length === 0) return ''; + const labels = notRun.map(suite => `\`ci:run-${suite.id}\``); + const list = + labels.length === 1 + ? labels[0] + : `${labels.slice(0, -1).join(', ')} or ${labels.at(-1)}`; + return `Not run on this PR: add ${list} to run ${labels.length === 1 ? 'it' : 'them'}.\n\n`; +} + +/** The compact report body, between the heading and the footer. */ +function buildCompactReport({ + analysis, + preview = null, + jobs = null, + a11yReport = null, + visualVerdict = null, + packageSize = null, +}) { + const origin = + preview?.state === 'ready' ? preview.url : preview?.aliasOrigin || ''; + return `${previewLine(preview)}${buildComponentTable({ + analysis, + origin, + a11yReport, + visualVerdict, + })}${buildVisualDeltas({verdict: visualVerdict})}${packagesLine(packageSize)}${notRunLine({jobs, a11yReport, visualVerdict})}`; +} + +module.exports = { + SUITES, + VISUAL_FRAME_LIMIT, + baseJobName, + suiteRunState, + sizeCell, + locCell, + componentRows, + buildComponentTable, + buildVisualDeltas, + checksCell, + previewLine, + packagesLine, + notRunLine, + buildCompactReport, + storybookLink, +}; diff --git a/.github/scripts/lib/pr-status.test.mjs b/.github/scripts/lib/pr-status.test.mjs new file mode 100644 index 0000000000000..60b13cf5c5f0d --- /dev/null +++ b/.github/scripts/lib/pr-status.test.mjs @@ -0,0 +1,396 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * @file pr-status.test.mjs + * Pins the compact PR report: the preview line's states; one table of + * modified components (the name linking its docs page, a Storybook link, JS + * and CSS size and lines of code against main with growth in bold, and the + * fixed-position Visual · A11y · RTL marks only when a suite ran), three rows + * visible and the rest collapsed, most important first; the visual deltas + * with frames for the published shots only; the package line; and the one + * not-run or wide-change line. + */ + +import {describe, expect, it} from 'vitest'; +import status from './pr-status.js'; + +const { + baseJobName, + suiteRunState, + sizeCell, + locCell, + checksCell, + buildComponentTable, + buildVisualDeltas, + previewLine, + packagesLine, + notRunLine, + SUITES, + VISUAL_FRAME_LIMIT, +} = status; + +const HEAD = 'a'.repeat(40); +const ORIGIN = 'https://astryx-abc123-fbopensource.vercel.app'; +const size = (importGzipBytes, cssGzipBytes = 1024) => ({ + importGzipBytes, + cssGzipBytes, +}); + +describe('previewLine', () => { + it.each([ + [ + 'ready', + {state: 'ready', sha: HEAD, url: ORIGIN}, + `### 🔎 Preview · \`aaaaaaa\`\n\n**[Storybook](${ORIGIN}/storybook/) · [Sandbox](${ORIGIN}/sandbox/)** · built from this PR's head`, + ], + [ + 'building', + {state: 'building', sha: HEAD, aliasOrigin: ORIGIN}, + `⏳ Building… Until it's ready, **[Storybook](${ORIGIN}/storybook/)`, + ], + ['failed', {state: 'failed', sha: HEAD}, 'failed to build'], + ['unknown', {state: 'unknown', sha: HEAD}, 'cannot be known in advance'], + ])('says what a %s preview is', (_label, preview, text) => { + expect(previewLine(preview)).toContain(text); + }); + + it('says nothing on a draft', () => { + expect(previewLine(null)).toBe(''); + }); +}); + +describe('sizeCell and locCell', () => { + it('shows JS and CSS with their change, growth in bold', () => { + expect(sizeCell(size(105_370, 25_600), size(105_062, 25_700))).toBe( + '102.9 kB (**+308 B**) · 25.0 kB CSS (−100 B)', + ); + expect(sizeCell(size(1024, 1024), size(1024, 1024))).toBe( + '1.0 kB (0) · 1.0 kB CSS (0)', + ); + }); + + it('marks growth past the touch-only budget, JavaScript or CSS', () => { + expect(sizeCell(size(5000), size(2000))).toMatch(/^🔴 /); + expect(sizeCell(size(1000, 3000), size(1000, 1000))).toMatch(/^🔴 /); + expect(sizeCell(size(4000), size(2000))).not.toMatch(/🔴/); + }); + + it('shows a new component as new, and an unmeasured one as a dash', () => { + expect(sizeCell(size(1024), {})).toBe('1.0 kB (new) · 1.0 kB CSS (new)'); + expect(sizeCell({}, size(1))).toBe('—'); + }); + + it('counts lines of code against main', () => { + expect(locCell(124, 120)).toBe('124 (**+4**)'); + expect(locCell(120, 124)).toBe('120 (−4)'); + expect(locCell(120, undefined)).toBe('120 (new)'); + expect(locCell(undefined, 1)).toBe('—'); + }); +}); + +describe('checksCell', () => { + it('keeps Visual · A11y · RTL in fixed positions', () => { + expect(checksCell({visual: 'pass', a11y: null, rtl: 'fail'})).toBe( + '✓ · — · ✗', + ); + expect(checksCell({visual: 'changed'})).toBe('✗ · — · —'); + }); +}); + +describe('buildComponentTable', () => { + const analysis = rows => ({ + componentSizes: Object.fromEntries( + rows.map(([key, after]) => [key, size(after)]), + ), + baseComponentSizes: Object.fromEntries( + rows.map(([key, , before]) => [key, size(before)]), + ), + componentStats: {B: {storyTitle: 'Core/B', linesOfCode: 50}}, + baseComponentStats: {B: {linesOfCode: 40}}, + }); + + it('shows three rows, the most important first, and collapses the rest', () => { + const table = buildComponentTable({ + analysis: analysis([ + ['core/A', 1000, 1000], + ['core/B', 2000, 1500], + ['core/C', 1200, 1100], + ['core/D', 900, 1000], + ]), + origin: ORIGIN, + a11yReport: {components: {D: {violations: [{id: 'x'}]}}}, + }); + const [visible, rest] = table.split('
'); + // D's failed accessibility result comes first, then the biggest growth. + expect(visible.match(/^\| \[[A-D]\]/gm)).toEqual([ + '| [D]', + '| [B]', + '| [C]', + ]); + expect(visible).toContain('### Modified components'); + expect(visible).toContain( + '| Component | Size | LoC | Visual · A11y · RTL |', + ); + expect(rest).toContain('1 more component'); + expect(rest).toContain('| [A]'); + expect(table).toContain( + `| [B](${ORIGIN}/components/B) · [Storybook](${ORIGIN}/storybook/?path=/docs/core-b--docs) | 2.0 kB (**+500 B**) · 1.0 kB CSS (0) | 50 (**+10**) | — · — · — |`, + ); + expect(table).toContain('| — · ✗ · — |'); + }); + + it('has no checks column when no suite ran, and no disclosure for three rows', () => { + const table = buildComponentTable({ + analysis: analysis([['core/A', 1000, 1000]]), + origin: '', + }); + expect(table).toContain('| Component | Size | LoC |\n'); + expect(table).toContain('| A |'); + expect(table).not.toContain('
'); + }); + + it('lists a modified component folder with no size', () => { + const table = buildComponentTable({ + analysis: { + baseSha: 'a'.repeat(40), + modifiedComponents: ['Card'], + componentStats: {Card: {package: '@astryxdesign/lab', linesOfCode: 9}}, + }, + origin: '', + }); + expect(table).toContain('| lab/Card | — | 9 (new) |'); + }); + + it('says not measured, never new, when main has no sizes for the merge base', () => { + const table = buildComponentTable({ + analysis: { + componentSizes: { + 'core/Card': {importGzipBytes: 2048, cssGzipBytes: null}, + }, + componentStats: {Card: {linesOfCode: 9}}, + }, + origin: '', + }); + expect(table).toContain( + '| Card | 2.0 kB (not measured) | 9 (not measured) |', + ); + }); + + it('never shows a pass for a visual run that failed', () => { + const table = buildComponentTable({ + analysis: { + baseSha: 'a'.repeat(40), + componentSizes: { + 'core/Card': {importGzipBytes: 2048, cssGzipBytes: null}, + }, + baseComponentSizes: { + 'core/Card': {importGzipBytes: 2048, cssGzipBytes: null}, + }, + }, + origin: '', + visualVerdict: {status: 'failed', changes: []}, + }); + expect(table).toContain('| ✗ · — · — |'); + }); + + it('renders nothing when no component changed', () => { + expect(buildComponentTable({analysis: {}, origin: ORIGIN})).toBe(''); + }); +}); + +describe('buildVisualDeltas', () => { + const change = (key, diffPixels, component = 'Card') => ({ + key, + component, + name: key, + theme: 'neutral', + mode: 'light', + diffPixels, + }); + + it('collapses the changed shots under their count, largest change first', () => { + const verdict = { + status: 'changed', + counts: {total: 20}, + changes: [change('a', 1), change('b', 1500), change('c', 9, 'Chip')], + added: ['core-dialog--default__neutral-light'], + removed: ['core-old--default__neutral-light'], + }; + const section = buildVisualDeltas({verdict}); + expect(section).toMatch( + /^
\nVisual changes: 5 snapshots in 4 components<\/summary>/, + ); + expect(section.match(/^\| \S+ · [a-c] /gm)).toEqual([ + '| Card · b ', + '| Chip · c ', + '| Card · a ', + ]); + expect(section).toContain( + '| Card · b (neutral, light) | changed (1,500 px) |', + ); + expect(section).toContain( + '| core-dialog--default\\_\\_neutral-light | added |', + ); + expect(section).toContain( + '| core-old--default\\_\\_neutral-light | removed |', + ); + }); + + it(`lists at most ${VISUAL_FRAME_LIMIT} shots and counts the rest`, () => { + const changes = Array.from({length: VISUAL_FRAME_LIMIT + 3}, (_, i) => + change(`s${i}`, 100 - i), + ); + const section = buildVisualDeltas({ + verdict: {status: 'changed', counts: {total: 99}, changes}, + }); + expect(section.match(/\| changed \(/g)).toHaveLength(VISUAL_FRAME_LIMIT); + expect(section).toContain("3 more are in the run's visual report."); + }); + + it('is absent without changes, or when the visual run failed', () => { + expect(buildVisualDeltas({verdict: {status: 'ok', changes: []}})).toBe(''); + expect(buildVisualDeltas({verdict: null})).toBe(''); + expect( + buildVisualDeltas({ + verdict: {status: 'failed', changes: [change('a', 1)]}, + }), + ).toBe(''); + }); +}); + +describe('packagesLine', () => { + const entry = (name, before, after, overBudget = false) => ({ + name, + base: {packedBytes: before}, + head: {packedBytes: after}, + overBudget, + }); + + it('puts one changed package on one line', () => { + expect( + packagesLine({ + verdict: 'pass', + packages: [ + entry('@astryxdesign/core', 3_570_000, 3_574_096), + entry('@astryxdesign/build', 5000, 5000), + ], + addedDependencies: [], + }), + ).toBe( + '**Package size:** @astryxdesign/core 3.41 MB packed (**+4.0 kB**); no new dependencies.\n\n', + ); + }); + + it('lists several changed packages, three then the rest collapsed, unchanged ones left out', () => { + const line = packagesLine({ + verdict: 'over-budget', + packages: [ + entry('@astryxdesign/core', 100000, 120000, true), + entry('@astryxdesign/richtext', 5000, 4000), + entry('@astryxdesign/cli', 9000, 9100), + entry('@astryxdesign/theme-neutral', 9000, 9000), + entry('@astryxdesign/build', 9000, 9200), + ], + addedDependencies: [ + { + package: '@astryxdesign/cli', + field: 'dependencies', + name: 'left-pad', + range: '1', + }, + ], + }); + expect(line).toContain( + '- @astryxdesign/core 117.2 kB packed (**+19.5 kB**) 🔴; no new dependencies', + ); + expect(line).toContain( + '- @astryxdesign/richtext 3.9 kB packed (−1000 B); no new dependencies', + ); + expect(line).toContain( + '- @astryxdesign/cli 8.9 kB packed (**+100 B**); new dependency: left-pad', + ); + expect(line).toContain('1 more package'); + expect(line).not.toContain('theme-neutral'); + }); + + it('says when nothing changed, and when main has no base yet', () => { + expect( + packagesLine({ + verdict: 'pass', + packages: [entry('@astryxdesign/core', 1, 1)], + }), + ).toBe( + '**Package size:** no published package changed; no new dependencies.\n\n', + ); + expect(packagesLine({verdict: 'no-base', packages: []})).toContain( + 'no measurement of the merge base', + ); + expect(packagesLine(null)).toBe(''); + }); +}); + +describe('notRunLine', () => { + const skipped = name => ({name, conclusion: 'skipped', steps: []}); + + it('names the label for one suite', () => { + expect( + notRunLine({ + jobs: [ + skipped('pr-rtl'), + {name: 'Stable visual regression', conclusion: 'success'}, + {name: 'pr-a11y', conclusion: 'success'}, + ], + }), + ).toBe('Not run on this PR: add `ci:run-rtl` to run it.\n\n'); + }); + + it('names several suites in one line', () => { + expect( + notRunLine({ + jobs: [ + skipped( + 'Stable visual scout (${{ matrix.shard }}/${{ strategy.job-total }})', + ), + skipped('Accessibility spec-test contracts'), + skipped('pr-rtl (core)'), + ], + }), + ).toBe( + 'Not run on this PR: add `ci:run-visual`, `ci:run-a11y` or `ci:run-rtl` to run them.\n\n', + ); + }); + + it('leaves the line out when the Storybook build failed', () => { + expect( + notRunLine({ + jobs: [ + {name: 'build-storybook', conclusion: 'failure'}, + {name: 'pr-visual', conclusion: 'skipped'}, + {name: 'pr-a11y (full audit)', conclusion: 'skipped'}, + ], + }), + ).toBe(''); + }); + + it('says nothing when every suite ran, and falls back to the reports without a job list', () => { + expect(notRunLine({jobs: []})).toBe(''); + expect(notRunLine({jobs: null, a11yReport: {components: {}}})).toBe( + 'Not run on this PR: add `ci:run-visual` to run it.\n\n', + ); + }); +}); + +describe('job names', () => { + it("folds a skipped matrix's unexpanded name, parentheses and all", () => { + expect( + baseJobName( + "pr-rtl (${{ matrix.package }}${{ (((matrix.parts > 1) && format(' {0}/{1}', matrix.part, matrix.parts)) || '') }})", + ), + ).toBe('pr-rtl'); + const rtl = SUITES.find(suite => suite.id === 'rtl'); + expect( + suiteRunState(rtl, [{name: 'pr-rtl (core)', conclusion: 'success'}]), + ).toBe('ran'); + expect(suiteRunState(rtl, [])).toBeNull(); + }); +}); diff --git a/.github/scripts/lib/report-text.js b/.github/scripts/lib/report-text.js index 111d929532694..9623fe830b4e8 100644 --- a/.github/scripts/lib/report-text.js +++ b/.github/scripts/lib/report-text.js @@ -45,9 +45,9 @@ function safeUrl(value) { */ function sizeText(bytes) { if (typeof bytes !== 'number' || !Number.isFinite(bytes)) return 'N/A'; - if (Math.abs(bytes) < 1024) return `${bytes}B`; - if (Math.abs(bytes) < 1024 * 1024) return `${(bytes / 1024).toFixed(1)}KB`; - return `${(bytes / (1024 * 1024)).toFixed(2)}MB`; + if (Math.abs(bytes) < 1024) return `${bytes} B`; + if (Math.abs(bytes) < 1024 * 1024) return `${(bytes / 1024).toFixed(1)} kB`; + return `${(bytes / (1024 * 1024)).toFixed(2)} MB`; } module.exports = {inline, num, safeUrl, sizeText}; diff --git a/.github/scripts/visual-gate/workflow-concurrency.test.mjs b/.github/scripts/visual-gate/workflow-concurrency.test.mjs index 8cb4c7978d294..35ca0688e2d95 100644 --- a/.github/scripts/visual-gate/workflow-concurrency.test.mjs +++ b/.github/scripts/visual-gate/workflow-concurrency.test.mjs @@ -64,7 +64,11 @@ describe('PR report and deployment workflow contracts', () => { expect(value.jobs[job].concurrency['cancel-in-progress']).toBe(false); } expect(value.jobs.comment.needs).toBe('resolve'); - expect(value.jobs.comment.permissions.deployments).toBeUndefined(); + // The comment job reads this head's preview state once; no job runs on a + // deployment event, and none waits for one. + expect(value.jobs.comment.permissions.deployments).toBe('read'); + // It writes no repository contents. + expect(value.jobs.comment.permissions.contents).toBeUndefined(); expect(workflow('pr-comment.yml')).toContain( 'validateAnalysisMetadata(metadata, {', ); @@ -137,8 +141,12 @@ describe('PR report and deployment workflow contracts', () => { 'utf8', ); expect(reconciler).toContain('branchPreviewOrigin(identity)'); - expect(reconciler).toContain('`${previewOrigin}/storybook/`'); - expect(reconciler).toContain('`${previewOrigin}/sandbox/`'); + const status = fs.readFileSync( + path.join(ROOT, '.github/scripts/lib/pr-status.js'), + 'utf8', + ); + expect(status).toContain('[Storybook](${origin}/storybook/)'); + expect(status).toContain('[Sandbox](${origin}/sandbox/)'); }); it('publishes only the static compatibility landing to GitHub Pages', () => { diff --git a/.github/workflows/pr-comment.yml b/.github/workflows/pr-comment.yml index 75c491e4ece3a..d16bcf1534527 100644 --- a/.github/workflows/pr-comment.yml +++ b/.github/workflows/pr-comment.yml @@ -199,7 +199,10 @@ jobs: runs-on: ubuntu-24.04-arm permissions: pull-requests: write + # actions: the artifacts and the source run's job list; deployments: + # this head's Vercel preview state. actions: read + deployments: read # The artifact downloads are optional (a run may not produce them), so # they are not failures of this job. outputs: