From a00d15dcf918f227e03d9a8f910bef6133fe0bc6 Mon Sep 17 00:00:00 2001 From: Rob Hogan <2590098+robhogan@users.noreply.github.com> Date: Fri, 23 Oct 2026 12:30:46 +0100 Subject: [PATCH 1/3] Retake visual screenshots that differ from the baseline or are new System UI can briefly cover the app during capture, for example the Apple Intelligence notification iOS shows shortly after boot. A covered screenshot published as the baseline from `main` fails every PR based on that commit, and three baselines on `main` were spoiled this way or by flows racing the UI. After capture, `capture.js` now finds the nearest baseline over the public bucket URL and reruns, once and after a short delay, only the flows whose screenshots differ from it or have no baseline: - A changed screenshot is replaced by its retake only if the retake matches the baseline, so a real change still fails. - A new screenshot is unstable unless both captures are identical. The manifest marks it, the report says so, and compare doesn't publish it as the baseline. Screenshots that already matched are never retaken. The step is skipped without a baseline or `VISUAL_PUBLIC_BASE_URL`. Changelog: [Internal] --- .../__tests__/visual-compare-test.js | 55 ++++- .../__tests__/visual-report-test.js | 12 +- .../__tests__/visual-retake-test.js | 138 +++++++++++++ .github/workflow-scripts/visual/capture.js | 57 +++++- .../workflow-scripts/visual/compare-run.js | 14 +- .github/workflow-scripts/visual/manifest.js | 25 ++- .github/workflow-scripts/visual/report.js | 5 +- .github/workflow-scripts/visual/retake.js | 188 ++++++++++++++++++ .github/workflow-scripts/visual/storage.js | 4 + .github/workflow-scripts/visual/validate.js | 3 + .github/workflows/visual-capture-rntester.yml | 15 +- packages/rn-tester/visual-tests/README.md | 2 +- 12 files changed, 499 insertions(+), 19 deletions(-) create mode 100644 .github/workflow-scripts/__tests__/visual-retake-test.js create mode 100644 .github/workflow-scripts/visual/retake.js diff --git a/.github/workflow-scripts/__tests__/visual-compare-test.js b/.github/workflow-scripts/__tests__/visual-compare-test.js index 9b9b92754370..543617239ce4 100644 --- a/.github/workflow-scripts/__tests__/visual-compare-test.js +++ b/.github/workflow-scripts/__tests__/visual-compare-test.js @@ -9,7 +9,7 @@ */ const {compareCapture, compareImages, policyFor} = require('../visual/compare'); -const {resolveContext} = require('../visual/compare-run'); +const {resolveContext, stageBaseline} = require('../visual/compare-run'); const {buildManifest, sha256} = require('../visual/manifest'); const {readPngDimensions} = require('../visual/png'); const {validateCapture} = require('../visual/validate'); @@ -117,6 +117,26 @@ describe('visual comparison', () => { expect(() => validateCapture(directory)).toThrow('path must be'); }); + test.each([ + [true, null], + [false, 'unstable must be true or absent'], + ['yes', 'unstable must be true or absent'], + ])('validates unstable: %p', (unstable, error) => { + const manifest = writeCapture(directory, { + 'text/width-mode/default': makePng(4, 4), + }); + manifest.images[0].unstable = unstable; + fs.writeFileSync( + path.join(directory, 'manifest.json'), + JSON.stringify(manifest), + ); + if (error == null) { + expect(validateCapture(directory)).toEqual(manifest); + } else { + expect(() => validateCapture(directory)).toThrow(error); + } + }); + test('rejects duplicates, mismatched hashes and unlisted files', () => { const manifest = writeCapture(directory, { 'text/width-mode/default': makePng(4, 4), @@ -261,6 +281,39 @@ describe('visual comparison', () => { }); }); + describe('stageBaseline', () => { + test('leaves out unstable screenshots', () => { + const captureDir = path.join(directory, 'capture'); + const outDir = path.join(directory, 'out'); + const manifest = writeCapture(captureDir, { + 'text/stable/default': makePng(4, 4), + 'text/unstable/default': makePng(4, 4), + }); + manifest.images[1].unstable = true; + stageBaseline({outDir, captureDir, manifest, sha: SHA_A}); + const prefix = `baselines/${manifest.environmentKey}/${SHA_A}`; + const staged = JSON.parse( + fs.readFileSync( + path.join(outDir, 'baseline-manifests', prefix, 'manifest.json'), + 'utf8', + ), + ); + expect(staged.images.map(image => image.id)).toEqual([ + 'text/stable/default', + ]); + expect( + fs.existsSync( + path.join( + outDir, + 'baseline-images', + prefix, + 'text/unstable/default.png', + ), + ), + ).toBe(false); + }); + }); + describe('resolveContext', () => { const ENV = { CAPTURE_HEAD_SHA: SHA_A, diff --git a/.github/workflow-scripts/__tests__/visual-report-test.js b/.github/workflow-scripts/__tests__/visual-report-test.js index d5fa54ec6cdf..ac6667324fbd 100644 --- a/.github/workflow-scripts/__tests__/visual-report-test.js +++ b/.github/workflow-scripts/__tests__/visual-report-test.js @@ -150,14 +150,17 @@ describe('visual reports and approval', () => { {id: 'a/b/same', status: 'unchanged'}, changed('a/b/changed'), ]), - platform('ios', [{id: 'a/b/new', status: 'added', captured: {}}]), + platform('ios', [ + {id: 'a/b/new', status: 'added', captured: {}}, + {id: 'a/b/flaky', status: 'added', captured: {unstable: true}}, + ]), ], headSha: SHA_A, gateSummary: 'Needs approval.', }); expect(body.startsWith(COMMENT_MARKER)).toBe(true); expect(body).toContain( - '**2 visual difference(s)** at `aaaaaaaaaaaa`. Needs approval.', + '**3 visual difference(s)** at `aaaaaaaaaaaa`. Needs approval.', ); expect(body).toContain( '### android `a/b/changed`: Changed, 3 px (3.000%)', @@ -168,7 +171,10 @@ describe('visual reports and approval', () => { expect(body).toContain( '
Expected and Captured', ); - expect(body).toContain('### ios `a/b/new`: New'); + expect(body).toContain('### ios `a/b/new`: New\n'); + expect(body).toContain( + '### ios `a/b/flaky`: New, unstable: its two captures differed', + ); expect(body).toContain('
Captured'); expect(body).not.toContain('a/b/same'); }); diff --git a/.github/workflow-scripts/__tests__/visual-retake-test.js b/.github/workflow-scripts/__tests__/visual-retake-test.js new file mode 100644 index 000000000000..27afaadee655 --- /dev/null +++ b/.github/workflow-scripts/__tests__/visual-retake-test.js @@ -0,0 +1,138 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @noflow + * @format + */ + +const {buildManifest} = require('../visual/manifest'); +const { + flowsTaking, + judgeRetakes, + retakeCandidates, + screenshotIds, +} = require('../visual/retake'); +const {ANDROID_ENVIRONMENT} = require('./__fixtures__/visual'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); + +const baselineManifest = { + images: [ + {id: 'switch/basic/default', sha256: 'a'}, + {id: 'switch/basic/toggled', sha256: 'b'}, + ], +}; + +const images = [ + {id: 'switch/basic/default', sha256: 'a'}, + {id: 'switch/basic/toggled', sha256: 'x'}, + {id: 'view/basic/default', sha256: 'n'}, +]; + +describe('visual retake', () => { + test('reads the test IDs a flow captures', () => { + const flow = [ + 'appId: ${APP_ID}', + '---', + '- takeScreenshot:', + ' path: ${VISUAL_OUTPUT_DIR}/switch/basic/default', + ' cropOn:', + " id: 'switch-basic'", + '- takeScreenshot:', + " path: '${VISUAL_OUTPUT_DIR}/switch/basic/toggled'", + ].join('\n'); + expect(screenshotIds(flow)).toEqual([ + 'switch/basic/default', + 'switch/basic/toggled', + ]); + }); + + test('selects only the flows that take the given IDs', () => { + const files = { + '/flows/switch.yml': + '- takeScreenshot: ${VISUAL_OUTPUT_DIR}/switch/basic/toggled', + '/flows/text.yml': + '- takeScreenshot: ${VISUAL_OUTPUT_DIR}/text/width-mode/default', + '/flows/view.yml': + '- takeScreenshot: ${VISUAL_OUTPUT_DIR}/view/basic/default', + }; + expect( + flowsTaking( + Object.keys(files), + ['text/width-mode/default', 'switch/basic/toggled'], + file => files[file], + ), + ).toEqual(['/flows/switch.yml', '/flows/text.yml']); + }); + + test('retakes changed and new screenshots but not unchanged ones', () => { + expect(retakeCandidates(images, baselineManifest)).toEqual({ + changed: ['switch/basic/toggled'], + added: ['view/basic/default'], + }); + }); + + test.each([ + ['matches the baseline', 'b', ['switch/basic/toggled']], + ['still differs', 'y', []], + ['is missing', null, []], + ])('replaces a changed screenshot if its retake %s', (_, sha, replaced) => { + expect( + judgeRetakes({ + changed: ['switch/basic/toggled'], + added: [], + images, + baselineManifest, + retakeSha: () => sha, + }).replaced, + ).toEqual(replaced); + }); + + test.each([ + ['is identical', 'n', []], + ['differs', 'y', ['view/basic/default']], + ['is missing', null, ['view/basic/default']], + ])('marks a new screenshot unstable if its retake %s', (_, sha, unstable) => { + expect( + judgeRetakes({ + changed: [], + added: ['view/basic/default'], + images, + baselineManifest, + retakeSha: () => sha, + }).unstable, + ).toEqual(unstable); + }); + + test('marks unstable screenshots in the manifest', () => { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), 'visual-test-')); + try { + const png = fs.readFileSync( + path.join(__dirname, '../../../packages/rn-tester/js/assets/bunny.png'), + ); + for (const id of ['view/basic/default', 'view/basic/flaky']) { + fs.mkdirSync(path.join(directory, path.dirname(id)), {recursive: true}); + fs.writeFileSync(path.join(directory, `${id}.png`), png); + } + const manifest = buildManifest({ + captureDir: directory, + platform: 'android', + environment: ANDROID_ENVIRONMENT, + git: null, + unstableIds: ['view/basic/flaky'], + }); + expect(manifest.images.map(({id, unstable}) => ({id, unstable}))).toEqual( + [ + {id: 'view/basic/default', unstable: undefined}, + {id: 'view/basic/flaky', unstable: true}, + ], + ); + } finally { + fs.rmSync(directory, {recursive: true, force: true}); + } + }); +}); diff --git a/.github/workflow-scripts/visual/capture.js b/.github/workflow-scripts/visual/capture.js index f85ff8dc3c1c..f564130f514f 100644 --- a/.github/workflow-scripts/visual/capture.js +++ b/.github/workflow-scripts/visual/capture.js @@ -12,9 +12,12 @@ * Captures every visual flow in one Maestro invocation, so the JVM and * driver startup (around two minutes on an iOS simulator) is paid once per * job rather than once per flow. Flows that fail are rerun, and only those, - * on later attempts. + * on later attempts. Screenshots that then differ from the baseline, or + * have none, are retaken once (see retake.js). */ +const {readAndroidEnvironment, readIosEnvironment} = require('./manifest'); +const {retakeScreenshots} = require('./retake'); const childProcess = require('node:child_process'); const fs = require('node:fs'); const path = require('node:path'); @@ -28,7 +31,10 @@ node capture.js [ios_udi Installs the app, then runs every flow in in one Maestro invocation, writing screenshots under . Failed flows are retried -up to ${DEFAULT_ATTEMPTS} attempts in total (VISUAL_CAPTURE_ATTEMPTS). +up to ${DEFAULT_ATTEMPTS} attempts in total (VISUAL_CAPTURE_ATTEMPTS). With +VISUAL_PUBLIC_BASE_URL set, screenshots that differ from the baseline or have +none are retaken once, and the IDs of new screenshots whose two captures +differ are written as a JSON array to VISUAL_UNSTABLE_PATH. `; /** The flows directly inside `flowsDir`, sorted. Helpers live elsewhere. */ @@ -140,7 +146,7 @@ function writeSummary(lines) { } } -function main(args = process.argv.slice(2)) { +async function main(args = process.argv.slice(2)) { const [platform, appPath, appId, flowsDir, outputDir, udid] = args; if ( !['android', 'ios'].includes(platform) || @@ -193,6 +199,46 @@ function main(args = process.argv.slice(2)) { summary.push('', `Failed after ${attempts} attempt(s):`); summary.push(...pending.map(flow => `- \`${flow}\``)); } + + const publicBaseUrl = process.env.VISUAL_PUBLIC_BASE_URL; + if (publicBaseUrl != null && publicBaseUrl !== '') { + try { + const {unstable, lines} = await retakeScreenshots({ + platform, + environment: + platform === 'android' + ? readAndroidEnvironment() + : readIosEnvironment(udid), + flows: allFlows, + outputDir, + publicBaseUrl, + runFlows: (flows, retakeDir) => + runAttempt( + maestroArgs({ + flows, + appId, + outputDir: retakeDir, + reportPath: path.resolve('visual-capture-report-retake.xml'), + udid, + attempt: attempts + 1, + }), + ), + }); + if (lines.length > 0) { + summary.push('', ...lines); + } + if (process.env.VISUAL_UNSTABLE_PATH) { + fs.writeFileSync( + process.env.VISUAL_UNSTABLE_PATH, + `${JSON.stringify(unstable)}\n`, + ); + } + } catch (error) { + // The original screenshots are still valid, so compare goes ahead + // with them. + console.warn('Retaking screenshots failed:', error); + } + } writeSummary(summary); if (pending.length > 0) { process.exitCode = 1; @@ -200,7 +246,10 @@ function main(args = process.argv.slice(2)) { } if (require.main === module) { - main(); + main().catch(error => { + console.error(error); + process.exitCode = 1; + }); } module.exports = {collectFlows, failedFlowsFromJUnit, maestroArgs}; diff --git a/.github/workflow-scripts/visual/compare-run.js b/.github/workflow-scripts/visual/compare-run.js index 7b1f7784eaa1..6e132ed8d681 100644 --- a/.github/workflow-scripts/visual/compare-run.js +++ b/.github/workflow-scripts/visual/compare-run.js @@ -33,6 +33,7 @@ const { renderPullRequestComment, } = require('./report'); const { + BASELINE_SEARCH_DEPTH, approvalKey, baselinePrefix, fetchBuffer, @@ -45,7 +46,6 @@ const {validateCapture} = require('./validate'); const fs = require('node:fs'); const path = require('node:path'); -const BASELINE_SEARCH_DEPTH = 20; const PLATFORMS = ['android', 'ios']; // The default Actions token posts as this bot. Only its comments are edited. const COMMENT_AUTHOR = 'github-actions[bot]'; @@ -221,9 +221,15 @@ function missingCapture(platform, context) { }; } +/** + * Stages the capture as the baseline for `sha`, leaving out new screenshots + * whose two captures differed. They stay new until a later push to the + * default branch captures them the same way twice. + */ function stageBaseline({outDir, captureDir, manifest, sha}) { const prefix = baselinePrefix(manifest.environmentKey, sha); - for (const image of manifest.images) { + const images = manifest.images.filter(image => image.unstable !== true); + for (const image of images) { writeFile( path.join(outDir, 'baseline-images', prefix, `${image.id}.png`), fs.readFileSync(path.join(captureDir, `${image.id}.png`)), @@ -232,7 +238,7 @@ function stageBaseline({outDir, captureDir, manifest, sha}) { // Uploaded after the images: a baseline exists once its manifest does. writeFile( path.join(outDir, 'baseline-manifests', prefix, 'manifest.json'), - `${JSON.stringify(manifest, null, 2)}\n`, + `${JSON.stringify({...manifest, images}, null, 2)}\n`, ); } @@ -466,4 +472,4 @@ if (require.main === module) { }); } -module.exports = {resolveContext}; +module.exports = {resolveContext, stageBaseline}; diff --git a/.github/workflow-scripts/visual/manifest.js b/.github/workflow-scripts/visual/manifest.js index 404caea5ebda..f5ffcf821446 100644 --- a/.github/workflow-scripts/visual/manifest.js +++ b/.github/workflow-scripts/visual/manifest.js @@ -111,7 +111,18 @@ function collectImages(captureDir) { return images.sort(); } -function buildManifest({captureDir, platform, environment, git}) { +/** + * `unstableIds` are new screenshots whose two captures differed, which + * compare doesn't publish as the baseline. + */ +function buildManifest({ + captureDir, + platform, + environment, + git, + unstableIds = [], +}) { + const unstable = new Set(unstableIds); const images = collectImages(captureDir).map(imagePath => { const relativePath = path .relative(captureDir, imagePath) @@ -130,6 +141,11 @@ function buildManifest({captureDir, platform, environment, git}) { sha256: sha256(buffer), }; }); + for (const image of images) { + if (unstable.has(image.id)) { + image.unstable = true; + } + } return { version: MANIFEST_VERSION, @@ -259,6 +275,11 @@ function main(args = process.argv.slice(2)) { platform, environment, git: readGitInfo(), + unstableIds: + process.env.VISUAL_UNSTABLE_PATH && + fs.existsSync(process.env.VISUAL_UNSTABLE_PATH) + ? JSON.parse(fs.readFileSync(process.env.VISUAL_UNSTABLE_PATH, 'utf8')) + : [], }); fs.writeFileSync(outputPath, `${JSON.stringify(manifest, null, 2)}\n`); console.info( @@ -279,5 +300,7 @@ module.exports = { buildManifest, canonicalJson, environmentKey, + readAndroidEnvironment, + readIosEnvironment, sha256, }; diff --git a/.github/workflow-scripts/visual/report.js b/.github/workflow-scripts/visual/report.js index 9e9d6a51c6af..6b1abbd3842d 100644 --- a/.github/workflow-scripts/visual/report.js +++ b/.github/workflow-scripts/visual/report.js @@ -43,7 +43,10 @@ function describeChange(result) { } function heading(result) { - const change = describeChange(result); + const change = + result.status === 'added' && result.captured?.unstable === true + ? 'unstable: its two captures differed' + : describeChange(result); return `\`${result.id}\`: ${STATUS_LABELS[result.status]}${change ? `, ${change}` : ''}`; } diff --git a/.github/workflow-scripts/visual/retake.js b/.github/workflow-scripts/visual/retake.js new file mode 100644 index 000000000000..d57a633cd301 --- /dev/null +++ b/.github/workflow-scripts/visual/retake.js @@ -0,0 +1,188 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @noflow + * @format + */ + +/** + * Retakes screenshots, once, before they're uploaded. System UI can briefly + * cover the app, for example the Apple Intelligence notification iOS shows + * shortly after boot, and a covered screenshot published from `main` fails + * every PR based on that commit. + * + * - A screenshot that differs from the baseline is retaken, and the retake + * replaces it only if the retake matches the baseline. A real change + * differs both times and is kept. + * - A screenshot with no baseline is retaken, and is unstable unless both + * captures are identical. Compare doesn't publish unstable screenshots as + * the baseline, so a new test's first baseline is one that was captured + * the same way twice. + * + * Screenshots that already matched the baseline are never retaken, so a + * retake can't spoil them. + */ + +const {buildManifest, sha256} = require('./manifest'); +const {BASELINE_SEARCH_DEPTH, findBaseline} = require('./storage'); +const childProcess = require('node:child_process'); +const fs = require('node:fs'); +const path = require('node:path'); + +// Long enough for a notification banner to be dismissed. +const RETAKE_DELAY_MS = 10000; + +/** The test IDs a flow's `takeScreenshot` commands write. */ +function screenshotIds(flowText) { + return [...flowText.matchAll(/\$\{VISUAL_OUTPUT_DIR\}\/([^\s'"]+)/g)].map( + match => match[1], + ); +} + +/** The flows that take any of `ids`, in the order of `flows`. */ +function flowsTaking(flows, ids, readFile = fs.readFileSync) { + const wanted = new Set(ids); + return flows.filter(flow => + screenshotIds(readFile(flow, 'utf8')).some(id => wanted.has(id)), + ); +} + +/** + * Splits captured images by how they relate to the baseline: `changed` exist + * in it with different contents, and `added` aren't in it. + */ +function retakeCandidates(images, baselineManifest) { + const expected = new Map( + baselineManifest.images.map(image => [image.id, image.sha256]), + ); + const changed = []; + const added = []; + for (const image of images) { + if (!expected.has(image.id)) { + added.push(image.id); + } else if (expected.get(image.id) !== image.sha256) { + changed.push(image.id); + } + } + return {changed, added}; +} + +/** + * Decides what to do with each retake. `retakeSha` maps a test ID to its + * retake's SHA-256, or null if the retake is missing. Returns the changed IDs + * whose retake matches the baseline, so replaces the original, and the added + * IDs whose retake doesn't match the original. + */ +function judgeRetakes({changed, added, images, baselineManifest, retakeSha}) { + const expected = new Map( + baselineManifest.images.map(image => [image.id, image.sha256]), + ); + const original = new Map(images.map(image => [image.id, image.sha256])); + return { + replaced: changed.filter(id => { + const sha = retakeSha(id); + return sha != null && sha === expected.get(id); + }), + unstable: added.filter(id => retakeSha(id) !== original.get(id)), + }; +} + +/** `HEAD^1` and its first-parent ancestors, newest first. */ +function baselineCandidates() { + return childProcess + .execFileSync( + 'git', + [ + 'rev-list', + '--first-parent', + `--max-count=${BASELINE_SEARCH_DEPTH}`, + 'HEAD^1', + ], + {encoding: 'utf8', timeout: 30000}, + ) + .trim() + .split('\n'); +} + +/** + * Retakes the changed and new screenshots in `outputDir` by running + * `runFlows` on the flows that take them, into a separate directory. Returns + * the IDs of unstable new screenshots, and summary lines. + */ +async function retakeScreenshots({ + platform, + environment, + flows, + outputDir, + publicBaseUrl, + runFlows, + candidateShas = baselineCandidates(), +}) { + const {environmentKey, images} = buildManifest({ + captureDir: outputDir, + platform, + environment, + git: null, + }); + const baseline = await findBaseline({ + publicBaseUrl, + environmentKey, + candidateShas, + }); + if (baseline == null) { + return { + unstable: [], + lines: [`No baseline for \`${environmentKey}\`, so nothing was retaken.`], + }; + } + const {changed, added} = retakeCandidates(images, baseline.manifest); + if (changed.length === 0 && added.length === 0) { + return {unstable: [], lines: []}; + } + + const retakeDir = `${outputDir}-retake`; + fs.rmSync(retakeDir, {recursive: true, force: true}); + fs.mkdirSync(retakeDir, {recursive: true}); + await new Promise(resolve => setTimeout(resolve, RETAKE_DELAY_MS)); + runFlows(flowsTaking(flows, [...changed, ...added]), retakeDir); + + const retakePath = id => path.join(retakeDir, `${id}.png`); + const {replaced, unstable} = judgeRetakes({ + changed, + added, + images, + baselineManifest: baseline.manifest, + retakeSha: id => + fs.existsSync(retakePath(id)) + ? sha256(fs.readFileSync(retakePath(id))) + : null, + }); + for (const id of replaced) { + fs.copyFileSync(retakePath(id), path.join(outputDir, `${id}.png`)); + } + fs.rmSync(retakeDir, {recursive: true, force: true}); + + const lines = [ + `Retook ${changed.length + added.length} screenshot(s) against the baseline at \`${baseline.sha.slice(0, 12)}\`:`, + ...changed.map( + id => + `- \`${id}\`: ${replaced.includes(id) ? 'matched the baseline on the retake, replaced' : 'differed again, kept'}`, + ), + ...added.map( + id => + `- \`${id}\`: new, ${unstable.includes(id) ? 'unstable (the two captures differ)' : 'identical twice'}`, + ), + ]; + return {unstable, lines}; +} + +module.exports = { + flowsTaking, + judgeRetakes, + retakeCandidates, + retakeScreenshots, + screenshotIds, +}; diff --git a/.github/workflow-scripts/visual/storage.js b/.github/workflow-scripts/visual/storage.js index d91c7552fa47..f74addc4932e 100644 --- a/.github/workflow-scripts/visual/storage.js +++ b/.github/workflow-scripts/visual/storage.js @@ -10,6 +10,9 @@ const {SHA_PATTERN} = require('./manifest'); +// How many commits back from a capture's base commit to look for a baseline. +const BASELINE_SEARCH_DEPTH = 20; + // Object layout. Every key is write-once: reports and approvals include the // run that produced them, and a baseline is only ever written for a new SHA. // @@ -130,6 +133,7 @@ async function findBaseline({publicBaseUrl, environmentKey, candidateShas}) { } module.exports = { + BASELINE_SEARCH_DEPTH, approvalKey, baselinePrefix, fetchBuffer, diff --git a/.github/workflow-scripts/visual/validate.js b/.github/workflow-scripts/visual/validate.js index 6dc9f4391c4b..4c79118d3b5d 100644 --- a/.github/workflow-scripts/visual/validate.js +++ b/.github/workflow-scripts/visual/validate.js @@ -137,6 +137,9 @@ function validateCapture(captureDir, limits = LIMITS) { ) { errors.push(`${id}: file does not match its manifest entry`); } + if (image.unstable !== undefined && image.unstable !== true) { + errors.push(`${id}: unstable must be true or absent`); + } } if (totalBytes > limits.maxTotalBytes) { errors.push(`Total size ${totalBytes} exceeds ${limits.maxTotalBytes}`); diff --git a/.github/workflows/visual-capture-rntester.yml b/.github/workflows/visual-capture-rntester.yml index 84a0f063f78a..b10cb7b71963 100644 --- a/.github/workflows/visual-capture-rntester.yml +++ b/.github/workflows/visual-capture-rntester.yml @@ -22,6 +22,11 @@ env: VISUAL_FLOWS: packages/rn-tester/visual-tests/flows VISUAL_OUTPUT_DIR: /tmp/visual-capture VISUAL_HEAD_SHA: ${{ github.event.pull_request.head.sha || github.sha }} + # Read-only, to retake screenshots that differ from the baseline. + VISUAL_PUBLIC_BASE_URL: ${{ vars.VISUAL_PUBLIC_BASE_URL }} + # New screenshots whose two captures differ, for manifest.js. Outside the + # output directory, so it isn't uploaded. + VISUAL_UNSTABLE_PATH: /tmp/visual-unstable.json jobs: capture_android: @@ -31,8 +36,9 @@ jobs: - name: Checkout uses: actions/checkout@v6 with: - # The manifest records HEAD^1, the commit this capture is based on. - fetch-depth: 2 + # The manifest records HEAD^1, the commit this capture is based on, + # and retakes look for a baseline up to 20 commits before it. + fetch-depth: 21 - name: Setup node.js uses: ./.github/actions/setup-node - name: Set up JDK 17 @@ -108,8 +114,9 @@ jobs: - name: Checkout uses: actions/checkout@v6 with: - # The manifest records HEAD^1, the commit this capture is based on. - fetch-depth: 2 + # The manifest records HEAD^1, the commit this capture is based on, + # and retakes look for a baseline up to 20 commits before it. + fetch-depth: 21 - name: Setup node.js uses: ./.github/actions/setup-node - name: Set up JDK 17 diff --git a/packages/rn-tester/visual-tests/README.md b/packages/rn-tester/visual-tests/README.md index d472af6d9d62..918066d35def 100644 --- a/packages/rn-tester/visual-tests/README.md +++ b/packages/rn-tester/visual-tests/README.md @@ -6,7 +6,7 @@ Each test is a [Maestro](https://maestro.dev) flow in `flows/`. Maestro only ope ## How it runs in CI -1. Test All builds RNTester Release for Android and iOS. Once a build is ready, the `test_visual_android_rntester` and `test_visual_ios_rntester` jobs (`.github/workflows/visual-capture-rntester.yml`) run every flow in this directory on one canonical device per platform: an API 35 Pixel 7 emulator and an iPhone 17 Pro simulator on iOS 26.2. They upload the screenshots and a manifest of the device environment as `visual-capture-android` and `visual-capture-ios` artifacts. +1. Test All builds RNTester Release for Android and iOS. Once a build is ready, the `test_visual_android_rntester` and `test_visual_ios_rntester` jobs (`.github/workflows/visual-capture-rntester.yml`) run every flow in this directory on one canonical device per platform: an API 35 Pixel 7 emulator and an iPhone 17 Pro simulator on iOS 26.2. Any screenshot that differs from the baseline is retaken once, and the retake is kept if it matches, so a notification or other system UI that briefly covers the app doesn't cause a failure. A new test is taken twice too, and if the two captures differ, the report marks it unstable and it isn't published as the baseline. They upload the screenshots and a manifest of the device environment as `visual-capture-android` and `visual-capture-ios` artifacts. 2. When Test All completes, Visual Compare RNTester (`.github/workflows/visual-compare-rntester.yml`) compares the screenshots with the baseline and reports a `Visual regression (android)` and `Visual regression (ios)` check and a `Visual regression approval` gate, plus a single PR comment with the expected, captured and diff images for anything that changed. 3. On pushes to `main`, Visual Compare RNTester also publishes the captures as the baseline for that commit. There's no separate workflow for this: the same run publishes them once it has compared them. From 84f6c29083954da9de453436fbfec62409dcf3e7 Mon Sep 17 00:00:00 2001 From: Rob Hogan <2590098+robhogan@users.noreply.github.com> Date: Mon, 26 Oct 2026 15:25:50 +0000 Subject: [PATCH 2/3] Retake every screenshot when the environment has no baseline --- .../__tests__/visual-retake-test.js | 58 +++++++++++++++++-- .github/workflow-scripts/visual/retake.js | 21 ++++--- 2 files changed, 64 insertions(+), 15 deletions(-) diff --git a/.github/workflow-scripts/__tests__/visual-retake-test.js b/.github/workflow-scripts/__tests__/visual-retake-test.js index 27afaadee655..c24fa1a915e3 100644 --- a/.github/workflow-scripts/__tests__/visual-retake-test.js +++ b/.github/workflow-scripts/__tests__/visual-retake-test.js @@ -13,9 +13,10 @@ const { flowsTaking, judgeRetakes, retakeCandidates, + retakeScreenshots, screenshotIds, } = require('../visual/retake'); -const {ANDROID_ENVIRONMENT} = require('./__fixtures__/visual'); +const {ANDROID_ENVIRONMENT, SHA_A} = require('./__fixtures__/visual'); const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); @@ -27,6 +28,11 @@ const baselineManifest = { ], }; +const BUNNY = path.join( + __dirname, + '../../../packages/rn-tester/js/assets/bunny.png', +); + const images = [ {id: 'switch/basic/default', sha256: 'a'}, {id: 'switch/basic/toggled', sha256: 'x'}, @@ -108,12 +114,56 @@ describe('visual retake', () => { ).toEqual(unstable); }); - test('marks unstable screenshots in the manifest', () => { + test('retakes every screenshot when the environment has no baseline', async () => { + // The repo's Jest config fakes timers, and the retake waits on one. + jest.useRealTimers(); const directory = fs.mkdtempSync(path.join(os.tmpdir(), 'visual-test-')); + const fetchSpy = jest + .spyOn(global, 'fetch') + .mockResolvedValue(new Response(null, {status: 404})); try { - const png = fs.readFileSync( - path.join(__dirname, '../../../packages/rn-tester/js/assets/bunny.png'), + const outputDir = path.join(directory, 'out'); + const png = fs.readFileSync(BUNNY); + const write = (dir, id, contents) => { + fs.mkdirSync(path.join(dir, path.dirname(id)), {recursive: true}); + fs.writeFileSync(path.join(dir, `${id}.png`), contents); + }; + write(outputDir, 'view/basic/default', png); + write(outputDir, 'view/basic/flaky', png); + const flow = path.join(directory, 'view.yml'); + fs.writeFileSync( + flow, + [ + '- takeScreenshot: ${VISUAL_OUTPUT_DIR}/view/basic/default', + '- takeScreenshot: ${VISUAL_OUTPUT_DIR}/view/basic/flaky', + ].join('\n'), ); + const runFlows = jest.fn((flows, retakeDir) => { + write(retakeDir, 'view/basic/default', png); + write(retakeDir, 'view/basic/flaky', Buffer.concat([png, png])); + }); + const {unstable} = await retakeScreenshots({ + platform: 'android', + environment: ANDROID_ENVIRONMENT, + flows: [flow], + outputDir, + publicBaseUrl: 'https://storage.example', + runFlows, + candidateShas: [SHA_A], + delayMs: 0, + }); + expect(runFlows).toHaveBeenCalledWith([flow], `${outputDir}-retake`); + expect(unstable).toEqual(['view/basic/flaky']); + } finally { + fetchSpy.mockRestore(); + fs.rmSync(directory, {recursive: true, force: true}); + } + }); + + test('marks unstable screenshots in the manifest', () => { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), 'visual-test-')); + try { + const png = fs.readFileSync(BUNNY); for (const id of ['view/basic/default', 'view/basic/flaky']) { fs.mkdirSync(path.join(directory, path.dirname(id)), {recursive: true}); fs.writeFileSync(path.join(directory, `${id}.png`), png); diff --git a/.github/workflow-scripts/visual/retake.js b/.github/workflow-scripts/visual/retake.js index d57a633cd301..2c1d8f6c9065 100644 --- a/.github/workflow-scripts/visual/retake.js +++ b/.github/workflow-scripts/visual/retake.js @@ -18,7 +18,8 @@ * replaces it only if the retake matches the baseline. A real change * differs both times and is kept. * - A screenshot with no baseline is retaken, and is unstable unless both - * captures are identical. Compare doesn't publish unstable screenshots as + * captures are identical. With no baseline for the environment at all, + * that's every screenshot. Compare doesn't publish unstable screenshots as * the baseline, so a new test's first baseline is one that was captured * the same way twice. * @@ -120,6 +121,7 @@ async function retakeScreenshots({ publicBaseUrl, runFlows, candidateShas = baselineCandidates(), + delayMs = RETAKE_DELAY_MS, }) { const {environmentKey, images} = buildManifest({ captureDir: outputDir, @@ -132,13 +134,8 @@ async function retakeScreenshots({ environmentKey, candidateShas, }); - if (baseline == null) { - return { - unstable: [], - lines: [`No baseline for \`${environmentKey}\`, so nothing was retaken.`], - }; - } - const {changed, added} = retakeCandidates(images, baseline.manifest); + const baselineManifest = baseline?.manifest ?? {images: []}; + const {changed, added} = retakeCandidates(images, baselineManifest); if (changed.length === 0 && added.length === 0) { return {unstable: [], lines: []}; } @@ -146,7 +143,7 @@ async function retakeScreenshots({ const retakeDir = `${outputDir}-retake`; fs.rmSync(retakeDir, {recursive: true, force: true}); fs.mkdirSync(retakeDir, {recursive: true}); - await new Promise(resolve => setTimeout(resolve, RETAKE_DELAY_MS)); + await new Promise(resolve => setTimeout(resolve, delayMs)); runFlows(flowsTaking(flows, [...changed, ...added]), retakeDir); const retakePath = id => path.join(retakeDir, `${id}.png`); @@ -154,7 +151,7 @@ async function retakeScreenshots({ changed, added, images, - baselineManifest: baseline.manifest, + baselineManifest, retakeSha: id => fs.existsSync(retakePath(id)) ? sha256(fs.readFileSync(retakePath(id))) @@ -166,7 +163,9 @@ async function retakeScreenshots({ fs.rmSync(retakeDir, {recursive: true, force: true}); const lines = [ - `Retook ${changed.length + added.length} screenshot(s) against the baseline at \`${baseline.sha.slice(0, 12)}\`:`, + baseline == null + ? `Retook ${added.length} screenshot(s), with no baseline for \`${environmentKey}\`:` + : `Retook ${changed.length + added.length} screenshot(s) against the baseline at \`${baseline.sha.slice(0, 12)}\`:`, ...changed.map( id => `- \`${id}\`: ${replaced.includes(id) ? 'matched the baseline on the retake, replaced' : 'differed again, kept'}`, From 0f34cd275b6d0640212a1e51c417edf196a0ef49 Mon Sep 17 00:00:00 2001 From: Rob Hogan <2590098+robhogan@users.noreply.github.com> Date: Mon, 26 Oct 2026 18:37:39 +0000 Subject: [PATCH 3/3] Wait for the home screen before deep-linking on Android --- packages/rn-tester/visual-tests/helpers/open-example.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/rn-tester/visual-tests/helpers/open-example.yml b/packages/rn-tester/visual-tests/helpers/open-example.yml index a300ba8c7a39..5569134d593c 100644 --- a/packages/rn-tester/visual-tests/helpers/open-example.yml +++ b/packages/rn-tester/visual-tests/helpers/open-example.yml @@ -2,6 +2,15 @@ appId: ${APP_ID} # iOS: com.meta.RNTester.localDevelopment | Android: com.facebo --- - launchApp: stopApp: true +# Android drops a deep link that arrives before RNTester is ready. iOS can +# restore another screen on relaunch, so it doesn't wait for the home screen. +- runFlow: + when: + platform: Android + commands: + - extendedWaitUntil: + visible: 'Components' + timeout: 30000 - openLink: ${EXAMPLE_URL} - runFlow: when: