fix: resolve 3 PHPUnit failures + 1 error on trunk test code#4
Closed
epeicher wants to merge 1 commit into
Closed
Conversation
Pre-existing trunk issues that became visible once the PHP test
infra on fix/php-tests started actually running phpunit in CI.
1. wpDesktopBuildDockItems::test_placement_distinguishes_core_from_plugin_menus
(ERROR: Undefined array key "menu-index-php")
The test helper `make_menu_row` built synthetic hooknames via
`'menu-' . sanitize_key( $slug )`. `sanitize_key` strips `.`
entirely, so `index.php` became `indexphp`, producing dock item
id `menu-indexphp` — but the test asserted `menu-index-php`.
Real WP hooknames normalize dots to dashes; pre-replace in the
helper so synthetic ids match what the tests assert.
2+3. Security::test_data_uri_rejected and
WpDesktopBuildDockItems::test_icon_data_svg_rejected
`wpdm_sanitize_dock_icon` carried a special-case branch that
accepted `data:image/svg+xml;base64,<valid-base64>` values,
directly contradicting its own JSDoc ("@SInCE 0.11.0 Rejects
`data:` URIs outright; previously accepted `data:image/svg+xml`
values"). Drop the branch so every data: URI falls through to
the fallback dashicon, matching the documented Phase-11 intent.
4. WpDesktopSession::test_save_session_accepts_equal_timestamp
(Failed asserting that false is true.)
`wpdm_sanitize_session` always stamped `$clean['updated'] =
time()` before the save logic ran, overwriting whatever
timestamp the client sent. That meant the stale-write guard in
`wpdm_save_session` compared `incoming` against a server-time
`stored`, so equal-timestamp writes (the scenario the test
covers) looked strictly-older and got rejected. Preserve the
client's `updated` when valid, fall back to `time()` only when
missing or invalid.
5 tasks
Collaborator
Author
|
Closed in favour of #7 |
9 tasks
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.
Depends on #2 for the working
vendor/bin/phpunit+ WP test-suite install. Once the CI infra from #2 started actually running PHPUnit, four real issues became visible in trunk's code/tests. Each fix below is targeted and explains why rather than re-stating what.Fixes
1.
WpDesktopBuildDockItems::test_placement_distinguishes_core_from_plugin_menus— ERRORUndefined array key "menu-index-php".The test helper
make_menu_rowbuilt synthetic hooknames as'menu-' . sanitize_key( $slug ).sanitize_keystrips.entirely, soindex.php→indexphp, producing dock idmenu-indexphp. The assertion expectsmenu-index-php(the real WP hookname style that normalizes dots to dashes).Fix: pre-replace
.→-in the slug before passing it tosanitize_keyinside the helper.2+3.
Security::test_data_uri_rejectedandWpDesktopBuildDockItems::test_icon_data_svg_rejected— FAILURESBoth expect every
data:URI (includingdata:image/svg+xml;base64,…) to fall back todashicons-admin-generic.wpdm_sanitize_dock_iconcarried a special-case branch that accepted base64-encoded SVG data URIs, directly contradicting its own@since 0.11.0 Rejects data: URIs outrightdocstring.Fix: drop the branch so every
data:URI falls through to the fallback, matching the documented Phase-11 intent.4.
WpDesktopSession::test_save_session_accepts_equal_timestamp— FAILUREFailed asserting that false is true.on the second save.wpdm_sanitize_sessionalways stamped$clean['updated'] = time()before the save logic ran, overwriting whateverupdatedthe client sent. That meant the stale-write guard comparedincomingagainst a server-timestored, so equal-timestamp writes looked strictly-older and got rejected.Fix: preserve the client's
updatedwhen valid, fall back totime()only when missing or invalid.test_save_session_rejects_stale_writeandtest_save_session_accepts_missing_timestampcontinue to pass under the same logic.Test plan
Tests: 331, Assertions: 698, Errors: 0, Failures: 0on both PHP 8.1 and 8.2 matrix legsjslane: still green