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
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.