Bug 49691

Created attachment 74162 [details] remove-uri

After this patch, I'll make another patch which makes Driver.run_test() take only one parameter, TestInput, to simplify the interface of Driver.run_test().

Comment on attachment 74162 [details] remove-uri View in context: https://bugs.webkit.org/attachment.cgi?id=74162&action=review LGTM otherwise. > WebKitTools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py:-175 > - test_input.uri = port.filename_to_uri(test_input.filename).strip() When you do make run_test() take a single input, just stuff this down into the port/* implementations so that the ports that do need to convert the test name to the uri can do so, and the ports that don't (e.g., the test port, the dryrun port), don't need to convert back. > WebKitTools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py:175 > + uri = port.filename_to_uri(test_input.filename).strip() Nit: You don't actually need the strip() . filename_to_uri() doesn't append any whitespace. I'm not sure why this crept in at some point, and I've removed it in the patches-that-have-not-yet-been-uploaded-for-review :)

Created attachment 74183 [details] remove-strip

Thank you for the review. I've updated patch. 'strip()' has been removed. (In reply to comment #3) > (From update of attachment 74162 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=74162&action=review > > LGTM otherwise. > > > WebKitTools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py:-175 > > - test_input.uri = port.filename_to_uri(test_input.filename).strip() > > When you do make run_test() take a single input, just stuff this down into the port/* implementations so that the ports that do need to convert the test name to the uri can do so, and the ports that don't (e.g., the test port, the dryrun port), don't need to convert back. Thank you. I'll do it for each ports. I don't think that is difficult work. > > > WebKitTools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py:175 > > + uri = port.filename_to_uri(test_input.filename).strip() > > Nit: You don't actually need the strip() . filename_to_uri() doesn't append any whitespace. I'm not sure why this crept in at some point, and I've removed it in the patches-that-have-not-yet-been-uploaded-for-review :) Done. I've removed 'strip()' in the patch-that-has-been-uploaded. :)

Comment on attachment 74183 [details] remove-strip Clearing flags on attachment: 74183 Committed r72267: <http://trac.webkit.org/changeset/72267>

All reviewed patches have been landed. Closing bug.

