Closed
Bug 1394603
Opened 7 years ago
Closed 7 years ago
Clean up fails-if(!styloVsGecko) reftests by resetting to fails
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox55 | --- | unaffected |
firefox56 | --- | wontfix |
firefox57 | --- | fixed |
People
(Reporter: jryans, Assigned: cpeterson)
References
(Blocks 1 open bug)
Details
(Whiteboard: [Stylo])
Attachments
(3 files)
`fails-if(!styloVsGecko)` effectively means the reftest fails on all style systems, but S vs. G passes since they both fail the same way. We should reset these annotations to `fails` and changes S vs. G mode to ignore them.
Assignee | ||
Updated•7 years ago
|
Blocks: stylo-tooling
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → cpeterson
status-firefox55:
--- → unaffected
status-firefox56:
--- → affected
status-firefox57:
--- → affected
status-firefox-esr52:
--- → unaffected
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 4•7 years ago
|
||
Here is a Try run that is mostly green, except for a bug found in my change to layout/reftests/scoped-style/reftest.list that I fixed and commented before submitting my review request here. https://treeherder.mozilla.org/#/jobs?repo=try&revision=216049533799e309bbc159e3576dc219bdc711e0
Reporter | ||
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8904157 [details] Bug 1394603 - Replace fails-if(!styloVsGecko) to fails. https://reviewboard.mozilla.org/r/175932/#review181338 Thanks for working on this, looks great overall! :) See my notes about potential tweaks to the bug annotations. ::: dom/tests/reftest/xml-stylesheet/reftest.list:6 (Diff revision 1) > == css_relative_href.xml pass.svg > HTTP == css_relative_href_also_external.xml pass.svg > HTTP == css_relative_href_also_external_override.xml pass.svg > == embedded_dtd_id.svg pass.svg > != error_no_href.svg fail.svg > -fails-if(!styloVsGecko) == lreas_selflink_dtd_id.svg pass.svg > +fails == lreas_selflink_dtd_id.svg pass.svg # bug 631575 I believe in most cases, an annotation like "bug NNN" is expected to mean an open bug on file to fix the problem, but this bug is a resolved bug that had marked this test as failing. Annotating the bug that marked something as failing less useful (since it can be recovered from VCS history), but if you'd like to keep it since you did the work to look it up, maybe expand the annotation to "Fails after bug NNN"? ::: layout/reftests/border-radius/reftest.list:40 (Diff revision 1) > # Test that radii too long are reduced > == border-reduce-height.html border-reduce-height-ref.html > skip-if(!webrender) pref(layers.advanced.border-layers,1) == border-reduce-height.html border-reduce-height-ref.html > > # Tests for border clipping > -fails-if(!styloVsGecko) == clipping-1.html clipping-1-ref.html # background color should completely fill box; bug 466572 > +fails == clipping-1.html clipping-1-ref.html # background color should completely fill box; bug 466572 and bug 458131 Another bug annotation we may want to clarify. ::: layout/reftests/css-charset/reftest.list:6 (Diff revision 1) > == test-attribute.html pass.html > == test-charset-quotes.html pass.html > == test-charset-leading-space.html pass.html > == test-charset-trailing-space.html pass.html > == test-charset-utf-16-le-no-bom.html pass.html > -fails-if(!styloVsGecko) == test-charset-utf-16-le-bom.html pass.html > +fails == test-charset-utf-16-le-bom.html pass.html # bug 462458 Another bug annotation we may want to clarify. ::: layout/reftests/css-charset/reftest.list:9 (Diff revision 1) > == test-charset-trailing-space.html pass.html > == test-charset-utf-16-le-no-bom.html pass.html > -fails-if(!styloVsGecko) == test-charset-utf-16-le-bom.html pass.html > +fails == test-charset-utf-16-le-bom.html pass.html # bug 462458 > == test-charset-utf-16-bom-le.html pass.html > == test-charset-utf-16-be-no-bom.html pass.html > -fails-if(!styloVsGecko) == test-charset-utf-16-be-bom.html pass.html > +fails == test-charset-utf-16-be-bom.html pass.html # bug 462458 Another bug annotation we may want to clarify. ::: layout/reftests/css-grid/reftest.list:3 (Diff revision 1) > default-preferences pref(layout.css.grid.enabled,true) > > -fails-if(!styloVsGecko) == grid-whitespace-handling-1a.xhtml grid-whitespace-handling-1-ref.xhtml > +fails == grid-whitespace-handling-1a.xhtml grid-whitespace-handling-1-ref.xhtml # bug 1000376 Another bug annotation we may want to clarify. ::: layout/reftests/view-source/reftest.list:1 (Diff revision 1) > -fails-if(!styloVsGecko) random-if(styloVsGecko) == view-source-image.html view-source-image-ref.html # security checks prevent loading view-source: > +fails == view-source-image.html view-source-image-ref.html # bug 989688: security checks prevent loading view-source: Another bug annotation we may want to clarify.
Attachment #8904157 -
Flags: review?(jryans) → review+
Reporter | ||
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8904158 [details] Bug 1394603 - Replace random-if(styloVsGecko) with skip-if. https://reviewboard.mozilla.org/r/175934/#review181388 It would be nice to know why they are random... :emilio suggested floating point differences between engines as one possibility. In any case, I believe this makes enough sense. We'll still have the skip-ifs in case we want to investigate further.
Attachment #8904158 -
Flags: review?(jryans) → review+
Reporter | ||
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8904159 [details] Bug 1394603 - Infer asserts-if(styloVsGecko, X*2) from asserts(X). https://reviewboard.mozilla.org/r/175936/#review181390 Thanks, this seems reasonable to me!
Attachment #8904159 -
Flags: review?(jryans) → review+
Assignee | ||
Comment 8•7 years ago
|
||
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #5) > Annotating the bug that marked something as failing less useful (since it > can be recovered from VCS history), but if you'd like to keep it since you > did the work to look it up, maybe expand the annotation to "Fails after bug > NNN"? OK. I removed some of my new bug comments that point to ancient bugs and added "fails after bug NNN" for references to recent Stylo bugs. (In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #6) > It would be nice to know why they are random... :emilio suggested floating > point differences between engines as one possibility. OK. I added comments explaining why we skip on these for styloVsGecko.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 12•7 years ago
|
||
I made the recommended changes, fixed a merge conflict, and ran another Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f31941d18867d2430e4f247d0730ff8fe9696886
Comment 13•7 years ago
|
||
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s 7957bf375853 -d 4f8769251cec: rebasing 418198:7957bf375853 "Bug 1394603 - Replace fails-if(!styloVsGecko) to fails. r=jryans" merging layout/reftests/forms/progress/reftest.list rebasing 418199:86b0afe1fbbc "Bug 1394603 - Replace random-if(styloVsGecko) with skip-if. r=jryans" merging layout/reftests/forms/progress/reftest.list warning: conflicts while merging layout/reftests/forms/progress/reftest.list! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)
Comment 14•7 years ago
|
||
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s 7957bf375853 -d 867b17852af8: rebasing 418233:7957bf375853 "Bug 1394603 - Replace fails-if(!styloVsGecko) to fails. r=jryans" merging layout/reftests/forms/progress/reftest.list merging layout/reftests/writing-mode/reftest.list rebasing 418234:86b0afe1fbbc "Bug 1394603 - Replace random-if(styloVsGecko) with skip-if. r=jryans" merging layout/reftests/forms/progress/reftest.list warning: conflicts while merging layout/reftests/forms/progress/reftest.list! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 18•7 years ago
|
||
Pushed by cpeterson@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/20551d68f602 Replace fails-if(!styloVsGecko) to fails. r=jryans https://hg.mozilla.org/integration/autoland/rev/31088c59d895 Replace random-if(styloVsGecko) with skip-if. r=jryans https://hg.mozilla.org/integration/autoland/rev/44117208f321 Infer asserts-if(styloVsGecko, X*2) from asserts(X). r=jryans
Comment 19•7 years ago
|
||
Backed out in https://hg.mozilla.org/integration/autoland/rev/634cb312d322 for https://treeherder.mozilla.org/logviewer.html#?job_id=129144150&repo=autoland
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 23•7 years ago
|
||
Pushed by cpeterson@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/fcf98211c151 Replace fails-if(!styloVsGecko) to fails. r=jryans https://hg.mozilla.org/integration/autoland/rev/ffbae2a9f132 Replace random-if(styloVsGecko) with skip-if. r=jryans https://hg.mozilla.org/integration/autoland/rev/026c2d5a3ee8 Infer asserts-if(styloVsGecko, X*2) from asserts(X). r=jryans
Comment 24•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/fcf98211c151 https://hg.mozilla.org/mozilla-central/rev/ffbae2a9f132 https://hg.mozilla.org/mozilla-central/rev/026c2d5a3ee8
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment 25•7 years ago
|
||
Too late for 56. Mass won't fix for 56.
You need to log in
before you can comment on or make changes to this bug.
Description
•