Write a rule to remove cases of run_test that aren't needed

RESOLVED FIXED in Firefox 57

Status

enhancement
RESOLVED FIXED
2 years ago
5 months ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

3 Branch
mozilla57

Firefox Tracking Flags

(firefox57 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

2 years ago
Bug 982852 made run_test() in xpcshell tests optional. There's still quite a few cases where we do:

function run_test() {
  run_next_test();
}

despite the fact that xpcshell's head.js defines this for us. Hence this is just extra noise in test files.

I keep on coming across these (and cleaning them up), so I've decided it is time just to get a rule setup with fix capability and remove them completely.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 5

2 years ago
mozreview-review
Comment on attachment 8899277 [details]
Bug 1392098 - Enable the new ESLint no-useless-run-test rule across the tree.

https://reviewboard.mozilla.org/r/170516/#review176498
Attachment #8899277 - Flags: review?(dtownsend) → review+

Comment 6

2 years ago
mozreview-review
Comment on attachment 8899276 [details]
Bug 1392098 - Add an ESLint rule to avoid unnecessary run_test() functions.

https://reviewboard.mozilla.org/r/170514/#review176500

Do you happen to know if eslint's test harness verifies that the fixer is producing valid code?
Attachment #8899276 - Flags: review?(dtownsend) → review+

Comment 7

2 years ago
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fde66058d330
Add an ESLint rule to avoid unnecessary run_test() functions. r=mossop
https://hg.mozilla.org/integration/autoland/rev/2dd4fb2e079a
Enable the new ESLint no-useless-run-test rule across the tree. r=mossop
(Assignee)

Comment 8

2 years ago
mozreview-review-reply
Comment on attachment 8899276 [details]
Bug 1392098 - Add an ESLint rule to avoid unnecessary run_test() functions.

https://reviewboard.mozilla.org/r/170514/#review176500

I just did an experiment that left the closing brace of the function in the code - it correctly returned that there was a parsing error. I've also just seen in the documentation (https://eslint.org/docs/developer-guide/working-with-rules#applying-fixes):

```
After applying fixes, ESLint will run all of the enabled rules again on the fixed code, potentially applying more fixes. This process will repeat up to 10 times, or until no more fixable problems are found.
```
https://hg.mozilla.org/mozilla-central/rev/fde66058d330
https://hg.mozilla.org/mozilla-central/rev/2dd4fb2e079a
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57

Updated

a year ago
Product: Testing → Firefox Build System
Version: Version 3 → 3 Branch
You need to log in before you can comment on or make changes to this bug.