1299551 - Update test_webextension_install.js to wait for add-ons to finish installing

Status: RESOLVED FIXED

(WebExtensions :: General, defect, P5)

Description

[Bob Silverberg [:bsilverberg]]

This is a follow-up to bug 1297752. The test in question installs a number of webextensions, but does not wait for the startup to complete before continuing through the test. It was suggested that we should add a promiseAddonStartup to the tests, although Kris also mentioned that that was a hack that we want to get rid of eventually.

This bug is to address the issue, either by simply adding promiseAddonStartup to the tests, or by implementing a better solution and then updating the tests accordingly.

Kris, can you comment on which approach you wanted to see taken for this?

Comment 1

[Bob Silverberg [:bsilverberg]]

Kris, you were going to add a comment about what you think would be a better approach for this.

Comment 2

[Kris Maglione [:kmag]]

Yeah, I think we should update as many of these tests as possible to just use ExtensionTestUtils.loadExtension rather than using hacks to wait for them to finish starting up.

Comment 3

[Bob Silverberg [:bsilverberg]]

Attachment: Bug 1299551 - Update test_webextension_install.js to wait for add-ons to finish installing,

Review commit: https://reviewboard.mozilla.org/r/79584/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/79584/

Comment 4

[Bob Silverberg [:bsilverberg]]

I updated as many as seemed logical. There are three tests that seem to need to load files from disk that I didn't change.

Comment 5

[Kris Maglione [:kmag]]

Comment on attachment 8792668 [details]
Bug 1299551 - Update test_webextension_install.js to wait for add-ons to finish installing,

https://reviewboard.mozilla.org/r/79584/#review80804

Looks great. Thanks!

::: toolkit/mozapps/extensions/test/xpcshell/test_webextension_install.js:240
(Diff revision 1)
> -  yield Assert.rejects(AddonManager.installTemporaryAddon(addonDir),
> +  yield Assert.rejects(extension.startup(),
>                         expectedMsg,
>                         "Install rejects when specified maxVersion is not valid");

Oh, good, I wasn't 100% sure this would work.

::: toolkit/mozapps/extensions/test/xpcshell/test_webextension_install.js:291
(Diff revision 1)
> -                                          "the-addon-sub-dir");
> +    manifest: testManifest,
> +    useAddonManager: "temporary",
> +  });
>  
>    expectedMsg = new RegExp("Add-on [e-mail] is not compatible with application version. " +
>                                 "add-on minVersion: 2. add-on maxVersion: 1.");

Can you fix the indentation of these regexps so the second line aligns with the opening `(`? It looks like whoever originally wrote it didn't re-align after they removed the `let`.

Comment 6

[Bob Silverberg [:bsilverberg]]

Comment on attachment 8792668 [details]
Bug 1299551 - Update test_webextension_install.js to wait for add-ons to finish installing,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/79584/diff/1-2/

Comment 7

[Bob Silverberg [:bsilverberg]]

Try looks good. Requesting check in.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4dfd7fba6157
Update test_webextension_install.js to wait for add-ons to finish installing, r=kmag

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4dfd7fba6157