From 58e053690b35098b17a0f4df044e368a1b3fad78 Mon Sep 17 00:00:00 2001 From: kevinccbsg Date: Mon, 7 Sep 2026 23:29:06 +0200 Subject: [PATCH] feat(actions): add a record composite action MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every consumer that wants clips in CI re-implements the same recipe. twd-vue-example's record.yml is 13 steps, five of which plus two files exist only to work around twd-cli defects. Those defects are now fixed, which is what makes this action worth building: packaging the workarounds would have distributed them as public input surface that then has to be deprecated rather than deleted. The preconditions the spec set are all met — a destroyed ffmpeg stream aborts the run with its stderr surfaced (#24), the output is playable outside Chrome (#24), the preflight checks capability rather than existence (#24), and --changed-since exists (#25). So the action is the four steps it was supposed to be: - uses: BRIKEV/twd-cli/.github/actions/record@v1 with: changed-since: ${{ github.event.pull_request.base.sha }} Notes on the two decisions that are not obvious from the spec: It installs ffmpeg 8.x from BtbN's n8.1 build, not the distro package and not "the obvious static build". Puppeteer passes -movflags hybrid_fragmented, which arrived after ffmpeg 7: 6.1.1 (ubuntu-24.04) no, 7.0.2 (johnvansickle release) no, 8.1.2 yes. The gpl variant also carries libx264 for the H.264 conversion, so one download covers both requirements. The URL is rolling and unchecksummable, which is acceptable because the CLI now probes the binary's real capability before launching a browser — an unusable build fails fast and says why. The action deliberately does not verify that capability itself. That belongs in the CLI preflight so every user gets it, not only Actions users. clip-count 0 is a success. A branch that changed no tests has nothing to record, so the upload step is skipped at zero rather than running with if-no-files-found: error and failing the job on a normal outcome. Workflow policy — the trigger, the PR comment, the dev server, timeout-minutes and continue-on-error — stays with the caller, exactly as it does for the `run` action. The reference workflow in the README shows all of it. Verified by extracting each step body from the YAML and running it against fixtures: argument construction (titles with spaces, quotes and apostrophes each arrive as one argument; a shell-injection attempt arrives as literal text and does not execute; CRLF input is stripped; --record-pace 0 survives), clip counting (nested and non-video files excluded, missing and unresolved directories give 0), directory resolution (absent, partial and unparseable configs all fall back), and input validation. Not verified: the assembled action has never run on a runner. test-example-app does not depend on twd-cli — the e2e workflow calls node ../bin/twd-cli.js — so an e2e job here would exercise the published CLI rather than this branch. The real proof is rewriting twd-vue-example's record.yml onto the action, which is a separate repo and a follow-up. Co-Authored-By: Claude Opus 5 (1M context) --- .github/actions/record/action.yml | 207 ++++++++++++++++++++++++++++++ CLAUDE.md | 32 +++++ README.md | 93 ++++++++++++++ 3 files changed, 332 insertions(+) create mode 100644 .github/actions/record/action.yml diff --git a/.github/actions/record/action.yml b/.github/actions/record/action.yml new file mode 100644 index 0000000..2655642 --- /dev/null +++ b/.github/actions/record/action.yml @@ -0,0 +1,207 @@ +name: 'TWD CLI Record' +description: 'Record TWD browser tests to video in CI and upload the clips as an artifact' +branding: + icon: 'video' + color: 'green' + +inputs: + working-directory: + description: 'Directory where twd.config.json lives' + required: false + default: '.' + changed-since: + description: >- + Record only the tests this branch added or changed since this ref, e.g. + github.event.pull_request.base.sha. Needs history, so set fetch-depth: 0 + on actions/checkout. Mutually exclusive with `tests`. + required: false + default: '' + tests: + description: >- + Newline-separated test titles. Each becomes one --test filter, and filters + are OR'd. Mutually exclusive with `changed-since`. + required: false + default: '' + pace: + description: >- + Milliseconds held after each command, passed to --record-pace. Leave empty + for the CLI default (300). 0 disables pacing. + required: false + default: '' + install-ffmpeg: + description: >- + Install a known-good ffmpeg 8.x. Set false to use whatever is on PATH. + required: false + default: 'true' + upload-artifact: + description: 'Upload the clips as a workflow artifact' + required: false + default: 'true' + artifact-name: + description: 'Name of the uploaded artifact' + required: false + default: 'twd-recording' + retention-days: + description: 'How long to keep the artifact' + required: false + default: '14' + +outputs: + clip-count: + description: >- + How many clips were produced. 0 is a valid, non-failing result: a branch + that changed no tests has nothing to record. + value: ${{ steps.clips.outputs.count }} + dir: + description: 'Directory the clips were written to, for a caller doing its own upload' + value: ${{ steps.resolve.outputs.dir }} + artifact-url: + description: 'URL of the uploaded artifact, when this action uploaded it' + value: ${{ steps.upload.outputs.artifact-url }} + +runs: + using: 'composite' + steps: + - name: Validate inputs + shell: bash + env: + TESTS: ${{ inputs.tests }} + CHANGED_SINCE: ${{ inputs.changed-since }} + run: | + if [ -n "$TESTS" ] && [ -n "$CHANGED_SINCE" ]; then + echo "::error::Set either 'tests' or 'changed-since', not both. They are two ways to choose the same thing, and combining them silently records more than you asked for." + exit 1 + fi + + - name: Install ffmpeg + if: inputs.install-ffmpeg == 'true' + shell: bash + env: + # Pinned to the 8.1 branch, not to a distro package and not to "the + # obvious static build". Puppeteer's screencast passes + # -movflags hybrid_fragmented, which arrived after ffmpeg 7: measured, + # 6.1.1 (ubuntu-24.04) no, 7.0.2 (johnvansickle release) no, 8.1.2 yes. + # apt-get install ffmpeg on a GitHub runner gets you 6.1.1 and a run + # that fails at the first frame. + # + # The gpl variant is deliberate: it carries libx264, which twd-cli uses + # to convert the finished mp4 into something that plays outside Chrome. + # One download covers both requirements. + # + # The URL is stable and the build behind it moves within 8.1.x, so there + # is no checksum to pin. That is acceptable here because twd-cli probes + # the binary's actual capability before it launches a browser, so an + # unusable build fails fast with an actionable message rather than + # silently producing a broken clip. + FFMPEG_URL: 'https://github.com/BtbN/FFmpeg-Builds/releases/download/latest/ffmpeg-n8.1-latest-linux64-gpl-8.1.tar.xz' + run: | + set -euo pipefail + + if [ "$RUNNER_OS" != "Linux" ]; then + echo "::warning::install-ffmpeg only provides a build for Linux runners, and this is $RUNNER_OS. Install ffmpeg 8 or newer yourself, or set install-ffmpeg: false to silence this. twd-cli will tell you if what is on PATH cannot record." + exit 0 + fi + + DEST="$RUNNER_TEMP/twd-ffmpeg" + mkdir -p "$DEST" + curl -fsSL --retry 3 --retry-delay 2 "$FFMPEG_URL" -o "$RUNNER_TEMP/ffmpeg.tar.xz" + tar -xJf "$RUNNER_TEMP/ffmpeg.tar.xz" -C "$DEST" --strip-components=1 + echo "$DEST/bin" >> "$GITHUB_PATH" + "$DEST/bin/ffmpeg" -version | head -n 1 + + - name: Cache Puppeteer browsers + uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4 + with: + path: ~/.cache/puppeteer + key: ${{ runner.os }}-puppeteer-${{ hashFiles(format('{0}/package-lock.json', inputs.working-directory)) }} + restore-keys: | + ${{ runner.os }}-puppeteer- + + - name: Install Chrome for Puppeteer + shell: bash + run: npx puppeteer browsers install chrome + + - name: Resolve the recording directory + id: resolve + shell: bash + working-directory: ${{ inputs.working-directory }} + run: | + # Same shape as the contract report lookup in the `run` action: the + # config is the source of truth, and the action only needs to know where + # to look afterwards. + DIR=$(node -e " + const fs = require('fs'); + let dir = './twd-artifacts'; + try { + const config = JSON.parse(fs.readFileSync('twd.config.json', 'utf-8')); + if (config.record && config.record.dir) dir = config.record.dir; + } catch {} + console.log(dir); + ") + echo "dir=$DIR" >> "$GITHUB_OUTPUT" + echo "Clips will be written to $DIR" + + - name: Record TWD tests + shell: bash + working-directory: ${{ inputs.working-directory }} + env: + CHANGED_SINCE: ${{ inputs.changed-since }} + TESTS: ${{ inputs.tests }} + PACE: ${{ inputs.pace }} + run: | + set -euo pipefail + + # Built as an array so a title containing spaces or quotes reaches the + # CLI as one argument. Every value arrives through the environment + # rather than expression interpolation, so nothing in an input can be + # read as shell. + args=(run --record) + + if [ -n "$CHANGED_SINCE" ]; then + args+=(--changed-since "$CHANGED_SINCE") + fi + + if [ -n "$TESTS" ]; then + while IFS= read -r title; do + title="${title%$'\r'}" + [ -n "$title" ] || continue + args+=(--test "$title") + done <<< "$TESTS" + fi + + if [ -n "$PACE" ]; then + args+=(--record-pace "$PACE") + fi + + npx twd-cli "${args[@]}" + + - name: Count the clips + id: clips + # always(): a failed run can still have written a partial clip, and that + # clip is evidence. The step above has already failed the action, so + # counting and uploading here cannot turn a red run green. + if: always() + shell: bash + working-directory: ${{ inputs.working-directory }} + env: + DIR: ${{ steps.resolve.outputs.dir }} + run: | + COUNT=0 + if [ -n "$DIR" ] && [ -d "$DIR" ]; then + COUNT=$(find "$DIR" -maxdepth 1 -type f \( -name '*.mp4' -o -name '*.webm' -o -name '*.gif' \) | wc -l | tr -d ' ') + fi + echo "count=$COUNT" >> "$GITHUB_OUTPUT" + echo "Recorded $COUNT clip(s)." + + - name: Upload the recording + id: upload + # Skipped at zero clips on purpose. "This branch changed no tests" is a + # normal outcome that must not fail the action, and upload-artifact with + # if-no-files-found: error would fail it. + if: always() && inputs.upload-artifact == 'true' && steps.clips.outputs.count != '' && steps.clips.outputs.count != '0' + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 + with: + name: ${{ inputs.artifact-name }} + path: ${{ inputs.working-directory }}/${{ steps.resolve.outputs.dir }} + retention-days: ${{ inputs.retention-days }} + if-no-files-found: error diff --git a/CLAUDE.md b/CLAUDE.md index 412283b..3ae108f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -68,6 +68,38 @@ These are load-bearing and easy to undo by accident: - **The required movflags are puppeteer's, not ours.** `REQUIRED_MOVFLAGS` mirrors `ScreenRecorder#getFormatArgs` in puppeteer-core, so re-read that method on a puppeteer bump. This is why the preflight probes `-h muxer=mp4` rather than pinning a version floor — a floor written from one measurement ("7 or newer") was already wrong on the second. - **The screencast's own output does not play outside Chrome.** Puppeteer feeds ffmpeg PNG frames with no `-pix_fmt`, so RGB rides into VP9 and the file lands as vp9/`gbrp` in an mp4 container. QuickTime and Preview open neither. `transcodeForPlayback()` re-encodes to h264/`yuv420p` in place after the run; measured on a real capture it also cut 202805 bytes to 49222. Failure there is a warning, never fatal — the untranscoded file is still a correct recording. +## Composite actions + +`.github/actions/run` and `.github/actions/record` are siblings and should stay +shaped alike. Both assume the app is **already served** at the url in +`twd.config.json` — starting a dev server belongs to the caller's workflow, not +to the action. + +Conventions that are load-bearing in both: + +- **Workflow policy stays with the caller.** The trigger, the PR comment, the + label, `timeout-minutes` and `continue-on-error` are per-repo decisions. An + action that comments on a PR also needs `pull-requests: write`, which a caller + should grant deliberately rather than inherit. +- **Every `${{ }}` reaches a script through `env:`**, never interpolated into a + `run` body, and multi-value inputs are built into a bash **array** so a title + containing spaces or quotes stays one argument. +- **Third-party actions are pinned by commit SHA** with a `# v4`-style comment. +- **`if: always()` on the upload steps**, so a failed run still surfaces its + partial evidence without turning the job green. + +`record` installs ffmpeg **8.x** from BtbN's `n8.1` build rather than apt: +ubuntu-24.04 ships 6.1.1, which rejects the `-movflags hybrid_fragmented` +puppeteer passes. The `gpl` variant also carries `libx264` for the H.264 +conversion, so one download covers both. There is deliberately **no capability +check in the action** — that lives in the CLI preflight, so every user gets it +and not only Actions users. The bundled build is Linux-only; other runners get a +warning and skip. + +`clip-count: 0` is a success, not a failure: a branch that changed no tests has +nothing to record. That is why the upload step is skipped at zero rather than +running with `if-no-files-found: error`. + **`test-example-app/`** — A React demo app with TWD tests integrated, used for manual testing/demonstration. Not part of the published package or test suite. ## Testing diff --git a/README.md b/README.md index 076d8c6..3916965 100644 --- a/README.md +++ b/README.md @@ -337,6 +337,99 @@ The action runs in the same job, so coverage data is available for subsequent st run: npm run collect:coverage:text ``` +### Recording a PR's tests (the `record` action) + +The sibling of the `run` action, for clips rather than results. It installs a +known-good ffmpeg, records, and uploads the result: + +```yaml +- uses: BRIKEV/twd-cli/.github/actions/record@main + with: + changed-since: ${{ github.event.pull_request.base.sha }} +``` + +`changed-since` is what keeps the clip watchable — it records only the tests the +branch touched, rather than the whole suite. See +[Running only what this branch changed](#running-only-what-this-branch-changed). + +#### Record action inputs + +| Input | Default | Description | +|-------|---------|-------------| +| `working-directory` | `.` | Directory where `twd.config.json` lives | +| `changed-since` | (empty) | A ref. Records only the tests changed since it. Needs `fetch-depth: 0` on checkout. Mutually exclusive with `tests` | +| `tests` | (empty) | Newline-separated test titles, one `--test` each. Mutually exclusive with `changed-since` | +| `pace` | (empty) | Passed to `--record-pace`. Empty uses the CLI default of 300; `0` disables pacing | +| `install-ffmpeg` | `true` | Install ffmpeg 8.x. Set `false` to use whatever is on `PATH` | +| `upload-artifact` | `true` | Upload the clips as an artifact | +| `artifact-name` | `twd-recording` | Name of the artifact | +| `retention-days` | `14` | How long to keep it | + +#### Record action outputs + +| Output | Description | +|--------|-------------| +| `clip-count` | How many clips were produced. **`0` is a valid, non-failing result** — a branch that changed no tests has nothing to record | +| `dir` | Where the clips are, for a caller that wants to do its own upload | +| `artifact-url` | URL of the artifact, when the action uploaded it | + +#### Why it installs ffmpeg + +Because the distro build is not good enough, and finding that out the hard way is +expensive. Puppeteer's screencast passes `-movflags hybrid_fragmented`, which +arrived after ffmpeg 7 — `apt-get install ffmpeg` on `ubuntu-24.04` gets you +6.1.1, which rejects it. The action installs an 8.1.x build whose `gpl` variant +also carries `libx264`, which the H.264 conversion needs. Set +`install-ffmpeg: false` if you manage your own; `twd-cli` checks the binary can +actually do the job before it launches a browser either way. + +Only Linux runners get the bundled build. On macOS or Windows the step warns and +skips, so install ffmpeg 8+ yourself there. + +#### Reference workflow + +Recording is triggered by a label here, but that part is policy — record every PR +to `main` if you prefer. The trigger, the PR comment and the dev server stay in +your workflow rather than the action, exactly as they do for `run`: + +```yaml +name: Record a PR's tests +on: + pull_request: + types: [labeled] + +jobs: + record: + if: github.event.label.name == 'record' + runs-on: ubuntu-latest + timeout-minutes: 15 # a hung recording must not cost the whole job + permissions: { contents: read, pull-requests: write } + steps: + - uses: actions/checkout@v5 + with: + ref: ${{ github.event.pull_request.head.sha }} + fetch-depth: 0 # --changed-since needs history + - uses: actions/setup-node@v5 + with: { node-version: 24, cache: npm } + - run: npm ci + - run: | + nohup npm run dev > vite.log 2>&1 & + npx wait-on http://localhost:5173 + - uses: BRIKEV/twd-cli/.github/actions/record@main + id: rec + continue-on-error: true # a clip is optional; the PR it describes is not + with: + changed-since: ${{ github.event.pull_request.base.sha }} + - if: steps.rec.outputs.clip-count != '0' + run: gh pr comment "$PR" --body "${{ steps.rec.outputs.clip-count }} clip(s): ${{ steps.rec.outputs.artifact-url }}" + env: + GH_TOKEN: ${{ github.token }} + PR: ${{ github.event.pull_request.number }} +``` + +The `timeout-minutes` and `continue-on-error` are belt, not workaround. A +recording is always optional; the pull request it describes is not. + ### Custom setup (without the action) If you prefer full control, set up each step manually. Puppeteer 24+ no longer auto-downloads Chrome, so you need to install it explicitly: