[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc
eric at webkit.org
eric at webkit.org
Wed Dec 22 14:46:19 UTC 2010
The following commit has been merged in the debian/experimental branch:
commit 1fd341b5aa6a2ec79f01ec2bc10fd70972566dc5
Author: eric at webkit.org <eric at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Tue Oct 19 21:19:58 2010 +0000
2010-10-19 Eric Seidel <eric at webkit.org>
Unreviewed. Fixing typos in my previous commit.
Make patch release explicit and not a magic part of "retry" status
https://bugs.webkit.org/show_bug.cgi?id=47909
All of these typos again due to our inability to unit
test much of this code. I added one unit test where
possible. activeworkitems_unittest.py will be in a separate patch.
* QueueStatusServer/handlers/releasepatch.py:
* QueueStatusServer/main.py:
* QueueStatusServer/model/activeworkitems.py:
* QueueStatusServer/model/workitems.py:
* QueueStatusServer/model/workitems_unittest.py:
* QueueStatusServer/templates/releasepatch.html:
* Scripts/webkitpy/common/net/statusserver.py:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@70088 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index abb4735..c116807 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,22 @@
+2010-10-19 Eric Seidel <eric at webkit.org>
+
+ Unreviewed. Fixing typos in my previous commit.
+
+ Make patch release explicit and not a magic part of "retry" status
+ https://bugs.webkit.org/show_bug.cgi?id=47909
+
+ All of these typos again due to our inability to unit
+ test much of this code. I added one unit test where
+ possible. activeworkitems_unittest.py will be in a separate patch.
+
+ * QueueStatusServer/handlers/releasepatch.py:
+ * QueueStatusServer/main.py:
+ * QueueStatusServer/model/activeworkitems.py:
+ * QueueStatusServer/model/workitems.py:
+ * QueueStatusServer/model/workitems_unittest.py:
+ * QueueStatusServer/templates/releasepatch.html:
+ * Scripts/webkitpy/common/net/statusserver.py:
+
2010-10-19 Tony Chang <tony at chromium.org>
Reviewed by Kent Tamura.
@@ -13,6 +32,35 @@
Reviewed by Adam Barth.
+ Make patch release explicit and not a magic part of "retry" status
+ https://bugs.webkit.org/show_bug.cgi?id=47909
+
+ This moves us another step closer to running r+ patches on the EWS bots.
+ Currently all bots just spam /update-work-items with their list of current
+ work items. queues.webkit.org uses that data for display. As part of making
+ the EWS run r+ patches, we're moving the official list of patches-to-process
+ into the server, and feeding them out to bots one at a time. We need to be
+ able to remove patches from the queues one at a time instead of just spamming
+ /update-work-items with a new complete list. That's what this patch adds.
+
+ * QueueStatusServer/handlers/nextpatch.py:
+ * QueueStatusServer/handlers/queuestatus.py:
+ * QueueStatusServer/handlers/releasepatch.py: Added.
+ * QueueStatusServer/handlers/statusbubble_unittest.py:
+ - Fix a typo causing test failure. This was not caught by the bots
+ because they don't have AppEngineLauncher installed and thus don't run
+ the QueueStatusServer tests.
+ * QueueStatusServer/handlers/updatestatus.py:
+ * QueueStatusServer/model/activeworkitems.py:
+ * QueueStatusServer/templates/releasepatch.html: Added.
+ * Scripts/webkitpy/common/net/statusserver.py:
+ * Scripts/webkitpy/tool/commands/queues.py:
+ * Scripts/webkitpy/tool/mocktool.py:
+
+2010-10-19 Eric Seidel <eric at webkit.org>
+
+ Reviewed by Adam Barth.
+
cr-mac bubble has caused status bubbles to wrap
https://bugs.webkit.org/show_bug.cgi?id=47928
diff --git a/WebKitTools/QueueStatusServer/handlers/nextpatch.py b/WebKitTools/QueueStatusServer/handlers/nextpatch.py
index 0571d7d..5f6d71d 100644
--- a/WebKitTools/QueueStatusServer/handlers/nextpatch.py
+++ b/WebKitTools/QueueStatusServer/handlers/nextpatch.py
@@ -35,6 +35,8 @@ from model.queues import Queue
class NextPatch(webapp.RequestHandler):
+ # FIXME: This should probably be a post, or an explict lock_patch
+ # since GET requests shouldn't really modify the datastore.
def get(self, queue_name):
queue = Queue.queue_with_name(queue_name)
if not queue:
diff --git a/WebKitTools/QueueStatusServer/handlers/queuestatus.py b/WebKitTools/QueueStatusServer/handlers/queuestatus.py
index ae2a439..5c31537 100644
--- a/WebKitTools/QueueStatusServer/handlers/queuestatus.py
+++ b/WebKitTools/QueueStatusServer/handlers/queuestatus.py
@@ -30,8 +30,6 @@ from google.appengine.ext import webapp
from google.appengine.ext.webapp import template
from model.queues import Queue
-from model.workitems import WorkItems
-from model.activeworkitems import ActiveWorkItems
from model import queuestatus
diff --git a/WebKitTools/QueueStatusServer/handlers/releasepatch.py b/WebKitTools/QueueStatusServer/handlers/releasepatch.py
new file mode 100644
index 0000000..a6a7175
--- /dev/null
+++ b/WebKitTools/QueueStatusServer/handlers/releasepatch.py
@@ -0,0 +1,64 @@
+# Copyright (C) 2010 Google Inc. All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+# * Redistributions in binary form must reproduce the above
+# copyright notice, this list of conditions and the following disclaimer
+# in the documentation and/or other materials provided with the
+# distribution.
+# * Neither the name of Google Inc. nor the names of its
+# contributors may be used to endorse or promote products derived from
+# this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (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 google.appengine.ext import webapp, db
+from google.appengine.ext.webapp import template
+
+from handlers.updatebase import UpdateBase
+from model.attachment import Attachment
+from model.queues import Queue
+
+
+class ReleasePatch(UpdateBase):
+ def get(self):
+ self.response.out.write(template.render("templates/releasepatch.html", None))
+
+ def post(self):
+ queue_name = self.request.get("queue_name")
+ # FIXME: This queue lookup should be shared between handlers.
+ queue = Queue.queue_with_name(queue_name)
+ if not queue:
+ self.error(404)
+ return
+
+ attachment_id = self._int_from_request("attachment_id")
+ attachment = Attachment(attachment_id)
+ last_status = attachment.status_for_queue(queue)
+ if not last_status:
+ self.error(404)
+ return
+
+ # Ideally we should use a transaction for the calls to
+ # WorkItems and ActiveWorkItems.
+
+ # Only remove it from the queue if the last message is not a retry request.
+ if not last_status.is_retry_request():
+ queue.work_items().remove_work_item(attachment_id)
+
+ # Always release the lock on the item.
+ queue.active_work_items().expire_item(attachment_id)
diff --git a/WebKitTools/QueueStatusServer/handlers/statusbubble_unittest.py b/WebKitTools/QueueStatusServer/handlers/statusbubble_unittest.py
index 81a945f..3ffbdaf 100644
--- a/WebKitTools/QueueStatusServer/handlers/statusbubble_unittest.py
+++ b/WebKitTools/QueueStatusServer/handlers/statusbubble_unittest.py
@@ -53,7 +53,7 @@ class StatusBubbleTest(unittest.TestCase):
# FIXME: assertDictEqual (in Python 2.7) would be better to use here.
self.assertEqual(bubble_dict["name"], "mac")
self.assertEqual(bubble_dict["attachment_id"], 1)
- self.assertEqual(bubble_dict["queue_position"], "1")
+ self.assertEqual(bubble_dict["queue_position"], 1)
self.assertEqual(bubble_dict["state"], "none")
self.assertEqual(bubble_dict["status"], None)
diff --git a/WebKitTools/QueueStatusServer/handlers/updatestatus.py b/WebKitTools/QueueStatusServer/handlers/updatestatus.py
index 408fea9..7301101 100644
--- a/WebKitTools/QueueStatusServer/handlers/updatestatus.py
+++ b/WebKitTools/QueueStatusServer/handlers/updatestatus.py
@@ -31,10 +31,10 @@ from google.appengine.ext import webapp, db
from google.appengine.ext.webapp import template
from handlers.updatebase import UpdateBase
-from model.activeworkitems import ActiveWorkItems
from model.attachment import Attachment
from model.queuestatus import QueueStatus
+
class UpdateStatus(UpdateBase):
def get(self):
self.response.out.write(template.render("templates/updatestatus.html", None))
@@ -59,23 +59,8 @@ class UpdateStatus(UpdateBase):
queue_status.results_file = db.Blob(str(results_file))
return queue_status
- @staticmethod
- def _expire_item(key, item_id):
- active_work_items = db.get(key)
- active_work_items.expire_item(item_id)
- active_work_items.put()
-
- # FIXME: An explicit lock_release request would be cleaner than this magical "Retry" status.
- def _update_active_work_items(self, queue_status):
- if not queue_status.is_retry_request():
- return
- active_items = queue_status.queue.active_work_items()
- # FIXME: This should move onto the ActiveWorkItems class.
- return db.run_in_transaction(self._expire_item, active_items.key(), queue_status.active_patch_id)
-
def post(self):
queue_status = self._queue_status_from_request()
queue_status.put()
- self._update_active_work_items(queue_status)
Attachment.dirty(queue_status.active_patch_id)
self.response.out.write(queue_status.key().id())
diff --git a/WebKitTools/QueueStatusServer/main.py b/WebKitTools/QueueStatusServer/main.py
index c85c9af..3fbee5c 100644
--- a/WebKitTools/QueueStatusServer/main.py
+++ b/WebKitTools/QueueStatusServer/main.py
@@ -40,6 +40,7 @@ from handlers.patch import Patch
from handlers.patchstatus import PatchStatus
from handlers.queuestatus import QueueStatus
from handlers.recentstatus import QueuesOverview
+from handlers.releasepatch import ReleasePatch
from handlers.showresults import ShowResults
from handlers.statusbubble import StatusBubble
from handlers.submittoews import SubmitToEWS
@@ -63,6 +64,7 @@ routes = [
(r'/svn-revision/(.*)', SVNRevision),
(r'/queue-status/(.*)', QueueStatus),
(r'/next-patch/(.*)', NextPatch),
+ (r'/release-patch', ReleasePatch),
('/update-status', UpdateStatus),
('/update-work-items', UpdateWorkItems),
('/update-svn-revision', UpdateSVNRevision),
diff --git a/WebKitTools/QueueStatusServer/model/activeworkitems.py b/WebKitTools/QueueStatusServer/model/activeworkitems.py
index b218e63..ab5d7a6 100644
--- a/WebKitTools/QueueStatusServer/model/activeworkitems.py
+++ b/WebKitTools/QueueStatusServer/model/activeworkitems.py
@@ -57,10 +57,19 @@ class ActiveWorkItems(db.Model, QueuePropertyMixin):
self.item_ids.append(pair[0])
self.item_dates.append(pair[1])
- def expire_item(self, item_id):
+ def _remove_item(self, item_id):
nonexpired_pairs = [pair for pair in self._item_time_pairs() if pair[0] != item_id]
self._set_item_time_pairs(nonexpired_pairs)
+ @staticmethod
+ def _expire_item(key, item_id):
+ active_work_items = db.get(key)
+ active_work_items._remove_item(item_id)
+ active_work_items.put()
+
+ def expire_item(self, item_id):
+ return db.run_in_transaction(self._expire_item, self.key(), item_id)
+
def deactivate_expired(self, now):
one_hour_ago = time.mktime((now - timedelta(minutes=60)).timetuple())
nonexpired_pairs = [pair for pair in self._item_time_pairs() if pair[1] > one_hour_ago]
diff --git a/WebKitTools/QueueStatusServer/model/workitems.py b/WebKitTools/QueueStatusServer/model/workitems.py
index d22e0c2..fae6830 100644
--- a/WebKitTools/QueueStatusServer/model/workitems.py
+++ b/WebKitTools/QueueStatusServer/model/workitems.py
@@ -58,8 +58,9 @@ class WorkItems(db.Model, QueuePropertyMixin):
@staticmethod
def _unguarded_remove(key, attachment_id):
work_items = db.get(key)
- # We should never have more than one entry for a work item, so we only need remove the first.
- work_items.item_ids.remove(attachment_id)
+ if attachment_id in work_items.item_ids:
+ # We should never have more than one entry for a work item, so we only need remove the first.
+ work_items.item_ids.remove(attachment_id)
work_items.put()
def remove_work_item(self, attachment_id):
diff --git a/WebKitTools/QueueStatusServer/model/workitems_unittest.py b/WebKitTools/QueueStatusServer/model/workitems_unittest.py
index b1ff1d3..d53302e 100644
--- a/WebKitTools/QueueStatusServer/model/workitems_unittest.py
+++ b/WebKitTools/QueueStatusServer/model/workitems_unittest.py
@@ -40,6 +40,14 @@ class WorkItemsTest(unittest.TestCase):
self.assertEquals(items.display_position_for_attachment(1), 2)
self.assertEquals(items.display_position_for_attachment(3), None)
+ def test_remove_work_item(self):
+ items = WorkItems()
+ items.item_ids = [0, 1, 2]
+ items.remove_work_item(0)
+ self.assertEqual(items.item_ids, [1, 2])
+ items.remove_work_item(4) # Should not throw
+ self.assertEqual(items.item_ids, [1, 2])
+
if __name__ == '__main__':
unittest.main()
diff --git a/WebKitTools/QueueStatusServer/templates/releasepatch.html b/WebKitTools/QueueStatusServer/templates/releasepatch.html
new file mode 100644
index 0000000..cbd6d6f
--- /dev/null
+++ b/WebKitTools/QueueStatusServer/templates/releasepatch.html
@@ -0,0 +1,3 @@
+<form name="release_patch" enctype="multipart/form-data" method="post">
+Patch to release: <input name="attachment_id"> from <input name="queue_name"><input type="submit" value="Release locks and remove from queue"></div>
+</form>
diff --git a/WebKitTools/Scripts/webkitpy/common/net/statusserver.py b/WebKitTools/Scripts/webkitpy/common/net/statusserver.py
index 95d2f16..bccdbd1 100644
--- a/WebKitTools/Scripts/webkitpy/common/net/statusserver.py
+++ b/WebKitTools/Scripts/webkitpy/common/net/statusserver.py
@@ -107,6 +107,18 @@ class StatusServer:
patch_status_url = "%s/next-patch/%s" % (self.url, queue_name)
return self._fetch_url(patch_status_url)
+ def _post_release_work_item(self, queue_name, patch):
+ release_patch_url = "%s/release-patch/%s" % (self.url, queue_name)
+ self._browser.open(release_patch_url)
+ self._browser.select_form(name="release_patch")
+ self._browser["queue_name"] = queue_name
+ self._browser["attachment_id"] = patch.id()
+ self._browser.submit()
+
+ def release_work_item(self, queue_name, patch):
+ _log.debug("Releasing work item %s from %s" % (patch.id(), queue_name))
+ return NetworkTransaction().run(lambda: self._post_release_work_item(queue_name, patch))
+
def update_work_items(self, queue_name, work_items):
_log.debug("Recording work items: %s for %s" % (work_items, queue_name))
return NetworkTransaction().run(lambda: self._post_work_items_to_server(queue_name, work_items))
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queues.py b/WebKitTools/Scripts/webkitpy/tool/commands/queues.py
index c7866dc..900cbad 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/queues.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/queues.py
@@ -198,6 +198,9 @@ class AbstractPatchQueue(AbstractQueue):
def _fetch_next_work_item(self):
return self._tool.status_server.next_work_item(self.name)
+ def _release_work_item(self, patch):
+ self._tool.status_server.release_work_item(self.name, patch)
+
def _did_pass(self, patch):
self._update_status(self._pass_status, patch)
@@ -247,6 +250,7 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler, CommitQueueTaskD
validator = CommitterValidator(self._tool.bugs)
validator.reject_patch_from_commit_queue(patch.id(), self._error_message_for_bug(task.failure_status_id, e))
self._did_fail(patch)
+ self._release_work_item(patch)
def _error_message_for_bug(self, status_id, script_error):
if not script_error.output:
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
index 0d0db4d..c3c3a42 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
@@ -237,6 +237,7 @@ MOCK: update_status: commit-queue Pass
"process_work_item": """MOCK: update_status: commit-queue Patch does not apply
MOCK setting flag 'commit-queue' to '-' on attachment '197' with comment 'Rejecting patch 197 from commit-queue.' and additional comment 'MOCK script error'
MOCK: update_status: commit-queue Fail
+MOCK: release_work_item: commit-queue 197
""",
"handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '197' with comment 'Rejecting patch 197 from commit-queue.' and additional comment 'Mock error message'\n",
"handle_script_error": "ScriptError error message\n",
@@ -318,6 +319,7 @@ MOCK: update_status: commit-queue Pass
MOCK: update_status: commit-queue Built patch
MOCK: update_status: commit-queue Passed tests
MOCK: update_status: commit-queue Retry
+MOCK: release_work_item: commit-queue 197
"""
OutputCapture().assert_outputs(self, queue.process_work_item, [MockPatch()], expected_stderr=expected_stderr)
diff --git a/WebKitTools/Scripts/webkitpy/tool/mocktool.py b/WebKitTools/Scripts/webkitpy/tool/mocktool.py
index 11b9cad..dd67ff8 100644
--- a/WebKitTools/Scripts/webkitpy/tool/mocktool.py
+++ b/WebKitTools/Scripts/webkitpy/tool/mocktool.py
@@ -547,6 +547,9 @@ class MockStatusServer(object):
return None
return self._work_items[0]
+ def release_work_item(self, queue_name, patch):
+ log("MOCK: release_work_item: %s %s" % (queue_name, patch.id()))
+
def update_work_items(self, queue_name, work_items):
self._work_items = work_items
log("MOCK: update_work_items: %s %s" % (queue_name, work_items))
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list