diff options
Diffstat (limited to 'WebKitTools/Scripts/webkitpy/tool')
19 files changed, 288 insertions, 94 deletions
diff --git a/WebKitTools/Scripts/webkitpy/tool/bot/queueengine.py b/WebKitTools/Scripts/webkitpy/tool/bot/queueengine.py index 2c45ab6..ac7a760 100644 --- a/WebKitTools/Scripts/webkitpy/tool/bot/queueengine.py +++ b/WebKitTools/Scripts/webkitpy/tool/bot/queueengine.py @@ -77,8 +77,8 @@ class QueueEngine: self._output_tee = OutputTee() log_date_format = "%Y-%m-%d %H:%M:%S" - sleep_duration_text = "5 mins" - seconds_to_sleep = 300 + sleep_duration_text = "2 mins" # This could be generated from seconds_to_sleep + seconds_to_sleep = 120 handled_error_code = 2 # Child processes exit with a special code to the parent queue process can detect the error was handled. @@ -142,8 +142,12 @@ class QueueEngine: self._output_tee.remove_log(self._work_log) self._work_log = None + def _now(self): + """Overriden by the unit tests to allow testing _sleep_message""" + return datetime.now() + def _sleep_message(self, message): - wake_time = datetime.now() + timedelta(seconds=self.seconds_to_sleep) + wake_time = self._now() + timedelta(seconds=self.seconds_to_sleep) return "%s Sleeping until %s (%s)." % (message, wake_time.strftime(self.log_date_format), self.sleep_duration_text) def _sleep(self, message): diff --git a/WebKitTools/Scripts/webkitpy/tool/bot/queueengine_unittest.py b/WebKitTools/Scripts/webkitpy/tool/bot/queueengine_unittest.py index 626181d..ec91bdb 100644 --- a/WebKitTools/Scripts/webkitpy/tool/bot/queueengine_unittest.py +++ b/WebKitTools/Scripts/webkitpy/tool/bot/queueengine_unittest.py @@ -26,6 +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. +import datetime import os import shutil import tempfile @@ -160,6 +161,17 @@ class QueueEngineTest(unittest.TestCase): expected_callbacks.append('should_continue_work_queue') self.assertEquals(delegate._callbacks, expected_callbacks) + def test_now(self): + """Make sure there are no typos in the QueueEngine.now() method.""" + engine = QueueEngine("test", None, None) + self.assertTrue(isinstance(engine._now(), datetime.datetime)) + + def test_sleep_message(self): + engine = QueueEngine("test", None, None) + engine._now = lambda: datetime.datetime(2010, 1, 1) + expected_sleep_message = "MESSAGE Sleeping until 2010-01-01 00:02:00 (2 mins)." + self.assertEqual(engine._sleep_message("MESSAGE"), expected_sleep_message) + def setUp(self): self.temp_dir = tempfile.mkdtemp(suffix="work_queue_test_logs") diff --git a/WebKitTools/Scripts/webkitpy/tool/bot/sheriff_unittest.py b/WebKitTools/Scripts/webkitpy/tool/bot/sheriff_unittest.py index dd048a1..c375ff9 100644 --- a/WebKitTools/Scripts/webkitpy/tool/bot/sheriff_unittest.py +++ b/WebKitTools/Scripts/webkitpy/tool/bot/sheriff_unittest.py @@ -33,7 +33,7 @@ from webkitpy.common.net.buildbot import Builder from webkitpy.common.system.outputcapture import OutputCapture from webkitpy.thirdparty.mock import Mock from webkitpy.tool.bot.sheriff import Sheriff -from webkitpy.tool.mocktool import MockTool, mock_builder +from webkitpy.tool.mocktool import MockTool class MockSheriffBot(object): diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/download.py b/WebKitTools/Scripts/webkitpy/tool/commands/download.py index c66b95c..a283da9 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/download.py +++ b/WebKitTools/Scripts/webkitpy/tool/commands/download.py @@ -260,8 +260,9 @@ class AbstractRolloutPrepCommand(AbstractSequencedCommand): # of create-rollout for bug URLs. It should do better # parsing instead. log("Preparing rollout for bug %s." % commit_info.bug_id()) - return commit_info - log("Unable to parse bug number from diff.") + else: + log("Unable to parse bug number from diff.") + return commit_info def _prepare_state(self, options, args, tool): revision = args[0] diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/download_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/download_unittest.py index 926037c..4dd9d7f 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/download_unittest.py +++ b/WebKitTools/Scripts/webkitpy/tool/commands/download_unittest.py @@ -26,9 +26,32 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +import unittest + +from webkitpy.common.system.outputcapture import OutputCapture from webkitpy.thirdparty.mock import Mock from webkitpy.tool.commands.commandtest import CommandsTest from webkitpy.tool.commands.download import * +from webkitpy.tool.mocktool import MockTool + + +class AbstractRolloutPrepCommandTest(unittest.TestCase): + def test_commit_info(self): + command = AbstractRolloutPrepCommand() + tool = MockTool() + command.bind_to_tool(tool) + output = OutputCapture() + + expected_stderr = "Preparing rollout for bug 42.\n" + commit_info = output.assert_outputs(self, command._commit_info, [1234], expected_stderr=expected_stderr) + self.assertTrue(commit_info) + + mock_commit_info = Mock() + mock_commit_info.bug_id = lambda: None + tool._checkout.commit_info_for_revision = lambda revision: mock_commit_info + expected_stderr = "Unable to parse bug number from diff.\n" + commit_info = output.assert_outputs(self, command._commit_info, [1234], expected_stderr=expected_stderr) + self.assertEqual(commit_info, mock_commit_info) class DownloadCommandsTest(CommandsTest): diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py index 4d23a4c..3d0ddd1 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py +++ b/WebKitTools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py @@ -39,37 +39,37 @@ class EarlyWarningSytemTest(QueuesTest): ews._can_build = lambda: True ews.review_patch(Mock()) - def test_chromium_linux_ews(self): + def _default_expected_stderr(self, ews): + string_replacemnts = { + "name": ews.name, + "checkout_dir": os.getcwd(), # FIXME: Use of os.getcwd() is wrong, should be scm.checkout_root + } expected_stderr = { - "begin_work_queue": "CAUTION: chromium-ews will discard all local changes in \"%s\"\nRunning WebKit chromium-ews.\n" % os.getcwd(), + "begin_work_queue": "CAUTION: %(name)s will discard all local changes in \"%(checkout_dir)s\"\nRunning WebKit %(name)s.\n" % string_replacemnts, "handle_unexpected_error": "Mock error message\n", + "process_work_item": "MOCK: update_status: %(name)s Pass\n" % string_replacemnts, } - self.assert_queue_outputs(ChromiumLinuxEWS(), expected_stderr=expected_stderr) + return expected_stderr + + def _test_ews(self, ews): + self.assert_queue_outputs(ews, expected_stderr=self._default_expected_stderr(ews)) + + # 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): + self._test_ews(ChromiumLinuxEWS()) def test_chromium_windows_ews(self): - expected_stderr = { - "begin_work_queue": "CAUTION: cr-win-ews will discard all local changes in \"%s\"\nRunning WebKit cr-win-ews.\n" % os.getcwd(), - "handle_unexpected_error": "Mock error message\n", - } - self.assert_queue_outputs(ChromiumWindowsEWS(), expected_stderr=expected_stderr) + self._test_ews(ChromiumWindowsEWS()) def test_qt_ews(self): - expected_stderr = { - "begin_work_queue": "CAUTION: qt-ews will discard all local changes in \"%s\"\nRunning WebKit qt-ews.\n" % os.getcwd(), - "handle_unexpected_error": "Mock error message\n", - } - self.assert_queue_outputs(QtEWS(), expected_stderr=expected_stderr) + self._test_ews(QtEWS()) def test_gtk_ews(self): - expected_stderr = { - "begin_work_queue": "CAUTION: gtk-ews will discard all local changes in \"%s\"\nRunning WebKit gtk-ews.\n" % os.getcwd(), - "handle_unexpected_error": "Mock error message\n", - } - self.assert_queue_outputs(GtkEWS(), expected_stderr=expected_stderr) + self._test_ews(GtkEWS()) def test_mac_ews(self): - expected_stderr = { - "begin_work_queue": "CAUTION: mac-ews will discard all local changes in \"%s\"\nRunning WebKit mac-ews.\n" % os.getcwd(), - "handle_unexpected_error": "Mock error message\n", - } - self.assert_queue_outputs(MacEWS(), expected_stderr=expected_stderr) + 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 :(\n" + self.assert_queue_outputs(ews, expected_stderr=expected_stderr) diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queues.py b/WebKitTools/Scripts/webkitpy/tool/commands/queues.py index 775aa44..78ca729 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/queues.py +++ b/WebKitTools/Scripts/webkitpy/tool/commands/queues.py @@ -46,7 +46,6 @@ from webkitpy.tool.multicommandtool import Command class AbstractQueue(Command, QueueEngineDelegate): watchers = [ - "webkit-bot-watchers@googlegroups.com", ] _pass_status = "Pass" @@ -168,16 +167,17 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler): # Not using BugzillaQueries.fetch_patches_from_commit_queue() so we can reject patches with invalid committers/reviewers. bug_ids = self.tool.bugs.queries.fetch_bug_ids_from_commit_queue() all_patches = sum([self.tool.bugs.fetch_bug(bug_id).commit_queued_patches(include_invalid=True) for bug_id in bug_ids], []) - valid_patches = self.committer_validator.patches_after_rejecting_invalid_commiters_and_reviewers(all_patches) - if not self._builders_are_green(): - return filter(lambda patch: patch.is_rollout(), valid_patches) - return valid_patches + return self.committer_validator.patches_after_rejecting_invalid_commiters_and_reviewers(all_patches) def next_work_item(self): patches = self._validate_patches_in_commit_queue() + builders_are_green = self._builders_are_green() + if not builders_are_green: + patches = filter(lambda patch: patch.is_rollout(), patches) # FIXME: We could sort the patches in a specific order here, was suggested by https://bugs.webkit.org/show_bug.cgi?id=33395 if not patches: - self._update_status("Empty queue") + queue_text = "queue" if builders_are_green else "rollout queue" + self._update_status("Empty %s" % queue_text) return None # Only bother logging if we have patches in the queue. self.log_progress([patch.id() for patch in patches]) @@ -211,7 +211,8 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler): if not patch.is_rollout(): if not self._builders_are_green(): return False - self._update_status("Landing patch", patch) + patch_text = "rollout patch" if patch.is_rollout() else "patch" + self._update_status("Landing %s" % patch_text, patch) return True def _land(self, patch, first_run=False): @@ -226,7 +227,6 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler): "land-attachment", "--force-clean", "--build", - "--test", "--non-interactive", # The master process is responsible for checking the status # of the builders (see above call to _builders_are_green). @@ -235,6 +235,9 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler): "--quiet", patch.id() ] + # We don't bother to run tests for rollouts as that makes them too slow. + if not patch.is_rollout(): + args.append("--test") if not first_run: # The first time through, we don't reject the patch from the # commit queue because we want to make sure we can build and diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py index 16eb053..0bd42fb 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py +++ b/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py @@ -118,11 +118,13 @@ class CommitQueueTest(QueuesTest): def test_commit_queue(self): expected_stderr = { "begin_work_queue" : "CAUTION: commit-queue will discard all local changes in \"%s\"\nRunning WebKit commit-queue.\n" % MockSCM.fake_checkout_root, + "should_proceed_with_work_item": "MOCK: update_status: commit-queue Landing patch\n", # FIXME: The commit-queue warns about bad committers twice. This is due to the fact that we access Attachment.reviewer() twice and it logs each time. "next_work_item" : """Warning, attachment 128 on bug 42 has invalid committer (non-committer@example.com) Warning, attachment 128 on bug 42 has invalid committer (non-committer@example.com) 2 patches in commit-queue [197, 106] """, + "process_work_item" : "MOCK: update_status: commit-queue Pass\n", } self.assert_queue_outputs(CommitQueue(), expected_stderr=expected_stderr) @@ -131,11 +133,14 @@ Warning, attachment 128 on bug 42 has invalid committer (non-committer@example.c tool.buildbot.light_tree_on_fire() expected_stderr = { "begin_work_queue" : "CAUTION: commit-queue will discard all local changes in \"%s\"\nRunning WebKit commit-queue.\n" % MockSCM.fake_checkout_root, + "should_proceed_with_work_item": "MOCK: update_status: commit-queue Builders [\"Builder2\"] are red. See http://build.webkit.org\n", # FIXME: The commit-queue warns about bad committers twice. This is due to the fact that we access Attachment.reviewer() twice and it logs each time. "next_work_item" : """Warning, attachment 128 on bug 42 has invalid committer (non-committer@example.com) Warning, attachment 128 on bug 42 has invalid committer (non-committer@example.com) +MOCK: update_status: commit-queue Builders ["Builder2"] are red. See http://build.webkit.org 1 patch in commit-queue [106] """, + "process_work_item" : "MOCK: update_status: commit-queue Builders [\"Builder2\"] are red. See http://build.webkit.org\n", } self.assert_queue_outputs(CommitQueue(), tool=tool, expected_stderr=expected_stderr) @@ -145,20 +150,33 @@ Warning, attachment 128 on bug 42 has invalid committer (non-committer@example.c rollout_patch = MockPatch() expected_stderr = { "begin_work_queue": "CAUTION: commit-queue will discard all local changes in \"%s\"\nRunning WebKit commit-queue.\n" % MockSCM.fake_checkout_root, + "should_proceed_with_work_item": "MOCK: update_status: commit-queue Landing rollout patch\n", # FIXME: The commit-queue warns about bad committers twice. This is due to the fact that we access Attachment.reviewer() twice and it logs each time. "next_work_item": """Warning, attachment 128 on bug 42 has invalid committer (non-committer@example.com) Warning, attachment 128 on bug 42 has invalid committer (non-committer@example.com) +MOCK: update_status: commit-queue Builders ["Builder2"] are red. See http://build.webkit.org 1 patch in commit-queue [106] """, - "process_work_item": "MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'land-attachment', '--force-clean', '--build', '--test', '--non-interactive', '--ignore-builders', '--build-style=both', '--quiet', 76543]\n", + "process_work_item": "MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'land-attachment', '--force-clean', '--build', '--non-interactive', '--ignore-builders', '--build-style=both', '--quiet', 76543]\nMOCK: update_status: commit-queue Pass\n", } self.assert_queue_outputs(CommitQueue(), tool=tool, work_item=rollout_patch, expected_stderr=expected_stderr) + def test_can_build_and_test(self): + queue = CommitQueue() + tool = MockTool() + tool.executive = Mock() + queue.bind_to_tool(tool) + self.assertTrue(queue._can_build_and_test()) + expected_run_args = ["echo", "--status-host=example.com", "build-and-test", "--force-clean", "--build", "--test", "--non-interactive", "--no-update", "--build-style=both", "--quiet"] + tool.executive.run_and_throw_if_fail.assert_called_with(expected_run_args) + class StyleQueueTest(QueuesTest): def test_style_queue(self): expected_stderr = { "begin_work_queue" : "CAUTION: style-queue will discard all local changes in \"%s\"\nRunning WebKit style-queue.\n" % MockSCM.fake_checkout_root, + "should_proceed_with_work_item": "MOCK: update_status: style-queue Checking style\n", + "process_work_item" : "MOCK: update_status: style-queue Pass\n", "handle_unexpected_error" : "Mock error message\n", } self.assert_queue_outputs(StyleQueue(), expected_stderr=expected_stderr) diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot.py b/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot.py index eb80d8f..24c8517 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot.py +++ b/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot.py @@ -57,17 +57,46 @@ class SheriffBot(AbstractQueue, StepSequenceErrorHandler): def work_item_log_path(self, new_failures): return os.path.join("%s-logs" % self.name, "%s.log" % new_failures.keys()[0]) - def next_work_item(self): - self._irc_bot.process_pending_messages() - self._update() + def _new_failures(self, revisions_causing_failures, old_failing_svn_revisions): + # We ignore failures that might have been caused by svn_revisions that + # we've already complained about. This is conservative in the sense + # that we might be ignoring some new failures, but our experience has + # been that skipping this check causes a lot of spam for builders that + # take a long time to cycle. + old_failing_builder_names = [] + for svn_revision in old_failing_svn_revisions: + old_failing_builder_names.extend( + [builder.name() for builder in revisions_causing_failures[svn_revision]]) + new_failures = {} - revisions_causing_failures = self.tool.buildbot.revisions_causing_failures() for svn_revision, builders in revisions_causing_failures.items(): - if self.tool.status_server.svn_revision(svn_revision): + if svn_revision in old_failing_svn_revisions: # FIXME: We should re-process the work item after some time delay. # https://bugs.webkit.org/show_bug.cgi?id=36581 continue - new_failures[svn_revision] = builders + new_builders = [builder for builder in builders + if builder.name() not in old_failing_builder_names] + if new_builders: + new_failures[svn_revision] = new_builders + + return new_failures + + def next_work_item(self): + self._irc_bot.process_pending_messages() + self._update() + + # We do one read from buildbot to ensure a consistent view. + revisions_causing_failures = self.tool.buildbot.revisions_causing_failures() + + # Similarly, we read once from our the status_server. + old_failing_svn_revisions = [] + for svn_revision in revisions_causing_failures.keys(): + if self.tool.status_server.svn_revision(svn_revision): + old_failing_svn_revisions.append(svn_revision) + + new_failures = self._new_failures(revisions_causing_failures, + old_failing_svn_revisions) + self._sheriff.provoke_flaky_builders(revisions_causing_failures) return new_failures diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py index f121eda..4b4b8b6 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py +++ b/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py @@ -30,18 +30,44 @@ import os from webkitpy.tool.commands.queuestest import QueuesTest from webkitpy.tool.commands.sheriffbot import SheriffBot -from webkitpy.tool.mocktool import mock_builder +from webkitpy.tool.mocktool import MockBuilder class SheriffBotTest(QueuesTest): + builder1 = MockBuilder("Builder1") + builder2 = MockBuilder("Builder2") + def test_sheriff_bot(self): mock_work_item = { - 29837: [mock_builder], + 29837: [self.builder1], } expected_stderr = { "begin_work_queue": "CAUTION: sheriff-bot will discard all local changes in \"%s\"\nRunning WebKit sheriff-bot.\n" % os.getcwd(), "next_work_item": "", - "process_work_item": "MOCK: irc.post: abarth, darin, eseidel: http://trac.webkit.org/changeset/29837 might have broken Mock builder name (Tests)\nMOCK bug comment: bug_id=42, cc=['webkit-bot-watchers@googlegroups.com', 'abarth@webkit.org', 'eric@webkit.org']\n--- Begin comment ---\\http://trac.webkit.org/changeset/29837 might have broken Mock builder name (Tests)\n--- End comment ---\n\n", + "process_work_item": "MOCK: irc.post: abarth, darin, eseidel: http://trac.webkit.org/changeset/29837 might have broken Builder1\nMOCK bug comment: bug_id=42, cc=['abarth@webkit.org', 'eric@webkit.org']\n--- Begin comment ---\\http://trac.webkit.org/changeset/29837 might have broken Builder1\n--- End comment ---\n\n", "handle_unexpected_error": "Mock error message\n" } self.assert_queue_outputs(SheriffBot(), work_item=mock_work_item, expected_stderr=expected_stderr) + + revisions_causing_failures = { + 1234: [builder1], + 1235: [builder1, builder2], + } + + def test_new_failures(self): + old_failing_svn_revisions = [] + self.assertEquals(SheriffBot()._new_failures(self.revisions_causing_failures, + old_failing_svn_revisions), + self.revisions_causing_failures) + + def test_new_failures_with_old_revisions(self): + old_failing_svn_revisions = [1234] + self.assertEquals(SheriffBot()._new_failures(self.revisions_causing_failures, + old_failing_svn_revisions), + {1235: [builder2]}) + + def test_new_failures_with_old_revisions(self): + old_failing_svn_revisions = [1235] + self.assertEquals(SheriffBot()._new_failures(self.revisions_causing_failures, + old_failing_svn_revisions), + {}) diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/upload.py b/WebKitTools/Scripts/webkitpy/tool/commands/upload.py index 99d45a6..4797ef6 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/upload.py +++ b/WebKitTools/Scripts/webkitpy/tool/commands/upload.py @@ -46,14 +46,23 @@ from webkitpy.tool.comments import bug_comment_from_svn_revision from webkitpy.tool.multicommandtool import AbstractDeclarativeCommand from webkitpy.common.system.deprecated_logging import error, log + class CommitMessageForCurrentDiff(AbstractDeclarativeCommand): name = "commit-message" help_text = "Print a commit message suitable for the uncommitted changes" + def __init__(self): + options = [ + steps.Options.squash, + steps.Options.git_commit, + ] + AbstractDeclarativeCommand.__init__(self, options=options) + def execute(self, options, args, tool): # This command is a useful test to make sure commit_message_for_this_commit # always returns the right value regardless of the current working directory. - print "%s" % tool.checkout().commit_message_for_this_commit().message() + print "%s" % tool.checkout().commit_message_for_this_commit(options.git_commit, options.squash).message() + class CleanPendingCommit(AbstractDeclarativeCommand): name = "clean-pending-commit" diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/upload_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/upload_unittest.py index eec3751..8f6483a 100644 --- a/WebKitTools/Scripts/webkitpy/tool/commands/upload_unittest.py +++ b/WebKitTools/Scripts/webkitpy/tool/commands/upload_unittest.py @@ -34,10 +34,7 @@ from webkitpy.tool.mocktool import MockTool class UploadCommandsTest(CommandsTest): def test_commit_message_for_current_diff(self): tool = MockTool() - mock_commit_message_for_this_commit = Mock() - mock_commit_message_for_this_commit.message = lambda: "Mock message" - tool._checkout.commit_message_for_this_commit = lambda: mock_commit_message_for_this_commit - expected_stdout = "Mock message\n" + expected_stdout = "This is a fake commit message that is at least 50 characters.\n" self.assert_execute_outputs(CommitMessageForCurrentDiff(), [], expected_stdout=expected_stdout, tool=tool) def test_clean_pending_commit(self): diff --git a/WebKitTools/Scripts/webkitpy/tool/mocktool.py b/WebKitTools/Scripts/webkitpy/tool/mocktool.py index 128362a..47fff1b 100644 --- a/WebKitTools/Scripts/webkitpy/tool/mocktool.py +++ b/WebKitTools/Scripts/webkitpy/tool/mocktool.py @@ -184,15 +184,6 @@ _bug4 = { } -class MockBuilder(object): - - def name(self): - return "Mock builder name (Tests)" - - -mock_builder = MockBuilder() - - class MockBugzillaQueries(Mock): def __init__(self, bugzilla): @@ -319,6 +310,9 @@ class MockBuilder(object): def __init__(self, name): self._name = name + def name(self): + return self._name + def force_build(self, username, comments): log("MOCK: force_build: name=%s, username=%s, comments=%s" % ( self._name, username, comments)) @@ -369,7 +363,7 @@ class MockBuildBot(object): def revisions_causing_failures(self): return { - "29837": [mock_builder] + "29837": [self.builder_with_name("Builder1")], } @@ -484,6 +478,7 @@ class MockStatusServer(object): return None def update_status(self, queue_name, status, patch=None, results_file=None): + log("MOCK: update_status: %s %s" % (queue_name, status)) return 187 def update_svn_revision(self, svn_revision, broken_bot): @@ -491,7 +486,12 @@ class MockStatusServer(object): class MockExecute(Mock): + def __init__(self, should_log): + self._should_log = should_log + def run_and_throw_if_fail(self, args, quiet=False): + if self._should_log: + log("MOCK run_and_throw_if_fail: %s" % args) return "MOCK output of child process" def run_command(self, @@ -502,6 +502,8 @@ class MockExecute(Mock): return_exit_code=False, return_stderr=True, decode_output=False): + if self._should_log: + log("MOCK run_command: %s" % args) return "MOCK output of child process" @@ -511,9 +513,7 @@ class MockTool(): self.wakeup_event = threading.Event() self.bugs = MockBugzilla() self.buildbot = MockBuildBot() - self.executive = MockExecute() - if log_executive: - self.executive.run_and_throw_if_fail = lambda args: log("MOCK run_and_throw_if_fail: %s" % args) + self.executive = MockExecute(should_log=log_executive) self._irc = None self.user = MockUser() self._scm = MockSCM() diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/checkstyle.py b/WebKitTools/Scripts/webkitpy/tool/steps/checkstyle.py index 7b2be99..93e6215 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/checkstyle.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/checkstyle.py @@ -48,16 +48,17 @@ class CheckStyle(AbstractStep): if not self._options.check_style: return os.chdir(self._tool.scm().checkout_root) - try: - args = [] - if self._options.git_commit: - args.append("--git-commit") - args.append(self._options.git_commit) - if self._tool.scm().should_squash(self._options.squash): - args.append("--squash") - else: - args.append("--no-squash") + args = [] + if self._options.git_commit: + args.append("--git-commit") + args.append(self._options.git_commit) + if self._tool.scm().should_squash(self._options.squash): + args.append("--squash") + else: + args.append("--no-squash") + + try: self._run_script("check-webkit-style", args) except ScriptError, e: if self._options.non_interactive: diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/checkstyle_unittest.py b/WebKitTools/Scripts/webkitpy/tool/steps/checkstyle_unittest.py new file mode 100644 index 0000000..a23ea1b --- /dev/null +++ b/WebKitTools/Scripts/webkitpy/tool/steps/checkstyle_unittest.py @@ -0,0 +1,46 @@ +# Copyright (C) 2009 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. + +import unittest + +from webkitpy.common.system.executive import ScriptError +from webkitpy.thirdparty.mock import Mock +from webkitpy.tool.mocktool import MockTool +from webkitpy.tool.steps.checkstyle import CheckStyle + + +class CheckStyleTest(unittest.TestCase): + def test_should_squash_error(self): + """should_squash can throw an error. That error should not be eaten by CheckStyle.""" + def should_squash(squash): + raise ScriptError(message="Dummy error") + + tool = MockTool() + tool._scm.should_squash = should_squash + step = CheckStyle(tool, Mock()) + self.assertRaises(ScriptError, step.run, []) diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/postcodereview.py b/WebKitTools/Scripts/webkitpy/tool/steps/postcodereview.py index 198cfce..8397519 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/postcodereview.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/postcodereview.py @@ -42,33 +42,32 @@ class PostCodeReview(AbstractStep): def run(self, state): if not self._options.fancy_review: return - # FIXME: This will always be None because we don't retrieve the issue - # number from the ChangeLog yet. - codereview_issue = state.get("codereview_issue") + + bug_id = state.get("bug_id") + if not bug_id: + raise ScriptError(message="Cannot upload a fancy review without a bug ID.") + message = self._options.description if not message: # If we have an issue number, then the message becomes the label # of the new patch. Otherwise, it becomes the title of the whole # issue. - if codereview_issue: - message = "Updated patch" - elif state.get("bug_title"): + if state.get("bug_title"): # This is the common case for the the first "upload" command. message = state.get("bug_title") - elif state.get("bug_id"): + elif bug_id: # This is the common case for the "post" command and - # subsequent runs of the "upload" command. In this case, - # I'd rather add the new patch to the existing issue, but - # that's not implemented yet. - message = "Code review for %s" % self._tool.bugs.bug_url_for_bug_id(state["bug_id"]) + # subsequent runs of the "upload" command. + message = "Code review for %s" % self._tool.bugs.bug_url_for_bug_id(bug_id) else: # Unreachable with our current commands, but we might hit # this case if we support bug-less code reviews. message = "Code review" + + # Use the bug ID as the rietveld issue number. This means rietveld code reviews + # when there are multiple different patches on a bug will be a bit wonky, but + # webkit-patch assumes one-patch-per-bug. created_issue = self._tool.codereview.post(diff=self.cached_lookup(state, "diff"), message=message, - codereview_issue=codereview_issue, + codereview_issue=bug_id, cc=self._options.cc) - if created_issue: - # FIXME: Record the issue number in the ChangeLog. - state["codereview_issue"] = created_issue diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/postdiff.py b/WebKitTools/Scripts/webkitpy/tool/steps/postdiff.py index a542dba..79739cd 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/postdiff.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/postdiff.py @@ -44,11 +44,6 @@ class PostDiff(AbstractStep): diff = self.cached_lookup(state, "diff") description = self._options.description or "Patch" comment_text = None - codereview_issue = state.get("codereview_issue") - # Include codereview issue number in patch name. This is a bit of a hack, - # but it makes doing the rietveld integration a lot easier. - if codereview_issue: - description += "-%s" % state["codereview_issue"] self._tool.bugs.add_patch_to_bug(state["bug_id"], diff, description, comment_text=comment_text, mark_for_review=self._options.review, mark_for_commit_queue=self._options.request_commit) if self._options.open_bug: self._tool.user.open_url(self._tool.bugs.bug_url_for_bug_id(state["bug_id"])) diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/runtests.py b/WebKitTools/Scripts/webkitpy/tool/steps/runtests.py index b1c2d3b..0734d8f 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/runtests.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/runtests.py @@ -57,6 +57,11 @@ class RunTests(AbstractStep): if self._options.non_interactive: args.append("--no-launch-safari") args.append("--exit-after-n-failures=1") + # FIXME: Hack to work around https://bugs.webkit.org/show_bug.cgi?id=38912 + # when running the commit-queue on a mac leopard machine. + if self.port().name() == "Mac" and self.port().is_leopard(): + args.extend(["--ignore-tests", "compositing/iframes"]) + if self._options.quiet: args.append("--quiet") self._tool.executive.run_and_throw_if_fail(args) diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/steps_unittest.py b/WebKitTools/Scripts/webkitpy/tool/steps/steps_unittest.py index 5abfc6d..eee183b 100644 --- a/WebKitTools/Scripts/webkitpy/tool/steps/steps_unittest.py +++ b/WebKitTools/Scripts/webkitpy/tool/steps/steps_unittest.py @@ -29,9 +29,11 @@ import unittest from webkitpy.common.system.outputcapture import OutputCapture +from webkitpy.common.config.ports import WebKitPort from webkitpy.thirdparty.mock import Mock from webkitpy.tool.mocktool import MockTool from webkitpy.tool.steps.update import Update +from webkitpy.tool.steps.runtests import RunTests from webkitpy.tool.steps.promptforbugortitle import PromptForBugOrTitle @@ -55,3 +57,27 @@ class StepsTest(unittest.TestCase): tool = MockTool() tool.user.prompt = lambda message: 42 self._run_step(PromptForBugOrTitle, tool=tool) + + def test_runtests_leopard_commit_queue_hack(self): + expected_stderr = "Running Python unit tests\nRunning Perl unit tests\nRunning JavaScriptCore tests\nRunning run-webkit-tests\n" + OutputCapture().assert_outputs(self, self._run_step, [RunTests], expected_stderr=expected_stderr) + + def test_runtests_leopard_commit_queue_hack(self): + mock_options = Mock() + mock_options.non_interactive = True + step = RunTests(MockTool(log_executive=True), mock_options) + # FIXME: We shouldn't use a real port-object here, but there is too much to mock at the moment. + mock_port = WebKitPort() + mock_port.name = lambda: "Mac" + mock_port.is_leopard = lambda: True + step.port = lambda: mock_port + expected_stderr = """Running Python unit tests +MOCK run_and_throw_if_fail: ['WebKitTools/Scripts/test-webkitpy'] +Running Perl unit tests +MOCK run_and_throw_if_fail: ['WebKitTools/Scripts/test-webkitperl'] +Running JavaScriptCore tests +MOCK run_and_throw_if_fail: ['WebKitTools/Scripts/run-javascriptcore-tests'] +Running run-webkit-tests +MOCK run_and_throw_if_fail: ['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--ignore-tests', 'compositing/iframes', '--quiet'] +""" + OutputCapture().assert_outputs(self, step.run, [{}], expected_stderr=expected_stderr) |