Mozmill Style Guide

v1.1 by the Mozilla QA Automation Team:

Anthony Hughes
Dave Hunt
Geo Mealer
Henrik Skupin
Aaron Train

Summary

The purpose of this style guide is to provide a clear set of DOs and DON'Ts for contributing code to the Mozmill automated testing framework with minimal iterations through the review process. The golden rule is to keep code human-readable while respecting an 80-character-per-line limit.

NOTE: All mozmill-tests and shared modules are written in JavaScript. In some cases, like test pages, HTML and CSS are used. For the purposes of this style guide, we will focus on the JavaScript rules.

EXCEPTION: In general, many of these rules are not set in stone and are always open to improvement. If you can find a good case for breaking or twisting a rule, be prepared to defend that case.

If ever in doubt, please refer to Neil Rashbrook's JavaScript Style Guide.

Test Scripts

All test scripts should contain a similar skeleton:

Each test script should only contain a single test function
Each test script should have the following blocks in the following order:
- MPL license block
- Use Strict declaration
- Shared Module inclusion block, modules listed alphabetically by filename
- Constants declaration block
- setupModule()
- teardownModule()
- testSomething()
Each test script should be named based on the Moztrap test from which it is being derived. For example, a Mozmill test script for the Moztrap test "[sessionstore] Restore a recently closed tab" would be placed in the mozmill-tests/firefox/testSessionStore folder with the name testUndoCloseTab.js with a test function testUndoCloseTab().

Click here to see a template of this skeleton.

License Block

The MPL2 license is used and must be present at the top of all tests. See MPL Headers for the license block content.

Whitespace

Please follow these few simple guidelines in regards to whitespace:

There should be no whitespace at the end of any line
There should be no trailing blank lines at the end of any file
All files should end with a newline character (make sure your patches don't contain files with the text "No newline at end of file" in them)
All lines should be indented 2 spaces from their parent
If you use tabs, configure your editor of choice to convert tabs to whitespace. For example, adding this line for Vim in a .vimrc
Separate top-level elements like class definitions with two blank lines

Naming

All functions should be named using camel-case with a trailing opening brace
```
function testPasswordNotSaved() {
```
Parameters should be named using lowercase 'a' prefix, followed by the original name with capital letter

function testPasswordNotSaved(aController) {

All variables should be named using camel-case and preceded with the var keyword
```
var loginButtonLabel = "value";
```
Constants should be named using all-caps
```
const LOGIN_BUTTON_LABEL = "value";
```

Important: All constants should be declared globally, between the shared module inclusion block and setupModule().
Important: All values which will not change during the course of a test should be declared as constants.

Components

In some cases, you may be required to include Mozilla interface components in your test. Ideally, these should be abstracted in a shared module. Should you need to include a component, please follow these simple rules:

Instantiate the component in setupModule()
Concatenate the component declaration by splitting on the period between the component and getService()
```
  // Effective TLD Service for grabbing certificate info
  gETLDService = Cc["@mozilla.org/network/effective-tld-service;1"]
                 .getService(Ci.nsIEffectiveTLDService);
```
Note: Cc is an alias for Components.classes, Ci is an alias for Components.interfaces
Important: Be sure to align getService with Cc

Error Messages

When hard coding error messages in assertions or waitFor(), the message should always convey a positive expectation instead of a negative error. Also, the error message should in some way indicate the specific element.

Good: Expected the password notification bar to be visible
Bad: Didn't find the password notification bar

Elements

Whenever you want to perform an action on any element, be sure to instantiate the element prior to performing the action.

var loginButton = new elementslib.ID(controller.tabs.activeTab, "LogIn");
controller.click(loginButton);

Sleep()

A word about using sleep(): DON'T

In general, you should avoid using sleep() calls as they make tests less reliable and robust. Always use a waitFor() instead. There are some edge cases where a sleep() may be necessary. In these cases, be prepared to defend your usage of sleep().

Iteration

There are basically two scenarios where you will want to iterate (i.e. step through): Arrays and Strings

When you want to iterate through the elements of an Array, use Array.forEach()

var arrayOfElements = ["Apples", "Oranges", "Pears"];

arrayOfElements.forEach(function (element, index) {
  controller.window.document.write("[" + index + "] is " + element + "<br />");
});

The output of the above code will be something like this:

[0] is Apples
[1] is Oranges
[2] is Pears

When you want to iterate through the characters of a String, use for()

var stringOfCharacters = "Apples";

for (var i = 0; i < stringOfCharacters.length; i++) {
  controller.window.document.write(stringOfCharacters.charAt(i) + "<br />");
}

The output of the above code will look something like this:

A
p
p
l
e
s

Commenting

In an effort to make code as readable as possible, all logical blocks of code should be commented. The following are the recommended commenting styles:

All functions should be preceded by a comment block using the JSDoc Toolkit style and have the implementation bug attached for better tracking:
```
/**
 * Bug XXXXXX
 * Purpose of the function
 */
function nameOfFunction(optional, parameter, list) {
```
Note: The use of JSDoc style is generally accepted at this point but may be modified to better suit Mozmill's needs in the future.

All logical blocks of code should use single-line comments describing the function of the code

// Click the login button and wait for the redirect page to load
var loginButton = new elementslib.ID(controller.tabs.activeTab, "LogIn");
controller.click(loginButton);
controller.waitForPageLoad();

Any code which is a temporary workaround should reference the blocking bug

// Bug 390724
// SelectedPane is broken, so iterate through all elements.
// Remove/replace this code after selectedPane has been fixed
var panel = deck.boxObject.firstChild;
for (var i = 0; i < deck.selectedIndex; i++) {
  panel = panel.nextSibling;
}

The above is example workaround code for a particular bug. Once the bug is resolved, we can go back and remove this workaround.

Concatenation

In an effort to respect the 80-character-per-line limit, sometimes you will have to split a line of code into multiple lines. The following details some common scenarios where concatenation is necessary and how to style them.

Function Parameters

function nameOfFunction(firstParameter,
                        secondParameter,
                        thirdParameter) {

Note: All parameters are aligned on the left, one per line

XPath Strings
```
var element = nameOfFunction("/path/to/" +
                             "location/" +
                             "of/element");
    
```
Note: XPath strings are split on the '/'
Note: The operator (+) is placed to the right of the parameter separated by a single space

Conditionals

Always use brackets:

if (condition) {
  ...
}
else if (condition) {
  ...
}
else {
  ...
}

Note: opening brace trails the condition separated by a single space
Note: closing brace is left aligned and exists on its own line
Note: single space between if and (condition) Important: The same rules apply for loops and try-catch blocks

Code Blocks

If they are on a single line, code blocks should be padded with a whitespace.

variable = { one, two };

Multi-line code blocks should not be padded.

variable = {
  one,
  two
};

Bug fixes vs Style changes

While having a clear style in our code is always a good thing, if you are working on fixing a bug please refrain yourself to do stylistic changes just for the sake of the Style Guide.

Specifically:

any new code written should adhere to the Style Guide
any changes to existing code should also update the style appropriately
no changes should be in a patch that are not related to the bug in question, especially stilistic changes

If you find a lot of style inconsistencies and wish to fix them, that is great.
But please file a bug along the lines of a "style cleanup" and prepare a separate patch that contains only the stilistic changes.
If you do this, try tidying things up in the whole project for the specific style item that is inconsistent.

Revisions

29/11/2010 - Draft version 1.0 by ashughes
11/04/2012 - Updated license references to MPL 2.0