MozillaWiki

QA/Mozmill Test Automation/Test Modules Refactor: Difference between revisions

Page Discussion

QA/Mozmill Test Automation/Test Modules Refactor (view source)

Revision as of 21:14, 16 February 2011

1,902 bytes added ,  16 February 2011
→‎Outstanding Items
 
(49 intermediate revisions by 3 users not shown)
Line 14: Line 14:
  |- valign="top"
  |- valign="top"
  | '''Status:'''
  | '''Status:'''
  | In process of determining style guidelines
  | '''on hold'''
  |- valign="top"
  |- valign="top"
  | '''Repository Location:'''
  | '''Repository Location:'''
Line 23: Line 23:
  |- valign="top"
  |- valign="top"
  | '''Documentation:'''
  | '''Documentation:'''
  | [https://docs.google.com/document/d/13OhM4T2nxYqrvCAtVicjB-PCrDeLiSOnc6N-pXw939A/edit?hl=en&authkey=CPmR7d4J Draft Proposal]
  |
* [https://docs.google.com/document/d/13OhM4T2nxYqrvCAtVicjB-PCrDeLiSOnc6N-pXw939A/edit?hl=en&authkey=CPmR7d4J Draft Proposal]
* [http://neil.rashbrook.org/JS.htm Neil Rashbrook's style guide]
* [http://javascript.crockford.com/code.html Douglas Crockford's style guide]
  |}
  |}


Line 30: Line 33:
The purpose of this project is to ensure all tests are implemented using an agreed style guideline.  The workflow of this project will mostly be parallel with the Shared Modules Refactor project.  This project will be rolled out in multiple phases.
The purpose of this project is to ensure all tests are implemented using an agreed style guideline.  The workflow of this project will mostly be parallel with the Shared Modules Refactor project.  This project will be rolled out in multiple phases.


== Phase I ==
== Style Guidelines ==
Goal: Refactor "lowest-hanging-fruit" changes<br>
The [https://developer.mozilla.org/en/Mozmill_Tests/Mozmill_Style_Guide Mozmill Style Guide] v1.0 has been created. The following refactoring work is an effort to implement those guidelines throughout the mozmill-tests repository. This refactoring will be implemented in multiple phases, outlined below.
Dependencies: Agreed upon style guidelines


== Refactoring  ==
== Refactoring  ==
; Timeouts & Delays
=== Phase I ===
* gDelay: replace with a discrete int value
{| class="fullwidth-table"
* gTimeout: replace with TIMEOUT
| Goal
 
| Implement lowest hanging fruit, make all tests reflect the styleguide
; Constants
|-
* const SOME_CONST = value;
| {{bug|}}
 
| Replace all constants with CONSTANTS
; setupModule & teardownModule
* gDelay, 100 => DELAY
* Remove ''module'' parameter
* gTimeout, 5000 => TIMEOUT
 
* sleep() parameter => SLEEP_TIMEOUT
; Mozilla Components
* unique timeouts => CONSTANT (ie. SEARCH_TIMEOUT)
* Ensure proper alignment of getService() on Cc[]
|-
 
| {{bug|}}
== Style Guidelines ==
| Replace discrete, unchanging values with CONSTANTS
; Timeouts & Delays
|-
* Proposed Guidelines:
| {{bug|}}
** Delay: Use discrete value
| Migrate tests away from testGeneral
** Timeout: Use global TIMEOUT
|-
* Henrik: Timeouts should be encapsulated within a global shared module
| {{bug|}}
* Geo: Use default params where appropriate, wrapper functions where not
| All test pages in LOCAL_TEST_PAGES array
|-
| {{bug|}}
| Replace waitForEval() with waitFor() where possible
|-
| {{bug|}}
| Replace DELAY/TIMEOUT with waitFor() where possible
|-
| {{bug|}}
| License block formatting
* "the Mozilla Foundation"
* contributor list alignment
|-
| {{bug|}}
| All error messages use format:
* message + " - got " + condition1 + ", expected " + condition2
* ''message'' uses positive wording
|-
| {{bug|}}
| Whitespace formatting
* No trailing whitespace
* 1 newline at end of file
* 2-space indentation where appropriate
* Wrap on 80-characters where appropriate
|-
| {{bug|}}
| Replace sleep() with waitFor() where possible
|-
| {{bug|}}
| Extract any ''new elementslib...'' from function parameter list
|-
| {{bug|}}
| Replace all array iteration with Array.forEach()
|-
| {{bug|}}
| Replace anonymous functions with named functions
|-
| {{bug|}}
| Adopt consistent commenting style
|-
| {{bug|}}
| All iterators as ''i'', not ''ii''
|-
| {{bug|}}
| Adopt consistent conditional cuddling
|-
| {{bug|}}
| All included modules should use variable names starting with capital
|-
| {{bug|}}
| All conditionals use !== or ===
|}


; Constants
''Note: The above comes from the agreed to refactoring [http://etherpad.mozilla.com:9000/mozmill-test-refactor guidelines]''
* Proposed Guidelines:
** Exist between ''requires'' and ''setupModule()''
** const SOME_CONSTANT = value;
* Geo: All ''unchanging'' variables as constants


; Function Signatures
=== Phase II ===
* Proposed Guidelines:
Goal: Bring tests in-line with Shared Module Refactor
** var nameOfFunction = function() {
* Geo: Agree on open-brace on same line but common in JS to have an next-line exception for named functions; could go either way.


; ''module'' as a param for setupModule & teardownModule
== Outstanding Items ==
* Proposed Guidelines:
Discussion [http://mozqa.ietherpad.com/refactor-discussion here]
** ???
* Geo: Strange to have a function not declare a parameter that’s actually being supplied, but maybe that’s more normal in JS.
* Henrik: Removing module was probably a mistake; used for injecting global functions into the module scope.


; Mozilla Components
; Generally Agreed
* Proposed Guidelines:
* We should always use the triple operator to not experience strange behavior
** var obj = Cc[“@mozilla.org/component;1”].getService(Ci.interface);
** ===/!== instead of ==/!=
** Declare in setupModule()
* Variables for imported modules have to start with a capital letter
** For word-wrapping, split after the ., getService() aligned with Cc[]
** i.e. var Tabs = require(".../tabs")
* Class names have to start with a capital letter
* setupModule(module) and teardownModule(module) have to specify the module parameter
* If variable name contains an acronym, capitalize the acronym
* Error messages should use the following format:
** Boolean: "<element> is <state>."
** Value: "<element> is <state>: got 'value', expected 'value'"
** Example: "Password notification is visible"
** Example: "mozilla.org is loaded: got '<loaded_domain>', expected 'mozilla.org'"
* sleep() should be avoided and only used when...
** element is not immediately available
** state is not immediately available
** waitFor() fails to capture event


; LOCAL_TEST_PAGES
; Needs Discussion
* Proposed Guidelines:
* TestFilesAPI for loading of test files
** Encapsulate all local test pages within a const LOCAL_TEST_PAGES array
** We should have a module specific to loading of test files
** Each element (test page) consists of a URL and an ID
** We should also check how we can make sure those locations are easily replacable with other servers (apache, ...)
* Henrik: We should use Name, Link, and XPath as well
* Parameters should be named using aParam format
** shows that a variable inside a long function is a parameter and hasn't been declared locally
* Constant scope - should they be global and local?
Ashughes
Confirmed users
14,525

edits

Retrieved from "https://wiki.allizom.org/Special:MobileDiff/266266...285247"