ESlint rule 'no-arbitrary-setTimeout' enabled on xpcshell tests

[Andrew Halberstadt]

As part of a larger effort to reduce oranges, we are starting to lint our
tests for common causes of intermittent failures. One low-hanging fruit is
preventing setTimeout with an arbitrary value (aka non-zero) as opposed to
waiting for an appropriate event. The mochitest harness already prevents
this in the harness itself (SimpleTest.requestFlakyTimeout), so this rule
is only enabled on xpcshell tests for now.

If you need to use a flaky setTimeout for some reason, you can disable the
rule at the directory level, file level or line level:
http://eslint.org/docs/user-guide/configuring#configuring-rules

It has been disabled in the following files due to pre-existing violations:
http://searchfox.org/mozilla-central/search?q=eslint-disable+mozilla%2Fno-arbitrary-setTimeout

Let me know if you think this should be enabled on any other test suites.
-Andrew

[Felipe G]

I'll note that requestFlakyTimeout is only enabled for mochitest-plain at
the moment: http://searchfox.org/mozilla-central/source/testing/
mochitest/tests/SimpleTest/SimpleTest.js#666
So browser-chrome / a11y / chrome tests are still able to use non-0
timeouts.

Cheers,
Felipe

> _______________________________________________
> dev-platform mailing list
> dev-pl...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>

[Andrew Halberstadt]

Ah, good to know! I'll file a follow-up to enable the eslint rule on
browser/a11y/chrome. Maybe eventually we can just replace the
requestFlakyTimeout mechanism with this eslint rule. I decided to punt on
that as I'm not sure if eslint is running on 100% of mochitests yet.

[Josh Matthews]

Since this seems to only look for setTimeout, which is a relatively rare
construct in xpcshell tests, should we be looking for do_timeout calls
that use a non-zero argument as well?

Cheers,
Josh

[Andrew Halberstadt]

This is now also enabled on browser-chrome tests. Bug 1389234 has been
filed to track deprecating SimpleTest.requestFlakyTimeout on
mochitest-plain and chrome in favour of this new rule.

-Andrew

On Fri, Jul 28, 2017 at 12:02 PM Andrew Halberstadt <
ahalbe...@mozilla.com> wrote:

> Ah, good to know! I'll file a follow-up to enable the eslint rule on
> browser/a11y/chrome. Maybe eventually we can just replace the
> requestFlakyTimeout mechanism with this eslint rule. I decided to punt on
> that as I'm not sure if eslint is running on 100% of mochitests yet.
>
> On Fri, Jul 28, 2017 at 11:56 AM Felipe G <fel...@gmail.com> wrote:
>
>> I'll note that requestFlakyTimeout is only enabled for mochitest-plain at
>> the moment:
>> http://searchfox.org/mozilla-central/source/testing/mochitest/tests/SimpleTest/SimpleTest.js#666
>> So browser-chrome / a11y / chrome tests are still able to use non-0
>> timeouts.
>>
>> Cheers,
>> Felipe
>>
>> On Fri, Jul 28, 2017 at 12:48 PM, Andrew Halberstadt <

>> ahalbe...@mozilla.com> wrote:
>>
>>> As part of a larger effort to reduce oranges, we are starting to lint our
>>> tests for common causes of intermittent failures. One low-hanging fruit
>>> is
>>> preventing setTimeout with an arbitrary value (aka non-zero) as opposed
>>> to
>>> waiting for an appropriate event. The mochitest harness already prevents
>>> this in the harness itself (SimpleTest.requestFlakyTimeout), so this rule
>>> is only enabled on xpcshell tests for now.
>>>
>>> If you need to use a flaky setTimeout for some reason, you can disable
>>> the
>>> rule at the directory level, file level or line level:
>>> http://eslint.org/docs/user-guide/configuring#configuring-rules
>>>
>>> It has been disabled in the following files due to pre-existing
>>> violations:
>>>
>>> http://searchfox.org/mozilla-central/search?q=eslint-disable+mozilla%2Fno-arbitrary-setTimeout
>>>
>>> Let me know if you think this should be enabled on any other test suites.
>>> -Andrew
>>>