At the time of this writing, https://doc.wikimedia.org/cover-extensions/AbuseFilter/ shows about 20% coverage (by lines or by functions) for what is in ./includes and 0% for what is in ./maintenance. Coverage by classes and traits is 2% for ./includes and 0% for ./maintenance.
Progress: https://tools.wmflabs.org/coverage/charts/AbuseFilter.png
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | Daimona | T201193 Code coverage is low in AbuseFilter, Aim to increase to 50% | |||
| Resolved | Daimona | T42478 AbuseFilter needs regression tests | |||
| Resolved | Daimona | T193596 Abstract out parts of AbuseFilterViewEdit that deal with saving a filter |
Change 454518 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Add missing @covers tag
Some of these potentially look like PHPUnit/Xdebug bugs that we'll need to work into smaller reproduction cases. I think you also figured out that some @covers tags were missing.
Also for cases like https://doc.wikimedia.org/cover-extensions/AbuseFilter/includes/parser/AFPData.php.html#307 where it should never happen, and therefore is impossible to write a test for, feel free to use // @codeCoverageIgnoreLine right before it
Change 454589 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Improve code coverage
Yes, now @covers tag should be right and in fact the coverage increased. As for untestable code, I'm not sure if it's worth it adding annotations everywhere: we could just be fine with 99% coverage or so, I guess.
Also, what should we use to check HTML? We'd need something to make sure that the output from View classes is correct, and maybe we should also check request values etc.
Change 454589 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Improve code coverage
Change 454782 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Improve parser coverage
Change 454782 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Improve parser coverage
Change 454027 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Improve tests for the AbuseFilter class
Change 454888 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] [WIP] Add unit tests for stashed edits
Change 455173 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Add unit tests for profiling
Change 455291 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Improve coverage for AbuseFilterTokenizer
Change 455324 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] [WIP] Add tests for global filters
I'm facing a problem when testing global filters (patch right above). For the test, we would ideally need two databases, one for local filter and one for global ones. Since I'm not aware of a way to create another database and then use both in tests, I just saved global filters to the existent DB. However, when in the AF code we search the external DB for global filters (here), the DB prefix 'unittest_' is gone, and thus the test fails. So my question is: is there a way to have two working DBs in a test or, if no, why is the prefix gone? CC'ing @Legoktm.
Do the examples provided in https://phabricator.wikimedia.org/diffusion/ECAU/browse/master/tests/phpunit/ help at all?
@Huji thanks, I'll take a look. For the moment, just a little note: this seems to happen in any of the following cases:
Getting local filters works perfectly fine instead.
EDIT: Unfortunately, CA tests don't seem to help. Also, I can't figure out if this is some sort of bug in MediaWikITestCase class or if we're using it the wrong way.
Change 455871 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Add tests for retrieving RC variables
Change 456089 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Add full tests for deprecated variables
Change 454557 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Restore (temporarily?) unit tests for CachingParser and fix it
Change 456089 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Add full tests for deprecated variables
Change 455291 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Improve coverage for AbuseFilterTokenizer
Change 473456 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Add new complete unit tests for most variables
Change 481524 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Add tests for storing and loading the variables dump
Change 481524 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Add tests for storing and loading the variables dump
Change 454888 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Add unit tests for stashed edits
Change 455173 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Add unit tests for profiling
Change 455324 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Add tests for global filters
Change 454557 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Restore (temporarily?) unit tests for CachingParser and fix it
Change 481524 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Add tests for storing and loading the variables dump
Change 503654 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Increase code coverage a bit
Change 503654 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Increase code coverage a bit
Change 503658 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Tweak coverage part 2
Change 503658 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Tweak coverage part 2
Change 503724 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Add tests for tokenizer caching
Change 503738 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Add tests for various data type casts
Change 503724 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Add tests for tokenizer caching
@Daimona I'd like to help, but know little-to-nothing about code coverage - how does one learn / what can I do?
@DannyS712 Thanks for the help! There are some good explanations on the web, just searching "code coverage" on google yields something interesting. Basically, code coverage is the amount of code which gets executed during tests.
Change 454888 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Add unit tests for stashed edits
Change 455173 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Add unit tests for profiling
Change 455324 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Add tests for global filters
Change 503738 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Add tests for various data type casts
Change 454557 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Restore unit tests for CachingParser and fix it
FTR, given that there are lots of patches bound to this task, the only missing ones are https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/AbuseFilter/+/455871/ and https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/AbuseFilter/+/473456/; It'll take some time to update the second.
Change 512476 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Add missing covers/group tags
Change 512476 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Add missing covers/group tags
Change 568029 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] tests: Increase and rebalance code coverage
Change 568029 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] tests: Increase and rebalance code coverage
Change 455871 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Add tests for retrieving RC variables
Change 627510 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Add tests for 'upload' action
Change 627766 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Add test traits for uploads and account creation
Change 629865 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Add a lot of selenium tests for the editing view
Change 627510 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Add tests for 'upload' action
Change 629865 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Add a lot of selenium tests for the editing view
@Daimona one of these tests is failing at https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MassMessage/+/631144 which shouldn't be related?
✖ cannot save an empty filter
The expression evaluated to a falsy value: assert( ViewEditPage.error.isDisplayed() )-
Hmm, it's not the first time I see it, but it always passes locally. Given what the test does, my guess is, it tries clicking the "submit" button before it works. This is not 100% convincing because the button is not JS-enriched etc., but perhaps it's worth trying. As anticipated, I cannot tell whether the fix will work because I don't get a failure locally (not even if I delay the page load).
Change 455871 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Add tests for retrieving RC variables
Change 627766 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Add test traits for uploads and account creation
This is still failing randomly, it seems https://integration.wikimedia.org/ci/job/wmf-quibble-selenium-php72-docker/63833/console It did pass on a recheck. Just letting you know :)
Hm :-/ I can't understand why. Given what the test does, I'd expect that the page might not be ready when we try clicking the button. OTOH, we're using waitForClickable(), plus that's a simple submit button with no special JS logic, so it should work immediately. Based on the comments at T264926, can we exclude that the test framework is bugged?
That said, if this failure happens again and blocks stuff, please do feel free to disable the test (by replacing it( with it.skip().
Removing task assignee due to inactivity, as this open task has been assigned to the same person for more than two years (see the emails sent to the task assignee on Oct27 and Nov23). Please assign this task to yourself again if you still realistically [plan to] work on this task - it would be welcome.
(See https://www.mediawiki.org/wiki/Bug_management/Assignee_cleanup for tips how to best manage your individual work in Phabricator.)
I'm working on this, although the task is quite tracking/neverending. Guess we can call this resolved once coverage reaches 50%.
Change 650738 had a related patch set uploaded (by Matěj Suchánek; owner: Matěj Suchánek):
[mediawiki/extensions/AbuseFilter@master] Improve code coverage
Change 647270 had a related patch set uploaded (by Matěj Suchánek; owner: Matěj Suchánek):
[mediawiki/extensions/AbuseFilter@master] Test some Consequence classes and clean up
Change 650738 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Improve code coverage
Change 647270 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Test some Consequence classes and clean up
Change 473456 abandoned by Daimona Eaytoy:
[mediawiki/extensions/AbuseFilter@master] Add new complete unit tests for most variables
Reason: