Right now, only chromium port implements diff_image. This would mean that python throws an exception like the one below whenever ref-test fails: Line:61 Path does not exist. platform/mac/fonts/color-bitmap.html worker/0 raised TypeError('argument 1 must be string or buffer, not None'): layout_tests/controllers/worker.py:91 (in run) self._worker_connection.run_message_loop() layout_tests/controllers/message_broker.py:191 (in run_message_loop) self._broker.run_message_loop(self._run_topic, self._client, delay_secs) layout_tests/controllers/message_broker.py:127 (in run_message_loop) self._run_loop(topic_name, client, block=True, delay_secs=delay_secs) layout_tests/controllers/message_broker.py:141 (in _run_loop) self._dispatch_message(msg, client) layout_tests/controllers/message_broker.py:150 (in _dispatch_message) message_handler(message.src, *optargs) layout_tests/controllers/worker.py:111 (in handle_test_list) self._run_test(test_input) layout_tests/controllers/worker.py:126 (in _run_test) result = self.run_test_with_timeout(test_input, test_timeout_sec) layout_tests/controllers/worker.py:165 (in run_test_with_timeout) return self._run_test_in_this_thread(test_input) layout_tests/controllers/worker.py:250 (in _run_test_in_this_thread) return self.run_single_test(self._driver, test_input) layout_tests/controllers/worker.py:254 (in run_single_test) test_input, driver, self._name) layout_tests/controllers/single_test_runner.py:46 (in run_single_test) return runner.run() layout_tests/controllers/single_test_runner.py:124 (in run) return self._run_reftest() layout_tests/controllers/single_test_runner.py:292 (in _run_reftest) test_result_writer.write_test_result(self._port, self._test_name, driver_output1, driver_output2, test_result.failures) layout_tests/controllers/test_result_writer.py:74 (in write_test_result) writer.write_image_diff_files(image_diff) layout_tests/controllers/test_result_writer.py:207 (in write_image_diff_files) fs.write_binary_file(diff_filename, image_diff) common/system/filesystem.py:205 (in write_binary_file) f.write(contents)
A quick fix might be simply skipping writing the result of image_diff on mac port. Skipping writing result doesn't affect the result of tests. Later we should implement image_diff on mac port. Any thought?
Created attachment 117096 [details] Patch
As far as I know, chromium is the only port that implements diff_image.
Comment on attachment 117096 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=117096&action=review Is it really hard to implement diff_image for other ports? I'd rather not leave it broken for other ports. > Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py:74 > + # Mac port returns 'None' since it doesn't implement diff_image. This should say non-chromium port. > Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py:75 > + if image_diff is not None: isn't != less verbose?
Created attachment 117106 [details] fix name
Comment on attachment 117106 [details] fix name View in context: https://bugs.webkit.org/attachment.cgi?id=117106&action=review > Tools/ChangeLog:9 > + Reviewed by NOBODY (OOPS!). This line should appear before the description.
Comment on attachment 117096 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=117096&action=review >> Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py:74 >> + # Mac port returns 'None' since it doesn't implement diff_image. > > This should say non-chromium port. Done. > Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py:76 > + writer.write_image_diff_files(image_diff) "is not" is better in this case because None is singleton. I guess that's slightly faster and well accepted usage.
Ops. I'll fix it and update the patch. (In reply to comment #6) > (From update of attachment 117106 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=117106&action=review > > > Tools/ChangeLog:9 > > + Reviewed by NOBODY (OOPS!). > > This line should appear before the description.
Created attachment 117109 [details] fix changelog
I noticed that mac port actually implements diff_image. WebKitPort is a super class of Apple port, which is super class of MacPort. WebKitPort implements diff_image. The problem is WebKitPorts's diff_image seems to be wrongly implemented. Let me investigate further and file another bug.
Committed r101455: <http://trac.webkit.org/changeset/101455>
I filed another bug in: https://bugs.webkit.org/show_bug.cgi?id=73406