Bug 82962 – run-webkit-tests with a relative --root causes tests to fail because DYLD_LIBRARY_PATH is not set

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Simon Fraser (smfr)]

On some of our bots we run run-webkit-tests with a --root parameter that is a folder name, rather than an absolute path.

That causes _build_path() to return just the folder name, which is then used to set DYLD_FRAMEWORK_PATH and DYLD_LIBRARY PATH, causing problems.

Its seems that _build_path() should always return an absolute path. Changing the last line to self._filesystem.join(os.getcwd(), build_directory, *comps) seems to work.

[Simon Fraser (smfr)]

Created attachment 135216 [details]
Patch

[Dirk Pranke]

Comment on attachment 135216 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=135216&action=review

> Tools/Scripts/webkitpy/layout_tests/port/webkit.py:378
> +        return self._filesystem.join(os.getcwd(), build_directory, *comps)

This is fine but I'd probably do:

return self._filesystem.join(self.abspath(build_directory), *comps)

instead; that captures the intent a little more clearly (IMO, of course).

[Dirk Pranke]

(In reply to comment #2)
> (From update of attachment 135216 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=135216&action=review
> 
> > Tools/Scripts/webkitpy/layout_tests/port/webkit.py:378
> > +        return self._filesystem.join(os.getcwd(), build_directory, *comps)
> 
> This is fine but I'd probably do:
> 
> return self._filesystem.join(self.abspath(build_directory), *comps)
> 
> instead; that captures the intent a little more clearly (IMO, of course).

Sorry, that should've been:

self._filesystem.join(self._filesystem.abspath(build_directory), *comps)

[Eric Seidel (no email)]

Comment on attachment 135216 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=135216&action=review

>>> Tools/Scripts/webkitpy/layout_tests/port/webkit.py:378
>>> +        return self._filesystem.join(os.getcwd(), build_directory, *comps)
>> 
>> This is fine but I'd probably do:
>> 
>> return self._filesystem.join(self.abspath(build_directory), *comps)
>> 
>> instead; that captures the intent a little more clearly (IMO, of course).
> 
> Sorry, that should've been:
> 
> self._filesystem.join(self._filesystem.abspath(build_directory), *comps)

You want self._filessystem.getcwd() any place you'd use os.getcwd(). :)

[Eric Seidel (no email)]

Comment on attachment 135216 [details]
Patch

r- for the use of os.  Discussing on IRC now. :)

[Simon Fraser (smfr)]

Created attachment 135236 [details]
Patch

[Eric Seidel (no email)]

Created attachment 135245 [details]
Patch

[Mark Rowe (bdash)]

It'd make a heck of a lot more sense to fix that existing issue in a separate patch. It makes it a lot harder to see what in this patch fixes the described issue.

[WebKit Review Bot]

Comment on attachment 135245 [details]
Patch

Clearing flags on attachment: 135245

Committed r112981: <http://trac.webkit.org/changeset/112981>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.