There have been some failures on WebKit Win after this patch landed. Do you think they might be related? Here's a sample crash: http://build.chromium.org/p/chromium/builders/Webkit%20Win/builds/1788/steps/webkit_tests/logs/stdio 2010-11-18 13:46:53,076 20544 dump_render_tree_thread.py:441 DEBUGThread-1 http/tests/xmlhttprequest/access-control-basic-post-fail-non-simple-content-type.html passed 2010-11-18 13:46:53,201 20544 executive.py:357 DEBUG"e:\b\build\slave\Webkit_Win\build\src\third_party\cygwin\bin\wdiff.exe --start-delete=##WDIFF_DEL## --end-delete=##WDIFF_END## --start-insert=##WDIFF_ADD## --end-insert=##WDIFF_END## e:\b\build\slave\Webkit_Win\build\src\webkit\Release\..\../../layout-test-results\platform/mac/editing/selection/25228-expected.txt e:\b\build\slave\Webkit_Win\build\src\webkit\Release\..\../../layout-test-results\platform/mac/editing/selection/25228-actual.txt" took 0.06s 2010-11-18 13:46:53,233 20544 dump_render_tree_thread.py:441 DEBUGThread-1 http/tests/xmlhttprequest/access-control-basic-whitelist-request-headers.html passed 2010-11-18 13:46:53,390 20544 dump_render_tree_thread.py:441 DEBUGThread-1 http/tests/xmlhttprequest/access-control-basic-whitelist-response-headers.html passed 2010-11-18 13:46:53,437 20544 executive.py:357 DEBUG"e:\b\build\slave\Webkit_Win\build\src\webkit\Release\image_diff.exe --diff c:\docume~1\chrome~2\locals~1\temp\tmpi4wul2\expected.png c:\docume~1\chrome~2\locals~1\temp\tmpi4wul2\actual.png e:\b\build\slave\Webkit_Win\build\src\webkit\Release\..\../../layout-test-results\platform/mac/editing/selection/25228-diff.png" took 0.22s 2010-11-18 13:46:53,437 20544 dump_render_tree_thread.py:438 DEBUGThread-4 platform/mac/editing/selection/25228.html failed: Text diff mismatch Image mismatch 2010-11-18 13:46:56,358 20544 dump_render_tree_thread.py:340 ERRORThread-2 dying, exception raised Traceback (most recent call last): File "e:\b\build\slave\Webkit_Win\build\src\third_party\WebKit\WebKitTools\Scripts\new-run-webkit-tests", line 38, in <module> sys.exit(run_webkit_tests.main()) File "e:\b\build\slave\Webkit_Win\build\src\third_party\WebKit\WebKitTools\Scripts\webkitpy\layout_tests\run_webkit_tests.py", line 1684, in main return run(port_obj, options, args) File "e:\b\build\slave\Webkit_Win\build\src\third_party\WebKit\WebKitTools\Scripts\webkitpy\layout_tests\run_webkit_tests.py", line 1373, in run num_unexpected_results = test_runner.run(result_summary) File "e:\b\build\slave\Webkit_Win\build\src\third_party\WebKit\WebKitTools\Scripts\webkitpy\layout_tests\run_webkit_tests.py", line 759, in run self._run_tests(self._test_files_list, result_summary)) File "e:\b\build\slave\Webkit_Win\build\src\third_party\WebKit\WebKitTools\Scripts\webkitpy\layout_tests\run_webkit_tests.py", line 639, in _run_tests result_summary) File "e:\b\build\slave\Webkit_Win\build\src\third_party\WebKit\WebKitTools\Scripts\webkitpy\layout_tests\layout_package\dump_render_tree_thread.py", line 330, in _covered_run self._run(test_runner=None, result_summary=None) File "e:\b\build\slave\Webkit_Win\build\src\third_party\WebKit\WebKitTools\Scripts\webkitpy\layout_tests\layout_package\dump_render_tree_thread.py", line 422, in _run result = self._run_test(test_input) File "e:\b\build\slave\Webkit_Win\build\src\third_party\WebKit\WebKitTools\Scripts\webkitpy\layout_tests\layout_package\dump_render_tree_thread.py", line 517, in _run_test self._driver) File "e:\b\build\slave\Webkit_Win\build\src\third_party\WebKit\WebKitTools\Scripts\webkitpy\layout_tests\layout_package\dump_render_tree_thread.py", line 176, in _run_single_test test_output = driver.run_test(uri, test_input.timeout, image_hash_to_driver) File "e:\b\build\slave\Webkit_Win\build\src\third_party\WebKit\WebKitTools\Scripts\webkitpy\layout_tests\port\chromium.py", line 509, in run_test ''.join(output), self._output_image(), actual_checksum, File "e:\b\build\slave\Webkit_Win\build\src\third_party\WebKit\WebKitTools\Scripts\webkitpy\layout_tests\port\chromium.py", line 452, in _output_image with open(png_path, 'rb') as image_file: IOError: [Errno 13] Permission denied: 'e:\\b\\build\\slave\\Webkit_Win\\build\\src\\webkit\\Release\\..\\../../layout-test-results\\png_result1.png' No handlers could be found for logger "webkitpy.common.system"

