Reftest harness doesn't report fuzzy passes as unexpected when reftest also has fails-if
Categories
(Testing :: Reftest, defect)
Tracking
(Not tracked)
People
(Reporter: kats, Unassigned)
References
Details
Attachments
(2 files)
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
Reporter | ||
Comment 1•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=84cf3e3c539222956b35ded4d39024204dafacdd
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").
Reporter | ||
Comment 4•7 years ago
|
||
(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.
Comment 5•4 years ago
|
||
With the defaults
feature now landed, I think we'll need to figure out how to solve this. Imagine a manifest that looks like this:
defaults fails-if(webrender)
== foo.html bar.html
fuzzy(0-1,0-10) == foo.html bar2.html
Since the last one that matches wins, we overwrite the fails-if
with the fuzzy
meaning the second test won't have EXPECTED_FAIL on webrender and the job turns orange.
The same issue will happen with skip/skip-if
. See my comments here:
https://bugzilla.mozilla.org/show_bug.cgi?id=1617261#c4
Basically as soon as a developer adds skip/skip-if/fuzzy/fuzzy-if/fails/fails-if
to the defaults section of a manifest, they are going to have surprising behaviour on any test that has its own failure annotation.
I have an idea on how to solve it. Will get some patches up soon.
Comment 6•4 years ago
|
||
Comment 7•4 years ago
|
||
Depends on D64457
Comment 8•4 years ago
|
||
These patches are WIP and not ready for review (though feel free to take a peek if you are interested).
Comment 9•4 years ago
|
||
The fuzzy/fuzzy-if
patch will need a little extra care (and it doesn't block my other work), so I'll likely land the skip
one first and come back to the other once I have time.
Comment 10•4 years ago
|
||
Pushed by ahalberstadt@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/17109740bffa [reftest] Stop treating 'skip/skip-if' as a failure type in the manifests r=kats
Comment 11•4 years ago
|
||
bugherder |
Comment 12•4 years ago
|
||
The leave-open keyword is there and there is no activity for 6 months.
:ahal, maybe it's time to close this bug?
Comment 13•4 years ago
|
||
I still hope to get back to this sometime in Q4 or later, but I haven't actively looked at this in awhile so unassigning for now.
Updated•2 years ago
|
Description
•