| xSavitar | |
| Jan 27 2019, 4:18 PM |
| F28222044: Screen Shot 2019-02-15 at 10.27.40.png | |
| Feb 15 2019, 9:28 AM |
| F28221480: Screen Shot 2019-02-15 at 09.10.11.png | |
| Feb 15 2019, 8:18 AM |
Only getPluralLicenseInfo() has been fully covered in this PHP file. The goal here is to cover all other methods in this file one step at a time. Methods coverage below;
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Declined | None | T208761 [Epic] Improve PHPUnit coverage in MobileFrontend to 50% | |||
| Declined | None | T214800 PHPUnit: Cover all methods in MobileFrontend.skin.hooks.php |
Change 486811 had a related patch set uploaded (by D3r1ck01; owner: Derick Alangi):
[mediawiki/extensions/MobileFrontend@master] Add PHPUnit test for MobileFrontendSkinHooks::getTermsLink()
This task will go through different rounds of TODO, Doing and Needs code review as I'll be writing tests for the methods one at a time.
Change 486811 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Add PHPUnit test for MobileFrontendSkinHooks::getTermsLink()
Change 487074 had a related patch set uploaded (by D3r1ck01; owner: Derick Alangi):
[mediawiki/extensions/MobileFrontend@master] Add PHPUnit test for ::interimTogglingSupport() method
Change 487074 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Add PHPUnit test for ::interimTogglingSupport() method
Change 487475 had a related patch set uploaded (by D3r1ck01; owner: Derick Alangi):
[mediawiki/extensions/MobileFrontend@master] Add PHPUnit test for MobileFrontendSkinHooks::gradeCImageSupport()
Change 487475 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Add PHPUnit test for MobileFrontendSkinHooks::gradeCImageSupport()
Change 490600 had a related patch set uploaded (by D3r1ck01; owner: Derick Alangi):
[mediawiki/extensions/MobileFrontend@master] Add PHPUnit test case for MobileFrontendSkinHooks::getLicense()
Change 490600 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Add PHPUnit test case for MobileFrontendSkinHooks::getLicense()
@Jdlrobson, code coverage https://doc.wikimedia.org/cover-extensions/MobileFrontend/MobileFrontend.skin.hooks.php.html reports 95.56% for ::getLicense() which is not the goal.
One line is not executed (L#153).
Made a follow-up patch: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MobileFrontend/+/490808 to fix this issue.
Thanks @thiemowmde for reviewing and merging the patch that attempts to fix the issue and nicely does so :), per https://doc.wikimedia.org/cover-extensions/MobileFrontend/MobileFrontend.skin.hooks.php.html, we now have the method fully covered to 100%.
@Jdlrobson should this task be in ready for signoff? According to https://doc.wikimedia.org/cover-extensions/MobileFrontend/MobileFrontend.skin.hooks.php.html, these methods still have 0% coverage:
- prepareFooter
- desktopFooter
- mobileFooter
Matching the task description, yes! I'll have to make 1 more patch to cover these 3 methods at once, so this class is stepped up to 100% once and for all :)
@Jdlrobson & @pmiazga, in preparation for finishing up the remaining tests, I noticed injecting MobileContext into prepareFooter() is an issue so I wrote up a service wiring for this. See: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MobileFrontend/+/494825. Is this a good idea?
This approach allows one to mock context and inject it into a test when need be but please if there are better ideas, let me know.
This ticket is over five years old and the layout of the repo has changed considerably since. As much as I love any excuse to write test coverage I'm going to close this ticket, but feel free to create a new one with an updated description!