Changeset 101834 in webkit
- Timestamp:
- Dec 2, 2011 11:10:30 AM (12 years ago)
- Location:
- trunk/Tools
- Files:
-
- 12 edited
- 1 copied
Legend:
- Unmodified
- Added
- Removed
-
trunk/Tools/ChangeLog
r101832 r101834 1 2011-12-01 Eric Seidel <eric@webkit.org> 2 3 Reviewed by Adam Barth. 4 5 webkit-patch post, post-commits, upload should warn when posting to a closed bug, and offer to reopen it 6 https://bugs.webkit.org/show_bug.cgi?id=32006 7 8 I decided not to make it warn, and just have it re-open the bug. 9 That's not that different from today's behavior which will 10 just silently attach the patch. 11 12 This patch makes behavior between upload and land-safely consistent 13 (previously one would assign patches and the other would not) 14 as well as adds the ability for both to ensure that the bug is open. 15 16 To test this I had to add a few more methods to MockBugzilla which 17 (positively) affected a few other test results. 18 19 I also made AbstractStep keep a cached copy of the Bug object 20 and used the cached copy where appropriate (including for 'bug_title'). 21 This should reduce the number of bug fetches we perform. 22 23 * Scripts/webkitpy/tool/commands/download_unittest.py: 24 * Scripts/webkitpy/tool/commands/upload.py: 25 * Scripts/webkitpy/tool/commands/upload_unittest.py: 26 * Scripts/webkitpy/tool/mocktool.py: 27 * Scripts/webkitpy/tool/steps/__init__.py: 28 * Scripts/webkitpy/tool/steps/abstractstep.py: 29 * Scripts/webkitpy/tool/steps/closebug.py: 30 * Scripts/webkitpy/tool/steps/ensurebugisopenandassigned.py: Added. 31 * Scripts/webkitpy/tool/steps/postdiff.py: 32 * Scripts/webkitpy/tool/steps/postdiffforcommit.py: 33 * Scripts/webkitpy/tool/steps/preparechangelog.py: 34 * Scripts/webkitpy/tool/steps/steps_unittest.py: 35 * Scripts/webkitpy/tool/steps/updatechangelogswithreviewer.py: 36 1 37 2011-12-02 Gustavo Noronha Silva <gns@gnome.org> 2 38 -
trunk/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla_mock.py
r99140 r101834 389 389 390 390 def reopen_bug(self, bug_id, message): 391 pass391 log("MOCK reopen_bug %s with comment '%s'" % (bug_id, message)) 392 392 393 393 def close_bug_as_fixed(self, bug_id, message): -
trunk/Tools/Scripts/webkitpy/tool/commands/download_unittest.py
r101266 r101834 243 243 Building WebKit 244 244 Committed r49824: <http://trac.webkit.org/changeset/49824> 245 MOCK reopen_bug 50000 with comment 'Reverted r852 for reason: 246 247 Reason 248 249 Committed r49824: <http://trac.webkit.org/changeset/49824>' 245 250 """ 246 251 self.assert_execute_outputs(Rollout(), [852, "Reason"], options=self._default_options(), expected_stderr=expected_stderr) -
trunk/Tools/Scripts/webkitpy/tool/commands/upload.py
r97553 r101834 217 217 steps.ObsoletePatches, 218 218 steps.SuggestReviewers, 219 steps.EnsureBugIsOpenAndAssigned, 219 220 steps.PostDiff, 220 221 ] … … 234 235 steps.ValidateChangeLogs, 235 236 steps.ObsoletePatches, 237 steps.EnsureBugIsOpenAndAssigned, 236 238 steps.PostDiffForCommit, 237 239 ] … … 268 270 steps.ObsoletePatches, 269 271 steps.SuggestReviewers, 272 steps.EnsureBugIsOpenAndAssigned, 270 273 steps.PostDiff, 271 274 ] -
trunk/Tools/Scripts/webkitpy/tool/commands/upload_unittest.py
r97504 r101834 96 96 97 97 def test_land_safely(self): 98 expected_stderr = "Obsoleting 2 old patches on bug 50000\nMOCK add_patch_to_bug: bug_id=50000, description=Patch for landing, mark_for_review=False, mark_for_commit_queue=False, mark_for_landing=True\n"98 expected_stderr = "Obsoleting 2 old patches on bug 50000\nMOCK reassign_bug: bug_id=50000, assignee=None\nMOCK add_patch_to_bug: bug_id=50000, description=Patch for landing, mark_for_review=False, mark_for_commit_queue=False, mark_for_landing=True\n" 99 99 self.assert_execute_outputs(LandSafely(), [50000], expected_stderr=expected_stderr) 100 100 -
trunk/Tools/Scripts/webkitpy/tool/steps/__init__.py
r96444 r101834 43 43 from webkitpy.tool.steps.createbug import CreateBug 44 44 from webkitpy.tool.steps.editchangelog import EditChangeLog 45 from webkitpy.tool.steps.ensurebugisopenandassigned import EnsureBugIsOpenAndAssigned 45 46 from webkitpy.tool.steps.ensurelocalcommitifneeded import EnsureLocalCommitIfNeeded 46 47 from webkitpy.tool.steps.obsoletepatches import ObsoletePatches … … 49 50 from webkitpy.tool.steps.postdiffforcommit import PostDiffForCommit 50 51 from webkitpy.tool.steps.postdiffforrevert import PostDiffForRevert 52 from webkitpy.tool.steps.preparechangelog import PrepareChangeLog 51 53 from webkitpy.tool.steps.preparechangelogfordepsroll import PrepareChangeLogForDEPSRoll 52 54 from webkitpy.tool.steps.preparechangelogforrevert import PrepareChangeLogForRevert 53 from webkitpy.tool.steps.preparechangelog import PrepareChangeLog54 55 from webkitpy.tool.steps.promptforbugortitle import PromptForBugOrTitle 55 56 from webkitpy.tool.steps.reopenbugafterrollout import ReopenBugAfterRollout … … 57 58 from webkitpy.tool.steps.runtests import RunTests 58 59 from webkitpy.tool.steps.suggestreviewers import SuggestReviewers 60 from webkitpy.tool.steps.update import Update 59 61 from webkitpy.tool.steps.updatechangelogswithreviewer import UpdateChangeLogsWithReviewer 60 62 from webkitpy.tool.steps.updatechromiumdeps import UpdateChromiumDEPS 61 from webkitpy.tool.steps.update import Update62 63 from webkitpy.tool.steps.validatechangelogs import ValidateChangeLogs 63 64 from webkitpy.tool.steps.validatereviewer import ValidateReviewer -
trunk/Tools/Scripts/webkitpy/tool/steps/abstractstep.py
r82771 r101834 41 41 42 42 _well_known_keys = { 43 "bug_title": lambda self, state: self._tool.bugs.fetch_bug(state["bug_id"]).title(), 43 # FIXME: Should this use state.get('bug_id') or state.get('patch').bug_id() like UpdateChangeLogsWithReviewer does? 44 "bug": lambda self, state: self._tool.bugs.fetch_bug(state["bug_id"]), 45 # bug_title can either be a new title given by the user, or one from an existing bug. 46 "bug_title": lambda self, state: self.cached_lookup(state, 'bug').title(), 44 47 "changed_files": lambda self, state: self._tool.scm().changed_files(self._options.git_commit), 45 48 "diff": lambda self, state: self._tool.scm().create_patch(self._options.git_commit, changed_files=self._changed_files(state)), -
trunk/Tools/Scripts/webkitpy/tool/steps/closebug.py
r68496 r101834 44 44 # Check to make sure there are no r? or r+ patches on the bug before closing. 45 45 # Assume that r- patches are just previous patches someone forgot to obsolete. 46 # FIXME: Should this use self.cached_lookup('bug')? It's unclear if 47 # state["patch"].bug_id() always equals state['bug_id']. 46 48 patches = self._tool.bugs.fetch_bug(state["patch"].bug_id()).patches() 47 49 for patch in patches: -
trunk/Tools/Scripts/webkitpy/tool/steps/ensurebugisopenandassigned.py
r101833 r101834 1 1 # Copyright (C) 2010 Google Inc. All rights reserved. 2 # 2 # 3 3 # Redistribution and use in source and binary forms, with or without 4 4 # modification, are permitted provided that the following conditions are 5 5 # met: 6 # 6 # 7 7 # * Redistributions of source code must retain the above copyright 8 8 # notice, this list of conditions and the following disclaimer. … … 14 14 # contributors may be used to endorse or promote products derived from 15 15 # this software without specific prior written permission. 16 # 16 # 17 17 # THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS 18 18 # "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT … … 28 28 29 29 from webkitpy.tool.steps.abstractstep import AbstractStep 30 from webkitpy.tool.steps.options import Options31 from webkitpy.common.system.deprecated_logging import log32 30 33 31 34 class CloseBug(AbstractStep): 35 @classmethod 36 def options(cls): 37 return AbstractStep.options() + [ 38 Options.close_bug, 39 ] 32 class EnsureBugIsOpenAndAssigned(AbstractStep): 33 def run(self, state): 34 bug = self.cached_lookup(state, "bug") 35 if bug.is_unassigned(): 36 self._tool.bugs.reassign_bug(bug.id()) 40 37 41 def run(self, state): 42 if not self._options.close_bug: 43 return 44 # Check to make sure there are no r? or r+ patches on the bug before closing. 45 # Assume that r- patches are just previous patches someone forgot to obsolete. 46 patches = self._tool.bugs.fetch_bug(state["patch"].bug_id()).patches() 47 for patch in patches: 48 if patch.review() == "?" or patch.review() == "+": 49 log("Not closing bug %s as attachment %s has review=%s. Assuming there are more patches to land from this bug." % (patch.bug_id(), patch.id(), patch.review())) 50 return 51 self._tool.bugs.close_bug_as_fixed(state["patch"].bug_id(), "All reviewed patches have been landed. Closing bug.") 38 if bug.is_closed(): 39 # FIXME: We should probably pass this message in somehow? 40 # Right now this step is only used before PostDiff steps, so this is OK. 41 self._tool.bugs.reopen_bug(bug.id(), "Reopening to attach new patch.") -
trunk/Tools/Scripts/webkitpy/tool/steps/postdiff.py
r84575 r101834 48 48 bug_id = state["bug_id"] 49 49 50 # FIXME: We should find some way of caching the Bug object instead of51 # going back to the network here.52 if self._tool.bugs.fetch_bug(bug_id).is_unassigned():53 self._tool.bugs.reassign_bug(bug_id)54 55 50 self._tool.bugs.add_patch_to_bug(bug_id, diff, description, comment_text=comment_text, mark_for_review=self._options.review, mark_for_commit_queue=self._options.request_commit) 56 51 if self._options.open_bug: -
trunk/Tools/Scripts/webkitpy/tool/steps/preparechangelog.py
r91210 r101834 64 64 if state.get("bug_id"): 65 65 args.append("--bug=%s" % state["bug_id"]) 66 args.append("--description=%s" % self. _tool.bugs.fetch_bug(state["bug_id"]).title())66 args.append("--description=%s" % self.cached_lookup(state, 'bug_title')) 67 67 if self._options.email: 68 68 args.append("--email=%s" % self._options.email) -
trunk/Tools/Scripts/webkitpy/tool/steps/steps_unittest.py
r98687 r101834 32 32 from webkitpy.common.config.ports import WebKitPort 33 33 from webkitpy.tool.mocktool import MockOptions, MockTool 34 from webkitpy.tool.steps.update import Update35 from webkitpy.tool.steps.runtests import RunTests36 from webkitpy.tool.steps.promptforbugortitle import PromptForBugOrTitle37 34 35 from webkitpy.tool import steps 38 36 39 37 class StepsTest(unittest.TestCase): … … 60 58 options.update = True 61 59 expected_stderr = "Updating working directory\n" 62 OutputCapture().assert_outputs(self, self._run_step, [ Update, tool, options], expected_stderr=expected_stderr)60 OutputCapture().assert_outputs(self, self._run_step, [steps.Update, tool, options], expected_stderr=expected_stderr) 63 61 64 62 def test_prompt_for_bug_or_title_step(self): 65 63 tool = MockTool() 66 64 tool.user.prompt = lambda message: 50000 67 self._run_step(PromptForBugOrTitle, tool=tool) 65 self._run_step(steps.PromptForBugOrTitle, tool=tool) 66 67 def _post_diff_options(self): 68 options = self._step_options() 69 options.git_commit = None 70 options.description = None 71 options.comment = None 72 options.review = True 73 options.request_commit = False 74 options.open_bug = True 75 return options 76 77 def _assert_step_output_with_bug(self, step, bug_id, expected_stderr, options=None): 78 state = {'bug_id': bug_id} 79 OutputCapture().assert_outputs(self, self._run_step, [step, MockTool(), options, state], expected_stderr=expected_stderr) 80 81 def _assert_post_diff_output_for_bug(self, step, bug_id, expected_stderr): 82 self._assert_step_output_with_bug(step, bug_id, expected_stderr, self._post_diff_options()) 83 84 def test_post_diff(self): 85 expected_stderr = "MOCK add_patch_to_bug: bug_id=78, description=Patch, mark_for_review=True, mark_for_commit_queue=False, mark_for_landing=False\nMOCK: user.open_url: http://example.com/78\n" 86 self._assert_post_diff_output_for_bug(steps.PostDiff, 78, expected_stderr) 87 88 def test_post_diff_for_commit(self): 89 expected_stderr = "MOCK add_patch_to_bug: bug_id=78, description=Patch for landing, mark_for_review=False, mark_for_commit_queue=False, mark_for_landing=True\n" 90 self._assert_post_diff_output_for_bug(steps.PostDiffForCommit, 78, expected_stderr) 91 92 def test_ensure_bug_is_open_and_assigned(self): 93 expected_stderr = "MOCK reopen_bug 50004 with comment 'Reopening to attach new patch.'\n" 94 self._assert_step_output_with_bug(steps.EnsureBugIsOpenAndAssigned, 50004, expected_stderr) 95 expected_stderr = "MOCK reassign_bug: bug_id=50002, assignee=None\n" 96 self._assert_step_output_with_bug(steps.EnsureBugIsOpenAndAssigned, 50002, expected_stderr) 68 97 69 98 def test_runtests_args(self): 70 99 mock_options = self._step_options() 71 step = RunTests(MockTool(log_executive=True), mock_options)100 step = steps.RunTests(MockTool(log_executive=True), mock_options) 72 101 # FIXME: We shouldn't use a real port-object here, but there is too much to mock at the moment. 73 102 mock_port = WebKitPort() … … 75 104 tool = MockTool(log_executive=True) 76 105 tool.port = lambda: mock_port 77 step = RunTests(tool, mock_options)106 step = steps.RunTests(tool, mock_options) 78 107 expected_stderr = """Running Python unit tests 79 108 MOCK run_and_throw_if_fail: ['Tools/Scripts/test-webkitpy'], cwd=/mock-checkout -
trunk/Tools/Scripts/webkitpy/tool/steps/updatechangelogswithreviewer.py
r95257 r101834 44 44 45 45 def _guess_reviewer_from_bug(self, bug_id): 46 # FIXME: It's unclear if it would be safe to use self.cached_lookup(state, 'bug') 47 # here as we don't currently have a way to invalidate a bug after making changes (like ObsoletePatches does). 46 48 patches = self._tool.bugs.fetch_bug(bug_id).reviewed_patches() 47 49 if not patches:
Note: See TracChangeset
for help on using the changeset viewer.