Add support to run-benchmark to use non-default copies of the browser apps
Created attachment 357763 [details] Patch
<rdar://problem/46845840>
Comment on attachment 357763 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=357763&action=review > Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_safari_driver.py:40 > + elif self._browser_path: It's strange that launch_url takes browser_build_path as an argument but keep '_browser_path' as an instance variable. It seems cleaner to me to change the signature of launch_url to also take browser_path as an extra argument instead of changing the contractor of browser drivers. What do you think? > Tools/Scripts/webkitpy/benchmark_runner/run_benchmark.py:47 > + parser.add_argument('--browser-path', help='Specify the path to a non-default copy of the target browser as a path to the .app.') This may need to be mutual exclusive with '--build-directory'. Refer: https://docs.python.org/2.7/library/argparse.html#argparse.ArgumentParser.add_mutually_exclusive_group
I agree with Dewei's comment about being consistent with browser_path and build_dir. I still think it's a bad idea to add this as an option. Using a bundle for performance testing can bite you in weird ways.
Comment on attachment 357763 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=357763&action=review > Tools/ChangeLog:10 > + Add support for a --browser-path argument to run-benchmark, which allows you to use > + a custom app bundle for a given browser (only implemented for Safari at present). We already have --build-directory? How is this option different from that?
--build-directory doesn't let me point at a custom Safari.app (e.g. an STP).
If you build the binary locally, --build-directory is expecting a folder than contains Safari.app. Also this folder should contain all related frameworks.
I want to run against pre-built app bundles (spades). Yes, I'm aware of the perf implications.
Created attachment 357879 [details] Patch
Created attachment 357883 [details] Patch
Comment on attachment 357883 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=357883&action=review > Tools/Scripts/webkitpy/benchmark_runner/run_benchmark.py:49 > + > + group = parser.add_mutually_exclusive_group() > + group.add_argument('--browser-path', help='Specify the path to a non-default copy of the target browser as a path to the .app.') > + group.add_argument('--build-directory', dest='build_dir', help='Path to the browser executable (e.g. WebKitBuild/Release/).') > + Nit: It seems cleaner if we move this section below the last time we invoke parser.add_argument.
https://trac.webkit.org/changeset/239522/webkit