[Webkit-unassigned] [Bug 147800] Use python 'with' statement to make cleanup more robust and minor code cleaning
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sun Aug 9 18:07:57 PDT 2015
https://bugs.webkit.org/show_bug.cgi?id=147800
--- Comment #7 from dewei_zhu at apple.com ---
Comment on attachment 258543
--> https://bugs.webkit.org/attachment.cgi?id=258543
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=258543&action=review
>>>>> Tools/Scripts/webkitpy/benchmark_runner/benchmark_runner.py:108
>>>>> result = self._http_server_driver.fetch_result()
>>>>
>>>> It looks like _browser_driver is not accessed anywhere else so I don't think it needs to be be on _runner at all.
>>>> Ideally, I'd like see the code like this here:
>>>>
>>>> with TestEnvironment(benchmark) as env, timeout(self._plan['timeout'])
>>>> result = env.fetch_results()
>>>
>>> Browser driver is specified by platform and browser which are provided by the parameter of BenchmarkRunner.__init__. The solution I can think of currently is storing those values in a structure, and supplying this structure as parameter of TextEnvironment.__init__ . Does that sounds good?
>>
>> In that case, I don't think we want test_environment object all.
>> It's better for both browser_driver and http_server_driver to independently implement __enter__ and __exit__ (without a wrapper class)
>> so that we can directly instantiate those classes in this function.
>> Otherwise, the extra data structures and abstractions will only increase the code complexity.
>
> Yes, we should avoid introducing unnecessary abstractions. The problem with both browser_driver and http_server is that they are instantiated by the factory function, are we expecting something like "with BrowserDriverFactory.create(....), HttpServerDriverFactory.create(....): "?
The purpose of having a 'with' statement is to reuse the code else where. Apparently, we do not reuse this code else where. Since 'with' statement is equivalent to 'try... finally...' block(http://effbot.org/zone/python-with-statement.htm), we may just use that instead.
--
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150810/5b2d0e4f/attachment.html>
More information about the webkit-unassigned
mailing list