Page MenuHomePhabricator

MFA: add tests for Drawer and CtaDrawer
Closed, ResolvedPublic4 Story Points

Description

Currently CtaDrawer.js and Drawer.js have no test coverage and this makes me a sad panda. This task aims to make this 100%.

Precursors:

  • Ensure T207805 has happened before beginning on this change. Tests for tests/qunit/mobile.references/ReferencesDrawer.test.js do exist, and we'll want to ensure the initial figure for code coverage on the wiki page is accurate before starting this work.

Developer notes

The Drawer and CtaDrawer are mostly logic-less templates. They do not contain gateways and are unlikely to need any kind of refactoring.

Sign off steps

Coverage should be 100%. If not, and there is a reason, please make a note on the table. If coverage is not 100% push this back to needs more work to find out why.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 7 2018, 12:47 AM
Jdlrobson triaged this task as Normal priority.Nov 7 2018, 12:47 AM
Jdlrobson set the point value for this task to 4.Nov 7 2018, 5:35 PM

We were jumping between a 5 and a 3 on this one.

Jdlrobson renamed this task from Test coverage for Drawer and CtaDrawer to MFA: Test coverage for Drawer and CtaDrawer.Nov 15 2018, 6:52 PM
Niedzielski renamed this task from MFA: Test coverage for Drawer and CtaDrawer to MFA: add tests for Drawer and CtaDrawer.Nov 28 2018, 11:32 PM

Change 476428 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/MobileFrontend@master] Hygiene: add tests for Drawer

https://gerrit.wikimedia.org/r/476428

Change 476428 merged by Jdlrobson:
[mediawiki/extensions/MobileFrontend@master] Hygiene: add tests for Drawer

https://gerrit.wikimedia.org/r/476428

Change 476607 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/MobileFrontend@master] Hygiene: add test for CtaDrawer

https://gerrit.wikimedia.org/r/476607

nray claimed this task.Dec 4 2018, 6:42 PM
nray removed nray as the assignee of this task.
nray added a subscriber: nray.
nray claimed this task.Dec 6 2018, 8:34 PM
nray removed nray as the assignee of this task.Dec 6 2018, 11:53 PM

Change 476607 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Hygiene: add tests for CtaDrawer

https://gerrit.wikimedia.org/r/476607

phuedx added a subscriber: phuedx.Dec 11 2018, 1:06 PM

Coverage should be 100%. If not, and there is a reason, please make a note on the table. If coverage is not 100% push this back to needs more work to find out why.

When I run npm run coverage I see the following:

| CtaDrawer.js                    |      100 |      100 |      100 |      100 |                   |
| Drawer.js                       |      100 |      100 |      100 |      100 |                   |

This change affects the call to action shown to an anonymous user when they click the watchstar and affects the reference drawer so both of those should be tested.

phuedx claimed this task.Dec 11 2018, 1:31 PM

This change affects the call to action shown to an anonymous user when they click the watchstar and affects the reference drawer so both of those should be tested.

Both features work just fine on the Beta Cluster.