Page MenuHomePhabricator

Increase unit test coverage in ve/ce/KeyDownHandlers : LinearEscape, LinearTab, TableArrow, TableDelete, TableEnter
Closed, ResolvedPublic

Description

Review the code and add test coverage where possible. Current statement coverage is attached.

Total124/13591.85%
LinearEscape14/1687.50%
LinearTab26/2989.66%
TableArrow56/5896.55%
TableDelete16/1794.12%
TableEnter12/1580.00%

Event Timeline

marcella triaged this task as Normal priority.Oct 15 2018, 7:44 PM
marcella created this task.

Change 468719 had a related patch set uploaded (by Esanders; owner: Esanders):
[VisualEditor/VisualEditor@master] KeyDownTests: Remove unused constructor calls for all static classes

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

Change 468720 had a related patch set uploaded (by Esanders; owner: Esanders):
[VisualEditor/VisualEditor@master] KeyDown tests: Add test for missing cases in TableEnter/LinearEscape

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

Esanders renamed this task from Increase unit test coverage in ve/ce/KeyDownHandlers : LinearEscapeKeyDownHandler, LinearTabKeyDownHandler, TableArrowKeyDownHandler, TableDeleteKeyDownHandler, TableEnterKeyDownHandler to Increase unit test coverage in ve/ce/KeyDownHandlers : LinearEscape, LinearTab, TableArrow, TableDelete, TableEnter.EditedOct 20 2018, 3:41 PM
Esanders updated the task description. (Show Details)

Coverage in these files after above patches is 130/130 (100%).

Esanders claimed this task.Oct 20 2018, 4:10 PM
Esanders moved this task from Incoming to Code review on the VisualEditor (Current work) board.

Change 468719 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] KeyDownTests: Remove unused constructor calls for all static classes

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

Change 468720 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] KeyDown tests: Add tests for missing cases to cover tab/escape/enter

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

Change 469038 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (b2ece1abc)

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

Change 469038 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (6c288b44f)

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

Jdforrester-WMF added a subscriber: Jdforrester-WMF.

Now all at 100% statements, but two missed conditional branches of four for TableDeleteKeyDownHandler. Good enough?

Can we add a screenshot here of the new test coverage, so that it can be compared to the old test coverage image in the description? I don't know where the test coverage dashboard shown here lives, so I can't do that myself.

Deskana closed this task as Resolved.Nov 1 2018, 11:26 AM

Cool, thanks! That looks like a good enough improvement to call this resolved.