tweak flexbox reftest manifest to use new "default-preferences" feature

RESOLVED FIXED in mozilla20

Status

()

defect
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

Trunk
mozilla20
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Assignee

Description

7 years ago
heycam recently added a "default-preferences" statement that can go in a reftest manifest, to specify a pref to be toggled for the remaining tests in that directory.

The flexbox reftest manifest
  https://mxr.mozilla.org/mozilla-central/source/layout/reftests/flexbox/reftest.list
would benefit from that feature. Filing this bug on taking advantage of it there.
Assignee

Updated

7 years ago
Assignee: nobody → dholbert
Assignee

Comment 1

7 years ago
Posted patch fixSplinter Review
Attachment #686376 - Flags: review?(cam)
Comment on attachment 686376 [details] [diff] [review]
fix

Review of attachment 686376 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, just one nit.  r=me with that.

::: layout/reftests/flexbox/reftest.list
@@ +6,5 @@
>  
> +# Enable pref for remaining tests
> +# (Most tests only need it in the testcase, but a few use it in the
> +# reference case, so we'll just enable it using "pref()" to make
> +# it available for both.

Needs a closing paren.
Attachment #686376 - Flags: review?(cam) → review+
Assignee

Comment 3

7 years ago
Thanks! Fixed and pushed: https://hg.mozilla.org/integration/mozilla-inbound/rev/82837149f001
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/82837149f001
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Sorry, backed this out on suspicion of causing intermittent Linux flexbox reftest failures, eg:

REFTEST TEST-UNEXPECTED-FAIL | file:///home/cltbld/talos-slave/test/build/reftest/tests/layout/reftests/flexbox/flexbox-basic-textarea-vert-2.xhtml | image comparison (==), max difference: 1, number of differing pixels: 2

REFTEST TEST-UNEXPECTED-FAIL | file:///home/cltbld/talos-slave/test/build/reftest/tests/layout/reftests/flexbox/flexbox-basic-fieldset-vert-2.xhtml | image comparison (==), max difference: 1, number of differing pixels: 1

https://tbpl.mozilla.org/php/getParsedLog.php?id=17450843&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=17445650&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=17445548&tree=Mozilla-Inbound

More retriggers on way:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&jobname=Fedora%2012%20&tochange=b86fba8cc592&fromchange=602add2d02e6
...but I'm pretty sure this bug is the cause.

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5369e76e0ffb
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee

Comment 7

7 years ago
This bug's patch just controls how the flexbox pref is enabled for flexbox reftests. If it broke stuff, it'd be in a "flexbox not enabled correctly --> test renders completely wrong" sort of mode, not in a "max difference: 1, number of differing pixels: 1" kind of way...

So I don't _think_ thinks could've caused comment 6, and it's possible that whatever did cause it is still checked-in... (Still, it's strange that this is specifically failing in flexbox tests.)

FWIW, the test-failure looks like a 1-2px difference just inside the lower-left corner of the border, with a color-channel difference of "1".  If this ends up being mysterious, we could just mark this with a sufficiently-specific "fuzzy-if" annotation.
Assignee

Comment 10

7 years ago
Mysterious as it is, this bug's change did seem to be responsible for comment 6.  The test-failures stopped after it was backed out, and two Try pushes provided additional evidence:

Try push with this bug's patch *applied*:
  https://tbpl.mozilla.org/?tree=Try&rev=1ebebced152b
That had 7 instances of comment 6's test-failure, out of 40+ reftest runs.

Try push with this bug's patch *backed out*:
  https://tbpl.mozilla.org/?tree=Try&rev=16de4a994590
That had no oranges at all, out of 40+ reftest runs.
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Target Milestone: mozilla20 → ---
Assignee

Comment 11

7 years ago
Sorry, didn't mean to re-resolve this bug... not sure how that happened. I blame bugzilla & session-restore resurrecting this tab in an odd state.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
That's super strange.  I can't think how the use of default-preferences could cause random orange like this.
Assignee

Comment 13

7 years ago
In each case, the difference is 1 or 2 pixels along the diagonal between the left-border and bottom border of a textarea or a fieldset.  These borders are "sunken"/beveled, and in some cases, there's a little bit of purple (the background color) showing through on these pixels, in both the testcase & the reference case, e.g. in this log from comment 8:
 https://tbpl.mozilla.org/php/getParsedLog.php?id=17450843&tree=Mozilla-Inbound

This seems like an issue with drawing these beveled borders reliably, and I think we could work around it by just specifying e.g. 'border: 1px solid gray" on these text areas & fieldsets.  

(We could also use fuzzy-if, but I'd rather avoid that if there's an easy test-fix, which I think there is in this case.)
Assignee

Comment 14

7 years ago
I re-landed this with a helper-patch that just adds a simple 1px border to fieldsets & textareas in flexbox reftests, as I suggested in comment 13.

helper patch:       https://hg.mozilla.org/integration/mozilla-inbound/rev/95e6c6b6e939
default-pref patch: https://hg.mozilla.org/integration/mozilla-inbound/rev/c5f98e718546
https://hg.mozilla.org/mozilla-central/rev/95e6c6b6e939
https://hg.mozilla.org/mozilla-central/rev/c5f98e718546
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
You need to log in before you can comment on or make changes to this bug.