Summary: | garden-o-matic should be able to roll out patches | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adam Barth <abarth> | ||||||||
Component: | Tools / Tests | Assignee: | Adam Barth <abarth> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | ||||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 64183 | ||||||||||
Attachments: |
|
Description
Adam Barth
2011-07-08 11:15:39 PDT
Created attachment 100218 [details]
work in progress
Created attachment 100225 [details]
work in progress
Created attachment 100226 [details]
Patch
Comment on attachment 100226 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=100226&action=review I'm not sure I believe in this config object, and I think tool should be held separate. But the change looks good. > Tools/Scripts/webkitpy/tool/commands/gardenomatic.py:38 > + def _prepare_config(self, options, args, tool): > + return { > + 'tool': tool, > + } Odd. I would think we would pass a tool around anyway as its own argument. > Tools/Scripts/webkitpy/tool/servers/gardeningserver_unittest.py:47 > + def __init__(self, server): > + self.server = server pylint is going to get mad that we don't call the original __init__. YOu shoudl call the original __init__ with mocks from this one. > Tools/Scripts/webkitpy/tool/servers/gardeningserver_unittest.py:58 > + def test_empty_state(self): > + expected_stderr = "MOCK run_command: ['echo', 'rollout', '--force-clean', '--non-interactive', '2314', 'MOCK rollout reason']\n" > + self._post_to_path("/rollout?revision=2314&reason=MOCK+rollout+reason", expected_stderr=expected_stderr) Very slick. Comment on attachment 100226 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=100226&action=review >> Tools/Scripts/webkitpy/tool/servers/gardeningserver_unittest.py:47 >> +# The real GardeningHTTPRequestHandler has a constructor that's too hard to >> +# call in a unit test, so we create a subclass that's easier to constrcut. >> +class TestGardeningHTTPRequestHandler(GardeningHTTPRequestHandler): >> + def __init__(self, server): >> + self.server = server > > pylint is going to get mad that we don't call the original __init__. YOu shoudl call the original __init__ with mocks from this one. As the comment says, the entire point of this class is to avoid calling the original __init__. Comment on attachment 100226 [details] Patch Clearing flags on attachment: 100226 Committed r90762: <http://trac.webkit.org/changeset/90762> All reviewed patches have been landed. Closing bug. |