Extend run-benchmark script capability to supoort mobile device

• Component: New Bugs
• Status: RESOLVED FIXED

Extend run-benchmark script capability to supoort mobile device

Created attachment 255143 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 255143 [details]
Patch

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

> Tools/ChangeLog:7
> +

You should explain what kind of code changes you're making here.
See other change log entires.

> Tools/Scripts/run-benchmark:14
>  _log.addHandler(ch)

This logging code should also be moved into some helper function in run_benchmark.py

> Tools/Scripts/webkitpy/benchmark_runner/benchmark_runner.py:29
> +        _log.info('Initializing benchmark runner')

This logging is probably not useful. Remove it?

> Tools/Scripts/webkitpy/benchmark_runner/browser_driver/browser_driver.py:9
> -    def prepareEnv(self):
> +    def prepareEnv(self, udid=None):

Why don't we stick with the same "deviceID"?

> Tools/Scripts/webkitpy/benchmark_runner/run_benchmark.py:13
> +# _log.setLevel(logging.INFO)

What's going on with these commented out code?
r- because of this.

Created attachment 255159 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 255159 [details]
Patch

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

> Tools/ChangeLog:8
> +        Extend capability of run-benchmark to support mobile device

You need a period after the sentence.

> Tools/ChangeLog:11
> +        Correct typo.
> +        * Scripts/run-benchmark:

You need a blank line between the line line of the description and the list of files.

> Tools/Scripts/webkitpy/benchmark_runner/browser_driver/browser_driver.py:9
> -    def prepareEnv(self):
> +    def prepareEnv(self, deviceID=None):

We don't need default arguments in these functions because we always call it with deviceID.

Created attachment 255162 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 255162 [details]
Patch

Clearing flags on attachment: 255162

Committed r185732: <http://trac.webkit.org/changeset/185732>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.