Closed Bug 1478472 Opened 6 years ago Closed 5 years ago

[wpt-sync] Sync PR 12187 - Add support for fuzzy matching in reftests.

Categories

(Testing :: web-platform-tests, enhancement, P4)

enhancement

Tracking

(firefox68 fixed)

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: mozilla.org, Unassigned)

References

(Blocks 1 open bug, )

Details

(Whiteboard: [wptsync downstream])

Sync web-platform-tests PR 12187 into mozilla-central (this bug is closed when the sync is complete).

PR: https://github.com/web-platform-tests/wpt/pull/12187
Details from upstream follow.

James Graham <james@hoppipolla.co.uk> wrote:
>  Add support for fuzzy matching in reftests.
>  
>  This allows fuzzy matching in reftests in which a comparison can
>  succeed if the images are different within a specified tolerance. It
>  is useful in the case of antialiasing, and in other scenarios where
>  it's not possible to make an exact match in all cases.
>  
>  Differences between tests are characterised by two values:
>  
>  * The maximum difference for any pixel on any color channel (in the
>    range 0 to 255)
>  
>  * The maximum total number of differing pixels
>  
>  The fuzziness can be supplied in two places, according to whether it's
>  a property of the test or of the implementation:
>  
>  * In the reftest itself, using a <meta name=fuzzy> tag
>  
>  * In the expectation metadata file using a fuzzy: key that takes a
>    list
>  
>  The general format of the fuzziness specifier is
>  
>  range = [digits, "-"], digits
>  fuzziness = [ url, "-" ], range, ";", range
>  
>  The first range represents the maximum difference of any channel per
>  pixel and the second represents the total number of pixel
>  differences. So for example a specifier could be:
>  
>  * "10;300" - meaning a difference of up to 10 per color channel and up to
>  300 pixels different in total (all ranges are inclusive).
>  
>  * "5-10;200-300" - meaning a maximum difference of between 5 and 10
>    per color channel and between 200 and 300 pixels differing in total
>  
>  The definition of url is a little different between the meta element
>  and the expecation metadata. In the first case the url is resolved
>  against the current file, and applies to any reference in the current
>  file with that name. So for example
>  
>  <meta name="fuzzy" content="option-1-ref.html:5;200">
>  
>  would allow a fuzziness of up to 5 on a specific channel and up to 200
>  opixels different for comparisons involving the file containing the
>  meta element and option-1-ref.html.
>  
>  In the case of expectation metadata, the metadata is always associated
>  with the root test, so urls are always resolved relative to that. In
>  the case as above where only a single URL is supplied, any reference
>  document with that URL will have the fuzziness applied for whatever
>  comparisons it's involved in e.g.
>  
>  [test1.html]
>    fuzzy: option-1-ref.html:5;200
>  
>  would apply the fuziness to any comparison involving option-1-ref.html
>  whilst running the set of reftests rooted on test1.html. To specify an
>  exact comparison for the fuzziness, one can also supply a full
>  reference pair e.g.
>  
>  [test1.html]
>    fuzzy: subtest.html==option-1-ref.html:5;200
>  
>  in which case the fuzziness would only apply to "match" comparison
>  involving subtest.html on the lhs and option-1-ref.html on the
>  rhs (both resolved relative to test1.html).
Whiteboard: [wptsync downstream] → [wptsync downstream error]
Whiteboard: [wptsync downstream error] → [wptsync downstream]
PR 12187 applied with additional changes from upstream: e45b651d2b0a4d0b8f239723b8bb9836ad18d9a5, 6146d600fd84c043ee3ce70e59f9abe1b8252d99

When we've got this support, list-style-021.xht is one test that really needs this annotation. Maybe it can be annotated as part of this bug, even, to serve as a sanity-check that the fuzzy functionality is working?

