Reftest harness doesn't report fuzzy passes as unexpected when reftest also has fails-if

RESOLVED WONTFIX

Status

RESOLVED WONTFIX
2 years ago
2 years ago

People

(Reporter: kats, Assigned: kats)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Say you have a reftest like so:

  fuzzy-if(true,10,10) fails-if(true) == test.html ref.html

If test.html is exactly equal to ref.html, this will result in a TEST-UNEXPECTED-PASS, and cause the reftest job to fail. However, if test.html is a fuzzy match for ref.html (within the 10,10 limits), this will result in a TEST-KNOWN-FAIL. I think the fuzzy match scenario should also result in a TEST-UNEXPECTED-PASS, because without the "fails-if" clause, the job would be passing.

In the code, if a fuzzy match happens, the harness runs the line at [1]. Since |expected| is EXPECTED_FAIL, |equal| becomes false. This results in |test_passed| being false at [2]. The harness then looks up outputs[EXPECTED_FAIL][false] at [3], which produces KNOWN-FAIL [4]. (I want it to produce UNEXPECTED-PASS here instead).

If the fails-if were to be removed, the |expected| would be EXPECTED_FUZZY at [1], so |equal| would be true. That would make |test_passed| true at [2], and outputs[EXPECTED_FUZZY][true] would produce PASS.

[1] http://searchfox.org/mozilla-central/rev/4f5f9f3222f35fa4635ea96a0c963c130854ef19/layout/tools/reftest/reftest.jsm#1691
[2] http://searchfox.org/mozilla-central/rev/4f5f9f3222f35fa4635ea96a0c963c130854ef19/layout/tools/reftest/reftest.jsm#1698
[3] http://searchfox.org/mozilla-central/rev/4f5f9f3222f35fa4635ea96a0c963c130854ef19/layout/tools/reftest/reftest.jsm#1700
[4] http://searchfox.org/mozilla-central/rev/4f5f9f3222f35fa4635ea96a0c963c130854ef19/layout/tools/reftest/reftest.jsm#1571
In general, the rule for failure annotations is that the last one that matches wins.  Do we want to make fuzzy-if special?

(See also bug 1252361, which I think is a rather high priority, actually.)
Also see bug 580786 comment 16 and bug 580786 comment 24.

But, on the main topic, comment 0 really does sound like the expected behavior to me, as documented in http://searchfox.org/mozilla-central/rev/4f5f9f3222f35fa4635ea96a0c963c130854ef19/layout/tools/reftest/README.txt#54 (although somehow when we added failure types for "include" that bit of documentation ended up only covering "include").
(In reply to David Baron :dbaron: ⌚️UTC-8 from comment #2)
> In general, the rule for failure annotations is that the last one that
> matches wins.  Do we want to make fuzzy-if special?

Hm, that's fair. I guess in my mind fuzzy[-if] was already special because when it is encountered it sets the fuzzy_max_delta and fuzzy_max_pixels values, which are not reset even if later a fails[-if] is encountered. So in the example in comment 0 it really shouldn't be doing a fuzzy match at all, since the fails-if should override the fuzzy-if.

(In reply to David Baron :dbaron: ⌚️UTC-8 from comment #3)
> But, on the main topic, comment 0 really does sound like the expected
> behavior to me, as documented in
> http://searchfox.org/mozilla-central/rev/
> 4f5f9f3222f35fa4635ea96a0c963c130854ef19/layout/tools/reftest/README.txt#54
> (although somehow when we added failure types for "include" that bit of
> documentation ended up only covering "include").

As defined in that README, yeah I guess this is expected behaviour. If we were to redefine "fails[-if]" as "the opposite of whatever would happen without the fails clause" then comment 0 would make more sense. But it may not be worth doing that.

Closing as WONTFIX. I can just check for these cases manually once in a while on the graphics branch.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.