diff --git a/.github/actions/plugin-check-report/README.md b/.github/actions/plugin-check-report/README.md new file mode 100644 index 0000000..f727f1d --- /dev/null +++ b/.github/actions/plugin-check-report/README.md @@ -0,0 +1,226 @@ +# plugin-check-report + +Compares a [WordPress.org Plugin Check](https://github.com/WordPress/plugin-check) results file with +a baseline: the counts of an earlier run of the same workflow. It saves this run's counts as the +next baseline and writes a short body for the pull request comment. + +Most repositories do not use this action directly. They call the reusable workflow +[`.github/workflows/plugin-check.yml`](../../workflows/plugin-check.yml), which builds the plugin, +runs [wordpress/plugin-check-action](https://github.com/WordPress/plugin-check-action), runs this +action and posts the comment. A caller to copy is in +[`templates/workflows/plugin-check.yml`](../../../templates/workflows/plugin-check.yml): + +```yaml +jobs: + plugin-check: + permissions: + actions: read + contents: read + issues: write + pull-requests: write + uses: stellarwp/plugin-toolbox/.github/workflows/plugin-check.yml@main + with: + plugin-slug: my-plugin +``` + +## What the repository needs + +- A `.puprc` whose `build` makes the plugin as it ships, and whose `zip_name` equals `plugin-slug`. + The zip's top folder is named after `zip_name`, and Plugin Check takes the slug from the folder + name. +- A `pup` Composer script that puts `pup.phar` in `bin/` and runs it, as Kadence Blocks + has: + + ```json + "pup": [ + "test -f ./bin/pup.phar || curl -o bin/pup.phar -L -C - https://github.com/stellarwp/pup/releases/download/2.0.0/pup.phar", + "@php bin/pup.phar" + ] + ``` + +- A `.distignore` (or pup's default ignore list) that keeps dev-only files out of the zip. +- The caller file named `plugin-check.yml`. See [The baseline](#the-baseline). + +The JavaScript toolchain comes from files the repository already has: `.nvmrc` sets up Node, +`package-lock.json` turns on the npm cache, and `bun.lock` or `bun.lockb` sets up bun. + +## How the plugin is built + +By default, with pup, the same steps the release zip uses, from the root of the checkout: + +```bash +composer run -q pup > /dev/null # downloads bin/pup.phar +composer -- pup build +version=$(php -d display_errors=0 bin/pup.phar get-version --dev) +composer -- pup package "$version" +name=$(php -d display_errors=0 bin/pup.phar zip-name "$version") +unzip -q "$name.zip" -d "$PLUGIN_CHECK_BUILD_DIR" +``` + +So `.puprc` and `.distignore` decide what is checked. If the release needs a step the check should +see too, such as making the POT file, put it in `.puprc`'s `build`, not in the workflow. + +### `build-command`, for a repository without pup + +`build-command` replaces the pup steps. Prefer `.puprc` when the repository has one: a custom +command is a second place that defines the build, and the two drift apart. + +The contract: + +- It runs with bash, from the root of the checkout, with `-e` and `pipefail`. +- PHP, Node or bun are set up before it, as for pup. The `composer-token` and `package-token` + secrets are in its environment. +- It must leave the plugin, as it ships, in `$PLUGIN_CHECK_BUILD_DIR//`, with the main + plugin file inside: `.php`, or `main-file` when the name differs. + +After the build, pup or not, the workflow checks that `$PLUGIN_CHECK_BUILD_DIR//` +exists, and fails naming what it found instead. A folder, not a zip, because the Plugin Check action +takes a folder and its slug from the folder name; asking for a zip would only add an unzip and a +guess at the top folder. + +```yaml + with: + plugin-slug: my-plugin + build-command: | + composer install --no-dev + npm ci && npm run build + mkdir -p "$PLUGIN_CHECK_BUILD_DIR/my-plugin" + rsync -a --exclude-from=.distignore ./ "$PLUGIN_CHECK_BUILD_DIR/my-plugin/" +``` + +## The baseline + +The baseline is never a committed file. It is the `baseline.json` in the `plugin-check-report` +artifact of an earlier run of the same workflow file: + +- A pull request compares with the run for its base commit. For a stacked pull request that is + the run of the pull request below it. A run still going is waited for, up to 20 minutes. +- If that run has no baseline (it was cancelled, say), the newest run on the target branch is used. + More errors than that older run only warn, because the target may have changed since. +- A push compares with the run before it on the same branch. +- With no earlier run at all, nothing is compared and the check passes with a warning. A failed + lookup (an API or download error) fails instead, so an outage never passes as "no baseline". + +Only runs of the repository's own branches count. A pull request from a fork has a run of the same +workflow, and the runs list matches it by branch name and commit, so a fork's run on a branch called +`develop` would look like a run of `develop`. Its artifact is whatever the fork's workflow uploaded, +so runs whose head commit is in another repository are skipped. + +The earlier runs are found by the caller's workflow file name, read from `GITHUB_WORKFLOW_REF`. +Renaming the caller file starts again with no baseline. Artifacts expire after the repository's +retention period (90 days by default), so a branch with no run for that long starts with none too. + +For a pull request to have a baseline, the branch it targets needs runs: list it under `push` in +the caller. + +## Modes + +- `ratchet` (default): fail only when a rule has more errors than the baseline. More warnings, or + fewer of either, only add a notice. The totals can go down while one rule goes up, so the check + looks at each rule. +- `strict`: fail on any error. The comment still compares with the baseline. Switch to it once a + plugin reaches zero errors; a new plugin with zero errors should start there. + +## The comment + +One comment per pull request, replaced on every run, found by a hidden first line: +``. It says "in progress" while the check runs, then shows the +result, the totals, up to five rules that went up, and links to the run and the artifact. It never +grows past 15 lines. Pull requests from forks get no comment: their token cannot write. + +## The check name + +A job in a called workflow reports as ` / `. With the template's job id, +the check to require in branch protection is `plugin-check / Plugin Check`. + +## Inputs of the workflow + +| Input | Default | What it does | +|---|---|---| +| `plugin-slug` | required | The plugin's folder name. Equals `zip_name` in `.puprc` | +| `main-file` | `.php` | The main plugin file, checked after the build | +| `build-command` | empty: pup | A bash command that builds the plugin instead of pup. See [`build-command`](#build-command-for-a-repository-without-pup) | +| `runs-on` | `ubuntu-latest` | Runner of the job that builds and checks | +| `php-version` | `7.4` | PHP that builds `vendor/`. Use the oldest the plugin supports | +| `exclude-checks` | empty | Checks to skip, one per line, passed to the Plugin Check action | +| `cache-paths` | empty | Build caches kept between runs, one path per line, relative to the repository | +| `mode` | `ratchet` | `ratchet` or `strict`. See [Modes](#modes) | +| `submodules` | `false` | Passed to `actions/checkout` | + +| Secret | Becomes, in the build step only | +|---|---| +| `composer-token` | `COMPOSER_AUTH` with a `github-oauth` entry | +| `package-token` | `NODE_AUTH_TOKEN`, `BUN_AUTH_TOKEN` and `GITHUB_TOKEN` | + +The secrets never reach the job's environment, so the third-party Plugin Check step that runs later +in the same job does not see them there. + +### Permissions the caller grants + +```yaml +permissions: + actions: read # read earlier runs and download their baseline + contents: read # check out the repository + issues: write # the sticky comment + pull-requests: write # the sticky comment +``` + +A called workflow can only lower these. If the caller grants less than a job asks for, GitHub does +not start the run. The job that runs the Plugin Check action lowers itself to `contents: read`. + +## Inputs of the action + +| Input | Required | Default | What it does | +|---|---|---|---| +| `results` | yes | | The results file. A missing file fails the compare | +| `comment-file` | no | none | Where to write the comment body | +| `mode` | no | `ratchet` | `ratchet` or `strict` | +| `comment-marker` | no | `` | First line of the comment body | +| `workflow-file` | no | from `GITHUB_WORKFLOW_REF` | Whose runs hold the baseline | +| `baseline-wait` | no | `1200` | Seconds to wait for the base commit's run. A whole number, or the check fails | +| `badge-url` | no | none | Image under the status line | +| `token` | no | `github.token` | Needs `actions: read` | + +| Output | What it is | +|---|---| +| `artifact-url` | The `plugin-check-report` artifact of this run | +| `baseline-url` | The run the baseline came from. Empty with no baseline | + +## Running it locally + +You need `bash`, `jq`, and for the baseline the GitHub CLI signed in (`gh auth login`). + +1. Build the plugin and run Plugin Check in a local WordPress, for example with wp-env: + + ```bash + npx wp-env start + npx wp-env run cli wp plugin check my-plugin --format=json > results.txt + ``` + + Keep only the command's output in the file. A notice from wp-env at the top fails the ratchet + with "not Plugin Check output", on purpose. + +2. Get a baseline from the branch you compare with: + + ```bash + GITHUB_REPOSITORY=my-org/my-plugin \ + bash .github/actions/plugin-check-report/fetch-baseline.sh develop baseline.json + ``` + +3. Compare: + + ```bash + PLUGIN_CHECK_BASELINE=baseline.json \ + bash .github/actions/plugin-check-report/ratchet.sh --comment-file comment.md results.txt + ``` + + Exit 0 means the check passes. `comment.md` holds what the pull request would show. Add + `PLUGIN_CHECK_MODE=strict` to see the strict result. Skip step 2 to see the counts with no + baseline. + +## How it is tested + +`tests/actions/plugin-check-report/` runs both scripts with bash from `node --test`. `ratchet.sh` +reads the sample results files in `fixtures/`; `fetch-baseline.sh` talks to a stand-in `gh` that +answers from JSON files and records each call. Run `npm install` once, then `node --test`. They need +`jq`, which GitHub's Ubuntu runners have. diff --git a/.github/actions/plugin-check-report/action.yml b/.github/actions/plugin-check-report/action.yml new file mode 100644 index 0000000..09a0a56 --- /dev/null +++ b/.github/actions/plugin-check-report/action.yml @@ -0,0 +1,195 @@ +# Compares a WordPress.org Plugin Check results file with a baseline: the counts of an earlier run +# of the same workflow, never a committed file. Only a rule with more errors than the baseline +# fails (ratchet mode), or any error at all (strict mode). It also saves this run's counts as the +# next baseline and writes the short body for the pull request comment. +# +# It does not run Plugin Check and it does not post the comment. The reusable workflow +# .github/workflows/plugin-check.yml does both, so this action stays usable with a results file +# from anywhere. +# +# The work is in two bash scripts next to this file, ratchet.sh and fetch-baseline.sh. They are +# run by path, so there is nothing to build: the file in the repository is the file that runs. +name: Plugin Check report +description: Compares Plugin Check results with an earlier run's baseline, and saves a new baseline + +inputs: + results: + description: >- + Path of the results file wordpress/plugin-check-action writes, `plugin-check-results.txt`. A + missing file fails the compare step: that is how a broken build or a broken check shows up. + required: true + comment-file: + description: >- + Where to write the short Markdown body for the pull request comment. Empty writes none. The + file is only written when the compare runs, so a caller can tell "no result" from a result. + required: false + default: '' + mode: + description: >- + `ratchet`: fail only when a rule has more errors than the baseline. `strict`: fail on any + error, baseline or not. + required: false + default: 'ratchet' + comment-marker: + description: >- + Hidden first line of the comment body, which the caller searches for to find its own + comment. Empty uses ``. + required: false + default: '' + workflow-file: + description: >- + File name of the workflow whose earlier runs hold the baseline, e.g. `plugin-check.yml`. + Empty reads it from GITHUB_WORKFLOW_REF, which names the calling workflow's file when this + runs inside a reusable workflow. + required: false + default: '' + baseline-wait: + description: >- + Seconds a pull request waits for the run of its base commit to finish, when that run is + still going. + required: false + default: '1200' + badge-url: + description: 'Image URL shown under the status line of the comment. Empty shows none.' + required: false + default: '' + token: + description: >- + Token that lists earlier runs and downloads their artifacts. It needs `actions: read`. + required: false + default: ${{ github.token }} + +outputs: + artifact-url: + description: 'Link to the plugin-check-report artifact this run uploaded. Empty when there was nothing to upload.' + value: ${{ steps.artifact.outputs.artifact-url }} + baseline-url: + description: 'Link to the run the baseline came from. Empty when there was no baseline.' + value: ${{ steps.baseline.outputs.url }} + +runs: + using: 'composite' + steps: + # One folder per run under RUNNER_TEMP, cleared first, so a second use of the action in the same + # job never reads the first one's files. + - name: Set file paths + id: paths + shell: bash + env: + INPUT_WORKFLOW_FILE: ${{ inputs.workflow-file }} + INPUT_COMMENT_MARKER: ${{ inputs.comment-marker }} + run: | + dir="$RUNNER_TEMP/plugin-check-report" + rm -rf "$dir" + mkdir -p "$dir/report" "$dir/baseline" + + # In a called workflow GITHUB_WORKFLOW_REF is the caller's, like + # example-org/my-plugin/.github/workflows/plugin-check.yml@refs/pull/12/merge. The runs that + # hold the baseline belong to that file, not to the toolbox's workflow. + workflow=$INPUT_WORKFLOW_FILE + if [ -z "$workflow" ]; then + ref=${GITHUB_WORKFLOW_REF%%@*} + workflow=${ref##*/} + fi + + { + echo "report=$dir/report" + echo "baseline=$dir/baseline/baseline.json" + echo "workflow=$workflow" + echo "marker=${INPUT_COMMENT_MARKER:-}" + } >> "$GITHUB_OUTPUT" + echo "Baselines are read from the runs of $workflow." + + # A baseline made from this run, uploaded even when the compare below fails. Later runs + # download it as their baseline: pull requests into this branch, and the next push. + - name: Write a new baseline + shell: bash + env: + RESULTS: ${{ inputs.results }} + REPORT_DIR: ${{ steps.paths.outputs.report }} + run: | + if [ -f "$RESULTS" ]; then + cp "$RESULTS" "$REPORT_DIR/" + PLUGIN_CHECK_BASELINE="$REPORT_DIR/baseline.json" bash "$GITHUB_ACTION_PATH/ratchet.sh" --update "$RESULTS" + fi + + # The name and the baseline.json inside it are what fetch-baseline.sh looks for. Keep both: + # repositories that ran an earlier copy of this check upload the same names, so their old + # baselines keep working. + - name: Upload the report + id: artifact + uses: actions/upload-artifact@v7 + with: + name: plugin-check-report + path: ${{ steps.paths.outputs.report }} + if-no-files-found: ignore + + # The baseline: what the branch the code goes into looks like now, from the artifact of an + # earlier run on it. A pull request takes the run for its base commit; for a stacked pull + # request that is the run of the pull request below it. When that run is still going, it is + # waited for. When it has no baseline (it was cancelled by a newer push, say), the latest run on + # the base branch is used instead, and more errors than it are only reported, not failed: that + # run is older than the target. A push takes the run before it on the same branch, so a merge + # that adds errors fails there, where the whole team sees it. + # With no run to take it from, ratchet.sh compares nothing and passes with a warning. A failed + # lookup (GitHub API or download error) fails the step instead, so an outage never passes as + # "no baseline". + - name: Get the baseline + id: baseline + shell: bash + env: + GH_TOKEN: ${{ inputs.token }} + EVENT: ${{ github.event_name }} + BASE_REF: ${{ github.event.pull_request.base.ref }} + BASE_SHA: ${{ github.event.pull_request.base.sha }} + WAIT: ${{ inputs.baseline-wait }} + WORKFLOW: ${{ steps.paths.outputs.workflow }} + BASELINE: ${{ steps.paths.outputs.baseline }} + run: | + fetch="$GITHUB_ACTION_PATH/fetch-baseline.sh" + status=0 + if [ "$EVENT" = pull_request ]; then + label="Target branch" + found=$(bash "$fetch" --workflow "$WORKFLOW" --sha "$BASE_SHA" --wait "$WAIT" "$BASE_REF" "$BASELINE") || status=$? + else + label="Previous run" + found=$(bash "$fetch" --workflow "$WORKFLOW" --before-run "$GITHUB_RUN_ID" "$GITHUB_REF_NAME" "$BASELINE") || status=$? + fi + case "$status" in + 0) ;; + 1) echo "No earlier run has a baseline."; exit 0 ;; + *) exit "$status" ;; + esac + read -r _ url match <<< "$found" + advisory=false + if [ "$match" = older ]; then + label="Target branch, older run" + advisory=true + fi + { + echo "label=$label" + echo "url=$url" + echo "advisory=$advisory" + } >> "$GITHUB_OUTPUT" + echo "Baseline from $url ($match)" + + # Fails the step when the mode says so. Also writes the comment body and the same text to the + # job summary. + - name: Compare with the baseline + shell: bash + env: + RESULTS: ${{ inputs.results }} + COMMENT_FILE: ${{ inputs.comment-file }} + PLUGIN_CHECK_MODE: ${{ inputs.mode }} + PLUGIN_CHECK_BASELINE: ${{ steps.paths.outputs.baseline }} + PLUGIN_CHECK_ADVISORY: ${{ steps.baseline.outputs.advisory }} + COMMENT_MARKER: ${{ steps.paths.outputs.marker }} + BASELINE_LABEL: ${{ steps.baseline.outputs.label }} + BASELINE_URL: ${{ steps.baseline.outputs.url }} + COMMENT_BADGE: ${{ inputs.badge-url }} + ARTIFACT_URL: ${{ steps.artifact.outputs.artifact-url }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: | + args=() + [ -z "$COMMENT_FILE" ] || args=(--comment-file "$COMMENT_FILE") + bash "$GITHUB_ACTION_PATH/ratchet.sh" "${args[@]}" "$RESULTS" diff --git a/.github/actions/plugin-check-report/fetch-baseline.sh b/.github/actions/plugin-check-report/fetch-baseline.sh new file mode 100755 index 0000000..a268971 --- /dev/null +++ b/.github/actions/plugin-check-report/fetch-baseline.sh @@ -0,0 +1,183 @@ +#!/usr/bin/env bash +# +# Downloads the baseline.json that the Plugin Check workflow wrote for a branch: the +# plugin-check-report artifact of an earlier run. That run checked the branch as it really is, so +# the baseline always matches it. Nothing is committed, so two branches worked on at the same time +# can never leave a stale baseline behind. See this action's README. +# +# Usage: +# fetch-baseline.sh [--workflow ] [--sha ] [--wait ] [--before-run ] [] +# +# --workflow is the file name of the workflow whose runs hold the baseline (default: +# plugin-check.yml). When the check runs as a reusable workflow, the runs belong to the CALLER's +# file, so the action passes that name, read from GITHUB_WORKFLOW_REF. +# +# Runs are tried in this order, and the first one with the artifact wins: +# 1. runs for --sha (a pull request's base commit), when given. With --wait, a run for that +# commit that is still going is waited for, up to that many seconds. A run that does not +# show up at all is waited for only a short time (GRACE seconds), since the commit may never +# have had one; +# 2. the latest runs on , newest first. +# --before-run keeps only runs started before that one (run ids only grow), so a run never +# compares with itself, and a re-run of an older push never compares with a newer one. +# +# defaults to baseline.json next to this script, which ratchet.sh reads by default +# and git ignores. +# +# Prints " ". is "exact" when the run is for --sha (or no --sha +# was given), and "older" when --sha was given but only an older run on the branch had a baseline. +# +# Exit status: +# 0 baseline written +# 1 no run has the artifact (none finished yet, or all expired) +# 2 the lookup itself failed (GitHub API or download error), so "no baseline" is not known +# +# Needs the GitHub CLI, signed in (GH_TOKEN in CI) with read access to Actions. + +set -euo pipefail + +workflow=plugin-check.yml +sha="" +wait_for=0 +before_run="" +# How long to wait for a run for --sha to show up at all. +GRACE=120 +usage="Usage: $0 [--workflow ] [--sha ] [--wait ] [--before-run ] []" + +lookup_failed() { + echo "Could not look up Plugin Check runs: $1" >&2 + exit 2 +} + +# A missing or malformed argument exits 2, like any other lookup that cannot be trusted. Exit 1 is +# kept for "no run has a baseline", so a typo must never read as that. +need_value() { + [ "$#" -ge 2 ] && [ -n "$2" ] || lookup_failed "$1 needs a value. $usage" +} + +# A plain count of seconds or a run id. At most 18 digits keeps it inside bash's 64-bit integers, +# and 10# keeps a leading zero from being read as octal later. +number() { + case "$2" in + '' | *[!0-9]*) lookup_failed "$1 must be a whole number, not '$2'" ;; + esac + [ "${#2}" -le 18 ] || lookup_failed "$1 is too large: '$2'" + echo $((10#$2)) +} + +while [ $# -gt 0 ]; do + case "$1" in + --workflow) need_value "$@"; workflow=$2; shift 2 ;; + --sha) need_value "$@"; sha=$2; shift 2 ;; + --wait) need_value "$@"; wait_for=$(number --wait "$2"); shift 2 ;; + --before-run) need_value "$@"; before_run=$(number --before-run "$2"); shift 2 ;; + *) break ;; + esac +done + +[ "$#" -ge 1 ] && [ -n "$1" ] || lookup_failed "a branch is needed. $usage" +branch=$1 +output=${2:-"$(cd "$(dirname "$0")" && pwd)/baseline.json"} + +# Any other command that fails (mktemp, mkdir, cp, ...) is a failure too, never "no baseline": +# exit 1 is kept for the one place below that means it. +trap 'echo "fetch-baseline.sh failed at line $LINENO." >&2; exit 2' ERR + +repo=${GITHUB_REPOSITORY:-$(gh repo view --json nameWithOwner --jq .nameWithOwner)} || lookup_failed "no repository" +# Only a bare file name: it goes into an API path, and a path or a ref there would ask for some +# other resource. +case "$workflow" in + */* | *@* | "") lookup_failed "--workflow must be a file name such as plugin-check.yml, not '$workflow'" ;; +esac +runs="repos/$repo/actions/workflows/$workflow/runs" + +# Only runs of this repository's own branches count. The runs API matches `branch` and `head_sha` +# against the head of a pull request too, so a run for a pull request from a fork, from a branch +# named like the target, would otherwise be taken for a run of the target, and its artifact is +# whatever that fork's workflow chose to upload. +# +# jq reads $repo, so the filters stay in single quotes. +# shellcheck disable=SC2016 +own='select((.head_repository.full_name // "" | ascii_downcase) == ($repo | ascii_downcase))' +# shellcheck disable=SC2016 +own_runs='.workflow_runs[] | '"$own"' | "\(.id) \(.html_url) \(.head_sha)"' +# shellcheck disable=SC2016 +own_counts='[.workflow_runs[] | '"$own"'] | "\(length) \([.[] | select(.status != "completed")] | length)"' + +# The run for the base commit may still be going: a pull request opened right after a merge starts +# at the same time as the push run on the target branch. Waiting for it beats comparing with an +# older run. +if [ -n "$sha" ] && [ "$wait_for" -gt 0 ]; then + started=$(date +%s) + deadline=$((started + wait_for)) + while :; do + # " " for the commit. + counts=$(gh api "$runs?head_sha=$sha&per_page=20" \ + | jq -r --arg repo "$repo" "$own_counts") \ + || lookup_failed "runs for $sha" + read -r total going <<< "$counts" + # A run can take a moment to show up after its push. After GRACE seconds with none, the + # commit never had one. + if [ "$total" -eq 0 ] && [ "$(date +%s)" -ge $((started + GRACE)) ]; then + break + fi + [ "$total" -eq 0 ] || [ "$going" -gt 0 ] || break + if [ "$(date +%s)" -ge "$deadline" ]; then + echo "The run for $sha is still going after $wait_for seconds; trying older runs." >&2 + break + fi + sleep 30 + done +fi + +# " " lines, best first. Captured, not streamed, so a failed API call +# stops here instead of looking like an empty list. +candidates="" +if [ -n "$sha" ]; then + candidates=$(gh api "$runs?status=completed&head_sha=$sha&per_page=10" \ + | jq -r --arg repo "$repo" "$own_runs") || lookup_failed "runs for $sha" +fi +# Newest first by run id (ids only grow), not by the order the API happens to return. One page of +# 100, the most the API gives, is plenty: if none of them has a baseline, an older one would be +# stale anyway. Fork runs are dropped after the page is read, so a flood of them could push ours +# off it; the result is then "no baseline", which passes with a warning, and no fork baseline is +# taken. +on_branch=$(gh api "$runs?status=completed&branch=$branch&per_page=100" \ + | jq -r --arg repo "$repo" "$own_runs" | sort -k1,1nr) || lookup_failed "runs on $branch" +candidates+=${candidates:+$'\n'}$on_branch + +tried=" " +tmp=$(mktemp -d) +trap 'rm -rf "$tmp"' EXIT + +while read -r id url head; do + [ -n "$id" ] || continue + if [ -n "$before_run" ] && [ "$id" -ge "$before_run" ]; then + continue + fi + case "$tried" in *" $id "*) continue ;; esac + tried+="$id " + + # A run that broke before the report job, or whose artifact expired, has nothing to give. + found=$(gh api "repos/$repo/actions/runs/$id/artifacts?name=plugin-check-report" \ + --jq '[.artifacts[] | select(.expired | not)] | length') || lookup_failed "artifacts of run $id" + [ "$found" -gt 0 ] || continue + + rm -rf "$tmp/report" + gh run download "$id" --repo "$repo" --name plugin-check-report --dir "$tmp/report" > /dev/null \ + || lookup_failed "download of the artifact of run $id" + [ -s "$tmp/report/baseline.json" ] || continue + + match=exact + if [ -n "$sha" ] && [ "$head" != "$sha" ]; then + match=older + fi + + mkdir -p "$(dirname "$output")" + cp "$tmp/report/baseline.json" "$output" + echo "$id $url $match" + exit 0 +done <<< "$candidates" + +echo "No Plugin Check run on $branch${sha:+ or $sha} has a baseline." >&2 +exit 1 diff --git a/.github/actions/plugin-check-report/ratchet.sh b/.github/actions/plugin-check-report/ratchet.sh new file mode 100755 index 0000000..be260c2 --- /dev/null +++ b/.github/actions/plugin-check-report/ratchet.sh @@ -0,0 +1,286 @@ +#!/usr/bin/env bash +# +# Plugin Check ratchet. Counts ERROR and WARNING results per rule code and compares them with a +# baseline: the counts of an earlier run, fetched with fetch-baseline.sh. Only a rule with more +# ERRORS than the baseline fails. More warnings only add a notice. Fewer of either passes. With no +# baseline at all, nothing is compared and the check passes with a warning. In strict mode any +# error fails, baseline or not. See this action's README. +# +# Usage: +# ratchet.sh [--comment-file ] compare the results with the baseline +# ratchet.sh --update write the baseline from the results +# +# is the output of `wp plugin check --format=json`: for each file, a +# "FILE: " line and then a line holding a JSON array of that file's results. +# +# --comment-file writes a short Markdown summary for the pull request comment. The same summary +# goes to the job summary when GITHUB_STEP_SUMMARY is set. +# +# Environment (all optional): +# PLUGIN_CHECK_MODE "ratchet" (default): fail only when a rule has more errors than the +# baseline. "strict": fail when there is any error at all. +# COMMENT_MARKER hidden first line of the comment (default: "") +# PLUGIN_CHECK_BASELINE baseline path (default: baseline.json next to this script, where +# fetch-baseline.sh puts it) +# BASELINE_LABEL what the baseline is, for the comment's column (default: "Baseline") +# BASELINE_URL link to the run the baseline came from +# PLUGIN_CHECK_ADVISORY "true" when the baseline is from an older commit than the one the pull +# request targets. Rules with more errors are then reported as warnings +# and do not fail the check: the target may have changed since. +# COMMENT_BADGE image URL shown under the status line +# RUN_URL link to the workflow run +# ARTIFACT_URL link to the uploaded results and fresh baseline + +set -euo pipefail + +# The pull request comment must stay short. Everything else is behind the links. +MAX_COMMENT_LINES=15 +MAX_RULES_LISTED=5 + +# Hidden line that marks the sticky comment. The workflow searches for it, so no other comment +# is ever replaced. The workflow passes the same value it searches for. +COMMENT_MARKER=${COMMENT_MARKER:-''} + +# Checked before anything else, so a typo in the caller never runs as the default. +mode=${PLUGIN_CHECK_MODE:-ratchet} +case "$mode" in + ratchet | strict) ;; + *) + echo "::error title=Plugin Check::Unknown mode '$mode'. Use 'ratchet' or 'strict'." + exit 1 + ;; +esac + +update=false +comment_file="" +while [ $# -gt 0 ]; do + case "$1" in + --update) update=true; shift ;; + --comment-file) comment_file=${2:?"--comment-file needs a path"}; shift 2 ;; + *) break ;; + esac +done + +results=${1:?"Usage: $0 [--update | --comment-file ] "} +baseline=${PLUGIN_CHECK_BASELINE:-"$(cd "$(dirname "$0")" && pwd)/baseline.json"} + +if [ ! -f "$results" ]; then + echo "::error title=Plugin Check::Results file not found: $results" + exit 1 +fi + +# A results file that holds anything else (an HTML database error, a PHP fatal) means the check +# itself broke. Without this guard that file would count as "no results" and pass. +unexpected=$({ grep -v -E '^(FILE: |\[|Success:)' "$results" || true; } | grep -v -E '^[[:space:]]*$' | head -n 1 | cut -c 1-200 || true) +if [ -n "$unexpected" ]; then + echo "::error title=Plugin Check::The results file is not Plugin Check output, so the check did not run. First unexpected line: $unexpected" + exit 1 +fi + +# A clean run prints "Success: ..." and a run with results prints "FILE: ..." lines. An empty +# file means the command failed with its error on stderr, which the results file does not hold. +if ! grep -q -E '^(FILE: |Success:)' "$results"; then + echo "::error title=Plugin Check::The results file has no Plugin Check output, so the check did not run." + exit 1 +fi + +# Only the JSON lines matter. grep exits 1 when there are none (a clean run), which is fine. +counts=$({ grep '^\[' "$results" || true; } | jq -s -S ' + def per_code($type): + map(select(.type == $type)) + | group_by(.code) + | map({ key: .[0].code, value: length }) + | from_entries; + (add // []) as $all + | { errors: ($all | per_code("ERROR")), warnings: ($all | per_code("WARNING")) } +') + +if [ "$update" = true ]; then + printf '%s\n' "$counts" > "$baseline" + echo "Wrote $baseline: $(jq '.errors | length' <<< "$counts") error codes, $(jq '.warnings | length' <<< "$counts") warning codes" + exit 0 +fi + +# No earlier run to compare with: report the counts and pass. Failing here would block every pull +# request whenever the target branch has no finished run (its first run, or expired artifacts). +if [ ! -f "$baseline" ]; then + echo "::warning title=Plugin Check::No baseline to compare with ($baseline), so nothing was compared. Run the workflow on the target branch to make one." + baseline=$(mktemp) + jq -n '{errors: {}, warnings: {}}' > "$baseline" + no_baseline=true +else + no_baseline=false +fi + +# One tab-separated row per kind and rule code found in either file: +# kind, code, baseline count, current count. Rules that went up come first, errors before +# warnings, biggest increase first, so the comment can show the top few. +rows=$(jq -r -n --argjson now "$counts" --slurpfile base "$baseline" ' + $base[0] as $base + | [ + ("errors", "warnings") as $kind + | ((($now[$kind] // {}) + ($base[$kind] // {})) | keys[]) as $code + | { kind: $kind, code: $code, base: ($base[$kind][$code] // 0), now: ($now[$kind][$code] // 0) } + ] + | sort_by((if .now > .base then 0 else 1 end), (if .kind == "errors" then 0 else 1 end), -(.now - .base)) + | .[] + | [.kind, .code, .base, .now] + | @tsv +') + +failed=false +errors_base=0 +errors_now=0 +warnings_base=0 +warnings_now=0 +went_down=0 +went_up=0 +errors_up=0 +up_list="" + +while IFS=$'\t' read -r kind code allowed found; do + [ -n "$code" ] || continue + + if [ "$kind" = errors ]; then + errors_base=$((errors_base + allowed)) + errors_now=$((errors_now + found)) + label="" + else + warnings_base=$((warnings_base + allowed)) + warnings_now=$((warnings_now + found)) + label=" warning" + fi + + # With no baseline, only the totals mean anything. + [ "$no_baseline" = false ] || continue + + if [ "$found" -gt "$allowed" ]; then + went_up=$((went_up + 1)) + if [ "$went_up" -le "$MAX_RULES_LISTED" ]; then + up_list+="${up_list:+, }\`$code\`$label (+$((found - allowed)))" + fi + if [ "$kind" = errors ]; then + errors_up=$((errors_up + 1)) + if [ "${PLUGIN_CHECK_ADVISORY:-}" = true ]; then + echo "::warning title=Plugin Check::$code has $found errors, an older run of the target branch has $allowed. Not enforced, because the target may have changed since." + else + failed=true + echo "::error title=Plugin Check::$code has $found errors, the baseline has $allowed. The full list is in the plugin-check-results artifact." + fi + else + echo "::notice title=Plugin Check::$code has $found warnings, the baseline has $allowed. Warnings do not fail the check." + fi + elif [ "$found" -lt "$allowed" ]; then + went_down=$((went_down + 1)) + echo "::notice title=Plugin Check::$code ($kind) went down from $allowed to $found." + fi +done <<< "$rows" + +# Strict mode ignores the baseline for pass or fail: any error fails. The counts and the table +# still compare with the baseline, so the comment shows what changed. +if [ "$mode" = strict ]; then + if [ "$errors_now" -gt 0 ]; then + failed=true + echo "::error title=Plugin Check::$errors_now errors found. Strict mode fails on any error." + else + failed=false + fi +fi + +# "+3", "-2" or "±0". +change() { + local diff=$(($1 - $2)) + if [ "$diff" -gt 0 ]; then echo "+$diff"; elif [ "$diff" -lt 0 ]; then echo "$diff"; else echo "±0"; fi +} + +# The totals can go down while a rule goes up, so the status names the rules, not the totals. +if [ "$mode" = strict ] && [ "$failed" = true ]; then + status="❌ Plugin Check failed: $errors_now errors, and strict mode allows none" +elif [ "$mode" = strict ]; then + status="✅ Plugin Check passed: no errors" +elif [ "$no_baseline" = true ]; then + status="⚠️ Plugin Check ran, but there was no baseline to compare with" +elif [ "$errors_up" -gt 0 ] && [ "${PLUGIN_CHECK_ADVISORY:-}" = true ]; then + status="⚠️ Plugin Check passed, not enforced: $errors_up rule(s) have more errors than an older run of the target branch" +elif [ "$failed" = true ]; then + if [ "$errors_up" -eq 1 ]; then + status="❌ Plugin Check failed: 1 rule has more errors than the baseline allows" + else + status="❌ Plugin Check failed: $errors_up rules have more errors than the baseline allows" + fi +else + status="✅ Plugin Check passed" +fi + +run_url=${RUN_URL:-} +if [ -z "$run_url" ] && [ -n "${GITHUB_RUN_ID:-}" ]; then + run_url="${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}" +fi +if [ -n "${ARTIFACT_URL:-}" ]; then + links="📎 [Full results and new baseline](${ARTIFACT_URL})" + [ -z "$run_url" ] || links+=" · [Run]($run_url)" +elif [ -n "$run_url" ]; then + links="📎 [Run]($run_url)" +else + links="📎 Local run, see $results" +fi + +body="$COMMENT_MARKER"$'\n'"$status"$'\n\n' +if [ -n "${COMMENT_BADGE:-}" ]; then + body+="![Plugin Check finished](${COMMENT_BADGE})"$'\n\n' +fi +# The column is named after where the baseline came from ("Target branch", "Previous run"), and +# links to that run when known. +column=${BASELINE_LABEL:-Baseline} +if [ -n "${BASELINE_URL:-}" ]; then + column="[$column](${BASELINE_URL})" +fi +if [ "$no_baseline" = true ]; then + body+="| | Now |"$'\n' + body+="|:--|--:|"$'\n' + body+="| Errors | $errors_now |"$'\n' + body+="| Warnings | $warnings_now |"$'\n\n' +else + body+="| | Now | $column | Change |"$'\n' + body+="|:--|--:|--:|:--|"$'\n' + body+="| Errors | $errors_now | $errors_base | $(change "$errors_now" "$errors_base") |"$'\n' + body+="| Warnings | $warnings_now | $warnings_base | $(change "$warnings_now" "$warnings_base") |"$'\n\n' +fi +if [ "$went_up" -gt "$MAX_RULES_LISTED" ]; then + body+="Went up (top $MAX_RULES_LISTED of $went_up): $up_list"$'\n' +elif [ "$went_up" -gt 0 ]; then + body+="Went up: $up_list"$'\n' +fi +if [ "$went_down" -gt 0 ]; then + body+="Rules that went down: $went_down."$'\n' +fi +# A blank line separates the notes above from the links, when there are notes. +if [ "$went_up" -gt 0 ] || [ "$went_down" -gt 0 ]; then + body+=$'\n' +fi +body+="$links" + +# Hard cap: whatever happens above, the comment never grows past MAX_COMMENT_LINES. +body=$(head -n "$MAX_COMMENT_LINES" <<< "$body") + +if [ -n "$comment_file" ]; then + printf '%s\n' "$body" > "$comment_file" +fi +if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then + printf '%s\n' "$body" >> "$GITHUB_STEP_SUMMARY" +fi + +if [ "$failed" = true ]; then + if [ "$mode" = strict ]; then + echo "Plugin Check found errors, and strict mode allows none." + else + echo "Plugin Check found more errors than the baseline allows." + fi + exit 1 +fi + +if [ "$no_baseline" = true ]; then + echo "Plugin Check ran with no baseline, so nothing was compared." +else + echo "Plugin Check error counts are within the baseline." +fi diff --git a/.github/workflows/plugin-check.yml b/.github/workflows/plugin-check.yml new file mode 100644 index 0000000..1a73db4 --- /dev/null +++ b/.github/workflows/plugin-check.yml @@ -0,0 +1,443 @@ +name: Plugin Check + +# Runs WordPress.org's Plugin Check against the built plugin: the same files the release zip ships, +# so tests, dotfiles, node_modules and other dev-only files cannot cause false positives. +# +# A reusable workflow. A plugin repository calls it from a thin workflow of its own; copy +# templates/workflows/plugin-check.yml. +# +# The plugin is built with pup by default (`composer -- pup build`, then `pup package`), so the +# repository's .puprc and .distignore decide what is checked, the same as the release. A repository +# without pup passes `build-command` instead; see the plugin-check-report README. +# +# Three jobs, so the third-party action never holds a token that can write: +# start posts "in progress" on the pull request. +# check builds the plugin and runs wordpress/plugin-check-action. Read-only token. +# report compares the results with the baseline and replaces the comment with the result. +# +# The action also tries to post its own comment. With a read-only token that call fails (it logs +# "Failed to post PR comment" and carries on), so the only comment is ours. +# +# The caller has to grant every permission a job below asks for: a called workflow can lower the +# token's permissions, never raise them, and GitHub refuses to start the run when a job asks for +# more than the caller gave. `concurrency` is the caller's too: here it would be evaluated with the +# caller's context and could clash with the caller's own group. +# +# Run it locally: see "Running it locally" in .github/actions/plugin-check-report/README.md. In +# short, `wp plugin check --format=json > results.txt` in a local WordPress, then +# `bash .github/actions/plugin-check-report/ratchet.sh results.txt`. + +on: + workflow_call: + inputs: + plugin-slug: + description: >- + The plugin's folder name, e.g. `my-plugin`. It has to equal the top folder of the zip pup + builds (`zip_name` in .puprc), because Plugin Check takes the slug from the folder name. + required: true + type: string + main-file: + description: 'The main plugin file inside the plugin folder. Empty means `.php`.' + required: false + type: string + default: '' + build-command: + description: >- + Empty (the default) builds with pup. Otherwise a bash command that runs in the root of + the checkout instead of pup, and leaves the plugin as it ships in + "$PLUGIN_CHECK_BUILD_DIR//". Prefer .puprc when the repository has one: this + is a second place that defines the build. + required: false + type: string + default: '' + runs-on: + description: 'Runner label for the job that builds and checks. The two small jobs run on ubuntu-latest.' + required: false + type: string + default: 'ubuntu-latest' + php-version: + description: >- + PHP that builds vendor/. Use the oldest version the plugin supports, as its release does, + so Composer resolves dependencies that still run there. + required: false + type: string + default: '7.4' + exclude-checks: + description: 'Checks to skip, one per line. Passed to wordpress/plugin-check-action as it is.' + required: false + type: string + default: '' + cache-paths: + description: >- + Build caches to keep between runs, one path per line, relative to the repository root, + e.g. `.cache/webpack`. Empty keeps none. A path inside node_modules does not survive: + pup's build reinstalls it. + required: false + type: string + default: '' + mode: + description: >- + `ratchet`: fail only when a rule has more errors than the baseline. `strict`: fail on any + error. Not called `strict` because wordpress/plugin-check-action has a `strict` input + that means something else. + required: false + type: string + default: 'ratchet' + submodules: + description: 'Passed to actions/checkout: `false`, `true` or `recursive`.' + required: false + type: string + default: 'false' + secrets: + composer-token: + description: >- + GitHub token for private Composer packages. Becomes COMPOSER_AUTH for the build step + only. + required: false + package-token: + description: >- + Token for private npm or bun packages. Becomes NODE_AUTH_TOKEN, BUN_AUTH_TOKEN and + GITHUB_TOKEN for the build step only. + required: false + +env: + # Hidden line that marks the sticky pull request comment. The report action writes it as the + # first line of the body, and both comment steps search for it. Named after the repository, so + # two plugin repositories never share a marker. + COMMENT_MARKER: '' + # Only pull requests from this repository get a comment: a fork's token is read-only. + COMMENT: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository }} + +jobs: + start: + name: Plugin Check start + if: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository }} + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + issues: write + pull-requests: write + steps: + # One comment per pull request, found by a hidden marker line and replaced on every run. The + # badge time is Eastern Time: %Z prints EST in winter and EDT in summer. + - name: Make the start badge + id: badge + run: | + stamp=$(LC_ALL=C TZ='America/New_York' date '+%m/%d %I:%M %p %Z') + stamp=${stamp// /%20} + stamp=${stamp//\//%2F} + echo "url=https://img.shields.io/badge/Check%20Started-${stamp}-blue" >> "$GITHUB_OUTPUT" + + - name: Find the Plugin Check comment + id: find_comment + continue-on-error: true + uses: peter-evans/find-comment@v4 + with: + issue-number: ${{ github.event.pull_request.number }} + comment-author: 'github-actions[bot]' + body-includes: ${{ env.COMMENT_MARKER }} + + - name: Post "in progress" + continue-on-error: true + uses: peter-evans/create-or-update-comment@v5 + with: + comment-id: ${{ steps.find_comment.outputs.comment-id }} + issue-number: ${{ github.event.pull_request.number }} + edit-mode: replace + body: | + ${{ env.COMMENT_MARKER }} + ⏳ Plugin Check in progress + + ![Plugin Check started](${{ steps.badge.outputs.url }}) + + check: + name: Plugin Check run + runs-on: ${{ inputs.runs-on }} + + # Build, WordPress start and the check together take about 15 minutes for a large plugin. The cap is + # for a container that stops answering. + timeout-minutes: 30 + + # Read-only on purpose: this job runs a third-party action. Its own attempt to post a comment + # fails with this token, which is what keeps the pull request to one comment. + permissions: + contents: read + + env: + PLUGIN_CHECK_BUILD_DIR: ${{ github.workspace }}/build + SLUG: ${{ inputs.plugin-slug }} + MAIN_FILE: ${{ inputs.main-file || format('{0}.php', inputs.plugin-slug) }} + + steps: + # The repo goes into source/, so the workspace root has no .wp-env.json. A repo's own file + # mounts the repo root as a plugin, and the action would merge into it. + - uses: actions/checkout@v7 + with: + path: source + submodules: ${{ inputs.submodules }} + # Nothing here pushes over git, and the job goes on to run third-party code. + persist-credentials: false + + # The action's wp-env pulls these images when it starts, and pulling them is part of the + # time that step takes. Started now, in the background, the download overlaps with the + # build. The images are the ones wp-env's generated files name (FROM wordpress..., FROM + # wordpress:cli..., image: mariadb:lts); if a later wp-env names others, these pulls are just + # wasted, never wrong. A failed pull only means wp-env pulls the image itself, so this step + # cannot fail the job. + - name: Start pulling the wp-env Docker images + continue-on-error: true + env: + IMAGES: wordpress wordpress:cli mariadb:lts + run: | + ( + set +e + start=$(date +%s) + for image in $IMAGES; do + docker pull --quiet "$image" || echo "Could not pull $image, wp-env will pull it." & + done + wait + echo "Pulled in $(( $(date +%s) - start )) seconds." + touch "$RUNNER_TEMP/docker-pull.done" + ) > "$RUNNER_TEMP/docker-pull.log" 2>&1 < /dev/null & + + - name: Set Up PHP + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ inputs.php-version }} + coverage: none + + - name: Find the Composer cache + id: composer-cache + working-directory: source + run: echo "dir=$(composer config cache-files-dir)" >> "$GITHUB_OUTPUT" + + - name: Cache dependencies + uses: actions/cache@v6 + with: + path: ${{ steps.composer-cache.outputs.dir }} + key: composer-${{ hashFiles('source/composer.lock') }} + + # The JavaScript toolchain is picked from the files the repository already has, not from an + # input: .nvmrc for Node, package-lock.json for the npm cache, bun.lock or bun.lockb for bun. + - name: Set Up Node + if: ${{ hashFiles('source/.nvmrc') != '' }} + uses: actions/setup-node@v7 + with: + node-version-file: source/.nvmrc + cache: ${{ hashFiles('source/package-lock.json') != '' && 'npm' || '' }} + cache-dependency-path: source/package-lock.json + + - name: Set Up Bun + if: ${{ hashFiles('source/bun.lock', 'source/bun.lockb') != '' }} + uses: oven-sh/setup-bun@v2 + with: + bun-version: latest + + # actions/cache takes paths relative to the workspace, and the repository sits in source/. + # A path that leaves the checkout is refused: it would cache files this job did not build. + - name: List the cache paths + id: cache-paths + if: ${{ inputs.cache-paths != '' }} + env: + CACHE_PATHS: ${{ inputs.cache-paths }} + run: | + { + echo 'paths<> "$GITHUB_OUTPUT" + + # Each commit saves a new entry; the restore key picks the newest one for the same lockfile. + # A stale cache must be safe for the tool that owns it (webpack checks every entry against + # the files). + - name: Restore the build cache + id: build-cache + if: ${{ inputs.cache-paths != '' }} + uses: actions/cache/restore@v6 + with: + path: ${{ steps.cache-paths.outputs.paths }} + key: plugin-check-${{ runner.os }}-${{ hashFiles('source/package-lock.json', 'source/bun.lock', 'source/bun.lockb') }}-${{ github.sha }} + restore-keys: | + plugin-check-${{ runner.os }}-${{ hashFiles('source/package-lock.json', 'source/bun.lock', 'source/bun.lockb') }}- + + # The same pup steps the release zip uses, so .puprc and .distignore decide what is checked. + # The secrets reach this step only, never the job, so the third-party action later in this + # job does not see them in its environment. + - name: Build the plugin + working-directory: source + env: + BUILD_COMMAND: ${{ inputs.build-command }} + COMPOSER_TOKEN: ${{ secrets.composer-token }} + PACKAGE_TOKEN: ${{ secrets.package-token }} + run: | + if [ -n "$COMPOSER_TOKEN" ]; then + COMPOSER_AUTH=$(jq -cn --arg token "$COMPOSER_TOKEN" '{"github-oauth": {"github.com": $token}}') + export COMPOSER_AUTH + fi + if [ -n "$PACKAGE_TOKEN" ]; then + export NODE_AUTH_TOKEN="$PACKAGE_TOKEN" BUN_AUTH_TOKEN="$PACKAGE_TOKEN" GITHUB_TOKEN="$PACKAGE_TOKEN" + fi + unset COMPOSER_TOKEN PACKAGE_TOKEN + mkdir -p "$PLUGIN_CHECK_BUILD_DIR" + + if [ -n "$BUILD_COMMAND" ]; then + echo "Building with build-command." + bash -eo pipefail -c "$BUILD_COMMAND" + exit 0 + fi + + composer run -q pup > /dev/null + composer -- pup build + version=$(php -d display_errors=0 bin/pup.phar get-version --dev) + composer -- pup package "$version" + name=$(php -d display_errors=0 bin/pup.phar zip-name "$version") + unzip -q "$name.zip" -d "$PLUGIN_CHECK_BUILD_DIR" + + # The one contract every build has to meet, pup or not: a folder named after the slug, with + # the main plugin file in it. Plugin Check takes the slug from the folder name, so a wrong + # folder would check nothing useful and still look green. + - name: Check the built plugin + env: + CACHE_PATHS: ${{ inputs.cache-paths }} + run: | + dir="$PLUGIN_CHECK_BUILD_DIR/$SLUG" + if [ ! -f "$dir/$MAIN_FILE" ]; then + echo "::error title=Plugin Check::The build did not leave $SLUG/$MAIN_FILE in $PLUGIN_CHECK_BUILD_DIR. Found: $(find "$PLUGIN_CHECK_BUILD_DIR" -mindepth 1 -maxdepth 1 -printf '%f ')" + exit 1 + fi + # A cache that ends up in the plugin would be checked, and shipped by the release. + while IFS= read -r line; do + line=$(echo "$line" | xargs) + [ -n "$line" ] || continue + if [ -e "$dir/$line" ]; then + echo "::error title=Plugin Check::The cache path $line is inside the built plugin. Add it to .distignore." + exit 1 + fi + done <<< "$CACHE_PATHS" + echo "Checking $dir ($(find "$dir" -type f | wc -l | xargs) files)." + + # Saved now, before the third-party action below runs, so nothing it does can end up in a + # cache that later runs restore. Skipped when this exact commit already saved one. + - name: Save the build cache + if: ${{ inputs.cache-paths != '' && steps.build-cache.outputs.cache-hit != 'true' }} + uses: actions/cache/save@v6 + with: + path: ${{ steps.cache-paths.outputs.paths }} + key: ${{ steps.build-cache.outputs.cache-primary-key }} + + # Waits for the pulls started earlier, so wp-env does not start a second download of an + # image that is still coming, and prints how long they took. Gives up after two minutes and + # lets wp-env carry on; never fails the job. + - name: Wait for the Docker image pulls + continue-on-error: true + timeout-minutes: 3 + run: | + for _ in $(seq 120); do + [ -f "$RUNNER_TEMP/docker-pull.done" ] && break + sleep 1 + done + cat "$RUNNER_TEMP/docker-pull.log" 2>/dev/null || true + + # Fails on any error, which is expected while a plugin still has known problems: the report + # job decides pass or fail from the counts. The action writes its results to + # $RUNNER_TEMP/plugin-check-results.txt and uploads them as the plugin-check-results + # artifact. Its `slug` input is not passed: it would rename that artifact. + - name: Run Plugin Check + continue-on-error: true + uses: wordpress/plugin-check-action@v1 + with: + build-dir: ${{ github.workspace }}/build/${{ inputs.plugin-slug }} + exclude-checks: ${{ inputs.exclude-checks }} + + report: + name: Plugin Check + needs: [start, check] + # "start" is skipped for forks and non-pull-request runs, which would skip this job too. + if: ${{ !cancelled() }} + runs-on: ubuntu-latest + # Up to 20 minutes of it can be waiting for the target branch's run. + timeout-minutes: 30 + + # Writes one sticky comment on the pull request, and reads an earlier run's artifact as the + # baseline. No checkout: the scripts come with the action. + permissions: + actions: read + issues: write + pull-requests: write + + steps: + - name: Set file paths + run: | + { + echo "RESULTS=$RUNNER_TEMP/results/plugin-check-results.txt" + echo "COMMENT_BODY=$RUNNER_TEMP/plugin-check-comment.md" + } >> "$GITHUB_ENV" + + # The action uploaded its results under this name. Missing when the build or the check broke + # before then: the compare reports that. + - name: Download the results + continue-on-error: true + uses: actions/download-artifact@v8 + with: + name: plugin-check-results + path: ${{ runner.temp }}/results + + - name: Make the finish badge + id: finish_badge + run: | + stamp=$(LC_ALL=C TZ='America/New_York' date '+%m/%d %I:%M %p %Z') + stamp=${stamp// /%20} + stamp=${stamp//\//%2F} + echo "url=https://img.shields.io/badge/Check%20Finished-${stamp}-darkgreen" >> "$GITHUB_OUTPUT" + + # `$/` is this repository at the commit that runs this workflow, so the action is always the + # same revision as the workflow, whichever ref the caller used, and it needs no checkout. It + # needs a runner of version 2.336.0 or newer; this job runs on ubuntu-latest. Not ./, which + # in a called workflow resolves against the caller's repository. + - name: Compare with the baseline + uses: $/.github/actions/plugin-check-report + with: + results: ${{ env.RESULTS }} + comment-file: ${{ env.COMMENT_BODY }} + comment-marker: ${{ env.COMMENT_MARKER }} + mode: ${{ inputs.mode }} + badge-url: ${{ steps.finish_badge.outputs.url }} + + # When the build or the check broke, the action never wrote a body. Say so instead of leaving + # "in progress" behind. + - name: Prepare the comment + if: ${{ !cancelled() && env.COMMENT == 'true' }} + env: + BADGE: ${{ steps.finish_badge.outputs.url }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: | + if [ ! -s "$COMMENT_BODY" ]; then + printf '%s\n❌ Plugin Check could not run\n\n![Plugin Check finished](%s)\n\n📎 [Run](%s)\n' "$COMMENT_MARKER" "$BADGE" "$RUN_URL" > "$COMMENT_BODY" + fi + + - name: Find the Plugin Check comment + id: find_comment + if: ${{ !cancelled() && env.COMMENT == 'true' }} + continue-on-error: true + uses: peter-evans/find-comment@v4 + with: + issue-number: ${{ github.event.pull_request.number }} + comment-author: 'github-actions[bot]' + body-includes: ${{ env.COMMENT_MARKER }} + + - name: Post the result + if: ${{ !cancelled() && env.COMMENT == 'true' }} + continue-on-error: true + uses: peter-evans/create-or-update-comment@v5 + with: + comment-id: ${{ steps.find_comment.outputs.comment-id }} + issue-number: ${{ github.event.pull_request.number }} + edit-mode: replace + body-path: ${{ env.COMMENT_BODY }} diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 0450a2e..08c2003 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -3,7 +3,8 @@ name: Tests # This repo's own checks. A bug here reaches every repo that pins a tag from it, so the logic in the # actions is unit tested rather than only exercised by a real release. # -# Run it locally: `npm install` once, then `node --test`. +# Run it locally: `npm install` once, then `node --test`. For the lint job, `actionlint` and +# `shellcheck .github/actions/*/*.sh`. on: push: @@ -40,3 +41,30 @@ jobs: # finds every *.test.js under the repository on all of them. - name: Run the unit tests run: node --test + + # The workflows and templates never run in this repository the way a plugin runs them, so a typo + # in one would only show up in someone else's CI. actionlint also runs shellcheck on every run: + # block it finds. + lint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + with: + persist-credentials: false + + # Pinned, so a new actionlint rule cannot fail a pull request that did not change anything. + # Two things are ignored because GitHub accepts them and actionlint 1.7.12 does not know them + # yet: `queue` in `concurrency` (release-tags.yml uses it), and a `uses: $/path` reference to + # this repository (the reusable workflows use it for their own actions). Only that exact shape + # is ignored, so any other action reference that lacks a ref still fails. + - name: Run actionlint + run: | + bash <(curl -sSfL https://raw.githubusercontent.com/rhysd/actionlint/v1.7.12/scripts/download-actionlint.bash) 1.7.12 + ./actionlint -ignore 'unexpected key "queue" for "concurrency" section' \ + -ignore 'specifying action "\$/[^"]*" in invalid format because ref is missing' \ + .github/workflows/*.yml templates/workflows/*.yml + + # actionlint does not read composite actions or standalone scripts. shellcheck comes with the + # runner image. + - name: Run shellcheck on the action scripts + run: shellcheck .github/actions/*/*.sh diff --git a/.gitignore b/.gitignore index da63dc1..0dbc1bd 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,6 @@ npm-shrinkwrap.json bun.lock bun.lockb yarn.lock + +# fetch-baseline.sh writes the baseline here when run by hand (see the plugin-check-report README). +.github/actions/plugin-check-report/baseline.json diff --git a/templates/workflows/plugin-check.yml b/templates/workflows/plugin-check.yml new file mode 100644 index 0000000..987d216 --- /dev/null +++ b/templates/workflows/plugin-check.yml @@ -0,0 +1,40 @@ +name: Plugin Check + +# Runs WordPress.org's Plugin Check on the plugin as it ships, and compares the error counts with +# an earlier run: a pull request with the branch it targets, a push with the run before it. +# +# A file to copy into .github/workflows/ of a plugin repository. Keep the file name +# plugin-check.yml: earlier runs of this file hold the baselines, and a new name starts with none. +# The work is in stellarwp/plugin-toolbox/.github/workflows/plugin-check.yml. The repository needs +# a .puprc whose zip_name equals plugin-slug, and a `pup` Composer script; see the +# plugin-check-report action's README. +# +# Run it locally: `wp plugin check --format=json > results.txt` in a local WordPress, then +# the ratchet script on that file. See "Running it locally" in the action's README. + +on: + pull_request: + # The branches pull requests target. A push there makes the baseline those pull requests read. + push: + branches: [main] + workflow_dispatch: + +# A new push to a pull request cancels the run still going for the older commit. +concurrency: + group: plugin-check-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + plugin-check: + # The most any job of the shared workflow asks for. A called workflow cannot go above this. + permissions: + actions: read # read earlier runs and download their baseline + contents: read # check out the repository + issues: write # the sticky pull request comment + pull-requests: write # the sticky pull request comment + uses: stellarwp/plugin-toolbox/.github/workflows/plugin-check.yml@main + with: + plugin-slug: my-plugin diff --git a/tests/actions/plugin-check-report/fetch-baseline.test.js b/tests/actions/plugin-check-report/fetch-baseline.test.js new file mode 100644 index 0000000..d8eae06 --- /dev/null +++ b/tests/actions/plugin-check-report/fetch-baseline.test.js @@ -0,0 +1,379 @@ +'use strict' + +const { describe, it } = require('node:test') +const assert = require('node:assert') +const fs = require('node:fs') +const os = require('node:os') +const path = require('node:path') +const { spawnSync } = require('node:child_process') + +const FETCH = path.join(__dirname, '../../../.github/actions/plugin-check-report/fetch-baseline.sh') + +const REPO = 'example-org/my-plugin' +const SHA = 'a'.repeat(40) +const OLD_SHA = 'b'.repeat(40) + +/** + * A stand-in for the GitHub CLI, put first on PATH. + * + * `gh api --jq ` answers from a JSON file named after the path, and applies the + * filter with the real jq, so the script's own filters run as they would against GitHub. A path + * with no file answers the way gh does for a 404: a message on stderr and exit 1. That is how a + * test makes the lookup fail. + * + * `gh run download --dir ` copies the folder artifact-, standing in for the + * plugin-check-report artifact of that run. + * + * Every call is appended to a log, so a test can assert which requests were sent. + */ +const FAKE_GH = `#!/usr/bin/env bash +printf '%s\\n' "$*" >> "$FAKE_GH_LOG" +if [ "$1" = api ]; then + key=$(printf '%s' "$2" | tr -c 'A-Za-z0-9' '_') + file="$FAKE_GH_DIR/$key.json" + if [ ! -f "$file" ]; then + echo "gh: Not Found (HTTP 404)" >&2 + exit 1 + fi + if [ "$3" = --jq ]; then + jq -r "$4" "$file" + else + cat "$file" + fi +elif [ "$1" = run ] && [ "$2" = download ]; then + id=$3 + while [ $# -gt 0 ]; do + [ "$1" = --dir ] && dir=$2 + shift + done + [ -d "$FAKE_GH_DIR/artifact-$id" ] || { echo "no artifact for run $id" >&2; exit 1; } + mkdir -p "$dir" + cp -R "$FAKE_GH_DIR/artifact-$id/." "$dir/" +else + echo "fake gh: no answer for: $*" >&2 + exit 1 +fi +` + +/** + * The file name the stand-in reads for an API path. + * + * @param {string} apiPath The path as the script passes it to `gh api`. + * + * @returns {string} The file name, every character other than a letter or digit made `_`. + */ +function keyFor(apiPath) { + return `${apiPath.replace(/[^A-Za-z0-9]/g, '_')}.json` +} + +/** + * The API paths fetch-baseline.sh asks for. + * + * @param {string} workflow The workflow file name. + */ +const API = (workflow = 'plugin-check.yml') => { + const runs = `repos/${REPO}/actions/workflows/${workflow}/runs` + + return { + forSha: (sha) => `${runs}?status=completed&head_sha=${sha}&per_page=10`, + goingForSha: (sha) => `${runs}?head_sha=${sha}&per_page=20`, + onBranch: (branch) => `${runs}?status=completed&branch=${branch}&per_page=100`, + artifacts: (id) => `repos/${REPO}/actions/runs/${id}/artifacts?name=plugin-check-report`, + } +} + +/** + * A run as the workflow runs API lists it. + * + * @param {number} id Run id. + * @param {string} sha Head commit. + * @param {string} headRepo Repository the run's head commit belongs to. A pull request from a + * fork has the fork's name here. + * + * @returns {object} The run. + */ +function run(id, sha = OLD_SHA, headRepo = REPO) { + return { + id, + html_url: `https://github.com/${REPO}/actions/runs/${id}`, + head_sha: sha, + status: 'completed', + head_repository: { full_name: headRepo }, + } +} + +/** + * Sets up the stand-in's answers and runs fetch-baseline.sh against them. + * + * @param {string[]} args Arguments for the script. + * @param {object} answers Each API path mapped to the JSON body it answers with. + * @param {object} baselines Each run id mapped to the baseline.json its artifact holds. + * + * @returns {{status: number, stdout: string, stderr: string, calls: string[], output: string}} + * The exit status, both streams, the gh calls made and the baseline file it wrote. + */ +function fetchBaseline(args, answers = {}, baselines = {}) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'plugin-check-fetch-')) + const bin = path.join(dir, 'bin') + const data = path.join(dir, 'data') + const log = path.join(dir, 'gh.log') + const output = path.join(dir, 'out', 'baseline.json') + + fs.mkdirSync(bin) + fs.mkdirSync(data) + fs.writeFileSync(path.join(bin, 'gh'), FAKE_GH, { mode: 0o755 }) + fs.writeFileSync(log, '') + + for (const [apiPath, body] of Object.entries(answers)) { + fs.writeFileSync(path.join(data, keyFor(apiPath)), JSON.stringify(body)) + } + for (const [id, baseline] of Object.entries(baselines)) { + fs.mkdirSync(path.join(data, `artifact-${id}`)) + fs.writeFileSync(path.join(data, `artifact-${id}`, 'baseline.json'), JSON.stringify(baseline)) + } + + const result = spawnSync('bash', [FETCH, ...args, output], { + encoding: 'utf8', + env: { + PATH: `${bin}:${process.env.PATH}`, + HOME: process.env.HOME, + GITHUB_REPOSITORY: REPO, + FAKE_GH_DIR: data, + FAKE_GH_LOG: log, + }, + }) + + return { + status: result.status, + stdout: result.stdout.trim(), + stderr: result.stderr, + calls: fs.readFileSync(log, 'utf8').trim().split('\n').filter(Boolean), + output: fs.existsSync(output) ? fs.readFileSync(output, 'utf8') : '', + } +} + +const BASELINE = { errors: { 'Some.Rule': 2 }, warnings: {} } +const ONE_ARTIFACT = { artifacts: [{ name: 'plugin-check-report', expired: false }] } +const EXPIRED = { artifacts: [{ name: 'plugin-check-report', expired: true }] } + +describe('fetch-baseline.sh', () => { + it('takes the run for the base commit and calls it exact', () => { + const api = API() + const result = fetchBaseline(['--sha', SHA, 'develop'], { + [api.forSha(SHA)]: { workflow_runs: [run(200, SHA)] }, + [api.onBranch('develop')]: { workflow_runs: [run(150)] }, + [api.artifacts(200)]: ONE_ARTIFACT, + }, { 200: BASELINE }) + + assert.equal(result.status, 0, result.stderr) + assert.equal(result.stdout, `200 https://github.com/${REPO}/actions/runs/200 exact`) + assert.deepEqual(JSON.parse(result.output), BASELINE) + }) + + it('falls back to an older run on the branch, skips an expired artifact, and calls it older', () => { + const api = API() + const result = fetchBaseline(['--sha', SHA, 'develop'], { + [api.forSha(SHA)]: { workflow_runs: [] }, + [api.onBranch('develop')]: { workflow_runs: [run(120), run(150)] }, + [api.artifacts(150)]: EXPIRED, + [api.artifacts(120)]: ONE_ARTIFACT, + }, { 120: BASELINE }) + + assert.equal(result.status, 0, result.stderr) + assert.equal(result.stdout, `120 https://github.com/${REPO}/actions/runs/120 older`) + // Newest first by run id, whatever order the API answered in. + assert.ok( + result.calls.findIndex((call) => call.includes('runs/150/')) < + result.calls.findIndex((call) => call.includes('runs/120/')) + ) + }) + + it('never compares a run with itself or with a newer run', () => { + const api = API() + const result = fetchBaseline(['--before-run', '150', 'develop'], { + [api.onBranch('develop')]: { workflow_runs: [run(160), run(150), run(140)] }, + [api.artifacts(140)]: ONE_ARTIFACT, + }, { 140: BASELINE }) + + assert.equal(result.status, 0, result.stderr) + assert.match(result.stdout, /^140 /) + assert.ok(!result.calls.some((call) => /runs\/1[56]0\//.test(call))) + }) + + it('does not wait for a run for the base commit that has already finished', () => { + const api = API() + const result = fetchBaseline(['--sha', SHA, '--wait', '1200', 'develop'], { + [api.goingForSha(SHA)]: { workflow_runs: [run(200, SHA)] }, + [api.forSha(SHA)]: { workflow_runs: [run(200, SHA)] }, + [api.onBranch('develop')]: { workflow_runs: [] }, + [api.artifacts(200)]: ONE_ARTIFACT, + }, { 200: BASELINE }) + + assert.equal(result.status, 0, result.stderr) + assert.match(result.stdout, /^200 .* exact$/) + }) + + it('exits 1 when no run has a baseline', () => { + const api = API() + const result = fetchBaseline(['develop'], { + [api.onBranch('develop')]: { workflow_runs: [run(150)] }, + [api.artifacts(150)]: { artifacts: [] }, + }) + + assert.equal(result.status, 1) + assert.match(result.stderr, /No Plugin Check run on develop has a baseline/) + assert.equal(result.output, '') + }) + + it('exits 2, not 1, when the lookup itself fails', () => { + // No answer for the branch's runs: the stand-in fails the way gh does on an API error. + const result = fetchBaseline(['develop'], {}) + + assert.equal(result.status, 2) + assert.match(result.stderr, /Could not look up Plugin Check runs/) + }) + + it('exits 2 when the download fails', () => { + const api = API() + const result = fetchBaseline(['develop'], { + [api.onBranch('develop')]: { workflow_runs: [run(150)] }, + [api.artifacts(150)]: ONE_ARTIFACT, + }) + + assert.equal(result.status, 2) + assert.match(result.stderr, /download of the artifact of run 150/) + }) + + it('reads the runs of plugin-check.yml by default', () => { + const result = fetchBaseline(['develop'], {}) + + assert.ok(result.calls[0].includes('/workflows/plugin-check.yml/runs'), result.calls[0]) + }) + + it('reads the runs of the workflow file --workflow names', () => { + const api = API('wporg-check.yml') + const result = fetchBaseline(['--workflow', 'wporg-check.yml', 'develop'], { + [api.onBranch('develop')]: { workflow_runs: [run(150)] }, + [api.artifacts(150)]: ONE_ARTIFACT, + }, { 150: BASELINE }) + + assert.equal(result.status, 0, result.stderr) + assert.ok(result.calls[0].includes('/workflows/wporg-check.yml/runs'), result.calls[0]) + }) + + it('ignores a run whose head commit is in a fork, even on a branch named like the target', () => { + const api = API() + const result = fetchBaseline(['--sha', SHA, 'develop'], { + // A fork's pull request run for the base commit, and a newer one on a fork branch "develop". + [api.forSha(SHA)]: { workflow_runs: [run(300, SHA, 'someone/my-plugin')] }, + [api.onBranch('develop')]: { workflow_runs: [run(250, OLD_SHA, 'someone/my-plugin'), run(150)] }, + [api.artifacts(150)]: ONE_ARTIFACT, + [api.artifacts(250)]: ONE_ARTIFACT, + [api.artifacts(300)]: ONE_ARTIFACT, + }, { 150: BASELINE, 250: { errors: { 'Some.Rule': 9999 }, warnings: {} }, 300: { errors: { 'Some.Rule': 9999 }, warnings: {} } }) + + assert.equal(result.status, 0, result.stderr) + assert.match(result.stdout, /^150 .* older$/) + assert.deepEqual(JSON.parse(result.output), BASELINE) + assert.ok(!result.calls.some((call) => call.includes('/runs/250/') || call.includes('/runs/300/')), 'no fork run was looked at') + }) + + it('exits 1 when only fork runs exist', () => { + const api = API() + const result = fetchBaseline(['develop'], { + [api.onBranch('develop')]: { workflow_runs: [run(250, OLD_SHA, 'someone/my-plugin')] }, + }) + + assert.equal(result.status, 1) + assert.match(result.stderr, /has a baseline/) + }) + + it('matches the repository name without regard to case', () => { + const api = API() + const result = fetchBaseline(['develop'], { + [api.onBranch('develop')]: { workflow_runs: [run(150, OLD_SHA, 'Example-Org/My-Plugin')] }, + [api.artifacts(150)]: ONE_ARTIFACT, + }, { 150: BASELINE }) + + assert.equal(result.status, 0, result.stderr) + }) + + it('does not wait for a fork run that is still going', () => { + const api = API() + const result = fetchBaseline(['--sha', SHA, '--wait', '1', 'develop'], { + // Our run for the commit is done, and a fork's run for the same commit is not. Counted, the + // fork's run would make the script sleep 30 seconds and then give up waiting. + [api.goingForSha(SHA)]: { + workflow_runs: [run(200, SHA), { ...run(300, SHA, 'someone/my-plugin'), status: 'in_progress' }], + }, + [api.forSha(SHA)]: { workflow_runs: [run(200, SHA)] }, + [api.onBranch('develop')]: { workflow_runs: [] }, + [api.artifacts(200)]: ONE_ARTIFACT, + }, { 200: BASELINE }) + + assert.equal(result.status, 0, result.stderr) + assert.match(result.stdout, /^200 .* exact$/) + assert.doesNotMatch(result.stderr, /still going/) + }) + + it('exits 2, not 1, for an option value that is empty or not a whole number', () => { + const cases = [ + [['--wait', '20m', 'develop'], /--wait must be a whole number, not '20m'/], + [['--wait', '', 'develop'], /--wait needs a value/], + [['--before-run', 'abc', 'develop'], /--before-run must be a whole number, not 'abc'/], + [['--before-run', '-5', 'develop'], /--before-run must be a whole number/], + [['--before-run', '9'.repeat(19), 'develop'], /--before-run is too large/], + [['--sha', '', 'develop'], /--sha needs a value/], + [['', 'baseline.json'], /a branch is needed/], + ] + + for (const [args, message] of cases) { + const result = fetchBaseline(args, {}) + + assert.equal(result.status, 2, args.join(' ')) + assert.match(result.stderr, message, args.join(' ')) + assert.deepEqual(result.calls, [], `${args.join(' ')} asked the API nothing`) + } + }) + + it('exits 2 when an option or the branch is missing altogether', () => { + // fetchBaseline() always adds an output path after the arguments, which a flag at the end of + // the line would take for its value, so these run the script as it is. + const cases = [ + [['--wait'], /--wait needs a value/], + [['--workflow'], /--workflow needs a value/], + [[], /a branch is needed/], + ] + + for (const [args, message] of cases) { + const result = spawnSync('bash', [FETCH, ...args], { + encoding: 'utf8', + env: { PATH: process.env.PATH, HOME: process.env.HOME, GITHUB_REPOSITORY: REPO }, + }) + + assert.equal(result.status, 2, args.join(' ')) + assert.match(result.stderr, message, args.join(' ')) + } + }) + + it('reads a run id with a leading zero as a decimal number', () => { + const api = API() + const result = fetchBaseline(['--before-run', '0150', 'develop'], { + [api.onBranch('develop')]: { workflow_runs: [run(150), run(120)] }, + [api.artifacts(120)]: ONE_ARTIFACT, + }, { 120: BASELINE }) + + assert.equal(result.status, 0, result.stderr) + assert.match(result.stdout, /^120 /) + }) + + it('refuses a --workflow that is a path or a ref, before calling the API', () => { + for (const workflow of ['.github/workflows/plugin-check.yml', 'plugin-check.yml@refs/heads/develop']) { + const result = fetchBaseline(['--workflow', workflow, 'develop'], {}) + + assert.equal(result.status, 2, workflow) + assert.match(result.stderr, /--workflow must be a file name/) + assert.deepEqual(result.calls, []) + } + }) +}) diff --git a/tests/actions/plugin-check-report/fixtures/baseline.json b/tests/actions/plugin-check-report/fixtures/baseline.json new file mode 100644 index 0000000..532809e --- /dev/null +++ b/tests/actions/plugin-check-report/fixtures/baseline.json @@ -0,0 +1,9 @@ +{ + "errors": { + "WordPress.Security.EscapeOutput.OutputNotEscaped": 2, + "WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents": 1 + }, + "warnings": { + "WordPress.DB.DirectDatabaseQuery.DirectQuery": 1 + } +} diff --git a/tests/actions/plugin-check-report/fixtures/clean.txt b/tests/actions/plugin-check-report/fixtures/clean.txt new file mode 100644 index 0000000..ad6c4f9 --- /dev/null +++ b/tests/actions/plugin-check-report/fixtures/clean.txt @@ -0,0 +1 @@ +Success: Checks complete. No errors found. diff --git a/tests/actions/plugin-check-report/fixtures/empty.txt b/tests/actions/plugin-check-report/fixtures/empty.txt new file mode 100644 index 0000000..e69de29 diff --git a/tests/actions/plugin-check-report/fixtures/errors-down.txt b/tests/actions/plugin-check-report/fixtures/errors-down.txt new file mode 100644 index 0000000..fe81472 --- /dev/null +++ b/tests/actions/plugin-check-report/fixtures/errors-down.txt @@ -0,0 +1,4 @@ +FILE: my-plugin.php +[{"line":12,"column":10,"type":"ERROR","code":"WordPress.Security.EscapeOutput.OutputNotEscaped","message":"All output should be run through an escaping function.","docs":""}] +FILE: src/Forms/Form.php +[{"line":40,"column":5,"type":"ERROR","code":"WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents","message":"file_get_contents() is discouraged.","docs":""},{"line":88,"column":9,"type":"WARNING","code":"WordPress.DB.DirectDatabaseQuery.DirectQuery","message":"Use of a direct database call is discouraged.","docs":""}] diff --git a/tests/actions/plugin-check-report/fixtures/errors-up.txt b/tests/actions/plugin-check-report/fixtures/errors-up.txt new file mode 100644 index 0000000..49ae2cc --- /dev/null +++ b/tests/actions/plugin-check-report/fixtures/errors-up.txt @@ -0,0 +1,4 @@ +FILE: my-plugin.php +[{"line":12,"column":10,"type":"ERROR","code":"WordPress.Security.EscapeOutput.OutputNotEscaped","message":"All output should be run through an escaping function.","docs":""},{"line":12,"column":10,"type":"ERROR","code":"WordPress.Security.EscapeOutput.OutputNotEscaped","message":"All output should be run through an escaping function.","docs":""},{"line":12,"column":10,"type":"ERROR","code":"WordPress.Security.EscapeOutput.OutputNotEscaped","message":"All output should be run through an escaping function.","docs":""}] +FILE: src/Forms/Form.php +[{"line":40,"column":5,"type":"ERROR","code":"WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents","message":"file_get_contents() is discouraged.","docs":""},{"line":88,"column":9,"type":"WARNING","code":"WordPress.DB.DirectDatabaseQuery.DirectQuery","message":"Use of a direct database call is discouraged.","docs":""}] diff --git a/tests/actions/plugin-check-report/fixtures/garbage.txt b/tests/actions/plugin-check-report/fixtures/garbage.txt new file mode 100644 index 0000000..c31d675 --- /dev/null +++ b/tests/actions/plugin-check-report/fixtures/garbage.txt @@ -0,0 +1,3 @@ + +Database Error +

