Page MenuHomePhabricator

[EPIC] Hovercards: Test Coverage
Closed, ResolvedPublic

Description

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

See subtasks for work.

Details

Reference
bz65103

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).
Prtksxna created this task.May 9 2014, 8:20 PM
Jaredzimmerman-WMF triaged this task as Normal priority.Dec 3 2014, 12:30 AM
Jaredzimmerman-WMF updated the task description. (Show Details)
Jaredzimmerman-WMF set Security to None.
Prtksxna moved this task from Backlog to In Development on the Page-Previews board.Dec 3 2014, 5:33 AM
Krinkle added a subscriber: Krinkle.

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

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

Patch-For-Review

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

Reason:
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?

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

Jdlrobson updated the task description. (Show Details)Sep 23 2015, 9:18 PM
Jdlrobson removed a project: Patch-For-Review.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 23 2015, 9:18 PM
Jdlrobson lowered the priority of this task from Normal to Low.Dec 17 2015, 8:28 PM
Jdlrobson added a subscriber: Jdlrobson.
Krinkle removed a subscriber: Krinkle.Mar 22 2016, 10:46 PM
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)
dr0ptp4kt set the point value for this task to 3.Apr 4 2016, 4:46 PM

OK, here is my analysis so far:

1. ext.popups.core.js

Not tested:

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

Tested:

  • 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

Tested:

  • 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

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

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

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

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

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

bmansurov updated the task description. (Show Details)Apr 14 2016, 10:26 PM

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

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

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
Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptApr 19 2016, 3:34 PM
bmansurov removed bmansurov as the assignee of this task.Apr 19 2016, 9:52 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 closed this task as Resolved.Feb 16 2017, 6:49 PM
Jdlrobson claimed this task.

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

PHP Backend is well tested