Bug 153951

Summary: "ref test hashes didn't match but diff passed" is useless, should not be logged
Product: WebKit Reporter: Darin Adler <darin>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: NEW    
Severity: Normal CC: ap, lforschler
Priority: P2    
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=142632
Attachments:
Description Flags
disable ImageDiff tolerance none

Darin Adler
Reported 2016-02-06 11:02:47 PST
When I run tests, I see output like this: compositing/hidpi-compositing-vs-non-compositing-check-on-testing-framework.html -> ref test hashes didn't match but diff passed compositing/masks/compositing-clip-path.html -> ref test hashes didn't match but diff passed compositing/overlap-blending/children-opacity-huge.html -> ref test hashes didn't match but diff passed compositing/overlap-blending/children-opacity-no-overlap.html -> ref test hashes didn't match but diff passed compositing/overlap-blending/nested-non-overlap-clipping.html -> ref test hashes didn't match but diff passed compositing/transitions/transform-on-large-layer.html -> ref test hashes didn't match but diff passed compositing/video/poster.html -> ref test hashes didn't match but diff passed fast/borders/border-image-fill-no-border.html -> ref test hashes didn't match but diff passed fast/css/object-fit/object-fit-canvas.html -> ref test hashes didn't match but diff passed fast/css/sticky/sticky-left-percentage.html -> ref test hashes didn't match but diff passed fast/images/animated-gif-no-layout.html -> ref test hashes didn't match but diff passed fast/images/gif-loop-count.html -> ref test hashes didn't match but diff passed fast/images/pdf-as-image-crop-box.html -> ref test hashes didn't match but diff passed fast/regions/overflow/overflow-size-change-with-stacking-context-rtl.html -> ref test hashes didn't match but diff passed imported/w3c/css/css-color-3/t424-hsl-basic-a.xht -> ref test hashes didn't match but diff passed imported/w3c/css/css-color-3/t424-hsl-parsing-f.xht -> ref test hashes didn't match but diff passed imported/w3c/css/css-color-3/t425-hsla-parsing-f.xht -> ref test hashes didn't match but diff passed imported/mozilla/svg/text/textpath-selection.svg -> ref test hashes didn't match but diff passed svg/canvas/canvas-global-alpha-svg.html -> ref test hashes didn't match but diff passed svg/repaint/buffered-rendering-static-image.html -> ref test hashes didn't match but diff passed This information is all useless to me, and to anyone, I think. If these were pixel tests, then I suppose someone might want to know that the tests are matching on disk, but the hash isn’t matching, so they can regenerate images with correct hashes for faster test running in the future. But for a reference test, who is this message for? What is the action needed here? We should not contaminate the output with these logs, unless there is some real world project we will do that makes use of them.
Attachments
disable ImageDiff tolerance (1.67 KB, patch)
2016-02-07 12:29 PST, Alexey Proskuryakov 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Alexey Proskuryakov
Comment 1 2016-02-06 12:34:43 PST
I'm torn on this one, as it's indeed just useless noise for most. Yet a lot of the time this indicates a bug somewhere. There were examples of that in a webkit-dev thread on this topic IIRC. There is indeed an intention to have another look at noise in reftests soon, hopefully culminating in enabling --accelerated-drawing in tests.
Darin Adler
Comment 2 2016-02-06 15:11:42 PST
You say that a lot of a time this indicates a bug. Do any of the these 20 failures I am seeing every time I run test today indicate a bug?
Alexey Proskuryakov
Comment 3 2016-02-06 17:37:24 PST
Two of these are tracked in bug 142632, which I think is a real bug that needs to be fixed. Another known reason was bug 139884, which definitely needed to be discovered and fixed. The best way to check the others is to edit single_test_runner.py so that it would report these tests as failing, and to manually compare the results. I think that the following should do it (untested): - if diff_result[0]: - failures.append(test_failures.FailureReftestMismatch(reference_filename)) - else: - _log.warning(" %s -> ref test hashes didn't match but diff passed" % self._test_name) + failures.append(test_failures.FailureReftestMismatch(reference_filename))
Darin Adler
Comment 4 2016-02-06 17:52:37 PST
Let me restate my request: Please fix checking so that when the *sizes* of the images don’t match, we do not report that as "ref test hashes didn't match but diff passed". Then get rid of that message entirely. All the useful cases of this message are associated with that mistake.
Alexey Proskuryakov
Comment 5 2016-02-07 12:19:32 PST
> I think that the following should do it (untested): That doesn't work for reftests, will attach a patch that does. Now that I have a build on this machine, and ran the tests, I only got 14 failures out of these 20. I got: - many that look like color management issues; - some antialiasing differences; - and several that I cannot easily categorize; - no size failures, so I'd like to know what hardware/OS combination you got these on (also WebKit1 or WebKit2, which could be seen in complete run-webkit-tests output). The other 4 failures that you got could be specific to your hardware, in which case they need to be reported internally at Apple. I do not think that hiding these messages would be the right answer. We should work to get these failures down to zero, yet even now, one can and should use them to detect regressions in their patches (see e.g. Dave's request in bug 69444).
Alexey Proskuryakov
Comment 6 2016-02-07 12:29:05 PST
Created attachment 270827 [details] disable ImageDiff tolerance
Note You need to log in before you can comment on or make changes to this bug.