Bug 87413
| Summary: | webkitpy.layout_tests.port.base.get_option should not return None if a default argument is supplied | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | jochen |
| Component: | Tools / Tests | Assignee: | Dirk Pranke <dpranke> |
| Status: | RESOLVED WONTFIX | ||
| Severity: | Normal | CC: | abarth, dpranke, eric, ojan, thorton, tony |
| Priority: | P2 | ||
| Version: | 528+ (Nightly build) | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Bug Depends on: | 89135 | ||
| Bug Blocks: | |||
jochen
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
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
Yes please.
Dirk Pranke
I'll try to get to this tomorrow ... sigh ...
Dirk Pranke
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
closing this as WONTFIX (although the reason code is debatable ...)