Confirmed users
213
edits
(create page) |
(First revision) |
||
| Line 7: | Line 7: | ||
First of all, make sure above mentioned guidelines are not violated, and check for the following: | First of all, make sure above mentioned guidelines are not violated, and check for the following: | ||
<br> | <br> | ||
=== Look for transitions that are not waited on === | === Look for transitions that are not waited on === | ||
Sometimes tests can intermittently fail. One reason of the failures can be the lack of waits. While reviewing code, check if every event that happens on the screen, is detailed in the Page class. For example: a panel that moves elsewhere, a button that gets enabled after the others, an element that can't be tapped until another is displayed, etc. | |||
=== Check for change/removal of methods === | === Check for change/removal of methods === | ||
In | In some occasions, some old code is removed. While performing you review, make sure there is no other calls to this class or method. | ||
If you have applied the patch form the Pull Request, <source>git grep my_function</source> can show you if my_function is still called somewhere. | |||
=== Are the locators correct? Could they be improved? === | === Are the locators correct? Could they be improved? === | ||
Locators in Gaia are usually either ID, or simple CSS selectors (with either CSS classes or data-l10n-id). Smaller locators are better. If one looks too long or not self-explanatory, don't hesitate to open WebIDE to see the DOM or ask questions in the review. | |||
=== Are the locators/methods used that were added? === | === Are the locators/methods used that were added? === | ||
TBD - mwargers | TBD - mwargers | ||
A linter would also help to enforce it, see: https://bugzilla.mozilla.org/show_bug.cgi?id=1186388 | A linter would also help to enforce it, see: https://bugzilla.mozilla.org/show_bug.cgi?id=1186388 | ||
=== Are workarounds explained in the code file? === | === Are workarounds explained in the code file? === | ||
Sometimes there is no other way than injecting a cryptic JavaScript piece of code, until something changes in Marionette or Firefox OS. In order to help the next readers of the code, make sure the workaround is near a bug number that tracks its removal. | |||