Skip to content

Fix the test runner image build and the pre-push hook - #72

Open
fafera wants to merge 17 commits into
mainfrom
fix/test-runner-image-build
Open

fafera wants to merge 17 commits into
mainfrom
fix/test-runner-image-build

Conversation

@fafera

@fafera fafera commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

Context

The runner image build has failed on every scheduled run since June, so nothing has been published since; the pre-push hook only ever worked on continuous integration. Both are described in #71.

This fixes everything up to the image push. It does not publish an image or make the local suite runnable on its own: the package grants this repository read and not write, and development machines have no read access at all. Both grants are on #71 and need an organization owner.

The build workflow

Two chained defects, the second hidden behind the first, plus one step added for parity with the sibling test-suite workflow:

  1. Wrong order. The WordPress bootstrap ran before dependencies were installed, but depends on them — the SQLite integration plugin arrives through Composer's installer paths, and the script resolves the WordPress command line through Composer. This was the visible error in the logs.
  2. Missing environment file. Codeception declares it as its parameter source and it is ignored by version control, so a fresh checkout has none and the suite never started.
  3. Support classes generated explicitly. Not a defect — codecept run rebuilds them when missing. The step states the dependency rather than relying on it.

The dependency step also picks up the sibling workflow's Composer flags.

The pre-push hook

