diff --git a/.github/workflows/_static-checks.yaml b/.github/workflows/_static-checks.yaml index 3411df5b..edf465e6 100644 --- a/.github/workflows/_static-checks.yaml +++ b/.github/workflows/_static-checks.yaml @@ -34,8 +34,8 @@ jobs: - name: Test release PR body renderer run: node --test scripts/upstream-release/render-pr-body.test.mjs - - name: Test screenshot summary updater - run: node --test scripts/update-pr-screenshot-summary.test.mjs + - name: Test screenshot summary helpers + run: node --test scripts/update-pr-screenshot-summary.test.mjs scripts/pr-screenshot-workflow.test.mjs - name: Test Helm reference generator run: node --test scripts/upstream-release/generate-helm-reference.test.mjs diff --git a/.github/workflows/pr-screenshot-summary.yaml b/.github/workflows/pr-screenshot-summary.yaml index 4933c91f..f136bb6d 100644 --- a/.github/workflows/pr-screenshot-summary.yaml +++ b/.github/workflows/pr-screenshot-summary.yaml @@ -95,16 +95,34 @@ jobs: JOBS_FILE="$RUNNER_TEMP/jobs.json" ARTIFACTS_FILE="$RUNNER_TEMP/artifacts.json" gh api "repos/$REPO/actions/runs/$RUN_ID/jobs?per_page=100" > "$JOBS_FILE" - gh api "repos/$REPO/actions/runs/$RUN_ID/artifacts?per_page=100" > "$ARTIFACTS_FILE" CONCLUSION="$(jq -r '[.jobs[] | select(.name == "Visual regression / Playwright visual snapshots")][0].conclusion // "missing"' "$JOBS_FILE")" JOB_URL="$(jq -r '[.jobs[] | select(.name == "Visual regression / Playwright visual snapshots")][0].html_url // ""' "$JOBS_FILE")" - jq '.artifacts |= ([.artifacts[] | select(.name == "visual-failure-details" and .expired == false)] + [.artifacts[] | select(.name == "playwright-report" and .expired == false)])' "$ARTIFACTS_FILE" > "$ARTIFACTS_FILE.selected" + # Preserve the conclusion so the always() comment step can still + # post a fallback if artifact discovery or parsing fails. + { + echo "conclusion=$CONCLUSION" + echo "job_url=$JOB_URL" + } >> "$GITHUB_OUTPUT" + JOB_STARTED_AT="$(jq -r '[.jobs[] | select(.name == "Visual regression / Playwright visual snapshots")][0].started_at // ""' "$JOBS_FILE")" + JOB_COMPLETED_AT="$(jq -r '[.jobs[] | select(.name == "Visual regression / Playwright visual snapshots")][0].completed_at // ""' "$JOBS_FILE")" + gh api "repos/$REPO/actions/runs/$RUN_ID/artifacts?per_page=100" > "$ARTIFACTS_FILE" + # A run's artifacts include prior attempts. GitHub returns these + # timestamps in UTC; accept only files created during this job. + jq --arg started_at "$JOB_STARTED_AT" --arg completed_at "$JOB_COMPLETED_AT" ' + .artifacts = [.artifacts[] | select( + .expired == false and + $started_at != "" and $completed_at != "" and + .created_at >= $started_at and .created_at <= $completed_at + )] | + .artifacts = ( + [.artifacts[] | select(.name == "visual-failure-details")] + + [.artifacts[] | select(.name == "playwright-report")] + ) + ' "$ARTIFACTS_FILE" > "$ARTIFACTS_FILE.selected" mv "$ARTIFACTS_FILE.selected" "$ARTIFACTS_FILE" ARTIFACT_ID="$(jq -r '.artifacts[0].id // ""' "$ARTIFACTS_FILE")" ARTIFACT_SIZE="$(jq -r '.artifacts[0].size_in_bytes // "0"' "$ARTIFACTS_FILE")" { - echo "conclusion=$CONCLUSION" - echo "job_url=$JOB_URL" echo "artifact_id=$ARTIFACT_ID" echo "artifact_size=$ARTIFACT_SIZE" } >> "$GITHUB_OUTPUT" diff --git a/scripts/pr-screenshot-workflow.test.mjs b/scripts/pr-screenshot-workflow.test.mjs new file mode 100644 index 00000000..2cd3668b --- /dev/null +++ b/scripts/pr-screenshot-workflow.test.mjs @@ -0,0 +1,184 @@ +// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import test from 'node:test'; +import { parse } from 'yaml'; + +const workflow = parse( + fs.readFileSync( + new URL('../.github/workflows/pr-screenshot-summary.yaml', import.meta.url), + 'utf8' + ) +); +const resolveStep = workflow.jobs['update-description'].steps.find( + (step) => step.id === 'visual' +); + +function resolveArtifacts(t, artifacts, { unavailable = false } = {}) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'visual-job-')); + t.after(() => fs.rmSync(root, { recursive: true, force: true })); + const bin = path.join(root, 'bin'); + fs.mkdirSync(bin); + fs.writeFileSync( + path.join(root, 'jobs.json.fixture'), + JSON.stringify({ + jobs: [ + { + name: 'Static checks / Lint and format checks', + conclusion: 'success', + }, + { + name: 'Visual regression / Playwright visual snapshots', + conclusion: 'failure', + html_url: 'https://github.com/example/docs/actions/runs/1/job/2', + started_at: '2026-10-30T17:00:00Z', + completed_at: '2026-10-30T17:05:00Z', + }, + ], + }) + ); + fs.writeFileSync( + path.join(root, 'artifacts.json.fixture'), + typeof artifacts === 'string' ? artifacts : JSON.stringify({ artifacts }) + ); + fs.writeFileSync( + path.join(bin, 'gh'), + `#!/bin/sh +case "$2" in + */jobs?*) cat "$RUNNER_TEMP/jobs.json.fixture" ;; + */artifacts?*) + if [ "$ARTIFACTS_UNAVAILABLE" = true ]; then exit 1; fi + cat "$RUNNER_TEMP/artifacts.json.fixture" ;; + *) exit 1 ;; +esac +`, + { mode: 0o755 } + ); + const outputFile = path.join(root, 'outputs'); + const result = spawnSync('bash', ['-eo', 'pipefail', '-c', resolveStep.run], { + encoding: 'utf8', + env: { + ...process.env, + PATH: `${bin}${path.delimiter}${process.env.PATH}`, + RUNNER_TEMP: root, + GITHUB_OUTPUT: outputFile, + REPO: 'example/docs', + RUN_ID: '1', + ARTIFACTS_UNAVAILABLE: String(unavailable), + }, + }); + const outputs = Object.fromEntries( + fs + .readFileSync(outputFile, 'utf8') + .trim() + .split('\n') + .map((line) => { + const separator = line.indexOf('='); + return [line.slice(0, separator), line.slice(separator + 1)]; + }) + ); + assert.equal(outputs.conclusion, 'failure'); + assert.equal( + outputs.job_url, + 'https://github.com/example/docs/actions/runs/1/job/2' + ); + return { result, outputs }; +} + +const fullReport = { + name: 'playwright-report', + expired: false, + id: 10, + size_in_bytes: 55_544_517, + created_at: '2026-10-30T17:04:00Z', +}; +const compactReport = { + name: 'visual-failure-details', + expired: false, + id: 20, + size_in_bytes: 21_006, + created_at: '2026-10-30T17:03:00Z', +}; + +test('selects compact details ahead of a large full report', (t) => { + const { result, outputs } = resolveArtifacts(t, [fullReport, compactReport]); + assert.equal(result.status, 0, result.stderr); + assert.equal(outputs.artifact_id, '20'); + assert.equal(outputs.artifact_size, '21006'); +}); + +test('falls back to the full report when compact details are expired', (t) => { + const { result, outputs } = resolveArtifacts(t, [ + { ...compactReport, expired: true }, + fullReport, + ]); + assert.equal(result.status, 0, result.stderr); + assert.equal(outputs.artifact_id, '10'); +}); + +test('preserves the failure conclusion when no artifact is available', (t) => { + const { result, outputs } = resolveArtifacts(t, []); + assert.equal(result.status, 0, result.stderr); + assert.equal(outputs.artifact_id, ''); + assert.equal(outputs.artifact_size, '0'); +}); + +test('preserves the failure conclusion when artifact discovery fails', (t) => { + const { result, outputs } = resolveArtifacts(t, [], { unavailable: true }); + assert.notEqual(result.status, 0); + assert.equal(outputs.artifact_id, undefined); +}); + +test('preserves the failure conclusion when artifact parsing fails', (t) => { + const { result, outputs } = resolveArtifacts(t, '{malformed'); + assert.notEqual(result.status, 0); + assert.equal(outputs.artifact_id, undefined); +}); + +test('uses the current full report ahead of prior-attempt compact details', (t) => { + const { result, outputs } = resolveArtifacts(t, [ + { ...compactReport, created_at: '2026-10-30T16:55:00Z' }, + fullReport, + ]); + assert.equal(result.status, 0, result.stderr); + assert.equal(outputs.artifact_id, '10'); +}); + +test('uses the fallback when a rerun produces no new artifacts', (t) => { + const { result, outputs } = resolveArtifacts(t, [ + { ...compactReport, created_at: '2026-10-30T16:55:00Z' }, + { ...fullReport, created_at: '2026-10-30T16:56:00Z' }, + ]); + assert.equal(result.status, 0, result.stderr); + assert.equal(outputs.artifact_id, ''); +}); + +test('selects current compact details when older compact details appear first', (t) => { + const { result, outputs } = resolveArtifacts(t, [ + { ...compactReport, id: 19, created_at: '2026-10-30T16:55:00Z' }, + compactReport, + ]); + assert.equal(result.status, 0, result.stderr); + assert.equal(outputs.artifact_id, '20'); +}); + +test('rejects artifacts uploaded after the current visual job finished', (t) => { + const { result, outputs } = resolveArtifacts(t, [ + { ...compactReport, created_at: '2026-10-30T17:10:00Z' }, + ]); + assert.equal(result.status, 0, result.stderr); + assert.equal(outputs.artifact_id, ''); +}); + +test('rejects artifacts with no creation timestamp', (t) => { + const { result, outputs } = resolveArtifacts(t, [ + { ...compactReport, created_at: undefined }, + ]); + assert.equal(result.status, 0, result.stderr); + assert.equal(outputs.artifact_id, ''); +}); diff --git a/tests/visual/pages.spec.ts b/tests/visual/pages.spec.ts index 404437a7..a1aa1bbd 100644 --- a/tests/visual/pages.spec.ts +++ b/tests/visual/pages.spec.ts @@ -2,7 +2,11 @@ // SPDX-License-Identifier: Apache-2.0 import { expect, test, type Page } from '@playwright/test'; -import { captureBothThemes, captureElementBothThemes } from './fixtures'; +import { + captureBothThemes, + captureColorScheme, + captureElementBothThemes, +} from './fixtures'; /** * The documented representative-page matrix for issue #1163. Keep this @@ -32,16 +36,20 @@ test('home page', async ({ page }, testInfo) => { await captureBothThemes(page, testInfo, 'Home page'); }); -test('theme preview page', async ({ page }, testInfo) => { - await gotoSuccessful(page, '/theme-preview'); - // Mermaid diagrams render client-side and asynchronously — wait for - // both of the page's diagrams to finish before capturing, or the - // snapshot flakes between "still rendering" and "done". - await expect(page.locator('.docusaurus-mermaid-container svg')).toHaveCount( - 2 - ); - await captureBothThemes(page, testInfo, 'Theme preview page'); -}); +for (const scheme of ['light', 'dark'] as const) { + test(`theme preview page - ${scheme}`, async ({ page }, testInfo) => { + // Mermaid rerenders asynchronously when the theme changes. Start each + // page in its final theme so full-page capture cannot measure the page + // while a theme switch temporarily removes a diagram. + await page.emulateMedia({ colorScheme: scheme }); + await gotoSuccessful(page, '/theme-preview'); + await expect(page.locator('html')).toHaveAttribute('data-theme', scheme); + await expect(page.locator('.docusaurus-mermaid-container svg')).toHaveCount( + 2 + ); + await captureColorScheme(page, testInfo, 'Theme preview page', scheme); + }); +} for (const { section, path } of NAV_PAGES) { test(`navigation loads - ${section}`, async ({ page }) => { diff --git a/tests/visual/pages.spec.ts-snapshots/theme-preview-page-dark-desktop-linux.json b/tests/visual/pages.spec.ts-snapshots/theme-preview-page-dark-desktop-linux.json index 61452b81..b2349cc0 100644 --- a/tests/visual/pages.spec.ts-snapshots/theme-preview-page-dark-desktop-linux.json +++ b/tests/visual/pages.spec.ts-snapshots/theme-preview-page-dark-desktop-linux.json @@ -1,12 +1,12 @@ { "titlePath": [ "pages.spec.ts", - "theme preview page" + "theme preview page - dark" ], "testFile": "tests/visual/pages.spec.ts", "url": { "origin": "http://localhost:3005", "path": "/theme-preview" }, - "reproCommand": "npx playwright test tests/visual/pages.spec.ts -g \"theme preview page\"" + "reproCommand": "npx playwright test tests/visual/pages.spec.ts -g \"theme preview page - dark\"" } diff --git a/tests/visual/pages.spec.ts-snapshots/theme-preview-page-dark-desktop-linux.png b/tests/visual/pages.spec.ts-snapshots/theme-preview-page-dark-desktop-linux.png index b18ed9fa..f5466d11 100644 Binary files a/tests/visual/pages.spec.ts-snapshots/theme-preview-page-dark-desktop-linux.png and b/tests/visual/pages.spec.ts-snapshots/theme-preview-page-dark-desktop-linux.png differ diff --git a/tests/visual/pages.spec.ts-snapshots/theme-preview-page-light-desktop-linux.json b/tests/visual/pages.spec.ts-snapshots/theme-preview-page-light-desktop-linux.json index 61452b81..f710370b 100644 --- a/tests/visual/pages.spec.ts-snapshots/theme-preview-page-light-desktop-linux.json +++ b/tests/visual/pages.spec.ts-snapshots/theme-preview-page-light-desktop-linux.json @@ -1,12 +1,12 @@ { "titlePath": [ "pages.spec.ts", - "theme preview page" + "theme preview page - light" ], "testFile": "tests/visual/pages.spec.ts", "url": { "origin": "http://localhost:3005", "path": "/theme-preview" }, - "reproCommand": "npx playwright test tests/visual/pages.spec.ts -g \"theme preview page\"" + "reproCommand": "npx playwright test tests/visual/pages.spec.ts -g \"theme preview page - light\"" }