Closed
Bug 1383120
Opened 7 years ago
Closed 7 years ago
Prevent arbitrary setTimeout's in xpcshell via eslint rule
Categories
(Testing :: XPCShell Harness, enhancement)
Testing
XPCShell Harness
Tracking
(firefox56 fixed)
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: ahal, Assigned: ahal)
References
Details
Attachments
(3 files, 1 obsolete file)
This is focusing on xpcshell for now, as the mochitest harness already prevents flaky setTimeouts at runtime. Enabling an additional eslint check to prevent it there would be annoying to developers, as then they'd need to opt-out of the check in two different ways. Eventually we may want to remove the runtime check from the mochitest harness and switch over to the eslint rule, but that is out of scope for now. Since xpcshell tests can potentially live alongside mochitests, it'll be difficult to run this *only* on xpcshell tests but not mochitests. The "overrides" key from eslint 4 might be of use here, but until then, the rule could do something like: if (helpers.getTestType() !== "xpcshell") { return; } This might mean that it misses a few xpcshell tests, but won't run on any mochitests.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•7 years ago
|
||
I'm not sure what the protocol is when enabling new rules, is it right to disable the rule at the config level? Should it be disabled at the file or line level instead?
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8889574 [details] Bug 1383120 - [eslint-plugin-mozilla] Fix getTestType() helper so mochitests aren't treated as xpcshell tests, https://reviewboard.mozilla.org/r/160592/#review166402
Attachment #8889574 -
Flags: review?(dtownsend) → review+
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8889575 [details] Bug 1383120 - [eslint-plugin-mozilla] Add no-arbitrary-setTimeout eslint rule, https://reviewboard.mozilla.org/r/160594/#review166404
Attachment #8889575 -
Flags: review?(dtownsend) → review+
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8889576 [details] Bug 1383120 - Enable no-arbitrary-setTimeout eslint rule on xpcshell tests, https://reviewboard.mozilla.org/r/160596/#review166432 I think I'd rather we just disable this rule on a per-file basis given that it isn't that many files. This way people can't add new tests that fail the test to those directories.
Attachment #8889576 -
Flags: review?(dtownsend) → review-
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8889576 -
Attachment is obsolete: true
Comment 11•7 years ago
|
||
mozreview-review |
Comment on attachment 8890375 [details] Bug 1383120 - Enable no-arbitrary-setTimeout eslint rule on xpcshell tests, https://reviewboard.mozilla.org/r/161492/#review166834 Thanks for this!
Attachment #8890375 -
Flags: review?(dtownsend) → review+
Comment 12•7 years ago
|
||
Pushed by ahalberstadt@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7d00186874cf [eslint-plugin-mozilla] Fix getTestType() helper so mochitests aren't treated as xpcshell tests, r=mossop https://hg.mozilla.org/integration/autoland/rev/4653ae9b9f17 [eslint-plugin-mozilla] Add no-arbitrary-setTimeout eslint rule, r=mossop https://hg.mozilla.org/integration/autoland/rev/e18f2b95fc8e Enable no-arbitrary-setTimeout eslint rule on xpcshell tests, r=mossop
Comment 13•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7d00186874cf https://hg.mozilla.org/mozilla-central/rev/4653ae9b9f17 https://hg.mozilla.org/mozilla-central/rev/e18f2b95fc8e
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in
before you can comment on or make changes to this bug.
Description
•