Tests: register core ability categories once in the bootstrap#854
Conversation
The WordPress test suite unhooks `wp_register_core_ability_categories()` at priority 1, so the `site` and `user` categories never exist during tests. The plugin's own abilities declare those categories, and `wp_register_ability()` refuses a category that is not registered. Every plugin ability therefore emitted an incorrect-usage notice as soon as the abilities registry booted. Three test classes worked around this with their own copy of an `ensure_*_category()` helper. The copies hid the problem instead of solving it. Whether a class passed depended on test order: run the whole suite and an earlier class had already registered the categories, so a later class passed even without its own helper. Run one class alone and it failed. On the default branch today, `SettingsTest` and `Ability_HandlerTest` both fail in isolation. Register the categories once in the test bootstrap, after the suite has unhooked core and before the plugin registers the categories it owns, then delete the three helpers. Removing them is what makes CI protect this: with the helpers gone and the bootstrap filter reverted, the full suite reports 50 errors, so a regression can no longer hide behind test ordering. Each of the three classes now passes on its own, and the full suite still passes with 986 tests.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
✅ WordPress Plugin Check Report
📊 ReportAll checks passed! No errors or warnings found. 🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #854 +/- ##
==========================================
Coverage 76.67% 76.67%
Complexity 2198 2198
==========================================
Files 100 100
Lines 9078 9078
==========================================
Hits 6961 6961
Misses 2117 2117
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The bootstrap filter runs at priority 5 and core's own callback sits at priority 10. Today the test suite unhooks core's callback, so only ours runs. If a future WordPress stops unhooking it, both would run, and the second one would report "Ability category site is already registered" and fail the suite. Skip the registration when core's callback is still hooked, so this filter does nothing on the day the suite no longer needs it.
`has_action()` returns the priority of the hooked callback, or `false` when it is not hooked. A callback hooked at priority 0 therefore reads as falsy, and the guard would register the core categories a second time. Core hooks its callback at the default priority 10 today, so this is latent. The comparison should still follow the documented contract.
Say why one missing category breaks unrelated tests, and why the setup grew a new copy with every ability. Drop the parts the code already shows.
justlevine
left a comment
justlevine
left a comment
There was a problem hiding this comment.
This makes sense to me both in concept and approach.
(Internally, we usually do this in the suite's TestCase, but that's definitely premature for the AI plugin )
3 participants
What
Register the core ability categories once in the PHPUnit bootstrap, and delete the three copies of
ensure_*_category()that test classes carried to work around their absence.Why
The WordPress test suite unhooks
wp_register_core_ability_categories(), so thesiteandusercategories do not exist during tests. Plugin abilities declare those categories, andwp_register_ability()refuses a category that is not registered.The catch is that booting the abilities registry registers every plugin ability at once. So one missing category breaks whichever test happens to touch abilities first, even a test that has nothing to do with settings or users. That is why
UsersTesthad to registersiteas well asuser. It was paying forcore/read-settings.This gets worse with every ability we add. Each new ability test class copied the same setup, and each new core category meant editing all the copies. We are at three copies on this branch, and #739 adds a fourth while it waits for review.
It also makes the suite order dependent. Run everything and an earlier class has already registered the categories, so a later class passes without its own copy. Run one class alone and it fails. On
developtoday:How
tests/bootstrap.phpregisters the categories onwp_abilities_api_categories_initat priority 5. That runs after the suite has unhooked core, and before the plugin registers the categories it owns. Categories the plugin owns, such asai-experiments, keep arriving through the plugin's own callbacks, so they need nothing here.The filter stands down if core's callback is still hooked, so it turns into a no-op the day the suite stops unhooking it.
Deleting the three helpers is what makes CI protect this. With the helpers gone and the bootstrap filter reverted, the full suite reports 50 errors. Before this change it passed either way.
Testing
Each class now passes on its own, and the full suite still passes with
Tests: 986, Assertions: 2706.Abilities\Settings\SettingsTestAbilities\Users\UsersTestAbilities_Explorer\Ability_HandlerTestTwo more checks. Simulating a future WordPress that no longer unhooks core, the filter stands down and registers nothing twice. Stacking this branch on #739 and deleting all three of
ContentTest's stubs, including the one for the plugin-ownedcontentcategory, givesOK (89 tests, 312 assertions)alone and 1082 tests green for the full suite.PHPCS and PHPStan are clean.
Follow-up
Once this lands,
ContentTest::ensure_ability_category()and its threesetUp()calls can be removed from #739.A test that wants a category it owns still has to fake the hook context, because the categories hook fires only once, when the registry boots. No test needs that today. If one does, it deserves a shared helper rather than a fourth copy.