Skip to content

Commit

Permalink
Bug 1793311 - [marionette] Fix markdown warnings. r=webdriver-reviewe…
Browse files Browse the repository at this point in the history
…rs,jdescottes DONTBUILD

Differential Revision: https://phabricator.services.mozilla.com/D160233
  • Loading branch information
whimboo committed Nov 2, 2022
1 parent c9ac344 commit cca2834
Show file tree
Hide file tree
Showing 13 changed files with 278 additions and 360 deletions.
19 changes: 9 additions & 10 deletions remote/doc/marionette/Building.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
Building
========
# Building

Marionette is built into Firefox by default and ships in the official
Firefox binary. As Marionette is written in [XPCOM] flavoured
Expand All @@ -14,7 +13,7 @@ Once you have a clone of [mozilla-unified], you can set up your
development environment by running this command and following the
on-screen instructions:

% ./mach bootstrap
% ./mach bootstrap

When you're getting asked to choose the version of Firefox you want to build,
you may want to consider choosing "Firefox for Desktop Artifact Mode". This
Expand All @@ -23,31 +22,31 @@ significantly reduces the time it takes to build Firefox on your machine

To perform a regular build, simply do:

% ./mach build
% ./mach build

You can clean out the objdir using this command:

% ./mach clobber
% ./mach clobber

Occasionally a clean build will be required after you fetch the
latest changes from mozilla-central. You will find that the the
build will error when this is the case. To automatically do clean
builds when this happens you may optionally add this line to the
[mozconfig] file in your top source directory:

mk_add_options AUTOCLOBBER=1
mk_add_options AUTOCLOBBER=1

If you compile Firefox frequently you will also want to enable
[ccache] and [sccache] if you develop on a macOS or Linux system:

mk_add_options 'export RUSTC_WRAPPER=sccache'
mk_add_options 'export CCACHE_CPP2=yes'
ac_add_options --with-ccache
mk_add_options 'export RUSTC_WRAPPER=sccache'
mk_add_options 'export CCACHE_CPP2=yes'
ac_add_options --with-ccache

You may also opt out of building all the WebDriver specific components
(Marionette, and the [Remote Agent]) by setting the following flag:

ac_add_options --disable-webdriver
ac_add_options --disable-webdriver

[mozilla-unified]: https://mozilla-version-control-tools.readthedocs.io/en/latest/hgmozilla/unifiedrepo.html
[artifact builds]: /contributing/build/artifact_builds.rst
Expand Down
129 changes: 60 additions & 69 deletions remote/doc/marionette/CodeStyle.md
Original file line number Diff line number Diff line change
@@ -1,24 +1,21 @@
Style guide
===========
# 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.
* 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.
* 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
remote/marionette a pleasant ecosystem to work in.
* Code is mutable and not written in stone. Nothing that
is checked in is sacred and we encourage change to make
remote/marionette a pleasant ecosystem to work in.


JavaScript
----------
## JavaScript

Marionette is written in JavaScript and ships
as part of Firefox. We have access to all the latest ECMAScript
Expand All @@ -41,42 +38,42 @@ requirements.
To export symbols to other Marionette modules, remember to assign
your exported symbols to the shared global `this`:

