[chromium] [linux] if check-sys-deps fails, output the failure reason
Created attachment 79339 [details] Patch
Previously, if you didn't have all the right fonts installed on Linux, NRWT would fail with the following output: 2011-01-18 12:25:45,331 13914 metered_stream.py:126 INFO Starting helper ... 2011-01-18 12:25:45,331 13914 metered_stream.py:126 INFO Checking system dependencies ... 2011-01-18 12:25:45,542 13914 executive.py:365 DEBUG "/src/chrome/src/out/Release/DumpRenderTree --check-layout-test-sys-deps" took 0.21s 2011-01-18 12:25:45,543 13914 chromium.py:131 ERROR System dependencies check failed. 2011-01-18 12:25:45,543 13914 chromium.py:132 ERROR To override, invoke with --nocheck-sys-deps 2011-01-18 12:25:45,543 13914 chromium.py:133 ERROR With this change, we include the actual error text. E.g., 2011-01-18 12:25:45,543 13914 chromium.py:134 ERROR You are missing /usr/share/fonts/truetype/ttf-dejavu/DejaVuSans.ttf. Try installing msttcorefonts. Also see http://code.google.com/p/chromium/wiki/LinuxBuildInstructions
Comment on attachment 79339 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=79339&action=review > Tools/Scripts/webkitpy/layout_tests/port/chromium.py:127 > + def error_handler(script_error, error=local_error): Is the error argument necessary (vs. referencing local_error inside directly)? You never actually use it. > Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py:197 > + class MockExecute: Call this MockExecutive? Also, we already have webkitpy.common.system.executive_mock.MockExecutive2 and webkitpy.tool.mocktool.MockExecutive, albeit neither one does things with the error_handler. > Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py:215 > + needs_http = False Not sure why this necessary (vs. using a named argument) > Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py:220 > + class MockLog: webkitpy.common.system.logtesting.LoggingTestCase should let you check that things get logged without having to do your own MockLog class or monkey-patching chromium._log.
Created attachment 79351 [details] Patch
(In reply to comment #3) > (From update of attachment 79339 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=79339&action=review > > > Tools/Scripts/webkitpy/layout_tests/port/chromium.py:127 > > + def error_handler(script_error, error=local_error): > > Is the error argument necessary (vs. referencing local_error inside directly)? You never actually use it. It is necessary because python scoping is weird. See http://www.python.org/dev/peps/pep-3104/. There's only global scope and local scope so this allows us to access local_error without making it global. > > Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py:197 > > + class MockExecute: > > Call this MockExecutive? Also, we already have webkitpy.common.system.executive_mock.MockExecutive2 and webkitpy.tool.mocktool.MockExecutive, albeit neither one does things with the error_handler. I didn't know about those. Switched to using webkitpy.common.system.executive_mock.MockExecutive2 for this test and test_diff_image. > > Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py:215 > > + needs_http = False > > Not sure why this necessary (vs. using a named argument) Switched to a named argument. > > Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py:220 > > + class MockLog: > > webkitpy.common.system.logtesting.LoggingTestCase should let you check that things get logged without having to do your own MockLog class or monkey-patching chromium._log. I didn't know about this either! Switched to using LoggingTestCase.
(In reply to comment #5) > It is necessary because python scoping is weird. See http://www.python.org/dev/peps/pep-3104/. There's only global scope and local scope so this allows us to access local_error without making it global. That says that Python 2.1 moved closer to static nested scoping. The second make_scoreboard example (with the Namespace class) is basically what you're doing, as far as modifying an outer variable (but not assigning to it, which still doesn't work).
Created attachment 79441 [details] Patch
(In reply to comment #6) > (In reply to comment #5) > > It is necessary because python scoping is weird. See http://www.python.org/dev/peps/pep-3104/. There's only global scope and local scope so this allows us to access local_error without making it global. > > That says that Python 2.1 moved closer to static nested scoping. The second make_scoreboard example (with the Namespace class) is basically what you're doing, as far as modifying an outer variable (but not assigning to it, which still doesn't work). You are absolutely correct! Sorry for not trying it. Updated to just access local_error directly.
Committed r76134: <http://trac.webkit.org/changeset/76134>