Changeset 184044 in webkit

Timestamp:

May 9, 2015, 11:46:33 PM (11 years ago)

Author:

rniwa@webkit.org

Message:

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.

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/run-benchmark (modified) (3 diffs)
• Scripts/webkitpy/benchmark_runner/benchmark_runner.py (modified) (1 diff)
• Scripts/webkitpy/benchmark_runner/browser_driver/browser_driver_factory.py (modified) (1 diff)
• Scripts/webkitpy/benchmark_runner/browser_driver/osx_chrome_driver.py (modified) (1 diff)
• Scripts/webkitpy/benchmark_runner/browser_driver/osx_safari_driver.py (modified) (1 diff)

trunk/Tools/ChangeLog

@@ +1 @@
+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>
 

trunk/Tools/Scripts/run-benchmark

@@ -3 +3 @@
 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
 
 
@@ -19 +21 @@
     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)
@@ -36 +38 @@
     return runner.execute()
 
+
 if __name__ == '__main__':
     sys.exit(main())

trunk/Tools/Scripts/webkitpy/benchmark_runner/benchmark_runner.py

@@ -27 +27 @@
         _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):

trunk/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/browser_driver_factory.py

@@ -15 +15 @@
 
     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

trunk/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_chrome_driver.py

@@ -20 +20 @@
         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

trunk/Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_safari_driver.py

@@ -24 +24 @@
         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.