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

← Older edit

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

Revision as of 21:14, 16 February 2011

1,871 bytes added , 16 February 2011

→‎Outstanding Items

Ashughes

Confirmed users

14,525

edits

@@ Line 14: / Line 14: @@
   |- valign="top"
   | '''Status:'''
-  | In process of determining style guidelines
+  | '''on hold'''
   |- valign="top"
   | '''Repository Location:'''
@@ Line 38: / Line 38: @@
 == Refactoring  ==
 === Phase I ===
-Goal: Implement lowest hanging fruit
+{| class="fullwidth-table"
+ | Goal
+ | Implement lowest hanging fruit, make all tests reflect the styleguide
+ |-
+ | {{bug|}}
+ | Replace all constants with CONSTANTS
+* gDelay, 100 => DELAY
+* gTimeout, 5000 => TIMEOUT
+* sleep() parameter => SLEEP_TIMEOUT
+* unique timeouts => CONSTANT (ie. SEARCH_TIMEOUT)
+ |-
+ | {{bug|}}
+ | Replace discrete, unchanging values with CONSTANTS
+ |-
+ | {{bug|}}
+ | Migrate tests away from testGeneral
+ |-
+ | {{bug|}}
+ | 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 ===
+ |}
+''Note: The above comes from the agreed to refactoring [http://etherpad.mozilla.com:9000/mozmill-test-refactor guidelines]''
 === Phase II ===
@@ Line 46: / Line 117: @@
 Discussion [http://mozqa.ietherpad.com/refactor-discussion here]
-; Arrays
+; Generally Agreed
-* Block style
+* We should always use the triple operator to not experience strange behavior
-** What style?
+** ===/!== instead of ==/!=
-** Single Index
+* Variables for imported modules have to start with a capital letter
-** Multiple Index
+** i.e. var Tabs = require(".../tabs")
-** Multiple Index, Multiple Object
+* Class names have to start with a capital letter
+* setupModule(module) and teardownModule(module) have to specify the module parameter
-; Sleep()
+* If variable name contains an acronym, capitalize the acronym
-* When is sleep() ok? Is it ever ok?
+* Error messages should use the following format:
+** Boolean: "<element> is <state>."
-; Local Test Pages
+** Value: "<element> is <state>: got 'value', expected 'value'"
-<pre>
+** Example: "Password notification is visible"
-const LOCAL_TEST_PAGES = [
+** Example: "mozilla.org is loaded: got '<loaded_domain>', expected 'mozilla.org'"
-  { url: 'some url', id: 'some id' },
+* sleep() should be avoided and only used when...
-  { url: 'some url', id: 'some id' }
+** element is not immediately available
-];
+** state is not immediately available
-</pre>
+** waitFor() fails to capture event
-* Should always be declared as a constant Array
-* Use array block formatting
-* Name "LOCAL_TEST_PAGES"
-; Test Files API Module
-* We should have a module specific to loading of test files
-; Parameter in setupModule() & teardownModule()
-* Is module needed, wanted?
-; Error Messages
-<pre>
-"Expected <element> to be <state>"
-"Expected password-save notification bar to be visible"
-</pre>
-* Messages should be positive in nature
-* <element> should be what the element represents
-* <state> should be what state we expect that element to be in
-; Parameter Naming
-* param or aParam?
-* Example: label or aLabel
-* the latter is common in Mozilla coding styles
-; Scope of Constants
-* Global or within the tightest scope?
-; Review Guidelines
+; Needs Discussion
-What are our "golden-rule" guidelines for different review states?
+* TestFilesAPI for loading of test files
-* r?
+** We should have a module specific to loading of test files
-* r+
+** We should also check how we can make sure those locations are easily replacable with other servers (apache, ...)
-* r-
+* Parameters should be named using aParam format
-* feedback?
+** shows that a variable inside a long function is a parameter and hasn't been declared locally
-* feedback+
+* Constant scope - should they be global and local?
-* feedback-