Closed Bug 1320236 Opened 4 years ago Closed 4 years ago

Intermittent dom/animation/test/chrome/test_animation_performance_warning.html | Test timed out. - Test timed out.

Categories

(Core :: DOM: Animation, defect, P1)

Product:
Core
Component:
DOM: Animation
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla53
Tracking Flags:
Tracking Status
firefox51 --- unaffected
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: hiro)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

Bug 1320236 - Use SimpleTest.requestLongerTimeout to avoid timeouts on Android. .
58 bytes, text/x-review-board-request
gbrown
: review+
Details
Flags: needinfo?(hiikezoe)
This seems a real time out.  It took 300s in the failure case.  I was thinking those tests in test_animation_performance_warning.html can be run in parallel (i.e. using async_test instead of promise_test) but once bug 1301305 fixed, it will not work in some test cases.
https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&tochange=f0c1e6b61ded83d3b646f55bb0f7cabdf822912b&fromchange=40c8129cffbf82cd89b4e22caf52aa042b5573e4&filter-searchStr=android%20opt%20chrome shows this test's run time increased significantly on Android with bug 1301305. Before the change, on Android 4.3 API15+ opt, test_animation_performance_warning.html typically completed in under 200 seconds; after the bug 1301305 change, it often timed out (330 seconds+) or took more than 270 seconds.

To resolve the timeout, options include:
 - optimize the test to significantly reduce the run time on Android
 - split the test into 2 or more shorter tests
 - requestLongerTimeout(2) in the test
 - skip the test on Android
Blocks: 1301305
(In reply to Geoff Brown [:gbrown] from comment #4)

> To resolve the timeout, options include:
>  - optimize the test to significantly reduce the run time on Android
>  - split the test into 2 or more shorter tests
>  - requestLongerTimeout(2) in the test
>  - skip the test on Android

This test file uses testharness.js, can we use requestLongerTimeout there?

I did try "<meta name=timeout content=long>" or "setup({timeout_multiplier: 2})" but no luck.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2af72271bc5f5e8b9087a939cfaafca717e0c92d&selectedJob=32578993
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8fd93ca1a222c20f412d75778ae516560451dd64&selectedJob=32565840
(In reply to Hiroyuki Ikezoe (:hiro) from comment #5)
> This test file uses testharness.js, can we use requestLongerTimeout there?

I'm not sure. Let's see if this works:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=73bbe6b0af87e6e5d5d9ea174ec798741927c3f9
I also tried.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0e2f213c733783b650b1f7d760c5e212b62db237

There seems to be something missing.  I did confirm it worked harmlessly on a single run locally.  But on the try, the test did not fail at this point, but the parser of the test results (or something) output 'Unnamed step' error.
requestLongerTimeout was effective there. For example, on below try test_animation_performance_warning.html took over 300s, but timeout did not happen.

https://archive.mozilla.org/pub/mobile/try-builds/hiikezoe@mozilla-japan.org-0e2f213c733783b650b1f7d760c5e212b62db237/try-android-api-15/try_ubuntu64_vm_armv7_mobile_test-mochitest-chrome-bm67-tests1-linux64-build96.txt.gz

We'd avoid the 'Unnamed step' error somehow, *or* extend testharness.js to able to multiply the timeout value in TestRunner.js.  I'd prefer the latter.
OK. I found that we can call TestRunner.requestLongerTimeout through W3C object in testharnessreports.js [1].

[1] https://hg.mozilla.org/mozilla-central/file/489f981e8c2b/dom/imptests/testharnessreport.js#l41

Let's see if it works on try.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=94637d7e3c7ded5929f37bc0638108e9cda6e956
Flags: needinfo?(hiikezoe)
Raising priority because this failure is very high frequent, currently top 5.
Priority: -- → P1
(In reply to Hiroyuki Ikezoe (:hiro) from comment #9)
> OK. I found that we can call TestRunner.requestLongerTimeout through W3C
> object in testharnessreports.js [1].
> 
> [1]
> https://hg.mozilla.org/mozilla-central/file/489f981e8c2b/dom/imptests/
> testharnessreport.js#l41
> 
> Let's see if it works on try.
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=94637d7e3c7ded5929f37bc0638108e9cda6e956

Yay! The try looks good to me. There is a run that test_animation_performance_warning.html took over 300s [1].

[1] https://archive.mozilla.org/pub/mobile/try-builds/hiikezoe@mozilla-japan.org-94637d7e3c7ded5929f37bc0638108e9cda6e956/try-android-api-15/try_ubuntu64_vm_armv7_mobile_test-mochitest-chrome-bm117-tests1-linux64-build951.txt.gz
Geoff, could you please review the patch?  Our animation team members are not available for a while.
Thanks!
Assignee: nobody → hiikezoe
Status: NEW → ASSIGNED
Comment on attachment 8818441 [details]
Bug 1320236 - Use SimpleTest.requestLongerTimeout to avoid timeouts on Android. .

https://reviewboard.mozilla.org/r/98496/#review98848

Thank you very much Hiroyuki!
Attachment #8818441 - Flags: review?(gbrown) → review+
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/403bfd796368
Use SimpleTest.requestLongerTimeout to avoid timeouts on Android. r=gbrown.
https://hg.mozilla.org/mozilla-central/rev/403bfd796368
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.