Make arguments of run-benchmark more user friendly
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 10 May 2015 06:46:33 +0000 (06:46 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 10 May 2015 06:46:33 +0000 (06:46 +0000)
https://bugs.webkit.org/show_bug.cgi?id=144835

Reviewed by Darin Adler.

Made --build-directory optional since I don't expect a typical WebKit developer to have a local build
of Chrome and Firefox. Also made --plan accept just a filename so that we can just say "speedometer"
instead of "Tools/Scripts/webkitpy/benchmark_runner/data/plans/speedometer.plan". Finally, removed
default values from --platform and --browser as they are required arguments.

* Scripts/run-benchmark:
(main): Made --build-directory optional, and removed default values from --platform and --browser.
Also added help text for --build-directory and --plan. In addition, the list of platforms and browsers
are not dynamically obtained via BrowserDriverFactory.
* Scripts/webkitpy/benchmark_runner/benchmark_runner.py:
(BenchmarkRunner.__init__): Raise when we can't find the plan file or the plan file is not a valid JSON
file instead of suppressing the error here and blowing up later mysteriously since we won't be able to
run any benchmark in that case.
(BenchmarkRunner._findPlanFile): Added. Look for the plan in webkitpy/benchmark_runner/data/plans if
the specified file isn't a valid relative or an absolute path.
* Scripts/webkitpy/benchmark_runner/browser_driver/browser_driver_factory.py:
(BrowserDriverFactory.available_platforms): Added. Used in main to provide the list of valid platforms
and browsers.
(BrowserDriverFactory.available_browsers): Ditto.
* Scripts/webkitpy/benchmark_runner/browser_driver/osx_chrome_driver.py:
(OSXChromeDriver.launchUrl): browserBuildPath is never optional since BenchmarkRunner.execute always
calls launchUrl with this argument so removed the default value. Also added a fallback path for when
browserBuildPath was None.
* Scripts/webkitpy/benchmark_runner/browser_driver/osx_safari_driver.py:
(OSXSafariDriver.launchUrl): Ditto. We also fallback when the build directory doesn't contain Safari
so that we can use locally built WebKit to launch Safari.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@184044 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog
Tools/Scripts/run-benchmark
Tools/Scripts/webkitpy/benchmark_runner/benchmark_runner.py
Tools/Scripts/webkitpy/benchmark_runner/browser_driver/browser_driver_factory.py
Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_chrome_driver.py
Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_safari_driver.py

index a8aae08..ac85df3 100644 (file)
@@ -1,3 +1,37 @@
+2015-05-09  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Make arguments of run-benchmark more user friendly
+        https://bugs.webkit.org/show_bug.cgi?id=144835
+
+        Reviewed by Darin Adler.
+
+        Made --build-directory optional since I don't expect a typical WebKit developer to have a local build
+        of Chrome and Firefox. Also made --plan accept just a filename so that we can just say "speedometer"
+        instead of "Tools/Scripts/webkitpy/benchmark_runner/data/plans/speedometer.plan". Finally, removed
+        default values from --platform and --browser as they are required arguments.
+
+        * Scripts/run-benchmark:
+        (main): Made --build-directory optional, and removed default values from --platform and --browser.
+        Also added help text for --build-directory and --plan. In addition, the list of platforms and browsers
+        are not dynamically obtained via BrowserDriverFactory.
+        * Scripts/webkitpy/benchmark_runner/benchmark_runner.py:
+        (BenchmarkRunner.__init__): Raise when we can't find the plan file or the plan file is not a valid JSON
+        file instead of suppressing the error here and blowing up later mysteriously since we won't be able to
+        run any benchmark in that case.
+        (BenchmarkRunner._findPlanFile): Added. Look for the plan in webkitpy/benchmark_runner/data/plans if
+        the specified file isn't a valid relative or an absolute path.
+        * Scripts/webkitpy/benchmark_runner/browser_driver/browser_driver_factory.py:
+        (BrowserDriverFactory.available_platforms): Added. Used in main to provide the list of valid platforms
+        and browsers.
+        (BrowserDriverFactory.available_browsers): Ditto.
+        * Scripts/webkitpy/benchmark_runner/browser_driver/osx_chrome_driver.py:
+        (OSXChromeDriver.launchUrl): browserBuildPath is never optional since BenchmarkRunner.execute always
+        calls launchUrl with this argument so removed the default value. Also added a fallback path for when
+        browserBuildPath was None.
+        * Scripts/webkitpy/benchmark_runner/browser_driver/osx_safari_driver.py:
+        (OSXSafariDriver.launchUrl): Ditto. We also fallback when the build directory doesn't contain Safari
+        so that we can use locally built WebKit to launch Safari.
+
 2015-05-09  Yoav Weiss  <yoav@yoav.ws>
 
         Remove the PICTURE_SIZES build flag
index 30480f8..6d5f66e 100755 (executable)
@@ -2,9 +2,11 @@
 
 import argparse
 import logging
+import platform
 import sys
 
 from webkitpy.benchmark_runner.benchmark_runner import BenchmarkRunner
+from webkitpy.benchmark_runner.browser_driver.browser_driver_factory import BrowserDriverFactory
 
 
 _log = logging.getLogger()
@@ -18,14 +20,14 @@ _log.addHandler(ch)
 def main():
     parser = argparse.ArgumentParser(description='Automate the browser based performance benchmarks')
     parser.add_argument('--output-file', dest='output', default=None)
-    parser.add_argument('--build-directory', dest='buildDir', required=True)
-    parser.add_argument('--plan', dest='plan', required=True)
-    parser.add_argument('--platform', dest='platform', default='osx', choices=['osx', 'ios', 'windows'], required=True)
+    parser.add_argument('--build-directory', dest='buildDir', help='Path to the browser executable. e.g. WebKitBuild/Release/')
+    parser.add_argument('--plan', dest='plan', required=True, help='Benchmark plan to run. e.g. speedometer, jetstream')
+    parser.add_argument('--platform', dest='platform', required=True, choices=BrowserDriverFactory.available_platforms())
     # FIXME: Should we add chrome as an option? Well, chrome uses webkit in iOS.
-    parser.add_argument('--browser', dest='browser', default='safari', choices=['safari', 'chrome'], required=True)
+    parser.add_argument('--browser', dest='browser', required=True, choices=BrowserDriverFactory.available_browsers())
     parser.add_argument('--debug', action='store_true')
     args = parser.parse_args()
-    
+
     if args.debug:
         _log.setLevel(logging.DEBUG)
     _log.debug('Initializing program with following parameters')
@@ -35,5 +37,6 @@ def main():
     runner = BenchmarkRunner(args.plan, args.buildDir, args.output, args.platform, args.browser)
     return runner.execute()
 
+
 if __name__ == '__main__':
     sys.exit(main())
index bafbb7a..179287d 100644 (file)
@@ -26,19 +26,31 @@ class BenchmarkRunner(object):
     def __init__(self, planFile, buildDir, outputFile, platform, browser):
         _log.info('Initializing benchmark running')
         try:
+            planFile = self._findPlanFile(planFile)
             with open(planFile, 'r') as fp:
                 self.plan = json.load(fp)
                 self.browserDriver = BrowserDriverFactory.create([platform, browser])
                 self.httpServerDriver = HTTPServerDriverFactory.create([self.plan['http_server_driver']])
-                self.buildDir = os.path.abspath(buildDir)
+                self.buildDir = os.path.abspath(buildDir) if buildDir else None
                 self.outputFile = outputFile
         except IOError:
             _log.error('Can not open plan file: %s' % planFile)
+            raise
         except ValueError:
             _log.error('Plan file:%s may not follow JSON format' % planFile)
-        except:
             raise
 
+    def _findPlanFile(self, planFile):
+        if not os.path.exists(planFile):
+            absPath = os.path.join(os.path.dirname(__file__), 'data/plans', planFile)
+            if os.path.exists(absPath):
+                return absPath
+            if not absPath.endswith('.plan'):
+                absPath += '.plan'
+            if os.path.exists(absPath):
+                return absPath
+        return planFile
+
     def execute(self):
         _log.info('Start to execute the plan')
         _log.info('Start a new benchmark')
index b998b4f..a140ec0 100644 (file)
@@ -14,3 +14,15 @@ driverFileName = 'browser_drivers.json'
 class BrowserDriverFactory(GenericFactory):
 
     products = loadJSONFromFile(os.path.join(os.path.dirname(__file__), driverFileName))
+
+    @classmethod
+    def available_platforms(cls):
+        return cls.products.keys()
+
+    @classmethod
+    def available_browsers(cls):
+        browsers = []
+        for platform in cls.products.values():
+            for browser in platform:
+                browsers.append(browser)
+        return browsers
index 266319a..878b140 100644 (file)
@@ -19,7 +19,9 @@ class OSXChromeDriver(BrowserDriver):
         self.closeBrowsers()
         self.chromePreferences = []
 
-    def launchUrl(self, url, browserBuildPath=None):
+    def launchUrl(self, url, browserBuildPath):
+        if not browserBuildPath:
+            browserBuildPath = '/Applications/'
         _log.info('Launching chrome: %s with url: %s' % (os.path.join(browserBuildPath, 'Google Chrome.app'), url))
         # FIXME: May need to be modified for develop build, such as setting up libraries
         subprocess.Popen(['open', '-a', os.path.join(browserBuildPath, 'Google Chrome.app'), '--args', '--homepage', url] + self.chromePreferences).communicate()
index fdb906b..62a365a 100644 (file)
@@ -23,11 +23,20 @@ class OSXSafariDriver(BrowserDriver):
         forceRemove(os.path.join(os.path.expanduser('~'), 'Library/Safari/LastSession.plist'))
         self.safariPreferences = ["-HomePage", "about:blank", "-WarnAboutFraudulentWebsites", "0", "-ExtensionsEnabled", "0", "-ShowStatusBar", "0", "-NewWindowBehavior", "1", "-NewTabBehavior", "1"]
 
-    def launchUrl(self, url, browserBuildPath=None):
-        args = [os.path.join(browserBuildPath, 'Safari.app/Contents/MacOS/Safari')]
+    def launchUrl(self, url, browserBuildPath):
+        args = ['/Applications/Safari.app/Contents/MacOS/SafariForWebKitDevelopment']
+        env = {}
+        if browserBuildPath:
+            safariAppInBuildPath = os.path.join(browserBuildPath, 'Safari.app/Contents/MacOS/Safari')
+            if os.path.exists(safariAppInBuildPath):
+                args = [safariAppInBuildPath]
+                env = {'DYLD_FRAMEWORK_PATH': browserBuildPath, 'DYLD_LIBRARY_PATH': browserBuildPath}
+            else:
+                _log.info('Could not find Safari.app at %s, using the system SafariForWebKitDevelopment in /Applications instead' % safariAppInBuildPath)
+
         args.extend(self.safariPreferences)
         _log.info('Launching safari: %s with url: %s' % (args[0], url))
-        self.safariProcess = subprocess.Popen(args, env={'DYLD_FRAMEWORK_PATH': browserBuildPath, 'DYLD_LIBRARY_PATH': browserBuildPath}, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+        self.safariProcess = subprocess.Popen(args, env=env, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
         # Stop for initialization of the safari process, otherwise, open
         # command may use the system safari.
         time.sleep(3)