[Webkit-unassigned] [Bug 144038] Add PerfAutoRun to automate browser based performance benchmarks(e.g. Speedometer and JetStream)

[bugzilla-daemon at webkit.org]

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

--- Comment #6 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 251306
  --> https://bugs.webkit.org/attachment.cgi?id=251306
Patch

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

> Tools/Scripts/PerfAutoRun/README.md:110
> +    * use ```shell git diff --relative HEAD >> your.patch``` to create your patch

what is "shell" ?

Shouldn't it be > your.patch ? so that it actually creates the file?

> Tools/Scripts/PerfAutoRun/README.md:112
> +* Create a plan for the benchmark(refer to **"How to create a plan"** for more details)

Missing space before parenthesis.

> Tools/Scripts/PerfAutoRun/README.md:113
> +* Do following instruction **ONLY IF NEEDED**, in most case, you do not have to.

Follow these instructions...
in most cases,

> Tools/Scripts/PerfAutoRun/Utilities/BenchmarkRunner.py:2
> +# coding : utf-8

Same comment as before.

> Tools/Scripts/PerfAutoRun/Utilities/BenchmarkRunner.py:6
> +import json

Please sort the imports alphabetically.

> Tools/Scripts/PerfAutoRun/Utilities/BenchmarkRunner.py:16
> +from Utils import timeout

Can be merged with the previous from Utils import.

> Tools/Scripts/PerfAutoRun/Utilities/BenchmarkRunner.py:77
> +                # wait for http server to launch and platform handler to clean

Missing capital letter.

> Tools/Scripts/PerfAutoRun/Utilities/BenchmarkRunner.py:78
> +                # the environment

Missing '.' at the end.

> Tools/Scripts/PerfAutoRun/Utilities/BenchmarkRunner.py:94
> +                    logger.error('No result. Something goes wrong')

"went wrong" ?

> Tools/Scripts/PerfAutoRun/Utilities/GenericHandleFactory.py:2
> +# coding : utf-8

Same comment as before.

> Tools/Scripts/PerfAutoRun/Utilities/GenericHandleFactory.py:6
> +import logging

alphabetical sorting.

> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/HTTPHandle.py:2
> +# coding : utf-8

Same comment as before.

> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/HTTPHandle.py:9
> +    def serve(self, webRoot, serveCnt):

What does "serveCnt" stand for? Please avoid abbreviations.

> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/HTTPHandleFactory.py:2
> +# coding : utf-8

Same comment as before.

> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/HTTPServer/NaiveHTTPServer.py:2
> +# coding:utf-8

Same comment as before.

> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/HTTPServer/NaiveHTTPServer.py:7
> +import posixpath

alphabetical sorting.

> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/HTTPServer/NaiveHTTPServer.py:69
> +        self.running = True

isRunning, we like using prefixes for boolean variables.

> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/HTTPServer/TwistedHTTPServer.py:2
> +# coding: utf-8

Same comment as before.

> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/HTTPServer/TwistedHTTPServer.py:28
> +    backdoor = BackdoorPage()

Not sure backdoor is a great name. It sounds scary :)

> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/SimpleHTTPHandle.py:2
> +# coding : utf-8

Same comment as before.

> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/SimpleHTTPHandle.py:24
> +        # FIXME: this may not be reliable

Missing '.' at the end.

> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/SimpleHTTPHandle.py:25
> +        logger.info('finding the ip address of current machine')

"Finding", "IP"

> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/SimpleHTTPHandle.py:32
> +            logger.error('cannot get the ip address of current machine')

Missing capital letter.

> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/SimpleHTTPHandle.py:50
> +        logger.info('start to fetching the port number of the http server')

Missing capital letter.

> Tools/Scripts/PerfAutoRun/Utilities/HTTPHandle/SimpleHTTPHandle.py:88
> +                                'Http Server is serving at port: %d',

HTTP

> Tools/Scripts/PerfAutoRun/Utilities/PlatformHandle/OSXChromeHandle.py:2
> +# coding : utf-8

Same comment as before.

> Tools/Scripts/PerfAutoRun/Utilities/PlatformHandle/OSXSafariHandle.py:35
> +        args = [browserBuildPath + '/Safari.app/Contents/MacOS/Safari']

os.path.join()

> Tools/Scripts/PerfAutoRun/Utilities/PlatformHandle/OSXSafariHandle.py:50
> +        safaris = NSRunningApplication.runningApplicationsWithBundleIdentifier_(

"safariInstances"? or simply "instances"?

> Tools/Scripts/PerfAutoRun/Utilities/PlatformHandle/PlatformHandle.py:2
> +# coding : utf-8

Same comment as before.

> Tools/Scripts/PerfAutoRun/Utilities/PlatformHandle/PlatformHandleFactory.py:2
> +# coding : utf-8

Same comment as before.

> Tools/Scripts/PerfAutoRun/Utilities/ResultWrapper/ResultWrapperFactory.py:2
> +# coding : utf-8

Same comment as before.

> Tools/Scripts/PerfAutoRun/Utilities/Utils.py:2
> +# coding: utf-8

Same comment as before.

> Tools/Scripts/PerfAutoRun/Utilities/Utils.py:25
> +def getPathFromProjRoot(relativePathToProjRoot):

Avoid abbreviations, Proj -> Project

-- 
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/20150422/f263591c/attachment.html>