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

Status: RESOLVED FIXED

(Core :: Layout, defect, P1)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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

Comment 1

[Phil Ringnalda (:philor)]

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.

Comment 2

[Astley Chen (inactive)]

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 ?

Comment 3

[Phil Ringnalda (:philor)]

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.

Comment 5

[Carsten Book [:Tomcat]]

(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

Comment 6

[Astley Chen (inactive)]

Attachment: Bug 1304684 - disable mask shorthand reftest on beta.

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

Comment 7

[Astley Chen (inactive)]

(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 ?

Comment 8

[Carsten Book [:Tomcat]]

landed as test-only in   https://hg.mozilla.org/releases/mozilla-beta/rev/fd3c6c15ec64591b145ab1b6dd71c5a2e7b8d9b1

Comment 9

[Carsten Book [:Tomcat]]

so we still need to land this in aurora (and trunk) ?

Comment 10

[Astley Chen (inactive)]

(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.

Comment 11

[Carsten Book [:Tomcat]]

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 ?

Comment 12

[Astley Chen (inactive)]

(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.

Comment 13

[Astley Chen (inactive)]

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.

Comment 14

[Astley Chen (inactive)]

Attachment: disable mask shorthand reftest on beta 52

updated patch to disable mask shorthand reftest for beta 52.

Comment 15

[Carsten Book [:Tomcat]]

(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

Comment 16

[Carsten Book [:Tomcat]]

landed as https://hg.mozilla.org/releases/mozilla-aurora/rev/2e25a162cd9893c3f9fa168b9b00f6f3e260abe3

Comment 17

[Carsten Book [:Tomcat]]

ok had to back this out for unexpected passes like https://treeherder.mozilla.org/logviewer.html#?job_id=67240756&repo=mozilla-aurora

Comment 18

[Astley Chen (inactive)]

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.

Comment 19

[Carsten Book [:Tomcat]]

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

Comment 20

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/c471ca9cfe84ee531d73ff74c34961e20575d327

Comment 21

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

There was another failing test, skipped in https://hg.mozilla.org/releases/mozilla-beta/rev/cc25e331401ce3404522a9c8fe5b8c54c068bf09

Comment 22

[Ryan VanderMeulen [:RyanVM]]

Finally out of the woods frlz this time.
https://hg.mozilla.org/releases/mozilla-beta/rev/d0ba70f3a3b08e4731a18fb35625421b40b11d69

Comment 25

[Astley Chen (inactive)]

Ryan, Tomcat and Wes, appreciate for your help on disabling those failure tests on beta and apologize for any inconvenience we caused here.