73381 2011-11-29 17:37:11 -0800 base port needs to implement diff_image 2022-03-01 03:09:57 -0800 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P1 Normal --- 1 rniwa hayato abarth eric hayato ojan webkit.review.bot oldest_to_newest 511253 0 rniwa 2011-11-29 17:37:11 -0800 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) 511273 1 hayato 2011-11-29 18:12:33 -0800 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? 511299 2 117096 hayato 2011-11-29 18:45:25 -0800 Created attachment 117096 Patch 511308 3 rniwa 2011-11-29 19:14:53 -0800 As far as I know, chromium is the only port that implements diff_image. 511310 4 117096 rniwa 2011-11-29 19:17:11 -0800 Comment on attachment 117096 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? 511329 5 117106 hayato 2011-11-29 20:31:26 -0800 Created attachment 117106 fix name 511330 6 117106 rniwa 2011-11-29 20:32:32 -0800 Comment on attachment 117106 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. 511333 7 117096 hayato 2011-11-29 20:33:08 -0800 Comment on attachment 117096 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. 511335 8 hayato 2011-11-29 20:33:46 -0800 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. 511338 9 117109 hayato 2011-11-29 20:35:58 -0800 Created attachment 117109 fix changelog 511414 10 hayato 2011-11-29 23:03:47 -0800 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. 511457 11 hayato 2011-11-30 00:17:46 -0800 Committed r101455: <http://trac.webkit.org/changeset/101455> 511465 12 hayato 2011-11-30 00:28:24 -0800 I filed another bug in: https://bugs.webkit.org/show_bug.cgi?id=73406 117096 2011-11-29 18:45:25 -0800 2022-03-01 03:09:57 -0800 Patch bug-73381-20111130114523.patch text/plain 1982 hayato U3VidmVyc2lvbiBSZXZpc2lvbjogMTAxNDM4CmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggOWQzZTRhYmRjYjUyOTFiMjExZDliNTExZGUwMGNjZThm MWIwYjkxMC4uMGM0MjNjOTNkNTQ5M2YxN2YyNTQ4ODg2Mzk4N2NlYjQyNDMyZDE4ZCAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1 IEBACisyMDExLTExLTI5ICBIYXlhdG8gSXRvICA8aGF5YXRvQGNocm9taXVtLm9yZz4KKworICAg ICAgICBTa2lwIHdyaXRpbmcgdGhlIHJlc3VsdCBvZiBkaWZmX2ltYWdlIHNpbmNlIG1hYyBwb3J0 IGRvZXNuJ3QgaW1wbGVtZW50IGRpZmZfaW1hZ2UuCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD03MzM4MQorCisgICAgICAgIFRoaXMgaXMgYSBxdWljayBm aXggZm9yIGJyZWFrYWdlIG9mIHRlc3RzIG9uIG1hYyBwb3J0LiBXZSBzaG91bGQgaW1wbGVtZW50 IGRpZmZfaW1hZ2UgbGF0ZXIgb24gbWFjIHBvcnQuCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9C T0RZIChPT1BTISkuCisKKyAgICAgICAgKiBTY3JpcHRzL3dlYmtpdHB5L2xheW91dF90ZXN0cy9j b250cm9sbGVycy90ZXN0X3Jlc3VsdF93cml0ZXIucHk6CisgICAgICAgICh3cml0ZV90ZXN0X3Jl c3VsdCk6CisKIDIwMTEtMTEtMjkgIE9qYW4gVmFmYWkgIDxvamFuQGNocm9taXVtLm9yZz4KIAog ICAgICAgICBMaXN0IG9mIGJ1aWxkZXJzIGluIHRoZSBmbGFraW5lc3MgZGFzaGJvYXJkIGlzIG91 dCBvZiBkYXRlCmRpZmYgLS1naXQgYS9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L2xheW91dF90ZXN0 cy9jb250cm9sbGVycy90ZXN0X3Jlc3VsdF93cml0ZXIucHkgYi9Ub29scy9TY3JpcHRzL3dlYmtp dHB5L2xheW91dF90ZXN0cy9jb250cm9sbGVycy90ZXN0X3Jlc3VsdF93cml0ZXIucHkKaW5kZXgg OTc4YzIzYmZkN2M4OGFhMTM2NjUxZmI1OWE3MTEwM2VkYmY1N2NjZC4uZDYyZDdmZmMyODM0NjJl MTFiNzliZDMxYTc5ZjhlYmYzMWJlOGQ3NSAxMDA2NDQKLS0tIGEvVG9vbHMvU2NyaXB0cy93ZWJr aXRweS9sYXlvdXRfdGVzdHMvY29udHJvbGxlcnMvdGVzdF9yZXN1bHRfd3JpdGVyLnB5CisrKyBi L1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvbGF5b3V0X3Rlc3RzL2NvbnRyb2xsZXJzL3Rlc3RfcmVz dWx0X3dyaXRlci5weQpAQCAtNzEsNyArNzEsOSBAQCBkZWYgd3JpdGVfdGVzdF9yZXN1bHQocG9y dCwgdGVzdF9uYW1lLCBkcml2ZXJfb3V0cHV0LAogICAgICAgICAgICAgIyBGSVhNRTogV2Ugc2hv dWxkIGFsd2F5cyBoYXZlIDIgaW1hZ2VzIGhlcmUuCiAgICAgICAgICAgICBpZiBkcml2ZXJfb3V0 cHV0LmltYWdlIGFuZCBleHBlY3RlZF9kcml2ZXJfb3V0cHV0LmltYWdlOgogICAgICAgICAgICAg ICAgIGltYWdlX2RpZmYgPSBwb3J0LmRpZmZfaW1hZ2UoZHJpdmVyX291dHB1dC5pbWFnZSwgZXhw ZWN0ZWRfZHJpdmVyX291dHB1dC5pbWFnZSlbMF0KLSAgICAgICAgICAgICAgICB3cml0ZXIud3Jp dGVfaW1hZ2VfZGlmZl9maWxlcyhpbWFnZV9kaWZmKQorICAgICAgICAgICAgICAgICMgTWFjIHBv cnQgcmV0dXJucyAnTm9uZScgc2luY2UgaXQgZG9lc24ndCBpbXBsZW1lbnQgZGlmZl9pbWFnZS4K KyAgICAgICAgICAgICAgICBpZiBpbWFnZV9kaWZmIGlzIG5vdCBOb25lOgorICAgICAgICAgICAg ICAgICAgICB3cml0ZXIud3JpdGVfaW1hZ2VfZGlmZl9maWxlcyhpbWFnZV9kaWZmKQogICAgICAg ICAgICAgd3JpdGVyLmNvcHlfZmlsZShmYWlsdXJlLnJlZmVyZW5jZV9maWxlbmFtZSkKICAgICAg ICAgZWxpZiBpc2luc3RhbmNlKGZhaWx1cmUsIHRlc3RfZmFpbHVyZXMuRmFpbHVyZVJlZnRlc3RN aXNtYXRjaERpZE5vdE9jY3VyKToKICAgICAgICAgICAgIHdyaXRlci53cml0ZV9pbWFnZV9maWxl cyhkcml2ZXJfb3V0cHV0LmltYWdlLCBleHBlY3RlZF9pbWFnZT1Ob25lKQo= 117106 2011-11-29 20:31:26 -0800 2011-11-29 20:35:52 -0800 fix name bug-73381-20111130133124.patch text/plain 2018 hayato U3VidmVyc2lvbiBSZXZpc2lvbjogMTAxNDM4CmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggOWQzZTRhYmRjYjUyOTFiMjExZDliNTExZGUwMGNjZThm MWIwYjkxMC4uMTM1OTQ1MzNkYjNhZmIzOWZkODIxZjRiMzcxYjdhYTg2YzQ5ZTc1NyAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2 IEBACisyMDExLTExLTI5ICBIYXlhdG8gSXRvICA8aGF5YXRvQGNocm9taXVtLm9yZz4KKworICAg ICAgICBTa2lwIHdyaXRpbmcgdGhlIHJlc3VsdCBvZiBkaWZmX2ltYWdlIHNpbmNlIG5vbi1jaHJv bWl1bSBwb3J0cyBkb24ndCBpbXBsZW1lbnQgZGlmZl9pbWFnZS4KKyAgICAgICAgaHR0cHM6Ly9i dWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTczMzgxCisKKyAgICAgICAgVGhpcyBpcyBh IHF1aWNrIGZpeCBmb3IgYnJlYWthZ2Ugb2YgdGVzdHMgb24gbWFjIHBvcnQuCisgICAgICAgIFdl IHNob3VsZCBpbXBsZW1lbnQgZGlmZl9pbWFnZSBsYXRlciBvbiBub24tY2hyb21pdW0gcG9ydHMu CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBTY3Jp cHRzL3dlYmtpdHB5L2xheW91dF90ZXN0cy9jb250cm9sbGVycy90ZXN0X3Jlc3VsdF93cml0ZXIu cHk6CisgICAgICAgICh3cml0ZV90ZXN0X3Jlc3VsdCk6CisKIDIwMTEtMTEtMjkgIE9qYW4gVmFm YWkgIDxvamFuQGNocm9taXVtLm9yZz4KIAogICAgICAgICBMaXN0IG9mIGJ1aWxkZXJzIGluIHRo ZSBmbGFraW5lc3MgZGFzaGJvYXJkIGlzIG91dCBvZiBkYXRlCmRpZmYgLS1naXQgYS9Ub29scy9T Y3JpcHRzL3dlYmtpdHB5L2xheW91dF90ZXN0cy9jb250cm9sbGVycy90ZXN0X3Jlc3VsdF93cml0 ZXIucHkgYi9Ub29scy9TY3JpcHRzL3dlYmtpdHB5L2xheW91dF90ZXN0cy9jb250cm9sbGVycy90 ZXN0X3Jlc3VsdF93cml0ZXIucHkKaW5kZXggOTc4YzIzYmZkN2M4OGFhMTM2NjUxZmI1OWE3MTEw M2VkYmY1N2NjZC4uOWVjMDVmMDk3OWFjZjgyNzE1YTNmNmMzMmM1YmIyNGVkMzY2MDc0NSAxMDA2 NDQKLS0tIGEvVG9vbHMvU2NyaXB0cy93ZWJraXRweS9sYXlvdXRfdGVzdHMvY29udHJvbGxlcnMv dGVzdF9yZXN1bHRfd3JpdGVyLnB5CisrKyBiL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvbGF5b3V0 X3Rlc3RzL2NvbnRyb2xsZXJzL3Rlc3RfcmVzdWx0X3dyaXRlci5weQpAQCAtNzEsNyArNzEsOSBA QCBkZWYgd3JpdGVfdGVzdF9yZXN1bHQocG9ydCwgdGVzdF9uYW1lLCBkcml2ZXJfb3V0cHV0LAog ICAgICAgICAgICAgIyBGSVhNRTogV2Ugc2hvdWxkIGFsd2F5cyBoYXZlIDIgaW1hZ2VzIGhlcmUu CiAgICAgICAgICAgICBpZiBkcml2ZXJfb3V0cHV0LmltYWdlIGFuZCBleHBlY3RlZF9kcml2ZXJf b3V0cHV0LmltYWdlOgogICAgICAgICAgICAgICAgIGltYWdlX2RpZmYgPSBwb3J0LmRpZmZfaW1h Z2UoZHJpdmVyX291dHB1dC5pbWFnZSwgZXhwZWN0ZWRfZHJpdmVyX291dHB1dC5pbWFnZSlbMF0K LSAgICAgICAgICAgICAgICB3cml0ZXIud3JpdGVfaW1hZ2VfZGlmZl9maWxlcyhpbWFnZV9kaWZm KQorICAgICAgICAgICAgICAgICMgTm9uLWNocm9taXVtIHBvcnRzIHJldHVybiAnTm9uZScgc2lu Y2UgdGhleSBkb24ndCBpbXBsZW1lbnQgZGlmZl9pbWFnZS4KKyAgICAgICAgICAgICAgICBpZiBp bWFnZV9kaWZmIGlzIG5vdCBOb25lOgorICAgICAgICAgICAgICAgICAgICB3cml0ZXIud3JpdGVf aW1hZ2VfZGlmZl9maWxlcyhpbWFnZV9kaWZmKQogICAgICAgICAgICAgd3JpdGVyLmNvcHlfZmls ZShmYWlsdXJlLnJlZmVyZW5jZV9maWxlbmFtZSkKICAgICAgICAgZWxpZiBpc2luc3RhbmNlKGZh aWx1cmUsIHRlc3RfZmFpbHVyZXMuRmFpbHVyZVJlZnRlc3RNaXNtYXRjaERpZE5vdE9jY3VyKToK ICAgICAgICAgICAgIHdyaXRlci53cml0ZV9pbWFnZV9maWxlcyhkcml2ZXJfb3V0cHV0LmltYWdl LCBleHBlY3RlZF9pbWFnZT1Ob25lKQo= 117109 2011-11-29 20:35:58 -0800 2011-11-30 00:10:16 -0800 fix changelog bug-73381-20111130133557.patch text/plain 2031 hayato U3VidmVyc2lvbiBSZXZpc2lvbjogMTAxNDM4CmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggOWQzZTRhYmRjYjUyOTFiMjExZDliNTExZGUwMGNjZThm MWIwYjkxMC4uYjY5MGEyNjdlNGJlNzVjM2JmY2Y1NGE4NTEyYmUwNzI3NzhjY2IzZSAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2 IEBACisyMDExLTExLTI5ICBIYXlhdG8gSXRvICA8aGF5YXRvQGNocm9taXVtLm9yZz4KKworICAg ICAgICBTa2lwIHdyaXRpbmcgdGhlIHJlc3VsdCBvZiBkaWZmX2ltYWdlIHNpbmNlIG5vbi1jaHJv bWl1bSBwb3J0cyBkb24ndCBpbXBsZW1lbnQgZGlmZl9pbWFnZS4KKyAgICAgICAgaHR0cHM6Ly9i dWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTczMzgxCisKKyAgICAgICAgUmV2aWV3ZWQg YnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgVGhpcyBpcyBhIHF1aWNrIGZpeCBmb3IgdGhl IGJyZWFrYWdlIG9mIHRlc3RzIG9uIG5vbi1jaHJvbWl1bSBwb3J0LgorICAgICAgICBXZSBzaG91 bGQgaW1wbGVtZW50IGRpZmZfaW1hZ2UgbGF0ZXIgb24gbm9uLWNocm9taXVtIHBvcnRzLgorCisg ICAgICAgICogU2NyaXB0cy93ZWJraXRweS9sYXlvdXRfdGVzdHMvY29udHJvbGxlcnMvdGVzdF9y ZXN1bHRfd3JpdGVyLnB5OgorICAgICAgICAod3JpdGVfdGVzdF9yZXN1bHQpOgorCiAyMDExLTEx LTI5ICBPamFuIFZhZmFpICA8b2phbkBjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgTGlzdCBvZiBi dWlsZGVycyBpbiB0aGUgZmxha2luZXNzIGRhc2hib2FyZCBpcyBvdXQgb2YgZGF0ZQpkaWZmIC0t Z2l0IGEvVG9vbHMvU2NyaXB0cy93ZWJraXRweS9sYXlvdXRfdGVzdHMvY29udHJvbGxlcnMvdGVz dF9yZXN1bHRfd3JpdGVyLnB5IGIvVG9vbHMvU2NyaXB0cy93ZWJraXRweS9sYXlvdXRfdGVzdHMv Y29udHJvbGxlcnMvdGVzdF9yZXN1bHRfd3JpdGVyLnB5CmluZGV4IDk3OGMyM2JmZDdjODhhYTEz NjY1MWZiNTlhNzExMDNlZGJmNTdjY2QuLjllYzA1ZjA5NzlhY2Y4MjcxNWEzZjZjMzJjNWJiMjRl ZDM2NjA3NDUgMTAwNjQ0Ci0tLSBhL1Rvb2xzL1NjcmlwdHMvd2Via2l0cHkvbGF5b3V0X3Rlc3Rz L2NvbnRyb2xsZXJzL3Rlc3RfcmVzdWx0X3dyaXRlci5weQorKysgYi9Ub29scy9TY3JpcHRzL3dl YmtpdHB5L2xheW91dF90ZXN0cy9jb250cm9sbGVycy90ZXN0X3Jlc3VsdF93cml0ZXIucHkKQEAg LTcxLDcgKzcxLDkgQEAgZGVmIHdyaXRlX3Rlc3RfcmVzdWx0KHBvcnQsIHRlc3RfbmFtZSwgZHJp dmVyX291dHB1dCwKICAgICAgICAgICAgICMgRklYTUU6IFdlIHNob3VsZCBhbHdheXMgaGF2ZSAy IGltYWdlcyBoZXJlLgogICAgICAgICAgICAgaWYgZHJpdmVyX291dHB1dC5pbWFnZSBhbmQgZXhw ZWN0ZWRfZHJpdmVyX291dHB1dC5pbWFnZToKICAgICAgICAgICAgICAgICBpbWFnZV9kaWZmID0g cG9ydC5kaWZmX2ltYWdlKGRyaXZlcl9vdXRwdXQuaW1hZ2UsIGV4cGVjdGVkX2RyaXZlcl9vdXRw dXQuaW1hZ2UpWzBdCi0gICAgICAgICAgICAgICAgd3JpdGVyLndyaXRlX2ltYWdlX2RpZmZfZmls ZXMoaW1hZ2VfZGlmZikKKyAgICAgICAgICAgICAgICAjIE5vbi1jaHJvbWl1bSBwb3J0cyByZXR1 cm4gJ05vbmUnIHNpbmNlIHRoZXkgZG9uJ3QgaW1wbGVtZW50IGRpZmZfaW1hZ2UuCisgICAgICAg ICAgICAgICAgaWYgaW1hZ2VfZGlmZiBpcyBub3QgTm9uZToKKyAgICAgICAgICAgICAgICAgICAg d3JpdGVyLndyaXRlX2ltYWdlX2RpZmZfZmlsZXMoaW1hZ2VfZGlmZikKICAgICAgICAgICAgIHdy aXRlci5jb3B5X2ZpbGUoZmFpbHVyZS5yZWZlcmVuY2VfZmlsZW5hbWUpCiAgICAgICAgIGVsaWYg aXNpbnN0YW5jZShmYWlsdXJlLCB0ZXN0X2ZhaWx1cmVzLkZhaWx1cmVSZWZ0ZXN0TWlzbWF0Y2hE aWROb3RPY2N1cik6CiAgICAgICAgICAgICB3cml0ZXIud3JpdGVfaW1hZ2VfZmlsZXMoZHJpdmVy X291dHB1dC5pbWFnZSwgZXhwZWN0ZWRfaW1hZ2U9Tm9uZSkK