(sdk/test) Need a way to extend test timeout time

RESOLVED FIXED

Status

defect
P2
normal
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: evold, Assigned: ochameau)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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.
Flags: needinfo?(rFobic)
OS: Mac OS X → All
Hardware: x86 → All
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

7 years ago
Posted file Pull request 744
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 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

7 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

7 years ago
Status: NEW → RESOLVED
Closed: 7 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 7

7 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.