Error establishing a database connection

diff --git a/tests/actions/plugin-check-report/fixtures/rule-up-total-same.txt b/tests/actions/plugin-check-report/fixtures/rule-up-total-same.txt new file mode 100644 index 0000000..2efabff --- /dev/null +++ b/tests/actions/plugin-check-report/fixtures/rule-up-total-same.txt @@ -0,0 +1,4 @@ +FILE: my-plugin.php +[{"line":12,"column":10,"type":"ERROR","code":"WordPress.Security.EscapeOutput.OutputNotEscaped","message":"All output should be run through an escaping function.","docs":""},{"line":12,"column":10,"type":"ERROR","code":"WordPress.Security.EscapeOutput.OutputNotEscaped","message":"All output should be run through an escaping function.","docs":""},{"line":12,"column":10,"type":"ERROR","code":"WordPress.Security.EscapeOutput.OutputNotEscaped","message":"All output should be run through an escaping function.","docs":""}] +FILE: src/Forms/Form.php +[{"line":88,"column":9,"type":"WARNING","code":"WordPress.DB.DirectDatabaseQuery.DirectQuery","message":"Use of a direct database call is discouraged.","docs":""}] diff --git a/tests/actions/plugin-check-report/fixtures/same-as-baseline.txt b/tests/actions/plugin-check-report/fixtures/same-as-baseline.txt new file mode 100644 index 0000000..b2ba8a2 --- /dev/null +++ b/tests/actions/plugin-check-report/fixtures/same-as-baseline.txt @@ -0,0 +1,4 @@ +FILE: my-plugin.php +[{"line":12,"column":10,"type":"ERROR","code":"WordPress.Security.EscapeOutput.OutputNotEscaped","message":"All output should be run through an escaping function.","docs":""},{"line":12,"column":10,"type":"ERROR","code":"WordPress.Security.EscapeOutput.OutputNotEscaped","message":"All output should be run through an escaping function.","docs":""}] +FILE: src/Forms/Form.php +[{"line":40,"column":5,"type":"ERROR","code":"WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents","message":"file_get_contents() is discouraged.","docs":""},{"line":88,"column":9,"type":"WARNING","code":"WordPress.DB.DirectDatabaseQuery.DirectQuery","message":"Use of a direct database call is discouraged.","docs":""}] diff --git a/tests/actions/plugin-check-report/fixtures/warnings-up.txt b/tests/actions/plugin-check-report/fixtures/warnings-up.txt new file mode 100644 index 0000000..4b9f1cc --- /dev/null +++ b/tests/actions/plugin-check-report/fixtures/warnings-up.txt @@ -0,0 +1,4 @@ +FILE: my-plugin.php +[{"line":12,"column":10,"type":"ERROR","code":"WordPress.Security.EscapeOutput.OutputNotEscaped","message":"All output should be run through an escaping function.","docs":""},{"line":12,"column":10,"type":"ERROR","code":"WordPress.Security.EscapeOutput.OutputNotEscaped","message":"All output should be run through an escaping function.","docs":""}] +FILE: src/Forms/Form.php +[{"line":40,"column":5,"type":"ERROR","code":"WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents","message":"file_get_contents() is discouraged.","docs":""},{"line":88,"column":9,"type":"WARNING","code":"WordPress.DB.DirectDatabaseQuery.DirectQuery","message":"Use of a direct database call is discouraged.","docs":""},{"line":88,"column":9,"type":"WARNING","code":"WordPress.DB.DirectDatabaseQuery.DirectQuery","message":"Use of a direct database call is discouraged.","docs":""},{"line":88,"column":9,"type":"WARNING","code":"WordPress.DB.DirectDatabaseQuery.DirectQuery","message":"Use of a direct database call is discouraged.","docs":""}] diff --git a/tests/actions/plugin-check-report/ratchet.test.js b/tests/actions/plugin-check-report/ratchet.test.js new file mode 100644 index 0000000..07d28c9 --- /dev/null +++ b/tests/actions/plugin-check-report/ratchet.test.js @@ -0,0 +1,297 @@ +'use strict' + +const { describe, it } = require('node:test') +const assert = require('node:assert') +const fs = require('node:fs') +const os = require('node:os') +const path = require('node:path') +const { spawnSync } = require('node:child_process') + +const RATCHET = path.join(__dirname, '../../../.github/actions/plugin-check-report/ratchet.sh') +const FIXTURES = path.join(__dirname, 'fixtures') +const BASELINE = path.join(FIXTURES, 'baseline.json') + +const ESCAPE = 'WordPress.Security.EscapeOutput.OutputNotEscaped' +const DIRECT_QUERY = 'WordPress.DB.DirectDatabaseQuery.DirectQuery' + +/** + * Runs ratchet.sh the way the action does, and returns what it printed and wrote. + * + * The environment is built from scratch rather than copied from process.env. In CI the test runner + * has GITHUB_STEP_SUMMARY, GITHUB_RUN_ID and friends set, and the script would append its summary + * to this repository's own job and link to this run. Only PATH and HOME are kept, so bash and jq + * are found. The script's `::error::` lines go to the captured stdout, never to the runner's log, + * so a passing test run does not annotate its own workflow. + * + * @param {string[]} args Arguments after the script, usually a results file. + * @param {object} env Extra environment, such as PLUGIN_CHECK_BASELINE. + * + * @returns {{status: number, stdout: string, stderr: string, comment: string, summary: string}} + * The exit status, both streams, the comment file and the job summary it wrote. + */ +function ratchet(args, env = {}) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'plugin-check-ratchet-')) + const comment = path.join(dir, 'comment.md') + const summary = path.join(dir, 'summary.md') + + const result = spawnSync('bash', [RATCHET, '--comment-file', comment, ...args], { + encoding: 'utf8', + env: { + PATH: process.env.PATH, + HOME: process.env.HOME, + GITHUB_STEP_SUMMARY: summary, + // A path that does not exist, so a test that wants no baseline gets none, instead of a + // baseline.json some earlier local run left next to the script. + PLUGIN_CHECK_BASELINE: path.join(dir, 'no-baseline.json'), + ...env, + }, + }) + + const read = (file) => (fs.existsSync(file) ? fs.readFileSync(file, 'utf8') : '') + + return { + status: result.status, + stdout: result.stdout, + stderr: result.stderr, + comment: read(comment), + summary: read(summary), + } +} + +/** + * The path of one of the sample results files. + * + * @param {string} name File name under fixtures/. + * + * @returns {string} The absolute path. + */ +function fixture(name) { + return path.join(FIXTURES, name) +} + +/** + * A results file with one new ERROR code per entry, for the tests about long lists. + * + * @param {number} codes How many different codes to write. + * + * @returns {string} The path of the file. + */ +function resultsWithNewCodes(codes) { + const results = Array.from({ length: codes }, (_, index) => ({ + line: index + 1, + column: 1, + type: 'ERROR', + code: `Test.Rule.Number${index + 1}`, + message: 'A rule added for the test.', + docs: '', + })) + const file = path.join(fs.mkdtempSync(path.join(os.tmpdir(), 'plugin-check-results-')), 'results.txt') + fs.writeFileSync(file, `FILE: plugin.php\n${JSON.stringify(results)}\n`) + + return file +} + +describe('ratchet.sh', () => { + describe('results it refuses', () => { + it('fails when the results file is missing', () => { + const run = ratchet([fixture('does-not-exist.txt')]) + + assert.equal(run.status, 1) + assert.match(run.stdout, /::error title=Plugin Check::Results file not found/) + assert.equal(run.comment, '', 'no comment body, so the workflow writes "could not run"') + }) + + it('fails when the results file is not Plugin Check output', () => { + const run = ratchet([fixture('garbage.txt')]) + + assert.equal(run.status, 1) + assert.match(run.stdout, /is not Plugin Check output.*First unexpected line: /) + }) + + it('fails when the results file is empty', () => { + const run = ratchet([fixture('empty.txt')]) + + assert.equal(run.status, 1) + assert.match(run.stdout, /has no Plugin Check output/) + }) + + it('fails on a mode it does not know, before reading anything', () => { + const run = ratchet([fixture('clean.txt')], { PLUGIN_CHECK_MODE: 'strcit' }) + + assert.equal(run.status, 1) + assert.match(run.stdout, /Unknown mode 'strcit'/) + }) + }) + + describe('--update', () => { + it('writes the counts per rule code', () => { + const out = path.join(fs.mkdtempSync(path.join(os.tmpdir(), 'plugin-check-update-')), 'baseline.json') + const run = ratchet(['--update', fixture('errors-up.txt')], { PLUGIN_CHECK_BASELINE: out }) + + assert.equal(run.status, 0) + assert.deepEqual(JSON.parse(fs.readFileSync(out, 'utf8')), { + errors: { + [ESCAPE]: 3, + 'WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents': 1, + }, + warnings: { [DIRECT_QUERY]: 1 }, + }) + }) + + it('writes empty counts for a clean run', () => { + const out = path.join(fs.mkdtempSync(path.join(os.tmpdir(), 'plugin-check-update-')), 'baseline.json') + const run = ratchet(['--update', fixture('clean.txt')], { PLUGIN_CHECK_BASELINE: out }) + + assert.equal(run.status, 0) + assert.deepEqual(JSON.parse(fs.readFileSync(out, 'utf8')), { errors: {}, warnings: {} }) + }) + }) + + describe('compare in ratchet mode', () => { + it('passes a clean run with no baseline, with a warning and the one-column table', () => { + const run = ratchet([fixture('clean.txt')]) + + assert.equal(run.status, 0) + assert.match(run.stdout, /::warning title=Plugin Check::No baseline to compare with/) + assert.match(run.comment, /⚠️ Plugin Check ran, but there was no baseline to compare with/) + assert.match(run.comment, /\| Errors {3}\| 0 \|/) + assert.doesNotMatch(run.comment, /Change/) + }) + + it('passes when every count matches the baseline', () => { + const run = ratchet([fixture('same-as-baseline.txt')], { PLUGIN_CHECK_BASELINE: BASELINE }) + + assert.equal(run.status, 0) + assert.match(run.comment, /✅ Plugin Check passed/) + assert.match(run.comment, /\| Errors {3}\| 3 \| 3 \| ±0 \|/) + }) + + it('fails when a rule has more errors than the baseline', () => { + const run = ratchet([fixture('errors-up.txt')], { PLUGIN_CHECK_BASELINE: BASELINE }) + + assert.equal(run.status, 1) + assert.match(run.stdout, new RegExp(`::error title=Plugin Check::${ESCAPE} has 3 errors, the baseline has 2`)) + assert.match(run.comment, /❌ Plugin Check failed: 1 rule has more errors than the baseline allows/) + assert.match(run.comment, new RegExp(`Went up: \`${ESCAPE}\` \\(\\+1\\)`)) + }) + + it('fails when a rule goes up even though the total stays the same', () => { + const run = ratchet([fixture('rule-up-total-same.txt')], { PLUGIN_CHECK_BASELINE: BASELINE }) + + assert.equal(run.status, 1) + assert.match(run.comment, /\| Errors {3}\| 3 \| 3 \| ±0 \|/) + assert.match(run.comment, /Rules that went down: 1\./) + }) + + it('passes with a notice when errors go down', () => { + const run = ratchet([fixture('errors-down.txt')], { PLUGIN_CHECK_BASELINE: BASELINE }) + + assert.equal(run.status, 0) + assert.match(run.stdout, new RegExp(`::notice title=Plugin Check::${ESCAPE} \\(errors\\) went down from 2 to 1`)) + assert.match(run.comment, /\| Errors {3}\| 2 \| 3 \| -1 \|/) + }) + + it('passes with a notice when only warnings go up', () => { + const run = ratchet([fixture('warnings-up.txt')], { PLUGIN_CHECK_BASELINE: BASELINE }) + + assert.equal(run.status, 0) + assert.match(run.stdout, /Warnings do not fail the check/) + assert.match(run.comment, new RegExp(`Went up: \`${DIRECT_QUERY}\` warning \\(\\+2\\)`)) + }) + + it('only warns when the baseline is from an older run of the target branch', () => { + const run = ratchet([fixture('errors-up.txt')], { + PLUGIN_CHECK_BASELINE: BASELINE, + PLUGIN_CHECK_ADVISORY: 'true', + }) + + assert.equal(run.status, 0) + assert.match(run.stdout, /::warning title=Plugin Check::.*Not enforced/) + assert.match(run.comment, /⚠️ Plugin Check passed, not enforced/) + }) + }) + + describe('compare in strict mode', () => { + it('fails on any error, even with no baseline', () => { + const run = ratchet([fixture('errors-down.txt')], { PLUGIN_CHECK_MODE: 'strict' }) + + assert.equal(run.status, 1) + assert.match(run.stdout, /2 errors found\. Strict mode fails on any error/) + assert.match(run.comment, /❌ Plugin Check failed: 2 errors, and strict mode allows none/) + }) + + it('fails on errors that went down, which ratchet mode would pass', () => { + const run = ratchet([fixture('errors-down.txt')], { + PLUGIN_CHECK_MODE: 'strict', + PLUGIN_CHECK_BASELINE: BASELINE, + }) + + assert.equal(run.status, 1) + }) + + it('passes a clean run', () => { + const run = ratchet([fixture('clean.txt')], { PLUGIN_CHECK_MODE: 'strict' }) + + assert.equal(run.status, 0) + assert.match(run.comment, /✅ Plugin Check passed: no errors/) + }) + + it('passes a run with warnings only', () => { + const results = fixture('warnings-up.txt') + const onlyWarnings = path.join(fs.mkdtempSync(path.join(os.tmpdir(), 'plugin-check-warn-')), 'results.txt') + // The warnings-up sample without its ERROR results. + const lines = fs.readFileSync(results, 'utf8').split('\n').map((line) => { + if (!line.startsWith('[')) { + return line + } + const kept = JSON.parse(line).filter((result) => result.type === 'WARNING') + return JSON.stringify(kept) + }) + fs.writeFileSync(onlyWarnings, lines.join('\n')) + + const run = ratchet([onlyWarnings], { PLUGIN_CHECK_MODE: 'strict' }) + + assert.equal(run.status, 0) + }) + }) + + describe('the comment', () => { + it('starts with the default marker', () => { + const run = ratchet([fixture('clean.txt')]) + + assert.equal(run.comment.split('\n')[0], '') + }) + + it('starts with the marker the workflow passes', () => { + const marker = '' + const run = ratchet([fixture('clean.txt')], { COMMENT_MARKER: marker }) + + assert.equal(run.comment.split('\n')[0], marker) + }) + + it('names the baseline column after where it came from, with a link', () => { + const run = ratchet([fixture('same-as-baseline.txt')], { + PLUGIN_CHECK_BASELINE: BASELINE, + BASELINE_LABEL: 'Target branch', + BASELINE_URL: 'https://github.com/o/r/actions/runs/1', + }) + + assert.match(run.comment, /\| Now \| \[Target branch\]\(https:\/\/github\.com\/o\/r\/actions\/runs\/1\) \| Change \|/) + }) + + it('lists at most five rules and never grows past fifteen lines', () => { + const run = ratchet([resultsWithNewCodes(7)], { PLUGIN_CHECK_BASELINE: BASELINE }) + + assert.equal(run.status, 1) + assert.match(run.comment, /Went up \(top 5 of 7\)/) + assert.doesNotMatch(run.comment, /Number6/) + assert.ok(run.comment.trimEnd().split('\n').length <= 15) + }) + + it('writes the same text to the job summary', () => { + const run = ratchet([fixture('errors-down.txt')], { PLUGIN_CHECK_BASELINE: BASELINE }) + + assert.equal(run.summary, run.comment) + }) + }) +})