Bug 69444

I assume you're running on the apple mac port ... is this a problem because by default the apple mac port runs with a tolerance set and allows some amount of fuzziness to match? IIRC, this happens even if you specify --tolerance=0.0. Also, to clarify, do you want to see the pixel hash mismatches on the console, in the results html file, or both?

Console would be sufficient for me.

Interesting. It sounds like we just want --tolerance=0.0 by default. But it's also possible to spit out these errors to the console. I'm not sure I understand the current --tolerance vs. not decisions on various ports.

Note that we added the %age of the pixel mismatch in bug 67253, and as I noted in comment #1, using a tolerance of 0.0 may still mask some differences on the apple ports. It should be easy to note the hash mismatch in either the console or the results file.

On a related note, currently, if you use the 'test' port, we have a test case failures/expected/checksum.html, where the checksum fails to match but the image matches; when you run rwt --platform test, this is reported as an 'unexpected pass', when it should be reported as an image hash mismatch

Adding Ojan and Tony to this ... If memory serves, Chromium has always considered the case where the image hashes don't match but the images themselves do to be a passing case. I think we used to report a warning in the results.html file, but at some point we stopped doing that (either when we made the results file generated from the json data, or when we embedded the checksums into the image, I think). The upshot is we currently ignore the problem, which seems wrong, but I'm not sure which is the right thing to do. I think we have four choices: 1) report a warning to the console but otherwise treat the test as passing across the board 2) report a warning to the console and include the warning in results.html / results.json, but consider the test as passing (similar to how we handle stderr output now) 3) consider the test as failing, but only if --tolerance is set to 0.0 4) consider the test as failing, regardless of what --tolerance is set to In any situation, we'd probably need to change the results.html output to identify this case properly and not just confuse the user. I have a patch that does #1 but only logs an error to the console that I will post, just to demonstrate some of the scope of work involved in fixing it. I have no idea how often this will come up in practice so we probably don't want to overengineer this.

Created attachment 127303 [details] Patch

Created attachment 127305 [details] make the tests pass

Created attachment 127306 [details] ignore - wrong bug

Comment on attachment 127305 [details] make the tests pass View in context: https://bugs.webkit.org/attachment.cgi?id=127305&action=review We should just match old-run-webkit-tests behavior, which is to log to the console. > Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py:270 > + _log.warning('%s had mismatched checksums but matching images (exp %s, got %s)' % > + (self._test_name, expected_driver_output.image_hash, driver_output.image_hash)) Can we match ORWT's text? If not, we should at least include the image diff percent.

(In reply to comment #10) > (From update of attachment 127305 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=127305&action=review > > We should just match old-run-webkit-tests behavior, which is to log to the console. > > > Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py:270 > > + _log.warning('%s had mismatched checksums but matching images (exp %s, got %s)' % > > + (self._test_name, expected_driver_output.image_hash, driver_output.image_hash)) > > Can we match ORWT's text? If not, we should at least include the image diff percent. Good idea. I will take a look.

Created attachment 127506 [details] change warning message to match ORWT

Output now is: $ ./new-run-webkit-tests -p fast/html/keygen.html fast/html/keygen.html -> pixel hash failed (but pixel test still passes) All 1 tests ran as expected. $ I would be happy to add the tolerance value to both NRWT and ORWT as well if desired.

Comment on attachment 127506 [details] change warning message to match ORWT OK.

Committed r108144: <http://trac.webkit.org/changeset/108144>