Summary: | mocktool.MockOptions shouldn't default values to objects | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dirk Pranke <dpranke> | ||||||
Component: | New Bugs | Assignee: | Dirk Pranke <dpranke> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, cjerdonek, eric | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Other | ||||||||
OS: | OS X 10.5 | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 47510 | ||||||||
Attachments: |
|
Description
Dirk Pranke
2010-10-14 19:40:20 PDT
Created attachment 70824 [details]
Patch
see ChangeLog for motivation ... Comment on attachment 70824 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=70824&action=review Fantastic! Thanks! > WebKitTools/Scripts/webkitpy/tool/commands/commandtest.py:46 > + options.blocks = True > + options.cc = 'MOCK cc' > + options.component = 'MOCK component' > + options.confirm = True > + options.email = 'MOCK email' > + options.git_commit = 'MOCK git commit' > + options.obsolete_patches = True > + options.open_bug = True > + options.port = 'MOCK port' > + options.quiet = True > + options.reviewer = 'MOCK reviewer' So this was the minimum list required to get the tests to pass I assume? > WebKitTools/Scripts/webkitpy/tool/mocktool.py:587 > +class MockOptions(object): > + def __init__(self, **kwargs): > + for key, value in kwargs.items(): > + self.__dict__[key] = value Very slick! You might want to add a comment here to explain why we don't have any default options here. Maybe we should? Or maybe we should have a WebKitPatchMockOptions which has a bunch of global defaults? (like port = None) (In reply to comment #3) > (From update of attachment 70824 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=70824&action=review > > Fantastic! Thanks! > > > WebKitTools/Scripts/webkitpy/tool/commands/commandtest.py:46 > > + options.blocks = True > > + options.cc = 'MOCK cc' > > + options.component = 'MOCK component' > > + options.confirm = True > > + options.email = 'MOCK email' > > + options.git_commit = 'MOCK git commit' > > + options.obsolete_patches = True > > + options.open_bug = True > > + options.port = 'MOCK port' > > + options.quiet = True > > + options.reviewer = 'MOCK reviewer' > > So this was the minimum list required to get the tests to pass I assume? > Yes. > > WebKitTools/Scripts/webkitpy/tool/mocktool.py:587 > > +class MockOptions(object): > > + def __init__(self, **kwargs): > > + for key, value in kwargs.items(): > > + self.__dict__[key] = value > > Very slick! > > You might want to add a comment here to explain why we don't have any default options here. Maybe we should? Or maybe we should have a WebKitPatchMockOptions which has a bunch of global defaults? (like port = None) I can add a comment. If we're going to reuse MockOptions for different kinds of unit tests (for example, tool and port) then there isn't a single set of defaults that make sense. Various unit tests do have some sets of defaults and wrappers. Created attachment 70940 [details]
Patch
Committed r69905: <http://trac.webkit.org/changeset/69905> |