[webkit-reviews] review granted: [Bug 86696] [Performance test] Support "description" for PerfTestRunner.run and PerfTestRunner.runPerSecond : [Attachment 142394] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Kentaro Hara
<haraken at chromium.org>'s request for review:
Bug 86696: [Performance test] Support "description" for PerfTestRunner.run and
PerfTestRunner.runPerSecond
https://bugs.webkit.org/show_bug.cgi?id=86696

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

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


> Tools/ChangeLog:16
> +	   DESCRIPTION: Benchmark for DOM attributes that return a Node object.


Nice!

> Tools/Scripts/webkitpy/performance_tests/perftest.py:98
> +	   description_regex = re.compile(r'^description (?P<description>.*)$')


We should probably ignore cases here :)

> PerformanceTests/resources/runner.js:97
> +	   this.log("description " + description);

Why don't we capitalize description and put a comma as in: "Description: ~~~"

> PerformanceTests/resources/runner.js:126
> +	   this.logStatistics(this._results, this._description);

It seems odd that a function named logStatistics also prints description. Why
don't we put the code here instead?