Closed
Bug 1388726
Opened 8 years ago
Closed 8 years ago
Reftest-unaccelerated suite might not be running as intended
Categories
(Testing :: General, enhancement)
Tracking
(firefox57 fixed)
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: kats, Assigned: kats)
References
Details
Attachments
(2 files)
So.. on Linux, the reftest-unaccelerated suite runs with layers.acceleration.force-enabled=disabled [1]. First of all, this is just plain wrong because layers.acceleration.force-enabled is a boolean pref and here we appear to be setting to the string "disabled".
On top of that, I'm pretty sure that just disabling the force-enabled pref is not what is desired here. If we want to ensure that the reftests run without acceleration we should actually be setting layers.acceleration.disabled=true to force-disable it [2] as opposed to just not force-enabling it [3].
AFAIK this dates back to bug 837695 when this was first introduced in mozharness in [4].
Also note that on Windows, the reftest-unaccelerated suite does in fact run with layers.acceleration.disabled=true [5]
[1] http://searchfox.org/mozilla-central/rev/0f16d437cce97733c6678d29982a6bcad49f817b/testing/mozharness/configs/unittests/linux_unittest.py#230
[2] http://searchfox.org/mozilla-central/rev/bd39b6170f04afeefc751a23bb04e18bbd10352b/gfx/thebes/gfxPlatform.cpp#2400
[3] http://searchfox.org/mozilla-central/rev/bd39b6170f04afeefc751a23bb04e18bbd10352b/gfx/thebes/gfxPlatform.cpp#2414
[4] https://hg.mozilla.org/build/mozharness/rev/8dc8a872f3cc
[5] http://searchfox.org/mozilla-central/rev/0f16d437cce97733c6678d29982a6bcad49f817b/testing/mozharness/configs/unittests/win_unittest.py#206
Assignee | ||
Comment 1•8 years ago
|
||
Assignee | ||
Comment 2•8 years ago
|
||
AFAICT linux has acceleration disabled by default so in this case this bug appears to be harmless. We're leaving force-enabled as effectively false (the default) and also not force-disabling, but it was never enabled to being with and so the end result is the same.
However, I *think* this also means that the regular Linux64 reftest suite is running without acceleration, and is effectively the same as the reftest-no-accel suite. So we're running twice the reftests for no reason?!
Assignee | ||
Comment 3•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=89c30b5246f40423fec29d76b392390670d7f33f
This try push forces acceleration on for linux64 reftests, and forces it off for linux64 reftest-no-accel. Seems to be pretty green so far, except for a few R8 failures which we can annotate in reftest.list.
Assignee | ||
Comment 4•8 years ago
|
||
So yeah, it's just a few reftests that need their fuzzy-if statements expanded a little. We should force acceleration on for the linux64 regular reftest suite, so that we're not just burning machine time uselessly. This will also allow us to land the patch in bug 1387764, or something similar to that anyway.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → bugmail
Comment 7•8 years ago
|
||
mozreview-review |
Comment on attachment 8895447 [details]
Bug 1388726 - Force-enable acceleration in the regular linux reftest suite.
https://reviewboard.mozilla.org/r/166646/#review171764
Attachment #8895447 -
Flags: review?(jmuizelaar) → review+
Comment 8•8 years ago
|
||
mozreview-review |
Comment on attachment 8895446 [details]
Bug 1388726 - Fix the pref used to disable acceleration in linux reftest-unaccelerated runs.
https://reviewboard.mozilla.org/r/166644/#review171770
Attachment #8895446 -
Flags: review?(jmaher) → review+
Comment 9•8 years ago
|
||
mozreview-review |
Comment on attachment 8895447 [details]
Bug 1388726 - Force-enable acceleration in the regular linux reftest suite.
https://reviewboard.mozilla.org/r/166646/#review171774
Attachment #8895447 -
Flags: review?(jmaher) → review+
Assignee | ||
Comment 10•8 years ago
|
||
Another small push to confirm, looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f063c33a9eba5653a833d4c8cc492114e1314758
Comment 11•8 years ago
|
||
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5f9f6cb40071
Fix the pref used to disable acceleration in linux reftest-unaccelerated runs. r=jmaher
https://hg.mozilla.org/integration/autoland/rev/556c779c67b4
Force-enable acceleration in the regular linux reftest suite. r=jmaher,jrmuizel
Comment 12•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/5f9f6cb40071
https://hg.mozilla.org/mozilla-central/rev/556c779c67b4
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•