⚓ T133020 Unit test ext.popups.core.js

Project	Branch	Lines +/-	Subject
mediawiki/extensions/Popups	master	+32 -0	Add test to cover mw.popups.setupTriggers
mediawiki/extensions/Popups	master	+7 -0	Add missing mw.popups.selectPopupElements test case
mediawiki/extensions/Popups	master	+89 -0	Add QUnit tests to cover ext.popups.core.js

Status	Assigned	Task
Declined	Jdlrobson	T111329 [GOAL] Page previews on mobileweb
Resolved	phuedx	T133947 [EPIC] Reduce bloat of Page Previews RL module(s)
Resolved	Jdlrobson	T144522 [EPIC] Hovercards technical debt and quality enhancements
Resolved	Jdlrobson	T67103 [EPIC] Hovercards: Test Coverage
Resolved	Jdlrobson	T133020 Unit test ext.popups.core.js
Resolved	Jdlrobson	T133163 BetaFeatures test failure

Jhernandez created this task.Apr 19 2016, 9:27 AM

Restricted Application removed a project: Patch-For-Review. · View Herald TranscriptApr 19 2016, 9:27 AM

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

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

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

gerritbot added a project: Patch-For-Review.Apr 19 2016, 9:28 AM

Jhernandez moved this task from Needs Analysis to -1 (Needs More Work) on the Reading-Web-Sprint-70-Lady-and-the-Trumps board.Apr 19 2016, 9:29 AM

bmansurov moved this task from -1 (Needs More Work) to Code Review on the Reading-Web-Sprint-70-Lady-and-the-Trumps board.Apr 19 2016, 12:13 PM

phuedx moved this task from Code Review to To Do on the Reading-Web-Sprint-70-Lady-and-the-Trumps board.Apr 20 2016, 5:42 PM

phuedx moved this task from To Do to Code Review on the Reading-Web-Sprint-70-Lady-and-the-Trumps board.

Danny_B moved this task from Unsorted to Test coverage on the Technical-Debt board.Apr 21 2016, 8:53 PM

phuedx updated the task description. (Show Details)Apr 22 2016, 8:53 AM

Change 283348 merged by jenkins-bot:
Add QUnit tests to cover ext.popups.core.js

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

ReleaseTaggerBot added a project: MW-1.27-release (WMF-deploy-2016-04-26_(1.27.0-wmf.22)).Apr 22 2016, 9:00 AM

Change 284873 had a related patch set uploaded (by Phuedx):
Add missing mw.popups.selectPopupElements test case

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

phuedx claimed this task.Apr 22 2016, 9:28 AM

phuedx added a subscriber: bmansurov.

Change 284876 had a related patch set uploaded (by Phuedx):
Add test to cover mw.popups.setupTriggers

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

Change 284876 abandoned by Phuedx:
Add test to cover mw.popups.setupTriggers

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

Not tested:

mw.popups.setupTriggers

After a few attempts at getting 284876 to pass in the build environment, I can only conclude that mw.popups.scrolled is being set to true and the "trigger handler" (event listener) is short-circuiting.

I abandoned the change as it felt awkward to override the global state or to manipulate the scroll position of the browser to ensure that the test passes.

dr0ptp4kt added a project: Reading-Web-Sprint-71-Matisse-Monet-Kandinsky-and-the-Departing-Painters.Apr 25 2016, 4:07 PM

dr0ptp4kt moved this task from Needs Analysis to Code Review on the Reading-Web-Sprint-71-Matisse-Monet-Kandinsky-and-the-Departing-Painters board.

284873 still needs review…

bmansurov moved this task from Code Review to -1 (Needs More Work) on the Reading-Web-Sprint-71-Matisse-Monet-Kandinsky-and-the-Departing-Painters board.Apr 25 2016, 5:25 PM

phuedx moved this task from -1 (Needs More Work) to Code Review on the Reading-Web-Sprint-71-Matisse-Monet-Kandinsky-and-the-Departing-Painters board.Apr 26 2016, 8:27 AM

The above patch has been +2'ed. Are we also going to test mw.popups.setupTriggers?

Change 284873 merged by jenkins-bot:
Add missing mw.popups.selectPopupElements test case

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

ReleaseTaggerBot added a project: MW-1.27-release (WMF-deploy-2016-05-03_(1.27.0-wmf.23)).Apr 26 2016, 2:00 PM

bmansurov moved this task from Code Review to -1 (Needs More Work) on the Reading-Web-Sprint-71-Matisse-Monet-Kandinsky-and-the-Departing-Painters board.Apr 26 2016, 9:19 PM

@bmansurov: Could you clarify why you moved this into -1 (Needs More Work)?

Change 284876 restored by Phuedx:
Add test to cover mw.popups.setupTriggers

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

In T133020#2230317, @phuedx wrote:

Not tested:

mw.popups.setupTriggers

After a few attempts at getting 284876 to pass in the build environment, I can only conclude that mw.popups.scrolled is being set to true and the "trigger handler" (event listener) is short-circuiting.

I abandoned the change as it felt awkward to override the global state or to manipulate the scroll position of the browser to ensure that the test passes.

I opted to manipulate global state… 284876 is ready for review.

Note well that the test failures are due to T133163.

Those tests should probably call HTMLForm::loadData() before HTMLForm::getHTML(). Nevertheless, https://gerrit.wikimedia.org/r/#/c/285623/ should fix them.

MBinder_WMF added a project: Readers-Web-Backlog.Apr 27 2016, 5:10 PM

MBinder_WMF moved this task from Incoming to 2015-16 Q4 on the Readers-Web-Backlog board.Apr 27 2016, 5:13 PM

Jdlrobson moved this task from Code Review to Blocked Externally on the Reading-Web-Sprint-71-Matisse-Monet-Kandinsky-and-the-Departing-Painters board.Apr 27 2016, 8:42 PM

Jdlrobson moved this task from Blocked Externally to Code Review on the Reading-Web-Sprint-71-Matisse-Monet-Kandinsky-and-the-Departing-Painters board.Apr 28 2016, 12:18 AM

phuedx removed phuedx as the assignee of this task.Apr 28 2016, 4:47 AM

phuedx closed subtask T133163: BetaFeatures test failure as Resolved.

phuedx updated the task description. (Show Details)

phuedx moved this task from Code Review to Ready for Signoff on the Reading-Web-Sprint-71-Matisse-Monet-Kandinsky-and-the-Departing-Painters board.

phuedx added a subscriber: phuedx.

Change 284876 merged by Phuedx:
Add test to cover mw.popups.setupTriggers

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

I wrote one of the changes for this task so I probably shouldn't sign this off.