const EXPORTED_SYMBOLS = ["PollPromise", "TimedPromise"];
const EXPORTED_SYMBOLS = ["PollPromise", "TimedPromise"];

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

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

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;
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;
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();
switch (x) {
case "foo":
doSomething();

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

case "baz":
case "bah": // <-- allowed (-:
doCrazyThings();
}
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
Expand All @@ -90,9 +87,9 @@ 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}`);
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
Expand All @@ -104,51 +101,51 @@ 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}`);
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>";
}
}
/** 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}) => { … };
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;
};
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/. */
/* 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
Expand All @@ -173,15 +170,11 @@ a re-build.
[MPL]: https://www.mozilla.org/en-US/MPL/2.0/
[Contributing.md]: ./Contributing.md


Python
------
## Python

TODO


Documentation
-------------
## Documentation

We keep our documentation in-tree under [remote/doc/marionette]
and [testing/geckodriver/doc]. Updates and minor changes to
Expand All @@ -203,23 +196,21 @@ other modules. Documentation for non-exported symbols is not required.
[remote/doc/marionette]: https://searchfox.org/mozilla-central/source/remote/marionette/doc
[testing/geckodriver/doc]: https://searchfox.org/mozilla-central/source/testing/geckodriver/doc


Linting
-------
## 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 remote/marionette
% ./mach lint -funix remote/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 remote/marionette
% ./mach eslint --fix remote/marionette

The linter is also run as a try job (shorthand `ES`) which means
any style violations will automatically block a patch from landing
Expand Down
58 changes: 25 additions & 33 deletions remote/doc/marionette/Contributing.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
Contributing
============
# Contributing

If you are new to open source or to Mozilla, you might like this
[tutorial for new Marionette contributors](NewContributors.md).
Expand All @@ -9,24 +8,24 @@ We are delighted that you want to help improve Marionette!
on who you talk to, but the overall scope of the project involves
these components:

* [_Marionette_] is a Firefox remote protocol to communicate with,
instrument, and control Gecko-based applications such as Firefox
and Firefox for mobile. It is built in to the application and
written in JavaScript.
* [_Marionette_] is a Firefox remote protocol to communicate with,
instrument, and control Gecko-based applications such as Firefox
and Firefox for mobile. It is built in to the application and
written in JavaScript.

It serves as the backend for the geckodriver WebDriver implementation,
and is used in the context of Firefox UI tests, reftesting,
Web Platform Tests, test harness bootstrapping, and in many
other far-reaching places where browser instrumentation is required.
It serves as the backend for the geckodriver WebDriver implementation,
and is used in the context of Firefox UI tests, reftesting,
Web Platform Tests, test harness bootstrapping, and in many
other far-reaching places where browser instrumentation is required.

* [_geckodriver_] provides the HTTP API described by the [WebDriver
protocol] to communicate with Gecko-based applications such as
Firefox and Firefox for mobile. It is a standalone executable
written in Rust, and can be used with compatible W3C WebDriver clients.
* [_geckodriver_] provides the HTTP API described by the [WebDriver
protocol] to communicate with Gecko-based applications such as
Firefox and Firefox for mobile. It is a standalone executable
written in Rust, and can be used with compatible W3C WebDriver clients.

* [_webdriver_] is a Rust crate providing interfaces, traits
and types, errors, type- and bounds checks, and JSON marshaling
for correctly parsing and emitting the [WebDriver protocol].
* [_webdriver_] is a Rust crate providing interfaces, traits
and types, errors, type- and bounds checks, and JSON marshaling
for correctly parsing and emitting the [WebDriver protocol].

By participating in this project, you agree to abide by the Mozilla
[Community Participation Guidelines]. Here are some guidelines
Expand All @@ -38,9 +37,7 @@ for contributing high-quality and actionable bugs and code.
[WebDriver protocol]: https://w3c.github.io/webdriver/webdriver-spec.html#protocol
[Community Participation Guidelines]: https://www.mozilla.org/en-US/about/governance/policies/participation/


Writing code
------------
## Writing code

Because there are many moving parts involved remote controlling
a web browser, it can be challenging to a new contributor to know
Expand All @@ -54,24 +51,19 @@ We have collected a lot of good advice for working on Marionette
code in our [code style document], which we highly recommend you read.

[ask questions]: ./index.rst#communication
[reach out to us]: ./index.rst#communication
[mozilla-central]: https://searchfox.org/mozilla-central/source/remote/marionette/
[Testing :: Marionette]: https://bugzilla.mozilla.org/buglist.cgi?resolution=---&component=Marionette
[good first bugs]: https://codetribute.mozilla.org/projects/automation?project%3DMarionette
[code style document]: CodeStyle.md

## Next steps

Next steps
----------

* [Building](Building.md)
* [Debugging](Debugging.md)
* [Testing](Testing.md)
* [Patching](Patches.md)

* [Building](Building.md)
* [Debugging](Debugging.md)
* [Testing](Testing.md)
* [Patching](Patches.md)

Other resources
---------------
## Other resources

* [Code style](CodeStyle.md)
* [New Contributor Tutorial](NewContributors.md)
* [Code style](CodeStyle.md)
* [New Contributor Tutorial](NewContributors.md)
Loading

0 comments on commit cca2834

Please sign in to comment.