We have been working on a new React+Redux based front-end for the console in Firefox DevTools for a while, and we're quite close to completion, but we still need to migrate a few tests so they work with the new front-end, and also use best practices.
The console is one of the oldest components in DevTools and that's why we're taking this opportunity to reduce technical debt.
We want to make the integration tests in devtools/client/webconsole/new-console-output/test/mochitest
work.
This is the list of tests that need work. You should attempt to work in one which is not assigned (i.e. status column reads NEW
).
The old console front-end is in devtools/client/webconsole
. The tests are in devtools/client/webconsole/test
.
The new console front-end is in devtools/client/webconsole/new-console-output
. The new tests are in devtools/client/webconsole/new-console-output/test
.
You will notice that the old tests folder had all of them in the same folder, whereas the new one divides the tests by type (e.g. testing the Redux store is separated from integration tests).
First try to run the new test. Run:
./mach test /devtools/client/webconsole/new-console-output/test/mochitest/{test-name.js}
where test-name.js
is the name of the file containing tests that you want to work on.
If there are no errors when you run the test, that's great! The end of the output would be something like this:
Browser Chrome Test Summary
Passed: 1
Failed: 0
Todo: 0
Mode: e10s
*** End BrowserChrome Test Results ***
Buffered messages finished
SUITE-END | took 19s
Overall Summary
mochitest-browser: 3/3
If there are errors, they end of the output would look a bit like this:
Browser Chrome Test Summary
Passed: 1
Failed: 1
Todo: 0
Mode: e10s
*** End BrowserChrome Test Results ***
Buffered messages finished
SUITE-END | took 10s
Overall Summary
mochitest-browser: 1/2
TEST-UNEXPECTED-FAIL | devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_time_methods.js | Uncaught exception - waitFor - timed out after 500 tries.
There's advice on how to fix the errors in the next section.
Supposing your test file has no errors and passes all the tests, the next step is to make sure the test is enabled in the browser.ini
file in devtools/client/webconsole/new-console-output/test/mochitest/browser.ini
.
Open the browser.ini file, and look for the file name of the test you're working on. You might encounter a section for the file, surrounded by brackets, like the following:
[browser_webconsole_time_methods.js]
skip-if = true # Bug 1404877
It tells you the test is disabled (it'll be skipped when running 'all the tests'). The bug number should provide a reason as to why the test was disabled.
In this case, if you go to our bug tracker and search for that bug number, you'll encounter that it actually corresponds to the bug migration. Since the test is passing already, the last step to fix the bug is to remove this section from browser.ini
, so the test is not skipped anymore.
So remove the two lines, and save the file:
-[browser_webconsole_time_methods.js]
-skip-if = true # Bug 1404877
You should now be ready to send a patch!
The general process to work in these tests is to first identify the new and the old test files. In general terms you should be able to find them easily as they have the same file name (they're just in different folders). But we might have used a new file name for the new test, if, for example, the old test file included a bug number on the file name. Do let us know if you can't find the counterpart!
Following on the example before, the old test file corresponding to devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_time_methods.js
is in devtools/client/webconsole/test/browser_webconsole_bug_658368_time_methods.js
. We removed the bug number from the file name when writing the new tests.
A test might also load support files, and it is possible the support files also included bug numbers in the file name. To tidy things up, these need to be renamed to remove the bug number, and evidently you also need to update the file name reference in the test file itself.
browser_webconsole_time_methods.js
uses a support file called test-bug-658368-time-methods.html
. It needs to be renamed using your version control command (to avoid losing any history):
- With git:
git mv test-bug-658368-time-methods.html test-time-methods.html
- With hg:
{TODO}
Also edit the mention to the support file in the test file:
// Old
const TEST_URI = "http://example.com/browser/devtools/client/webconsole/" +
"new-console-output/test/mochitest/test-bug-658368-time-methods.html";
// New
const TEST_URI = "http://example.com/browser/devtools/client/webconsole/" +
"new-console-output/test/mochitest/test-time-methods.html";
Following is a list of constructs and methods that replace what was used in the old tests. The new test helpers are simpler and tend to accept fewer arguments, and should also be more robust and easy to understand.
This is not meant to be exhaustive. Please collaborate if you think something should be added: fork this repository and send us a pull request! :-)
When you don't know where functions are coming from, you can look at the head.js
file in the each test directory. This file is included before each test is run, and it's where we defined helper functions such as openNewTabAndConsole
which are used throughout the Console tests.
The head.js
file in the new test folder has way fewer helpers than the old one, because we're trying to simplify, but if you feel something should be ported from the previous head.js
file, by all means do port it (but try to adhere to the code conventions in the new head.js
).
Older tests used add_task
with a generator:
add_task(function* ...
New tests should use async functions instead of generator functions:
add_task(async function(...
Where you see a sequence like this:
yield loadTab(TEST_URI);
let hud1 = yield openConsole();
... you can replace it with a single helper function call:
let hud1 = await openNewTabAndConsole(TEST_URI);
Replace:
yield loadBrowser(gBrowser.selectedBrowser);
with
await BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser);
waitForMessages
sets a listener on an event which is emitted when a message is rendered on the console output, which means it works well if you can call waitForMessages
before actually logging the message.
In some cases, we load a URL which does the logging so we can't really use waitForMessages
as the event is probably already fired before we set up the listener. In those cases, we fall back to polling with waitFor
, which takes a function and resolves when it is truthy:
Therefore you could replace:
yield waitForMessages({
hud: hud,
messages: [{
name: "text of the message",
}]
});
with
await waitFor(() => findMessage(hud, "text of the message"));