Page MenuHomePhabricator

[EPIC] Hovercards: Test Coverage
Closed, ResolvedPublic


Test coverage is very low. What needs to be set up and tested?

See subtasks for work.

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 3:14 AM
bzimport added a project: Page-Previews.
bzimport set Reference to bz65103.
bzimport added a subscriber: Unknown Object (MLST).
Jaredzimmerman-WMF updated the task description. (Show Details)
Jaredzimmerman-WMF set Security to None.

Change 179106 had a related patch set uploaded (by Prtksxna):
tests: Add render.article.createImgThumbnail


Change 179106 abandoned by Prtksxna:
tests: Add render.article.createImgThumbnail

I think I am doing something utterly useless here. The code in the test is the same as the function.

Krinkle, does this make sense?

Jdlrobson lowered the priority of this task from Medium to Low.Dec 17 2015, 8:28 PM
Jdlrobson added a subscriber: Jdlrobson.
dr0ptp4kt renamed this task from Hovercards: Write Tests to Hovercards: Test Coverage.Apr 4 2016, 4:41 PM
dr0ptp4kt updated the task description. (Show Details)

OK, here is my analysis so far:

1. ext.popups.core.js

Not tested:

  • mw.popups.removeTooltips
  • mw.popups.setupTriggers
  • mw.popups.selectPopupElements


  • mw.popups.getTitle

2. ext.popups.renderer.article.js

Not tested:

  • article.createPopup
  • article.removeParensFromText
  • article.createSVGTag
  • article.createThumbnail
  • article.createSvgImageThumbnail
  • article.createImgThumbnail
  • article.getOffset
  • article.getClasses
  • article.processPopup


  • article.getProcessedElements
  • mw.popups.render.getClosestYPosition

3. These files are not tested: ext.popups.disablenavpop.js, ext.popups.logger.js, ext.popups.settings.js.

I think this task is definitely more than a 3 pointer (probably an 8) because the extension is covered sparsely.

Change 283348 had a related patch set uploaded (by Bmansurov):
WIP: Add QUnit tests to cover ext.popups.core.js

We should take care when doing this that we write the write tests. Browser tests may be better in some places and there may not be value in testing certain units.

phuedx added a subscriber: phuedx.

… as this was brought into the sprint after it began.

Change 283566 had a related patch set uploaded (by Bmansurov):
Add basic browser tests

Change 283568 had a related patch set uploaded (by Bmansurov):
Create Popups jenkins job

Change 283762 had a related patch set uploaded (by Bmansurov):
Add QUnit tests for ext.popups.logger

I'm taking this back to transform it to an Epic and get it better fleshed out and split to pieces.

The spike already done and other work we need to do taking into account the spike's results.

@bmansurov I'll split tasks for the patches you already have, and change the patches to link to the correct bugs. We can talk tomorrow if there is something wrong.

Jhernandez removed the point value for this task.Apr 19 2016, 3:34 PM
jhobs renamed this task from Hovercards: Test Coverage to [EPIC] Hovercards: Test Coverage.Jun 1 2016, 9:50 PM
jhobs removed a project: Unplanned-Sprint-Work.
jhobs moved this task from 2015-16 Q4 to 2014-15 Q4 on the Readers-Web-Backlog board.
Jdlrobson claimed this task.

The repo is pretty well tested now. Remaining work is tracked in T133022 and T133162