(I've just marked it as disabled in bug 1383451, but it really wants a fuzzy(8,1) annotation, for 1px differing by up to 8 in every color channel, with rgb(255,255,255) != rgb(247,247,247))

James, do you know what is blocking this from landing?

Flags: needinfo?(james)

Hi, James,
Do we have any progress on this bug? Because there are lots of WebVTT wpt tests need this fuzzy feature.
Thank you!

Flags: needinfo?(james)

Is there a more recent try run to look at then the link in comment 5? I'm not sure where the comment 8 link comes from.

There isn't a more recent try run, but I expect this to land this week.

Flags: needinfo?(james)

Sorry to poke on this bug again, just want to know is there any good progress we got for landing this one?
Thank you!

Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/mozilla-inbound/rev/18fb7bd95d12
[wpt PR 12187] - Add support for fuzzy matching in reftests., a=testonly
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

Yes, you can.

Platform selection via the is/expression syntax is a generic feature not tied to any specific field.

For general docs on the fuzziness specification see https://searchfox.org/mozilla-central/source/testing/web-platform/tests/infrastructure/metadata/infrastructure/reftest/reftest_fuzzy.html.ini#2 for an example or the documentation [1], [2].

Bit generally you should be able to do something like:

fuzzy:
if os == "linux": 10;1-20
if os == "mac": 5;10

or with whatever values are appropriate.

No doubt the docs could be improved, so let me know what problems you run into.

[1] https://searchfox.org/mozilla-central/source/testing/web-platform/tests/tools/wptrunner/docs/expectation.rst#203-214
[2] https://web-platform-tests.org/writing-tests/reftests.html#fuzzy-matching

Flags: needinfo?(james)

I think it would be a good idea to post a PSA about this feature in dev.platform.

Hi, James,
I've added the fuzzy keyword on some wpts, but it seems not work at all [1]. Is there any thing I miss?
Thank you.

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=de80f58a1a1c5a927347a77afcb5967ff11d92c0&selectedJob=239776817

Flags: needinfo?(james)

(In reply to Alastor Wu [:alwu] from comment #18)

I've added the fuzzy keyword on some wpts, but it seems not work at all [1]. Is there any thing I miss?

One guess: it looks like in your fuzzy annotations, you're repeating the test name, like here in your try commit:

 --- /dev/null
 +++ b/testing/web-platform/meta/webvtt/rendering/cues-with-video/processing-model/align_center.html.ini
 @@ -0,0 +1,2 @@
 +[align_center.html]
 +  fuzzy: align_center.html:maxDifference=2;totalPixels=5000
           ^^^^^^^^^^^^^^^^^----(I think this is the problem)

I think you're supposed to use the reference name in the fuzzy annotation. (The test name is already present in the brackets, so it'd be redundant to list it again.)

I think (?) the idea is that tests can have multiple references (though they don't often), so to handle that correctly, you have to clarify which reference you're referring to in the fuzzy annotation. Or something like that. You don't have to clarify which testcase you're referring to, because the testcase has already been mentioned.

(Sorry I often miss comments on these bugs because for [reasons] I get bugmail for all PRs we're syncing and so it tends to get filtered away).

Yeah, so if you just don't put any filename then the annotation should get applied to all comparisons in that test. In the typical case that's fine because there's only one comparison. However wpt reftests are complex because tests can have multiple allowed renderings and you can build up chains of references for a single test. So for those complex cases it can be necessary to specify which part of the chain the fuzziness applies to. In that case, if the ref is only used once or the fuzziness applies every time it's used, you can simply name the ref (as a relative path from the test) using the <key>:<fuzzy> syntax you used above. In the most complex case where you want to identify a specific comparison you can use the full <lhs><reftype><rhs>:<fuzzy> form, where <lhs> and <rhs> are the relative paths from the initial test to the files being compared and <reftype> is either == or !=.

The tl;dr summary of all of that is that for your case you probably just want to write maxDifference=2;totalPixels=5000, but dholbert is also correct that for more precision you need to specify the ref not the test.

There is a proposal to reform the way that complex reftests work in wpt which might hekp avoid some of this complexity; see [1] if you would like to give feedback on that.

I also noticed that there was a bug in the code comment explaining this; apologies if that caused the confusion.

[1] https://github.com/web-platform-tests/rfcs/pull/15

Flags: needinfo?(james)
Blocks: 1544661
You need to log in before you can comment on or make changes to this bug.