Bug 153951 – "ref test hashes didn't match but diff passed" is useless, should not be logged

• Component: Tools / Tests
• Status: NEW

[Darin Adler]

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.

[Alexey Proskuryakov]

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]

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]

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]

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]

> 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]

Created attachment 270827 [details]
disable ImageDiff tolerance