gecko-dev/testing/marionette/doc/CodeStyle.md

Style guide
===========

Like other projects, we also have some guidelines to keep to the code.
For the overall Marionette project, a few rough rules are:

  * Make your code readable and sensible, and don’t try to be
    clever.  Prefer simple and easy solutions over more convoluted
    and foreign syntax.

  * Fixing style violations whilst working on a real change as a
    preparatory clean-up step is good, but otherwise avoid useless
    code churn for the sake of conforming to the style guide.

  * Code is mutable and not written in stone.  Nothing that
    is checked in is sacred and we encourage change to make
    testing/marionette a pleasant ecosystem to work in.


JavaScript
----------

Marionette is written in [XPCOM] flavoured JavaScript and ships
as part of Firefox.  We have access to all the latest ECMAScript
features currently in development, usually before it ships in the
wild and we try to make use of new features when appropriate,
especially when they move us off legacy internal replacements
(such as Promise.jsm and Task.jsm).

One of the peculiarities of working on JavaScript code that ships as
part of a runtime platform is, that unlike in a regular web document,
we share a single global state with the rest of Firefox.  This means
we have to be responsible and not leak resources unnecessarily.

JS code in Gecko is organised into _modules_ carrying _.js_ or _.jsm_
file extensions.  Depending on the area of Gecko you’re working on,
you may find they have different techniques for exporting symbols,
varying indentation and code style, as well as varying linting
requirements.

To export symbols to other Marionette modules, remember to assign
your exported symbols to the shared global `this`:

	this.EXPORTED_SYMBOLS = ["PollPromise", "TimedPromise"];

When importing symbols in Marionette code, try to be specific about
what you need:

	const {TimedPromise} = Cu.import("chrome://marionette/content/sync.js", {});

The [linter] will complain if you import a named symbol that is
not in use.  If however you _need_ to import every symbol, you can:

	const wait = {};
	Cu.import("chrome://marionette/content/sync.js", wait);

	wait.sleep(42);
	await wait.TimedPromise(…);

We prefer object assignment shorthands when redefining names,
for example when you use functionality from the `Components` global:

	const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;

When using symbols by their own name, the assignment name can be
omitted:

	const {TYPE_ONE_SHOT, TYPE_REPEATING_SLACK} = Ci.nsITimer;

In addition to the default [Mozilla eslint rules], we have [our
own specialisations] that are stricter and enforce more security.
A few notable examples are that we disallow fallthrough `case`
statements unless they are explicitly grouped together:

	switch (x) {
	  case "foo":
	    doSomething();

	  case "bar":  // <-- disallowed!
	    doSomethingElse();
	    break;

	  case "baz":
	  case "bah":  // <-- allowed (-:
	    doCrazyThings();
	}

We disallow the use of `var`, for which we always prefer `let` and
`const` as replacements.  Do be aware that `const` does not mean
that the variable is immutable: just that it cannot be reassigned.
We require all lines to end with semicolons, disallow construction
of plain `new Object()`, require variable names to be camel-cased,
and complain about unused variables.

For purely aesthetic reasons we indent our code with two spaces,
which includes switch-statement `case`s, and limit the maximum
line length to 78 columns.  When you need to wrap a statement to
the next line, the second line is indented with four spaces, like this:

	throw new TypeError(
	    "Expected an element or WindowProxy, " +
	    pprint`got: ${el}`);

This is not normally something you have to think to deeply about as
it is enforced by the [linter].  The linter also has an automatic
mode that fixes and formats certain classes of style violations.

If you find yourself struggling to fit a long statement on one line,
this is usually an indication that it is too long and should be
split into multiple lines.  This is also a helpful tip to make the
code easier to read.  Assigning transitive values to descriptive
variable names can serve as self-documentation:

	let location = event.target.documentURI || event.target.location.href;
	log.debug(`Received DOM event ${event.type} for ${location}`);

On the topic of variable naming the opinions are as many as programmers
writing code, but it is often helpful to keep the input and output
arguments to functions descriptive (longer), and let transitive
internal values to be described more succinctly:

	/** Prettifies instance of Error and its stacktrace to a string. */
	function stringify(error) {
	  try {
	    let s = error.toString();
	    if ("stack" in error) {
	      s += "\n" + error.stack;
	    }
	    return s;
	  } catch (e) {
	    return "<unprintable error>";
	  }
	}

