[webkit-reviews] review granted: [Bug 53126] nrwt: add a "mock" DRT implementation for better testing of new-run-webkit-tests : [Attachment 80155] replace --mock-drt flag with mock-* port implementation

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has granted Dirk Pranke <dpranke at chromium.org>'s
request for review:
Bug 53126: nrwt: add a "mock" DRT implementation for better testing of
new-run-webkit-tests
https://bugs.webkit.org/show_bug.cgi?id=53126

Attachment 80155: replace --mock-drt flag with mock-* port implementation
https://bugs.webkit.org/attachment.cgi?id=80155&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=80155&action=review

r=me with the following changes.

> Tools/Scripts/webkitpy/layout_tests/port/mock_drt.py:41
> +import base
> +import factory

This should include the full import path from Scripts.	E.g., see how files in
webkitpy/common/system import other files from webkitpy/common/system.

> Tools/Scripts/webkitpy/layout_tests/port/mock_drt.py:50
> +	   pfx = 'mock-'

pfx -> prefix

> Tools/Scripts/webkitpy/layout_tests/port/mock_drt.py:138
> +class MockDRT(object):
> +    def __init__(self, options, args, stdin, stdout, stderr):

Please add a docstring.  At some point, we may want to move this into a
separate file. Maybe we're not there yet.

> Tools/Scripts/webkitpy/layout_tests/port/mock_drt_unittest.py:38
> +import mock_drt
> +import factory
> +import port_testcase

These should also be imports with the full path.

> Tools/Scripts/webkitpy/layout_tests/port/mock_drt_unittest.py:87
> +	       drt_output = """Content-Type: text/plain
> +%s#EOF
> +
> +ActualHash: %s
> +ExpectedHash: %s
> +#EOF
> +""" % (text_output, expected_checksum, expected_checksum)

I find code like this easier to read when the indention is preserved.  E.g.,
    drt_output = (
	"Content-Type: text/plain\n"
	"%s#EOF\n"
	... ") % (text_output, expected_checksum, expected_checksum)

> Tools/Scripts/webkitpy/layout_tests/port/mock_drt_unittest.py:102
> +	   res = drt.run()

res -> result