ci: wire PHP tests through wp-env + fix 4 pre-existing failures#7
Merged
Conversation
`npm run test:php` used to call a bare `phpunit` binary that wasn't on anyone's PATH. Replace the toolchain with wp-env so tests run inside a container that already ships the WordPress test library, MariaDB, and the right PHP version — no local svn, no local MySQL, no install step beyond `npm install`. - `.wp-env.json` mounts the repo at `wp-content/plugins/wp-desktop-mode` - `composer.json` pins `phpunit/phpunit ^9.6` (WP 6.9's test library still calls `PHPUnit\Util\Test::parseTestMethodAnnotations()`, which PHPUnit 10 removed — the polyfills don't bridge direct WP-core calls) - `bootstrap.php` autoloads composer (for phpunit-polyfills) and defaults `WP_TESTS_DIR` to `/wordpress-phpunit` where wp-env installs the library - `phpunit.xml.dist` schema realigned to 9.5 to match the installed PHPUnit version (the attributes were already 9.x) - new scripts: `env:start`, `env:stop`, `env:destroy`, `test:php:install` - untrack the PHPUnit result cache
With the test harness actually running, four pre-existing bugs surface. All of them pre-date the wp-env switch. - `wpdm_sanitize_dock_icon` kept a stale branch accepting `data:image/svg+xml;base64,…` icons. The function's own docstring (@SInCE 0.11.0) declares every `data:` URI rejected; code and doc disagreed. Remove the branch so every `data:` URI falls through to the generic dashicon fallback. Behavior change: any plugin currently shipping an SVG data-URI dock icon will silently fall back to `dashicons-admin-generic`. - `wpdm_sanitize_session` stamped `updated = time()` unconditionally. The stale-write guard in `wpdm_save_session` compares the incoming client timestamp against the stored value — but with server wallclock stored, equal-timestamp writes (two saves inside the same second) looked strictly older and got rejected, silently dropping user work. Preserve the client's `updated` when valid; fall back to `time()` only when missing/invalid. - The `make_menu_row` test helper built synthetic hooknames via `'menu-' . sanitize_key( $slug )`, but `sanitize_key` strips dots entirely, so `index.php` became `menu-indexphp` instead of the `menu-index-php` the assertion expects. Normalize `.` to `-` before `sanitize_key`. Run: 339 tests, 723 assertions, 0 failures.
Mirrors the local setup introduced in the previous commit, so CI and `npm run test:php` run against the same stack. Drops the host-side `phpunit` binary, the `shivammathur/setup-php` step, the `mysql:8.0` service, the `subversion` apt install, and the `install-wp-tests.sh` fetch — wp-env provides all of it. Matrix stays at PHP 8.1 / 8.2 against the latest WordPress; the PHP version is switched via `WP_ENV_PHP_VERSION`, which wp-env reads when building the container.
`npm ci` fell over on Linux because the lockfile was generated on macOS and missed Linux-specific optional deps (esbuild platform binaries, plus a couple of transitives that never got written). Regenerate the lockfile inside a Linux Node container so v3's cross-platform package list stays complete for both local (darwin) and CI (linux). Also bump the Node version in both CI jobs from 20 to 24, matching local dev.
The old `npm run test:php` line assumed a pre-installed WP test suite on the host. The new flow runs inside wp-env, which has its own `env:start` + `test:php:install` steps. Rewrite the Dev loop block to spell the full sequence out, and clarify that this plugin's `env:start` is scoped to automated tests — manual QA still runs against the parent Core-checkout's Docker host. Also update the "Where things are tested" PHPUnit entry so it no longer points at `WP_TESTS_DIR`.
epeicher
commented
Apr 23, 2026
Comment on lines
-488
to
-503
| // SVG data URI — the JS dock renders these as CSS background-image, | ||
| // which does NOT execute script in any currently shipping browser | ||
| // (Chrome/Firefox/Safari all treat SVG-in-CSS-background as an | ||
| // image-only context since ~2017). Strict validation on the base64 | ||
| // payload prevents anything other than proper base64 from reaching | ||
| // the DOM; any other data:* scheme (javascript:, text/html, etc.) | ||
| // is rejected outright by the prefix check. | ||
| $svg_prefix = 'data:image/svg+xml;base64,'; | ||
| if ( 0 === stripos( $icon, $svg_prefix ) ) { | ||
| $payload = substr( $icon, strlen( $svg_prefix ) ); | ||
| if ( preg_match( '/^[A-Za-z0-9+\/=]+$/', $payload ) ) { | ||
| return $icon; | ||
| } | ||
| return $fallback; | ||
| } | ||
|
|
Collaborator
Author
There was a problem hiding this comment.
Flagged as stale code
This was referenced Apr 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
npm run test:phpnow actually runs. The toolchain moves to@wordpress/env— the WP core test library, MariaDB, and PHP all live inside a container, so the only local prereq is Docker +npm install.^9.6. Tried bumping to 10; WP 6.9.4's test library still callsPHPUnit\Util\Test::parseTestMethodAnnotations()(removed in PHPUnit 10) and the polyfills don't bridge direct WP-core calls. Blocked upstream.What's in here
Commit 1 —
ci: wire PHP tests through wp-env.wp-env.jsonmounts the repo atwp-content/plugins/wp-desktop-modecomposer.jsonwithphpunit/phpunit ^9.6+yoast/phpunit-polyfills ^2.0bootstrap.phpautoloads composer and defaultsWP_TESTS_DIRto/wordpress-phpunitphpunit.xml.distschema realigned to 9.5 (attributes were already 9.x)env:start,env:stop,env:destroy,test:php:install,test:php.phpunit.result.cachestate fileCommit 2 —
fix: resolve 4 pre-existing PHPUnit failuresincludes/helpers.php— remove the stale SVG data-URI accept branch inwpdm_sanitize_dock_icon. The docstring (@since 0.11.0) already declared everydata:URI rejected; code and doc disagreed.dashicons-admin-generic.includes/session.php—wpdm_sanitize_sessionnow preserves the client'supdatedtimestamp when valid, so two saves inside the same second no longer lose the second write to the stale-write guard.tests/phpunit/tests/wpDesktopBuildDockItems.php— normalize.to-in the synthetic hookname helper (sanitize_key('index.php')strips the dot entirely, producingindexphpinstead ofindex-php).Test plan
npm installnpm run env:start(first run pulls WP + MariaDB images, ~2–5 min)npm run test:php:install(composer install inside thetests-clicontainer)npm run test:php→ expectOK (339 tests, 723 assertions)Notes
.github/workflows/ci.yml) is not updated in this PR — itsphplane will still fail until switched to wp-env. Worth a follow-up once this lands.fix/php-testsbranch (PR ci: makenpm run test:phpactually work #2) andfix/php-tests-assertionsbranch (PR fix: resolve 3 PHPUnit failures + 1 error on trunk test code #4). Both can be closed.🤖 Generated with Claude Code