DOM imptests can time out

RESOLVED FIXED in Firefox 28

Status

()

defect
RESOLVED FIXED
6 years ago
4 months ago

People

(Reporter: Ehsan, Assigned: Ms2ger)

Tracking

Trunk
mozilla28
x86
macOS
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(firefox28 fixed)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment)

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!
Flags: needinfo?(Ms2ger)
https://hg.mozilla.org/mozilla-central/rev/0c5c910798b7
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Please submit this patch upstream.
Status: RESOLVED → REOPENED
Flags: needinfo?(Ms2ger) → needinfo?(ehsan)
Resolution: FIXED → ---
Sure.  Can you please tell me where I need to submit it exactly?
Flags: needinfo?(ehsan)
Can you please tell me what the review of that upstream submission said?
Flags: needinfo?(Ms2ger)
> 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?
Flags: needinfo?(Ms2ger) → needinfo?(ehsan)
OK, my patch was accepted upstream, what should we do about this bug now?
Flags: needinfo?(Ms2ger)
Posted patch Patch v1Splinter Review
Assignee: ehsan → Ms2ger
Status: REOPENED → ASSIGNED
Attachment #8336750 - Flags: review?
Flags: needinfo?(Ms2ger)
This is upstream code, it shouldn't need review.
Attachment #8336750 - Flags: review? → review?(james)
Attachment #8336750 - Flags: review?(james) → review+
https://hg.mozilla.org/mozilla-central/rev/dc71a9c74cab
Status: ASSIGNED → RESOLVED
Closed: 6 years ago6 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: mozilla24 → mozilla28
Whiteboard: [qa-]
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.