Hooks.php and the HookHandlers are one of the more critical groups of files in CheckUser. Issues from it are more likely to affect more than just CheckUsers, as the handlers are called in many more places than just CheckUser (including edits and page views on certain pages). Fully testing the Hooks.php file and other handlers will ensure that patches are properly tested and therefore also encourage testing new additions to keep the high coverage.
Description
Description
Details
Details
Customize query in gerrit
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | Dreamy_Jazz | T201154 Code coverage for CheckUser is very low | |||
| Open | None | T175920 Improve unit testing for CheckUser | |||
| Resolved | Dreamy_Jazz | T315648 Fully test Hooks.php / HookHandlers | |||
| Declined | None | T321864 Add CentralAuth to the gated extensions list |
Event Timeline
Comment Actions
Change 824746 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):
[mediawiki/extensions/CheckUser@master] Add multiple integration tests for Hooks.php
Comment Actions
Change 824746 merged by jenkins-bot:
[mediawiki/extensions/CheckUser@master] Add multiple integration tests for Hooks.php
Comment Actions
Change 843927 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):
[mediawiki/extensions/CheckUser@master] Add multiple integration tests for Hooks.php
Comment Actions
Change 843927 merged by jenkins-bot:
[mediawiki/extensions/CheckUser@master] Add multiple integration tests for Hooks.php
Comment Actions
Adding selenium tests that check for a end-to-end logging of a checkuser action makes sense to me. For example:
- Selenium test logs in to an account
- Selenium test runs a check on that account
- Verifies that the last shown row for that account was as expected (in this case is a login row).
Having the structured data change would be useful here as classes could be applied for what the action is, making testing that it's the "correct" action easier as it could just be a class check.
Comment Actions
Change 850270 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):
[mediawiki/extensions/CheckUser@master] Integration test Hooks::onAuthManagerLoginAuthenticateAudit
Comment Actions
Change 850270 merged by jenkins-bot:
[mediawiki/extensions/CheckUser@master] Integration test Hooks::onAuthManagerLoginAuthenticateAudit
Comment Actions
Change 873051 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):
[mediawiki/extensions/CheckUser@master] Split maybePruneIPData and then test newly created pruneIPData
Comment Actions
Change 873084 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):
[mediawiki/extensions/CheckUser@master] Test Hooks::onUserToolLinksEdit
Comment Actions
Change 873051 merged by jenkins-bot:
[mediawiki/extensions/CheckUser@master] Split maybePruneIPData and then test newly created pruneIPData
Comment Actions
Change 873084 merged by jenkins-bot:
[mediawiki/extensions/CheckUser@master] Test Hooks::onUserToolLinksEdit
Comment Actions
Change 928527 merged by jenkins-bot:
[mediawiki/extensions/CheckUser@master] Move Hooks::onRenameUserSQL to hook handler file and unit test
Comment Actions
Change 928553 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):
[mediawiki/extensions/CheckUser@master] Move Hooks::onSpecialPage_initList to a hook handler file
Comment Actions
Change 928553 merged by jenkins-bot:
[mediawiki/extensions/CheckUser@master] Move Hooks::onSpecialPage_initList to a hook handler file
Comment Actions
Going to mark this as resolved, given that the test coverage is nearly full based on https://doc.wikimedia.org/cover-extensions/CheckUser/src/Hooks.php.html