RESOLVED FIXED 92934
nrwt: handle errors from image diff better
https://bugs.webkit.org/show_bug.cgi?id=92934
Summary nrwt: handle errors from image diff better
Dirk Pranke
Reported 2012-08-01 19:23:22 PDT
nrwt: handle errors from image diff better
Attachments
Patch (20.62 KB, patch)
2012-08-01 19:25 PDT, Dirk Pranke
no flags
Archive of layout-test-results from gce-cr-linux-05 (427.21 KB, application/zip)
2012-08-01 20:42 PDT, WebKit Review Bot
no flags
delete test code accidentally left in (19.72 KB, patch)
2012-08-02 15:18 PDT, Dirk Pranke
no flags
fix leak of tmpdir in chromium diff_image() (19.73 KB, patch)
2012-08-02 16:07 PDT, Dirk Pranke
no flags
Patch (20.38 KB, patch)
2012-08-07 15:59 PDT, Dirk Pranke
ojan: review+
Dirk Pranke
Comment 1 2012-08-01 19:25:25 PDT
Dirk Pranke
Comment 2 2012-08-01 19:27:37 PDT
*** Bug 66581 has been marked as a duplicate of this bug. ***
WebKit Review Bot
Comment 3 2012-08-01 20:42:10 PDT
Comment on attachment 155952 [details] Patch Attachment 155952 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13415434 New failing tests: compositing/checkerboard.html animations/3d/matrix-transform-type-animation.html animations/3d/state-at-end-event-transform.html compositing/direct-image-compositing.html animations/additive-transform-animations.html animations/3d/replace-filling-transform.html compositing/scrollbar-painting.html compositing/clip-change.html compositing/text-on-large-layer.html compositing/layers-inside-overflow-scroll.html compositing/animation/state-at-end-event-transform-layer.html compositing/sibling-positioning.html compositing/generated-content.html compositing/fixed-position-changed-in-composited-layer.html compositing/self-painting-layers.html animations/cross-fade-webkit-mask-box-image.html animations/3d/change-transform-in-end-event.html compositing/absolute-position-changed-with-composited-parent-layer.html compositing/absolute-position-changed-in-composited-layer.html compositing/flat-with-transformed-child.html compositing/backface-visibility/backface-visibility-3d.html animations/cross-fade-list-style-image.html animations/cross-fade-background-image.html animations/cross-fade-border-image-source.html compositing/fixed-position-scroll-offset-history-restore.html compositing/compositing-visible-descendant.html compositing/fixed-position-changed-within-composited-parent-layer.html compositing/animation/busy-indicator.html
WebKit Review Bot
Comment 4 2012-08-01 20:42:16 PDT
Created attachment 155959 [details] Archive of layout-test-results from gce-cr-linux-05 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-05 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Dirk Pranke
Comment 5 2012-08-02 11:30:34 PDT
(In reply to comment #4) > Created an attachment (id=155959) [details] > Archive of layout-test-results from gce-cr-linux-05 > > The attached test failures were seen while running run-webkit-tests on the chromium-ews. > Bot: gce-cr-linux-05 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid Well, the patch did something, since all of the failures are image diff crashes, supposedly :). I will see if I can reproduce this locally. Nat, James -- this is kinda interesting since these seem to be compositing and animation tests; any reason the PNGs from those might be different?
James Robinson
Comment 6 2012-08-02 13:01:27 PDT
Not that I can think of, should be the same as any other PNG. Can you get crash stacks or anything of that sort?
Dirk Pranke
Comment 7 2012-08-02 15:15:48 PDT
never mind ... amazing that tests fail if you forgot to delete your test code
Dirk Pranke
Comment 8 2012-08-02 15:18:22 PDT
Created attachment 156179 [details] delete test code accidentally left in
Ojan Vafai
Comment 9 2012-08-02 15:25:29 PDT
Comment on attachment 156179 [details] delete test code accidentally left in View in context: https://bugs.webkit.org/attachment.cgi?id=156179&action=review r- just until I understand why the rmtree change makes sense. The rest LGTM. > Tools/Scripts/webkitpy/layout_tests/port/chromium.py:230 > + self._filesystem.rmtree(str(tempdir)) We used to always do this and now only do it for exit_code == 1. Can you explain why in the ChangeLog (assuming this is correct)?
Dirk Pranke
Comment 10 2012-08-02 15:31:50 PDT
Comment on attachment 156179 [details] delete test code accidentally left in View in context: https://bugs.webkit.org/attachment.cgi?id=156179&action=review >> Tools/Scripts/webkitpy/layout_tests/port/chromium.py:230 >> + self._filesystem.rmtree(str(tempdir)) > > We used to always do this and now only do it for exit_code == 1. Can you explain why in the ChangeLog (assuming this is correct)? Oh, whoops. That is incorrect and was not intentional. Will fix.
Dirk Pranke
Comment 11 2012-08-02 16:07:19 PDT
Created attachment 156192 [details] fix leak of tmpdir in chromium diff_image()
Dirk Pranke
Comment 12 2012-08-06 14:00:39 PDT
WebKit Review Bot
Comment 13 2012-08-06 23:03:06 PDT
Re-opened since this is blocked by 93338
Csaba Osztrogonác
Comment 14 2012-08-06 23:09:34 PDT
(In reply to comment #13) > Re-opened since this is blocked by 93338 I had to roll it out, because it broke NRWT on Qt: 14:48:41.399 21814 worker/0 ietestcenter/css3/multicolumn/column-containing-block-002.htm failed: 14:48:41.399 21814 worker/0 Mismatch with reference 14:48:41.650 21814 worker/0 cleaning up 14:48:41.650 21814 worker/0 killing driver 14:48:41.681 21814 ValueError("need more than 2 values to unpack") raised, exiting ValueError raised: need more than 2 values to unpack 14:48:41.718 21814 File "/home/webkitbuildbot/slaves/release32bit-qt5/buildslave/qt-linux-32-release-qt5/build/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 110, in run 14:48:41.718 21814 unexpected_result_count = manager.run(args) 14:48:41.719 21814 File "/home/webkitbuildbot/slaves/release32bit-qt5/buildslave/qt-linux-32-release-qt5/build/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 418, in run 14:48:41.719 21814 self._run_tests(self._test_names, result_summary, int(self._options.child_processes)) 14:48:41.719 21814 File "/home/webkitbuildbot/slaves/release32bit-qt5/buildslave/qt-linux-32-release-qt5/build/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 471, in _run_tests 14:48:41.719 21814 return self._runner.run_tests(test_inputs, self._expectations, result_summary, num_workers, needs_http, needs_websockets, self._retrying) 14:48:41.719 21814 File "/home/webkitbuildbot/slaves/release32bit-qt5/buildslave/qt-linux-32-release-qt5/build/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 140, in run_tests 14:48:41.719 21814 pool.run(('test_list', shard.name, shard.test_inputs) for shard in all_shards) 14:48:41.719 21814 File "/home/webkitbuildbot/slaves/release32bit-qt5/buildslave/qt-linux-32-release-qt5/build/Tools/Scripts/webkitpy/common/message_pool.py", line 97, in run 14:48:41.719 21814 self.wait() 14:48:41.719 21814 File "/home/webkitbuildbot/slaves/release32bit-qt5/buildslave/qt-linux-32-release-qt5/build/Tools/Scripts/webkitpy/common/message_pool.py", line 127, in wait 14:48:41.719 21814 self._workers[0].run() 14:48:41.719 21814 File "/home/webkitbuildbot/slaves/release32bit-qt5/buildslave/qt-linux-32-release-qt5/build/Tools/Scripts/webkitpy/common/message_pool.py", line 267, in run 14:48:41.719 21814 self._raise(sys.exc_info()) 14:48:41.719 21814 File "/home/webkitbuildbot/slaves/release32bit-qt5/buildslave/qt-linux-32-release-qt5/build/Tools/Scripts/webkitpy/common/message_pool.py", line 255, in run 14:48:41.719 21814 worker.handle(message.name, message.src, *message.args) 14:48:41.719 21814 File "/home/webkitbuildbot/slaves/release32bit-qt5/buildslave/qt-linux-32-release-qt5/build/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 304, in handle 14:48:41.719 21814 self._run_test(test_input) 14:48:41.719 21814 File "/home/webkitbuildbot/slaves/release32bit-qt5/buildslave/qt-linux-32-release-qt5/build/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 323, in _run_test 14:48:41.720 21814 result = self._run_test_with_timeout(test_input, test_timeout_sec) 14:48:41.720 21814 File "/home/webkitbuildbot/slaves/release32bit-qt5/buildslave/qt-linux-32-release-qt5/build/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 365, in _run_test_with_timeout 14:48:41.720 21814 return self._run_test_in_this_thread(test_input) 14:48:41.720 21814 File "/home/webkitbuildbot/slaves/release32bit-qt5/buildslave/qt-linux-32-release-qt5/build/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 450, in _run_test_in_this_thread 14:48:41.720 21814 return self._run_single_test(self._driver, test_input) 14:48:41.720 21814 File "/home/webkitbuildbot/slaves/release32bit-qt5/buildslave/qt-linux-32-release-qt5/build/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py", line 454, in _run_single_test 14:48:41.720 21814 test_input, driver, self._name) 14:48:41.720 21814 File "/home/webkitbuildbot/slaves/release32bit-qt5/buildslave/qt-linux-32-release-qt5/build/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py", line 46, in run_single_test 14:48:41.720 21814 return runner.run() 14:48:41.720 21814 File "/home/webkitbuildbot/slaves/release32bit-qt5/buildslave/qt-linux-32-release-qt5/build/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py", line 99, in run 14:48:41.720 21814 return self._run_reftest() 14:48:41.720 21814 File "/home/webkitbuildbot/slaves/release32bit-qt5/buildslave/qt-linux-32-release-qt5/build/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py", line 304, in _run_reftest 14:48:41.720 21814 test_result_writer.write_test_result(self._filesystem, self._port, self._test_name, test_output, reference_output, test_result.failures) 14:48:41.720 21814 File "/home/webkitbuildbot/slaves/release32bit-qt5/buildslave/qt-linux-32-release-qt5/build/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py", line 72, in write_test_result 14:48:41.720 21814 diff_image, diff_percent, err_str = port.diff_image(driver_output.image, expected_driver_output.image, tolerance=0) Failed to execute Tools/Scripts/new-run-webkit-tests at ./Tools/Scripts/run-webkit-tests line 124. program finished with exit code 254
Dirk Pranke
Comment 15 2012-08-07 15:59:37 PDT
Dirk Pranke
Comment 16 2012-08-07 16:01:46 PDT
thanks for handling the rollout, ossy! I've uploaded a patch that fixes the problem; can you or Ojan take another look? (fix was in image_diff.py:110 changing from [None, 0] to (None, 0, None) ).
Dirk Pranke
Comment 17 2012-08-07 18:08:58 PDT
Note You need to log in before you can comment on or make changes to this bug.