At first glance, I have no ideas why 'Permission Denied' occurs on only Windows. That might be related with my change. I'll investigate that. (In reply to comment #8) > There have been some failures on WebKit Win after this patch landed. Do you think they might be related? Here's a sample crash: > http://build.chromium.org/p/chromium/builders/Webkit%20Win/builds/1788/steps/webkit_tests/logs/stdio > > 2010-11-18 13:46:53,076 20544 dump_render_tree_thread.py:441 DEBUGThread-1 http/tests/xmlhttprequest/access-control-basic-post-fail-non-simple-content-type.html passed > 2010-11-18 13:46:53,201 20544 executive.py:357 DEBUG"e:\b\build\slave\Webkit_Win\build\src\third_party\cygwin\bin\wdiff.exe --start-delete=##WDIFF_DEL## --end-delete=##WDIFF_END## --start-insert=##WDIFF_ADD## --end-insert=##WDIFF_END## e:\b\build\slave\Webkit_Win\build\src\webkit\Release\..\../../layout-test-results\platform/mac/editing/selection/25228-expected.txt e:\b\build\slave\Webkit_Win\build\src\webkit\Release\..\../../layout-test-results\platform/mac/editing/selection/25228-actual.txt" took 0.06s > 2010-11-18 13:46:53,233 20544 dump_render_tree_thread.py:441 DEBUGThread-1 http/tests/xmlhttprequest/access-control-basic-whitelist-request-headers.html passed > 2010-11-18 13:46:53,390 20544 dump_render_tree_thread.py:441 DEBUGThread-1 http/tests/xmlhttprequest/access-control-basic-whitelist-response-headers.html passed > 2010-11-18 13:46:53,437 20544 executive.py:357 DEBUG"e:\b\build\slave\Webkit_Win\build\src\webkit\Release\image_diff.exe --diff c:\docume~1\chrome~2\locals~1\temp\tmpi4wul2\expected.png c:\docume~1\chrome~2\locals~1\temp\tmpi4wul2\actual.png e:\b\build\slave\Webkit_Win\build\src\webkit\Release\..\../../layout-test-results\platform/mac/editing/selection/25228-diff.png" took 0.22s > 2010-11-18 13:46:53,437 20544 dump_render_tree_thread.py:438 DEBUGThread-4 platform/mac/editing/selection/25228.html failed: > Text diff mismatch > Image mismatch > 2010-11-18 13:46:56,358 20544 dump_render_tree_thread.py:340 ERRORThread-2 dying, exception raised > Traceback (most recent call last): > File "e:\b\build\slave\Webkit_Win\build\src\third_party\WebKit\WebKitTools\Scripts\new-run-webkit-tests", line 38, in <module> > sys.exit(run_webkit_tests.main()) > File "e:\b\build\slave\Webkit_Win\build\src\third_party\WebKit\WebKitTools\Scripts\webkitpy\layout_tests\run_webkit_tests.py", line 1684, in main > return run(port_obj, options, args) > File "e:\b\build\slave\Webkit_Win\build\src\third_party\WebKit\WebKitTools\Scripts\webkitpy\layout_tests\run_webkit_tests.py", line 1373, in run > num_unexpected_results = test_runner.run(result_summary) > File "e:\b\build\slave\Webkit_Win\build\src\third_party\WebKit\WebKitTools\Scripts\webkitpy\layout_tests\run_webkit_tests.py", line 759, in run > self._run_tests(self._test_files_list, result_summary)) > File "e:\b\build\slave\Webkit_Win\build\src\third_party\WebKit\WebKitTools\Scripts\webkitpy\layout_tests\run_webkit_tests.py", line 639, in _run_tests > result_summary) > File "e:\b\build\slave\Webkit_Win\build\src\third_party\WebKit\WebKitTools\Scripts\webkitpy\layout_tests\layout_package\dump_render_tree_thread.py", line 330, in _covered_run > self._run(test_runner=None, result_summary=None) > File "e:\b\build\slave\Webkit_Win\build\src\third_party\WebKit\WebKitTools\Scripts\webkitpy\layout_tests\layout_package\dump_render_tree_thread.py", line 422, in _run > result = self._run_test(test_input) > File "e:\b\build\slave\Webkit_Win\build\src\third_party\WebKit\WebKitTools\Scripts\webkitpy\layout_tests\layout_package\dump_render_tree_thread.py", line 517, in _run_test > self._driver) > File "e:\b\build\slave\Webkit_Win\build\src\third_party\WebKit\WebKitTools\Scripts\webkitpy\layout_tests\layout_package\dump_render_tree_thread.py", line 176, in _run_single_test > test_output = driver.run_test(uri, test_input.timeout, image_hash_to_driver) > File "e:\b\build\slave\Webkit_Win\build\src\third_party\WebKit\WebKitTools\Scripts\webkitpy\layout_tests\port\chromium.py", line 509, in run_test > ''.join(output), self._output_image(), actual_checksum, > File "e:\b\build\slave\Webkit_Win\build\src\third_party\WebKit\WebKitTools\Scripts\webkitpy\layout_tests\port\chromium.py", line 452, in _output_image > with open(png_path, 'rb') as image_file: > IOError: [Errno 13] Permission denied: 'e:\\b\\build\\slave\\Webkit_Win\\build\\src\\webkit\\Release\\..\\../../layout-test-results\\png_result1.png' > No handlers could be found for logger "webkitpy.common.system"