Closed Bug 1470455 Opened 5 years ago Closed 5 years ago

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

Categories

(Toolkit :: General, enhancement, P1)

Product:
Toolkit
Component:
General
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla63
Tracking Flags:
Tracking Status
firefox63 --- fixed

People

(Reporter: standard8, Assigned: standard8)

References

Details

Attachments

(5 files)

Bug 1470455 - Add the 'expected' arguments to throws/rejects for devtools.
59 bytes, text/x-review-board-request
jdescottes
: review+
Details
Bug 1470455 - Add the 'expected' arguments to throws/rejects for netwerk.
59 bytes, text/x-review-board-request
mcmanus
: review+
Details
Bug 1470455 - Add the 'expected' arguments to throws/rejects for dom/push/test/xpcshell/test_broadcast_success.js.
59 bytes, text/x-review-board-request
lina
: review+
Details
Bug 1470455 - Add the 'expected' arguments to throws/rejects for security/manager/ssl/tests/unit/test_sts_parser.js.
59 bytes, text/x-review-board-request
keeler
: review+
Details
Bug 1470455 - Add the 'expected' arguments to throws/rejects for toolkit.
59 bytes, text/x-review-board-request
mossop
: review+
Details 
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 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
Attachment #8987094 - Flags: review?(kit) → review+
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
Attachment #8987095 - Flags: review?(dkeeler) → review+
Priority: -- → P1
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 "."
Attachment #8987092 - Flags: review?(jdescottes) → review+
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 = "empty-manifest@test.embedded.web.extension";
>    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.
Attachment #8987096 - Flags: review?(dtownsend) → review+
Comment on attachment 8987093 [details]
Bug 1470455 - Add the 'expected' arguments to throws/rejects for netwerk.

https://reviewboard.mozilla.org/r/252318/#review261022
Attachment #8987093 - Flags: review?(mcmanus) → review+
Pushed by mbanner@mozilla.com:
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
You need to log in before you can comment on or make changes to this bug.