[Webkit-unassigned] [Bug 174331] Add setup script for Web Driver part of Benchmark Runner script

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=174331

--- Comment #13 from dewei_zhu at apple.com ---
Comment on attachment 315857
  --> https://bugs.webkit.org/attachment.cgi?id=315857
Add functions to autoinstaller needed for Benchmark Runner script

View in context: https://bugs.webkit.org/attachment.cgi?id=315857&action=review

> Tools/Scripts/webkitpy/benchmark_runner/browser_driver/linux_chrome_driver.py:28
> +import webkitpy.thirdparty.autoinstalled.chromedriver

As we discussed in person, we should avoid installing those binaries every time we call run-benchmark script.
A lazy installation is preferred.

> Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_chrome_driver.py:22
> +driver_executable = os.path.join(os.path.dirname(os.path.realpath(driver_init_file)), 'chromedriver')

Maybe worth abstract this function into utils.py since this has been called multiple times.

> Tools/Scripts/webkitpy/thirdparty/__init__.py:33
> +import json

Nit: sort import by alphabet.

> Tools/Scripts/webkitpy/thirdparty/__init__.py:170
> +        (url, url_subpath) = self.get_latest_pypi_url('selenium')

No need to change, but worth to mention that braces are not necessary here.

> Tools/Scripts/webkitpy/thirdparty/__init__.py:226
> +def get_driver_filename():
> +    os_name, os_type = get_os_info()
> +    chrome_os, filefox_os = 'unsupported', 'unsupported'
> +    if os_name == 'Linux' and os_type == '64':
> +        chrome_os, firefox_os = 'linux64', 'linux64'
> +    elif os_name == 'Linux':
> +        chrome_os, firefox_os = 'linux32', 'linux32'
> +    elif os_name == 'Darwin':
> +        chrome_os, firefox_os = 'mac64', 'macos'
> +    return (chrome_os, firefox_os)

Would it better to become a function that takes an argument and returns the platform string base on the argument?
This could make it less coupled. Otherwise, we are making an assumption on the order of returned tuple.

> Tools/Scripts/webkitpy/thirdparty/__init__.py:232
> +    return (os_name, os_type)

No need to change, but worth to mention that braces are not necessary here.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170719/7807dbeb/attachment.html>