Mark Banner (:standard8) Assignee
	Description • 6 years ago

This is continuing roll-out of require-expected-throws-or-rejects as per bug 1452706.

	Andreas Tolfsen ❲:ato❳
	Comment 2 • 6 years ago
mozreview-review

Comment on attachment 8981957 [details]
Bug 1465530 - Enable ESLint rule require-expected-throws-or-rejects for testing/marionette/.

https://reviewboard.mozilla.org/r/247970/#review254096

This is great!

We have had real bugs in our test suite as a result of incorrect
Assert.throws usage in the past, so I’m very grateful you took the
time to write this rule.

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 3 • 6 years ago

So this only adds the tests but not any js file under testing/marionette directly. Or do I miss something?

	Mark Banner (:standard8) Assignee
	Comment 5 • 6 years ago

(In reply to Henrik Skupin (:whimboo) from comment #3)
> So this only adds the tests but not any js file under testing/marionette
> directly. Or do I miss something?

The ESLint configuration is controlled from the top-level srcdir .eslintrc.js file (sub directories can set extra rules, but they're either extending the top-level one or overriding it, generally I prefer not overriding). In this case the ESLint rule is set in recommended.js (which the top-level extends), but was whitelisted for certain directories in that file whilst we roll it out.

If you mean the Assert.throws/rejects calls, then they're just being updated to be explicit about which exception is expected (we've had lots of cases of these catching the wrong thing, or just passing because of broken code). The exceptions are already thrown in the code.

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 6 • 6 years ago

I see. Thanks.

	Pulsebot
	Comment 7 • 6 years ago

Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3e9d04bbec91
Enable ESLint rule require-expected-throws-or-rejects for testing/marionette/. r=ato

	Noemi Erli[:noemi_erli]
	Comment 8 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/3e9d04bbec91