[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:39:39 UTC 2010
The following commit has been merged in the debian/experimental branch:
commit bfcf8b0ef10ec570b5bc66fa0e5469b3e4f4da46
Author: eric at webkit.org <eric at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Fri Oct 15 00:51:07 2010 +0000
2010-10-14 Eric Seidel <eric at webkit.org>
Reviewed by Adam Barth.
commit-queue should not fail patches due to flaky tests
https://bugs.webkit.org/show_bug.cgi?id=47647
This patch makes it so that the *same* flaky test has to fail
twice in a row to have a false negative from a flaky test.
If different flaky tests fail (or if a test fails and then passes
in a second run) then we will warn in the bug that we encountered
a flaky test.
This patch grew to include moving port off of steps onto tool
(which Adam wrote and then I integrated), as well as removing the
use of tool from CommitQueueTask.
* Scripts/webkitpy/common/config/ports.py:
- Added a layout_test_results_path method. This covers old-run-webkit-tests
but doesn't cover NRWT. This is probably not the long term solution, but
putting this knowledge on port makes more sense than in LayoutTestResults.
* Scripts/webkitpy/common/net/buildbot.py:
- LayoutTestResults shouldn't know how to fetch from the network, make
the Build code do that instead.
* Scripts/webkitpy/common/net/buildbot_unittest.py:
- Code style fix.
* Scripts/webkitpy/common/net/layouttestresults.py:
- Remove code for reading from the network.
* Scripts/webkitpy/common/net/layouttestresults_unittest.py:
- Test the new entrypoint.
* Scripts/webkitpy/tool/bot/commitqueuetask.py:
- Make the delegate interface explicit.
- Remove the _tool member, since using the delegate for
everything is cleaner.
- Teach the testing logic how to deal with flaky tests.
* Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py:
- Update to match the CommitQueueTask changes.
* Scripts/webkitpy/tool/commands/queues.py:
- Use the new CommitQueueTaskDelegate interface.
* Scripts/webkitpy/tool/commands/queues_unittest.py:
- Fix the SecondThoughtsCommitQueue test which was broken.
- Add a new test to make sure the flaky test reporting works.
* Scripts/webkitpy/tool/main.py:
- Store the port on the tool object.
* Scripts/webkitpy/tool/mocktool.py:
- Add a port() accessor to MockTool
* Scripts/webkitpy/tool/steps/abstractstep.py:
- Move port() off of Step and onto Tool.
* Scripts/webkitpy/tool/steps/build.py:
* Scripts/webkitpy/tool/steps/preparechangelog.py:
* Scripts/webkitpy/tool/steps/runtests.py:
* Scripts/webkitpy/tool/steps/steps_unittest.py:
- Two tests with the same name! only the latter was being run.
* Scripts/webkitpy/tool/steps/update.py:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@69829 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index 03b8e6a..5ea3206 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,59 @@
+2010-10-14 Eric Seidel <eric at webkit.org>
+
+ Reviewed by Adam Barth.
+
+ commit-queue should not fail patches due to flaky tests
+ https://bugs.webkit.org/show_bug.cgi?id=47647
+
+ This patch makes it so that the *same* flaky test has to fail
+ twice in a row to have a false negative from a flaky test.
+
+ If different flaky tests fail (or if a test fails and then passes
+ in a second run) then we will warn in the bug that we encountered
+ a flaky test.
+
+ This patch grew to include moving port off of steps onto tool
+ (which Adam wrote and then I integrated), as well as removing the
+ use of tool from CommitQueueTask.
+
+ * Scripts/webkitpy/common/config/ports.py:
+ - Added a layout_test_results_path method. This covers old-run-webkit-tests
+ but doesn't cover NRWT. This is probably not the long term solution, but
+ putting this knowledge on port makes more sense than in LayoutTestResults.
+ * Scripts/webkitpy/common/net/buildbot.py:
+ - LayoutTestResults shouldn't know how to fetch from the network, make
+ the Build code do that instead.
+ * Scripts/webkitpy/common/net/buildbot_unittest.py:
+ - Code style fix.
+ * Scripts/webkitpy/common/net/layouttestresults.py:
+ - Remove code for reading from the network.
+ * Scripts/webkitpy/common/net/layouttestresults_unittest.py:
+ - Test the new entrypoint.
+ * Scripts/webkitpy/tool/bot/commitqueuetask.py:
+ - Make the delegate interface explicit.
+ - Remove the _tool member, since using the delegate for
+ everything is cleaner.
+ - Teach the testing logic how to deal with flaky tests.
+ * Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py:
+ - Update to match the CommitQueueTask changes.
+ * Scripts/webkitpy/tool/commands/queues.py:
+ - Use the new CommitQueueTaskDelegate interface.
+ * Scripts/webkitpy/tool/commands/queues_unittest.py:
+ - Fix the SecondThoughtsCommitQueue test which was broken.
+ - Add a new test to make sure the flaky test reporting works.
+ * Scripts/webkitpy/tool/main.py:
+ - Store the port on the tool object.
+ * Scripts/webkitpy/tool/mocktool.py:
+ - Add a port() accessor to MockTool
+ * Scripts/webkitpy/tool/steps/abstractstep.py:
+ - Move port() off of Step and onto Tool.
+ * Scripts/webkitpy/tool/steps/build.py:
+ * Scripts/webkitpy/tool/steps/preparechangelog.py:
+ * Scripts/webkitpy/tool/steps/runtests.py:
+ * Scripts/webkitpy/tool/steps/steps_unittest.py:
+ - Two tests with the same name! only the latter was being run.
+ * Scripts/webkitpy/tool/steps/update.py:
+
2010-10-14 Dirk Pranke <dpranke at chromium.org>
Reviewed by Eric Seidel.
diff --git a/WebKitTools/Scripts/webkitpy/common/config/ports.py b/WebKitTools/Scripts/webkitpy/common/config/ports.py
index 79cb0c2..d268865 100644
--- a/WebKitTools/Scripts/webkitpy/common/config/ports.py
+++ b/WebKitTools/Scripts/webkitpy/common/config/ports.py
@@ -103,6 +103,10 @@ class WebKitPort(object):
def run_perl_unittests_command(cls):
return [cls.script_path("test-webkitperl")]
+ @classmethod
+ def layout_tests_results_path(cls):
+ return "/tmp/layout-test-results/results.html"
+
class MacPort(WebKitPort):
diff --git a/WebKitTools/Scripts/webkitpy/common/net/buildbot.py b/WebKitTools/Scripts/webkitpy/common/net/buildbot.py
index 8cb902c..d3d08ee 100644
--- a/WebKitTools/Scripts/webkitpy/common/net/buildbot.py
+++ b/WebKitTools/Scripts/webkitpy/common/net/buildbot.py
@@ -215,9 +215,19 @@ class Build(object):
results_directory = "r%s (%s)" % (self.revision(), self._number)
return "%s/%s" % (self._builder.results_url(), urllib.quote(results_directory))
+ def _fetch_results_html(self):
+ results_html = "%s/results.html" % (self.results_url())
+ # FIXME: We need to move this sort of 404 logic into NetworkTransaction or similar.
+ try:
+ return urllib2.urlopen(results_html)
+ except urllib2.HTTPError, error:
+ if error.code != 404:
+ raise
+
def layout_test_results(self):
if not self._layout_test_results:
- self._layout_test_results = LayoutTestResults.results_from_url(self.results_url())
+ # FIXME: This should cache that the result was a 404 and stop hitting the network.
+ self._layout_test_results = LayoutTestResults.results_from_string(self._fetch_results_html())
return self._layout_test_results
def builder(self):
diff --git a/WebKitTools/Scripts/webkitpy/common/net/buildbot_unittest.py b/WebKitTools/Scripts/webkitpy/common/net/buildbot_unittest.py
index 57a0c8f..afc9a39 100644
--- a/WebKitTools/Scripts/webkitpy/common/net/buildbot_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/common/net/buildbot_unittest.py
@@ -41,10 +41,8 @@ class BuilderTest(unittest.TestCase):
revision=build_number + 1000,
is_green=build_number < 4
)
- build._layout_test_results = LayoutTestResults(
- "http://buildbot.example.com/foo", {
- LayoutTestResults.fail_key: failure(build_number),
- })
+ parsed_results = {LayoutTestResults.fail_key: failure(build_number)}
+ build._layout_test_results = LayoutTestResults(parsed_results)
return build
self.builder._fetch_build = _mock_fetch_build
diff --git a/WebKitTools/Scripts/webkitpy/common/net/layouttestresults.py b/WebKitTools/Scripts/webkitpy/common/net/layouttestresults.py
index 5ab8292..eb5ea7d 100644
--- a/WebKitTools/Scripts/webkitpy/common/net/layouttestresults.py
+++ b/WebKitTools/Scripts/webkitpy/common/net/layouttestresults.py
@@ -34,6 +34,8 @@ from webkitpy.thirdparty.BeautifulSoup import BeautifulSoup, SoupStrainer
# FIXME: This should be unified with all the layout test results code in the layout_tests package
# This doesn't belong in common.net, but we don't have a better place for it yet.
class LayoutTestResults(object):
+ """This class knows how to parse old-run-webkit-tests results.html files."""
+
stderr_key = u'Tests that had stderr output:'
fail_key = u'Tests where results did not match expected results:'
timeout_key = u'Tests that timed out:'
@@ -62,27 +64,14 @@ class LayoutTestResults(object):
return parsed_results
- # FIXME: This class shouldn't know how to fetch from the network.
- @classmethod
- def _fetch_results_html(cls, base_url):
- results_html = "%s/results.html" % base_url
- # FIXME: We need to move this sort of 404 logic into NetworkTransaction or similar.
- try:
- page = urllib2.urlopen(results_html)
- return cls._parse_results_html(page)
- except urllib2.HTTPError, error:
- if error.code != 404:
- raise
-
@classmethod
- def results_from_url(cls, base_url):
- parsed_results = cls._fetch_results_html(base_url)
+ def results_from_string(cls, string):
+ parsed_results = cls._parse_results_html(string)
if not parsed_results:
return None
- return cls(base_url, parsed_results)
+ return cls(parsed_results)
- def __init__(self, base_url, parsed_results):
- self._base_url = base_url
+ def __init__(self, parsed_results):
self._parsed_results = parsed_results
def parsed_results(self):
diff --git a/WebKitTools/Scripts/webkitpy/common/net/layouttestresults_unittest.py b/WebKitTools/Scripts/webkitpy/common/net/layouttestresults_unittest.py
index 034848e..44e4dbc 100644
--- a/WebKitTools/Scripts/webkitpy/common/net/layouttestresults_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/common/net/layouttestresults_unittest.py
@@ -69,3 +69,8 @@ class LayoutTestResultsTest(unittest.TestCase):
def test_parse_layout_test_results(self):
results = LayoutTestResults._parse_results_html(self._example_results_html)
self.assertEqual(self._expected_layout_test_results, results)
+
+ def test_results_from_string(self):
+ self.assertEqual(LayoutTestResults.results_from_string(""), None)
+ results = LayoutTestResults.results_from_string(self._example_results_html)
+ self.assertEqual(len(results.failing_tests()), 0)
diff --git a/WebKitTools/Scripts/webkitpy/tool/bot/commitqueuetask.py b/WebKitTools/Scripts/webkitpy/tool/bot/commitqueuetask.py
index a5c2ca2..02e203c 100644
--- a/WebKitTools/Scripts/webkitpy/tool/bot/commitqueuetask.py
+++ b/WebKitTools/Scripts/webkitpy/tool/bot/commitqueuetask.py
@@ -27,19 +27,39 @@
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
from webkitpy.common.system.executive import ScriptError
+from webkitpy.common.net.layouttestresults import LayoutTestResults
+
+
+class CommitQueueTaskDelegate(object):
+ def run_command(self, command):
+ raise NotImplementedError("subclasses must implement")
+
+ def command_passed(self, message, patch):
+ raise NotImplementedError("subclasses must implement")
+
+ def command_failed(self, message, script_error, patch):
+ raise NotImplementedError("subclasses must implement")
+
+ def refetch_patch(self, patch):
+ raise NotImplementedError("subclasses must implement")
+
+ def layout_test_results(self):
+ raise NotImplementedError("subclasses must implement")
+
+ def report_flaky_tests(self, patch, flaky_tests):
+ raise NotImplementedError("subclasses must implement")
class CommitQueueTask(object):
- def __init__(self, tool, commit_queue, patch):
- self._tool = tool
- self._commit_queue = commit_queue
+ def __init__(self, delegate, patch):
+ self._delegate = delegate
self._patch = patch
self._script_error = None
def _validate(self):
# Bugs might get closed, or patches might be obsoleted or r-'d while the
# commit-queue is processing.
- self._patch = self._tool.bugs.fetch_attachment(self._patch.id())
+ self._patch = self._delegate.refetch_patch(self._patch)
if self._patch.is_obsolete():
return False
if self._patch.bug().is_closed():
@@ -52,12 +72,12 @@ class CommitQueueTask(object):
def _run_command(self, command, success_message, failure_message):
try:
- self._commit_queue.run_webkit_patch(command)
- self._commit_queue.command_passed(success_message, patch=self._patch)
+ self._delegate.run_command(command)
+ self._delegate.command_passed(success_message, patch=self._patch)
return True
except ScriptError, e:
self._script_error = e
- self.failure_status_id = self._commit_queue.command_failed(failure_message, script_error=self._script_error, patch=self._patch)
+ self.failure_status_id = self._delegate.command_failed(failure_message, script_error=self._script_error, patch=self._patch)
return False
def _apply(self):
@@ -119,6 +139,12 @@ class CommitQueueTask(object):
"Able to pass tests without patch",
"Unable to pass tests without patch (tree is red?)")
+ def _failing_tests_from_last_run(self):
+ results = self._delegate.layout_test_results()
+ if not results:
+ return None
+ return results.failing_tests()
+
def _land(self):
return self._run_command([
"land-attachment",
@@ -132,6 +158,29 @@ class CommitQueueTask(object):
"Landed patch",
"Unable to land patch")
+ def _report_flaky_tests(self, flaky_tests):
+ self._delegate.report_flaky_tests(self._patch, flaky_tests)
+
+ def _test_patch(self):
+ if self._patch.is_rollout():
+ return True
+ if self._test():
+ return True
+
+ first_failing_tests = self._failing_tests_from_last_run()
+ if self._test():
+ self._report_flaky_tests(first_failing_tests)
+ return True
+
+ second_failing_tests = self._failing_tests_from_last_run()
+ if first_failing_tests != second_failing_tests:
+ self._report_flaky_tests(first_failing_tests + second_failing_tests)
+ return False
+
+ if self._build_and_test_without_patch():
+ raise self._script_error # The error from the previous ._test() run is real, report it.
+ return False # Tree must be red, just retry later.
+
def run(self):
if not self._validate():
return False
@@ -141,12 +190,8 @@ class CommitQueueTask(object):
if not self._build_without_patch():
return False
raise self._script_error
- if not self._patch.is_rollout():
- if not self._test():
- if not self._test():
- if not self._build_and_test_without_patch():
- return False
- raise self._script_error
+ if not self._test_patch():
+ return False
# 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():
diff --git a/WebKitTools/Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py b/WebKitTools/Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py
index 38c4747..6fa0667 100644
--- a/WebKitTools/Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py
@@ -36,11 +36,11 @@ from webkitpy.tool.bot.commitqueuetask import *
from webkitpy.tool.mocktool import MockTool
-class MockCommitQueue:
+class MockCommitQueue(CommitQueueTaskDelegate):
def __init__(self, error_plan):
self._error_plan = error_plan
- def run_webkit_patch(self, command):
+ def run_command(self, command):
log("run_webkit_patch: %s" % command)
if self._error_plan:
error = self._error_plan.pop(0)
@@ -56,12 +56,21 @@ class MockCommitQueue:
failure_message, script_error, patch.id()))
return 3947
+ def refetch_patch(self, patch):
+ return patch
+
+ def layout_test_results(self):
+ return None
+
+ def report_flaky_tests(self, patch, flaky_tests):
+ log("report_flaky_tests: patch='%s' flaky_tests='%s'" % (patch.id(), flaky_tests))
+
class CommitQueueTaskTest(unittest.TestCase):
def _run_through_task(self, commit_queue, expected_stderr, expected_exception=None):
tool = MockTool(log_executive=True)
patch = tool.bugs.fetch_attachment(197)
- task = CommitQueueTask(tool, commit_queue, patch)
+ task = CommitQueueTask(commit_queue, patch)
OutputCapture().assert_outputs(self, task.run, expected_stderr=expected_stderr, expected_exception=expected_exception)
def test_success_case(self):
@@ -129,6 +138,7 @@ run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--q
command_failed: failure_message='Patch does not pass tests' script_error='MOCK tests failure' patch='197'
run_webkit_patch: ['build-and-test', '--no-clean', '--no-update', '--test', '--quiet', '--non-interactive']
command_passed: success_message='Passed tests' patch='197'
+report_flaky_tests: patch='197' flaky_tests='None'
run_webkit_patch: ['land-attachment', '--force-clean', '--ignore-builders', '--quiet', '--non-interactive', '--parent-command=commit-queue', 197]
command_passed: success_message='Landed patch' patch='197'
"""
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queues.py b/WebKitTools/Scripts/webkitpy/tool/commands/queues.py
index 4e00e3e..e0c80bd 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/queues.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/queues.py
@@ -27,6 +27,9 @@
# (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 __future__ import with_statement
+
+import codecs
import time
import traceback
import os
@@ -40,7 +43,7 @@ from webkitpy.common.net.statusserver import StatusServer
from webkitpy.common.system.executive import ScriptError
from webkitpy.common.system.deprecated_logging import error, log
from webkitpy.tool.commands.stepsequence import StepSequenceErrorHandler
-from webkitpy.tool.bot.commitqueuetask import CommitQueueTask
+from webkitpy.tool.bot.commitqueuetask import CommitQueueTask, CommitQueueTaskDelegate
from webkitpy.tool.bot.feeders import CommitQueueFeeder
from webkitpy.tool.bot.patchcollection import PersistentPatchCollection, PersistentPatchCollectionDelegate
from webkitpy.tool.bot.queueengine import QueueEngine, QueueEngineDelegate
@@ -211,7 +214,7 @@ class AbstractPatchQueue(AbstractQueue):
return os.path.join(self._log_directory(), "%s.log" % patch.bug_id())
-class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler):
+class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler, CommitQueueTaskDelegate):
name = "commit-queue"
# AbstractPatchQueue methods
@@ -233,7 +236,7 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler):
def process_work_item(self, patch):
self._cc_watchers(patch.bug_id())
- task = CommitQueueTask(self._tool, self, patch)
+ task = CommitQueueTask(self, patch)
try:
if task.run():
self._did_pass(patch)
@@ -244,9 +247,20 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler):
validator.reject_patch_from_commit_queue(patch.id(), self._error_message_for_bug(task.failure_status_id, e))
self._did_fail(patch)
+ def _error_message_for_bug(self, status_id, script_error):
+ if not script_error.output:
+ return script_error.message_with_output()
+ results_link = self._tool.status_server.results_url_for_status(status_id)
+ return "%s\nFull output: %s" % (script_error.message_with_output(), results_link)
+
def handle_unexpected_error(self, patch, message):
self.committer_validator.reject_patch_from_commit_queue(patch.id(), message)
+ # CommitQueueTaskDelegate methods
+
+ def run_command(self, command):
+ self.run_webkit_patch(command)
+
def command_passed(self, message, patch):
self._update_status(message, patch=patch)
@@ -254,11 +268,22 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler):
failure_log = self._log_from_script_error_for_upload(script_error)
return self._update_status(message, patch=patch, results_file=failure_log)
- def _error_message_for_bug(self, status_id, script_error):
- if not script_error.output:
- return script_error.message_with_output()
- results_link = self._tool.status_server.results_url_for_status(status_id)
- return "%s\nFull output: %s" % (script_error.message_with_output(), results_link)
+ # FIXME: This may belong on the Port object.
+ def layout_test_results(self):
+ results_path = self._tool.port().layout_tests_results_path()
+ try:
+ # FIXME: We need a nice open() abstraction for better mocking here.
+ with codecs.open(results_path, "r", "utf-8") as results_file:
+ return LayoutTestResults.results_from_string(results_file)
+ except OSError, e: # File does not exist or can't be read.
+ return None
+
+ def refetch_patch(self, patch):
+ return self._tool.bugs.fetch_attachment(patch.id())
+
+ def report_flaky_tests(self, patch, flaky_tests):
+ message = "The %s encountered the following flaky tests while processing attachment %s:\n\n%s\n\nPlease file bugs against the tests. The commit-queue is continuing to process your patch." % (self.name, patch.id(), flaky_tests)
+ self._tool.bugs.post_comment_to_bug(patch.bug_id(), message, cc=self.watchers)
# StepSequenceErrorHandler methods
diff --git a/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py b/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
index d680098..c8f63ec 100644
--- a/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py
@@ -183,7 +183,20 @@ class AlwaysCommitQueueTool(object):
class SecondThoughtsCommitQueue(CommitQueue):
- def _build_and_test_patch(self, patch, first_run=True):
+ def __init__(self):
+ self._reject_patch = False
+ CommitQueue.__init__(self)
+
+ def run_command(self, command):
+ # We want to reject the patch after the first validation,
+ # so wait to reject it until after some other command has run.
+ self._reject_patch = True
+ return CommitQueue.run_command(self, command)
+
+ def refetch_patch(self, patch):
+ if not self._reject_patch:
+ return self._tool.bugs.fetch_attachment(patch.id())
+
attachment_dictionary = {
"id": patch.id(),
"bug_id": patch.bug_id(),
@@ -196,9 +209,7 @@ class SecondThoughtsCommitQueue(CommitQueue):
"committer_email": "foo at bar.com",
"attacher_email": "Contributer1",
}
- patch = Attachment(attachment_dictionary, None)
- self._tool.bugs.set_override_patch(patch)
- return True
+ return Attachment(attachment_dictionary, None)
class CommitQueueTest(QueuesTest):
@@ -306,11 +317,25 @@ MOCK: update_status: commit-queue Pass
expected_stderr = """MOCK: update_status: commit-queue Applied patch
MOCK: update_status: commit-queue Built patch
MOCK: update_status: commit-queue Passed tests
-MOCK: update_status: commit-queue Landed patch
-MOCK: update_status: commit-queue Pass
+MOCK: update_status: commit-queue Retry
"""
OutputCapture().assert_outputs(self, queue.process_work_item, [MockPatch()], 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=[]
+--- Begin comment ---
+The commit-queue encountered the following flaky tests while processing attachment 197:
+
+['foo/bar.html', 'bar/baz.html']
+
+Please file bugs against the tests. The commit-queue is continuing to process your patch.
+--- End comment ---
+
+"""
+ OutputCapture().assert_outputs(self, queue.report_flaky_tests, [MockPatch(), ["foo/bar.html", "bar/baz.html"]], expected_stderr=expected_stderr)
+
class RietveldUploadQueueTest(QueuesTest):
def test_rietveld_upload_queue(self):
diff --git a/WebKitTools/Scripts/webkitpy/tool/main.py b/WebKitTools/Scripts/webkitpy/tool/main.py
index 721bf59..ce6666e 100755
--- a/WebKitTools/Scripts/webkitpy/tool/main.py
+++ b/WebKitTools/Scripts/webkitpy/tool/main.py
@@ -34,6 +34,7 @@ import threading
from webkitpy.common.checkout.api import Checkout
from webkitpy.common.checkout.scm import default_scm
+from webkitpy.common.config.ports import WebKitPort
from webkitpy.common.net.bugzilla import Bugzilla
from webkitpy.common.net.buildbot import BuildBot
from webkitpy.common.net.rietveld import Rietveld
@@ -52,7 +53,6 @@ from webkitpy.tool.commands.queues import *
from webkitpy.tool.commands.sheriffbot import *
from webkitpy.tool.commands.upload import *
from webkitpy.tool.multicommandtool import MultiCommandTool
-from webkitpy.common.system.deprecated_logging import log
class WebKitPatch(MultiCommandTool):
@@ -74,6 +74,7 @@ class WebKitPatch(MultiCommandTool):
self.buildbot = BuildBot()
self.executive = Executive()
self._irc = None
+ self._port = None
self.user = User()
self._scm = None
self._checkout = None
@@ -92,6 +93,9 @@ class WebKitPatch(MultiCommandTool):
self._checkout = Checkout(self.scm())
return self._checkout
+ def port(self):
+ return self._port
+
def ensure_irc_connected(self, irc_delegate):
if not self._irc:
self._irc = IRCProxy(irc_delegate)
@@ -129,6 +133,8 @@ class WebKitPatch(MultiCommandTool):
self.status_server.set_bot_id(options.bot_id)
if options.irc_password:
self.irc_password = options.irc_password
+ # If options.port is None, we'll get the default port for this platform.
+ self._port = WebKitPort.port(options.port)
def should_execute_command(self, command):
if command.requires_local_commits and not self.scm().supports_local_commits():
diff --git a/WebKitTools/Scripts/webkitpy/tool/mocktool.py b/WebKitTools/Scripts/webkitpy/tool/mocktool.py
index f84ec2b..fb49db9 100644
--- a/WebKitTools/Scripts/webkitpy/tool/mocktool.py
+++ b/WebKitTools/Scripts/webkitpy/tool/mocktool.py
@@ -30,6 +30,7 @@ import os
import threading
from webkitpy.common.config.committers import CommitterList, Reviewer
+from webkitpy.common.config.ports import MacPort
from webkitpy.common.checkout.commitinfo import CommitInfo
from webkitpy.common.checkout.scm import CommitMessage
from webkitpy.common.net.bugzilla import Bug, Attachment
@@ -645,6 +646,10 @@ class MockTool():
def path(self):
return "echo"
+ def port(self):
+ # FIXME: Consider using a mock here.
+ return MacPort
+
class MockBrowser(object):
params = {}
diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/abstractstep.py b/WebKitTools/Scripts/webkitpy/tool/steps/abstractstep.py
index 9ceb2cb..02a876a 100644
--- a/WebKitTools/Scripts/webkitpy/tool/steps/abstractstep.py
+++ b/WebKitTools/Scripts/webkitpy/tool/steps/abstractstep.py
@@ -36,23 +36,15 @@ class AbstractStep(object):
def __init__(self, tool, options):
self._tool = tool
self._options = options
- self._port = None
+ # FIXME: This should use tool.port()
def _run_script(self, script_name, args=None, quiet=False, port=WebKitPort):
log("Running %s" % script_name)
command = [port.script_path(script_name)]
if args:
command.extend(args)
- # FIXME: This should use self.port()
self._tool.executive.run_and_throw_if_fail(command, quiet)
- # FIXME: The port should live on the tool.
- def port(self):
- if self._port:
- return self._port
- self._port = WebKitPort.port(self._options.port)
- return self._port
-
_well_known_keys = {
"diff": lambda self, state: self._tool.scm().create_patch(self._options.git_commit),
"changelogs": lambda self, state: self._tool.checkout().modified_changelogs(self._options.git_commit),
diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/build.py b/WebKitTools/Scripts/webkitpy/tool/steps/build.py
index 456db25..0990b8b 100644
--- a/WebKitTools/Scripts/webkitpy/tool/steps/build.py
+++ b/WebKitTools/Scripts/webkitpy/tool/steps/build.py
@@ -41,7 +41,7 @@ class Build(AbstractStep):
]
def build(self, build_style):
- self._tool.executive.run_and_throw_if_fail(self.port().build_webkit_command(build_style=build_style), self._options.quiet)
+ self._tool.executive.run_and_throw_if_fail(self._tool.port().build_webkit_command(build_style=build_style), self._options.quiet)
def run(self, state):
if not self._options.build:
diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/preparechangelog.py b/WebKitTools/Scripts/webkitpy/tool/steps/preparechangelog.py
index 6a6960a..0382738 100644
--- a/WebKitTools/Scripts/webkitpy/tool/steps/preparechangelog.py
+++ b/WebKitTools/Scripts/webkitpy/tool/steps/preparechangelog.py
@@ -61,7 +61,7 @@ class PrepareChangeLog(AbstractStep):
self._ensure_bug_url(state)
return
os.chdir(self._tool.scm().checkout_root)
- args = [self.port().script_path("prepare-ChangeLog")]
+ args = [self._tool.port().script_path("prepare-ChangeLog")]
if state.get("bug_id"):
args.append("--bug=%s" % state["bug_id"])
if self._options.email:
diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/runtests.py b/WebKitTools/Scripts/webkitpy/tool/steps/runtests.py
index 6fc94a1..dcbfc44 100644
--- a/WebKitTools/Scripts/webkitpy/tool/steps/runtests.py
+++ b/WebKitTools/Scripts/webkitpy/tool/steps/runtests.py
@@ -45,17 +45,17 @@ class RunTests(AbstractStep):
# Run the scripting unit tests first because they're quickest.
log("Running Python unit tests")
- self._tool.executive.run_and_throw_if_fail(self.port().run_python_unittests_command())
+ self._tool.executive.run_and_throw_if_fail(self._tool.port().run_python_unittests_command())
log("Running Perl unit tests")
- self._tool.executive.run_and_throw_if_fail(self.port().run_perl_unittests_command())
+ self._tool.executive.run_and_throw_if_fail(self._tool.port().run_perl_unittests_command())
- javascriptcore_tests_command = self.port().run_javascriptcore_tests_command()
+ javascriptcore_tests_command = self._tool.port().run_javascriptcore_tests_command()
if javascriptcore_tests_command:
log("Running JavaScriptCore tests")
self._tool.executive.run_and_throw_if_fail(javascriptcore_tests_command, quiet=True)
log("Running run-webkit-tests")
- args = self.port().run_webkit_tests_command()
+ args = self._tool.port().run_webkit_tests_command()
if self._options.non_interactive:
args.append("--no-launch-safari")
args.append("--exit-after-n-failures=1")
@@ -63,7 +63,7 @@ class RunTests(AbstractStep):
# FIXME: Hack to work around https://bugs.webkit.org/show_bug.cgi?id=38912
# when running the commit-queue on a mac leopard machine since compositing
# does not work reliably on Leopard due to various graphics driver/system bugs.
- if self.port().name() == "Mac" and self.port().is_leopard():
+ if self._tool.port().name() == "Mac" and self._tool.port().is_leopard():
tests_to_ignore = []
tests_to_ignore.append("compositing")
diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/steps_unittest.py b/WebKitTools/Scripts/webkitpy/tool/steps/steps_unittest.py
index 15f275a..69575db 100644
--- a/WebKitTools/Scripts/webkitpy/tool/steps/steps_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/tool/steps/steps_unittest.py
@@ -57,19 +57,20 @@ class StepsTest(unittest.TestCase):
tool.user.prompt = lambda message: 42
self._run_step(PromptForBugOrTitle, tool=tool)
- def test_runtests_leopard_commit_queue_hack(self):
+ def test_runtests_leopard_commit_queue_hack_step(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):
+ def test_runtests_leopard_commit_queue_hack_command(self):
mock_options = MockOptions()
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
+ tool = MockTool(log_executive=True)
+ tool.port = lambda: mock_port
+ step = RunTests(tool, mock_options)
expected_stderr = """Running Python unit tests
MOCK run_and_throw_if_fail: ['WebKitTools/Scripts/test-webkitpy']
Running Perl unit tests
diff --git a/WebKitTools/Scripts/webkitpy/tool/steps/update.py b/WebKitTools/Scripts/webkitpy/tool/steps/update.py
index 8c290f9..cd1d4d8 100644
--- a/WebKitTools/Scripts/webkitpy/tool/steps/update.py
+++ b/WebKitTools/Scripts/webkitpy/tool/steps/update.py
@@ -42,4 +42,4 @@ class Update(AbstractStep):
if not self._options.update:
return
log("Updating working directory")
- self._tool.executive.run_and_throw_if_fail(self.port().update_webkit_command(), quiet=True)
+ self._tool.executive.run_and_throw_if_fail(self._tool.port().update_webkit_command(), quiet=True)
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list