Closed Bug 1351548 Opened 3 years ago Closed 3 years ago

Merge reftest-stylo.list back to reftest.list

Categories

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

x86_64
Linux
enhancement

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: shinglyu, Assigned: shinglyu)

References

(Blocks 1 open bug)

Details

(Whiteboard: [Stylo])

Attachments

(3 files)

Bug 1344132 enable us to run stylo-to-gecko test using reftest.list. But it is not enabled on the try server. So we need to mark stylo-specific fails inside reftest.list, remove all the reftest-stylo.list and change the try configuration to run reftest.list instead of reftest-stylo.list.
Blocks: stylo-tooling
No longer blocks: stylo-reftest
Summary: stylo: Merge reftest-stylo.list back to reftest.list → Merge reftest-stylo.list back to reftest.list
Whiteboard: [Stylo]
Priority: -- → P3
No longer blocks: 1356988
Status: NEW → ASSIGNED
The reftest-stylo.list thing is creating too much confusion, so I guess it's time to do this.

These patches depends on Bug 1344132. I'll land the two together when they both received r+.

There are some rules I use to prepare the patch:

* Things that clearly fail on stylo-vs-gecko test are marked as "fails-if(stylo)"
* Stylo-specific crashes or intermittents are marked as "skip-if(stylo)", we'll try them periodically to see if they are fixed or not.
* Things that are marked as "fails" or "fails-if(foo)" but is passing in stylo-vs-gecko test will be changed to "fails-if(!stylo)" and "fails-if(foo&&!stylo)"
* Some tests (e.g. reftest-sanity) uses the same test file against different ref files. We only test the first one and mark the rest as "skip-if(stylo)"
* We put the "skip-if(stylo)" and "fails-if(stylo)" at the end of the list, so it will override other conditions
* Tests in the form "foo.html#id" will break the test because it only scrolls but not reload. This is tricker to fix so I'll create a follow-up bug for that, there are only ~6 of them in the whole reftest, so we won't lose a lot of test coverage
Comment on attachment 8859004 [details]
Bug 1351548 - Add stylo-vs-gecko expectations to reftest.lists

Hi Bobby,

I usually ask heycam for the reftest stuff, but he is on PTO so would you mind review it for me?
Attachment #8859004 - Flags: review?(bobbyholley)
Attachment #8859005 - Flags: review?(bobbyholley)
Attachment #8859006 - Flags: review?(bobbyholley)
Comment on attachment 8859004 [details]
Bug 1351548 - Add stylo-vs-gecko expectations to reftest.lists

https://reviewboard.mozilla.org/r/131024/#review133998

::: layout/reftests/font-face/reftest.list:72
(Diff revision 1)
>  HTTP(..) == unicoderange-4.html unicoderange-4-ref.html
>  
>  # Dynamic changes
>  # we need to skip these because of the bug that's causing order-2.html to fail
> -HTTP(..) == enable-sheet-1.html enable-sheet-1-ref.html
> -skip HTTP(..) == enable-sheet-2.html multiple-in-family-1-ref.html
> +skip-if(stylo) HTTP(..) == enable-sheet-1.html enable-sheet-1-ref.html
> +skip skip-if(stylo) HTTP(..) == enable-sheet-2.html multiple-in-family-1-ref.html

This is kind of redundant I suppose, but we can fix it up later.
Attachment #8859004 - Flags: review?(bobbyholley) → review+
Comment on attachment 8859005 [details]
Bug 1351548 - Switch from reftest-stylo.list to reftest.list on

https://reviewboard.mozilla.org/r/131026/#review134004
Attachment #8859005 - Flags: review?(bobbyholley) → review+
Comment on attachment 8859006 [details]
Bug 1351548 - Remove reftest-stylo.lists

https://reviewboard.mozilla.org/r/131028/#review134006
Attachment #8859006 - Flags: review?(bobbyholley) → review+
Nice! I didn't review every line of the diffs but this seems to be doing the right thing, and CI will give us the canonical results anyway. Let's get this landed - it's one step closer to victory!
Depends on: 1344132
Pushed by slyu@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1f9d0f8e65b9
Add stylo-vs-gecko expectations to reftest.lists.  r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/4d8dfee7ebf3
Switch from reftest-stylo.list to reftest.list on linux64-stylo. r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/aebb0ceeb20a
Remove reftest-stylo.lists. r=bholley
Pushed by slyu@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/46fc8d459483
Add stylo-vs-gecko expectations to reftest.lists.  r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/3940a4760c84
Switch from reftest-stylo.list to reftest.list on linux64-stylo. r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1ceacb7312d
Remove reftest-stylo.lists. r=bholley
Pushed by slyu@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/107d824d3657
Update stylo reftest expectations. r=me
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/1c6fa11d1fed
Follow-up to fix incorrect changes in cset 46fc8d459483. r=me a=Tomcat
FYI ^ there were some errors when merging the reftest.list files. Two of the four errors were showing up as R4 failures on m-c because of TEST-UNEXPECTED-PASS. The other two were fuzzy passes so they are green regardless of whether they are marked fails-if or not (we really should fix that...). You might want to recheck that cset to make sure there weren't other errors, I only looked for webrender mismatches.
Depends on: 1360791
This accidentally re-enabled the test disabled in bug 1351548. Please check that no other changes to test expectations got reverted by accident.
This bug is 1351548.
Thank you. The test that got re-enabled is bug 1352942.
Flags: needinfo?(shing.lyu)
You need to log in before you can comment on or make changes to this bug.