From 1115650e78e4fab88edcd9f9fdebde393d60284f Mon Sep 17 00:00:00 2001 From: Anso Date: Sat, 2 May 2026 15:57:45 -0400 Subject: [PATCH] chore(ci): drop auto screenshot refresh job, switch to manual capture (#884) The release-only `update-screenshots` job opened a `chore/refresh-screenshots` PR and immediately tried to squash-merge it. Branch protection (1 review, 6 status checks) rejected the merge on every release, leaving an open PR behind. Screenshots will instead be refreshed manually after UI changes. Removed: - The `update-screenshots` job from ci.yml (~57 lines). - The `paths-ignore: docs/images/**` push trigger filter; its sole purpose was to break the auto-merge re-trigger cascade. Its absence also fixes a latent bug where docs-only pushes to main would have skipped sync-docs. - Four `head_ref != 'chore/refresh-screenshots'` guards in other jobs. - The "doc screenshots" mention in the skip-bot-PRs comment. Reworked the screenshot capture spec to be opt-in: - playwright.config.ts now defines two projects. The default `chromium` project ignores screenshots.spec.ts; a separate `screenshots` project matches it and is invoked manually. - The e2e CI job runs `--project=chromium` so the screenshots project cannot accidentally run in CI. - Updated the spec's module comment with the new manual invocation. Net: 86 lines removed, 27 added. --- .github/actions/start-app/action.yml | 4 +- .github/workflows/ci.yml | 88 +++------------------------- e2e/screenshots.spec.ts | 9 +-- playwright.config.ts | 15 ++++- 4 files changed, 30 insertions(+), 86 deletions(-) diff --git a/.github/actions/start-app/action.yml b/.github/actions/start-app/action.yml index 4898190d..aadff411 100644 --- a/.github/actions/start-app/action.yml +++ b/.github/actions/start-app/action.yml @@ -19,8 +19,8 @@ inputs: When 'true', skip running `npm run build` in backend/ and assume the caller has already materialized backend/dist (e.g. via download-artifact from an upstream job that already built and verified it). Leave 'false' - for standalone invocations like the update-screenshots job that do not - run in a job graph with a prior build step. + for standalone invocations that do not run in a job graph with a + prior build step. required: false default: 'false' diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 202ef394..0855731b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,20 +5,13 @@ on: branches: [main] push: branches: [main] - # Screenshot auto-merge commits only touch docs/images/. - # Ignoring this path breaks the cascade: - # merge PR → screenshots PR auto-merges → would re-trigger CI → now skipped. - # The next real merge will sync everything (rsync --delete does a full mirror). - paths-ignore: - - 'docs/images/**' # Cancel previous runs on the same branch/PR concurrency: group: ci-${{ github.ref }} cancel-in-progress: true -# Least-privilege default for every job. Individual jobs may override (e.g. -# update-screenshots needs contents: write + pull-requests: write to open a PR). +# Least-privilege default for every job. permissions: contents: read @@ -30,13 +23,11 @@ jobs: name: Backend (Build, Test, Lint) runs-on: ubuntu-latest timeout-minutes: 10 - # Skip on bot PRs that only touch version metadata (release-please) or - # doc screenshots. Neither changes the backend surface, so running the - # full build+test+lint+audit stack is pure action-minute waste. + # Skip on the release-please bot PR. It only touches version metadata, + # so the full build+test+lint+audit stack is pure action-minute waste. if: >- github.event_name == 'pull_request' && github.head_ref != 'release-please--branches--main--components--sencho' - && github.head_ref != 'chore/refresh-screenshots' steps: - name: Checkout Code uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 @@ -87,7 +78,6 @@ jobs: if: >- github.event_name == 'pull_request' && github.head_ref != 'release-please--branches--main--components--sencho' - && github.head_ref != 'chore/refresh-screenshots' steps: - name: Checkout Code uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 @@ -123,7 +113,6 @@ jobs: if: >- github.event_name == 'pull_request' && github.head_ref != 'release-please--branches--main--components--sencho' - && github.head_ref != 'chore/refresh-screenshots' steps: - name: Checkout Code uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 @@ -191,7 +180,6 @@ jobs: if: >- github.event_name == 'pull_request' && github.head_ref != 'release-please--branches--main--components--sencho' - && github.head_ref != 'chore/refresh-screenshots' steps: - name: Checkout Code uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 @@ -212,7 +200,12 @@ jobs: skip-backend-build: 'true' - name: Run E2E tests - run: npx playwright test + # `--project=chromium` is explicit because playwright.config.ts also + # defines a `screenshots` project that captures docs/images/ for + # manual review. That project must not run in CI: its output would + # be discarded after the run, wasting minutes per PR. Keep this + # flag; do not "simplify" back to `npx playwright test`. + run: npx playwright test --project=chromium - name: Upload E2E report and service logs if: failure() @@ -225,69 +218,6 @@ jobs: ci-logs/ retention-days: 7 - # --------------------------------------------------------------------------- - # Post-release only: Screenshot refresh - # Only runs when release-please merges a "chore(main): release" commit, - # so screenshots are refreshed once per release - not on every merge. - # --------------------------------------------------------------------------- - update-screenshots: - name: Refresh Doc Screenshots - runs-on: ubuntu-latest - timeout-minutes: 15 - if: >- - github.event_name == 'push' - && github.ref == 'refs/heads/main' - && startsWith(github.event.head_commit.message, 'chore(main): release') - permissions: - contents: write - pull-requests: write - steps: - # Mint a short-lived installation token for the `sencho-token-app` - # GitHub App. Reused by the checkout, PR creation, and auto-merge - # steps below so we make exactly one /app/installations token call. - - name: Generate GitHub App installation token - id: app-token - uses: actions/create-github-app-token@1b10c78c7865c340bc4f6099eb2f838309f1e8c3 # v3.1.1 - with: - app-id: ${{ secrets.APP_ID }} - private-key: ${{ secrets.APP_PRIVATE_KEY }} - owner: ${{ github.repository_owner }} - repositories: sencho - permission-contents: write - permission-pull-requests: write - - - name: Checkout Code - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 - with: - token: ${{ steps.app-token.outputs.token }} - - - name: Start app & install Playwright - uses: ./.github/actions/start-app - - - name: Capture screenshots - run: npx playwright test e2e/screenshots.spec.ts --project=chromium - - - name: Open / update screenshots PR - id: create-pr - uses: peter-evans/create-pull-request@5f6978faf089d4d20b00c7766989d076bb2fc7f1 # v8.1.1 - with: - token: ${{ steps.app-token.outputs.token }} - branch: chore/refresh-screenshots - commit-message: "docs: refresh screenshots" - title: "docs: refresh screenshots" - body: "Automated screenshot refresh - generated by the `update-screenshots` CI job on push to `main`." - add-paths: docs/images/ - delete-branch: true - - - name: Auto-merge screenshots PR - if: steps.create-pr.outputs.pull-request-number != '' - env: - GH_TOKEN: ${{ steps.app-token.outputs.token }} - run: | - gh pr merge ${{ steps.create-pr.outputs.pull-request-number }} \ - --squash \ - --repo ${{ github.repository }} - sync-docs: name: Sync Docs to sencho-docs runs-on: ubuntu-latest diff --git a/e2e/screenshots.spec.ts b/e2e/screenshots.spec.ts index 2ac2b997..5e3490db 100644 --- a/e2e/screenshots.spec.ts +++ b/e2e/screenshots.spec.ts @@ -2,10 +2,11 @@ * Docs screenshot capture. * * Takes canonical screenshots of key UI views and writes them to docs/images/. - * Run via: npx playwright test e2e/screenshots.spec.ts - * - * The CI `update-screenshots` job runs this on every push to develop and - * commits any changed images back to the repo so sync-docs picks them up. + * Run manually after a UI change that affects a documented view: + * npx playwright test --project=screenshots + * Then review the diff under docs/images/ and commit on a chore branch. + * The default `playwright test` invocation skips this spec via the + * project-level testIgnore in playwright.config.ts. */ import * as fs from 'fs'; import * as path from 'path'; diff --git a/playwright.config.ts b/playwright.config.ts index 429ce5d2..39d29a34 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -30,6 +30,19 @@ export default defineConfig({ }, projects: [ - { name: 'chromium', use: { ...devices['Desktop Chrome'] } }, + { + // Default project: skips the manual screenshot capture spec so + // `npx playwright test` does not regenerate docs images on every run. + name: 'chromium', + use: { ...devices['Desktop Chrome'] }, + testIgnore: ['**/screenshots.spec.ts'], + }, + { + // Manual-only project for capturing docs/images/. Run explicitly: + // npx playwright test --project=screenshots + name: 'screenshots', + use: { ...devices['Desktop Chrome'] }, + testMatch: ['**/screenshots.spec.ts'], + }, ], });