1298909 - slowest test information is incorrect

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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!

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The first two lines quoted seem correct based on how long things took; it's the third quoted line that's wrong.

Comment 2

[Prakhar Agrawal [:prakhar0409]]

Attachment: 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

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

This is an interesting find!  Can we fix TestRunner.js to update the slowestTestTime ignoring timeoutFactor?

Comment 4

[Prakhar Agrawal [:prakhar0409]]

Attachment: Bug 1298909 - report slowest test in all cases, even with requestLongerTimeout() is called.

Review commit: https://reviewboard.mozilla.org/r/78986/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/78986/

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

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 6

[Prakhar Agrawal [:prakhar0409]]

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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/78986/diff/1-2/

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

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.

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

pushed to try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=15f961009c80

and all is well!

Comment 9

[Joel Maher ( :jmaher ) (UTC -8)]

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

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/42fd3f718fdc
report slowest test in all cases, even with requestLongerTimeout() is called. r=jmaher

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/42fd3f718fdc