[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:52:17 UTC 2010
The following commit has been merged in the debian/experimental branch:
commit f348e747d513c5993419e24d073c14e6afe4d86e
Author: eric at webkit.org <eric at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Fri Oct 22 20:19:20 2010 +0000
2010-10-22 Eric Seidel <eric at webkit.org>
Reviewed by Adam Barth.
EWS does not need to process obsolete patches
https://bugs.webkit.org/show_bug.cgi?id=48093
This was an easy change, but to test it I had to pipe
real Attachment objects into the queue testing system.
Doing so revealed a whole bunch of bugs in our unit tests,
which I fixed as part of this patch.
* Scripts/webkitpy/tool/commands/earlywarningsystem.py:
- This is the actual code change. This will not reduce the
backlog in the EWS queues much, but it will make rejections
much quicker for obsolete patches or closed bugs.
* Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py:
- Test my new code.
- Used a real attachment object and got rid of MockPatch
- Shared code between the mac-ews and cr-mac-ews tests.
* Scripts/webkitpy/tool/commands/queues_unittest.py:
- Can't use MockPatch anymore.
- Removing MockPatch found more bugs here!
* Scripts/webkitpy/tool/commands/queuestest.py:
* Scripts/webkitpy/tool/commands/sheriffbot_unittest.py:
* Scripts/webkitpy/tool/mocktool.py:
- MockBugzilla should not be a "Mock" object. Right now tool.bugs()
is allowed, but wrong. Making it not a Mock will make tool.bugs() correctly fail.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@70328 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index a91fe85..d3ef50f 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,32 @@
+2010-10-22 Eric Seidel <eric at webkit.org>
+
+ Reviewed by Adam Barth.
+
+ EWS does not need to process obsolete patches
+ https://bugs.webkit.org/show_bug.cgi?id=48093
+
+ This was an easy change, but to test it I had to pipe
+ real Attachment objects into the queue testing system.
+ Doing so revealed a whole bunch of bugs in our unit tests,
+ which I fixed as part of this patch.
+
+ * Scripts/webkitpy/tool/commands/earlywarningsystem.py:
+ - This is the actual code change. This will not reduce the
+ backlog in the EWS queues much, but it will make rejections
+ much quicker for obsolete patches or closed bugs.
+ * Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py:
+ - Test my new code.
+ - Used a real attachment object and got rid of MockPatch
+ - Shared code between the mac-ews and cr-mac-ews tests.
+ * Scripts/webkitpy/tool/commands/queues_unittest.py:
+ - Can't use MockPatch anymore.
+ - Removing MockPatch found more bugs here!
+ * Scripts/webkitpy/tool/commands/queuestest.py:
+ * Scripts/webkitpy/tool/commands/sheriffbot_unittest.py:
+ * Scripts/webkitpy/tool/mocktool.py:
+ - MockBugzilla should not be a "Mock" object. Right now tool.bugs()
+ is allowed, but wrong. Making it not a Mock will make tool.bugs() correctly fail.
+
2010-10-22 Sheriff Bot <webkit.review.bot at gmail.com>
Unreviewed, rolling out r70301.
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem.py b/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem.py
index 5ec468e..b433dba 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem.py
@@ -81,6 +81,14 @@ class AbstractEarlyWarningSystem(AbstractReviewQueue):
raise
def review_patch(self, patch):
+ if patch.is_obsolete():
+ self._did_error(patch, "%s does not process obsolete patches." % self.name)
+ return False
+
+ if patch.bug().is_closed():
+ self._did_error(patch, "%s does not process patches on closed bugs." % self.name)
+ return False
+
if not self._build(patch, first_run=True):
if not self._can_build():
return False
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py
index 675df72..448c314 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py
@@ -31,13 +31,17 @@ import os
from webkitpy.thirdparty.mock import Mock
from webkitpy.tool.commands.earlywarningsystem import *
from webkitpy.tool.commands.queuestest import QueuesTest
+from webkitpy.tool.mocktool import MockTool
+
class EarlyWarningSytemTest(QueuesTest):
def test_failed_builds(self):
ews = ChromiumLinuxEWS()
+ ews.bind_to_tool(MockTool())
ews._build = lambda patch, first_run=False: False
ews._can_build = lambda: True
- ews.review_patch(Mock())
+ mock_patch = ews._tool.bugs.fetch_attachment(197)
+ ews.review_patch(mock_patch)
def _default_expected_stderr(self, ews):
string_replacemnts = {
@@ -46,20 +50,29 @@ class EarlyWarningSytemTest(QueuesTest):
"watchers": ews.watchers,
}
expected_stderr = {
- "begin_work_queue": self._default_begin_work_queue_stderr(ews.name, os.getcwd()), # FIXME: Use of os.getcwd() is wrong, should be scm.checkout_root
+ "begin_work_queue": self._default_begin_work_queue_stderr(ews.name, ews._tool.scm().checkout_root),
"handle_unexpected_error": "Mock error message\n",
"next_work_item": "",
"process_work_item": "MOCK: update_status: %(name)s Pass\nMOCK: release_work_item: %(name)s 197\n" % string_replacemnts,
- "handle_script_error": "MOCK: update_status: %(name)s ScriptError error message\nMOCK bug comment: bug_id=142, cc=%(watchers)s\n--- Begin comment ---\nAttachment 197 did not build on %(port)s:\nBuild output: http://dummy_url\n--- End comment ---\n\n" % string_replacemnts,
+ "handle_script_error": "MOCK: update_status: %(name)s ScriptError error message\nMOCK bug comment: bug_id=42, cc=%(watchers)s\n--- Begin comment ---\nAttachment 197 did not build on %(port)s:\nBuild output: http://dummy_url\n--- End comment ---\n\n" % string_replacemnts,
}
return expected_stderr
def _test_ews(self, ews):
+ ews.bind_to_tool(MockTool())
expected_exceptions = {
"handle_script_error": SystemExit,
}
self.assert_queue_outputs(ews, expected_stderr=self._default_expected_stderr(ews), expected_exceptions=expected_exceptions)
+ def _test_committer_only_ews(self, ews):
+ ews.bind_to_tool(MockTool())
+ expected_stderr = self._default_expected_stderr(ews)
+ string_replacemnts = {"name": ews.name}
+ expected_stderr["process_work_item"] = "MOCK: update_status: %(name)s Error: %(name)s cannot process patches from non-committers :(\nMOCK: release_work_item: %(name)s 197\n" % string_replacemnts
+ expected_exceptions = {"handle_script_error": SystemExit}
+ self.assert_queue_outputs(ews, expected_stderr=expected_stderr, expected_exceptions=expected_exceptions)
+
# FIXME: If all EWSes are going to output the same text, we
# could test them all in one method with a for loop over an array.
def test_chromium_linux_ews(self):
@@ -78,19 +91,7 @@ class EarlyWarningSytemTest(QueuesTest):
self._test_ews(EflEWS())
def test_mac_ews(self):
- ews = MacEWS()
- expected_stderr = self._default_expected_stderr(ews)
- expected_stderr["process_work_item"] = "MOCK: update_status: mac-ews Error: mac-ews cannot process patches from non-committers :(\nMOCK: release_work_item: mac-ews 197\n"
- expected_exceptions = {
- "handle_script_error": SystemExit,
- }
- self.assert_queue_outputs(ews, expected_stderr=expected_stderr, expected_exceptions=expected_exceptions)
+ self._test_committer_only_ews(MacEWS())
def test_chromium_mac_ews(self):
- ews = ChromiumMacEWS()
- expected_stderr = self._default_expected_stderr(ews)
- expected_stderr["process_work_item"] = "MOCK: update_status: cr-mac-ews Error: cr-mac-ews cannot process patches from non-committers :(\nMOCK: release_work_item: cr-mac-ews 197\n"
- expected_exceptions = {
- "handle_script_error": SystemExit,
- }
- self.assert_queue_outputs(ews, expected_stderr=expected_stderr, expected_exceptions=expected_exceptions)
+ self._test_committer_only_ews(ChromiumMacEWS())
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
index ea25833..f04b9aa 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
@@ -34,7 +34,7 @@ from webkitpy.common.system.outputcapture import OutputCapture
from webkitpy.thirdparty.mock import Mock
from webkitpy.tool.commands.commandtest import CommandsTest
from webkitpy.tool.commands.queues import *
-from webkitpy.tool.commands.queuestest import QueuesTest, MockPatch
+from webkitpy.tool.commands.queuestest import QueuesTest
from webkitpy.tool.commands.stepsequence import StepSequence
from webkitpy.tool.mocktool import MockTool, MockSCM, MockStatusServer
@@ -51,11 +51,6 @@ class TestFeederQueue(FeederQueue):
_sleep_duration = 0
-class MockRolloutPatch(MockPatch):
- def is_rollout(self):
- return True
-
-
class AbstractQueueTest(CommandsTest):
def test_log_directory(self):
self.assertEquals(TestQueue()._log_directory(), "test-queue-logs")
@@ -263,23 +258,22 @@ MOCK: release_work_item: commit-queue 197
def test_rollout_lands(self):
tool = MockTool(log_executive=True)
tool.buildbot.light_tree_on_fire()
- rollout_patch = MockRolloutPatch()
+ rollout_patch = tool.bugs.fetch_attachment(106) # _patch6, a rollout patch.
+ assert(rollout_patch.is_rollout())
expected_stderr = {
"begin_work_queue": self._default_begin_work_queue_stderr("commit-queue", MockSCM.fake_checkout_root),
"should_proceed_with_work_item": "MOCK: update_status: commit-queue Processing rollout patch\n",
"next_work_item": "",
- "process_work_item": """MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'apply-attachment', '--force-clean', '--non-interactive', '--quiet', 197]
+ "process_work_item": """MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'apply-attachment', '--force-clean', '--non-interactive', '--quiet', 106]
MOCK: update_status: commit-queue Applied patch
MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'build', '--no-clean', '--no-update', '--build-style=both', '--quiet']
MOCK: update_status: commit-queue Built patch
-MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'build-and-test', '--no-clean', '--no-update', '--test', '--quiet', '--non-interactive']
-MOCK: update_status: commit-queue Passed tests
-MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'land-attachment', '--force-clean', '--ignore-builders', '--quiet', '--non-interactive', '--parent-command=commit-queue', 197]
+MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'land-attachment', '--force-clean', '--ignore-builders', '--quiet', '--non-interactive', '--parent-command=commit-queue', 106]
MOCK: update_status: commit-queue Landed patch
MOCK: update_status: commit-queue Pass
-MOCK: release_work_item: commit-queue 197
+MOCK: release_work_item: commit-queue 106
""",
- "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_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '106' with comment 'Rejecting patch 106 from commit-queue.' and additional comment 'Mock error message'\n",
"handle_script_error": "ScriptError error message\n",
}
self.assert_queue_outputs(CommitQueue(), tool=tool, work_item=rollout_patch, expected_stderr=expected_stderr)
@@ -310,12 +304,12 @@ 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)
+ OutputCapture().assert_outputs(self, queue.process_work_item, [QueuesTest.mock_work_item], expected_stderr=expected_stderr)
def test_report_flaky_tests(self):
queue = CommitQueue()
queue.bind_to_tool(MockTool())
- expected_stderr = """MOCK bug comment: bug_id=142, cc=None
+ expected_stderr = """MOCK bug comment: bug_id=42, cc=None
--- Begin comment ---
The commit-queue encountered the following flaky tests while processing attachment 197:
@@ -326,7 +320,7 @@ Please file bugs against the tests. The author(s) of the test(s) are abarth at web
--- End comment ---
"""
- OutputCapture().assert_outputs(self, queue.report_flaky_tests, [MockPatch(), ["foo/bar.html", "bar/baz.html"]], expected_stderr=expected_stderr)
+ OutputCapture().assert_outputs(self, queue.report_flaky_tests, [QueuesTest.mock_work_item, ["foo/bar.html", "bar/baz.html"]], expected_stderr=expected_stderr)
def test_layout_test_results(self):
queue = CommitQueue()
@@ -356,7 +350,7 @@ class StyleQueueTest(QueuesTest):
"should_proceed_with_work_item": "MOCK: update_status: style-queue Checking style\n",
"process_work_item": "MOCK: update_status: style-queue Pass\nMOCK: release_work_item: style-queue 197\n",
"handle_unexpected_error": "Mock error message\n",
- "handle_script_error": "MOCK: update_status: style-queue ScriptError error message\nMOCK bug comment: bug_id=142, cc=[]\n--- Begin comment ---\nAttachment 197 did not pass style-queue:\n\nScriptError error message\n\nIf any of these errors are false positives, please file a bug against check-webkit-style.\n--- End comment ---\n\n",
+ "handle_script_error": "MOCK: update_status: style-queue ScriptError error message\nMOCK bug comment: bug_id=42, cc=[]\n--- Begin comment ---\nAttachment 197 did not pass style-queue:\n\nScriptError error message\n\nIf any of these errors are false positives, please file a bug against check-webkit-style.\n--- End comment ---\n\n",
}
expected_exceptions = {
"handle_script_error": SystemExit,
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queuestest.py b/WebKitTools/Scripts/webkitpy/tool/commands/queuestest.py
index 379d380..6455617 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/queuestest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/queuestest.py
@@ -44,25 +44,9 @@ class MockQueueEngine(object):
pass
-class MockPatch():
- def id(self):
- return 197
-
- def bug_id(self):
- return 142
-
- def is_rollout(self):
- return False
-
-
class QueuesTest(unittest.TestCase):
- # Ids match patch1 in mocktool.py
- mock_work_item = Attachment({
- "id": 197,
- "bug_id": 142,
- "name": "Patch",
- "attacher_email": "adam at example.com",
- }, None)
+ # This is _patch1 in mocktool.py
+ mock_work_item = MockTool().bugs.fetch_attachment(197)
def assert_outputs(self, func, func_name, args, expected_stdout, expected_stderr, expected_exceptions):
exception = None
@@ -108,4 +92,4 @@ class QueuesTest(unittest.TestCase):
self.assert_outputs(queue.handle_unexpected_error, "handle_unexpected_error", [work_item, "Mock error message"], expected_stdout, expected_stderr, expected_exceptions)
# Should we have a different function for testing StepSequenceErrorHandlers?
if isinstance(queue, StepSequenceErrorHandler):
- self.assert_outputs(queue.handle_script_error, "handle_script_error", [tool, {"patch": MockPatch()}, ScriptError(message="ScriptError error message", script_args="MockErrorCommand")], expected_stdout, expected_stderr, expected_exceptions)
+ self.assert_outputs(queue.handle_script_error, "handle_script_error", [tool, {"patch": self.mock_work_item}, ScriptError(message="ScriptError error message", script_args="MockErrorCommand")], expected_stdout, expected_stderr, expected_exceptions)
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py
index 32eb016..4db463e 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py
@@ -38,9 +38,10 @@ class SheriffBotTest(QueuesTest):
builder2 = MockBuilder("Builder2")
def test_sheriff_bot(self):
- mock_work_item = MockFailureMap(MockTool().buildbot)
+ tool = MockTool()
+ mock_work_item = MockFailureMap(tool.buildbot)
expected_stderr = {
- "begin_work_queue": self._default_begin_work_queue_stderr("sheriff-bot", os.getcwd()),
+ "begin_work_queue": self._default_begin_work_queue_stderr("sheriff-bot", tool.scm().checkout_root),
"next_work_item": "",
"process_work_item": """MOCK: irc.post: abarth, darin, eseidel: http://trac.webkit.org/changeset/29837 might have broken Builder1
MOCK bug comment: bug_id=42, cc=['abarth at webkit.org', 'eric at webkit.org']
diff --git a/WebKitTools/Scripts/webkitpy/tool/mocktool.py b/WebKitTools/Scripts/webkitpy/tool/mocktool.py
index 94f1106..0970930 100644
--- a/WebKitTools/Scripts/webkitpy/tool/mocktool.py
+++ b/WebKitTools/Scripts/webkitpy/tool/mocktool.py
@@ -192,6 +192,7 @@ _bug4 = {
}
+# FIXME: This should not inherit from Mock
class MockBugzillaQueries(Mock):
def __init__(self, bugzilla):
@@ -243,6 +244,7 @@ _mock_reviewer = Reviewer("Foo Bar", "foo at bar.com")
# FIXME: Bugzilla is the wrong Mock-point. Once we have a BugzillaNetwork
# class we should mock that instead.
# Most of this class is just copy/paste from Bugzilla.
+# FIXME: This should not inherit from Mock
class MockBugzilla(Mock):
bug_server_url = "http://example.com"
@@ -420,6 +422,7 @@ class MockBuildBot(object):
return MockFailureMap(self)
+# FIXME: This should not inherit from Mock
class MockSCM(Mock):
fake_checkout_root = os.path.realpath("/tmp") # realpath is needed to allow for Mac OS X's /private/tmp
@@ -573,6 +576,7 @@ class MockStatusServer(object):
return "http://dummy_url"
+# FIXME: This should not inherit from Mock
class MockExecute(Mock):
def __init__(self, should_log):
self._should_log = should_log
@@ -608,7 +612,7 @@ class MockOptions(object):
self.__dict__[key] = value
-class MockRietveld():
+class MockRietveld(object):
def __init__(self, executive, dryrun=False):
pass
@@ -617,25 +621,25 @@ class MockRietveld():
log("MOCK: Uploading patch to rietveld")
-class MockTestPort1():
+class MockTestPort1(object):
def skips_layout_test(self, test_name):
return test_name in ["media/foo/bar.html", "foo"]
-class MockTestPort2():
+class MockTestPort2(object):
def skips_layout_test(self, test_name):
return test_name == "media/foo/bar.html"
-class MockPortFactory():
+class MockPortFactory(object):
def get_all(self, options=None):
return {"test_port1": MockTestPort1(), "test_port2": MockTestPort2()}
-class MockTool():
+class MockTool(object):
def __init__(self, log_executive=False):
self.wakeup_event = threading.Event()
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list