[webkit-reviews] review denied: [Bug 71237] BaselineOptimizer tests should use mocks instead of real Executive/FileSystem objects : [Attachment 113086] Patch

[bugzilla-daemon at webkit.org]

Dirk Pranke <dpranke at chromium.org> has denied Eric Seidel <eric at webkit.org>'s
request for review:
Bug 71237: BaselineOptimizer tests should use mocks instead of real
Executive/FileSystem objects
https://bugs.webkit.org/show_bug.cgi?id=71237

Attachment 113086: Patch
https://bugs.webkit.org/attachment.cgi?id=113086&action=review

------- Additional Comments from Dirk Pranke <dpranke at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=113086&action=review


> Tools/Scripts/webkitpy/common/checkout/baselineoptimizer.py:-34
> -def _baseline_search_hypergraph(fs):

I think it is better to be explicit about the objects that you depend on,
rather than looking for a global context-like object, like host.

> Tools/Scripts/webkitpy/common/checkout/baselineoptimizer.py:47
> +	   port = port_factory.get(port_name)

Why aren't you passing the filesystem to the port any more?

> Tools/Scripts/webkitpy/common/checkout/baselineoptimizer.py:67
> +    def __init__(self, host):

Same comment as above, I think it is better to be explicit about the objects
that you depend on, rather than looking for a global context-like object, like
host.

> Tools/Scripts/webkitpy/common/checkout/baselineoptimizer_unittest.py:39
> +	   BaselineOptimizer.__init__(self, MockHost())

Same comment.