QA/Mozmill Test Automation/Test Modules Refactor

Overview

Lead:	Anthony Hughes
Co-workers:	TBD
Dates:	TBD
Status:	In process of determining style guidelines
Repository Location:	TBD
Tracking Bug(s)	bug 604700
Documentation:	Draft Proposal

Project Details

Summary

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.

Style Guidelines

Timeouts & Delays

• Proposed Guidelines:
  • Delay: Use discrete value
  • Timeout: Use global TIMEOUT
• Henrik: Timeouts should be encapsulated within a global shared module
• Geo: Use default params where appropriate, wrapper functions where not

Constants

• Proposed Guidelines:
  • Exist between requires and setupModule()
  • const SOME_CONSTANT = value;
• Geo: All unchanging variables as constants

Function Signatures

• Proposed Guidelines:
  • Top-level functions should be named ie. function testDisableEnablePlugin() {
  • All other functions should be anonymous ie. var nameOfFunction = function() {
  • Opening brace should exist on the signature line
• Geo: Only top-level and object funtions should be named
• 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

• Proposed Guidelines:
  • ???
• 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

• Proposed Guidelines:
  • var obj = Cc[“@mozilla.org/component;1”].getService(Ci.interface);
  • Declare in setupModule()
  • For word-wrapping, split after the ., getService() aligned with Cc[]

LOCAL_TEST_PAGES

• Proposed Guidelines:
  • Encapsulate all local test pages within a const LOCAL_TEST_PAGES array
  • Each element (test page) consists of a URL and an ID
• Henrik: We should use Name, Link, and XPath as well
• Aaron: Establish a module/API to handle loading of any needed test files

Array Formatting

• Proposed Guidelines:
  • Each array element on its own line
  • Each element member on its own line

const LOCAL_TEST_PAGES = [
 { id: “Some_ID”,
   url: LOCAL_TEST_FOLDER + ‘someFolder/someFile.html’ },
 { id: “Some_ID”,
   url: LOCAL_TEST_FOLDER + ‘someFolder/someFile.html’ },
 { id: “Some_ID”,
   url: LOCAL_TEST_FOLDER + ‘someFolder/someFile.html’ }
];

Assertions

• Use waitFor() for all timeout assertions

controller.waitFor(function() {
  return someObject.value == expectedValue;
}, TIMEOUT, 100, “My failure message”);

• Use assertDOMProperty() for all DOM property assertions

controller.assertDOMProperty(someObject.someProperty, expectedValue);

• Use assertJSProperty() for all JS property assertions

controller.assertJSProperty(someObject.someProperty, expectedValue);

• Geo: Not necessarily needs definition in style guide; “Use the most appropriate function” can be a common sense standard.

Element Before Action

• Proposed Guideline:
  • Always declare an element before performing an action on it
  • Example:

var obj = new elementGetter(params);
controller.action(obj, params);

• Geo: Element getting will be considerably simpler post-refactor and won’t require a long line.

Iteration

• Proposed Guideline:
  • Use array.forEach() for iterating arrays
  • Use traditional for() for iterating strings

array.forEach(function(elem, [index, [array]]) {
  statements;   
}, [thisObject]);

• Geo: Documentation should call out that thisObject is the fourth parameter
• Henrik: index and array are optional but can be helpful in some cases when you need the current index or other elements from the array.

Word-wrapping and Indentation

• Proposed Guideline:
  • Adhere to an 80 character per line limitation
  • In most cases, indentation should be 2-spaces from the parent
• Geo: This keeps it mostly aligned with the actual structure. When we pull it back to the 0-column, makes things that are variables decls look like flow control structures instead, and I really don’t like that.

Examples:

• Function call:

controller.function(
  param1,
  param2,
  paramN
);

• Variable from Function call:

var obj = controller.method(
            param1,
            param2,
            paramN
          );

• XPath:

var obj = new elementslib.Lookup(
            controller.window.document,
            ‘/path’ +
            ‘/to’ +
            ‘/object’
          );

• Arrays:

var obj = [
  { child: ‘value’,
    child: ‘value }
  { child: ‘value’,
    child: ‘value}
];

sleep

• Proposed Guideline:
  • Avoid use of sleep() where possible and use waitFor() instead
• Geo: If you’re only saving a few milliseconds and not causing a robustness issue, you should use sleep for simplicity’s sake.

Misc Tests

• Proposed Guideline:
  • No "general" or "misc" tests should exist ie. no firefox/testGeneral/
• Geo: Sometimes a “misc” folder makes sense. We just might be leaning on it too hard. Agree we should review the organization.

One Test per File

• Proposed Guideline:
  • Only one test per file
• Geo: Basically OK with that. This means we’ll be managing thousands of files eventually. However, unit test “common setup” paradigm doesn’t work that well with UI tests, I’m cool subverting it with 1:1.

Commenting

• Proposed Guideline:
  • Block-style for function headers
  • JSDoc-style for additional helper functions

Camel-case

• Proposed Guideline:
  • Use camel-case for all non-constant variables and functions

var thisIsAVariable = value;
var thisIsATestMethod = function() {

Notes on the Goals

• Geo: Style guide should offer guidelines for...
  • naming/caps
  • commenting style
  • block style
  • indentation style
  • line length
  • local idioms (waitFor, assert)
  • things we lock down because alternatives are risky (array.forEach)
• Aaron: We should offer style guidelines on the license block

Style Guidelines

1. Local variables should be named using camel-case

 var exampleVariableName = value;

2. Constants should be named using all-caps

const EXAMPLE_CONSTANT = value;

3. Blocks of code should be commented in-line:

  // What the code does
  some code;

4. Functions should use JSDoc block-style comments:

  /**
   * Purpose of the function
   *
   * @param {type} paramName
   *        Purpose of the parameter
   */
   function someFunction(someParam) {

5. Arrays block-style

  var obj = [
    { child: ‘value’,
      child: ‘value }
    { child: ‘value’,
      child: ‘value}
  ];

6. Named Functions

  function someFunction() {
    some code;
  }

7. Anonymous Functions

  var someFunction() = function() {
    some code;
  }

8. Function Usage

  var someVariable = someFunction(
                       param1,
                       param2,
                       paramN,
                     );

9. Function Parameter Concatenation

  var someVariabe = someFunction(param1,
                      'some' +
                      'concat' +
                      'string'
                    );

10. waitFor() block-style

  controller.waitFor(function() {
    return something == somethingElse;
  }, "Some error message", TIMEOUT);

11. XPath: split on '/'

 var path = "/something" +
            "/something" + 
            "/something";

12. Lines of code should be indented 2-spaces to the right of their parent

var cancelButton = new elementslib.Lookup(controller.window.document,
                     'path/to/element'
                   );

13. Component declarations should be indented in line with the parent

var svc = Cc["string/for/service/component"].
          getService(Ci.nsINameOfInterface);

14. Lines of code should not exceed 80 characters

15. Use waitFor() as much as possible. Only use sleep() when a waitFor() fails.

16. Always use a discrete int value for any DELAYs

17. Always use a global constant for any TIMEOUTs

const TIMEOUT = 5000;
 ...
controller.waitFor(function() {
  return something == something;
} "Some message", TIMEOUT);

18. Use assertDOMProperty(object, property) when evaluating DOM Object Properties

19. Use assertJSProperty(object, property) when evaluating JS Object Properties

20. Use array.forEach() for iterating array elements

array.forEach(function(elem, [index, [array]]) {
  statements;   
}, [thisObject]);

21. Don't put tests in testGeneral

22. Only one test per file

23. Element before action

var obj = new elementGetter(params);
controller.action(obj, params);

24. Local test pages encapsulated in Array

const LOCAL_TEST_PAGES = [
 { id: “Some_ID”,
   url: LOCAL_TEST_FOLDER + ‘someFolder/someFile.html’ },
 { id: “Some_ID”,
   url: LOCAL_TEST_FOLDER + ‘someFolder/someFile.html’ },
 { id: “Some_ID”,
   url: LOCAL_TEST_FOLDER + ‘someFolder/someFile.html’ }
];

25. Constants between requires and setupModule()

// Include required modules
var addons = require("../../../shared-modules/addons");
var modalDialog = require("../../../shared-modules/modal-dialog");

const TIMEOUT = 5000;
const SEARCH_TIMEOUT = 30000;
const INSTALL_TIMEOUT = 30000;

var setupModule = function() {

26. Do not pass a module parameter in setupModule() and teardownModule()

Refactoring

Timeouts & Delays

• gDelay: replace with a discrete int value
• gTimeout: replace with TIMEOUT

Constants

• const SOME_CONST = value;

setupModule & teardownModule

• Remove module parameter

Mozilla Components

• Ensure proper alignment of getService() on Cc[]

Local Test Pages

• Replace any LOCAL_PAGE or LOCAL_TEST_PAGE with a LOCAL_TEST_PAGES array
• Each array element should have at least a URL and ID member

Arrays

• Ensure proper alignment from style guidelines

Assertions

• waitForEval: replace with waitFor()