Closed Bug 1351548 Opened 6 years ago Closed 6 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.
Flags: needinfo?(shing.lyu)
You need to log in before you can comment on or make changes to this bug.