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 Neil Rashbrook's style guide Douglas Crockford's style guide

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

The 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.

Refactoring

Phase I

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 guidelines

Phase II

Goal: Bring tests in-line with Shared Module Refactor

Outstanding Items

Discussion here

Generally Agreed

• We should always use the triple operator to not experience strange behavior
  • ===/!== instead of ==/!=
• Variables for imported modules have to start with a capital letter
  • 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

Needs Discussion

• sleep() should only be used when an element/state is not available for waitFor()
• TestFilesAPI for loading of test files
  • We should have a module specific to loading of test files
  • We should also check how we can make sure those locations are easily replacable with other servers (apache, ...)
• Error message format
  • "Expected password-save notification bar to be visible"
  • 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
  • Mochitests are using: "- got XXX, expected YYY". We should show both values in the failure message. That will make it much easier to spot failures. The element itself isn't that necessary. You will see this by the line reference.
• Parameters should be named using aParam format
  • shows that a variable inside a long function is a parameter and hasn't been declared locally