1315872 - Cleanup tests

Status: RESOLVED FIXED

(WebExtensions :: General, defect)

Description

[Kris Maglione [:kmag]]

Follow-up to bug 1313956

Comment 1

[Kris Maglione [:kmag]]

Attachment: Bug 1315872: Add browser.test.assertRejects and assertThrows.

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

Comment 2

[Kris Maglione [:kmag]]

Attachment: Bug 1315872: Refactor tests that check for promise rejection to use assertRejects.

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

Comment 3

[Kris Maglione [:kmag]]

Attachment: Bug 1315872: Refactor browser_ext_tabs_cookieStoreId.

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

Comment 4

[Andrew Swan [:aswan]]

Comment on attachment 8808484 [details]
Bug 1315872: Add browser.test.assertRejects and assertThrows.

https://reviewboard.mozilla.org/r/91320/#review91316

so long testAPIFactory...

Comment 5

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9ae69a2f818dcc3f37a5966b33da433976b03906
Bug 1315872: Add browser.test.assertRejects and assertThrows. r=aswan

Comment 6

[Andrew Swan [:aswan]]

Comment on attachment 8808485 [details]
Bug 1315872: Refactor tests that check for promise rejection to use assertRejects.

https://reviewboard.mozilla.org/r/91322/#review91326

::: browser/components/extensions/test/browser/browser_ext_browserAction_pageAction_icon_permissions.js:26
(Diff revision 1)
>              detail.tabId = tabId;
>              promises.push(
> -              browser[api].setIcon(detail).then(
> -                () => {
> -                  browser.test.fail("Expected an error on invalid icon size.");
> -                  browser.test.notifyFail("setIcon with invalid icon size");
> +              browser.test.assertRejects(
> +                browser[api].setIcon(detail),
> +                /must be an integer/,
> +                "setIcon with invalid icon size"));

We lost the notifyFail() here, though I guess you can make a case that it didn't need to be there in the first place?
(the same pattern is repeated below)

Comment 7

[Andrew Swan [:aswan]]

Comment on attachment 8808486 [details]
Bug 1315872: Refactor browser_ext_tabs_cookieStoreId.

https://reviewboard.mozilla.org/r/91324/#review91472

::: browser/components/extensions/test/browser/browser_ext_tabs_cookieStoreId.js:55
(Diff revision 1)
> +            tab = await browser.tabs.create({
> -            windowId: data.privateTab ? this.privateWindowId : this.defaultWindowId,
> +              windowId: data.privateTab ? this.privateWindowId : this.defaultWindowId,
> -            cookieStoreId: data.cookieStoreId,
> +              cookieStoreId: data.cookieStoreId,
> -          }).then((tab) => {
> -            // Tests for tab creation
> -            testTab(data, tab);
> +            });
> +
> +            browser.test.assertTrue(!data.failure, "we want a success");

I think the call to `testTab()` below covers this?  What a helpful message...

Comment 8

[Kris Maglione [:kmag]]

Comment on attachment 8808485 [details]
Bug 1315872: Refactor tests that check for promise rejection to use assertRejects.

https://reviewboard.mozilla.org/r/91322/#review91326

> We lost the notifyFail() here, though I guess you can make a case that it didn't need to be there in the first place?
> (the same pattern is repeated below)

Yeah, but it wasn't really necessary. We get a failure from the error check alone.

Comment 9

[Kris Maglione [:kmag]]

Comment on attachment 8808486 [details]
Bug 1315872: Refactor browser_ext_tabs_cookieStoreId.

https://reviewboard.mozilla.org/r/91324/#review91472

> I think the call to `testTab()` below covers this?  What a helpful message...

Hm. Yeah, I guess... Blah.

Comment 10

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a57a92087010c2814d3f65dd95beaebf1b94a966
Bug 1315872: Refactor tests that check for promise rejection to use assertRejects. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/9e5ff7c0e899b12583deb2a74ea81f6ad197e33e
Bug 1315872: Refactor browser_ext_tabs_cookieStoreId. r=aswan

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9ae69a2f818d
https://hg.mozilla.org/mozilla-central/rev/a57a92087010
https://hg.mozilla.org/mozilla-central/rev/9e5ff7c0e899