[webkit-reviews] review granted: [Bug 100030] run-perf-tests should have a --repeat option : [Attachment 191921] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Glenn Adams <glenn at skynav.com>'s
request for review:
Bug 100030: run-perf-tests should have a --repeat option
https://bugs.webkit.org/show_bug.cgi?id=100030

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

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=191921&action=review


> Tools/ChangeLog:9
> +	   Add --repeat option to run-perf-tests, with default value of 1,
which runs
> +	   test set repeat count times then uploads and shows results as
appropriate.

I have a hard time following the phrase "which runs test set repeat count
times". Revise?

> Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:182
> +	   runCount = 0

Please don't use camelCase in PEP8 styled code. Use run_count. instead.

> Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:188
> +	       runs = ' [Run %d of %d]' % (runCount, repeat) if repeat > 1 else
''

I would use regular parenthesis instead of square brackets unless there is a
compelling reason not to.

> Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:251
> +	       results_page_path =
self._host.filesystem.splitext(output_json_path)[0] + '.html'

I don't want to se this code duplicated in two places. Maybe add
_results_page_path() ?