Closed
Bug 832590
Opened 11 years ago
Closed 11 years ago
(sdk/test) Need a way to extend test timeout time
Categories
(Add-on SDK Graveyard :: General, defect, P2)
Add-on SDK Graveyard
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: evold, Assigned: ochameau)
Details
Attachments
(1 file)
See https://groups.google.com/forum/?fromgroups=#!topic/mozilla-labs-jetpack/14jtywmej4o Some tests take a long time, and the default timeout time needs to be bumped up in the NodeJS style tests.
Reporter | ||
Updated•11 years ago
|
Flags: needinfo?(rFobic)
OS: Mac OS X → All
Hardware: x86 → All
Comment 1•11 years ago
|
||
I do believe commonjs test spec does not specifies timeout or a way to specify it because test are not supposed to timeout. In other words test will be `done` only after done is called. If for some reason you still want to set time limit for tests it's super easy too: setTimeout(done, time) So I'd suggest just switch default timeout to infinity and only opt-in to timeout if explicit number is specified.
Flags: needinfo?(rFobic)
Assignee | ||
Comment 2•11 years ago
|
||
Irakli, I think that we should still keep a timeout per test, just in order to ensure releasing the test runner ASAP and allow it to run test on another revision. I can easily agree on the fact that developers should only set shorter timeout but should never have to specify a longer one. What do you think about that: we increase the default timeout to 5 minutes ? There is multiple level of timeouts: - One is done by tinderbox itself (if I remember correctly), - Two other timeouts are done by cfx in python: * If nothing comes out of stdout after 1 minutes, cfx quits, * If the test takes more than 30 minutes to run, cfx quites. - Test runner executes a timeout per test. Default to 10s, and now suggesting 5m All these timeouts are set to values so that when you hit them, you really are in troubles with such slow tests.
Assignee: nobody → poirot.alex
Attachment #705931 -
Flags: review?(rFobic)
Priority: -- → P2
Comment 3•11 years ago
|
||
Comment on attachment 705931 [details]
Pull request 744
I think cfx will kill the process if there is no output for that long anyway, but sure this works for me.
Attachment #705931 -
Flags: review?(rFobic) → review+
Comment 4•11 years ago
|
||
Commits pushed to master at https://github.com/mozilla/addon-sdk https://github.com/mozilla/addon-sdk/commit/ef498664fe6cc3023cac324ee50e3ebd96171639 Bug 832590 - (sdk/test) Need a way to extend test timeout time https://github.com/mozilla/addon-sdk/commit/c5e7a2420efc6f7f3b97f542ba4c2433a034415a Merge pull request #744 from ochameau/max-timeout@832590 Fix Bug 832590 - (sdk/test) Need a way to extend test timeout time
Updated•11 years ago
|
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
I updated the Jetpack tests being run on mozilla-inbound to pick up this change: http://hg.mozilla.org/integration/mozilla-inbound/rev/8ee1b05b3b86
Comment 6•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/8ee1b05b3b86
Comment 7•11 years ago
|
||
Commits pushed to integration at https://github.com/mozilla/addon-sdk https://github.com/mozilla/addon-sdk/commit/ef498664fe6cc3023cac324ee50e3ebd96171639 Bug 832590 - (sdk/test) Need a way to extend test timeout time https://github.com/mozilla/addon-sdk/commit/c5e7a2420efc6f7f3b97f542ba4c2433a034415a Merge pull request #744 from ochameau/max-timeout@832590
You need to log in
before you can comment on or make changes to this bug.
Description
•