Summary: | [iOS] run-webkit-tests runs webkit-build-directory on every test | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Farler <dfarler> | ||||||
Component: | Tools / Tests | Assignee: | David Farler <dfarler> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | ap, commit-queue, dbates, ddkilzer, glenn, ossy, simon.fraser | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | iPhone / iPad | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
David Farler
2014-07-29 21:54:52 PDT
I checked, this script is called only twice on EFL, so it is an iOS specific bug. Maybe mac_config.build_directory call in relay_path causes this problem: http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/port/ios.py?rev=171789#L76 relay_path is only called a handful of times. The culprit is in webkitpy.port.config.Config.build_directory, so unless a port overrides this somehow, it would be affecting everyone. You can check to see if it is eating up time by enabling debug logging in run-webkit-tests with --debug-rwt-logging. I stand corrected - _path_to_image_diff is called for each ref test and I’m constructing a mac Config object in there. Silly. I’ll save the path during construction of the port. Created attachment 235966 [details]
Patch
Comment on attachment 235966 [details]
Patch
Wrong patch, apologies.
Created attachment 236056 [details]
Patch
Comment on attachment 236056 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=236056&action=review > Tools/ChangeLog:10 > + (IOSSimulatorPort.__init__): > + Cache Mac build directory once. Nit: It seems sufficient to collapse these two line into a single line such that it reads: (IOSSimulatorPort.__init__): Cache Mac build directory. (Notice that I removed the word "once" since this is implied by the use of the word "cache"). > Tools/ChangeLog:12 > + (IOSSimulatorPort.relay_path): > + Use cached build directory. Nit: Similarly I would write the contents of these two lines in a single line. > Tools/ChangeLog:14 > + (IOSSimulatorPort._path_to_image_diff): > + Use cached build directory. Ditto. > Tools/Scripts/webkitpy/port/ios.py:67 > + self._mac_config = port_config.Config(self._executive, self._filesystem, 'mac') This instance variable is referenced exactly once (on the line below). Unless you plan to make use of it in subsequent patches in the near future, I suggest we either inline its value into the line below or make it a local variable. Committed r172602: <http://trac.webkit.org/changeset/172602> Re-fixed in commit r180845: <http://trac.webkit.org/changeset/180845> (In reply to comment #9) > Re-fixed in commit r180845: <http://trac.webkit.org/changeset/180845> This regressed with the fix for Bug 140949: [iOS] run-webkit-tests --platform=ios* --lint-test-files does not work https://bugs.webkit.org/show_bug.cgi?id=140949 <http://trac.webkit.org/changeset/179216> |