Changeset 245981 in webkit

Timestamp:

May 31, 2019, 1:52:31 PM (7 years ago)

Author:

Dewei Zhu

Message:

run-benchmark should report an error if the argument to --build-directory is bogus
​https://bugs.webkit.org/show_bug.cgi?id=198316

Reviewed by Ryosuke Niwa.

'run-benchmark' should not fallback to system safari when browser or browser build path is
specified but not valid.
Add a run-time check to ensure at least one of the resource from build directory is opened by
Safari when build directory is specified.

• Scripts/webkitpy/benchmark_runner/browser_driver/osx_safari_driver.py:

(OSXSafariDriver.launch_url): Raise an exception when browser or browser build path is
specified but not valid.

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/benchmark_runner/browser_driver/osx_safari_driver.py (modified) (4 diffs)

trunk/Tools/ChangeLog

@@ +1 @@
+2019-05-28  Dewei Zhu  <dewei_zhu@apple.com>
+
+        run-benchmark should report an error if the argument to --build-directory is bogus
+        https://bugs.webkit.org/show_bug.cgi?id=198316
+
+        Reviewed by Ryosuke Niwa.
+
+        'run-benchmark' should not fallback to system safari when browser or browser build path is
+        specified but not valid.
+        Add a run-time check to ensure at least one of the resource from build directory is opened by
+        Safari when build directory is specified.
+
+        * Scripts/webkitpy/benchmark_runner/browser_driver/osx_safari_driver.py:
+        (OSXSafariDriver.launch_url): Raise an exception when browser or browser build path is
+        specified but not valid.
+
 2019-05-31  Tim Horton  <timothy_horton@apple.com>
 

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

@@ -1 +1 @@
 #!/usr/bin/env python
 
+import itertools
 import logging
 import os
@@ -28 +29 @@
         env = {}
         if browser_build_path:
-            safari_app_in_build_path = os.path.join(browser_build_path, 'Safari.app/Contents/MacOS/Safari')
-            if os.path.exists(safari_app_in_build_path):
+            browser_build_absolute_path = os.path.abspath(browser_build_path)
+            safari_app_in_build_path = os.path.join(browser_build_absolute_path, 'Safari.app/Contents/MacOS/Safari')
+            has_safari_app = os.path.exists(safari_app_in_build_path)
+            content_in_path = os.listdir(browser_build_absolute_path)
+            contains_frameworks = any(itertools.imap(lambda entry: entry.endswith('.framework'), os.listdir(browser_build_absolute_path)))
+
+            if has_safari_app:
                 args = [safari_app_in_build_path]
-                env = {'DYLD_FRAMEWORK_PATH': browser_build_path, 'DYLD_LIBRARY_PATH': browser_build_path, '__XPC_DYLD_FRAMEWORK_PATH': browser_build_path, '__XPC_DYLD_LIBRARY_PATH': browser_build_path}
-            else:
-                _log.info('Could not find Safari.app at %s, using the system Safari instead' % safari_app_in_build_path)
+
+            if contains_frameworks:
+                env = {'DYLD_FRAMEWORK_PATH': browser_build_absolute_path, 'DYLD_LIBRARY_PATH': browser_build_absolute_path,
+                    '__XPC_DYLD_FRAMEWORK_PATH': browser_build_absolute_path, '__XPC_DYLD_LIBRARY_PATH': browser_build_absolute_path}
+            elif not has_safari_app:
+                raise Exception('Could not find any framework "{}"'.format(browser_build_path))
+
         elif browser_path:
             safari_app_in_browser_path = os.path.join(browser_path, 'Contents/MacOS/Safari')
@@ -39 +49 @@
                 args = [safari_app_in_browser_path]
             else:
-                _log.info('Could not find application at %s, using the system Safari instead' % safari_app_in_browser_path)
+                raise Exception('Could not find Safari.app at {}'.format(safari_app_in_browser_path))
 
         args.extend(self._safari_preferences)
@@ -48 +58 @@
         # command may use the system safari.
         time.sleep(3)
+
+        if browser_build_path:
+            _log.info('Checking if any open file is from "{}".'.format(browser_build_path))
+            # Cannot use 'check_call' here as '+D' option will have non-zero return code when not all files under
+            # specified folder are used.
+            process = subprocess.Popen(['/usr/sbin/lsof', '-a', '-p', str(self._safari_process.pid), '+D', browser_build_absolute_path], stdout=subprocess.PIPE)
+            output = process.communicate()[0]
+            if has_safari_app:
+                assert 'Safari.app/Contents/MacOS/Safari' in output, 'Safari.app is not launched from "{}"'.format(browser_build_path)
+            if contains_frameworks:
+                assert '.framework' in output, 'No framework is loaded from "{}"'.format(browser_build_path)
+
         subprocess.Popen(['open', '-a', args[0], url])