Bug 47709 – mocktool.MockOptions shouldn't default values to objects

• Component: New Bugs
• Status: RESOLVED FIXED

[Dirk Pranke]

mocktool.MockOptions shouldn't default values to objects

[Dirk Pranke]

Created attachment 70824 [details]
Patch

[Dirk Pranke]

see ChangeLog for motivation ...

[Eric Seidel (no email)]

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)

[Dirk Pranke]

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

[Dirk Pranke]

Created attachment 70940 [details]
Patch

[Dirk Pranke]

Committed r69905: <http://trac.webkit.org/changeset/69905>