Bug 92934

Summary: nrwt: handle errors from image diff better
Product: WebKit Reporter: Dirk Pranke <dpranke>
Component: New BugsAssignee: Dirk Pranke <dpranke>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dglazkov, jamesr, nduca, ojan, ossy, shawnsingh, simon.fraser, tony, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 93338    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from gce-cr-linux-05
none
delete test code accidentally left in
none
fix leak of tmpdir in chromium diff_image()
none
Patch ojan: review+

Description Dirk Pranke 2012-08-01 19:23:22 PDT
nrwt: handle errors from image diff better
Comment 1 Dirk Pranke 2012-08-01 19:25:25 PDT
Created attachment 155952 [details]
Patch
Comment 2 Dirk Pranke 2012-08-01 19:27:37 PDT
*** Bug 66581 has been marked as a duplicate of this bug. ***
Comment 3 WebKit Review Bot 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
Comment 4 WebKit Review Bot 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
Comment 5 Dirk Pranke 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?
Comment 6 James Robinson 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?
Comment 7 Dirk Pranke 2012-08-02 15:15:48 PDT
never mind ... amazing that tests fail if you forgot to delete your test code
Comment 8 Dirk Pranke 2012-08-02 15:18:22 PDT
Created attachment 156179 [details]
delete test code accidentally left in
Comment 9 Ojan Vafai 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)?
Comment 10 Dirk Pranke 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.
Comment 11 Dirk Pranke 2012-08-02 16:07:19 PDT
Created attachment 156192 [details]
fix leak of tmpdir in chromium diff_image()
Comment 12 Dirk Pranke 2012-08-06 14:00:39 PDT
Committed r124801: <http://trac.webkit.org/changeset/124801>
Comment 13 WebKit Review Bot 2012-08-06 23:03:06 PDT
Re-opened since this is blocked by 93338
Comment 14 Csaba Osztrogon√°c 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
Comment 15 Dirk Pranke 2012-08-07 15:59:37 PDT
Created attachment 157030 [details]
Patch
Comment 16 Dirk Pranke 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) ).
Comment 17 Dirk Pranke 2012-08-07 18:08:58 PDT
Committed r124958: <http://trac.webkit.org/changeset/124958>