Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions .github/scripts/generate-pr-comment.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@

/**
* @description Generates and posts PR comment with analysis results
* @input --analysis <file> --a11y <file> --storybook-url <url> --run-url <url>
* @input --analysis <file> --a11y <file> --visual <file> --storybook-url <url> --run-url <url>
* @output Formatted markdown comment body to stdout
*/

const fs = require('node:fs');
const { buildA11ySection } = require('./lib/a11y-format');
const { buildVisualSection } = require('./lib/visual-format');

const args = process.argv.slice(2);
const getArg = (name) => {
Expand All @@ -19,6 +20,7 @@ const getArg = (name) => {

const analysisFile = getArg('analysis') || 'analysis.json';
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') || '';
Expand Down Expand Up @@ -115,6 +117,18 @@ if (analysis.modifiedComponents && analysis.modifiedComponents.length > 0) {
// Build accessibility section using shared module
const a11ySection = buildA11ySection(a11yReport);

// 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'));

// 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
Expand Down Expand Up @@ -187,7 +201,7 @@ const body = `## PR Analysis Report
${diffModeCaveat}${storybookSection}${sandboxSection}${componentSection || '_No new or modified components detected._\n\n'}
${bundleSection}
${a11ySection}
---
${visualSection}---

<sub>Generated by PR Enrichment workflow${footerLinksStr}</sub>
`;
Expand Down
11 changes: 8 additions & 3 deletions .github/scripts/lib/visual-format.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,15 @@ function buildVisualSection(verdict, reportUrl) {
return `### Visual Regression\n\n**Status:** ${verdict.counts.failed} shot(s) could not be captured.${link}\n\n`;
}
if (!verdict.changes || verdict.changes.length === 0) {
const added = verdict.counts?.added
? ` ${verdict.counts.added} new shot(s) had no baseline to compare against.`
const compared = verdict.counts.total - (verdict.counts.added ?? 0);
// A PR-scoped run shoots every story of the touched component in every
// theme that styles it, which is deeper than the daily gate's baseline
// reaches — so some shots legitimately have nothing to compare against.
// Saying "added" there reads as a problem; saying it plainly does not.
const unbaselined = verdict.counts?.added
? ` ${verdict.counts.added} shot(s) have no baseline yet and were not compared.`
: '';
return `### Visual Regression\n\n**Status:** No visual change across ${verdict.counts.total} shot(s).${added}\n\n`;
return `### Visual Regression\n\n**Status:** No visual change across ${compared} compared shot(s).${unbaselined}\n\n`;
}

const rows = verdict.changes
Expand Down
99 changes: 99 additions & 0 deletions .github/scripts/lib/visual-format.test.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
// Copyright (c) Meta Platforms, Inc. and affiliates.

import {describe, expect, it} from 'vitest';
import {createRequire} from 'node:module';

const {buildVisualSection} = createRequire(import.meta.url)('./visual-format.js');

const verdict = (overrides = {}) => ({
status: 'pass',
counts: {total: 16, unchanged: 16, changed: 0, added: 0, removed: 0, failed: 0},
changes: [],
...overrides,
});

describe('buildVisualSection', () => {
it('renders nothing when the job did not run', () => {
expect(buildVisualSection(null)).toBe('');
});

it('says so plainly when nothing moved', () => {
expect(buildVisualSection(verdict())).toContain('No visual change across 16 compared shot(s)');
});

it('calls an unbaselined shot exactly that, not an addition', () => {
const section = buildVisualSection(
verdict({counts: {total: 20, unchanged: 14, changed: 0, added: 6, removed: 0, failed: 0}}),
);
expect(section).toContain('No visual change across 14 compared shot(s)');
expect(section).toContain('6 shot(s) have no baseline yet');
});

it('states the reason for a skip, so a broad PR does not look like a pass', () => {
const section = buildVisualSection(
verdict({status: 'skipped', reason: '900 shots exceeds the 240-shot budget'}),
);
expect(section).toContain('Skipped');
expect(section).toContain('900 shots exceeds the 240-shot budget');
});

it('lists each changed shot with the theme and mode it changed in', () => {
const section = buildVisualSection(
verdict({
status: 'changed',
counts: {total: 16, unchanged: 14, changed: 2, added: 0, removed: 0, failed: 0},
changes: [
{key: 'a', component: 'Button', name: 'Primary', theme: 'y2k', mode: 'light', diffPixels: 1126},
{key: 'b', component: 'Button', name: 'Primary', theme: 'y2k', mode: 'dark', diffPixels: 1401},
],
}),
);
expect(section).toContain('2 of 16 shot(s) changed');
expect(section).toContain('| Button | Primary | y2k | light | 1,126 |');
});

it('frames a change as a question rather than a failure', () => {
const section = buildVisualSection(
verdict({
status: 'changed',
counts: {total: 2, unchanged: 1, changed: 1, added: 0, removed: 0, failed: 0},
changes: [{key: 'a', component: 'B', name: 'S', theme: 't', mode: 'light', diffPixels: 5}],
}),
);
expect(section).toMatch(/question, not a failure/);
});

it('caps the table and says how many were left out', () => {
const changes = Array.from({length: 25}, (_, index) => ({
key: `k${index}`,
component: 'C',
name: 'S',
theme: 't',
mode: 'light',
diffPixels: index,
}));
const section = buildVisualSection(
verdict({status: 'changed', counts: {total: 25, changed: 25}, changes}),
);
expect(section).toContain('and 5 more');
});

it('reports a capture failure distinctly from a change', () => {
const section = buildVisualSection(
verdict({status: 'failed', counts: {total: 4, failed: 2, changed: 0}}),
);
expect(section).toContain('2 shot(s) could not be captured');
});

it('links the report when one was published', () => {
const section = buildVisualSection(
verdict({
status: 'changed',
counts: {total: 1, changed: 1},
changes: [{key: 'a', component: 'B', name: 'S', theme: 't', mode: 'light', diffPixels: 1}],
}),
'https://example.com/report/',
);
expect(section).toContain('https://example.com/report/');
});
});
33 changes: 33 additions & 0 deletions .github/scripts/visual-gate/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,39 @@ runner would read as "everything changed". The gate refuses that comparison
instead of showing you 500 false diffs, and the shared baseline is only ever
written by CI, from the pinned runner label.

## Two different questions

The gate asks two things, and only one of them is a screenshot.

**Did anything move?** — the shot tiers, compared against an accepted baseline.
Catches any visual regression, in any theme.

**Did each theming target's override actually reach the pixels?** — `gate.mjs
reach`, and no baseline is involved. A pixel diff cannot answer this: when an
override stops applying, the frame is captured broken and promoted as correct,
and every later run agrees with it forever. The probe theme gives every
selector a unique deterministic colour, so this is an equality test — compute
the colour that selector should have produced, read the element, compare. It
names the target instead of a rectangle, and it cannot flake.

Three outcomes, because the difference matters:

| | meaning |
| -------- | -------------------------------------------------------------------------------- |
| reached | the override painted |
| shadowed | another target on the _same element_ won — a fact about the markup, not a defect |
| missed | nothing probe-coloured won |

```bash
node .github/scripts/visual-gate/gate.mjs reach
```

It runs in the daily gate and is **reported, not enforced**. Today 50 targets
miss, from one systemic cause: StyleX emits into `@layer priority1-4`, which
sort _after_ `astryx-theme`, so wherever a component sets a property the theme
override loses. Failing the gate on a known systemic issue would only teach
everyone to ignore it — enforce it once that is fixed and the count is zero.

## How the release cut uses it

The daily cut (08:00 PT) reads the gate's verdict before it merges the version
Expand Down
Loading
Loading