Page MenuHomePhabricator

Spike: What's tested and what needs testing in Hovercards
Closed, ResolvedPublic

Description

What is tested and what needs to be tested in Hovercards?

Event Timeline

Restricted Application removed a project: Patch-For-Review. · View Herald TranscriptApr 19 2016, 9:11 AM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Results from @bmansurov

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.

bmansurov removed bmansurov as the assignee of this task.Apr 19 2016, 9:58 PM
bmansurov added a subscriber: bmansurov.
Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptApr 19 2016, 9:58 PM

@bmansurov Did you review all the files?

I see there are for example

  • resources/ext.popups.renderer/desktopRenderer.js
  • resources/ext.popups.renderer/mobileRenderer.js

They are not on the analysis. Is there a reason for that? :) Should we reopen this?

@bmansurov I think they don't have any tests, at the very least the would need some browser tests to check they render in the different environments, what do you think.

bmansurov added a comment.EditedApr 20 2016, 12:19 PM

@Jhernandez you're right. I totally missed those two folders and all files in them. No need to re-open as those have already been caught by you.