1396411 - stylo: 1394311.htm is permafailing on OSX reftest-stylo runs

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P3)

Description

[Ryan VanderMeulen [:RyanVM]]

Tier 2 job, so not requesting tracking status at this point. However, it would be good to understand why this job is failing consistently in a Beta build configuration, and only OSX.

The screenshots show differences in sizing of the top row between the test and the reference.
https://treeherder.mozilla.org/logviewer.html#?job_id=128066445&repo=try

REFTEST TEST-UNEXPECTED-FAIL | file:///Users/cltbld/tasks/task_1504399869/build/tests/reftest/tests/layout/reftests/font-matching/1394311.htm == file:///Users/cltbld/tasks/task_1504399869/build/tests/reftest/tests/layout/reftests/font-matching/1394311.htm | image comparison, max difference: 255, number of differing pixels: 8828
REFTEST TEST-UNEXPECTED-FAIL | file:///Users/cltbld/tasks/task_1504399869/build/tests/reftest/tests/layout/reftests/font-matching/1394311.htm == file:///Users/cltbld/tasks/task_1504399869/build/tests/reftest/tests/layout/reftests/font-matching/1394311.htm | image comparison, max difference: 255, number of differing pixels: 9018

The patch below should be sufficient for reproducing the specific build configuration on Try.
https://hg.mozilla.org/try/rev/c565af5d87b45f4b67d7a5f0b6b21d9ebe01908b

Comment 1

[Kevin Hsieh]

Seems related to Bug 1391341, but this is not quite the same because it doesn't reproduce on 57. I'll run mozregression.

Comment 2

[Ryan VanderMeulen [:RyanVM]]

You won't be able to mozregression this because it requires builds in a custom configuration (57 built as if it were merged to Beta where we have different build options)

Comment 3

[Kevin Hsieh]

I re-applied the patches and pushed to Try but I don't seem to be able to reproduce this
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5f6faec85178486290083eebba9247745cd8df4c&selectedJob=128146854

Comment 4

[Ryan VanderMeulen [:RyanVM]]

You're not running reftest-stylo - tc-R-e10s(Rs5). I've gone ahead and added them to your push.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

*sigh* or not because Taskcluster hates me. Re-push with "-u reftest-stylo" in your Try syntax and things should work.

Comment 6

[Kevin Hsieh]

Nevermind. I looked at the original test again and realized that it's a Gecko vs. Stylo failure that happens specifically on the zh lines, and I'm able to reproduce it on currently Nightly. So it is Bug 1391341 after all.

Comment 7

[Kevin Hsieh]

In Bug 1394762, I used a bogus font-family in 1394311.htm to avoid Bug 1367860 so that we would be able to uplift the testcase to beta without uplifting the patch from Bug 1367860. In this case, we could do something similar and modify the testcase to avoid Bug 1391341...or we could fix the bug.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

FWIW, I tried the current patch from bug 1391341 on a run today and it basically broke all the OSX reftest-stylo tests :(
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6c068cec057faf80691dad3ef86955597e6d9598&filter-resultStatus=success&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=running&filter-resultStatus=pending&filter-resultStatus=runnable&filter-searchStr=reftest-stylo&group_state=expanded

Comment 9

[Chris Peterson [:cpeterson]]

Kevin, are you looking into this bug? Or should a Stylo engineer take a look?

Comment 10

[Kevin Hsieh]

The conclusion in Bug 1391341 was to leave stylo's inheritance behavior as-is. I've filed a separate bug for an alternate solution in this basic case where font-size isn't specified.

Comment 11

[Chris Peterson [:cpeterson]]

(In reply to Kevin Hsieh from comment #10)
> The conclusion in Bug 1391341 was to leave stylo's inheritance behavior
> as-is.

If we're not going to change Stylo's behavior, do we need to annotate the layout/reftests/font-matching/1394311.htm test so it won't fail on Beta?

Comment 12

[Kevin Hsieh]

If Bug 1396731 is resolved, then the test should no longer fail (i.e., it should render the same on Gecko and Stylo), even though Stylo's internal inheritance behavior remains different. Or do you mean we should be concerned about what happens in the current beta (56)?

Comment 13

[Chris Peterson [:cpeterson]]

(In reply to Kevin Hsieh from comment #12)
> If Bug 1396731 is resolved, then the test should no longer fail (i.e., it
> should render the same on Gecko and Stylo), even though Stylo's internal
> inheritance behavior remains different. Or do you mean we should be
> concerned about what happens in the current beta (56)?

I just meant in the future Beta 57. :)

Comment 14

[Ryan VanderMeulen [:RyanVM]]

This isn't actually a merge day permafail, this is just an issue we're only seeing on Try as it's the only place this suite is currently running (I've verified that regular m-c tip with no special config changes hits the same failures).

Comment 15

[Kevin Hsieh]

Now that Bug 1396731 has landed, this issue should be resolved. Are you able to verify this?

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Actually, no, it doesn't look like I can. Those jobs are no longer available on Try for anything other than Linux64 :(

Comment 17

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Should we consider reviving reftest-stylo for other platforms for some time frame, given this macOS specific issue?  Or should we just hope there aren't any others and call this done?

Comment 18

[Bobby Holley (:bholley)]

I think we can just call it done at this point. We should have bugs on file for any issues that appeared in styloVsGecko, and we can prioritize them appropriately.

Comment 19

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Let's call this "probably fixed" then, since we had a believed fix and the relevant test combo is no longer running.