Mark list-style-021.xht as fuzzy rather than disabled

RESOLVED FIXED in Firefox 68

Status

()

task
P3
normal
RESOLVED FIXED
2 months ago
2 months ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

Trunk
mozilla68
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox68 fixed)

Details

Attachments

(2 attachments)

Assignee

Description

2 months ago

The Web Platform Test list-style-021.xht has a single occasionally fuzzy pixel (visually imperceptible), as discussed in bug 1383451 comment 81 and bug 1383451 comment 89.

We disabled the test in that bug, because we had no other way at the time of stemming the intermittent-failure-spam.

However, now we have support for fuzzy annotations for WPT tests (bug 1478472) so we should convert the disabled annotation to a fuzzy annotation.

Assignee

Updated

2 months ago
Flags: needinfo?(dholbert)
Assignee

Updated

2 months ago
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Flags: needinfo?(dholbert)
Assignee

Comment 2

2 months ago

The attached one-line patch doesn't actually work yet, though I don't understand why.

It causes us to report a test failure, with the reftest screenshots for the two files being identical. (If I remove the "fuzzy" line entirely, then the test is reported as passing.) I'm probably misunderstanding something about the syntax, but I'm not sure what.

Flags: needinfo?(james)
Assignee

Comment 4

2 months ago

Relevant snippet from the log:

INFO Allowed 0-1 pixels different, maximum difference per channel 8-8
INFO Found 0 pixels different, maximum difference per channel 0
TEST_END: FAIL, expected PASS - Testing http://web-platform.test:8000/css/CSS2/lists/list-style-021.xht == http://web-platform.test:8000/css/CSS2/lists/list-style-021-ref.xht

TEST-UNEXPECTED-FAIL | /css/CSS2/lists/list-style-021.xht | Testing http://web-platform.test:8000/css/CSS2/lists/list-style-021.xht == http://web-platform.test:8000/css/CSS2/lists/list-style-021-ref.xht
REFTEST IMAGE 1 (TEST): data:image/png;[...]
REFTEST IMAGE 2 (REFERENCE): data:image/png;[...same data URI as above...]

Right now my fuzzy annotation has maxDifference=8;totalPixels=0-1. If I change it to have maxDifference=0-8, then the test starts being reported as passing. James, should I make that change? (i.e. should I use a 0-8 range for maxDifference)

I might be misremembering, but I think the "regular" reftest harness would treat my current annotation (without 0 included in the maxDifference range) as meaning: "1 pixel is allowed to differ, but it's also fine if it doesn't differ. If it does differ, then you should expect its maxDifference to be exactly 8". Apparently that is not how WPT is interpreting my annotation though, because it treats no-pixels-differing as a failure, despite the totalPixels=0-1 in my syntax.

Yeah, at the moment there isn't any special logic to handle 0 pixels different as meaning the maxDifference can be ignored. Maybe there should be.

Flags: needinfo?(james)
Assignee

Comment 6

2 months ago

Can you clarify what syntax I should be using here, then? (Should I use ranges that include 0 for both values? That seems to work, but it feels quite verbose. Is there a more compact way of expressing the condition "sometimes 1 pixel differs, with a difference of 8"?)

Flags: needinfo?(james)

No, you currently have to use both ranges. I can look at special casing where total pixels is allowed to be 0 to allow any range for maxDifference (I thought what I had matched gecko reftests post dbaron's recent changes but I guess not).

Flags: needinfo?(james)
Assignee

Comment 8

2 months ago

(In reply to James Graham [:jgraham] from comment #7)

No, you currently have to use both ranges.

OK, thanks.

I can look at special casing where total pixels is allowed to be 0 to allow any range for maxDifference (I thought what I had matched gecko reftests post dbaron's recent changes but I guess not).

Intuitively that makes sense to me; otherwise 0 isn't a useful value to include in a pixel value range, really. (unless you specify both ranges as I'll be doing here)

Basically, if 0 is in either range, it intuitively seems like an exact match should be allowed. Having said that, I haven't used the current incarnation of gecko fuzzy annotations very much, so it's possible that your implementation does match the gecko reftest harness's semantics and that those just don't make sense to me. :)

Assignee

Comment 9

2 months ago

(In reply to Daniel Holbert [:dholbert] from comment #8)

Basically, if 0 is in either range, it intuitively seems like an exact match should be allowed.

Aha, I see you've now implemented that in bug 1542329. Thanks!

Assignee

Comment 10

2 months ago

(ab)using needinfo just in case you missed the review-requested-phabmail -- see https://phabricator.services.mozilla.com/D26242 . Thanks!

Flags: needinfo?(james)

I think I had processed the review mail when it wasn't actionable and missed the followup, or something. Anyway reviewed now.

Flags: needinfo?(james)

Comment 12

2 months ago
Pushed by dholbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/642d92a8a58e
Mark list-style-021.xht as fuzzy rather than disabled. r=jgraham
Assignee

Comment 13

2 months ago

Thanks! Yeah, I think I recall that the "requested --> not-requested --> requested-again" phabricator-mail flow isn't sufficiently clear about the final next-action state. :)

Anyway: landed now.

Type: defect → task

Comment 14

2 months ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.