slowest test information is incorrect

RESOLVED FIXED in Firefox 51

Status

Testing
Mochitest
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: dbaron, Assigned: prakhar0409)

Tracking

Trunk
mozilla51
Points:
---

Firefox Tracking Flags

(firefox51 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

a year ago
I just ran:

./mach test layout/style/test/test_selectors.html layout/style/test/test_change_hint_optimizations.html

in a Linux debug build.

and a few key lines quoted from within the output are:

>4 INFO TEST-OK | layout/style/test/test_change_hint_optimizations.html | took 1407ms

>3976 INFO TEST-OK | layout/style/test/test_selectors.html | took 26924ms

>3982 INFO Slowest: 1406ms - /tests/layout/style/test/test_change_hint_optimizations.html

This seems to be lying about which test was slowest!
(Reporter)

Comment 1

a year ago
The first two lines quoted seem correct based on how long things took; it's the third quoted line that's wrong.
(Assignee)

Comment 2

a year ago
Created attachment 8791198 [details] [diff] [review]
mypatch.diff is an potential solution to the posted bug.

removed the requestLongerTimeout(2) from layout/style/test/test_selector.html -> It was changing the timeoutFactor to 2 but TestRunner.js updates slowestTestTime only if timeoutFactor is 1
This is an interesting find!  Can we fix TestRunner.js to update the slowestTestTime ignoring timeoutFactor?
Assignee: nobody → prakhar0409
Comment hidden (mozreview-request)
Prakhar, thanks for the mozreview request!  I would like to not comment out the request for longer timeout, that is needed in cases of debug tests on certain platforms.

Can we have this print out the proper information inside of TestRunner.js ?

One other thing, in the commit message for the patch/review- could you adjust it slightly:
Bug 1298909 - <description>. r?jmaher

make the description something smaller that can easily be read on one line, maybe "report slowest test in all cases, even with requestLongerTimeout() is called."
Comment hidden (mozreview-request)

Comment 7

a year ago
mozreview-review
Comment on attachment 8791580 [details]
Bug 1298909 - report slowest test in all cases, even with requestLongerTimeout() is called.

https://reviewboard.mozilla.org/r/78986/#review77844

this seems like the right thing!  let me push to try to make sure nothing odd comes out of this.
Attachment #8791580 - Flags: review?(jmaher) → review+
pushed to try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=15f961009c80

and all is well!

Comment 9

a year ago
mozreview-review
Comment on attachment 8791580 [details]
Bug 1298909 - report slowest test in all cases, even with requestLongerTimeout() is called.

https://reviewboard.mozilla.org/r/78986/#review77918

Comment 10

a year ago
Pushed by jmaher@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/42fd3f718fdc
report slowest test in all cases, even with requestLongerTimeout() is called. r=jmaher

Comment 11

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/42fd3f718fdc
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.