Create mochitests for existing WebExtension APIs

RESOLVED FIXED in Thunderbird 66.0

Status

enhancement
RESOLVED FIXED
8 months ago
7 months ago

People

(Reporter: darktrojan, Assigned: darktrojan)

Tracking

unspecified
Thunderbird 66.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

We're nearly ready to run mochitests on TaskCluster, so I've created some tests for the addressBooks, browserAction, and composeAction APIs.
Posted patch 1509246-webext-tests-1.diff (obsolete) β€” β€” Splinter Review
Posted patch 1509246-webext-tests-2.diff (obsolete) β€” β€” Splinter Review
Attachment #9026904 - Attachment is obsolete: true
Posted patch 1509246-webext-tests-3.diff (obsolete) β€” β€” Splinter Review
Attachment #9029362 - Attachment is obsolete: true
Comment on attachment 9029375 [details] [diff] [review]
1509246-webext-tests-3.diff

You might as well review this one too.
Attachment #9029375 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9029375 [details] [diff] [review]
1509246-webext-tests-3.diff

Review of attachment 9029375 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/components/extensions/test/browser/browser_ext_addressBooksUI.js
@@ +18,5 @@
> +
> +    await awaitMessage("check", 0);
> +
> +    await browser.addressBooks.openUI();
> +    await awaitMessage("check", 1);

it could be useful for these tests to have more descriptions of what they are doing, so people don't have to read the code that carefully ;)

::: mail/components/extensions/test/browser/browser_ext_composeAction.js
@@ +70,5 @@
> +    composeWindow.close();
> +  }
> +}
> +
> +let account;

why isn't this inside the function?

::: mail/components/extensions/test/browser/head.js
@@ +36,5 @@
> +async function promiseAnimationFrame(win = window) {
> +  await new Promise(resolve => win.requestAnimationFrame(resolve));
> +
> +  let {tm} = Services;
> +  return new Promise(resolve => tm.dispatchToMainThread(resolve));

Why not Services.tm.dispatchToMainThread directly?
Attachment #9029375 - Flags: review?(mkmelin+mozilla)
Posted patch 1509246-webext-tests-4.diff (obsolete) β€” β€” Splinter Review
A few small updates. I made a patch of the pref changes needed to run this stuff locally, and put it here: https://gist.github.com/darktrojan/3474b57149635c322dd8b3e59cdfa89f.
Attachment #9029375 - Attachment is obsolete: true
Attachment #9032272 - Flags: review?(mkmelin+mozilla)
Managed to get rid of a lot of flakiness. I think it still fails every now and then, but as far as I can tell, that's because of something dodgy in the WebExtensions framework.
Attachment #9032272 - Attachment is obsolete: true
Attachment #9032272 - Flags: review?(mkmelin+mozilla)
Attachment #9032316 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9032316 [details] [diff] [review]
1509246-webext-tests-5.diff

Review of attachment 9032316 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/components/extensions/test/browser/browser_ext_browserAction.js
@@ +76,5 @@
> +    },
> +    manifest: {
> +      applications: {
> +        gecko: {
> +          id: "mochitest@thunderbird.net",

might be better to use @example.com

::: mail/components/extensions/test/browser/browser_ext_composeAction.js
@@ +1,5 @@
> +/* 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/. */
> +
> +let account;

what's up with this?
Attachment #9032316 - Flags: review?(mkmelin+mozilla) → review+
> might be better to use @example.com

Okay.

> what's up with this?

It's assigned in one function and used in another and I don't want to pass it around all over the place. What's the problem?
I see. Perhaps best to use the gAccount convention to make that clearer.
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/4b4bc20aa6de
Create mochitests for existing WebExtension APIs; fix some nits found; r=mkmelin
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 66.0
You need to log in before you can comment on or make changes to this bug.