885878 - DOM imptests can time out

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[(no longer active)]

See <https://tbpl.mozilla.org/php/getParsedLog.php?id=24434726&tree=Mozilla-Inbound&full=1> for example.  We already disabled testharness.js timeouts: <http://mxr.mozilla.org/mozilla-central/source/dom/imptests/testharnessreport.js#294>... or, did we?  Turns out that because of this line <http://hg.mozilla.org/mozilla-central/annotate/61c3c8b85563/dom/imptests/testharness.js#l1209>, the testharness.js framework was setting up its timeout mechanism way before setup() was ever called.  That's insane.

I'm landing an unreviewed patch to fix that.  Ms2ger, I'd appreciate if you post-review it.  Thanks!

Comment 1

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0c5c910798b7

Comment 2

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/0c5c910798b7

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Please submit this patch upstream.

Comment 4

[(no longer active)]

Sure.  Can you please tell me where I need to submit it exactly?

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://github.com/w3c/testharness.js/

Comment 6

[(no longer active)]

https://github.com/w3c/testharness.js/pull/32

Comment 7

[Phil Ringnalda (:philor)]

Can you please tell me what the review of that upstream submission said?

Comment 8

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

> This looks wrong to me.
>
> If there is no call to setup() in the test, the timeout will never be set, which is wrong.
>
> You probably want to make setting explicit_timeout in the testharnessreport.js
> file have the effect of clearing any existing timeout on the tests object, but
> not setting a new one.

Ehsan, can you address that?

Comment 9

[(no longer active)]

Done: https://github.com/w3c/testharness.js/pull/32#issuecomment-28723094

Comment 10

[(no longer active)]

OK, my patch was accepted upstream, what should we do about this bug now?

Comment 11

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Patch v1

Comment 12

[(no longer active)]

This is upstream code, it shouldn't need review.

Comment 13

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/dc71a9c74cab