It invoked the container runtime directly and upgraded every package before testing — both right on continuous integration, wrong on a development machine (#71). Every container command now goes through the helper script, which forwards the calling user and group; dependencies are installed, not updated. Image selection moves there too, since the hook read the image variable as a bare tag while the helper script and the README read it as a full reference.

Four further defects surfaced while fixing it.

The map no longer matched the tree. The Plugin Subnamespace refactor moved src/Method/ to src/WooCommerce/Method/ and the map still pointed at the old paths — every one. Nothing resolved to a Cest, so every push fell through to the full suite: safe, slow, and it kept the defect below from ever surfacing. The mapping tests passed because they carried their own copy of the same stale map. Each Method Trait now points at every Cest that exercises it rather than the Cest of the same name: the product builders seed the fixtures the cart, checkout and subscription Cests assert on, and the order traits point at both the Legacy and the HPOS Cest.

An unmapped file did not widen the run. It was documented as falling through to the full suite; it did not. The selection is empty only when no rule contributed a Cest at all, so changing a trait with no acceptance coverage alongside a mapped one ran just the mapped Cest. Shipped code under src/ that no rule claims now takes the full-suite branch explicitly, so a subnamespace added later inherits the safe behaviour.

An empty selection passed the push. codecept run exits zero when a --filter matches nothing, unlike a positional test name, which errors out. The hook now refuses the push when a mapped Cest is missing from tests/acceptance/, and again if the run selects no tests for any other reason.

The mapping tests never ran — not a workflow, not a Composer script, which is how the map drifted without a single failure. The static-analysis workflow runs them now, and composer test:hooks exposes the same entry point locally. Both workflows also gained the hooks directory in their path filters: a change confined to the hooks previously ran no check at all.

Failure notification

A failing scheduled run opens an issue, and comments on that same issue on every later failure rather than opening one a week. Manual runs are excluded — whoever dispatched them is already looking at the result. An issue was chosen over a chat or mail notification because the repository has no webhook or secret for either; swapping the channel later is a change to one step.

The job restates contents: read alongside issues: write: a job-level permissions block replaces the workflow-level one rather than extending it, and the issue lookup resolves through the GraphQL API, which asks for more than issue scope.

Documentation

  • The registry login, missing from a setup block that opened straight at the first container command — which then fails with a denial that reads like a missing image. The README documents it now, and states what the indistinguishable second case means: a login that succeeds while the pull is denied is an account that was never granted read access to the package. The GitHub CLI does not request the package read scope when it first authenticates, so refreshing its scopes comes before borrowing its token.
  • The environment file — same omission, same effect.
  • Both steps also land in AGENTS.md, whose command block had the same two gaps.
  • Refreshing the image. docker run only downloads a tag it does not already have, so a machine stays on whatever version it first pulled while the image is rebuilt weekly; the symptom is a test that fails locally and passes here. Checking the registry on every run was considered and rejected: it would make the suite depend on network access even when the image is already present.

Validation

The workflow, from a clone into a clean directory — no dependencies, no site, no environment file, exactly like the continuous integration checkout:

Step Result
Prepare environment file ok
Install dependencies no security advisories
Bootstrap WordPress test site ready
Generate support classes 545 methods
Acceptance suite 198 tests, 427 assertions, OK

Same result against the PHP 8.0 image, which resolves its own dependency set because the committed lock file targets 8.4. Before the change, that same clean clone reproduces the continuous integration error exactly.

Dispatched from this branch — run 35261615734 — for the part that cannot be exercised locally: both matrix legs cleared every step that had been failing since June, including the multi-architecture build, then both failed on the push with denied: permission_denied: write_package. So the sequence is correct in the real environment, and the missing write grant — invisible until now, because no run had ever reached the push — is all that is left. The notification job was correctly skipped on a manual dispatch.

The hook: the mapping tests pass 15 of 15 and now run on every pull request. Six are new — the order traits mapping to both storage Cests, two that read the paths and Cest names out of the hook itself so the next drift fails the tests, and three for the unmapped-file case. Each was checked by reintroducing the defect it exists to catch.

Exercised end to end against an image built locally from this Dockerfile, carrying a user identity that differs from the developer's — which is what the old hook fell over. A coupon-trait change ran CouponCest and the unit suite (12 and 29 tests); an order-trait change ran both order Cests (52 tests); shared infrastructure and the traits with no acceptance coverage of their own took the full-suite branch. Every run left composer.lock byte-identical, where the old hook moved twenty-nine packages.

The notification step cannot run here, so I checked it outside the workflow: shell syntax, the lookup for an already-open issue against three cases, and both branches of the script with the command line tool stubbed.

One limit worth stating: the tests carry their own copy of the hook's case statement. The two match today, compared arm by arm, but nothing enforces it — the tests that read from the hook catch a renamed path or Cest, not a changed mapping, so six arms have no assertion behind them. Closing that means generating the map from one source instead of two, which is larger than this pull request should carry.

Remaining

Both package grants on #71 are in place and verified. Dispatching this branch again — run 35385117647 — published on both PHP versions, where the previous attempt had been denied write on the package; the image had last been refreshed on 29 May 2026 and now carries today's build. A development machine pulls it, and the full suite passes against it locally: 227 acceptance tests, 466 assertions, plus 29 unit tests, with the lock file left untouched.

Nothing else is outstanding. Note that this workflow runs only on a schedule or on manual dispatch, so merging publishes nothing by itself — what merging fixes is the scheduled run, which still fails on the default branch, and the pre-push hook, which is still the version that only works on continuous integration.

The workflow has failed on every scheduled run, so the runner image was
never published and the GHCR package does not exist. Anyone running
bin/test or the pre-push hook hits a pull failure that reads as a
permissions problem but is an absent image.

Three defects, each hidden behind the previous one. The sibling
test-suite workflow already has the correct sequence; align with it.

- Bootstrap ran before Composer, but resources/install.sh needs the
  SQLite drop-in that installer-paths materialises and resolves WP-CLI
  through `composer exec`. This is the reported failure:
  `cp: can't stat '.../sqlite-database-integration/db.copy'`.
- No .env was created. codeception.yml declares `params: - .env` and the
  file is gitignored, so a fresh checkout has none.
- Codeception actors were never built, though tests/_support/_generated
  is gitignored.

Verified by replaying the corrected step sequence against a clean clone
with no vendor/, public/packages/ or .env: the full acceptance suite
passes (208 tests, 461 assertions). The same clone reproduces the CI
error exactly before the change.
The hook ran the container the same way the build workflow does: no
--user, and `composer update` before the tests.

Both are right for continuous integration and wrong locally. The image
bakes UID 1001, which matches the runner but not a typical development
machine, so git refuses the repository over unexpected ownership and
Composer cannot write. And a verification gate must run against the
locked versions, not rewrite composer.lock in the working tree of
whoever is pushing.

bin/test already forwards the calling user and group, mounts the git
common dir for worktrees and persists the Composer cache, so the three
duplicated docker run invocations become three bin/test calls.

Image selection moves to bin/test as well. The hook read
AZTEC_TEST_IMAGE as a bare tag and built the reference around it, while
bin/test and the README read it as a full image reference; keeping both
would give one variable two meanings.

Contributes to #71.
@fafera fafera changed the title Fix the test runner image build workflow Fix the test runner image build and the pre-push hook Sep 17, 2026
The workflow that runs the test suite installs dependencies with
--no-progress --prefer-dist. Use the same flags here: smaller logs and
faster downloads, and one less difference between the two.
The image build runs on a schedule with nobody watching it, which is how
it reached thirteen failures before anyone noticed the package had never
been published. On failure it now opens an issue, and adds a comment to
that same issue on every later failure rather than opening a new one each
week. Manual runs are excluded: whoever dispatched them is already
looking at the result.

The README also now says how to refresh the runner image. `docker run`
only downloads a tag it does not already have, so a developer machine
stays on whatever version it first pulled while the published image is
rebuilt weekly. The usual symptom is a test that fails locally and
passes on continuous integration, which is a confusing thing to debug
without knowing to pull first.

Contributes to #71.
@fafera
fafera force-pushed the fix/test-runner-image-build branch from 6cea918 to 49eb648 Compare September 17, 2026 14:52
`codecept run` declares two optional arguments, suite and test, and
neither is variadic. The hook expanded every impacted Cest into that
second slot, so the moment two of them changed together the run aborted
with "Too many arguments" and the push was blocked with it. The mapping
test suite asserts that case works, but it exercises a copy of the case
statement rather than the hook, so it never caught this.

Pass the classes through --filter instead. The filter is matched against
the test signature, which for a Cest is the fully qualified class name
followed by a colon and the method, so a trailing colon anchors each
alternative at the class boundary. Single and multiple Cests now take
the same path, since one alternative is just a shorter pattern.

Contributes to #71.
codeception.yml names .env as its parameter source and the file is
ignored by version control, so a fresh clone has none and Codeception
refuses to start: the params file is missing, which is a hard error
rather than a warning.

All three workflows already copy it from the example. Nothing written
for a person did — the only mention of .env in the repository assumed
the file was already there, to say the server port could be overridden
in it. So the one obligatory step of the local setup was the one step
never written down.

Copying it is documented, not automated. The hook could create the file,
but it would then overwrite whatever the developer had configured on
every push, and on a machine where .env is a symlink the copy writes
through the link and destroys its target.

Contributes to #71.
The Plugin Subnamespace refactor moved src/Method/ to
src/WooCommerce/Method/, and the pre-push map kept the old paths — all of
them, including the shared-infrastructure patterns. Nothing resolved to a
Cest any more, so every push fell through to the full suite and the
selective path was dead code. The mapping tests kept passing because they
carry their own copy of the same map.

Each Method Trait now points at every Cest that exercises it rather than
the Cest of the same name: the product builders seed the fixtures the
cart, checkout and subscription Cests assert on, so a change there is
only verified if those run too, and the order traits point at both the
Legacy and the HPOS Cest since the same methods run against both
storages. Traits with no acceptance coverage of their own, and everything
under src/CodeSniffer/, stay unmapped and fall through to the full suite.
`codecept run` exits zero when a --filter matches nothing: it prints
"No tests executed!" and reports success, unlike a positional test name,
which errors out. So the map drift fixed in the previous commit would
have stayed invisible even once the filter path became reachable, and any
future rename would read as a green gate that verified nothing.

A Cest named in the map but missing from tests/acceptance/ now fails
before the container starts, and a run that selects no tests fails after
it. Two mapping tests cover this by reading the paths and Cest names out
of the hook itself rather than restating them, so the next drift fails
the tests instead of quietly disabling the feature.
A job-level permissions block replaces the workflow-level one rather than
extending it, so the notify job ran on issues: write alone and lost the
contents: read granted at the top of the file.

`gh issue list --search` resolves through the GraphQL API, which asks for
more than issue scope. The job only runs once a scheduled build has
already failed, so being wrong here costs a second silent failure in the
one step built to make failures loud.

Contributes to #71.
The map's comment said a file matching nothing fell through to the full
suite. That only held when nothing else matched: the selection is empty
only if no arm contributed a Cest, so changing ActionMethods alongside
CouponMethods resolved to CouponCest and the unmapped change was never
exercised.

A src/*.php arm kept last in the case now widens the run instead. It
covers ActionScheduler, all of src/CodeSniffer/ and any subnamespace
added later, since * matches slashes in a case pattern.

Three mapping tests cover the arm, including the mixed case that used to
narrow the run. The existing test that checks every mapped path still
exists in the tree only understood globs ending in *, so it read
src/*.php as a literal filename; it now checks the directory preceding
the first wildcard wherever that wildcard sits.

Contributes to #71.
Nothing executed .githooks/test-pre-push.sh — not a workflow, not a
Composer script. That is how the map drifted through the Plugin
Subnamespace refactor without a single failure: the tests written to
catch exactly that were never run.

The static-analysis job now runs them, ahead of the PHP setup since the
script needs neither PHP nor the dependencies, and `composer test:hooks`
exposes the same entry point locally.

Both workflows also gained .githooks/** in their path filters. Every
filter listed src, tests, the manifests and the workflow itself, so a
change confined to the hooks matched nothing and no check ran at all.

Contributes to #71.
The release workflow builds its notes from CHANGELOG.md, and the 0.1.0
section already names .githooks/pre-push, bin/test and the image workflow
— tooling changes are in scope for the file.

Fixed covers the three defects: the step ordering that had kept the image
from being published since June, the uid mismatch that broke the hook on
any machine whose user is not the image's, and the impacted-test
selection that had stopped selecting. Added covers the failure
notification and the newly wired map tests.

Contributes to #71.
The runner image is a private package, so `docker pull` — and with it
every bin/test command — is denied on a machine that has never logged in
to the registry. Nothing in the repository said so: the setup block
opened straight at `bin/test composer install`, and the failure surfaces
as a registry denial that reads like a missing image rather than a
missing login.

The login is one step per machine and a personal access token with the
read:packages scope covers it. A token that logs in successfully and is
still denied on the pull means the account was never granted read access
to the package — worth stating, because the two failures are
indistinguishable from the command output.

Contributes to #71.
`gh auth login` requests gist, read:org and repo — not read:packages. So
`gh auth token` on a machine that authenticated normally yields a token
that logs in to the registry successfully and is then denied on the pull,
which is the one failure mode the surrounding paragraph warns about. The
command worked where it was written only because that account had the
scope added at some earlier point.

`gh auth refresh -s read:packages` adds it, and the paragraph no longer
implies the CLI carries the scope by default.

Contributes to #71.
AGENTS.md is the file #71 names in its first sentence, and its command
block opened straight at `bin/test composer install`. Two obligatory
steps were missing, and both fail in ways that do not name themselves:
the runner image is a private package, so without a registry login every
command is denied in a way that reads as a missing image; and
codeception.yml resolves its parameters from .env, which is gitignored,
so a fresh clone makes Codeception refuse to start.

The README gained both already. This is the file agents actually read,
so leaving it to point at a broken sequence defeats the purpose of
documenting the fix at all.

Contributes to #71.
Several of them carried the history that motivated them instead: an issue
number, the refactor that broke the file-to-Cest map, the count of silent
scheduled failures, the date of the last published image. That belongs in
the issue and the commit log — in the tree it goes stale at the next change
and reads as noise to anyone who was not there. The changelog entries get
the same treatment, keeping their issue links.
The entries describe the fix and the constraint behind it on their own, so
the trailing link only pointed a reader at the ticket to learn nothing the
line does not already say.
@fafera fafera self-assigned this Sep 18, 2026
@fafera
fafera requested a review from edpittol September 18, 2026 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant