Bug 49691 – [NRWT] Remove a 'uri' member from TestInput class

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Hayato Ito]

Remove uri member from TestInput class because uri member is not used anywhere.

[Hayato Ito]

Created attachment 74162 [details]
remove-uri

[Hayato Ito]

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().

[Dirk Pranke]

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 :)

[Hayato Ito]

Created attachment 74183 [details]
remove-strip

[Hayato Ito]

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. :)

[Hayato Ito]

Comment on attachment 74183 [details]
remove-strip

Clearing flags on attachment: 74183

Committed r72267: <http://trac.webkit.org/changeset/72267>

[Hayato Ito]

All reviewed patches have been landed.  Closing bug.

[Tony Chang]

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"

[Hayato Ito]

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"