Diff
Modified: trunk/Tools/ChangeLog (174407 => 174408)
--- trunk/Tools/ChangeLog 2014-10-07 21:35:26 UTC (rev 174407)
+++ trunk/Tools/ChangeLog 2014-10-07 22:14:01 UTC (rev 174408)
@@ -1,3 +1,39 @@
+2014-10-07 Jake Nielsen <jacob_niel...@apple.com>
+
+ Commit queue doesn't drop obsolete patches sometimes
+ https://bugs.webkit.org/show_bug.cgi?id=137460
+
+ Reviewed by Alexey Proskuryakov.
+
+ * Scripts/webkitpy/common/net/bugzilla/bugzilla_mock.py:
+ Adds another test patch for use in queues_unittest.py.
+ courtesy of Csaba Osztrogonác <o...@webkit.org>
+ (MockBugzilla):
+ * Scripts/webkitpy/tool/bot/commitqueuetask.py:
+ Raises a PatchIsNotValid exception in the case of validate() returning
+ false.
+ (CommitQueueTask.run):
+ * Scripts/webkitpy/tool/bot/earlywarningsystemtask.py:
+ Raises a PatchIsNotValid exception in the case of validate() returning
+ false.
+ (EarlyWarningSystemTask.run):
+ * Scripts/webkitpy/tool/bot/patchanalysistask.py:
+ Defines PatchIsNotValid exception.
+ (PatchIsNotValid):
+ (PatchIsNotValid.__init__):
+ * Scripts/webkitpy/tool/commands/earlywarningsystem.py:
+ Remove call to validate() and instead catches the PatchIsNotValid
+ exception.
+ (AbstractEarlyWarningSystem.review_patch):
+ * Scripts/webkitpy/tool/commands/queues.py:
+ Adds logic to catch the PatchIsNotValid exception.
+ (CommitQueue.process_work_item):
+ * Scripts/webkitpy/tool/commands/queues_unittest.py:
+ Adds the test_non_valid_patch test to ensure that invalid patches are
+ handled properly, and don't just result in indefinite spinning.
+ courtesy of Csaba Osztrogonác <o...@webkit.org>
+ (test_non_valid_patch):
+
2014-10-07 Daniel Bates <daba...@apple.com>
[iOS] Teach run-webkit-tests to honor TestExpectation file for iOS Simulator 64-bit builds
Modified: trunk/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla_mock.py (174407 => 174408)
--- trunk/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla_mock.py 2014-10-07 21:35:26 UTC (rev 174407)
+++ trunk/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla_mock.py 2014-10-07 22:14:01 UTC (rev 174408)
@@ -141,6 +141,15 @@
"attacher_email": "e...@webkit.org",
}
+_patch8 = { # Resolved bug, without review flag, not marked obsolete (maybe already landed)
+ "id": 10007,
+ "bug_id": 50005,
+ "url": "http://example.com/10002",
+ "name": "Patch8",
+ "is_obsolete": False,
+ "is_patch": True,
+ "attacher_email": "e...@webkit.org",
+}
# This matches one of Bug.unassigned_emails
_unassigned_email = "webkit-unassig...@lists.webkit.org"
@@ -242,7 +251,7 @@
"reporter_email": _commit_queue_email,
"assigned_to_email": "f...@foo.com",
"cc_emails": [],
- "attachments": [],
+ "attachments": [_patch8],
"bug_status": "RESOLVED",
"comments": [{"comment_date": datetime.datetime(2011, 6, 11, 9, 4, 3),
"comment_email": "b...@foo.com",
@@ -343,7 +352,8 @@
_patch4,
_patch5,
_patch6,
- _patch7)
+ _patch7,
+ _patch8)
def __init__(self):
self.queries = MockBugzillaQueries(self)
Modified: trunk/Tools/Scripts/webkitpy/tool/bot/commitqueuetask.py (174407 => 174408)
--- trunk/Tools/Scripts/webkitpy/tool/bot/commitqueuetask.py 2014-10-07 21:35:26 UTC (rev 174407)
+++ trunk/Tools/Scripts/webkitpy/tool/bot/commitqueuetask.py 2014-10-07 22:14:01 UTC (rev 174408)
@@ -26,7 +26,7 @@
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-from webkitpy.tool.bot.patchanalysistask import PatchAnalysisTask, PatchAnalysisTaskDelegate
+from webkitpy.tool.bot.patchanalysistask import PatchAnalysisTask, PatchAnalysisTaskDelegate, PatchIsNotValid
class CommitQueueTaskDelegate(PatchAnalysisTaskDelegate):
@@ -69,7 +69,7 @@
def run(self):
if not self.validate():
- return False
+ raise PatchIsNotValid(self._patch)
if not self._clean():
return False
if not self._update():
@@ -88,7 +88,7 @@
# Make sure the patch is still valid before landing (e.g., make sure
# no one has set commit-queue- since we started working on the patch.)
if not self.validate():
- return False
+ raise PatchIsNotValid(self._patch)
# FIXME: We should understand why the land failure occurred and retry if possible.
if not self._land():
return self.report_failure()
Modified: trunk/Tools/Scripts/webkitpy/tool/bot/earlywarningsystemtask.py (174407 => 174408)
--- trunk/Tools/Scripts/webkitpy/tool/bot/earlywarningsystemtask.py 2014-10-07 21:35:26 UTC (rev 174407)
+++ trunk/Tools/Scripts/webkitpy/tool/bot/earlywarningsystemtask.py 2014-10-07 22:14:01 UTC (rev 174408)
@@ -26,7 +26,7 @@
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-from webkitpy.tool.bot.patchanalysistask import PatchAnalysisTask, PatchAnalysisTaskDelegate, UnableToApplyPatch
+from webkitpy.tool.bot.patchanalysistask import PatchAnalysisTask, PatchAnalysisTaskDelegate, UnableToApplyPatch, PatchIsNotValid
class EarlyWarningSystemTaskDelegate(PatchAnalysisTaskDelegate):
@@ -50,7 +50,7 @@
def run(self):
if not self.validate():
- return False
+ raise PatchIsNotValid(self._patch)
if not self._clean():
return False
if not self._update():
Modified: trunk/Tools/Scripts/webkitpy/tool/bot/patchanalysistask.py (174407 => 174408)
--- trunk/Tools/Scripts/webkitpy/tool/bot/patchanalysistask.py 2014-10-07 21:35:26 UTC (rev 174407)
+++ trunk/Tools/Scripts/webkitpy/tool/bot/patchanalysistask.py 2014-10-07 22:14:01 UTC (rev 174408)
@@ -36,6 +36,12 @@
self.patch = patch
+class PatchIsNotValid(Exception):
+ def __init__(self, patch):
+ Exception.__init__(self)
+ self.patch = patch
+
+
class PatchAnalysisTaskDelegate(object):
def parent_command(self):
raise NotImplementedError("subclasses must implement")
Modified: trunk/Tools/Scripts/webkitpy/tool/commands/earlywarningsystem.py (174407 => 174408)
--- trunk/Tools/Scripts/webkitpy/tool/commands/earlywarningsystem.py 2014-10-07 21:35:26 UTC (rev 174407)
+++ trunk/Tools/Scripts/webkitpy/tool/commands/earlywarningsystem.py 2014-10-07 22:14:01 UTC (rev 174408)
@@ -37,7 +37,7 @@
from webkitpy.tool.bot.earlywarningsystemtask import EarlyWarningSystemTask, EarlyWarningSystemTaskDelegate
from webkitpy.tool.bot.expectedfailures import ExpectedFailures
from webkitpy.tool.bot.layouttestresultsreader import LayoutTestResultsReader
-from webkitpy.tool.bot.patchanalysistask import UnableToApplyPatch
+from webkitpy.tool.bot.patchanalysistask import UnableToApplyPatch, PatchIsNotValid
from webkitpy.tool.bot.queueengine import QueueEngine
from webkitpy.tool.commands.queues import AbstractReviewQueue
@@ -83,15 +83,14 @@
def review_patch(self, patch):
task = EarlyWarningSystemTask(self, patch, self._options.run_tests)
- if not task.validate():
- self._did_error(patch, "%s did not process patch." % self.name)
- return False
try:
succeeded = task.run()
if not succeeded:
# Caller unlocks when review_patch returns True, so we only need to unlock on transient failure.
self._unlock_patch(patch)
return succeeded
+ except PatchIsNotValid:
+ self._did_error(patch, "%s did not process patch." % self.name)
except UnableToApplyPatch, e:
self._did_error(patch, "%s unable to apply patch." % self.name)
return False
Modified: trunk/Tools/Scripts/webkitpy/tool/commands/queues.py (174407 => 174408)
--- trunk/Tools/Scripts/webkitpy/tool/commands/queues.py 2014-10-07 21:35:26 UTC (rev 174407)
+++ trunk/Tools/Scripts/webkitpy/tool/commands/queues.py 2014-10-07 22:14:01 UTC (rev 174408)
@@ -50,7 +50,7 @@
from webkitpy.tool.bot.feeders import CommitQueueFeeder, EWSFeeder
from webkitpy.tool.bot.flakytestreporter import FlakyTestReporter
from webkitpy.tool.bot.layouttestresultsreader import LayoutTestResultsReader
-from webkitpy.tool.bot.patchanalysistask import UnableToApplyPatch
+from webkitpy.tool.bot.patchanalysistask import UnableToApplyPatch, PatchIsNotValid
from webkitpy.tool.bot.queueengine import QueueEngine, QueueEngineDelegate
from webkitpy.tool.bot.stylequeuetask import StyleQueueTask, StyleQueueTaskDelegate
from webkitpy.tool.commands.stepsequence import StepSequenceErrorHandler
@@ -322,6 +322,8 @@
return True
self._unlock_patch(patch)
return False
+ except PatchIsNotValid:
+ self._did_error(patch, "%s did not process patch." % self.name)
except ScriptError, e:
validator = CommitterValidator(self._tool)
validator.reject_patch_from_commit_queue(patch.id(), self._error_message_for_bug(task, patch, e))
Modified: trunk/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py (174407 => 174408)
--- trunk/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py 2014-10-07 21:35:26 UTC (rev 174407)
+++ trunk/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py 2014-10-07 22:14:01 UTC (rev 174408)
@@ -370,6 +370,18 @@
}
self.assert_queue_outputs(CommitQueue(), tool=tool, work_item=rollout_patch, expected_logs=expected_logs)
+ def test_non_valid_patch(self):
+ tool = MockTool()
+ patch = tool.bugs.fetch_attachment(10007) # _patch8, resolved bug, without review flag, not marked obsolete (maybe already landed)
+ expected_logs = {
+ "begin_work_queue": self._default_begin_work_queue_logs("commit-queue"),
+ "process_work_item": """MOCK: update_status: commit-queue Error: commit-queue did not process patch.
+MOCK: release_work_item: commit-queue 10007
+""",
+ }
+ self.assert_queue_outputs(CommitQueue(), tool=tool, work_item=patch, expected_logs=expected_logs)
+
+
def test_auto_retry(self):
queue = CommitQueue()
options = Mock()
@@ -406,7 +418,8 @@
MOCK: update_status: commit-queue Built patch
Running: webkit-patch --status-host=example.com build-and-test --no-clean --no-update --test --non-interactive --port=mac
MOCK: update_status: commit-queue Passed tests
-MOCK: release_lock: commit-queue 10000
+MOCK: update_status: commit-queue Error: commit-queue did not process patch.
+MOCK: release_work_item: commit-queue 10000
"""
self.maxDiff = None
OutputCapture().assert_outputs(self, queue.process_work_item, [QueuesTest.mock_work_item], expected_logs=expected_logs)
