[webkit-reviews] review granted: [Bug 71237] BaselineOptimizer tests should use mocks instead of real Executive/FileSystem objects : [Attachment 113087] Make our PortFactory mocking slightly more explicit

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted  review:
Bug 71237: BaselineOptimizer tests should use mocks instead of real
Executive/FileSystem objects
https://bugs.webkit.org/show_bug.cgi?id=71237

Attachment 113087: Make our PortFactory mocking slightly more explicit
https://bugs.webkit.org/attachment.cgi?id=113087&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=113087&action=review


> Tools/Scripts/webkitpy/common/checkout/baselineoptimizer_unittest.py:35
> +from webkitpy.tool.mocktool import MockHost

We should move MockHost to common now that we've moved Host to common.	As
written, Dirk is correct that importing tool packages in common is a layering
violation.

> Tools/Scripts/webkitpy/common/checkout/baselineoptimizer_unittest.py:59
> +	   host.port_factory = PortFactory(host)  # We want a real PortFactory,
but it should use mocks.

Perhaps that's what the default port_factory on MockHost should be?

> Tools/Scripts/webkitpy/common/checkout/baselineoptimizer_unittest.py:139
> +	   # FIXME: What functionality does this rely on?  When can we remove
this if?

I should have made a more detailed comment.  This can probably be uncommented
if we've succeeded in injecting all the dependencies.

> Tools/Scripts/webkitpy/tool/commands/rebaseline.py:119
> -	   self._baseline_optimizer = BaselineOptimizer(tool.scm(),
tool.filesystem)
> +	   self._baseline_optimizer = BaselineOptimizer(tool)

We should do a global search-and-replace for tool -> host.  It's way past time
for that.

> Tools/Scripts/webkitpy/tool/mocktool.py:842
> -class MockTool(object):
> +class MockHost(object):

This should move to common.