Repository navigation
Conversation
0888650 to
1f31da5
Compare
|
Follow-up finding (2026-09-12): a separate, already-known blocker was preventing this PR's CI from ever reaching With that fix in place, real CI on #294 gets all the way past This is the exact same issue already found and fixed on sibling branch I attempted to apply this fix directly to this PR's commit (an amend, matching this project's established "edit an unreviewed single commit" convention) but the force-push was blocked outright by this environment's own permission classifier (reason given: "Blocked by classifier", no further detail) on two different push invocations. Per this project's standing rule against ever routing around a permission-system block, I stopped rather than trying another way around it — this PR's commit is unchanged, still exactly |
`wp-env` shells out to the standalone `docker-compose` v1 binary via the `docker-compose` npm package's default export, and `run.js` spawns a literal `docker-compose` binary directly. GitHub's current Actions runner images no longer ship that binary — only the `docker compose` v2 subcommand (via the `docker` CLI plugin) is present — so every `wp-env` command (`start`, `stop`, `clean`, `logs`, `run`, `destroy`) fails immediately with `Error: spawn docker-compose ENOENT`, breaking every e2e/PHPUnit CI job that depends on `wp-env start`. Switch every `require( 'docker-compose' )` call to the package's `v2` export, which shells out to `docker compose` instead of the standalone binary, and update `run.js`'s direct `spawn()` call to invoke `docker` with a `compose` subcommand rather than the removed `docker-compose` binary. Update `cli.js`'s error-handling message and its accompanying unit test in `test/cli.js` to match the new wording. Bump the `docker-compose` npm dependency from `^0.22.2` to `^0.24.3`, the first release line whose `v2` export is available, and update `package-lock.json` accordingly (including a nested `yaml@2.2.2` dependency required by that version, since the rest of the lockfile is pinned to `yaml@^1.x`). `docker-compose@^0.24.3`'s new `yaml` dependency ships as an ESM package, which Jest cannot parse by default since `node_modules` is excluded from transformation unless explicitly allow-listed. Add a `transformIgnorePatterns` entry to `test/unit/jest.config.js` allow-listing `docker-compose`/`yaml` so Jest can transform them, otherwise `packages/env`'s own unit tests fail to load with "Jest encountered an unexpected token." This is the same fix WordPress#51339 (commit a0943f2, "wp-env: Migrate to Compose V2") applied to trunk in December 2023 to fix WordPress#51249, adapted to this branch's own file layout and dependency versions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1f31da5 to
b237237
Compare
…vice that talks to the database.
Real CI on this PR (after the Docker Compose v2 fix let `wp-env start`
get this far for the first time) showed `wp-env start` hanging for ~30
retries and then failing at the `wp db check` step with:
```
mariadb-check: Got error: 1045: Access denied for user 'example username'@'172.18.0.5' (using password: YES) when trying to connect
```
The literal, space-containing username is not a real credential anyone
configured — it is the hardcoded placeholder default baked into the
official `wordpress` Docker image's `wp-config-docker.php` template:
```php
define( 'DB_USER', getenv_docker('WORDPRESS_DB_USER', 'example username') );
define( 'DB_PASSWORD', getenv_docker('WORDPRESS_DB_PASSWORD', 'example password') );
```
`getenv_docker()` re-evaluates these environment variables every time
`wp-config.php` is loaded, in whatever process loads it — it does not
bake a value in once and share it. `wp-config.php` is generated once by
the `wordpress`/`tests-wordpress` apache containers and then read from
the same shared volume by `cli`/`tests-cli` (which is what actually
runs `wp db check`/`wp core install`) and by `phpunit` (via the
`phpunit-wp-config.php` copy `sed` produces from it). Each of those
containers evaluates `getenv_docker()` using *its own* environment, so
setting `WORDPRESS_DB_USER`/`WORDPRESS_DB_PASSWORD` only on
`wordpress`/`tests-wordpress` is not enough — confirmed directly with a
local reproduction: doing only that still failed with the identical
error, because `cli`'s own environment (which is what `wp db check`
actually runs in) still had neither variable set.
This branch's `wordpress`/`tests-wordpress`/`cli`/`tests-cli`/`phpunit`
services only ever set `WORDPRESS_DB_NAME` (or nothing at all, for
`cli`/`tests-cli`/`phpunit`) — never `WORDPRESS_DB_USER` or
`WORDPRESS_DB_PASSWORD` — so every one of them falls back to the
placeholder default. The `mysql` service, meanwhile, is configured with
`MYSQL_ALLOW_EMPTY_PASSWORD: 'yes'` and no `MYSQL_USER`/`MYSQL_PASSWORD`
of its own, so the only account that actually exists in the database is
`root` with an empty password — never a match for the placeholder.
This is not a Docker Compose v1-vs-v2 regression: the environment
variables a `docker-compose.yml` `environment:` block passes into a
container work identically under both `docker-compose` and
`docker compose`, and this branch's own `build-docker-compose-config.js`
never set these two variables anywhere, in either case. The bug was
simply unreachable until now, because `wp-env start` always failed
earlier, at the `docker-compose` v1 binary's `ENOENT`.
Setting `WORDPRESS_DB_USER: 'root'` and `WORDPRESS_DB_PASSWORD: ''`
explicitly on every service that ever reads `wp-config.php` (matching
the `mysql` service's actual, empty-password `root` account) makes each
of them connect as a user that genuinely exists, instead of silently
falling back to the Docker image's documentation placeholder.
8b9a33c to
402977f
Compare
|
Superseded by #310, which applies this same fix as a properly stacked layer on the real |
What?
Fixes
wp-env(and therefore every e2e/PHPUnit CI job that runswp-env start) onwp/5.6, which is currently broken byError: spawn docker-compose ENOENT, and fixes a follow-on Jest failure that migrating todocker-compose@^0.24.3introduces.Why?
packages/envshells out to a literaldocker-composebinary (Docker Compose v1, standalone) via thedocker-composenpm package's default (non-.v2) export, andrun.jsalso spawns that binary directly forwp-env run. GitHub's current Actions runner images no longer ship the standalonedocker-composev1 binary — only thedocker composev2 subcommand (via thedockerCLI plugin, bundled withdocker) is present. As a result, everywp-envcommand (start,stop,clean,logs,run,destroy) fails immediately, which is the root cause of essentially all e2e/PHPUnit CI failures on this branch (the same root cause already confirmed and fixed on several otherwp/X.Ysibling branches in this project).This branch has no Playwright e2e at all (only Puppeteer, via
end2end-test.yml, confirmed viapackage.json), so there is no separate Noble-browser-install issue to worry about here.Separately,
docker-compose@^0.24.3pulls inyaml@^2.2.2as a new transitive dependency, andyaml@2.xships as an ESM package. Jest cannot parse ESMnode_modulescontent by default —node_modulesis excluded from transformation unless explicitly allow-listed — so without atransformIgnorePatternsentry allow-listingdocker-compose/yaml,packages/env's own unit tests (and theJavaScriptCI job that runs them) fail outright with "Jest encountered an unexpected token."How?
This is the same fix WordPress#51339 (commit
a0943f2fba4a5df0e5ee051bfc567b7f6f7f1942, "wp-env: Migrate to Compose V2") applied to trunk in December 2023 to fix WordPress#51249, adapted to this branch's own file layout (packages/env/lib/commands/{clean,destroy,logs,run,start,stop}.js,packages/env/lib/wordpress.js,packages/env/lib/cli.js) and dependency versions:const dockerCompose = require( 'docker-compose' );becomesconst { v2: dockerCompose } = require( 'docker-compose' );inclean.js,destroy.js,logs.js,start.js,stop.js, andwordpress.js.run.js's directspawn( 'docker-compose', composeCommand, ... )becomesspawn( 'docker', [ 'compose', ...composeCommand ], ... )(adapted to this branch's actualrun.js, which uses arun --rmcompose invocation rather thanexec).cli.js's error-handling comment/message updated from "docker-compose error"/"docker-compose command" to "docker compose error"/"docker compose command" to match, along with the corresponding assertion intest/cli.js('Error while running docker-compose command.'→'Error while running docker compose command.'), which otherwise would have started failing this branch's ownJavaScriptunit-test job.packages/env/package.json'sdocker-composedependency bumped from^0.22.2to^0.24.3— the.v2export only exists starting in thedocker-composenpm package's0.23.xline, and this branch'spackage-lock.jsonhad it resolved at0.22.2, so the code change alone would not have been sufficient.package-lock.jsonupdated accordingly:docker-compose'sversion/resolved/integritybumped to0.24.3, plus a nestedyaml@2.9.1dependency added under it (that release ofdocker-composerequiresyaml@^2.2.2, while every other package on this branch is pinned toyaml@^1.x, so it needs its own nested entry rather than sharing the top-level one;2.9.1is the actual current latest2.xrelease satisfying that range, matching what a realnpm installwould resolve to).test/unit/jest.config.jsgets a newtransformIgnorePatternsentry — this branch had no such key at all beforehand, so this is a plain addition, not a merge with any pre-existing pattern:Verification
node -cpasses on every changed.jsfile, includingtest/unit/jest.config.js;package.json/package-lock.jsonparse as valid JSON.Installed
docker-compose@0.24.3(withyaml@2.9.1) in an isolatednode_modulesand confirmed at runtime (require.resolve, not just static inspection) thatrequire( 'docker-compose' ).v2resolves and exposes every method this branch's code actually calls (exec,run,upOne,rm,logs,pullAll,upMany,down), using the exact versions this PR pins inpackage-lock.json.Triggered a real CI run on the pushed branch and polled every check to a final conclusion — none of it could exercise this fix, because this branch's CI is currently broken at an earlier, unrelated step, on every single job:
npm cifails while running a git-dependency'spreparescript (react-native-screens@2.9.0'sbob build, which runstscagainst@types/nodeand hits severalTS1005/TS1109/TS1110syntax errors), long beforewp-env/docker-compose/Jest are ever invoked. This is pre-existing and unrelated to this change, not something introduced by it — the identical failure, in the identical step, already occurs on this PR's own base branch (fix-actions-5.6/add-workflow-concurrency) in a run from before any of this PR's commits existed (run34560410598, job103141789146).Final result, all checks resolved (nothing left pending), run 34694620579–34694620604 (triggered 2026-09-12 by the force-push that added the
jest.config.jscommit):build,PHP,JavaScript,Mobile,Admin - 1throughAdmin - 4,Check/All(Static Analysis),Compare performance with master— all failure, every one at the identicalNpm install and buildstep with the identicalreact-native-screens/TS1005/TS1109/TS1110error.auto_label_pull_request— success (unrelated bot, unaffected).Generate a list of props— skipped (unrelated, conditional job).So while a real CI run was triggered and fully polled to completion rather than left pending, it provides no positive confirmation that either half of this fix (the
docker-composev2 migration or the JesttransformIgnorePatternsaddition) actually works — the branch needs the separatenpm ci/react-native-screens/TypeScript issue resolved first (out of scope for this PR) beforewp-env/docker-compose/Jest behavior can be verified by real CI. Reviewers should be aware of this gap rather than assume CI green-lit the fix; the localnode -c/isolated-node_modulesverification above is the only positive evidence currently available.Use of AI Tools
This PR was created by Claude Code under my supervision. All code should be treated as AI-produced and not yet reviewed by a human until this PR is marked Ready for Review.