Closed Bug 1304684 Opened 3 years ago Closed 3 years ago

Every test in w3c-css/submitted/masking/ will fail when Gecko 51 is merged to beta on November 14th

Categories

(Core :: Layout, defect, P1)

defect

Tracking

()

RESOLVED FIXED
Tracking Status
firefox49 --- unaffected
firefox-esr45 --- unaffected
firefox50 --- unaffected
firefox51 --- fixed
firefox52 --- fixed
firefox53 --- unaffected

People

(Reporter: aryx, Assigned: bmo)

References

Details

Attachments

(2 files)

If central gets a simulated uplift to beta https://wiki.mozilla.org/Sheriffing/Uplift-Sim/Trunk-Beta , the following error gets shown in reftests:

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/test/build/tests/reftest/tests/layout/reftests/w3c-css/submitted/masking/mask-composite-1a.html == file:///builds/slave/test/build/tests/reftest/tests/layout/reftests/w3c-css/submitted/masking/mask-composite-1-ref.html | image comparison, max difference: 255, number of differing pixels: 30000

https://treeherder.mozilla.org/logviewer.html#?job_id=27844251&repo=try
Going to be inconvenient for you that you don't have a pref you can set, hope an ifdef around including the reftest manifest works.
Blocks: 1294660
Summary: TEST-UNEXPECTED-FAIL | file:///builds/slave/test/build/tests/reftest/tests/layout/reftests/w3c-css/submitted/masking/mask-composite-1a.html == file:///builds/slave/test/build/tests/reftest/tests/layout/reftests/w3c-css/submitted/masking/mask-composite-1-r → Every test in w3c-css/submitted/masking/ will fail when Gecko 51 is merged to beta on November 7th
Blocks: 1307010
Phil, runtime perf control is definitely a way we should go, however, for shorter term, is there any way to avoid it whiling we are turning FF51 to beta ?
Assignee: nobody → aschen
Status: NEW → ASSIGNED
Depends on: 1301638
Priority: -- → P1
Nothing pretty that I know of, the reftest sandbox doesn't seem to contain anything that varies for release_build. layout/tools/reftest/reftest.jsm is preprocessed, so you could add a condition to it #if RELEASE_BUILD, though I'd expect dbaron will frown at you.

Then there's the problem that doing what you would want to do, making the "include masking/reftest.list" conditional, works in surprising and often unworkable ways. I don't remember whether I tested anything but skip when I discovered bug 1299325, but I suspect that fuzzy-if beats every other condition on an include, so whether you put skip-if or fails-if on the include the tests that are fuzzy-if in that reftest.list will still run and expect to pass with some fuzz, and so you'll have to put the skip/fail on each individual test's manifest line.
Summary: Every test in w3c-css/submitted/masking/ will fail when Gecko 51 is merged to beta on November 7th → Every test in w3c-css/submitted/masking/ will fail when Gecko 51 is merged to beta on November 14th
Duplicate of this bug: 1317339
(In reply to Astley Chen [:astley] (UTC+8) from comment #2)
> Phil, runtime perf control is definitely a way we should go, however, for
> shorter term, is there any way to avoid it whiling we are turning FF51 to
> beta ?

and now we had merge day and the test are failing, Astley is there a way to fix the problem and to allow reopen of the trees
Flags: needinfo?(aschen)
(In reply to Carsten Book [:Tomcat] from comment #5)
> and now we had merge day and the test are failing, Astley is there a way to
> fix the problem and to allow reopen of the trees

Yes, the failure is expected and need the patch in this bug to disable relevant test cases to get over this.
Note that we need to manually do so on FF51 and FF52, from FF53, we will fully enable CSS mask image feature so that we can bail out from this.

cbook, could you suggest what to do next ? looking for RM's help to approve the uplift request ?
Flags: needinfo?(aschen)
so we still need to land this in aurora (and trunk) ?
Flags: needinfo?(aschen)
(In reply to Carsten Book [:Tomcat] from comment #9)
> so we still need to land this in aurora (and trunk) ?

No need. CSS masking feature was enabled on nightly & aurora in bug 1294660. Therefore, relevant test cases will get pass on nightly and aurora. In bug 1251161, we will ship to all channels from 53, that is, we no longer run into this problem(fail on beta) after then.
I'm sorry for the inconvenience I made here.
Flags: needinfo?(aschen)
Hey Astley i have a new question, aurora as beta uplift simulations show test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=65820628&repo=try is this expected like covered in this bug or this need new bugs ?
Flags: needinfo?(aschen)
(In reply to Carsten Book [:Tomcat] from comment #11)
> this expected like covered in this bug or this need new bugs ?

Yes, it's expected and will be the last time happens on merge since we've formally enabled the feature from Firefox 53. Again, thanks for your help.
Flags: needinfo?(aschen)
Tomcat, it seems I need to update the patch accordingly to resolve a few conflicts when applying to current aurora 52. I'll upload the patch as soon as I ensure the try is green.
updated patch to disable mask shorthand reftest for beta 52.
(In reply to Astley Chen [:astley] UTC+8 from comment #14)
> Created attachment 8823664 [details] [diff] [review]
> disable mask shorthand reftest on beta 52
> 
> updated patch to disable mask shorthand reftest for beta 52.

cool, yeah i guess we need to land the patch in aurora then
ok had to back this out for unexpected passes like https://treeherder.mozilla.org/logviewer.html#?job_id=67240756&repo=mozilla-aurora
Flags: needinfo?(aschen)
Tomcat, the patch needs to be applied on the build with variable $RELEASE_OR_BETA set to true.
We need to get similar thing done just like comment 7 and comment 8, and fix those failures by using the updated patch I submitted here. Let me know what help I can do. Thanks.
Flags: needinfo?(aschen)
i guess we could now land this on beta since we have merge day. Astly, Wes what do you think, this should fix test failures then i think
Flags: needinfo?(wkocher)
Flags: needinfo?(aschen)
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Duplicate of this bug: 1333150
Duplicate of this bug: 1333152
Ryan, Tomcat and Wes, appreciate for your help on disabling those failure tests on beta and apologize for any inconvenience we caused here.
You need to log in before you can comment on or make changes to this bug.