WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED WONTFIX
87413
webkitpy.layout_tests.port.base.get_option should not return None if a default argument is supplied
https://bugs.webkit.org/show_bug.cgi?id=87413
Summary
webkitpy.layout_tests.port.base.get_option should not return None if a defaul...
jochen
Reported
2012-05-24 12:30:19 PDT
if an option 'foo' is set to None, get_option('foo', 'bar') returns None, but it should return 'bar'
Attachments
Add attachment
proposed patch, testcase, etc.
Dirk Pranke
Comment 1
2012-05-24 12:31:38 PDT
For the record, there was a reason I did it this way, but in retrospect it was probably a dumb reason as it has led to a confusing and unintuitive interface. I need to go back and poke around to try and figure out why I did it this way in the first place.
Tim Horton
Comment 2
2012-06-13 20:56:36 PDT
Yes please.
Dirk Pranke
Comment 3
2012-06-13 20:59:10 PDT
I'll try to get to this tomorrow ... sigh ...
Dirk Pranke
Comment 4
2012-06-14 15:41:43 PDT
Okay, having poked around with this a fair amount, it turns out that the code is doing the "correct" thing. Meaning, if you have: obj.x = None getattr(obj, 'x', 4) == None # is True d = {'x': None} d.get('x', 4) == None # is True i.e., the code is mirroring the normal python behavior. admittedly, this will be confusing on occasion, but I think having it do the opposite is also confusing. In the long run what we probably need is some consistent way to make sure port._options has all the values the port/* classes will need, and then make port.get_option() and port.set_option_default() protected/private functions so that callers of port objects are responsible for their own options management. Fixing this is annoying because, even in NRWT, right now the responsibility for setting default values in options is split between code in run_webkit_tests.py and code in the port packages. I'm going to post a separate patch that cleans up this code slightly, but I wanted people to get a chance to read and comment on this before I spent time refactoring a bunch of code to do what I describe above.
Dirk Pranke
Comment 5
2012-06-15 15:08:25 PDT
closing this as WONTFIX (although the reason code is debatable ...)
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug