[Webkit-unassigned] [Bug 90284] [GTK] WebKit test runner ignores all system environment variables

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=90284


Dirk Pranke <dpranke at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #152729|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #15 from Dirk Pranke <dpranke at chromium.org>  2012-07-17 12:01:44 PST ---
(From update of attachment 152729)
View in context: https://bugs.webkit.org/attachment.cgi?id=152729&action=review

> Tools/Scripts/webkitpy/layout_tests/port/base.py:787
> +        for string_variable in self._options.additional_env_var:

You should probably use self.get_option('additional_env_var', []) here; that should eliminate most (if not all) of the concerns about having to ensure additonal_env_var is defined in the options object.

> Tools/Scripts/webkitpy/layout_tests/port/base_unittest.py:62
> +        return Port(host, config=port_config, options=port_options)

Why don't you need to propagate the other **kwargs ?

> Tools/Scripts/webkitpy/layout_tests/servers/http_server_unittest.py:37
> +from webkitpy.tool.mocktool import MockOptions

nit: I'm trying to move away from using MockOptions. import optparse and use optparse.Values() instead, since it's a built-in class in the standard library that does the same thing.

> Tools/Scripts/webkitpy/tool/mocktool.py:51
> +        self.ensure_value('additional_env_var', [])

don't do this, it's a bad way of letting additional hidden dependencies creep in. tests that need this value to be set should be explicit about it (yes, that's kind of a hassle).

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.