[Webkit-unassigned] [Bug 147800] Make run-benchmark cleanup more robust and minor code cleaning

[bugzilla-daemon at webkit.org]

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

--- Comment #11 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 258655
  --> https://bugs.webkit.org/attachment.cgi?id=258655
Patch

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

> Tools/Scripts/webkitpy/benchmark_runner/benchmark_runner.py:63
> +        for iteration in xrange(1, int(self._plan['count']) + 1):

I think it's better for the caller of _run_benchmark to take care of reading the values out of plan

> Tools/Scripts/webkitpy/benchmark_runner/benchmark_runner.py:87
> +        _log.info('Start to execute the plan')
> +        _log.info('Start a new benchmark')

These logging seems useless.  Please remove them.

> Tools/Scripts/webkitpy/benchmark_runner/benchmark_runner.py:90
> +        benchmark_builder = BenchmarkBuilder()
> +        self._web_root = benchmark_builder.prepare(self._plan_name, self._plan)

This shouldn't be an instance variable since it's only used within _run_benchmark.
It should be an argument to _run_benchmark instead.

It's also strange that BenchmarkBuilder constructor takes no value and we have to call prepare separately.
We should combine these two and use "with" statement instead of manually calling clean() below.

-- 
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/687e4e64/attachment-0001.html>