Closed Bug 1246152 Opened 7 years ago Closed 7 years ago

Bump the mochitest time out from 45 to 90 seconds

Categories

(Testing :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: armenzg, Assigned: armenzg)

References

Details

Attachments

(1 file, 1 obsolete file)

<jmaher> armenzg: I looked into the bc6 failure (browser_crashedTabs.js)
<jmaher> guess what...it normally takes 80+ seconds to run, lets add 20-30% overhead and we cross the 90 second timeout
...
<jmaher> https://dxr.mozilla.org/mozilla-central/source/browser/components/sessionstore/test/browser_crashedTabs.js#6
<jmaher> armenzg: default timeout is 45 seconds for browser-chrome, ^ makes it 90, lets change that to requestLongerTimeout(3)
<jmaher> sadly ehsan just bumped the timeout 5 weeks ago
...
<jmaher> ehsan: just that we need to do it again because docker runs tests ~20% slower
...
<jmaher> ehsan: it wasn't sad that you bumped it, just sad that we need to do it again
<ehsan> we should revise the harness timeout
<jmaher> ehsan: the default timeout for browser-chrome is 45
<jmaher> 45 seconds
<ehsan> I think we should bump it up to 1:30 mins
<jmaher> so 90 seconds, and remove a lot of the extra multipliers?
<ehsan> I'd leave the multipliers in
<ehsan> they won't really hurt us
<jmaher> alright
<ehsan> unless if one of those tests actually times out in which case we're wasting a few mins
<ehsan> machine time $$$ <<< human $$$
<jmaher> well, we can justify it "because docker takes longer to run, lets do this across the board"
<ehsan> jmaher: smart plan :D
<ehsan> r-me
<ehsan> err
<ehsan> = :D
<jmaher> ehsan: well, we don't want a job sitting for 20 minutes, then timing out- ideally we can fail faster
<jmaher> ehsan: for browser-chrome only or mochitest/chrome also?
<jmaher> armenzg: here is where the default is set: https://dxr.mozilla.org/mozilla-central/source/testing/mochitest/browser-test.js#3
<ehsan> jmaher: I don't think we have a lot of tests with multipliers > 4
<ehsan> 4*90 << 20mins :)
<ehsan> jmaher: I haven't really looked at how bad this is on other test suites...
<ehsan> but don't have anything against doing it across the board
<jmaher> ehsan: I would prefer to do it just for browser-chrome (and devtools would get it as well)
<ehsan> sgtm
<jmaher> ok, nice
<jmaher> armenzg_brb: I don't think this will solve the other issues, it might fix a few cases in devtools e10s, but those are still in cleanup mode outside of the scope of taskcluster/docker
<jmaher> ehsan: we have a bunch that are 10 or higher: https://dxr.mozilla.org/mozilla-central/search?q=requestlongertimeout%281&redirect=false&case=false
<jmaher> ^ that has some mochitest-plain in there as well, so 12 test cases have a multiplier of 10
<ehsan> jmaher: hmm, ok maybe we can lower those
Comment on attachment 8716408 [details] [diff] [review]
Bump mochitest time out from 45 to 90 seconds + reduce browser_chrome multiplies over 5

Review of attachment 8716408 [details] [diff] [review]:
-----------------------------------------------------------------

these changes look great, but we are missing the 45 -> 90 second adjustment in this patch.
Attachment #8716408 - Flags: feedback?(jmaher) → feedback+
Not embarassing whatsoever! :) I've got too focused on the multipliers. I will try again later today.
Comment on attachment 8717111 [details] [diff] [review]
Bump mochitest time out from 45 to 90 seconds + reduce browser_chrome multiplies over 5

Review of attachment 8717111 [details] [diff] [review]:
-----------------------------------------------------------------

Please announce this change on dev-platform and firefox-dev.  Thanks!
Attachment #8717111 - Flags: review?(ehsan) → review+
We've got another change going on which might require this unneeded.

We can run the tests on mounted dirs that are run on SSD.
As far as I can tell, browser_crashedTabs.js is not timing out anymore [1]

For my own record, 60 seconds also works well.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=84fbd2ecb0dd&filter-searchStr=browser-chrome&group_state=expanded
I don't think we need this anymore.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.