When we can, we try to extract the relevant object properties in
the arguments to an event handler or a function:

	const responseListener = ({name, target, json, data}) => { … };

Instead of:

	const responseListener = msg => {
	  let name = msg.name;
	  let target = msg.target;
	  let json = msg.json;
	  let data = msg.data;
	  …
	};

All source files should have `"use strict";` as the first directive
so that the file is parsed in [strict mode].

Every source code file that ships as part of the Firefox bundle
must also have a [copying header], such as this:

	/* This Source Code Form is subject to the terms of the Mozilla Public
	 * License, v. 2.0. If a copy of the MPL was not distributed with this file,
	 * You can obtain one at http://mozilla.org/MPL/2.0/. */

New xpcshell test files _should not_ have a license header as all
new Mozilla tests should be in the [public domain] so that they can
easily be shared with other browser vendors.  We want to re-license
existing tests covered by the [MPL] so that they can be shared.
We very much welcome your help in doing version control archeology
to make this happen!

The practical details of working on the Marionette code is outlined
in [CONTRIBUTING.md], but generally you do not have to re-build
Firefox when changing code.  Any change to testing/marionette/*.js
will be picked up on restarting Firefox.  The only notable exception
is testing/marionette/components/marionette.js, which does require
a re-build.

[XPCOM]: https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM
[strict mode]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode
[our own specialisations]: https://searchfox.org/mozilla-central/source/testing/marionette/.eslintrc.js
[linter]: #linting
[copying header]: https://www.mozilla.org/en-US/MPL/headers/
[public domain]: https://creativecommons.org/publicdomain/zero/1.0/
[MPL]: https://www.mozilla.org/en-US/MPL/2.0/
[CONTRIBUTING.md]: ../CONTRIBUTING.md


Python
------

TODO


Documentation
-------------

We keep our documentation in-tree under [testing/marionette/doc]
and [testing/geckodriver/doc].  Updates and minor changes to
documentation should ideally not be scrutinised to the same degree
as code changes to encourage frequent updates so that the documentation
does not go stale.  To that end, documentation changes with `r=me`
from module peers are permitted.

Use fmt(1) or an equivalent editor specific mechanism (such as Meta-Q
in Emacs) to format paragraphs at a maximum width of 75 columns
with a goal of roughly 65.  This is equivalent to `fmt -w 75 -g 65`,
which happens to be the default on BSD and macOS.

We endeavour to document all _public APIs_ of the Marionette component.
These include public functions—or command implementations—on
the `GeckoDriver` class, as well as all exported symbols from
other modules.  Documentation for non-exported symbols is not required.

The API documentation can be regenerated to [testing/marionette/doc/api]
so:

The API documentation uses [jsdoc] and is generated to <https://firefox-source-docs.mozilla.org/testing/marionette/marionette/internals> on Taskcluster.  You may also build the documentation locally:

	% ./mach doc

[Mozilla eslint rules]: https://searchfox.org/mozilla-central/source/.eslintrc.js
[testing/geckodriver/doc]: https://searchfox.org/mozilla-central/source/testing/geckodriver/doc
[testing/marionette/doc]: https://searchfox.org/mozilla-central/source/testing/marionette/doc
[jsdoc]: http://usejsdoc.org/


Linting
-------

Marionette consists mostly of JavaScript (server) and Python (client,
harness, test runner) code.  We lint our code with [mozlint],
which harmonises the output from [eslint] and [flake8].

To run the linter with a sensible output:

	% ./mach lint -funix testing/marionette

For certain classes of style violations the eslint linter has
an automatic mode for fixing and formatting your code.  This is
particularly useful to keep to whitespace and indentation rules:

	% ./mach eslint --fix testing/marionette

The linter is also run as a try job (shorthand `ES`) which means
any style violations will automatically block a patch from landing
(if using Autoland) or cause your changeset to be backed out (if
landing directly on mozilla-inbound).

If you use git(1) you can [enable automatic linting] before you push
to a remote through a pre-push (or pre-commit) hook.  This will
run the linters on the changed files before a push and abort if
there are any problems.  This is convenient for avoiding a try run
failing due to a stupid linting issue.

[mozlint]: https://firefox-source-docs.mozilla.org/tools/lint/usage.html
[eslint]: https://eslint.org/
[flake8]: http://flake8.pycqa.org/en/latest/
[enable automatic linting]: https://firefox-source-docs.mozilla.org/tools/lint/usage.html#using-a-vcs-hook