1470455 - Add the expected arguments to throws/rejects where missing throughout the tree

Status: RESOLVED FIXED

(Toolkit :: General, enhancement, P1)

Description

[Mark Banner (:standard8)]

In bug 1452706 we're making the expected argument to Assert.throws/rejects (and the variants) a required one - see that bug for more details.

This bug is for adding the expected requirements in the remaining tests where we're hitting these.

I'm not yet making it a required argument as bug 1470396 needs to import some external code.

Comment 1

[Mark Banner (:standard8)]

Attachment: Bug 1470455 - Add the 'expected' arguments to throws/rejects for devtools.

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

Comment 2

[Mark Banner (:standard8)]

Attachment: Bug 1470455 - Add the 'expected' arguments to throws/rejects for netwerk.

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

Comment 3

[Mark Banner (:standard8)]

Attachment: Bug 1470455 - Add the 'expected' arguments to throws/rejects for dom/push/test/xpcshell/test_broadcast_success.js.

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

Comment 4

[Mark Banner (:standard8)]

Attachment: Bug 1470455 - Add the 'expected' arguments to throws/rejects for security/manager/ssl/tests/unit/test_sts_parser.js.

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

Comment 5

[Mark Banner (:standard8)]

Attachment: Bug 1470455 - Add the 'expected' arguments to throws/rejects for toolkit.

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

Comment 6

[Lina Butler [:lina]]

Comment on attachment 8987094 [details]
Bug 1470455 - Add the 'expected' arguments to throws/rejects for dom/push/test/xpcshell/test_broadcast_success.js.

https://reviewboard.mozilla.org/r/252320/#review258790

Comment 7

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Comment on attachment 8987095 [details]
Bug 1470455 - Add the 'expected' arguments to throws/rejects for security/manager/ssl/tests/unit/test_sts_parser.js.

https://reviewboard.mozilla.org/r/252322/#review258842

Comment 8

[Julian Descottes [:jdescottes]]

Comment on attachment 8987092 [details]
Bug 1470455 - Add the 'expected' arguments to throws/rejects for devtools.

https://reviewboard.mozilla.org/r/252316/#review259356

Thanks, this looks good! Just one minor nit.

::: devtools/client/shared/test/unit/test_suggestion-picker.js:141
(Diff revision 1)
>  }
>  
>  function ensureErrorThrownWithInvalidArguments() {
>    info("Running ensureErrorThrownWithInvalidTypeArgument()");
>  
> -  const expectedError = "Please provide valid items and sortedItems arrays.";
> +  const expectedError = /Please provide valid items and sortedItems arrays./;

nit: If we really want to use regexps, we should escape the final "."

Comment 9

[Dave Townsend [:mossop]]

Comment on attachment 8987096 [details]
Bug 1470455 - Add the 'expected' arguments to throws/rejects for toolkit.

https://reviewboard.mozilla.org/r/252324/#review259384

::: toolkit/components/extensions/test/xpcshell/test_ext_legacy_extension_embedding.js:144
(Diff revision 1)
>  add_task(async function test_startup_error_empty_manifest() {
>    const id = "[e-mail]";
>    const files = {
>      "webextension/manifest.json": ``,
>    };
> -  const expectedError = "(NS_BASE_STREAM_CLOSED)";
> +  const expectedError = /(NS_BASE_STREAM_CLOSED)/;

These brackets either need to be escaped or can be removed.

Comment 10

[Mark Banner (:standard8)]

Comment on attachment 8987092 [details]
Bug 1470455 - Add the 'expected' arguments to throws/rejects for devtools.

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

Comment 11

[Mark Banner (:standard8)]

Comment on attachment 8987093 [details]
Bug 1470455 - Add the 'expected' arguments to throws/rejects for netwerk.

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

Comment 12

[Mark Banner (:standard8)]

Comment on attachment 8987094 [details]
Bug 1470455 - Add the 'expected' arguments to throws/rejects for dom/push/test/xpcshell/test_broadcast_success.js.

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

Comment 13

[Mark Banner (:standard8)]

Comment on attachment 8987095 [details]
Bug 1470455 - Add the 'expected' arguments to throws/rejects for security/manager/ssl/tests/unit/test_sts_parser.js.

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

Comment 14

[Mark Banner (:standard8)]

Comment on attachment 8987096 [details]
Bug 1470455 - Add the 'expected' arguments to throws/rejects for toolkit.

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

Comment 15

[Patrick McManus [:mcmanus]]

Comment on attachment 8987093 [details]
Bug 1470455 - Add the 'expected' arguments to throws/rejects for netwerk.

https://reviewboard.mozilla.org/r/252318/#review261022

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2223f56e1188
Add the 'expected' arguments to throws/rejects for devtools. r=jdescottes
https://hg.mozilla.org/integration/autoland/rev/0c3cfe04d1ad
Add the 'expected' arguments to throws/rejects for netwerk. r=mcmanus
https://hg.mozilla.org/integration/autoland/rev/f4c80c25ab35
Add the 'expected' arguments to throws/rejects for dom/push/test/xpcshell/test_broadcast_success.js. r=lina
https://hg.mozilla.org/integration/autoland/rev/1b2e6b7ea43a
Add the 'expected' arguments to throws/rejects for security/manager/ssl/tests/unit/test_sts_parser.js. r=keeler
https://hg.mozilla.org/integration/autoland/rev/f37e9ac2078d
Add the 'expected' arguments to throws/rejects for toolkit. r=mossop

Comment 17

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/2223f56e1188
https://hg.mozilla.org/mozilla-central/rev/0c3cfe04d1ad
https://hg.mozilla.org/mozilla-central/rev/f4c80c25ab35
https://hg.mozilla.org/mozilla-central/rev/1b2e6b7ea43a
https://hg.mozilla.org/mozilla-central/rev/f37e9ac2078d