87413 2012-05-24 12:30:19 -0700 webkitpy.layout_tests.port.base.get_option should not return None if a default argument is supplied 2012-06-15 15:08:25 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Unspecified Unspecified RESOLVED WONTFIX P2 Normal --- 89135 1 jochen dpranke abarth dpranke eric ojan thorton tony oldest_to_newest 633053 0 jochen 2012-05-24 12:30:19 -0700 if an option 'foo' is set to None, get_option('foo', 'bar') returns None, but it should return 'bar' 633056 1 dpranke 2012-05-24 12:31:38 -0700 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. 648756 2 thorton 2012-06-13 20:56:36 -0700 Yes please. 648759 3 dpranke 2012-06-13 20:59:10 -0700 I'll try to get to this tomorrow ... sigh ... 649554 4 dpranke 2012-06-14 15:41:43 -0700 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. 650558 5 dpranke 2012-06-15 15:08:25 -0700 closing this as WONTFIX (although the reason code is debatable ...)