Steeplechase_long tests need to be enhanced to run for 3 hours

RESOLVED FIXED in Firefox 42

Status

()

defect
P2
major
Rank:
25
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: rshenthar, Assigned: rshenthar, Mentored)

Tracking

unspecified
mozilla42
Points:
---

Firefox Tracking Flags

(firefox42 fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

It seems like due to recent migration of the webRTC mochitest scripts to ES6, the steeplechase_long scripts have been broken. We need to correct this.
Assignee: nobody → rshenthar
Severity: normal → minor
Priority: -- → P4
Severity: minor → major
backlog: --- → webRTC+
Rank: 25
Priority: P4 → P2
Bug 1182626 - Steeplechase_long tests need to be enhanced to run for 3 hours
Added var for declaration of variable
Attachment #8633001 - Flags: review?(drno)
Attachment #8633000 - Flags: review?(drno)
Comment on attachment 8633000 [details]
MozReview Request: Bug 1182626 - Steeplechase_long tests need to be enhanced to run for 3 hours

Bug 1182626 - Steeplechase_long tests need to be enhanced to run for 3 hours
Attachment #8633000 - Flags: review?(drno)
Comment on attachment 8633000 [details]
MozReview Request: Bug 1182626 - Steeplechase_long tests need to be enhanced to run for 3 hours

https://reviewboard.mozilla.org/r/13163/#review11891

::: dom/media/moz.build:89
(Diff revision 1)
> +    WEBRTC_SIGNALLING_TEST_MANIFESTS += ['tests/mochitest/steeplechase_long.ini']

I think this change is not needed for fixing the _long functionality, or?

::: dom/media/tests/mochitest/head.js:158
(Diff revision 1)
> +  // This conditional code is meant to handle timeouts for scripts that 
> +  // run for long. The variable STEEPLECHASE_TIMEOUT 
> +  // is optionally set from within the testcase.

I would re-word this into something like:
"The optional declaration of the variable STEEPLECHASE_TIMEOUT allows to override the default script timeout of 30s".

::: dom/media/tests/mochitest/long.js:177
(Diff revision 1)
> + * This function returns a promise that will resolve once the link repeatedly 

Can you please remove the trailing space here?

::: dom/media/tests/mochitest/long.js:213
(Diff revision 1)
> -          test.next();
> +          //test.next();

Please remove this commented code.

::: dom/media/tests/mochitest/long.js:192
(Diff revision 1)
> -  return [
> -    name,
> +  var commandForInterval =
> +    function INTERVAL_COMMAND(test) {

I would prefer if we would simply return the function here directly instead of declaring a variable just to return the variable later.
Comment on attachment 8633001 [details]
MozReview Request: Added var for declaration of variable

https://reviewboard.mozilla.org/r/13165/#review11897

Hmm this change appears empty for me in mozreview.
Attachment #8633001 - Flags: review?(drno) → review+
Comment on attachment 8633000 [details]
MozReview Request: Bug 1182626 - Steeplechase_long tests need to be enhanced to run for 3 hours

https://reviewboard.mozilla.org/r/13163/#review11899

Ship It!
Attachment #8633000 - Flags: review+
(In reply to Nils Ohlmeier [:drno] from comment #6)
> Comment on attachment 8633000 [details]
> MozReview Request: Bug 1182626 - Steeplechase_long tests need to be enhanced
> to run for 3 hours
> 
> https://reviewboard.mozilla.org/r/13163/#review11899
> 
> Ship It!

Just to be clear: what I meant is "fix it, then ship it" :-)
Fixed review comments
Attachment #8633805 - Flags: review?(drno)
Removed unnecessary ini file change
Attachment #8633809 - Flags: review?(drno)
Keywords: checkin-needed
Comment on attachment 8633805 [details]
MozReview Request: Fixed review comments

https://reviewboard.mozilla.org/r/13267/#review11933

Ship It!
Attachment #8633805 - Flags: review?(drno) → review+
Attachment #8633809 - Flags: review?(drno) → review+
Comment on attachment 8633809 [details]
MozReview Request: Removed unnecessary ini file change

https://reviewboard.mozilla.org/r/13269/#review11935

Ship It!
Needs to be rebased on inbound tip. Also, can you please squash your commits into one suitable for landing?
Keywords: checkin-needed
Comment on attachment 8633000 [details]
MozReview Request: Bug 1182626 - Steeplechase_long tests need to be enhanced to run for 3 hours

Bug 1182626 - Steeplechase_long tests need to be enhanced to run for 3 hours
Attachment #8633000 - Flags: review+ → review?(drno)
Attachment #8633001 - Attachment is obsolete: true
Attachment #8633805 - Attachment is obsolete: true
Attachment #8633809 - Attachment is obsolete: true
Rebased on inbound and squashed collapsed all =>
https://reviewboard.mozilla.org/r/13161/
Comment on attachment 8633000 [details]
MozReview Request: Bug 1182626 - Steeplechase_long tests need to be enhanced to run for 3 hours

https://reviewboard.mozilla.org/r/13163/#review13623

Ship It!
Attachment #8633000 - Flags: review?(drno) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/ea804beb9ff6
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.