diff options
| author | Simon Hausmann <simon.hausmann@digia.com> | 2012-11-29 12:18:48 +0100 |
|---|---|---|
| committer | Simon Hausmann <simon.hausmann@digia.com> | 2012-11-29 12:18:57 +0100 |
| commit | 4c01d0526ba4dd8cff0c0ff22a6f0ab5eb973064 (patch) | |
| tree | bed2fe914fe0f7ec70abfb47d2d84af8a3604d09 /Tools/Scripts/webkitpy | |
| parent | 01485457c9a5da3f1121015afd25bb53af77662e (diff) | |
| download | qtwebkit-4c01d0526ba4dd8cff0c0ff22a6f0ab5eb973064.tar.gz | |
Imported WebKit commit c60cfe0fc09efd257aa0111d7b133b02deb8a63e (http://svn.webkit.org/repository/webkit/trunk@136119)
New snapshot that includes the fix for installing the QtWebProcess into libexec
Change-Id: I01344e079cbdac5678c4cba6ffcc05f4597cf0d7
Reviewed-by: Simon Hausmann <simon.hausmann@digia.com>
Diffstat (limited to 'Tools/Scripts/webkitpy')
112 files changed, 1166 insertions, 633 deletions
diff --git a/Tools/Scripts/webkitpy/common/checkout/changelog.py b/Tools/Scripts/webkitpy/common/checkout/changelog.py index ae7b71fc0..c5cf42c79 100644 --- a/Tools/Scripts/webkitpy/common/checkout/changelog.py +++ b/Tools/Scripts/webkitpy/common/checkout/changelog.py @@ -30,13 +30,15 @@ import codecs import fileinput # inplace file editing for set_reviewer_in_changelog +import logging import re import textwrap from webkitpy.common.config.committers import CommitterList from webkitpy.common.config.committers import Account import webkitpy.common.config.urls as config_urls -from webkitpy.common.system.deprecated_logging import log + +_log = logging.getLogger(__name__) # FIXME: parse_bug_id_from_changelog should not be a free function. @@ -162,7 +164,7 @@ class ChangeLogEntry(object): def _parse_entry(self): match = re.match(self.date_line_regexp, self._contents, re.MULTILINE) if not match: - log("WARNING: Creating invalid ChangeLogEntry:\n%s" % self._contents) + _log.warning("Creating invalid ChangeLogEntry:\n%s" % self._contents) # FIXME: group("name") does not seem to be Unicode? Probably due to self._contents not being unicode. self._author_text = match.group("authors") if match else None diff --git a/Tools/Scripts/webkitpy/common/checkout/checkout.py b/Tools/Scripts/webkitpy/common/checkout/checkout.py index 8f450249c..fb686f4d6 100644 --- a/Tools/Scripts/webkitpy/common/checkout/checkout.py +++ b/Tools/Scripts/webkitpy/common/checkout/checkout.py @@ -35,7 +35,6 @@ from webkitpy.common.checkout.scm import CommitMessage from webkitpy.common.checkout.deps import DEPS from webkitpy.common.memoized import memoized from webkitpy.common.system.executive import ScriptError -from webkitpy.common.system.deprecated_logging import log # This class represents the WebKit-specific parts of the checkout (like ChangeLogs). diff --git a/Tools/Scripts/webkitpy/common/checkout/checkout_unittest.py b/Tools/Scripts/webkitpy/common/checkout/checkout_unittest.py index e9c2cddda..a3b47c95e 100644 --- a/Tools/Scripts/webkitpy/common/checkout/checkout_unittest.py +++ b/Tools/Scripts/webkitpy/common/checkout/checkout_unittest.py @@ -259,5 +259,5 @@ class CheckoutTest(unittest.TestCase): mock_patch = Mock() mock_patch.contents = lambda: "foo" mock_patch.reviewer = lambda: None - expected_stderr = "MOCK run_command: ['svn-apply', '--force'], cwd=/mock-checkout, input=foo\n" - OutputCapture().assert_outputs(self, checkout.apply_patch, [mock_patch], expected_stderr=expected_stderr) + expected_logs = "MOCK run_command: ['svn-apply', '--force'], cwd=/mock-checkout, input=foo\n" + OutputCapture().assert_outputs(self, checkout.apply_patch, [mock_patch], expected_logs=expected_logs) diff --git a/Tools/Scripts/webkitpy/common/checkout/deps_mock.py b/Tools/Scripts/webkitpy/common/checkout/deps_mock.py index cb57e8b28..423debae0 100644 --- a/Tools/Scripts/webkitpy/common/checkout/deps_mock.py +++ b/Tools/Scripts/webkitpy/common/checkout/deps_mock.py @@ -26,8 +26,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. +import logging -from webkitpy.common.system.deprecated_logging import log +_log = logging.getLogger(__name__) class MockDEPS(object): @@ -35,4 +36,4 @@ class MockDEPS(object): return 6564 def write_variable(self, name, value): - log("MOCK: MockDEPS.write_variable(%s, %s)" % (name, value)) + _log.info("MOCK: MockDEPS.write_variable(%s, %s)" % (name, value)) diff --git a/Tools/Scripts/webkitpy/common/checkout/scm/detection.py b/Tools/Scripts/webkitpy/common/checkout/scm/detection.py index 44bc9265d..e635b4075 100644 --- a/Tools/Scripts/webkitpy/common/checkout/scm/detection.py +++ b/Tools/Scripts/webkitpy/common/checkout/scm/detection.py @@ -27,14 +27,16 @@ # (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 logging + from webkitpy.common.system.filesystem import FileSystem from webkitpy.common.system.executive import Executive -from webkitpy.common.system.deprecated_logging import log - from .svn import SVN from .git import Git +_log = logging.getLogger(__name__) + class SCMDetector(object): def __init__(self, filesystem, executive): @@ -55,7 +57,7 @@ class SCMDetector(object): script_directory = self._filesystem.dirname(self._filesystem.path_to_module(self.__module__)) scm_system = self.detect_scm_system(script_directory, patch_directories) if scm_system: - log("The current directory (%s) is not a WebKit checkout, using %s" % (cwd, scm_system.checkout_root)) + _log.info("The current directory (%s) is not a WebKit checkout, using %s" % (cwd, scm_system.checkout_root)) else: raise Exception("FATAL: Failed to determine the SCM system for either %s or %s" % (cwd, script_directory)) return scm_system diff --git a/Tools/Scripts/webkitpy/common/checkout/scm/detection_unittest.py b/Tools/Scripts/webkitpy/common/checkout/scm/detection_unittest.py index ecd91259f..1d7484826 100644 --- a/Tools/Scripts/webkitpy/common/checkout/scm/detection_unittest.py +++ b/Tools/Scripts/webkitpy/common/checkout/scm/detection_unittest.py @@ -42,7 +42,7 @@ class SCMDetectorTest(unittest.TestCase): executive = MockExecutive(should_log=True) detector = SCMDetector(filesystem, executive) - expected_stderr = "MOCK run_command: ['svn', 'info'], cwd=/\nMOCK run_command: ['git', 'rev-parse', '--is-inside-work-tree'], cwd=/\n" - scm = OutputCapture().assert_outputs(self, detector.detect_scm_system, ["/"], expected_stderr=expected_stderr) + expected_logs = "MOCK run_command: ['svn', 'info'], cwd=/\nMOCK run_command: ['git', 'rev-parse', '--is-inside-work-tree'], cwd=/\n" + scm = OutputCapture().assert_outputs(self, detector.detect_scm_system, ["/"], expected_logs=expected_logs) self.assertEqual(scm, None) # FIXME: This should make a synthetic tree and test SVN and Git detection in that tree. diff --git a/Tools/Scripts/webkitpy/common/checkout/scm/git.py b/Tools/Scripts/webkitpy/common/checkout/scm/git.py index f68823871..6313256d8 100644 --- a/Tools/Scripts/webkitpy/common/checkout/scm/git.py +++ b/Tools/Scripts/webkitpy/common/checkout/scm/git.py @@ -32,14 +32,12 @@ import os import re from webkitpy.common.memoized import memoized -from webkitpy.common.system.deprecated_logging import log from webkitpy.common.system.executive import Executive, ScriptError from .commitmessage import CommitMessage from .scm import AuthenticationError, SCM, commit_error_handler from .svn import SVN, SVNRepository - _log = logging.getLogger(__name__) @@ -96,7 +94,7 @@ class Git(SCM, SVNRepository): return webkit_dev_thread_url = "https://lists.webkit.org/pipermail/webkit-dev/2010-December/015287.html" - log("Warning: This machine is 64-bit, but the git binary (%s) does not support 64-bit.\nInstall a 64-bit git for better performance, see:\n%s\n" % (git_path, webkit_dev_thread_url)) + _log.warning("This machine is 64-bit, but the git binary (%s) does not support 64-bit.\nInstall a 64-bit git for better performance, see:\n%s\n" % (git_path, webkit_dev_thread_url)) def _run_git(self, command_args, **kwargs): full_command_args = [self.executable_name] + command_args @@ -403,7 +401,7 @@ class Git(SCM, SVNRepository): self._run_git(['commit', '-m', message]) output = self.push_local_commits_to_server(username=username, password=password) except Exception, e: - log("COMMIT FAILED: " + str(e)) + _log.warning("COMMIT FAILED: " + str(e)) output = "Commit failed." commit_succeeded = False finally: diff --git a/Tools/Scripts/webkitpy/common/checkout/scm/scm.py b/Tools/Scripts/webkitpy/common/checkout/scm/scm.py index ee63b7130..7d6e1804d 100644 --- a/Tools/Scripts/webkitpy/common/checkout/scm/scm.py +++ b/Tools/Scripts/webkitpy/common/checkout/scm/scm.py @@ -31,11 +31,13 @@ import logging import re +import sys -from webkitpy.common.system.deprecated_logging import error, log from webkitpy.common.system.executive import Executive, ScriptError from webkitpy.common.system.filesystem import FileSystem +_log = logging.getLogger(__name__) + class CheckoutNeedsUpdate(ScriptError): def __init__(self, script_args, exit_code, output, cwd): @@ -94,7 +96,7 @@ class SCM: if not force_clean: print self.run(self.status_command(), error_handler=Executive.ignore_error, cwd=self.checkout_root) raise ScriptError(message="Working directory has modifications, pass --force-clean or --no-clean to continue.") - log("Cleaning working directory") + _log.info("Cleaning working directory") self.clean_working_directory() def ensure_no_local_commits(self, force): @@ -104,7 +106,8 @@ class SCM: if not len(commits): return if not force: - error("Working directory has local commits, pass --force-clean to continue.") + _log.error("Working directory has local commits, pass --force-clean to continue.") + sys.exit(1) self.discard_local_commits() def run_status_and_extract_filenames(self, status_command, status_regexp): @@ -238,7 +241,8 @@ class SCM: SCM._subclass_must_implement() def commit_locally_with_message(self, message): - error("Your source control manager does not support local commits.") + _log.error("Your source control manager does not support local commits.") + sys.exit(1) def discard_local_commits(self): pass diff --git a/Tools/Scripts/webkitpy/common/checkout/scm/svn.py b/Tools/Scripts/webkitpy/common/checkout/scm/svn.py index 112be057d..1323b702c 100644 --- a/Tools/Scripts/webkitpy/common/checkout/scm/svn.py +++ b/Tools/Scripts/webkitpy/common/checkout/scm/svn.py @@ -35,12 +35,10 @@ import sys import tempfile from webkitpy.common.memoized import memoized -from webkitpy.common.system.deprecated_logging import log from webkitpy.common.system.executive import Executive, ScriptError from .scm import AuthenticationError, SCM, commit_error_handler - _log = logging.getLogger(__name__) @@ -311,8 +309,8 @@ class SVN(SCM, SVNRepository): def apply_reverse_diff(self, revision): # '-c -revision' applies the inverse diff of 'revision' svn_merge_args = ['merge', '--non-interactive', '-c', '-%s' % revision, self._repository_url()] - log("WARNING: svn merge has been known to take more than 10 minutes to complete. It is recommended you use git for rollouts.") - log("Running 'svn %s'" % " ".join(svn_merge_args)) + _log.warning("svn merge has been known to take more than 10 minutes to complete. It is recommended you use git for rollouts.") + _log.debug("Running 'svn %s'" % " ".join(svn_merge_args)) # FIXME: Should this use cwd=self.checkout_root? self._run_svn(svn_merge_args) diff --git a/Tools/Scripts/webkitpy/common/config/committers.py b/Tools/Scripts/webkitpy/common/config/committers.py index 92a6a1ead..e49eebf52 100644 --- a/Tools/Scripts/webkitpy/common/config/committers.py +++ b/Tools/Scripts/webkitpy/common/config/committers.py @@ -201,6 +201,7 @@ committers_unable_to_review = [ Committer("Andrei Popescu", "andreip@google.com", "andreip"), Committer("Andrew Wellington", ["andrew@webkit.org", "proton@wiretapped.net"], "proton"), Committer("Andrew Scherkus", "scherkus@chromium.org", "scherkus"), + Committer("Andrey Adaykin", "aandrey@chromium.org", "aandrey"), Committer("Andrey Kosyakov", "caseq@chromium.org", "caseq"), Committer("Andras Becsi", ["abecsi@webkit.org", "andras.becsi@digia.com"], "bbandix"), Committer("Andy Wingo", "wingo@igalia.com", "wingo"), @@ -229,6 +230,7 @@ committers_unable_to_review = [ Committer("Chris Guillory", ["ctguil@chromium.org", "chris.guillory@google.com"], "ctguil"), Committer("Chris Petersen", "cpetersen@apple.com", "cpetersen"), Committer("Christian Dywan", ["christian@twotoasts.de", "christian@webkit.org", "christian@lanedo.com"]), + Committer("Christophe Dumez", ["christophe.dumez@intel.com", "dchris@gmail.com"], "chris-qBT"), Committer("Collin Jackson", "collinj@webkit.org", "collinjackson"), Committer("Cris Neckar", "cdn@chromium.org", "cneckar"), Committer("Dan Winship", "danw@gnome.org", "danw"), @@ -325,6 +327,7 @@ committers_unable_to_review = [ Committer("Leandro Gracia Gil", "leandrogracia@chromium.org", "leandrogracia"), Committer("Leandro Pereira", ["leandro@profusion.mobi", "leandro@webkit.org"], "acidx"), Committer("Leo Yang", ["leoyang@rim.com", "leoyang@webkit.org", "leoyang.webkit@gmail.com"], "leoyang"), + Committer("Li Yin", ["li.yin@intel.com"], "liyin"), Committer("Lucas De Marchi", ["demarchi@webkit.org", "lucas.demarchi@profusion.mobi"], "demarchi"), Committer("Lucas Forschler", ["lforschler@apple.com"], "lforschler"), Committer("Luciano Wolf", "luciano.wolf@openbossa.org", "luck"), @@ -336,6 +339,7 @@ committers_unable_to_review = [ Committer("Mark Lam", "mark.lam@apple.com", "mlam"), Committer("Mary Wu", ["mary.wu@torchmobile.com.cn", "wwendy2007@gmail.com"], "marywu"), Committer("Matt Delaney", "mdelaney@apple.com"), + Committer("Matt Falkenhagen", "falken@chromium.org", "falken"), Committer("Matt Lilek", ["mlilek@apple.com", "webkit@mattlilek.com", "pewtermoose@webkit.org"], "pewtermoose"), Committer("Matt Perry", "mpcomplete@chromium.org"), Committer("Maxime Britto", ["maxime.britto@gmail.com", "britto@apple.com"]), @@ -355,6 +359,7 @@ committers_unable_to_review = [ Committer("Nat Duca", ["nduca@chromium.org", "nduca@google.com"], "nduca"), Committer("Nayan Kumar K", ["nayankk@motorola.com", "nayankk@gmail.com"], "xc0ffee"), Committer("Nico Weber", ["thakis@chromium.org", "thakis@google.com"], "thakis"), + Committer("Nima Ghanavatian", ["nghanavatian@rim.com", "nima.ghanavatian@gmail.com"], "nghanavatian"), Committer("Noel Gordon", ["noel.gordon@gmail.com", "noel@chromium.org", "noel@google.com"], "noel"), Committer("Pam Greene", "pam@chromium.org", "pamg"), Committer("Patrick Gansterer", ["paroga@paroga.com", "paroga@webkit.org"], "paroga"), @@ -392,6 +397,7 @@ committers_unable_to_review = [ Committer("Takashi Sakamoto", "tasak@google.com", "tasak"), Committer("Takashi Toyoshima", "toyoshim@chromium.org", "toyoshim"), Committer("Terry Anderson", "tdanderson@chromium.org", "tdanderson"), + Committer("Thiago Marcos P. Santos", ["tmpsantos@gmail.com", "thiago.santos@intel.com"], "tmpsantos"), Committer("Thomas Sepez", "tsepez@chromium.org", "tsepez"), Committer("Tom Hudson", ["tomhudson@google.com", "tomhudson@chromium.org"], "tomhudson"), Committer("Tom Zakrajsek", "tomz@codeaurora.org", "tomz"), @@ -406,6 +412,7 @@ committers_unable_to_review = [ Committer("Vincent Scheib", "scheib@chromium.org", "scheib"), Committer("Vineet Chaudhary", "rgf748@motorola.com", "vineetc"), Committer("Vitaly Repeshko", "vitalyr@chromium.org"), + Committer("Vivek Galatage", ["vivekg@webkit.org", "vivek.vg@samsung.com"], "vivekg"), Committer("William Siegrist", "wsiegrist@apple.com", "wms"), Committer("W. James MacLean", "wjmaclean@chromium.org", "seumas"), Committer("Xianzhu Wang", ["wangxianzhu@chromium.org", "phnixwxz@gmail.com", "wangxianzhu@google.com"], "wangxianzhu"), diff --git a/Tools/Scripts/webkitpy/common/net/bugzilla/attachment.py b/Tools/Scripts/webkitpy/common/net/bugzilla/attachment.py index 6e10d65a9..c749a1512 100644 --- a/Tools/Scripts/webkitpy/common/net/bugzilla/attachment.py +++ b/Tools/Scripts/webkitpy/common/net/bugzilla/attachment.py @@ -28,8 +28,11 @@ # (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 logging + from webkitpy.common.memoized import memoized -from webkitpy.common.system.deprecated_logging import log + +_log = logging.getLogger(__name__) class Attachment(object): @@ -102,7 +105,7 @@ class Attachment(object): "%s_by_email" % flag)(email) if committer: return committer - log("Warning, attachment %s on bug %s has invalid %s (%s)" % ( + _log.warning("Warning, attachment %s on bug %s has invalid %s (%s)" % ( self._attachment_dictionary['id'], self._attachment_dictionary['bug_id'], flag, email)) diff --git a/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py b/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py index 651e1b374..957f04dca 100644 --- a/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py +++ b/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla.py @@ -30,6 +30,7 @@ # # WebKit's Python module for interacting with Bugzilla +import logging import mimetypes import re import StringIO @@ -41,13 +42,14 @@ from datetime import datetime # used in timestamp() from .attachment import Attachment from .bug import Bug -from webkitpy.common.system.deprecated_logging import log from webkitpy.common.config import committers import webkitpy.common.config.urls as config_urls from webkitpy.common.net.credentials import Credentials from webkitpy.common.system.user import User from webkitpy.thirdparty.BeautifulSoup import BeautifulSoup, BeautifulStoneSoup, SoupStrainer +_log = logging.getLogger(__name__) + class EditUsersParser(object): def __init__(self): @@ -435,7 +437,7 @@ class Bugzilla(object): def _fetch_bug_page(self, bug_id): bug_url = self.bug_url_for_bug_id(bug_id, xml=True) - log("Fetching: %s" % bug_url) + _log.info("Fetching: %s" % bug_url) return self.browser.open(bug_url) def fetch_bug_dictionary(self, bug_id): @@ -472,7 +474,7 @@ class Bugzilla(object): self.authenticate() attachment_url = self.attachment_url_for_id(attachment_id, 'edit') - log("Fetching: %s" % attachment_url) + _log.info("Fetching: %s" % attachment_url) page = self.browser.open(attachment_url) return self._parse_bug_id_from_attachment_page(page) @@ -503,7 +505,7 @@ class Bugzilla(object): attempts += 1 username, password = credentials.read_credentials() - log("Logging in as %s..." % username) + _log.info("Logging in as %s..." % username) self.browser.open(config_urls.bug_server_url + "index.cgi?GoAheadAndLogIn=1") self.browser.select_form(name="login") @@ -519,7 +521,7 @@ class Bugzilla(object): errorMessage = "Bugzilla login failed: %s" % match.group(1) # raise an exception only if this was the last attempt if attempts < 5: - log(errorMessage) + _log.error(errorMessage) else: raise Exception(errorMessage) else: @@ -532,10 +534,10 @@ class Bugzilla(object): user = self.committers.account_by_email(self.username) mark_for_commit_queue = True if not user: - log("Your Bugzilla login is not listed in committers.py. Uploading with cq? instead of cq+") + _log.warning("Your Bugzilla login is not listed in committers.py. Uploading with cq? instead of cq+") mark_for_landing = False elif not user.can_commit: - log("You're not a committer yet or haven't updated committers.py yet. Uploading with cq? instead of cq+") + _log.warning("You're not a committer yet or haven't updated committers.py yet. Uploading with cq? instead of cq+") mark_for_landing = False if mark_for_landing: @@ -585,14 +587,14 @@ class Bugzilla(object): def add_attachment_to_bug(self, bug_id, file_or_string, description, filename=None, comment_text=None, mimetype=None): self.authenticate() - log('Adding attachment "%s" to %s' % (description, self.bug_url_for_bug_id(bug_id))) + _log.info('Adding attachment "%s" to %s' % (description, self.bug_url_for_bug_id(bug_id))) self.browser.open(self.add_attachment_url(bug_id)) self.browser.select_form(name="entryform") file_object = self._file_object_for_upload(file_or_string) filename = filename or self._filename_for_upload(file_object, bug_id) self._fill_attachment_form(description, file_object, filename=filename, mimetype=mimetype) if comment_text: - log(comment_text) + _log.info(comment_text) self.browser['comment'] = comment_text self.browser.submit() @@ -607,7 +609,7 @@ class Bugzilla(object): mark_for_commit_queue=False, mark_for_landing=False): self.authenticate() - log('Adding patch "%s" to %s' % (description, self.bug_url_for_bug_id(bug_id))) + _log.info('Adding patch "%s" to %s' % (description, self.bug_url_for_bug_id(bug_id))) self.browser.open(self.add_attachment_url(bug_id)) self.browser.select_form(name="entryform") @@ -621,7 +623,7 @@ class Bugzilla(object): is_patch=True, filename=filename) if comment_text: - log(comment_text) + _log.info(comment_text) self.browser['comment'] = comment_text self.browser.submit() @@ -658,7 +660,7 @@ class Bugzilla(object): mark_for_commit_queue=False): self.authenticate() - log('Creating bug with title "%s"' % bug_title) + _log.info('Creating bug with title "%s"' % bug_title) self.browser.open(config_urls.bug_server_url + "enter_bug.cgi?product=WebKit") self.browser.select_form(name="Create") component_items = self.browser.find_control('component').items @@ -694,8 +696,8 @@ class Bugzilla(object): response = self.browser.submit() bug_id = self._check_create_bug_response(response.read()) - log("Bug %s created." % bug_id) - log("%sshow_bug.cgi?id=%s" % (config_urls.bug_server_url, bug_id)) + _log.info("Bug %s created." % bug_id) + _log.info("%sshow_bug.cgi?id=%s" % (config_urls.bug_server_url, bug_id)) return bug_id def _find_select_element_for_flag(self, flag_name): @@ -714,7 +716,7 @@ class Bugzilla(object): comment_text = "Clearing flags on attachment: %s" % attachment_id if additional_comment_text: comment_text += "\n\n%s" % additional_comment_text - log(comment_text) + _log.info(comment_text) self.browser.open(self.attachment_url_for_id(attachment_id, 'edit')) self.browser.select_form(nr=1) @@ -737,7 +739,7 @@ class Bugzilla(object): # FIXME: additional_comment_text seems useless and should be merged into comment-text. if additional_comment_text: comment_text += "\n\n%s" % additional_comment_text - log(comment_text) + _log.info(comment_text) self.browser.open(self.attachment_url_for_id(attachment_id, 'edit')) self.browser.select_form(nr=1) @@ -754,7 +756,7 @@ class Bugzilla(object): def obsolete_attachment(self, attachment_id, comment_text=None): self.authenticate() - log("Obsoleting attachment: %s" % attachment_id) + _log.info("Obsoleting attachment: %s" % attachment_id) self.browser.open(self.attachment_url_for_id(attachment_id, 'edit')) self.browser.select_form(nr=1) self.browser.find_control('isobsolete').items[0].selected = True @@ -762,7 +764,7 @@ class Bugzilla(object): self._find_select_element_for_flag('review').value = ("X",) self._find_select_element_for_flag('commit-queue').value = ("X",) if comment_text: - log(comment_text) + _log.info(comment_text) # Bugzilla has two textareas named 'comment', one is somehow # hidden. We want the first. self.browser.set_value(comment_text, name='comment', nr=0) @@ -771,7 +773,7 @@ class Bugzilla(object): def add_cc_to_bug(self, bug_id, email_address_list): self.authenticate() - log("Adding %s to the CC list for bug %s" % (email_address_list, bug_id)) + _log.info("Adding %s to the CC list for bug %s" % (email_address_list, bug_id)) self.browser.open(self.bug_url_for_bug_id(bug_id)) self.browser.select_form(name="changeform") self.browser["newcc"] = ", ".join(email_address_list) @@ -780,7 +782,7 @@ class Bugzilla(object): def post_comment_to_bug(self, bug_id, comment_text, cc=None): self.authenticate() - log("Adding comment to bug %s" % bug_id) + _log.info("Adding comment to bug %s" % bug_id) self.browser.open(self.bug_url_for_bug_id(bug_id)) self.browser.select_form(name="changeform") self.browser["comment"] = comment_text @@ -791,7 +793,7 @@ class Bugzilla(object): def close_bug_as_fixed(self, bug_id, comment_text=None): self.authenticate() - log("Closing bug %s as fixed" % bug_id) + _log.info("Closing bug %s as fixed" % bug_id) self.browser.open(self.bug_url_for_bug_id(bug_id)) self.browser.select_form(name="changeform") if comment_text: @@ -809,12 +811,12 @@ class Bugzilla(object): if not assignee: assignee = self.username - log("Assigning bug %s to %s" % (bug_id, assignee)) + _log.info("Assigning bug %s to %s" % (bug_id, assignee)) self.browser.open(self.bug_url_for_bug_id(bug_id)) self.browser.select_form(name="changeform") if not self._has_control(self.browser, "assigned_to"): - log("""Failed to assign bug to you (can't find assigned_to) control. + _log.warning("""Failed to assign bug to you (can't find assigned_to) control. Do you have EditBugs privileges at bugs.webkit.org? https://bugs.webkit.org/userprefs.cgi?tab=permissions @@ -823,7 +825,7 @@ for someone to add EditBugs to your bugs.webkit.org account.""") return if comment_text: - log(comment_text) + _log.info(comment_text) self.browser["comment"] = comment_text self.browser["assigned_to"] = assignee self.browser.submit() @@ -831,10 +833,10 @@ for someone to add EditBugs to your bugs.webkit.org account.""") def reopen_bug(self, bug_id, comment_text): self.authenticate() - log("Re-opening bug %s" % bug_id) + _log.info("Re-opening bug %s" % bug_id) # Bugzilla requires a comment when re-opening a bug, so we know it will # never be None. - log(comment_text) + _log.info(comment_text) self.browser.open(self.bug_url_for_bug_id(bug_id)) self.browser.select_form(name="changeform") bug_status = self.browser.find_control("bug_status", type="select") @@ -851,6 +853,6 @@ for someone to add EditBugs to your bugs.webkit.org account.""") else: # FIXME: This logic is slightly backwards. We won't print this # message if the bug is already open with state "UNCONFIRMED". - log("Did not reopen bug %s, it appears to already be open with status %s." % (bug_id, bug_status.value)) + _log.info("Did not reopen bug %s, it appears to already be open with status %s." % (bug_id, bug_status.value)) self.browser['comment'] = comment_text self.browser.submit() diff --git a/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla_mock.py b/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla_mock.py index 71b080ce9..473a9fa6e 100644 --- a/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla_mock.py +++ b/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla_mock.py @@ -27,12 +27,13 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. import datetime +import logging from .bug import Bug from .attachment import Attachment from webkitpy.common.config.committers import CommitterList, Reviewer -from webkitpy.common.system.deprecated_logging import log +_log = logging.getLogger(__name__) def _id_to_object_dictionary(*objects): @@ -328,15 +329,15 @@ class MockBugzilla(object): blocked=None, mark_for_review=False, mark_for_commit_queue=False): - log("MOCK create_bug") - log("bug_title: %s" % bug_title) - log("bug_description: %s" % bug_description) + _log.info("MOCK create_bug") + _log.info("bug_title: %s" % bug_title) + _log.info("bug_description: %s" % bug_description) if component: - log("component: %s" % component) + _log.info("component: %s" % component) if cc: - log("cc: %s" % cc) + _log.info("cc: %s" % cc) if blocked: - log("blocked: %s" % blocked) + _log.info("blocked: %s" % blocked) return 60001 def quips(self): @@ -374,11 +375,11 @@ class MockBugzilla(object): return "%s/%s%s" % (self.bug_server_url, attachment_id, action_param) def reassign_bug(self, bug_id, assignee=None, comment_text=None): - log("MOCK reassign_bug: bug_id=%s, assignee=%s" % (bug_id, assignee)) + _log.info("MOCK reassign_bug: bug_id=%s, assignee=%s" % (bug_id, assignee)) if comment_text: - log("-- Begin comment --") - log(comment_text) - log("-- End comment --") + _log.info("-- Begin comment --") + _log.info(comment_text) + _log.info("-- End comment --") def set_flag_on_attachment(self, attachment_id, @@ -386,20 +387,20 @@ class MockBugzilla(object): flag_value, comment_text=None, additional_comment_text=None): - log("MOCK setting flag '%s' to '%s' on attachment '%s' with comment '%s' and additional comment '%s'" % ( - flag_name, flag_value, attachment_id, comment_text, additional_comment_text)) + _log.info("MOCK setting flag '%s' to '%s' on attachment '%s' with comment '%s' and additional comment '%s'" % ( + flag_name, flag_value, attachment_id, comment_text, additional_comment_text)) def post_comment_to_bug(self, bug_id, comment_text, cc=None): - log("MOCK bug comment: bug_id=%s, cc=%s\n--- Begin comment ---\n%s\n--- End comment ---\n" % ( - bug_id, cc, comment_text)) + _log.info("MOCK bug comment: bug_id=%s, cc=%s\n--- Begin comment ---\n%s\n--- End comment ---\n" % ( + bug_id, cc, comment_text)) def add_attachment_to_bug(self, bug_id, file_or_string, description, filename=None, comment_text=None, mimetype=None): - log("MOCK add_attachment_to_bug: bug_id=%s, description=%s filename=%s mimetype=%s" % - (bug_id, description, filename, mimetype)) + _log.info("MOCK add_attachment_to_bug: bug_id=%s, description=%s filename=%s mimetype=%s" % + (bug_id, description, filename, mimetype)) if comment_text: - log("-- Begin comment --") - log(comment_text) - log("-- End comment --") + _log.info("-- Begin comment --") + _log.info(comment_text) + _log.info("-- End comment --") def add_patch_to_bug(self, bug_id, @@ -409,12 +410,12 @@ class MockBugzilla(object): mark_for_review=False, mark_for_commit_queue=False, mark_for_landing=False): - log("MOCK add_patch_to_bug: bug_id=%s, description=%s, mark_for_review=%s, mark_for_commit_queue=%s, mark_for_landing=%s" % - (bug_id, description, mark_for_review, mark_for_commit_queue, mark_for_landing)) + _log.info("MOCK add_patch_to_bug: bug_id=%s, description=%s, mark_for_review=%s, mark_for_commit_queue=%s, mark_for_landing=%s" % + (bug_id, description, mark_for_review, mark_for_commit_queue, mark_for_landing)) if comment_text: - log("-- Begin comment --") - log(comment_text) - log("-- End comment --") + _log.info("-- Begin comment --") + _log.info(comment_text) + _log.info("-- End comment --") def add_cc_to_bug(self, bug_id, ccs): pass @@ -423,7 +424,7 @@ class MockBugzilla(object): pass def reopen_bug(self, bug_id, message): - log("MOCK reopen_bug %s with comment '%s'" % (bug_id, message)) + _log.info("MOCK reopen_bug %s with comment '%s'" % (bug_id, message)) def close_bug_as_fixed(self, bug_id, message): pass diff --git a/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla_unittest.py b/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla_unittest.py index 538d39e85..90e4c83fc 100644 --- a/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla_unittest.py +++ b/Tools/Scripts/webkitpy/common/net/bugzilla/bugzilla_unittest.py @@ -250,8 +250,8 @@ Ignore this bug. Just for testing failure modes of webkit-patch and the commit- bugzilla = Bugzilla() bugzilla.browser = MockBrowser() bugzilla.authenticate = lambda: None - expected_stderr = "Adding ['adam@example.com'] to the CC list for bug 42\n" - OutputCapture().assert_outputs(self, bugzilla.add_cc_to_bug, [42, ["adam@example.com"]], expected_stderr=expected_stderr) + expected_logs = "Adding ['adam@example.com'] to the CC list for bug 42\n" + OutputCapture().assert_outputs(self, bugzilla.add_cc_to_bug, [42, ["adam@example.com"]], expected_logs=expected_logs) def _mock_control_item(self, name): mock_item = Mock() @@ -264,23 +264,23 @@ Ignore this bug. Just for testing failure modes of webkit-patch and the commit- mock_control.value = [item_names[selected_index]] if item_names else None return lambda name, type: mock_control - def _assert_reopen(self, item_names=None, selected_index=None, extra_stderr=None): + def _assert_reopen(self, item_names=None, selected_index=None, extra_logs=None): bugzilla = Bugzilla() bugzilla.browser = MockBrowser() bugzilla.authenticate = lambda: None mock_find_control = self._mock_find_control(item_names, selected_index) bugzilla.browser.find_control = mock_find_control - expected_stderr = "Re-opening bug 42\n['comment']\n" - if extra_stderr: - expected_stderr += extra_stderr - OutputCapture().assert_outputs(self, bugzilla.reopen_bug, [42, ["comment"]], expected_stderr=expected_stderr) + expected_logs = "Re-opening bug 42\n['comment']\n" + if extra_logs: + expected_logs += extra_logs + OutputCapture().assert_outputs(self, bugzilla.reopen_bug, [42, ["comment"]], expected_logs=expected_logs) def test_reopen_bug(self): self._assert_reopen(item_names=["REOPENED", "RESOLVED", "CLOSED"], selected_index=1) self._assert_reopen(item_names=["UNCONFIRMED", "RESOLVED", "CLOSED"], selected_index=1) - extra_stderr = "Did not reopen bug 42, it appears to already be open with status ['NEW'].\n" - self._assert_reopen(item_names=["NEW", "RESOLVED"], selected_index=0, extra_stderr=extra_stderr) + extra_logs = "Did not reopen bug 42, it appears to already be open with status ['NEW'].\n" + self._assert_reopen(item_names=["NEW", "RESOLVED"], selected_index=0, extra_logs=extra_logs) def test_file_object_for_upload(self): bugzilla = Bugzilla() diff --git a/Tools/Scripts/webkitpy/common/net/buildbot/buildbot_mock.py b/Tools/Scripts/webkitpy/common/net/buildbot/buildbot_mock.py index f5b6042b3..d20bdb75c 100644 --- a/Tools/Scripts/webkitpy/common/net/buildbot/buildbot_mock.py +++ b/Tools/Scripts/webkitpy/common/net/buildbot/buildbot_mock.py @@ -26,7 +26,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 webkitpy.common.system.deprecated_logging import log +import logging + +_log = logging.getLogger(__name__) class MockBuild(object): @@ -55,7 +57,7 @@ class MockBuilder(object): return self.accumulated_results_url() def force_build(self, username, comments): - log("MOCK: force_build: name=%s, username=%s, comments=%s" % ( + _log.info("MOCK: force_build: name=%s, username=%s, comments=%s" % ( self._name, username, comments)) diff --git a/Tools/Scripts/webkitpy/common/net/credentials.py b/Tools/Scripts/webkitpy/common/net/credentials.py index 21aeaeafe..7038b7e3c 100644 --- a/Tools/Scripts/webkitpy/common/net/credentials.py +++ b/Tools/Scripts/webkitpy/common/net/credentials.py @@ -29,6 +29,7 @@ # # Python module for reading stored web credentials from the OS. +import logging import os import platform import re @@ -36,7 +37,6 @@ import re from webkitpy.common.checkout.scm import Git from webkitpy.common.system.executive import Executive, ScriptError from webkitpy.common.system.user import User -from webkitpy.common.system.deprecated_logging import log try: # Use keyring, a cross platform keyring interface, as a fallback: @@ -45,6 +45,8 @@ try: except ImportError: keyring = None +_log = logging.getLogger(__name__) + class Credentials(object): _environ_prefix = "webkit_bugzilla_" @@ -98,15 +100,15 @@ class Credentials(object): if username: security_command += ["-a", username] - log("Reading Keychain for %s account and password. " - "Click \"Allow\" to continue..." % self.host) + _log.info("Reading Keychain for %s account and password. " + "Click \"Allow\" to continue..." % self.host) try: return self.executive.run_command(security_command) except ScriptError: # Failed to either find a keychain entry or somekind of OS-related # error occured (for instance, couldn't find the /usr/sbin/security # command). - log("Could not find a keychain entry for %s." % self.host) + _log.error("Could not find a keychain entry for %s." % self.host) return None def _credentials_from_keychain(self, username=None): diff --git a/Tools/Scripts/webkitpy/common/net/credentials_unittest.py b/Tools/Scripts/webkitpy/common/net/credentials_unittest.py index 15682f3b8..3659d69d1 100644 --- a/Tools/Scripts/webkitpy/common/net/credentials_unittest.py +++ b/Tools/Scripts/webkitpy/common/net/credentials_unittest.py @@ -116,8 +116,8 @@ password: "SECRETSAUCE" executive_mock = Mock() credentials = MockedCredentials("example.com", executive=executive_mock) - expected_stderr = "Reading Keychain for example.com account and password. Click \"Allow\" to continue...\n" - OutputCapture().assert_outputs(self, credentials._run_security_tool, [username], expected_stderr=expected_stderr) + expected_logs = "Reading Keychain for example.com account and password. Click \"Allow\" to continue...\n" + OutputCapture().assert_outputs(self, credentials._run_security_tool, [username], expected_logs=expected_logs) security_args = ["/usr/bin/security", "find-internet-password", "-g", "-s", "example.com"] if username: diff --git a/Tools/Scripts/webkitpy/common/net/irc/irc_mock.py b/Tools/Scripts/webkitpy/common/net/irc/irc_mock.py index 734be0670..b2ae0715a 100644 --- a/Tools/Scripts/webkitpy/common/net/irc/irc_mock.py +++ b/Tools/Scripts/webkitpy/common/net/irc/irc_mock.py @@ -26,12 +26,14 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -from webkitpy.common.system.deprecated_logging import log +import logging + +_log = logging.getLogger(__name__) class MockIRC(object): def post(self, message): - log("MOCK: irc.post: %s" % message) + _log.info("MOCK: irc.post: %s" % message) def disconnect(self): - log("MOCK: irc.disconnect") + _log.info("MOCK: irc.disconnect") diff --git a/Tools/Scripts/webkitpy/common/net/irc/ircproxy.py b/Tools/Scripts/webkitpy/common/net/irc/ircproxy.py index 13348b4af..521f6f761 100644 --- a/Tools/Scripts/webkitpy/common/net/irc/ircproxy.py +++ b/Tools/Scripts/webkitpy/common/net/irc/ircproxy.py @@ -26,11 +26,13 @@ # (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 logging import threading from webkitpy.common.net.irc.ircbot import IRCBot from webkitpy.common.thread.threadedmessagequeue import ThreadedMessageQueue -from webkitpy.common.system.deprecated_logging import log + +_log = logging.getLogger(__name__) class _IRCThread(threading.Thread): @@ -48,7 +50,7 @@ class _IRCThread(threading.Thread): class IRCProxy(object): def __init__(self, irc_delegate, irc_bot=IRCBot): - log("Connecting to IRC") + _log.info("Connecting to IRC") self._message_queue = ThreadedMessageQueue() self._child_thread = _IRCThread(self._message_queue, irc_delegate, irc_bot) self._child_thread.start() @@ -57,6 +59,6 @@ class IRCProxy(object): self._message_queue.post(message) def disconnect(self): - log("Disconnecting from IRC...") + _log.info("Disconnecting from IRC...") self._message_queue.stop() self._child_thread.join() diff --git a/Tools/Scripts/webkitpy/common/net/irc/ircproxy_unittest.py b/Tools/Scripts/webkitpy/common/net/irc/ircproxy_unittest.py index b44ce400b..bce9d855d 100644 --- a/Tools/Scripts/webkitpy/common/net/irc/ircproxy_unittest.py +++ b/Tools/Scripts/webkitpy/common/net/irc/ircproxy_unittest.py @@ -39,5 +39,5 @@ class IRCProxyTest(unittest.TestCase): proxy.post("hello") proxy.disconnect() - expected_stderr = "Connecting to IRC\nDisconnecting from IRC...\n" - OutputCapture().assert_outputs(self, fun, expected_stderr=expected_stderr) + expected_logs = "Connecting to IRC\nDisconnecting from IRC...\n" + OutputCapture().assert_outputs(self, fun, expected_logs=expected_logs) diff --git a/Tools/Scripts/webkitpy/common/net/layouttestresults.py b/Tools/Scripts/webkitpy/common/net/layouttestresults.py index f0d807edc..74322c757 100644 --- a/Tools/Scripts/webkitpy/common/net/layouttestresults.py +++ b/Tools/Scripts/webkitpy/common/net/layouttestresults.py @@ -29,12 +29,15 @@ # A module for parsing results.html files generated by old-run-webkit-tests # This class is one big hack and only needs to exist until we transition to new-run-webkit-tests. +import logging + from webkitpy.common.net.resultsjsonparser import ResultsJSONParser -from webkitpy.common.system.deprecated_logging import log from webkitpy.thirdparty.BeautifulSoup import BeautifulSoup, SoupStrainer from webkitpy.layout_tests.models import test_results from webkitpy.layout_tests.models import test_failures +_log = logging.getLogger(__name__) + # 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. @@ -78,7 +81,7 @@ class ORWTResultsHTMLParser(object): elif anchor_text in ["expected", "actual", "diff", "pretty diff"]: failures.add(test_failures.FailureTextMismatch()) else: - log("Unhandled link text in results.html parsing: %s. Please file a bug against webkitpy." % anchor_text) + _log.warning("Unhandled link text in results.html parsing: %s. Please file a bug against webkitpy." % anchor_text) # FIXME: Its possible the row contained no links due to ORWT brokeness. # We should probably assume some type of failure anyway. return failures diff --git a/Tools/Scripts/webkitpy/common/net/layouttestresults_unittest.py b/Tools/Scripts/webkitpy/common/net/layouttestresults_unittest.py index 462a88c66..4131bdf85 100644 --- a/Tools/Scripts/webkitpy/common/net/layouttestresults_unittest.py +++ b/Tools/Scripts/webkitpy/common/net/layouttestresults_unittest.py @@ -122,8 +122,8 @@ class ORWTResultsHTMLParserTest(unittest.TestCase): self.assertEqual(type(sorted(failures)[0]), test_failures.FailureImageHashMismatch) row = BeautifulSoup("<tr><td><a>test.hml</a><a>foo</a></td></tr>") - expected_stderr = "Unhandled link text in results.html parsing: foo. Please file a bug against webkitpy.\n" - OutputCapture().assert_outputs(self, ORWTResultsHTMLParser._failures_from_fail_row, [row], expected_stderr=expected_stderr) + expected_logs = "Unhandled link text in results.html parsing: foo. Please file a bug against webkitpy.\n" + OutputCapture().assert_outputs(self, ORWTResultsHTMLParser._failures_from_fail_row, [row], expected_logs=expected_logs) class LayoutTestResultsTest(unittest.TestCase): diff --git a/Tools/Scripts/webkitpy/common/net/networktransaction.py b/Tools/Scripts/webkitpy/common/net/networktransaction.py index 03b143267..60acaaba3 100644 --- a/Tools/Scripts/webkitpy/common/net/networktransaction.py +++ b/Tools/Scripts/webkitpy/common/net/networktransaction.py @@ -30,9 +30,6 @@ import logging import time import urllib2 -from webkitpy.common.system.deprecated_logging import log - - _log = logging.getLogger(__name__) diff --git a/Tools/Scripts/webkitpy/common/net/resultsjsonparser.py b/Tools/Scripts/webkitpy/common/net/resultsjsonparser.py index 42ce56a17..1a2a70f4b 100644 --- a/Tools/Scripts/webkitpy/common/net/resultsjsonparser.py +++ b/Tools/Scripts/webkitpy/common/net/resultsjsonparser.py @@ -26,16 +26,17 @@ # (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 json +import logging from webkitpy.common.memoized import memoized -from webkitpy.common.system.deprecated_logging import log # FIXME: common should never import from new-run-webkit-tests, one of these files needs to move. from webkitpy.layout_tests.layout_package import json_results_generator from webkitpy.layout_tests.models import test_expectations, test_results, test_failures from webkitpy.layout_tests.models.test_expectations import TestExpectations +_log = logging.getLogger(__name__) + # These are helper functions for navigating the results json structure. def for_each_test(tree, handler, prefix=''): @@ -83,7 +84,7 @@ class JSONTestResult(object): def _tokenize(self, results_string): tokens = map(TestExpectations.expectation_from_string, results_string.split(' ')) if None in tokens: - log("Unrecognized result in %s" % results_string) + _log.warning("Unrecognized result in %s" % results_string) return set(tokens) @memoized @@ -123,7 +124,7 @@ class JSONTestResult(object): elif actual == test_expectations.MISSING: return [test_failures.FailureMissingResult(), test_failures.FailureMissingImageHash(), test_failures.FailureMissingImage()] else: - log("Failed to handle: %s" % self._result_dict['actual']) + _log.warning("Failed to handle: %s" % self._result_dict['actual']) return [] def _failures(self): diff --git a/Tools/Scripts/webkitpy/common/net/statusserver.py b/Tools/Scripts/webkitpy/common/net/statusserver.py index 2bda1ce88..99850f55d 100644 --- a/Tools/Scripts/webkitpy/common/net/statusserver.py +++ b/Tools/Scripts/webkitpy/common/net/statusserver.py @@ -29,7 +29,6 @@ # This the client designed to talk to Tools/QueueStatusServer. from webkitpy.common.net.networktransaction import NetworkTransaction -from webkitpy.common.system.deprecated_logging import log from webkitpy.thirdparty.BeautifulSoup import BeautifulSoup import logging @@ -145,11 +144,11 @@ class StatusServer: return NetworkTransaction().run(lambda: self._post_work_items_to_server(queue_name, work_items)) def update_status(self, queue_name, status, patch=None, results_file=None): - log(status) + _log.info(status) return NetworkTransaction().run(lambda: self._post_status_to_server(queue_name, status, patch, results_file)) def update_svn_revision(self, svn_revision_number, broken_bot): - log("SVN revision: %s broke %s" % (svn_revision_number, broken_bot)) + _log.info("SVN revision: %s broke %s" % (svn_revision_number, broken_bot)) return NetworkTransaction().run(lambda: self._post_svn_revision_to_server(svn_revision_number, broken_bot)) def _fetch_url(self, url): diff --git a/Tools/Scripts/webkitpy/common/net/statusserver_mock.py b/Tools/Scripts/webkitpy/common/net/statusserver_mock.py index 69d1ae807..22fa12f13 100644 --- a/Tools/Scripts/webkitpy/common/net/statusserver_mock.py +++ b/Tools/Scripts/webkitpy/common/net/statusserver_mock.py @@ -26,7 +26,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 webkitpy.common.system.deprecated_logging import log +import logging + +_log = logging.getLogger(__name__) class MockStatusServer(object): @@ -48,17 +50,17 @@ class MockStatusServer(object): return self._work_items.pop(0) def release_work_item(self, queue_name, patch): - log("MOCK: release_work_item: %s %s" % (queue_name, patch.id())) + _log.info("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)) + _log.info("MOCK: update_work_items: %s %s" % (queue_name, work_items)) def submit_to_ews(self, patch_id): - log("MOCK: submit_to_ews: %s" % (patch_id)) + _log.info("MOCK: submit_to_ews: %s" % (patch_id)) def update_status(self, queue_name, status, patch=None, results_file=None): - log("MOCK: update_status: %s %s" % (queue_name, status)) + _log.info("MOCK: update_status: %s %s" % (queue_name, status)) return 187 def update_svn_revision(self, svn_revision, broken_bot): diff --git a/Tools/Scripts/webkitpy/common/net/unittestresults.py b/Tools/Scripts/webkitpy/common/net/unittestresults.py index bb82b0503..b616c0946 100644 --- a/Tools/Scripts/webkitpy/common/net/unittestresults.py +++ b/Tools/Scripts/webkitpy/common/net/unittestresults.py @@ -26,9 +26,10 @@ # (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 logging import xml.dom.minidom -from webkitpy.common.system.deprecated_logging import log +_log = logging.getLogger(__name__) class UnitTestResults(object): @@ -46,5 +47,5 @@ class UnitTestResults(object): failures.append("%s.%s" % (classname, testname)) return failures except xml.parsers.expat.ExpatError, e: - log("XML error %s parsing unit test output" % str(e)) + _log.error("XML error %s parsing unit test output" % str(e)) return None diff --git a/Tools/Scripts/webkitpy/common/system/executive_mock.py b/Tools/Scripts/webkitpy/common/system/executive_mock.py index 47eddea8b..a83f5b245 100644 --- a/Tools/Scripts/webkitpy/common/system/executive_mock.py +++ b/Tools/Scripts/webkitpy/common/system/executive_mock.py @@ -26,12 +26,14 @@ # (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 logging import os import StringIO -from webkitpy.common.system.deprecated_logging import log from webkitpy.common.system.executive import ScriptError +_log = logging.getLogger(__name__) + class MockProcess(object): def __init__(self, stdout='MOCK STDOUT\n', stderr=''): @@ -71,7 +73,7 @@ class MockExecutive(object): if process_name_filter(process_name): running_pids.append(process_pid) - log("MOCK running_pids: %s" % running_pids) + _log.info("MOCK running_pids: %s" % running_pids) return running_pids def run_and_throw_if_fail(self, args, quiet=False, cwd=None, env=None): @@ -79,7 +81,7 @@ class MockExecutive(object): env_string = "" if env: env_string = ", env=%s" % env - log("MOCK run_and_throw_if_fail: %s, cwd=%s%s" % (args, cwd, env_string)) + _log.info("MOCK run_and_throw_if_fail: %s, cwd=%s%s" % (args, cwd, env_string)) if self._should_throw_when_run.intersection(args): raise ScriptError("Exception for %s" % args, output="MOCK command output") return "MOCK output of child process" @@ -104,7 +106,7 @@ class MockExecutive(object): input_string = "" if input: input_string = ", input=%s" % input - log("MOCK run_command: %s, cwd=%s%s%s" % (args, cwd, env_string, input_string)) + _log.info("MOCK run_command: %s, cwd=%s%s%s" % (args, cwd, env_string, input_string)) output = "MOCK output of child process" if self._should_throw: raise ScriptError("MOCK ScriptError", output=output) @@ -128,7 +130,7 @@ class MockExecutive(object): env_string = "" if env: env_string = ", env=%s" % env - log("MOCK popen: %s%s%s" % (args, cwd_string, env_string)) + _log.info("MOCK popen: %s%s%s" % (args, cwd_string, env_string)) if not self._proc: self._proc = MockProcess() return self._proc diff --git a/Tools/Scripts/webkitpy/common/system/profiler.py b/Tools/Scripts/webkitpy/common/system/profiler.py new file mode 100644 index 000000000..264a4e238 --- /dev/null +++ b/Tools/Scripts/webkitpy/common/system/profiler.py @@ -0,0 +1,99 @@ +# Copyright (C) 2012 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 Google name 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 logging +import re + +_log = logging.getLogger(__name__) + + +class ProfilerFactory(object): + @classmethod + def create_profiler(cls, host, executable_path, output_dir, identifier=None): + if host.platform.is_mac(): + return Instruments(host, executable_path, output_dir, identifier) + return GooglePProf(host, executable_path, output_dir, identifier) + + +class Profiler(object): + def __init__(self, host, executable_path, output_dir, identifier=None): + self._host = host + self._executable_path = executable_path + self._output_dir = output_dir + self._identifier = "test" + self._host.filesystem.maybe_make_directory(self._output_dir) + + def adjusted_environment(self, env): + return env + + def attach_to_pid(self, pid): + pass + + def profile_after_exit(self): + pass + + +class SingleFileOutputProfiler(Profiler): + def __init__(self, host, executable_path, output_dir, output_suffix, identifier=None): + super(SingleFileOutputProfiler, self).__init__(host, executable_path, output_dir, identifier) + self._output_path = self._host.workspace.find_unused_filename(self._output_dir, self._identifier, output_suffix) + + +class GooglePProf(SingleFileOutputProfiler): + def __init__(self, host, executable_path, output_dir, identifier=None): + super(GooglePProf, self).__init__(host, executable_path, output_dir, "pprof", identifier) + + def adjusted_environment(self, env): + env['CPUPROFILE'] = self._output_path + return env + + def _first_ten_lines_of_profile(self, pprof_output): + match = re.search("^Total:[^\n]*\n((?:[^\n]*\n){0,10})", pprof_output, re.MULTILINE) + return match.group(1) if match else None + + def profile_after_exit(self): + # FIXME: We should have code to find the right google-pprof executable, some Googlers have + # google-pprof installed as "pprof" on their machines for them. + # FIXME: Similarly we should find the right perl! + pprof_args = ['/usr/bin/perl', '/usr/bin/google-pprof', '--text', self._executable_path, self._output_path] + profile_text = self._host.executive.run_command(pprof_args) + print self._first_ten_lines_of_profile(profile_text) + + +# FIXME: iprofile is a newer commandline interface to replace /usr/bin/instruments. +class Instruments(SingleFileOutputProfiler): + def __init__(self, host, executable_path, output_dir, identifier=None): + super(Instruments, self).__init__(host, executable_path, output_dir, "trace", identifier) + + # FIXME: We may need a way to find this tracetemplate on the disk + _time_profile = "/Applications/Xcode.app/Contents/Applications/Instruments.app/Contents/Resources/templates/Time Profiler.tracetemplate" + + def attach_to_pid(self, pid): + cmd = ["instruments", "-t", self._time_profile, "-D", self._output_path, "-p", pid] + cmd = map(unicode, cmd) + self._host.executive.popen(cmd) diff --git a/Tools/Scripts/webkitpy/common/system/profiler_unittest.py b/Tools/Scripts/webkitpy/common/system/profiler_unittest.py new file mode 100644 index 000000000..059b7cfa1 --- /dev/null +++ b/Tools/Scripts/webkitpy/common/system/profiler_unittest.py @@ -0,0 +1,87 @@ +# Copyright (C) 2012 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.systemhost_mock import MockSystemHost + +from .profiler import ProfilerFactory, Instruments, GooglePProf + + +class ProfilerFactoryTest(unittest.TestCase): + def test_basic(self): + host = MockSystemHost() + self.assertFalse(host.filesystem.exists("/tmp/output")) + profiler = ProfilerFactory.create_profiler(host, '/bin/executable', '/tmp/output') + self.assertTrue(host.filesystem.exists("/tmp/output")) + self.assertEquals(profiler._output_path, "/tmp/output/test.trace") + + host.platform.os_name = 'linux' + profiler = ProfilerFactory.create_profiler(host, '/bin/executable', '/tmp/output') + self.assertEquals(profiler._output_path, "/tmp/output/test.pprof") + + def test_pprof_output_regexp(self): + pprof_output = """ +sometimes +there +is +junk before the total line + + +Total: 3770 samples + 76 2.0% 2.0% 104 2.8% lookup (inline) + 60 1.6% 3.6% 60 1.6% FL_SetPrevious (inline) + 56 1.5% 5.1% 56 1.5% MaskPtr (inline) + 51 1.4% 6.4% 222 5.9% WebCore::HTMLTokenizer::nextToken + 42 1.1% 7.6% 47 1.2% WTF::Vector::shrinkCapacity + 35 0.9% 8.5% 35 0.9% WTF::RefPtr::get (inline) + 33 0.9% 9.4% 43 1.1% append (inline) + 29 0.8% 10.1% 67 1.8% WTF::StringImpl::deref (inline) + 29 0.8% 10.9% 100 2.7% add (inline) + 28 0.7% 11.6% 28 0.7% WebCore::QualifiedName::localName (inline) + 25 0.7% 12.3% 27 0.7% WebCore::Private::addChildNodesToDeletionQueue + 24 0.6% 12.9% 24 0.6% __memcpy_ssse3_back + 23 0.6% 13.6% 23 0.6% intHash (inline) + 23 0.6% 14.2% 76 2.0% tcmalloc::FL_Next + 23 0.6% 14.8% 95 2.5% tcmalloc::FL_Push + 22 0.6% 15.4% 22 0.6% WebCore::MarkupTokenizerBase::InputStreamPreprocessor::peek (inline) +""" + expected_first_ten_lines = """ 76 2.0% 2.0% 104 2.8% lookup (inline) + 60 1.6% 3.6% 60 1.6% FL_SetPrevious (inline) + 56 1.5% 5.1% 56 1.5% MaskPtr (inline) + 51 1.4% 6.4% 222 5.9% WebCore::HTMLTokenizer::nextToken + 42 1.1% 7.6% 47 1.2% WTF::Vector::shrinkCapacity + 35 0.9% 8.5% 35 0.9% WTF::RefPtr::get (inline) + 33 0.9% 9.4% 43 1.1% append (inline) + 29 0.8% 10.1% 67 1.8% WTF::StringImpl::deref (inline) + 29 0.8% 10.9% 100 2.7% add (inline) + 28 0.7% 11.6% 28 0.7% WebCore::QualifiedName::localName (inline) +""" + host = MockSystemHost() + profiler = GooglePProf(host, '/bin/executable', '/tmp/output') + self.assertEquals(profiler._first_ten_lines_of_profile(pprof_output), expected_first_ten_lines) diff --git a/Tools/Scripts/webkitpy/common/system/user_mock.py b/Tools/Scripts/webkitpy/common/system/user_mock.py index 16f79a0c4..d17ea9a90 100644 --- a/Tools/Scripts/webkitpy/common/system/user_mock.py +++ b/Tools/Scripts/webkitpy/common/system/user_mock.py @@ -26,7 +26,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 webkitpy.common.system.deprecated_logging import log +import logging + +_log = logging.getLogger(__name__) class MockUser(object): @@ -52,7 +54,7 @@ class MockUser(object): pass def confirm(self, message=None, default='y'): - log(message) + _log.info(message) return default == 'y' def can_open_url(self): @@ -61,6 +63,6 @@ class MockUser(object): def open_url(self, url): self.opened_urls.append(url) if url.startswith("file://"): - log("MOCK: user.open_url: file://...") + _log.info("MOCK: user.open_url: file://...") return - log("MOCK: user.open_url: %s" % url) + _log.info("MOCK: user.open_url: %s" % url) diff --git a/Tools/Scripts/webkitpy/common/system/workspace_unittest.py b/Tools/Scripts/webkitpy/common/system/workspace_unittest.py index 49094ac63..eca386ac3 100644 --- a/Tools/Scripts/webkitpy/common/system/workspace_unittest.py +++ b/Tools/Scripts/webkitpy/common/system/workspace_unittest.py @@ -50,18 +50,23 @@ class WorkspaceTest(unittest.TestCase): def test_create_zip(self): workspace = Workspace(None, MockExecutive(should_log=True)) - expected_stderr = "MOCK run_command: ['zip', '-9', '-r', '/zip/path', '.'], cwd=/source/path\n" + expected_logs = "MOCK run_command: ['zip', '-9', '-r', '/zip/path', '.'], cwd=/source/path\n" class MockZipFile(object): def __init__(self, path): self.filename = path - archive = OutputCapture().assert_outputs(self, workspace.create_zip, ["/zip/path", "/source/path", MockZipFile], expected_stderr=expected_stderr) + archive = OutputCapture().assert_outputs(self, workspace.create_zip, ["/zip/path", "/source/path", MockZipFile], expected_logs=expected_logs) self.assertEqual(archive.filename, "/zip/path") def test_create_zip_exception(self): workspace = Workspace(None, MockExecutive(should_log=True, should_throw=True)) - expected_stderr = "MOCK run_command: ['zip', '-9', '-r', '/zip/path', '.'], cwd=/source/path\n" + expected_logs = """MOCK run_command: ['zip', '-9', '-r', '/zip/path', '.'], cwd=/source/path +Workspace.create_zip failed: +MOCK ScriptError + +MOCK output of child process +""" class MockZipFile(object): def __init__(self, path): self.filename = path - archive = OutputCapture().assert_outputs(self, workspace.create_zip, ["/zip/path", "/source/path", MockZipFile], expected_stderr=expected_stderr) + archive = OutputCapture().assert_outputs(self, workspace.create_zip, ["/zip/path", "/source/path", MockZipFile], expected_logs=expected_logs) self.assertEqual(archive, None) diff --git a/Tools/Scripts/webkitpy/common/watchlist/watchlist_mock.py b/Tools/Scripts/webkitpy/common/watchlist/watchlist_mock.py index 2fd2f880f..cbbf0718a 100644 --- a/Tools/Scripts/webkitpy/common/watchlist/watchlist_mock.py +++ b/Tools/Scripts/webkitpy/common/watchlist/watchlist_mock.py @@ -26,10 +26,12 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -from webkitpy.common.system.deprecated_logging import log +import logging + +_log = logging.getLogger(__name__) class MockWatchList(object): def determine_cc_and_messages(self, diff): - log("MockWatchList: determine_cc_and_messages") + _log.info("MockWatchList: determine_cc_and_messages") return {'cc_list': ['abarth@webkit.org', 'eric@webkit.org', 'levin@chromium.org'], 'messages': ['Message1.', 'Message2.'], } diff --git a/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py b/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py index a077dc92e..c18cd0941 100644 --- a/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py @@ -68,7 +68,9 @@ SCALING_GOVERNORS_PATTERN = "/sys/devices/system/cpu/cpu*/cpufreq/scaling_govern # All the test cases are still served to DumpRenderTree through file protocol, # but we use a file-to-http feature to bridge the file request to host's http # server to get the real test files and corresponding resources. -TEST_PATH_PREFIX = '/all-tests' +# See webkit/support/platform_support_android.cc for the other side of this bridge. +PERF_TEST_PATH_PREFIX = '/all-perf-tests' +LAYOUT_TEST_PATH_PREFIX = '/all-tests' # All ports the Android forwarder to forward. # 8000, 8080 and 8443 are for http/https tests. @@ -129,7 +131,8 @@ DEVICE_FONTS_DIR = DEVICE_DRT_DIR + 'fonts/' # 1. as a virtual path in file urls that will be bridged to HTTP. # 2. pointing to some files that are pushed to the device for tests that # don't work on file-over-http (e.g. blob protocol tests). -DEVICE_LAYOUT_TESTS_DIR = DEVICE_SOURCE_ROOT_DIR + 'third_party/WebKit/LayoutTests/' +DEVICE_WEBKIT_BASE_DIR = DEVICE_SOURCE_ROOT_DIR + 'third_party/WebKit/' +DEVICE_LAYOUT_TESTS_DIR = DEVICE_WEBKIT_BASE_DIR + 'LayoutTests/' # Test resources that need to be accessed as files directly. # Each item can be the relative path of a directory or a file. @@ -242,7 +245,8 @@ class ChromiumAndroidPort(chromium.ChromiumPort): def start_http_server(self, additional_dirs=None, number_of_servers=0): if not additional_dirs: additional_dirs = {} - additional_dirs[TEST_PATH_PREFIX] = self.layout_tests_dir() + additional_dirs[PERF_TEST_PATH_PREFIX] = self.perf_tests_dir() + additional_dirs[LAYOUT_TEST_PATH_PREFIX] = self.layout_tests_dir() super(ChromiumAndroidPort, self).start_http_server(additional_dirs, number_of_servers) def create_driver(self, worker_number, no_timeout=False): @@ -665,10 +669,10 @@ class ChromiumAndroidDriver(driver.Driver): def _command_from_driver_input(self, driver_input): command = super(ChromiumAndroidDriver, self)._command_from_driver_input(driver_input) if command.startswith('/'): - # Convert the host file path to a device file path. See comment of - # DEVICE_LAYOUT_TESTS_DIR for details. + fs = self._port._filesystem # FIXME: what happens if command lies outside of the layout_tests_dir on the host? - command = DEVICE_LAYOUT_TESTS_DIR + self._port.relative_test_filename(command) + relative_test_filename = fs.relpath(command, fs.dirname(self._port.layout_tests_dir())) + command = DEVICE_WEBKIT_BASE_DIR + relative_test_filename return command def _read_prompt(self, deadline): diff --git a/Tools/Scripts/webkitpy/layout_tests/port/chromium_win_unittest.py b/Tools/Scripts/webkitpy/layout_tests/port/chromium_win_unittest.py index 1c0ddc1fe..a84b5ee15 100644 --- a/Tools/Scripts/webkitpy/layout_tests/port/chromium_win_unittest.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/chromium_win_unittest.py @@ -64,9 +64,9 @@ class ChromiumWinTest(chromium_port_testcase.ChromiumPortTestCase): def test_setup_environ_for_server_register_cygwin(self): port = self.make_port(options=MockOptions(register_cygwin=True, results_directory='/')) port._executive = MockExecutive(should_log=True) - expected_stderr = "MOCK run_command: ['/mock-checkout/Source/WebKit/chromium/third_party/cygwin/setup_mount.bat'], cwd=None\n" + expected_logs = "MOCK run_command: ['/mock-checkout/Source/WebKit/chromium/third_party/cygwin/setup_mount.bat'], cwd=None\n" output = outputcapture.OutputCapture() - output.assert_outputs(self, port.setup_environ_for_server, expected_stderr=expected_stderr) + output.assert_outputs(self, port.setup_environ_for_server, expected_logs=expected_logs) def assert_name(self, port_name, os_version_string, expected): port = self.make_port(port_name=port_name, os_version=os_version_string) diff --git a/Tools/Scripts/webkitpy/layout_tests/port/driver.py b/Tools/Scripts/webkitpy/layout_tests/port/driver.py index 7993d0577..e883590cf 100644 --- a/Tools/Scripts/webkitpy/layout_tests/port/driver.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/driver.py @@ -36,6 +36,7 @@ import time import os from webkitpy.common.system import path +from webkitpy.common.system.profiler import ProfilerFactory _log = logging.getLogger(__name__) @@ -140,6 +141,10 @@ class Driver(object): self._server_process = None self._measurements = {} + if self._port.get_option("profile"): + self._profiler = ProfilerFactory.create_profiler(self._port.host, self._port._path_to_driver(), self._port.results_directory()) + else: + self._profiler = None def __del__(self): self.stop() @@ -282,15 +287,21 @@ class Driver(object): environment['LOCAL_RESOURCE_ROOT'] = self._port.layout_tests_dir() if 'WEBKITOUTPUTDIR' in os.environ: environment['WEBKITOUTPUTDIR'] = os.environ['WEBKITOUTPUTDIR'] + if self._profiler: + environment = self._profiler.adjusted_environment(environment) self._crashed_process_name = None self._crashed_pid = None self._server_process = self._port._server_process_constructor(self._port, server_name, self.cmd_line(pixel_tests, per_test_args), environment) self._server_process.start() + if self._profiler: + self._profiler.attach_to_pid(self._server_process.pid()) def stop(self): if self._server_process: self._server_process.stop(self._port.driver_stop_timeout()) self._server_process = None + if self._profiler: + self._profiler.profile_after_exit() if self._driver_tempdir: self._port._filesystem.rmtree(str(self._driver_tempdir)) diff --git a/Tools/Scripts/webkitpy/layout_tests/port/efl_unittest.py b/Tools/Scripts/webkitpy/layout_tests/port/efl_unittest.py index d9851b32e..1ac687b18 100644 --- a/Tools/Scripts/webkitpy/layout_tests/port/efl_unittest.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/efl_unittest.py @@ -39,5 +39,5 @@ class EflPortTest(port_testcase.PortTestCase): def test_show_results_html_file(self): port = self.make_port() port._executive = MockExecutive(should_log=True) - expected_stderr = "MOCK run_command: ['Tools/Scripts/run-launcher', '--release', '--efl', 'file://test.html'], cwd=/mock-checkout\n" - OutputCapture().assert_outputs(self, port.show_results_html_file, ["test.html"], expected_stderr=expected_stderr) + expected_logs = "MOCK run_command: ['Tools/Scripts/run-launcher', '--release', '--efl', 'file://test.html'], cwd=/mock-checkout\n" + OutputCapture().assert_outputs(self, port.show_results_html_file, ["test.html"], expected_logs=expected_logs) diff --git a/Tools/Scripts/webkitpy/layout_tests/port/gtk_unittest.py b/Tools/Scripts/webkitpy/layout_tests/port/gtk_unittest.py index 6f5fae68d..7002495a4 100644 --- a/Tools/Scripts/webkitpy/layout_tests/port/gtk_unittest.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/gtk_unittest.py @@ -46,8 +46,8 @@ class GtkPortTest(port_testcase.PortTestCase): def test_show_results_html_file(self): port = self.make_port() port._executive = MockExecutive(should_log=True) - expected_stderr = "MOCK run_command: ['Tools/Scripts/run-launcher', '--release', '--gtk', 'file://test.html'], cwd=/mock-checkout\n" - OutputCapture().assert_outputs(self, port.show_results_html_file, ["test.html"], expected_stderr=expected_stderr) + expected_logs = "MOCK run_command: ['Tools/Scripts/run-launcher', '--release', '--gtk', 'file://test.html'], cwd=/mock-checkout\n" + OutputCapture().assert_outputs(self, port.show_results_html_file, ["test.html"], expected_logs=expected_logs) def test_default_timeout_ms(self): self.assertEqual(self.make_port(options=MockOptions(configuration='Release')).default_timeout_ms(), 6000) diff --git a/Tools/Scripts/webkitpy/layout_tests/port/mac_unittest.py b/Tools/Scripts/webkitpy/layout_tests/port/mac_unittest.py index 511201464..831fbf7c9 100644 --- a/Tools/Scripts/webkitpy/layout_tests/port/mac_unittest.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/mac_unittest.py @@ -139,8 +139,8 @@ java/ port = self.make_port() # Delay setting a should_log executive to avoid logging from MacPort.__init__. port._executive = MockExecutive(should_log=True) - expected_stderr = "MOCK popen: ['Tools/Scripts/run-safari', '--release', '--no-saved-state', '-NSOpen', 'test.html'], cwd=/mock-checkout\n" - OutputCapture().assert_outputs(self, port.show_results_html_file, ["test.html"], expected_stderr=expected_stderr) + expected_logs = "MOCK popen: ['Tools/Scripts/run-safari', '--release', '--no-saved-state', '-NSOpen', 'test.html'], cwd=/mock-checkout\n" + OutputCapture().assert_outputs(self, port.show_results_html_file, ["test.html"], expected_logs=expected_logs) def test_operating_system(self): self.assertEqual('mac', self.make_port().operating_system()) diff --git a/Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py b/Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py index e7cd76c92..8ea108ba0 100755 --- a/Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py @@ -535,28 +535,33 @@ class PortTestCase(unittest.TestCase): # Delay setting _executive to avoid logging during construction port._executive = MockExecutive(should_log=True) port._options = MockOptions(configuration="Release") # This should not be necessary, but I think TestWebKitPort is actually reading from disk (and thus detects the current configuration). - expected_stderr = "MOCK run_command: ['Tools/Scripts/build-dumprendertree', '--release'], cwd=/mock-checkout, env={'LC_ALL': 'C', 'MOCK_ENVIRON_COPY': '1'}\n" - self.assertTrue(output.assert_outputs(self, port._build_driver, expected_stderr=expected_stderr, expected_logs='')) + expected_logs = "MOCK run_command: ['Tools/Scripts/build-dumprendertree', '--release'], cwd=/mock-checkout, env={'LC_ALL': 'C', 'MOCK_ENVIRON_COPY': '1'}\n" + self.assertTrue(output.assert_outputs(self, port._build_driver, expected_logs=expected_logs)) # Make sure when passed --webkit-test-runner we build the right tool. port._options = MockOptions(webkit_test_runner=True, configuration="Release") - expected_stderr = "MOCK run_command: ['Tools/Scripts/build-dumprendertree', '--release'], cwd=/mock-checkout, env={'LC_ALL': 'C', 'MOCK_ENVIRON_COPY': '1'}\nMOCK run_command: ['Tools/Scripts/build-webkittestrunner', '--release'], cwd=/mock-checkout, env={'LC_ALL': 'C', 'MOCK_ENVIRON_COPY': '1'}\n" - self.assertTrue(output.assert_outputs(self, port._build_driver, expected_stderr=expected_stderr, expected_logs='')) + expected_logs = "MOCK run_command: ['Tools/Scripts/build-dumprendertree', '--release'], cwd=/mock-checkout, env={'LC_ALL': 'C', 'MOCK_ENVIRON_COPY': '1'}\nMOCK run_command: ['Tools/Scripts/build-webkittestrunner', '--release'], cwd=/mock-checkout, env={'LC_ALL': 'C', 'MOCK_ENVIRON_COPY': '1'}\n" + self.assertTrue(output.assert_outputs(self, port._build_driver, expected_logs=expected_logs)) # Make sure we show the build log when --verbose is passed, which we simulate by setting the logging level to DEBUG. output.set_log_level(logging.DEBUG) port._options = MockOptions(configuration="Release") - expected_stderr = "MOCK run_command: ['Tools/Scripts/build-dumprendertree', '--release'], cwd=/mock-checkout, env={'LC_ALL': 'C', 'MOCK_ENVIRON_COPY': '1'}\n" - expected_logs = "Output of ['Tools/Scripts/build-dumprendertree', '--release']:\nMOCK output of child process\n" - self.assertTrue(output.assert_outputs(self, port._build_driver, expected_stderr=expected_stderr, expected_logs=expected_logs)) + expected_logs = """MOCK run_command: ['Tools/Scripts/build-dumprendertree', '--release'], cwd=/mock-checkout, env={'LC_ALL': 'C', 'MOCK_ENVIRON_COPY': '1'} +Output of ['Tools/Scripts/build-dumprendertree', '--release']: +MOCK output of child process +""" + self.assertTrue(output.assert_outputs(self, port._build_driver, expected_logs=expected_logs)) output.set_log_level(logging.INFO) # Make sure that failure to build returns False. port._executive = MockExecutive(should_log=True, should_throw=True) # Because WK2 currently has to build both webkittestrunner and DRT, if DRT fails, that's the only one it tries. - expected_stderr = "MOCK run_command: ['Tools/Scripts/build-dumprendertree', '--release'], cwd=/mock-checkout, env={'LC_ALL': 'C', 'MOCK_ENVIRON_COPY': '1'}\n" - expected_logs = "MOCK ScriptError\n\nMOCK output of child process\n" - self.assertFalse(output.assert_outputs(self, port._build_driver, expected_stderr=expected_stderr, expected_logs=expected_logs)) + expected_logs = """MOCK run_command: ['Tools/Scripts/build-dumprendertree', '--release'], cwd=/mock-checkout, env={'LC_ALL': 'C', 'MOCK_ENVIRON_COPY': '1'} +MOCK ScriptError + +MOCK output of child process +""" + self.assertFalse(output.assert_outputs(self, port._build_driver, expected_logs=expected_logs)) def _assert_config_file_for_platform(self, port, platform, config_file): self.assertEqual(port._apache_config_file_name_for_platform(platform), config_file) diff --git a/Tools/Scripts/webkitpy/layout_tests/port/qt_unittest.py b/Tools/Scripts/webkitpy/layout_tests/port/qt_unittest.py index 4a558f8dd..d6ef8d85e 100644 --- a/Tools/Scripts/webkitpy/layout_tests/port/qt_unittest.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/qt_unittest.py @@ -104,8 +104,8 @@ class QtPortTest(port_testcase.PortTestCase): def test_show_results_html_file(self): port = self.make_port() port._executive = MockExecutive(should_log=True) - expected_stderr = "MOCK run_command: ['Tools/Scripts/run-launcher', '--release', '--qt', 'file://test.html'], cwd=/mock-checkout\n" - OutputCapture().assert_outputs(self, port.show_results_html_file, ["test.html"], expected_stderr=expected_stderr) + expected_logs = "MOCK run_command: ['Tools/Scripts/run-launcher', '--release', '--qt', 'file://test.html'], cwd=/mock-checkout\n" + OutputCapture().assert_outputs(self, port.show_results_html_file, ["test.html"], expected_logs=expected_logs) def test_setup_environ_for_server(self): port = self.make_port() diff --git a/Tools/Scripts/webkitpy/layout_tests/port/win_unittest.py b/Tools/Scripts/webkitpy/layout_tests/port/win_unittest.py index 668685f56..9def7246a 100644 --- a/Tools/Scripts/webkitpy/layout_tests/port/win_unittest.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/win_unittest.py @@ -51,11 +51,11 @@ class WinPortTest(port_testcase.PortTestCase): capture = OutputCapture() capture.capture_output() port.show_results_html_file('test.html') - _, stderr, _ = capture.restore_output() + _, _, logs = capture.restore_output() # We can't know for sure what path will be produced by cygpath, but we can assert about # everything else. - self.assertTrue(stderr.startswith("MOCK run_command: ['Tools/Scripts/run-safari', '--release', '")) - self.assertTrue(stderr.endswith("test.html'], cwd=/mock-checkout\n")) + self.assertTrue(logs.startswith("MOCK run_command: ['Tools/Scripts/run-safari', '--release', '")) + self.assertTrue(logs.endswith("test.html'], cwd=/mock-checkout\n")) def _assert_search_path(self, expected_search_paths, version, use_webkit2=False): port = self.make_port(port_name='win', os_version=version, options=MockOptions(webkit_test_runner=use_webkit2)) diff --git a/Tools/Scripts/webkitpy/layout_tests/port/xvfbdriver_unittest.py b/Tools/Scripts/webkitpy/layout_tests/port/xvfbdriver_unittest.py index 367131039..241b37c1f 100644 --- a/Tools/Scripts/webkitpy/layout_tests/port/xvfbdriver_unittest.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/xvfbdriver_unittest.py @@ -26,9 +26,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. +import logging import unittest -from webkitpy.common.system.deprecated_logging import log from webkitpy.common.system.filesystem_mock import MockFileSystem from webkitpy.common.system.executive_mock import MockExecutive2 from webkitpy.common.system.outputcapture import OutputCapture @@ -38,6 +38,8 @@ from webkitpy.layout_tests.port.server_process_mock import MockServerProcess from webkitpy.layout_tests.port.xvfbdriver import XvfbDriver from webkitpy.tool.mocktool import MockOptions +_log = logging.getLogger(__name__) + class XvfbDriverTest(unittest.TestCase): def make_driver(self, worker_number=0, xorg_running=False, executive=None): @@ -57,27 +59,27 @@ class XvfbDriverTest(unittest.TestCase): # intend to test the behavior of XvfbDriver.stop. driver._xvfb_process = None - def assertDriverStartSuccessful(self, driver, expected_stderr, expected_display, pixel_tests=False): - OutputCapture().assert_outputs(self, driver.start, [pixel_tests, []], expected_stderr=expected_stderr) + def assertDriverStartSuccessful(self, driver, expected_logs, expected_display, pixel_tests=False): + OutputCapture().assert_outputs(self, driver.start, [pixel_tests, []], expected_logs=expected_logs) self.assertTrue(driver._server_process.started) self.assertEqual(driver._server_process.env["DISPLAY"], expected_display) def test_start_no_pixel_tests(self): driver = self.make_driver() - expected_stderr = "MOCK run_command: ['ps', '-eo', 'comm,command'], cwd=None\nMOCK popen: ['Xvfb', ':0', '-screen', '0', '800x600x24', '-nolisten', 'tcp']\n" - self.assertDriverStartSuccessful(driver, expected_stderr=expected_stderr, expected_display=":0") + expected_logs = "MOCK run_command: ['ps', '-eo', 'comm,command'], cwd=None\nMOCK popen: ['Xvfb', ':0', '-screen', '0', '800x600x24', '-nolisten', 'tcp']\n" + self.assertDriverStartSuccessful(driver, expected_logs=expected_logs, expected_display=":0") self.cleanup_driver(driver) def test_start_pixel_tests(self): driver = self.make_driver() - expected_stderr = "MOCK run_command: ['ps', '-eo', 'comm,command'], cwd=None\nMOCK popen: ['Xvfb', ':0', '-screen', '0', '800x600x24', '-nolisten', 'tcp']\n" - self.assertDriverStartSuccessful(driver, expected_stderr=expected_stderr, expected_display=":0", pixel_tests=True) + expected_logs = "MOCK run_command: ['ps', '-eo', 'comm,command'], cwd=None\nMOCK popen: ['Xvfb', ':0', '-screen', '0', '800x600x24', '-nolisten', 'tcp']\n" + self.assertDriverStartSuccessful(driver, expected_logs=expected_logs, expected_display=":0", pixel_tests=True) self.cleanup_driver(driver) def test_start_arbitrary_worker_number(self): driver = self.make_driver(worker_number=17) - expected_stderr = "MOCK run_command: ['ps', '-eo', 'comm,command'], cwd=None\nMOCK popen: ['Xvfb', ':0', '-screen', '0', '800x600x24', '-nolisten', 'tcp']\n" - self.assertDriverStartSuccessful(driver, expected_stderr=expected_stderr, expected_display=":0", pixel_tests=True) + expected_logs = "MOCK run_command: ['ps', '-eo', 'comm,command'], cwd=None\nMOCK popen: ['Xvfb', ':0', '-screen', '0', '800x600x24', '-nolisten', 'tcp']\n" + self.assertDriverStartSuccessful(driver, expected_logs=expected_logs, expected_display=":0", pixel_tests=True) self.cleanup_driver(driver) def disabled_test_next_free_display(self): @@ -105,19 +107,19 @@ class XvfbDriverTest(unittest.TestCase): def test_start_next_worker(self): driver = self.make_driver() driver._next_free_display = lambda: 0 - expected_stderr = "MOCK popen: ['Xvfb', ':0', '-screen', '0', '800x600x24', '-nolisten', 'tcp']\n" - self.assertDriverStartSuccessful(driver, expected_stderr=expected_stderr, expected_display=":0", pixel_tests=True) + expected_logs = "MOCK popen: ['Xvfb', ':0', '-screen', '0', '800x600x24', '-nolisten', 'tcp']\n" + self.assertDriverStartSuccessful(driver, expected_logs=expected_logs, expected_display=":0", pixel_tests=True) self.cleanup_driver(driver) driver = self.make_driver() driver._next_free_display = lambda: 3 - expected_stderr = "MOCK popen: ['Xvfb', ':3', '-screen', '0', '800x600x24', '-nolisten', 'tcp']\n" - self.assertDriverStartSuccessful(driver, expected_stderr=expected_stderr, expected_display=":3", pixel_tests=True) + expected_logs = "MOCK popen: ['Xvfb', ':3', '-screen', '0', '800x600x24', '-nolisten', 'tcp']\n" + self.assertDriverStartSuccessful(driver, expected_logs=expected_logs, expected_display=":3", pixel_tests=True) self.cleanup_driver(driver) def test_stop(self): filesystem = MockFileSystem(files={'/tmp/.X42-lock': '1234\n'}) port = Port(host=MockSystemHost(log_executive=True, filesystem=filesystem), options=MockOptions(configuration='Release')) - port._executive.kill_process = lambda x: log("MOCK kill_process pid: " + str(x)) + port._executive.kill_process = lambda x: _log.info("MOCK kill_process pid: " + str(x)) driver = XvfbDriver(port, worker_number=0, pixel_tests=True) class FakeXvfbProcess(object): @@ -126,8 +128,8 @@ class XvfbDriverTest(unittest.TestCase): driver._xvfb_process = FakeXvfbProcess() driver._lock_file = '/tmp/.X42-lock' - expected_stderr = "MOCK kill_process pid: 1234\n" - OutputCapture().assert_outputs(self, driver.stop, [], expected_stderr=expected_stderr) + expected_logs = "MOCK kill_process pid: 1234\n" + OutputCapture().assert_outputs(self, driver.stop, [], expected_logs=expected_logs) self.assertEqual(driver._xvfb_process, None) self.assertFalse(port._filesystem.exists(driver._lock_file)) diff --git a/Tools/Scripts/webkitpy/layout_tests/servers/apache_http_server_unittest.py b/Tools/Scripts/webkitpy/layout_tests/servers/apache_http_server_unittest.py index 34ab97b40..f3ca6a1a8 100644 --- a/Tools/Scripts/webkitpy/layout_tests/servers/apache_http_server_unittest.py +++ b/Tools/Scripts/webkitpy/layout_tests/servers/apache_http_server_unittest.py @@ -63,8 +63,8 @@ class TestLayoutTestApacheHttpd(unittest.TestCase): server.start() server.stop() finally: - out, err, logs = oc.restore_output() - self.assertTrue("StartServers 4" in err) - self.assertTrue("MinSpareServers 4" in err) - self.assertTrue("MaxSpareServers 4" in err) + _, _, logs = oc.restore_output() + self.assertTrue("StartServers 4" in logs) + self.assertTrue("MinSpareServers 4" in logs) + self.assertTrue("MaxSpareServers 4" in logs) self.assertTrue(host.filesystem.exists("/mock/output_dir/httpd.conf")) diff --git a/Tools/Scripts/webkitpy/performance_tests/perftest.py b/Tools/Scripts/webkitpy/performance_tests/perftest.py index 9e2f87d47..623aef341 100644 --- a/Tools/Scripts/webkitpy/performance_tests/perftest.py +++ b/Tools/Scripts/webkitpy/performance_tests/perftest.py @@ -70,6 +70,7 @@ class PerfTest(object): def run(self, driver, time_out_ms): output = self.run_single(driver, self.path_or_url(), time_out_ms) + self._filter_stderr(output) if self.run_failed(output): return None return self.parse_output(output) @@ -92,6 +93,27 @@ class PerfTest(object): return True + def _should_ignore_line(self, regexps, line): + if not line: + return True + for regexp in regexps: + if regexp.search(line): + return True + return False + + _lines_to_ignore_in_stderr = [ + re.compile(r'^Unknown option:'), + re.compile(r'^\[WARNING:proxy_service.cc')] + + def _should_ignore_line_in_stderr(self, line): + return self._should_ignore_line(self._lines_to_ignore_in_stderr, line) + + def _filter_stderr(self, output): + if not output.error: + return + filtered_error = '\n'.join([line for line in re.split('\n', output.error) if not self._should_ignore_line_in_stderr(line)]) + output.error = filtered_error if filtered_error else None + _lines_to_ignore_in_parser_result = [ re.compile(r'^Running \d+ times$'), re.compile(r'^Ignoring warm-up '), @@ -105,12 +127,7 @@ class PerfTest(object): re.compile(re.escape("""Blocked access to external URL http://www.whatwg.org/specs/web-apps/current-work/"""))] def _should_ignore_line_in_parser_test_result(self, line): - if not line: - return True - for regex in self._lines_to_ignore_in_parser_result: - if regex.search(line): - return True - return False + return self._should_ignore_line(self._lines_to_ignore_in_parser_result, line) _description_regex = re.compile(r'^Description: (?P<description>.*)$', re.IGNORECASE) _result_classes = ['Time', 'JS Heap', 'Malloc'] @@ -166,11 +183,12 @@ class PerfTest(object): _log.error("The test didn't report all statistics.") return None - for result_name in ordered_results_keys: - if result_name == test_name: - self.output_statistics(result_name, results[result_name], description_string) - else: - self.output_statistics(result_name, results[result_name]) + if not self._port.get_option('profile'): + for result_name in ordered_results_keys: + if result_name == test_name: + self.output_statistics(result_name, results[result_name], description_string) + else: + self.output_statistics(result_name, results[result_name]) return results def output_statistics(self, test_name, results, description_string=None): @@ -327,7 +345,7 @@ class ReplayPerfTest(PageLoadingPerfTest): _log.info("Preparing replay for %s" % self.test_name()) - driver = self._port.create_driver(worker_number=1, no_timeout=True) + driver = self._port.create_driver(worker_number=0, no_timeout=True) try: output = self.run_single(driver, self._archive_path, time_out_ms, record=True) finally: diff --git a/Tools/Scripts/webkitpy/performance_tests/perftest_unittest.py b/Tools/Scripts/webkitpy/performance_tests/perftest_unittest.py index 259fc7854..9e275b635 100755 --- a/Tools/Scripts/webkitpy/performance_tests/perftest_unittest.py +++ b/Tools/Scripts/webkitpy/performance_tests/perftest_unittest.py @@ -63,7 +63,7 @@ class MainTest(unittest.TestCase): output_capture = OutputCapture() output_capture.capture_output() try: - test = PerfTest(None, 'some-test', '/path/some-dir/some-test') + test = PerfTest(MockPort(), 'some-test', '/path/some-dir/some-test') self.assertEqual(test.parse_output(output), {'some-test': {'avg': 1100.0, 'median': 1101.0, 'min': 1080.0, 'max': 1120.0, 'stdev': 11.0, 'unit': 'ms', 'values': [i for i in range(1, 20)]}}) @@ -91,7 +91,7 @@ class MainTest(unittest.TestCase): output_capture = OutputCapture() output_capture.capture_output() try: - test = PerfTest(None, 'some-test', '/path/some-dir/some-test') + test = PerfTest(MockPort(), 'some-test', '/path/some-dir/some-test') self.assertEqual(test.parse_output(output), None) finally: actual_stdout, actual_stderr, actual_logs = output_capture.restore_output() diff --git a/Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py b/Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py index 42e0d96e1..6dc4742b7 100755 --- a/Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py +++ b/Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py @@ -68,6 +68,8 @@ class PerfTestsRunner(object): self._base_path = self._port.perf_tests_dir() self._results = {} self._timestamp = time.time() + self._needs_http = None + self._has_http_lock = False @staticmethod def _parse_args(args=None): @@ -83,6 +85,8 @@ class PerfTestsRunner(object): help="Specify port/platform being tested (i.e. chromium-mac)"), optparse.make_option("--chromium", action="store_const", const='chromium', dest='platform', help='Alias for --platform=chromium'), + optparse.make_option("--chromium-android", + action="store_const", const='chromium-android', dest='platform', help='Alias for --platform=chromium-android'), optparse.make_option("--builder-name", help=("The name of the builder shown on the waterfall running this script e.g. google-mac-2.")), optparse.make_option("--build-number", @@ -117,6 +121,8 @@ class PerfTestsRunner(object): help="Run replay tests."), optparse.make_option("--force", dest="skipped", action="store_true", default=False, help="Run all tests, including the ones in the Skipped list."), + optparse.make_option("--profile", action="store_true", + help="Output per-test profile information."), ] return optparse.OptionParser(option_list=(perf_option_list)).parse_args(args) @@ -151,8 +157,21 @@ class PerfTestsRunner(object): return tests + def _start_servers(self): + if self._needs_http: + self._port.acquire_http_lock() + self._port.start_http_server(number_of_servers=2) + self._has_http_lock = True + + def _stop_servers(self): + if self._has_http_lock: + self._port.stop_http_server() + self._port.release_http_lock() + def run(self): - if not self._port.check_build(needs_http=False): + self._needs_http = self._port.requires_http_server() + + if not self._port.check_build(needs_http=self._needs_http): _log.error("Build not up to date for %s" % self._port._path_to_driver()) return self.EXIT_CODE_BAD_BUILD @@ -163,8 +182,14 @@ class PerfTestsRunner(object): if not test.prepare(self._options.time_out_ms): return self.EXIT_CODE_BAD_PREPARATION - unexpected = self._run_tests_set(sorted(list(tests), key=lambda test: test.test_name()), self._port) - if self._options.generate_results: + try: + self._start_servers() + unexpected = self._run_tests_set(sorted(list(tests), key=lambda test: test.test_name()), self._port) + + finally: + self._stop_servers() + + if self._options.generate_results and not self._options.profile: exit_code = self._generate_and_show_results() if exit_code: return exit_code @@ -290,7 +315,7 @@ class PerfTestsRunner(object): driver = None for test in tests: - driver = port.create_driver(worker_number=1, no_timeout=True) + driver = port.create_driver(worker_number=0, no_timeout=True) if self._options.pause_before_testing: driver.start() diff --git a/Tools/Scripts/webkitpy/performance_tests/perftestsrunner_unittest.py b/Tools/Scripts/webkitpy/performance_tests/perftestsrunner_unittest.py index 62e7353df..16a05599c 100755 --- a/Tools/Scripts/webkitpy/performance_tests/perftestsrunner_unittest.py +++ b/Tools/Scripts/webkitpy/performance_tests/perftestsrunner_unittest.py @@ -242,9 +242,8 @@ max 548000 bytes self.assertEqual(TestDriverWithStartCount.start_count, 1) finally: stdout, stderr, log = output.restore_output() - self.assertEqual(stderr, "Ready to run test?\n") self.assertEqual(self.normalizeFinishedTime(log), - "Running inspector/pass.html (1 of 1)\nRESULT group_name: test_name= 42 ms\nFinished: 0.1 s\n\n") + "Ready to run test?\nRunning inspector/pass.html (1 of 1)\nRESULT group_name: test_name= 42 ms\nFinished: 0.1 s\n\n") def test_run_test_set_for_parser_tests(self): runner, port = self.create_runner() @@ -289,14 +288,16 @@ max 548000 bytes 'RESULT Parser: memory-test: Malloc= 532000.0 bytes', 'median= 529000.0 bytes, stdev= 13000.0 bytes, min= 511000.0 bytes, max= 548000.0 bytes', 'Finished: 0.1 s', - '', ''])) + '', + 'MOCK: user.open_url: file://...', + ''])) results = runner.load_output_json()[0]['results'] values = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19] self.assertEqual(results['Parser/memory-test'], {'min': 1080.0, 'max': 1120.0, 'median': 1101.0, 'stdev': 11.0, 'avg': 1100.0, 'unit': 'ms', 'values': values}) self.assertEqual(results['Parser/memory-test:JSHeap'], {'min': 811000.0, 'max': 848000.0, 'median': 829000.0, 'stdev': 15000.0, 'avg': 832000.0, 'unit': 'bytes', 'values': values}) self.assertEqual(results['Parser/memory-test:Malloc'], {'min': 511000.0, 'max': 548000.0, 'median': 529000.0, 'stdev': 13000.0, 'avg': 532000.0, 'unit': 'bytes', 'values': values}) - def _test_run_with_json_output(self, runner, filesystem, upload_suceeds=False, expected_exit_code=0): + def _test_run_with_json_output(self, runner, filesystem, upload_suceeds=False, results_shown=True, expected_exit_code=0): filesystem.write_text_file(runner._base_path + '/inspector/pass.html', 'some content') filesystem.write_text_file(runner._base_path + '/Bindings/event-target-wrapper.html', 'some content') @@ -318,18 +319,19 @@ max 548000 bytes stdout, stderr, logs = output_capture.restore_output() if not expected_exit_code: - self.assertEqual(self.normalizeFinishedTime(logs), - '\n'.join(['Running 2 tests', - 'Running Bindings/event-target-wrapper.html (1 of 2)', - 'RESULT Bindings: event-target-wrapper= 1489.05 ms', - 'median= 1487.0 ms, stdev= 14.46 ms, min= 1471.0 ms, max= 1510.0 ms', - 'Finished: 0.1 s', - '', - 'Running inspector/pass.html (2 of 2)', - 'RESULT group_name: test_name= 42 ms', - 'Finished: 0.1 s', - '', - ''])) + expected_logs = '\n'.join(['Running 2 tests', + 'Running Bindings/event-target-wrapper.html (1 of 2)', + 'RESULT Bindings: event-target-wrapper= 1489.05 ms', + 'median= 1487.0 ms, stdev= 14.46 ms, min= 1471.0 ms, max= 1510.0 ms', + 'Finished: 0.1 s', + '', + 'Running inspector/pass.html (2 of 2)', + 'RESULT group_name: test_name= 42 ms', + 'Finished: 0.1 s', + '', '']) + if results_shown: + expected_logs += 'MOCK: user.open_url: file://...\n' + self.assertEqual(self.normalizeFinishedTime(logs), expected_logs) self.assertEqual(uploaded[0], upload_suceeds) @@ -373,7 +375,7 @@ max 548000 bytes def test_run_respects_no_results(self): runner, port = self.create_runner(args=['--output-json-path=/mock-checkout/output.json', '--test-results-server=some.host', '--no-results']) - self._test_run_with_json_output(runner, port.host.filesystem, upload_suceeds=False) + self._test_run_with_json_output(runner, port.host.filesystem, upload_suceeds=False, results_shown=False) self.assertFalse(port.host.filesystem.isfile('/mock-checkout/output.json')) def test_run_generates_json_by_default(self): @@ -428,7 +430,7 @@ max 548000 bytes page_shown = [] port.show_results_html_file = lambda path: page_shown.append(path) filesystem = port.host.filesystem - self._test_run_with_json_output(runner, filesystem) + self._test_run_with_json_output(runner, filesystem, results_shown=False) expected_entry = {"timestamp": 123456789, "results": self._event_target_wrapper_and_inspector_results, "webkit-revision": "5678", "branch": "webkit-trunk"} @@ -441,7 +443,7 @@ max 548000 bytes '<script>%s</script>END' % json_output) self.assertEqual(page_shown[0], '/mock-checkout/output.html') - self._test_run_with_json_output(runner, filesystem) + self._test_run_with_json_output(runner, filesystem, results_shown=False) json_output = port.host.filesystem.read_text_file('/mock-checkout/output.json') self.assertEqual(json.loads(json_output), [expected_entry, expected_entry]) self.assertEqual(filesystem.read_text_file('/mock-checkout/output.html'), @@ -454,14 +456,14 @@ max 548000 bytes runner, port = self.create_runner_and_setup_results_template(args=['--output-json-path=/mock-checkout/output.json']) page_shown = [] port.show_results_html_file = show_results_html_file - self._test_run_with_json_output(runner, port.host.filesystem) + self._test_run_with_json_output(runner, port.host.filesystem, results_shown=False) self.assertEqual(page_shown[0], '/mock-checkout/output.html') runner, port = self.create_runner_and_setup_results_template(args=['--output-json-path=/mock-checkout/output.json', '--no-show-results']) page_shown = [] port.show_results_html_file = show_results_html_file - self._test_run_with_json_output(runner, port.host.filesystem) + self._test_run_with_json_output(runner, port.host.filesystem, results_shown=False) self.assertEqual(page_shown, []) def test_run_with_bad_output_json(self): diff --git a/Tools/Scripts/webkitpy/style/checkers/cpp.py b/Tools/Scripts/webkitpy/style/checkers/cpp.py index f9d534b8c..c99cbea3d 100644 --- a/Tools/Scripts/webkitpy/style/checkers/cpp.py +++ b/Tools/Scripts/webkitpy/style/checkers/cpp.py @@ -1210,6 +1210,44 @@ class _FileState(object): return self.is_c() or self.is_objective_c() +class _EnumState(object): + """Maintains whether currently in an enum declaration, and checks whether + enum declarations follow the style guide. + """ + + def __init__(self): + self.in_enum_decl = False + + def process_clean_line(self, line): + # FIXME: The regular expressions for expr_all_uppercase and expr_enum_end only accept integers + # and identifiers for the value of the enumerator, but do not accept any other constant + # expressions. However, this is sufficient for now (11/27/2012). + expr_all_uppercase = r'\s*[A-Z0-9_]+\s*(?:=\s*[a-zA-Z0-9]+\s*)?,?\s*$' + expr_starts_lowercase = r'\s*[a-z]' + expr_enum_end = r'}\s*(?:[a-zA-Z0-9]+\s*(?:=\s*[a-zA-Z0-9]+)?)?\s*;\s*' + expr_enum_start = r'\s*enum(?:\s+[a-zA-Z0-9]+)?\s*\{?\s*' + if self.in_enum_decl: + if match(r'\s*' + expr_enum_end + r'$', line): + self.in_enum_decl = False + elif match(expr_all_uppercase, line): + return False + elif match(expr_starts_lowercase, line): + return False + else: + if match(expr_enum_start + r'$', line): + self.in_enum_decl = True + else: + matched = match(expr_enum_start + r'(?P<members>.*)' + expr_enum_end + r'$', line) + if matched: + members = matched.group('members').split(',') + for member in members: + if match(expr_all_uppercase, member): + return False + if match(expr_starts_lowercase, member): + return False + return True + return True + def check_for_non_standard_constructs(clean_lines, line_number, class_state, error): """Logs an error if we see certain non-ANSI constructs ignored by gcc-2. @@ -2040,6 +2078,21 @@ def check_namespace_indentation(clean_lines, line_number, file_extension, file_s break; +def check_enum_casing(clean_lines, line_number, enum_state, error): + """Looks for incorrectly named enum values. + + Args: + clean_lines: A CleansedLines instance containing the file. + line_number: The number of the line to check. + enum_state: A _EnumState instance which maintains enum declaration state. + error: The function to call with any errors found. + """ + + line = clean_lines.elided[line_number] # Get rid of comments and strings. + if not enum_state.process_clean_line(line): + error(line_number, 'readability/enum_casing', 4, + 'enum members should use InterCaps with an initial capital letter.') + def check_directive_indentation(clean_lines, line_number, file_state, error): """Looks for indentation of preprocessor directives. @@ -2535,7 +2588,7 @@ def get_line_width(line): return len(line) -def check_style(clean_lines, line_number, file_extension, class_state, file_state, error): +def check_style(clean_lines, line_number, file_extension, class_state, file_state, enum_state, error): """Checks rules from the 'C++ style rules' section of cppguide.html. Most of these rules are hard to test (naming, comment style), but we @@ -2550,6 +2603,7 @@ def check_style(clean_lines, line_number, file_extension, class_state, file_stat the current stack of nested class declarations being parsed. file_state: A _FileState instance which maintains information about the state of things in the file. + enum_state: A _EnumState instance which maintains the current enum state. error: The function to call with any errors found. """ @@ -2604,6 +2658,7 @@ def check_style(clean_lines, line_number, file_extension, class_state, file_stat check_for_comparisons_to_zero(clean_lines, line_number, error) check_for_null(clean_lines, line_number, file_state, error) check_indentation_amount(clean_lines, line_number, error) + check_enum_casing(clean_lines, line_number, enum_state, error) _RE_PATTERN_INCLUDE_NEW_STYLE = re.compile(r'#include +"[^/]+\.h"') @@ -3478,7 +3533,7 @@ def check_for_include_what_you_use(filename, clean_lines, include_state, error): def process_line(filename, file_extension, clean_lines, line, include_state, function_state, - class_state, file_state, error): + class_state, file_state, enum_state, error): """Processes a single line in the file. Args: @@ -3493,6 +3548,8 @@ def process_line(filename, file_extension, the current stack of nested class declarations being parsed. file_state: A _FileState instance which maintains information about the state of things in the file. + enum_state: A _EnumState instance which maintains an enum declaration + state. error: A callable to which errors are reported, which takes arguments: line number, error level, and message @@ -3508,7 +3565,7 @@ def process_line(filename, file_extension, check_pass_ptr_usage(clean_lines, line, function_state, error) check_for_leaky_patterns(clean_lines, line, function_state, error) check_for_multiline_comments_and_strings(clean_lines, line, error) - check_style(clean_lines, line, file_extension, class_state, file_state, error) + check_style(clean_lines, line, file_extension, class_state, file_state, enum_state, error) check_language(filename, clean_lines, line, file_extension, include_state, file_state, error) check_for_non_standard_constructs(clean_lines, line, class_state, error) @@ -3541,9 +3598,11 @@ def _process_lines(filename, file_extension, lines, error, min_confidence): remove_multi_line_comments(lines, error) clean_lines = CleansedLines(lines) file_state = _FileState(clean_lines, file_extension) + enum_state = _EnumState() for line in xrange(clean_lines.num_lines()): process_line(filename, file_extension, clean_lines, line, - include_state, function_state, class_state, file_state, error) + include_state, function_state, class_state, file_state, + enum_state, error) class_state.check_finished(error) check_for_include_what_you_use(filename, clean_lines, include_state, error) @@ -3585,6 +3644,7 @@ class CppChecker(object): 'readability/comparison_to_zero', 'readability/constructors', 'readability/control_flow', + 'readability/enum_casing', 'readability/fn_size', 'readability/function', 'readability/multiline_comment', diff --git a/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py b/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py index 6de7df619..822ed77c9 100644 --- a/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py +++ b/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py @@ -3258,6 +3258,43 @@ class NoNonVirtualDestructorsTest(CppStyleTestBase): };''', 'The class Foo probably needs a virtual destructor') + def test_enum_casing(self): + self.assert_multi_line_lint( + '''\ + enum Foo { + FOO_ONE = 1, + FOO_TWO + }; + enum { FOO_ONE }; + enum {FooOne, fooTwo}; + enum { + FOO_ONE + };''', + ['enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4]'] * 5) + + self.assert_multi_line_lint( + '''\ + enum Foo { + fooOne = 1, + FooTwo = 2 + };''', + 'enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4]') + + self.assert_multi_line_lint( + '''\ + enum Foo { + FooOne = 1, + FooTwo + } fooVar = FooOne; + enum { FooOne, FooTwo }; + enum { FooOne, FooTwo } fooVar = FooTwo; + enum { FooOne= FooTwo } foo; + enum Enum123 { + FooOne, + FooTwo = FooOne, + };''', + '') + def test_destructor_non_virtual_when_virtual_needed(self): self.assert_multi_line_lint_re( '''\ @@ -3280,7 +3317,7 @@ class NoNonVirtualDestructorsTest(CppStyleTestBase): '''\ class Foo { enum Goo { - GOO + Goo }; virtual void foo(); };''', diff --git a/Tools/Scripts/webkitpy/test/main.py b/Tools/Scripts/webkitpy/test/main.py index d8f997805..5f16beca6 100644 --- a/Tools/Scripts/webkitpy/test/main.py +++ b/Tools/Scripts/webkitpy/test/main.py @@ -139,7 +139,7 @@ class Tester(object): self._options.child_processes = 1 import webkitpy.thirdparty.autoinstalled.coverage as coverage - cov = coverage.coverage() + cov = coverage.coverage(omit=["/usr/*", "*/webkitpy/thirdparty/autoinstalled/*"]) cov.start() self.printer.write_update("Checking imports ...") diff --git a/Tools/Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py b/Tools/Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py index 2958c6cc1..2211b1de0 100644 --- a/Tools/Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py +++ b/Tools/Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py @@ -27,11 +27,11 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. from datetime import datetime +import logging import unittest from webkitpy.common.net import bugzilla from webkitpy.common.net.layouttestresults import LayoutTestResults -from webkitpy.common.system.deprecated_logging import error, log from webkitpy.common.system.executive import ScriptError from webkitpy.common.system.outputcapture import OutputCapture from webkitpy.layout_tests.models import test_results @@ -41,6 +41,8 @@ from webkitpy.tool.bot.commitqueuetask import * from webkitpy.tool.bot.expectedfailures import ExpectedFailures from webkitpy.tool.mocktool import MockTool +_log = logging.getLogger(__name__) + class MockCommitQueue(CommitQueueTaskDelegate): def __init__(self, error_plan): @@ -48,18 +50,18 @@ class MockCommitQueue(CommitQueueTaskDelegate): self._failure_status_id = 0 def run_command(self, command): - log("run_webkit_patch: %s" % command) + _log.info("run_webkit_patch: %s" % command) if self._error_plan: error = self._error_plan.pop(0) if error: raise error def command_passed(self, success_message, patch): - log("command_passed: success_message='%s' patch='%s'" % ( + _log.info("command_passed: success_message='%s' patch='%s'" % ( success_message, patch.id())) def command_failed(self, failure_message, script_error, patch): - log("command_failed: failure_message='%s' script_error='%s' patch='%s'" % ( + _log.info("command_failed: failure_message='%s' script_error='%s' patch='%s'" % ( failure_message, script_error, patch.id())) self._failure_status_id += 1 return self._failure_status_id @@ -75,10 +77,10 @@ class MockCommitQueue(CommitQueueTaskDelegate): def report_flaky_tests(self, patch, flaky_results, results_archive): flaky_tests = [result.filename for result in flaky_results] - log("report_flaky_tests: patch='%s' flaky_tests='%s' archive='%s'" % (patch.id(), flaky_tests, results_archive.filename)) + _log.info("report_flaky_tests: patch='%s' flaky_tests='%s' archive='%s'" % (patch.id(), flaky_tests, results_archive.filename)) def archive_last_test_results(self, patch): - log("archive_last_test_results: patch='%s'" % patch.id()) + _log.info("archive_last_test_results: patch='%s'" % patch.id()) archive = Mock() archive.filename = "mock-archive-%s.zip" % patch.id() return archive @@ -121,18 +123,18 @@ class GoldenScriptError(ScriptError): class CommitQueueTaskTest(unittest.TestCase): - def _run_through_task(self, commit_queue, expected_stderr, expected_exception=None, expect_retry=False): + def _run_through_task(self, commit_queue, expected_logs, expected_exception=None, expect_retry=False): tool = MockTool(log_executive=True) patch = tool.bugs.fetch_attachment(10000) task = CommitQueueTask(commit_queue, patch) - success = OutputCapture().assert_outputs(self, task.run, expected_stderr=expected_stderr, expected_exception=expected_exception) + success = OutputCapture().assert_outputs(self, task.run, expected_logs=expected_logs, expected_exception=expected_exception) if not expected_exception: self.assertEqual(success, not expect_retry) return task def test_success_case(self): commit_queue = MockCommitQueue([]) - expected_stderr = """run_webkit_patch: ['clean'] + expected_logs = """run_webkit_patch: ['clean'] command_passed: success_message='Cleaned working directory' patch='10000' run_webkit_patch: ['update'] command_passed: success_message='Updated working directory' patch='10000' @@ -147,12 +149,12 @@ command_passed: success_message='Passed tests' patch='10000' run_webkit_patch: ['land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 10000] command_passed: success_message='Landed patch' patch='10000' """ - self._run_through_task(commit_queue, expected_stderr) + self._run_through_task(commit_queue, expected_logs) def test_fast_success_case(self): commit_queue = MockCommitQueue([]) commit_queue.did_pass_testing_ews = lambda patch: True - expected_stderr = """run_webkit_patch: ['clean'] + expected_logs = """run_webkit_patch: ['clean'] command_passed: success_message='Cleaned working directory' patch='10000' run_webkit_patch: ['update'] command_passed: success_message='Updated working directory' patch='10000' @@ -165,28 +167,28 @@ command_passed: success_message='Built patch' patch='10000' run_webkit_patch: ['land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 10000] command_passed: success_message='Landed patch' patch='10000' """ - self._run_through_task(commit_queue, expected_stderr) + self._run_through_task(commit_queue, expected_logs) def test_clean_failure(self): commit_queue = MockCommitQueue([ ScriptError("MOCK clean failure"), ]) - expected_stderr = """run_webkit_patch: ['clean'] + expected_logs = """run_webkit_patch: ['clean'] command_failed: failure_message='Unable to clean working directory' script_error='MOCK clean failure' patch='10000' """ - self._run_through_task(commit_queue, expected_stderr, expect_retry=True) + self._run_through_task(commit_queue, expected_logs, expect_retry=True) def test_update_failure(self): commit_queue = MockCommitQueue([ None, ScriptError("MOCK update failure"), ]) - expected_stderr = """run_webkit_patch: ['clean'] + expected_logs = """run_webkit_patch: ['clean'] command_passed: success_message='Cleaned working directory' patch='10000' run_webkit_patch: ['update'] command_failed: failure_message='Unable to update working directory' script_error='MOCK update failure' patch='10000' """ - self._run_through_task(commit_queue, expected_stderr, expect_retry=True) + self._run_through_task(commit_queue, expected_logs, expect_retry=True) def test_apply_failure(self): commit_queue = MockCommitQueue([ @@ -194,14 +196,14 @@ command_failed: failure_message='Unable to update working directory' script_erro None, GoldenScriptError("MOCK apply failure"), ]) - expected_stderr = """run_webkit_patch: ['clean'] + expected_logs = """run_webkit_patch: ['clean'] command_passed: success_message='Cleaned working directory' patch='10000' run_webkit_patch: ['update'] command_passed: success_message='Updated working directory' patch='10000' run_webkit_patch: ['apply-attachment', '--no-update', '--non-interactive', 10000] command_failed: failure_message='Patch does not apply' script_error='MOCK apply failure' patch='10000' """ - self._run_through_task(commit_queue, expected_stderr, GoldenScriptError) + self._run_through_task(commit_queue, expected_logs, GoldenScriptError) def test_validate_changelog_failure(self): commit_queue = MockCommitQueue([ @@ -210,7 +212,7 @@ command_failed: failure_message='Patch does not apply' script_error='MOCK apply None, GoldenScriptError("MOCK validate failure"), ]) - expected_stderr = """run_webkit_patch: ['clean'] + expected_logs = """run_webkit_patch: ['clean'] command_passed: success_message='Cleaned working directory' patch='10000' run_webkit_patch: ['update'] command_passed: success_message='Updated working directory' patch='10000' @@ -219,7 +221,7 @@ command_passed: success_message='Applied patch' patch='10000' run_webkit_patch: ['validate-changelog', '--non-interactive', 10000] command_failed: failure_message='ChangeLog did not pass validation' script_error='MOCK validate failure' patch='10000' """ - self._run_through_task(commit_queue, expected_stderr, GoldenScriptError) + self._run_through_task(commit_queue, expected_logs, GoldenScriptError) def test_build_failure(self): commit_queue = MockCommitQueue([ @@ -229,7 +231,7 @@ command_failed: failure_message='ChangeLog did not pass validation' script_error None, GoldenScriptError("MOCK build failure"), ]) - expected_stderr = """run_webkit_patch: ['clean'] + expected_logs = """run_webkit_patch: ['clean'] command_passed: success_message='Cleaned working directory' patch='10000' run_webkit_patch: ['update'] command_passed: success_message='Updated working directory' patch='10000' @@ -242,7 +244,7 @@ command_failed: failure_message='Patch does not build' script_error='MOCK build run_webkit_patch: ['build', '--force-clean', '--no-update', '--build-style=both'] command_passed: success_message='Able to build without patch' patch='10000' """ - self._run_through_task(commit_queue, expected_stderr, GoldenScriptError) + self._run_through_task(commit_queue, expected_logs, GoldenScriptError) def test_red_build_failure(self): commit_queue = MockCommitQueue([ @@ -253,7 +255,7 @@ command_passed: success_message='Able to build without patch' patch='10000' ScriptError("MOCK build failure"), ScriptError("MOCK clean build failure"), ]) - expected_stderr = """run_webkit_patch: ['clean'] + expected_logs = """run_webkit_patch: ['clean'] command_passed: success_message='Cleaned working directory' patch='10000' run_webkit_patch: ['update'] command_passed: success_message='Updated working directory' patch='10000' @@ -266,7 +268,7 @@ command_failed: failure_message='Patch does not build' script_error='MOCK build run_webkit_patch: ['build', '--force-clean', '--no-update', '--build-style=both'] command_failed: failure_message='Unable to build without patch' script_error='MOCK clean build failure' patch='10000' """ - self._run_through_task(commit_queue, expected_stderr, expect_retry=True) + self._run_through_task(commit_queue, expected_logs, expect_retry=True) def test_flaky_test_failure(self): commit_queue = MockCommitQueue([ @@ -280,7 +282,7 @@ command_failed: failure_message='Unable to build without patch' script_error='MO # CommitQueueTask will only report flaky tests if we successfully parsed # results.html and returned a LayoutTestResults object, so we fake one. commit_queue.test_results = lambda: LayoutTestResults([]) - expected_stderr = """run_webkit_patch: ['clean'] + expected_logs = """run_webkit_patch: ['clean'] command_passed: success_message='Cleaned working directory' patch='10000' run_webkit_patch: ['update'] command_passed: success_message='Updated working directory' patch='10000' @@ -299,7 +301,7 @@ report_flaky_tests: patch='10000' flaky_tests='[]' archive='mock-archive-10000.z run_webkit_patch: ['land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 10000] command_passed: success_message='Landed patch' patch='10000' """ - self._run_through_task(commit_queue, expected_stderr) + self._run_through_task(commit_queue, expected_logs) def test_failed_archive(self): commit_queue = MockCommitQueue([ @@ -314,7 +316,7 @@ command_passed: success_message='Landed patch' patch='10000' # It's possible delegate to fail to archive layout tests, don't try to report # flaky tests when that happens. commit_queue.archive_last_test_results = lambda patch: None - expected_stderr = """run_webkit_patch: ['clean'] + expected_logs = """run_webkit_patch: ['clean'] command_passed: success_message='Cleaned working directory' patch='10000' run_webkit_patch: ['update'] command_passed: success_message='Updated working directory' patch='10000' @@ -331,7 +333,7 @@ command_passed: success_message='Passed tests' patch='10000' run_webkit_patch: ['land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 10000] command_passed: success_message='Landed patch' patch='10000' """ - self._run_through_task(commit_queue, expected_stderr) + self._run_through_task(commit_queue, expected_logs) def test_double_flaky_test_failure(self): commit_queue = FailingTestCommitQueue([ @@ -348,9 +350,9 @@ command_passed: success_message='Landed patch' patch='10000' "foo.html", ]) # The (subtle) point of this test is that report_flaky_tests does not appear - # in the expected_stderr for this run. + # in the expected_logs for this run. # Note also that there is no attempt to run the tests w/o the patch. - expected_stderr = """run_webkit_patch: ['clean'] + expected_logs = """run_webkit_patch: ['clean'] command_passed: success_message='Cleaned working directory' patch='10000' run_webkit_patch: ['update'] command_passed: success_message='Updated working directory' patch='10000' @@ -369,7 +371,7 @@ command_failed: failure_message='Patch does not pass tests' script_error='MOCK t tool = MockTool(log_executive=True) patch = tool.bugs.fetch_attachment(10000) task = CommitQueueTask(commit_queue, patch) - success = OutputCapture().assert_outputs(self, task.run, expected_stderr=expected_stderr) + success = OutputCapture().assert_outputs(self, task.run, expected_logs=expected_logs) self.assertEqual(success, False) def test_test_failure(self): @@ -382,7 +384,7 @@ command_failed: failure_message='Patch does not pass tests' script_error='MOCK t GoldenScriptError("MOCK test failure"), ScriptError("MOCK test failure again"), ]) - expected_stderr = """run_webkit_patch: ['clean'] + expected_logs = """run_webkit_patch: ['clean'] command_passed: success_message='Cleaned working directory' patch='10000' run_webkit_patch: ['update'] command_passed: success_message='Updated working directory' patch='10000' @@ -401,7 +403,7 @@ archive_last_test_results: patch='10000' run_webkit_patch: ['build-and-test', '--force-clean', '--no-update', '--build', '--test', '--non-interactive'] command_passed: success_message='Able to pass tests without patch' patch='10000' """ - self._run_through_task(commit_queue, expected_stderr, GoldenScriptError) + self._run_through_task(commit_queue, expected_logs, GoldenScriptError) def test_red_test_failure(self): commit_queue = FailingTestCommitQueue([ @@ -421,7 +423,7 @@ command_passed: success_message='Able to pass tests without patch' patch='10000' # Tests always fail, and always return the same results, but we # should still be able to land in this case! - expected_stderr = """run_webkit_patch: ['clean'] + expected_logs = """run_webkit_patch: ['clean'] command_passed: success_message='Cleaned working directory' patch='10000' run_webkit_patch: ['update'] command_passed: success_message='Updated working directory' patch='10000' @@ -442,7 +444,7 @@ command_failed: failure_message='Unable to pass tests without patch (tree is red run_webkit_patch: ['land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 10000] command_passed: success_message='Landed patch' patch='10000' """ - self._run_through_task(commit_queue, expected_stderr) + self._run_through_task(commit_queue, expected_logs) def test_very_red_tree_retry(self): lots_of_failing_tests = map(lambda num: "test-%s.html" % num, range(0, 100)) @@ -464,7 +466,7 @@ command_passed: success_message='Landed patch' patch='10000' # Tests always fail, and return so many failures that we do not # trust the results (see ExpectedFailures._can_trust_results) so we # just give up and retry the patch. - expected_stderr = """run_webkit_patch: ['clean'] + expected_logs = """run_webkit_patch: ['clean'] command_passed: success_message='Cleaned working directory' patch='10000' run_webkit_patch: ['update'] command_passed: success_message='Updated working directory' patch='10000' @@ -483,7 +485,7 @@ archive_last_test_results: patch='10000' run_webkit_patch: ['build-and-test', '--force-clean', '--no-update', '--build', '--test', '--non-interactive'] command_failed: failure_message='Unable to pass tests without patch (tree is red?)' script_error='MOCK clean test failure' patch='10000' """ - self._run_through_task(commit_queue, expected_stderr, expect_retry=True) + self._run_through_task(commit_queue, expected_logs, expect_retry=True) def test_red_tree_patch_rejection(self): commit_queue = FailingTestCommitQueue([ @@ -503,7 +505,7 @@ command_failed: failure_message='Unable to pass tests without patch (tree is red # Tests always fail, but the clean tree only fails one test # while the patch fails two. So we should reject the patch! - expected_stderr = """run_webkit_patch: ['clean'] + expected_logs = """run_webkit_patch: ['clean'] command_passed: success_message='Cleaned working directory' patch='10000' run_webkit_patch: ['update'] command_passed: success_message='Updated working directory' patch='10000' @@ -522,7 +524,7 @@ archive_last_test_results: patch='10000' run_webkit_patch: ['build-and-test', '--force-clean', '--no-update', '--build', '--test', '--non-interactive'] command_failed: failure_message='Unable to pass tests without patch (tree is red?)' script_error='MOCK clean test failure' patch='10000' """ - task = self._run_through_task(commit_queue, expected_stderr, GoldenScriptError) + task = self._run_through_task(commit_queue, expected_logs, GoldenScriptError) self.assertEqual(task.results_from_patch_test_run(task._patch).failing_tests(), ["foo.html", "bar.html"]) # failure_status_id should be of the test with patch (1), not the test without patch (2). self.assertEqual(task.failure_status_id, 1) @@ -537,7 +539,7 @@ command_failed: failure_message='Unable to pass tests without patch (tree is red None, GoldenScriptError("MOCK land failure"), ]) - expected_stderr = """run_webkit_patch: ['clean'] + expected_logs = """run_webkit_patch: ['clean'] command_passed: success_message='Cleaned working directory' patch='10000' run_webkit_patch: ['update'] command_passed: success_message='Updated working directory' patch='10000' @@ -553,7 +555,7 @@ run_webkit_patch: ['land-attachment', '--force-clean', '--non-interactive', '--p command_failed: failure_message='Unable to land patch' script_error='MOCK land failure' patch='10000' """ # FIXME: This should really be expect_retry=True for a better user experiance. - self._run_through_task(commit_queue, expected_stderr, GoldenScriptError) + self._run_through_task(commit_queue, expected_logs, GoldenScriptError) def _expect_validate(self, patch, is_valid): class MockDelegate(object): diff --git a/Tools/Scripts/webkitpy/tool/bot/feeders.py b/Tools/Scripts/webkitpy/tool/bot/feeders.py index 4ba2f0485..f4bc4b927 100644 --- a/Tools/Scripts/webkitpy/tool/bot/feeders.py +++ b/Tools/Scripts/webkitpy/tool/bot/feeders.py @@ -26,10 +26,13 @@ # (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 logging + from webkitpy.common.config.committervalidator import CommitterValidator -from webkitpy.common.system.deprecated_logging import log from webkitpy.tool.grammar import pluralize +_log = logging.getLogger(__name__) + class AbstractFeeder(object): def __init__(self, tool): @@ -50,7 +53,7 @@ class CommitQueueFeeder(AbstractFeeder): # FIXME: This is the last use of update_work_items, the commit-queue # should move to feeding patches one at a time like the EWS does. self._tool.status_server.update_work_items(self.queue_name, item_ids) - log("Feeding %s items %s" % (self.queue_name, item_ids)) + _log.info("Feeding %s items %s" % (self.queue_name, item_ids)) def feed(self): patches = self._validate_patches() @@ -89,7 +92,7 @@ class EWSFeeder(AbstractFeeder): def feed(self): ids_needing_review = set(self._tool.bugs.queries.fetch_attachment_ids_from_review_queue()) new_ids = ids_needing_review.difference(self._ids_sent_to_server) - log("Feeding EWS (%s, %s new)" % (pluralize("r? patch", len(ids_needing_review)), len(new_ids))) + _log.info("Feeding EWS (%s, %s new)" % (pluralize("r? patch", len(ids_needing_review)), len(new_ids))) for attachment_id in new_ids: # Order doesn't really matter for the EWS. self._tool.status_server.submit_to_ews(attachment_id) self._ids_sent_to_server.add(attachment_id) diff --git a/Tools/Scripts/webkitpy/tool/bot/feeders_unittest.py b/Tools/Scripts/webkitpy/tool/bot/feeders_unittest.py index 060a5c85b..9d0b71408 100644 --- a/Tools/Scripts/webkitpy/tool/bot/feeders_unittest.py +++ b/Tools/Scripts/webkitpy/tool/bot/feeders_unittest.py @@ -38,7 +38,7 @@ from webkitpy.tool.mocktool import MockTool class FeedersTest(unittest.TestCase): def test_commit_queue_feeder(self): feeder = CommitQueueFeeder(MockTool()) - expected_stderr = u"""Warning, attachment 10001 on bug 50000 has invalid committer (non-committer@example.com) + expected_logs = """Warning, attachment 10001 on bug 50000 has invalid committer (non-committer@example.com) Warning, attachment 10001 on bug 50000 has invalid committer (non-committer@example.com) MOCK setting flag 'commit-queue' to '-' on attachment '10001' with comment 'Rejecting attachment 10001 from commit-queue.' and additional comment 'non-committer@example.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py. @@ -48,7 +48,7 @@ MOCK setting flag 'commit-queue' to '-' on attachment '10001' with comment 'Reje MOCK: update_work_items: commit-queue [10005, 10000] Feeding commit-queue items [10005, 10000] """ - OutputCapture().assert_outputs(self, feeder.feed, expected_stderr=expected_stderr) + OutputCapture().assert_outputs(self, feeder.feed, expected_logs=expected_logs) def _mock_attachment(self, is_rollout, attach_date): attachment = Mock() diff --git a/Tools/Scripts/webkitpy/tool/bot/flakytestreporter_unittest.py b/Tools/Scripts/webkitpy/tool/bot/flakytestreporter_unittest.py index eeb06c3af..48c511281 100644 --- a/Tools/Scripts/webkitpy/tool/bot/flakytestreporter_unittest.py +++ b/Tools/Scripts/webkitpy/tool/bot/flakytestreporter_unittest.py @@ -70,7 +70,7 @@ class FlakyTestReporterTest(unittest.TestCase): def test_create_bug_for_flaky_test(self): reporter = FlakyTestReporter(MockTool(), 'dummy-queue') - expected_stderr = """MOCK create_bug + expected_logs = """MOCK create_bug bug_title: Flaky Test: foo/bar.html bug_description: This is an automatically generated bug from the dummy-queue. foo/bar.html has been flaky on the dummy-queue. @@ -90,7 +90,7 @@ component: Tools / Tests cc: test@test.com blocked: 50856 """ - OutputCapture().assert_outputs(self, reporter._create_bug_for_flaky_test, ['foo/bar.html', ['test@test.com'], 'FLAKE_MESSAGE'], expected_stderr=expected_stderr) + OutputCapture().assert_outputs(self, reporter._create_bug_for_flaky_test, ['foo/bar.html', ['test@test.com'], 'FLAKE_MESSAGE'], expected_logs=expected_logs) def test_follow_duplicate_chain(self): tool = MockTool() @@ -105,7 +105,8 @@ blocked: 50856 reporter = FlakyTestReporter(tool, 'dummy-queue') reporter._lookup_bug_for_flaky_test = lambda bug_id: None patch = tool.bugs.fetch_attachment(10000) - expected_stderr = """MOCK create_bug + expected_logs = """Bug does not already exist for foo/bar.html, creating. +MOCK create_bug bug_title: Flaky Test: foo/bar.html bug_description: This is an automatically generated bug from the dummy-queue. foo/bar.html has been flaky on the dummy-queue. @@ -144,7 +145,7 @@ The dummy-queue is continuing to process your patch. def namelist(self): return ['foo/bar-diffs.txt'] - OutputCapture().assert_outputs(self, reporter.report_flaky_tests, [patch, test_results, MockZipFile()], expected_stderr=expected_stderr) + OutputCapture().assert_outputs(self, reporter.report_flaky_tests, [patch, test_results, MockZipFile()], expected_logs=expected_logs) def test_optional_author_string(self): reporter = FlakyTestReporter(MockTool(), 'dummy-queue') diff --git a/Tools/Scripts/webkitpy/tool/bot/irc_command_unittest.py b/Tools/Scripts/webkitpy/tool/bot/irc_command_unittest.py index 4abee6678..e307e6ea9 100644 --- a/Tools/Scripts/webkitpy/tool/bot/irc_command_unittest.py +++ b/Tools/Scripts/webkitpy/tool/bot/irc_command_unittest.py @@ -86,8 +86,8 @@ class IRCCommandTest(unittest.TestCase): rollout = Rollout() tool = MockTool() tool.executive = MockExecutive(should_log=True) - expected_stderr = "MOCK run_and_throw_if_fail: ['mock-update-webkit'], cwd=/mock-checkout\n" - OutputCapture().assert_outputs(self, rollout._update_working_copy, [tool], expected_stderr=expected_stderr) + expected_logs = "MOCK run_and_throw_if_fail: ['mock-update-webkit'], cwd=/mock-checkout\n" + OutputCapture().assert_outputs(self, rollout._update_working_copy, [tool], expected_logs=expected_logs) def test_rollout(self): rollout = Rollout() diff --git a/Tools/Scripts/webkitpy/tool/bot/ircbot_unittest.py b/Tools/Scripts/webkitpy/tool/bot/ircbot_unittest.py index ce9a76bda..f96b7b6b5 100644 --- a/Tools/Scripts/webkitpy/tool/bot/ircbot_unittest.py +++ b/Tools/Scripts/webkitpy/tool/bot/ircbot_unittest.py @@ -71,8 +71,8 @@ class IRCBotTest(unittest.TestCase): raise Exception("mock_exception") bot._parse_command_and_args = lambda request: (CommandWithException, []) - expected_stderr = 'MOCK: irc.post: Exception executing command: mock_exception\n' - OutputCapture().assert_outputs(self, bot.process_message, args=["mock_nick", "ignored message"], expected_stderr=expected_stderr) + expected_logs = 'MOCK: irc.post: Exception executing command: mock_exception\n' + OutputCapture().assert_outputs(self, bot.process_message, args=["mock_nick", "ignored message"], expected_logs=expected_logs) class CommandWithException(object): def execute(self, nick, args, tool, sheriff): @@ -84,80 +84,80 @@ class IRCBotTest(unittest.TestCase): def test_hi(self): random.seed(23324) - expected_stderr = 'MOCK: irc.post: "Only you can prevent forest fires." -- Smokey the Bear\n' - OutputCapture().assert_outputs(self, run, args=["hi"], expected_stderr=expected_stderr) + expected_logs = 'MOCK: irc.post: "Only you can prevent forest fires." -- Smokey the Bear\n' + OutputCapture().assert_outputs(self, run, args=["hi"], expected_logs=expected_logs) def test_help(self): - expected_stderr = "MOCK: irc.post: mock_nick: Available commands: create-bug, help, hi, restart, roll-chromium-deps, rollout, whois\n" - OutputCapture().assert_outputs(self, run, args=["help"], expected_stderr=expected_stderr) + expected_logs = "MOCK: irc.post: mock_nick: Available commands: create-bug, help, hi, restart, roll-chromium-deps, rollout, whois\n" + OutputCapture().assert_outputs(self, run, args=["help"], expected_logs=expected_logs) def test_restart(self): - expected_stderr = "MOCK: irc.post: Restarting...\n" - OutputCapture().assert_outputs(self, run, args=["restart"], expected_stderr=expected_stderr, expected_exception=TerminateQueue) + expected_logs = "MOCK: irc.post: Restarting...\n" + OutputCapture().assert_outputs(self, run, args=["restart"], expected_logs=expected_logs, expected_exception=TerminateQueue) def test_rollout(self): - expected_stderr = "MOCK: irc.post: mock_nick: Preparing rollout for http://trac.webkit.org/changeset/21654 ...\nMOCK: irc.post: mock_nick, abarth, darin, eseidel: Created rollout: http://example.com/36936\n" - OutputCapture().assert_outputs(self, run, args=["rollout 21654 This patch broke the world"], expected_stderr=expected_stderr) + expected_logs = "MOCK: irc.post: mock_nick: Preparing rollout for http://trac.webkit.org/changeset/21654 ...\nMOCK: irc.post: mock_nick, abarth, darin, eseidel: Created rollout: http://example.com/36936\n" + OutputCapture().assert_outputs(self, run, args=["rollout 21654 This patch broke the world"], expected_logs=expected_logs) def test_revert(self): - expected_stderr = "MOCK: irc.post: mock_nick: Preparing rollout for http://trac.webkit.org/changeset/21654 ...\nMOCK: irc.post: mock_nick, abarth, darin, eseidel: Created rollout: http://example.com/36936\n" - OutputCapture().assert_outputs(self, run, args=["revert 21654 This patch broke the world"], expected_stderr=expected_stderr) + expected_logs = "MOCK: irc.post: mock_nick: Preparing rollout for http://trac.webkit.org/changeset/21654 ...\nMOCK: irc.post: mock_nick, abarth, darin, eseidel: Created rollout: http://example.com/36936\n" + OutputCapture().assert_outputs(self, run, args=["revert 21654 This patch broke the world"], expected_logs=expected_logs) def test_roll_chromium_deps(self): - expected_stderr = "MOCK: irc.post: mock_nick: Rolling Chromium DEPS to r21654\nMOCK: irc.post: mock_nick: Created DEPS roll: http://example.com/36936\n" - OutputCapture().assert_outputs(self, run, args=["roll-chromium-deps 21654"], expected_stderr=expected_stderr) + expected_logs = "MOCK: irc.post: mock_nick: Rolling Chromium DEPS to r21654\nMOCK: irc.post: mock_nick: Created DEPS roll: http://example.com/36936\n" + OutputCapture().assert_outputs(self, run, args=["roll-chromium-deps 21654"], expected_logs=expected_logs) def test_roll_chromium_deps_to_lkgr(self): - expected_stderr = "MOCK: irc.post: mock_nick: Rolling Chromium DEPS to last-known good revision\nMOCK: irc.post: mock_nick: Created DEPS roll: http://example.com/36936\n" - OutputCapture().assert_outputs(self, run, args=["roll-chromium-deps"], expected_stderr=expected_stderr) + expected_logs = "MOCK: irc.post: mock_nick: Rolling Chromium DEPS to last-known good revision\nMOCK: irc.post: mock_nick: Created DEPS roll: http://example.com/36936\n" + OutputCapture().assert_outputs(self, run, args=["roll-chromium-deps"], expected_logs=expected_logs) def test_multi_rollout(self): - expected_stderr = "MOCK: irc.post: mock_nick: Preparing rollout for http://trac.webkit.org/changeset/21654, http://trac.webkit.org/changeset/21655, and http://trac.webkit.org/changeset/21656 ...\nMOCK: irc.post: mock_nick, abarth, darin, eseidel: Created rollout: http://example.com/36936\n" - OutputCapture().assert_outputs(self, run, args=["rollout 21654 21655 21656 This 21654 patch broke the world"], expected_stderr=expected_stderr) + expected_logs = "MOCK: irc.post: mock_nick: Preparing rollout for http://trac.webkit.org/changeset/21654, http://trac.webkit.org/changeset/21655, and http://trac.webkit.org/changeset/21656 ...\nMOCK: irc.post: mock_nick, abarth, darin, eseidel: Created rollout: http://example.com/36936\n" + OutputCapture().assert_outputs(self, run, args=["rollout 21654 21655 21656 This 21654 patch broke the world"], expected_logs=expected_logs) def test_rollout_with_r_in_svn_revision(self): - expected_stderr = "MOCK: irc.post: mock_nick: Preparing rollout for http://trac.webkit.org/changeset/21654 ...\nMOCK: irc.post: mock_nick, abarth, darin, eseidel: Created rollout: http://example.com/36936\n" - OutputCapture().assert_outputs(self, run, args=["rollout r21654 This patch broke the world"], expected_stderr=expected_stderr) + expected_logs = "MOCK: irc.post: mock_nick: Preparing rollout for http://trac.webkit.org/changeset/21654 ...\nMOCK: irc.post: mock_nick, abarth, darin, eseidel: Created rollout: http://example.com/36936\n" + OutputCapture().assert_outputs(self, run, args=["rollout r21654 This patch broke the world"], expected_logs=expected_logs) def test_multi_rollout_with_r_in_svn_revision(self): - expected_stderr = "MOCK: irc.post: mock_nick: Preparing rollout for http://trac.webkit.org/changeset/21654, http://trac.webkit.org/changeset/21655, and http://trac.webkit.org/changeset/21656 ...\nMOCK: irc.post: mock_nick, abarth, darin, eseidel: Created rollout: http://example.com/36936\n" - OutputCapture().assert_outputs(self, run, args=["rollout r21654 21655 r21656 This r21654 patch broke the world"], expected_stderr=expected_stderr) + expected_logs = "MOCK: irc.post: mock_nick: Preparing rollout for http://trac.webkit.org/changeset/21654, http://trac.webkit.org/changeset/21655, and http://trac.webkit.org/changeset/21656 ...\nMOCK: irc.post: mock_nick, abarth, darin, eseidel: Created rollout: http://example.com/36936\n" + OutputCapture().assert_outputs(self, run, args=["rollout r21654 21655 r21656 This r21654 patch broke the world"], expected_logs=expected_logs) def test_rollout_bananas(self): - expected_stderr = "MOCK: irc.post: mock_nick: Usage: rollout SVN_REVISION [SVN_REVISIONS] REASON\n" - OutputCapture().assert_outputs(self, run, args=["rollout bananas"], expected_stderr=expected_stderr) + expected_logs = "MOCK: irc.post: mock_nick: Usage: rollout SVN_REVISION [SVN_REVISIONS] REASON\n" + OutputCapture().assert_outputs(self, run, args=["rollout bananas"], expected_logs=expected_logs) def test_rollout_invalidate_revision(self): # When folks pass junk arguments, we should just spit the usage back at them. - expected_stderr = "MOCK: irc.post: mock_nick: Usage: rollout SVN_REVISION [SVN_REVISIONS] REASON\n" + expected_logs = "MOCK: irc.post: mock_nick: Usage: rollout SVN_REVISION [SVN_REVISIONS] REASON\n" OutputCapture().assert_outputs(self, run, args=["rollout --component=Tools 21654"], - expected_stderr=expected_stderr) + expected_logs=expected_logs) def test_rollout_invalidate_reason(self): # FIXME: I'm slightly confused as to why this doesn't return the USAGE message. - expected_stderr = """MOCK: irc.post: mock_nick: Preparing rollout for http://trac.webkit.org/changeset/21654 ... + expected_logs = """MOCK: irc.post: mock_nick: Preparing rollout for http://trac.webkit.org/changeset/21654 ... MOCK: irc.post: mock_nick, abarth, darin, eseidel: Failed to create rollout patch: MOCK: irc.post: The rollout reason may not begin with - (\"-bad (Requested by mock_nick on #webkit).\"). """ OutputCapture().assert_outputs(self, run, args=["rollout 21654 -bad"], - expected_stderr=expected_stderr) + expected_logs=expected_logs) def test_multi_rollout_invalidate_reason(self): - expected_stderr = """MOCK: irc.post: mock_nick: Preparing rollout for http://trac.webkit.org/changeset/21654, http://trac.webkit.org/changeset/21655, and http://trac.webkit.org/changeset/21656 ... + expected_logs = """MOCK: irc.post: mock_nick: Preparing rollout for http://trac.webkit.org/changeset/21654, http://trac.webkit.org/changeset/21655, and http://trac.webkit.org/changeset/21656 ... MOCK: irc.post: mock_nick, abarth, darin, eseidel: Failed to create rollout patch: MOCK: irc.post: The rollout reason may not begin with - (\"-bad (Requested by mock_nick on #webkit).\"). """ OutputCapture().assert_outputs(self, run, args=["rollout " "21654 21655 r21656 -bad"], - expected_stderr=expected_stderr) + expected_logs=expected_logs) def test_rollout_no_reason(self): - expected_stderr = "MOCK: irc.post: mock_nick: Usage: rollout SVN_REVISION [SVN_REVISIONS] REASON\n" - OutputCapture().assert_outputs(self, run, args=["rollout 21654"], expected_stderr=expected_stderr) + expected_logs = "MOCK: irc.post: mock_nick: Usage: rollout SVN_REVISION [SVN_REVISIONS] REASON\n" + OutputCapture().assert_outputs(self, run, args=["rollout 21654"], expected_logs=expected_logs) def test_multi_rollout_no_reason(self): - expected_stderr = "MOCK: irc.post: mock_nick: Usage: rollout SVN_REVISION [SVN_REVISIONS] REASON\n" - OutputCapture().assert_outputs(self, run, args=["rollout 21654 21655 r21656"], expected_stderr=expected_stderr) + expected_logs = "MOCK: irc.post: mock_nick: Usage: rollout SVN_REVISION [SVN_REVISIONS] REASON\n" + OutputCapture().assert_outputs(self, run, args=["rollout 21654 21655 r21656"], expected_logs=expected_logs) diff --git a/Tools/Scripts/webkitpy/tool/bot/layouttestresultsreader.py b/Tools/Scripts/webkitpy/tool/bot/layouttestresultsreader.py index 94a70b2bc..4e09f896f 100644 --- a/Tools/Scripts/webkitpy/tool/bot/layouttestresultsreader.py +++ b/Tools/Scripts/webkitpy/tool/bot/layouttestresultsreader.py @@ -26,11 +26,14 @@ # (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 logging + from webkitpy.common.net.layouttestresults import LayoutTestResults from webkitpy.common.net.unittestresults import UnitTestResults -from webkitpy.common.system.deprecated_logging import error, log from webkitpy.tool.steps.runtests import RunTests +_log = logging.getLogger(__name__) + class LayoutTestResultsReader(object): def __init__(self, tool, archive_directory): @@ -92,7 +95,7 @@ class LayoutTestResultsReader(object): if not zip_path: return None if not self._tool.filesystem.isdir(results_directory): - log("%s does not exist, not archiving." % results_directory) + _log.info("%s does not exist, not archiving." % results_directory) return None archive = self._tool.workspace.create_zip(zip_path, results_directory) # Remove the results directory to prevent http logs, etc. from getting huge between runs. diff --git a/Tools/Scripts/webkitpy/tool/bot/layouttestresultsreader_unittest.py b/Tools/Scripts/webkitpy/tool/bot/layouttestresultsreader_unittest.py index 96e2e4eef..6079632bd 100644 --- a/Tools/Scripts/webkitpy/tool/bot/layouttestresultsreader_unittest.py +++ b/Tools/Scripts/webkitpy/tool/bot/layouttestresultsreader_unittest.py @@ -91,8 +91,8 @@ class LayoutTestResultsReaderTest(unittest.TestCase): patch = tool.bugs.fetch_attachment(10001) tool.filesystem = MockFileSystem() # Should fail because the results_directory does not exist. - expected_stderr = "/mock-results does not exist, not archiving.\n" - archive = OutputCapture().assert_outputs(self, reader.archive, [patch], expected_stderr=expected_stderr) + expected_logs = "/mock-results does not exist, not archiving.\n" + archive = OutputCapture().assert_outputs(self, reader.archive, [patch], expected_logs=expected_logs) self.assertEqual(archive, None) results_directory = "/mock-results" diff --git a/Tools/Scripts/webkitpy/tool/bot/queueengine.py b/Tools/Scripts/webkitpy/tool/bot/queueengine.py index 1d7535967..0d2c97820 100644 --- a/Tools/Scripts/webkitpy/tool/bot/queueengine.py +++ b/Tools/Scripts/webkitpy/tool/bot/queueengine.py @@ -27,13 +27,16 @@ # (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 logging import sys import traceback from datetime import datetime, timedelta from webkitpy.common.system.executive import ScriptError -from webkitpy.common.system.deprecated_logging import log, OutputTee +from webkitpy.common.system.deprecated_logging import OutputTee + +_log = logging.getLogger(__name__) # FIXME: This will be caught by "except Exception:" blocks, we should consider @@ -80,7 +83,7 @@ class QueueEngine: # Child processes exit with a special code to the parent queue process can detect the error was handled. @classmethod def exit_after_handled_error(cls, error): - log(error) + _log.error(error) sys.exit(cls.handled_error_code) def run(self): @@ -100,7 +103,7 @@ class QueueEngine: self._open_work_log(work_item) try: if not self._delegate.process_work_item(work_item): - log("Unable to process work item.") + _log.warning("Unable to process work item.") continue except ScriptError, e: # Use a special exit code to indicate that the error was already @@ -123,7 +126,7 @@ class QueueEngine: return 0 def _stopping(self, message): - log("\n%s" % message) + _log.info("\n%s" % message) self._delegate.stop_work_queue(message) # Be careful to shut down our OutputTee or the unit tests will be unhappy. self._ensure_work_log_closed() @@ -154,6 +157,6 @@ class QueueEngine: return "%s Sleeping until %s (%s)." % (message, wake_time.strftime(self.log_date_format), self.sleep_duration_text) def _sleep(self, message): - log(self._sleep_message(message)) + _log.info(self._sleep_message(message)) self._wakeup_event.wait(self.seconds_to_sleep) self._wakeup_event.clear() diff --git a/Tools/Scripts/webkitpy/tool/bot/queueengine_unittest.py b/Tools/Scripts/webkitpy/tool/bot/queueengine_unittest.py index 26789ef05..0ee8b5ad8 100644 --- a/Tools/Scripts/webkitpy/tool/bot/queueengine_unittest.py +++ b/Tools/Scripts/webkitpy/tool/bot/queueengine_unittest.py @@ -145,8 +145,8 @@ class QueueEngineTest(unittest.TestCase): engine = QueueEngine("test-queue", delegate, threading.Event()) if not termination_message: termination_message = "Delegate terminated queue." - expected_stderr = "\n%s\n" % termination_message - OutputCapture().assert_outputs(self, engine.run, expected_stderr=expected_stderr) + expected_logs = "\n%s\n" % termination_message + OutputCapture().assert_outputs(self, engine.run, expected_logs=expected_logs) def _test_terminating_queue(self, exception, termination_message): work_item_index = LoggingDelegate.expected_callbacks.index('process_work_item') diff --git a/Tools/Scripts/webkitpy/tool/bot/sheriff.py b/Tools/Scripts/webkitpy/tool/bot/sheriff.py index a8c928c9b..b4e95aec0 100644 --- a/Tools/Scripts/webkitpy/tool/bot/sheriff.py +++ b/Tools/Scripts/webkitpy/tool/bot/sheriff.py @@ -27,7 +27,6 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. from webkitpy.common.config import urls -from webkitpy.common.system.deprecated_logging import log from webkitpy.common.system.executive import ScriptError from webkitpy.tool.grammar import join_with_separators diff --git a/Tools/Scripts/webkitpy/tool/bot/sheriff_unittest.py b/Tools/Scripts/webkitpy/tool/bot/sheriff_unittest.py index 3ff5082f6..02fc03608 100644 --- a/Tools/Scripts/webkitpy/tool/bot/sheriff_unittest.py +++ b/Tools/Scripts/webkitpy/tool/bot/sheriff_unittest.py @@ -65,7 +65,7 @@ class SheriffTest(unittest.TestCase): sheriff.post_blame_comment_on_bug(commit_info, builders, ["mock-test-1"]) sheriff.post_blame_comment_on_bug(commit_info, builders, ["mock-test-1", "mock-test-2"]) - expected_stderr = u"""MOCK bug comment: bug_id=1234, cc=['watcher@example.com'] + expected_logs = u"""MOCK bug comment: bug_id=1234, cc=['watcher@example.com'] --- Begin comment --- http://trac.webkit.org/changeset/4321 might have broken Foo and Bar --- End comment --- @@ -86,4 +86,4 @@ mock-test-2 --- End comment --- """ - OutputCapture().assert_outputs(self, run, expected_stderr=expected_stderr) + OutputCapture().assert_outputs(self, run, expected_logs=expected_logs) diff --git a/Tools/Scripts/webkitpy/tool/commands/abstractsequencedcommand.py b/Tools/Scripts/webkitpy/tool/commands/abstractsequencedcommand.py index 5eaf249c5..0593f2cfc 100644 --- a/Tools/Scripts/webkitpy/tool/commands/abstractsequencedcommand.py +++ b/Tools/Scripts/webkitpy/tool/commands/abstractsequencedcommand.py @@ -26,11 +26,14 @@ # (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 logging + from webkitpy.common.system.executive import ScriptError -from webkitpy.common.system.deprecated_logging import log from webkitpy.tool.commands.stepsequence import StepSequence from webkitpy.tool.multicommandtool import AbstractDeclarativeCommand +_log = logging.getLogger(__name__) + class AbstractSequencedCommand(AbstractDeclarativeCommand): steps = None @@ -45,7 +48,7 @@ class AbstractSequencedCommand(AbstractDeclarativeCommand): try: state = self._prepare_state(options, args, tool) except ScriptError, e: - log(e.message_with_output()) + _log.error(e.message_with_output()) self._exit(e.exit_code or 2) self._sequence.run_and_handle_errors(tool, options, state) diff --git a/Tools/Scripts/webkitpy/tool/commands/applywatchlistlocal_unittest.py b/Tools/Scripts/webkitpy/tool/commands/applywatchlistlocal_unittest.py index 91818d1c2..e88a86fa5 100644 --- a/Tools/Scripts/webkitpy/tool/commands/applywatchlistlocal_unittest.py +++ b/Tools/Scripts/webkitpy/tool/commands/applywatchlistlocal_unittest.py @@ -33,18 +33,23 @@ from webkitpy.tool.commands.applywatchlistlocal import ApplyWatchListLocal class ApplyWatchListLocalTest(CommandsTest): def test_args_parsing(self): - expected_stderr = 'MockWatchList: determine_cc_and_messages\n' - self.assert_execute_outputs(ApplyWatchListLocal(), [''], expected_stderr=expected_stderr) + expected_logs = """MockWatchList: determine_cc_and_messages +No bug was updated because no id was given. +Result of watchlist: cc "abarth@webkit.org, eric@webkit.org, levin@chromium.org" messages "Message1. + +Message2." +""" + self.assert_execute_outputs(ApplyWatchListLocal(), [''], expected_logs=expected_logs) def test_args_parsing_with_bug(self): - expected_stderr = """MockWatchList: determine_cc_and_messages + expected_logs = """MockWatchList: determine_cc_and_messages MOCK bug comment: bug_id=50002, cc=set(['eric@webkit.org', 'levin@chromium.org', 'abarth@webkit.org']) --- Begin comment --- Message1. Message2. --- End comment ---\n\n""" - self.assert_execute_outputs(ApplyWatchListLocal(), ['50002'], expected_stderr=expected_stderr) + self.assert_execute_outputs(ApplyWatchListLocal(), ['50002'], expected_logs=expected_logs) def test_args_parsing_with_two_bugs(self): self._assertRaisesRegexp(Exception, 'Too many arguments given: 1234 5678', self.assert_execute_outputs, ApplyWatchListLocal(), ['1234', '5678']) diff --git a/Tools/Scripts/webkitpy/tool/commands/commandtest.py b/Tools/Scripts/webkitpy/tool/commands/commandtest.py index eea0a6156..65f45b58f 100644 --- a/Tools/Scripts/webkitpy/tool/commands/commandtest.py +++ b/Tools/Scripts/webkitpy/tool/commands/commandtest.py @@ -32,7 +32,7 @@ from webkitpy.tool.mocktool import MockOptions, MockTool class CommandsTest(TestCase): - def assert_execute_outputs(self, command, args=[], expected_stdout="", expected_stderr="", expected_exception=None, options=MockOptions(), tool=MockTool()): + def assert_execute_outputs(self, command, args=[], expected_stdout="", expected_stderr="", expected_exception=None, expected_logs=None, options=MockOptions(), tool=MockTool()): options.blocks = None options.cc = 'MOCK cc' options.component = 'MOCK component' @@ -45,4 +45,4 @@ class CommandsTest(TestCase): options.quiet = True options.reviewer = 'MOCK reviewer' command.bind_to_tool(tool) - OutputCapture().assert_outputs(self, command.execute, [options, args, tool], expected_stdout=expected_stdout, expected_stderr=expected_stderr, expected_exception=expected_exception) + OutputCapture().assert_outputs(self, command.execute, [options, args, tool], expected_stdout=expected_stdout, expected_stderr=expected_stderr, expected_exception=expected_exception, expected_logs=expected_logs) diff --git a/Tools/Scripts/webkitpy/tool/commands/download.py b/Tools/Scripts/webkitpy/tool/commands/download.py index b3ee1239c..bdd780d2c 100644 --- a/Tools/Scripts/webkitpy/tool/commands/download.py +++ b/Tools/Scripts/webkitpy/tool/commands/download.py @@ -27,6 +27,8 @@ # (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 logging + from webkitpy.tool import steps from webkitpy.common.checkout.changelog import ChangeLog @@ -37,7 +39,8 @@ from webkitpy.tool.commands.stepsequence import StepSequence from webkitpy.tool.comments import bug_comment_from_commit_text from webkitpy.tool.grammar import pluralize from webkitpy.tool.multicommandtool import AbstractDeclarativeCommand -from webkitpy.common.system.deprecated_logging import error, log + +_log = logging.getLogger(__name__) class Clean(AbstractSequencedCommand): @@ -164,7 +167,7 @@ class AbstractPatchProcessingCommand(AbstractDeclarativeCommand): # It's nice to print out total statistics. bugs_to_patches = self._collect_patches_by_bug(patches) - log("Processing %s from %s." % (pluralize("patch", len(patches)), pluralize("bug", len(bugs_to_patches)))) + _log.info("Processing %s from %s." % (pluralize("patch", len(patches)), pluralize("bug", len(bugs_to_patches)))) for patch in patches: self._process_patch(patch, options, args, tool) @@ -199,13 +202,13 @@ class ProcessBugsMixin(object): all_patches = [] for bug_id in args: patches = tool.bugs.fetch_bug(bug_id).reviewed_patches() - log("%s found on bug %s." % (pluralize("reviewed patch", len(patches)), bug_id)) + _log.info("%s found on bug %s." % (pluralize("reviewed patch", len(patches)), bug_id)) all_patches += patches if not all_patches: - log("No reviewed patches found, looking for unreviewed patches.") + _log.info("No reviewed patches found, looking for unreviewed patches.") for bug_id in args: patches = tool.bugs.fetch_bug(bug_id).patches() - log("%s found on bug %s." % (pluralize("patch", len(patches)), bug_id)) + _log.info("%s found on bug %s." % (pluralize("patch", len(patches)), bug_id)) all_patches += patches return all_patches @@ -217,7 +220,7 @@ class ProcessURLsMixin(object): bug_id = urls.parse_bug_id(url) if bug_id: patches = tool.bugs.fetch_bug(bug_id).patches() - log("%s found on bug %s." % (pluralize("patch", len(patches)), bug_id)) + _log.info("%s found on bug %s." % (pluralize("patch", len(patches)), bug_id)) all_patches += patches attachment_id = urls.parse_attachment_id(url) @@ -370,9 +373,9 @@ class AbstractRolloutPrepCommand(AbstractSequencedCommand): # SheriffBot because the SheriffBot just greps the output # of create-rollout for bug URLs. It should do better # parsing instead. - log("Preparing rollout for bug %s." % commit_info.bug_id()) + _log.info("Preparing rollout for bug %s." % commit_info.bug_id()) else: - log("Unable to parse bug number from diff.") + _log.info("Unable to parse bug number from diff.") return commit_info def _prepare_state(self, options, args, tool): diff --git a/Tools/Scripts/webkitpy/tool/commands/download_unittest.py b/Tools/Scripts/webkitpy/tool/commands/download_unittest.py index b71f3daaf..14bf2ce5e 100644 --- a/Tools/Scripts/webkitpy/tool/commands/download_unittest.py +++ b/Tools/Scripts/webkitpy/tool/commands/download_unittest.py @@ -43,15 +43,15 @@ class AbstractRolloutPrepCommandTest(unittest.TestCase): command.bind_to_tool(tool) output = OutputCapture() - expected_stderr = "Preparing rollout for bug 50000.\n" - commit_info = output.assert_outputs(self, command._commit_info, [1234], expected_stderr=expected_stderr) + expected_logs = "Preparing rollout for bug 50000.\n" + commit_info = output.assert_outputs(self, command._commit_info, [1234], expected_logs=expected_logs) 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) + expected_logs = "Unable to parse bug number from diff.\n" + commit_info = output.assert_outputs(self, command._commit_info, [1234], expected_logs=expected_logs) self.assertEqual(commit_info, mock_commit_info) def test_prepare_state(self): @@ -90,51 +90,71 @@ class DownloadCommandsTest(CommandsTest): return options def test_build(self): - expected_stderr = "Updating working directory\nBuilding WebKit\n" - self.assert_execute_outputs(Build(), [], options=self._default_options(), expected_stderr=expected_stderr) + expected_logs = "Updating working directory\nBuilding WebKit\n" + self.assert_execute_outputs(Build(), [], options=self._default_options(), expected_logs=expected_logs) def test_build_and_test(self): - expected_stderr = "Updating working directory\nBuilding WebKit\nRunning Python unit tests\nRunning Perl unit tests\nRunning JavaScriptCore tests\nRunning WebKit unit tests\nRunning run-webkit-tests\n" - self.assert_execute_outputs(BuildAndTest(), [], options=self._default_options(), expected_stderr=expected_stderr) + expected_logs = """Updating working directory +Building WebKit +Running Python unit tests +Running Perl unit tests +Running JavaScriptCore tests +Running WebKit unit tests +Running run-webkit-tests +""" + self.assert_execute_outputs(BuildAndTest(), [], options=self._default_options(), expected_logs=expected_logs) def test_apply_attachment(self): options = self._default_options() options.update = True options.local_commit = True - expected_stderr = "Updating working directory\nProcessing 1 patch from 1 bug.\nProcessing patch 10000 from bug 50000.\n" - self.assert_execute_outputs(ApplyAttachment(), [10000], options=options, expected_stderr=expected_stderr) + expected_logs = "Updating working directory\nProcessing 1 patch from 1 bug.\nProcessing patch 10000 from bug 50000.\n" + self.assert_execute_outputs(ApplyAttachment(), [10000], options=options, expected_logs=expected_logs) def test_apply_from_bug(self): options = self._default_options() options.update = True options.local_commit = True - expected_stderr = "Updating working directory\n0 reviewed patches found on bug 50001.\nNo reviewed patches found, looking for unreviewed patches.\n1 patch found on bug 50001.\nProcessing 1 patch from 1 bug.\nProcessing patch 10002 from bug 50001.\n" - self.assert_execute_outputs(ApplyFromBug(), [50001], options=options, expected_stderr=expected_stderr) + expected_logs = "Updating working directory\n0 reviewed patches found on bug 50001.\nNo reviewed patches found, looking for unreviewed patches.\n1 patch found on bug 50001.\nProcessing 1 patch from 1 bug.\nProcessing patch 10002 from bug 50001.\n" + self.assert_execute_outputs(ApplyFromBug(), [50001], options=options, expected_logs=expected_logs) - expected_stderr = "Updating working directory\n2 reviewed patches found on bug 50000.\nProcessing 2 patches from 1 bug.\nProcessing patch 10000 from bug 50000.\nProcessing patch 10001 from bug 50000.\n" - self.assert_execute_outputs(ApplyFromBug(), [50000], options=options, expected_stderr=expected_stderr) + expected_logs = "Updating working directory\n2 reviewed patches found on bug 50000.\nProcessing 2 patches from 1 bug.\nProcessing patch 10000 from bug 50000.\nProcessing patch 10001 from bug 50000.\n" + self.assert_execute_outputs(ApplyFromBug(), [50000], options=options, expected_logs=expected_logs) def test_apply_watch_list(self): - expected_stderr = """Processing 1 patch from 1 bug. + expected_logs = """Processing 1 patch from 1 bug. Updating working directory -MOCK run_and_throw_if_fail: ['mock-update-webkit'], cwd=/mock-checkout\nProcessing patch 10000 from bug 50000. +MOCK run_and_throw_if_fail: ['mock-update-webkit'], cwd=/mock-checkout +Processing patch 10000 from bug 50000. MockWatchList: determine_cc_and_messages +No bug was updated because no id was given. +Result of watchlist: cc "abarth@webkit.org, eric@webkit.org, levin@chromium.org" messages "Message1. + +Message2." """ - self.assert_execute_outputs(ApplyWatchList(), [10000], options=self._default_options(), expected_stderr=expected_stderr, tool=MockTool(log_executive=True)) + self.assert_execute_outputs(ApplyWatchList(), [10000], options=self._default_options(), expected_logs=expected_logs, tool=MockTool(log_executive=True)) def test_land(self): - expected_stderr = "Building WebKit\nRunning Python unit tests\nRunning Perl unit tests\nRunning JavaScriptCore tests\nRunning WebKit unit tests\nRunning run-webkit-tests\nCommitted r49824: <http://trac.webkit.org/changeset/49824>\nUpdating bug 50000\n" + expected_logs = """Building WebKit +Running Python unit tests +Running Perl unit tests +Running JavaScriptCore tests +Running WebKit unit tests +Running run-webkit-tests +Committed r49824: <http://trac.webkit.org/changeset/49824> +Updating bug 50000 +""" mock_tool = MockTool() mock_tool.scm().create_patch = Mock(return_value="Patch1\nMockPatch\n") mock_tool.checkout().modified_changelogs = Mock(return_value=[]) - self.assert_execute_outputs(Land(), [50000], options=self._default_options(), expected_stderr=expected_stderr, tool=mock_tool) + self.assert_execute_outputs(Land(), [50000], options=self._default_options(), expected_logs=expected_logs, tool=mock_tool) # Make sure we're not calling expensive calls too often. self.assertEqual(mock_tool.scm().create_patch.call_count, 0) self.assertEqual(mock_tool.checkout().modified_changelogs.call_count, 1) def test_land_cowboy(self): - expected_stderr = """MOCK run_and_throw_if_fail: ['mock-prepare-ChangeLog', '--email=MOCK email', '--merge-base=None', 'MockFile1'], cwd=/mock-checkout + expected_logs = """MOCK run_and_throw_if_fail: ['mock-prepare-ChangeLog', '--email=MOCK email', '--merge-base=None', 'MockFile1'], cwd=/mock-checkout MOCK run_and_throw_if_fail: ['mock-check-webkit-style', '--git-commit', 'MOCK git commit', '--diff-files', 'MockFile1', '--filter', '-changelog'], cwd=/mock-checkout MOCK run_command: ['ruby', '-I', '/mock-checkout/Websites/bugs.webkit.org/PrettyPatch', '/mock-checkout/Websites/bugs.webkit.org/PrettyPatch/prettify.rb'], cwd=None, input=Patch1 MOCK: user.open_url: file://... @@ -156,30 +176,38 @@ Committed r49824: <http://trac.webkit.org/changeset/49824> No bug id provided. """ mock_tool = MockTool(log_executive=True) - self.assert_execute_outputs(LandCowboy(), [50000], options=self._default_options(), expected_stderr=expected_stderr, tool=mock_tool) + self.assert_execute_outputs(LandCowboy(), [50000], options=self._default_options(), expected_logs=expected_logs, tool=mock_tool) def test_land_red_builders(self): - expected_stderr = 'Building WebKit\nRunning Python unit tests\nRunning Perl unit tests\nRunning JavaScriptCore tests\nRunning WebKit unit tests\nRunning run-webkit-tests\nCommitted r49824: <http://trac.webkit.org/changeset/49824>\nUpdating bug 50000\n' + expected_logs = """Building WebKit +Running Python unit tests +Running Perl unit tests +Running JavaScriptCore tests +Running WebKit unit tests +Running run-webkit-tests +Committed r49824: <http://trac.webkit.org/changeset/49824> +Updating bug 50000 +""" mock_tool = MockTool() mock_tool.buildbot.light_tree_on_fire() - self.assert_execute_outputs(Land(), [50000], options=self._default_options(), expected_stderr=expected_stderr, tool=mock_tool) + self.assert_execute_outputs(Land(), [50000], options=self._default_options(), expected_logs=expected_logs, tool=mock_tool) def test_check_style(self): - expected_stderr = """Processing 1 patch from 1 bug. + expected_logs = """Processing 1 patch from 1 bug. Updating working directory MOCK run_and_throw_if_fail: ['mock-update-webkit'], cwd=/mock-checkout Processing patch 10000 from bug 50000. MOCK run_and_throw_if_fail: ['mock-check-webkit-style', '--git-commit', 'MOCK git commit', '--diff-files', 'MockFile1'], cwd=/mock-checkout """ - self.assert_execute_outputs(CheckStyle(), [10000], options=self._default_options(), expected_stderr=expected_stderr, tool=MockTool(log_executive=True)) + self.assert_execute_outputs(CheckStyle(), [10000], options=self._default_options(), expected_logs=expected_logs, tool=MockTool(log_executive=True)) def test_build_attachment(self): - expected_stderr = "Processing 1 patch from 1 bug.\nUpdating working directory\nProcessing patch 10000 from bug 50000.\nBuilding WebKit\n" - self.assert_execute_outputs(BuildAttachment(), [10000], options=self._default_options(), expected_stderr=expected_stderr) + expected_logs = "Processing 1 patch from 1 bug.\nUpdating working directory\nProcessing patch 10000 from bug 50000.\nBuilding WebKit\n" + self.assert_execute_outputs(BuildAttachment(), [10000], options=self._default_options(), expected_logs=expected_logs) def test_land_attachment(self): # FIXME: This expected result is imperfect, notice how it's seeing the same patch as still there after it thought it would have cleared the flags. - expected_stderr = """Processing 1 patch from 1 bug. + expected_logs = """Processing 1 patch from 1 bug. Updating working directory Processing patch 10000 from bug 50000. Building WebKit @@ -191,11 +219,11 @@ Running run-webkit-tests Committed r49824: <http://trac.webkit.org/changeset/49824> Not closing bug 50000 as attachment 10000 has review=+. Assuming there are more patches to land from this bug. """ - self.assert_execute_outputs(LandAttachment(), [10000], options=self._default_options(), expected_stderr=expected_stderr) + self.assert_execute_outputs(LandAttachment(), [10000], options=self._default_options(), expected_logs=expected_logs) def test_land_from_bug(self): # FIXME: This expected result is imperfect, notice how it's seeing the same patch as still there after it thought it would have cleared the flags. - expected_stderr = """2 reviewed patches found on bug 50000. + expected_logs = """2 reviewed patches found on bug 50000. Processing 2 patches from 1 bug. Updating working directory Processing patch 10000 from bug 50000. @@ -218,11 +246,11 @@ Running run-webkit-tests Committed r49824: <http://trac.webkit.org/changeset/49824> Not closing bug 50000 as attachment 10000 has review=+. Assuming there are more patches to land from this bug. """ - self.assert_execute_outputs(LandFromBug(), [50000], options=self._default_options(), expected_stderr=expected_stderr) + self.assert_execute_outputs(LandFromBug(), [50000], options=self._default_options(), expected_logs=expected_logs) def test_land_from_url(self): # FIXME: This expected result is imperfect, notice how it's seeing the same patch as still there after it thought it would have cleared the flags. - expected_stderr = """2 patches found on bug 50000. + expected_logs = """2 patches found on bug 50000. Processing 2 patches from 1 bug. Updating working directory Processing patch 10000 from bug 50000. @@ -245,14 +273,14 @@ Running run-webkit-tests Committed r49824: <http://trac.webkit.org/changeset/49824> Not closing bug 50000 as attachment 10000 has review=+. Assuming there are more patches to land from this bug. """ - self.assert_execute_outputs(LandFromURL(), ["https://bugs.webkit.org/show_bug.cgi?id=50000"], options=self._default_options(), expected_stderr=expected_stderr) + self.assert_execute_outputs(LandFromURL(), ["https://bugs.webkit.org/show_bug.cgi?id=50000"], options=self._default_options(), expected_logs=expected_logs) def test_prepare_rollout(self): - expected_stderr = "Preparing rollout for bug 50000.\nUpdating working directory\n" - self.assert_execute_outputs(PrepareRollout(), [852, "Reason"], options=self._default_options(), expected_stderr=expected_stderr) + expected_logs = "Preparing rollout for bug 50000.\nUpdating working directory\n" + self.assert_execute_outputs(PrepareRollout(), [852, "Reason"], options=self._default_options(), expected_logs=expected_logs) def test_create_rollout(self): - expected_stderr = """Preparing rollout for bug 50000. + expected_logs = """Preparing rollout for bug 50000. Updating working directory MOCK create_bug bug_title: REGRESSION(r852): Reason @@ -272,11 +300,11 @@ If you would like to land the rollout faster, you can use the following command: where ATTACHMENT_ID is the ID of this attachment. -- End comment -- """ - self.assert_execute_outputs(CreateRollout(), [852, "Reason"], options=self._default_options(), expected_stderr=expected_stderr) - self.assert_execute_outputs(CreateRollout(), ["855 852 854", "Reason"], options=self._default_options(), expected_stderr=expected_stderr) + self.assert_execute_outputs(CreateRollout(), [852, "Reason"], options=self._default_options(), expected_logs=expected_logs) + self.assert_execute_outputs(CreateRollout(), ["855 852 854", "Reason"], options=self._default_options(), expected_logs=expected_logs) def test_create_rollout_resolved(self): - expected_stderr = """Preparing rollout for bug 50004. + expected_logs = """Preparing rollout for bug 50004. Updating working directory MOCK create_bug bug_title: REGRESSION(r3001): Reason @@ -297,10 +325,10 @@ If you would like to land the rollout faster, you can use the following command: where ATTACHMENT_ID is the ID of this attachment. -- End comment -- """ - self.assert_execute_outputs(CreateRollout(), [3001, "Reason"], options=self._default_options(), expected_stderr=expected_stderr) + self.assert_execute_outputs(CreateRollout(), [3001, "Reason"], options=self._default_options(), expected_logs=expected_logs) def test_rollout(self): - expected_stderr = """Preparing rollout for bug 50000. + expected_logs = """Preparing rollout for bug 50000. Updating working directory MOCK: user.open_url: file://... Was that diff correct? @@ -312,5 +340,5 @@ Reason Committed r49824: <http://trac.webkit.org/changeset/49824>' """ - self.assert_execute_outputs(Rollout(), [852, "Reason"], options=self._default_options(), expected_stderr=expected_stderr) + self.assert_execute_outputs(Rollout(), [852, "Reason"], options=self._default_options(), expected_logs=expected_logs) diff --git a/Tools/Scripts/webkitpy/tool/commands/earlywarningsystem.py b/Tools/Scripts/webkitpy/tool/commands/earlywarningsystem.py index 3ebdf390a..98a9a36ed 100644 --- a/Tools/Scripts/webkitpy/tool/commands/earlywarningsystem.py +++ b/Tools/Scripts/webkitpy/tool/commands/earlywarningsystem.py @@ -26,11 +26,11 @@ # (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 logging from optparse import make_option from webkitpy.common.config.committers import CommitterList from webkitpy.common.config.ports import DeprecatedPort -from webkitpy.common.system.deprecated_logging import error, log from webkitpy.common.system.executive import ScriptError from webkitpy.tool.bot.earlywarningsystemtask import EarlyWarningSystemTask, EarlyWarningSystemTaskDelegate from webkitpy.tool.bot.expectedfailures import ExpectedFailures @@ -39,6 +39,8 @@ from webkitpy.tool.bot.patchanalysistask import UnableToApplyPatch from webkitpy.tool.bot.queueengine import QueueEngine from webkitpy.tool.commands.queues import AbstractReviewQueue +_log = logging.getLogger(__name__) + class AbstractEarlyWarningSystem(AbstractReviewQueue, EarlyWarningSystemTaskDelegate): _build_style = "release" @@ -135,7 +137,7 @@ class AbstractEarlyWarningSystem(AbstractReviewQueue, EarlyWarningSystemTaskDele @classmethod def handle_script_error(cls, tool, state, script_error): # FIXME: Why does this not exit(1) like the superclass does? - log(script_error.message_with_output()) + _log.error(script_error.message_with_output()) class GtkEWS(AbstractEarlyWarningSystem): diff --git a/Tools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py b/Tools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py index 7feff0d62..b33129a20 100644 --- a/Tools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py +++ b/Tools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py @@ -43,7 +43,7 @@ class AbstractEarlyWarningSystemTest(QueuesTest): ews = TestEWS() ews.bind_to_tool(MockTool()) ews._options = MockOptions(port=None, confirm=False) - OutputCapture().assert_outputs(self, ews.begin_work_queue, expected_stderr=self._default_begin_work_queue_stderr(ews.name)) + OutputCapture().assert_outputs(self, ews.begin_work_queue, expected_logs=self._default_begin_work_queue_logs(ews.name)) ews._expected_failures.unexpected_failures_observed = lambda results: set(["foo.html", "bar.html"]) task = Mock() patch = ews._tool.bugs.fetch_attachment(10000) @@ -51,33 +51,31 @@ class AbstractEarlyWarningSystemTest(QueuesTest): class EarlyWarningSytemTest(QueuesTest): - def _default_expected_stderr(self, ews): + def _default_expected_logs(self, ews): string_replacemnts = { "name": ews.name, "port": ews.port_name, } - expected_stderr = { - "begin_work_queue": self._default_begin_work_queue_stderr(ews.name), - "handle_unexpected_error": "Mock error message\n", - "next_work_item": "", + expected_logs = { + "begin_work_queue": self._default_begin_work_queue_logs(ews.name), "process_work_item": "MOCK: update_status: %(name)s Pass\nMOCK: release_work_item: %(name)s 10000\n" % string_replacemnts, + "handle_unexpected_error": "Mock error message\n", "handle_script_error": "ScriptError error message\n\nMOCK output\n", } - return expected_stderr + return expected_logs def _test_builder_ews(self, ews): ews.bind_to_tool(MockTool()) options = Mock() options.port = None options.run_tests = ews._default_run_tests - self.assert_queue_outputs(ews, expected_stderr=self._default_expected_stderr(ews), options=options) + self.assert_queue_outputs(ews, expected_logs=self._default_expected_logs(ews), options=options) def _test_testing_ews(self, ews): ews.test_results = lambda: None ews.bind_to_tool(MockTool()) - expected_stderr = self._default_expected_stderr(ews) - expected_stderr["handle_script_error"] = "ScriptError error message\n\nMOCK output\n" - self.assert_queue_outputs(ews, expected_stderr=expected_stderr) + expected_logs = self._default_expected_logs(ews) + self.assert_queue_outputs(ews, expected_logs=expected_logs) def test_builder_ewses(self): self._test_builder_ews(MacEWS()) diff --git a/Tools/Scripts/webkitpy/tool/commands/openbugs.py b/Tools/Scripts/webkitpy/tool/commands/openbugs.py index 1b51c9ff6..8c55aba14 100644 --- a/Tools/Scripts/webkitpy/tool/commands/openbugs.py +++ b/Tools/Scripts/webkitpy/tool/commands/openbugs.py @@ -26,11 +26,13 @@ # (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 logging import re import sys from webkitpy.tool.multicommandtool import AbstractDeclarativeCommand -from webkitpy.common.system.deprecated_logging import log + +_log = logging.getLogger(__name__) class OpenBugs(AbstractDeclarativeCommand): @@ -58,6 +60,6 @@ class OpenBugs(AbstractDeclarativeCommand): # This won't open bugs until stdin is closed but could be made to easily. That would just make unit testing slightly harder. bug_ids = self._find_bugs_in_iterable(sys.stdin) - log("%s bugs found in input." % len(bug_ids)) + _log.info("%s bugs found in input." % len(bug_ids)) self._open_bugs(bug_ids) diff --git a/Tools/Scripts/webkitpy/tool/commands/openbugs_unittest.py b/Tools/Scripts/webkitpy/tool/commands/openbugs_unittest.py index 06dac1fd6..680e5142e 100644 --- a/Tools/Scripts/webkitpy/tool/commands/openbugs_unittest.py +++ b/Tools/Scripts/webkitpy/tool/commands/openbugs_unittest.py @@ -46,5 +46,5 @@ class OpenBugsTest(CommandsTest): self.assertEqual(openbugs._find_bugs_in_string(expectation[0]), expectation[1]) def test_args_parsing(self): - expected_stderr = "2 bugs found in input.\nMOCK: user.open_url: http://example.com/12345\nMOCK: user.open_url: http://example.com/23456\n" - self.assert_execute_outputs(OpenBugs(), ["12345\n23456"], expected_stderr=expected_stderr) + expected_logs = "2 bugs found in input.\nMOCK: user.open_url: http://example.com/12345\nMOCK: user.open_url: http://example.com/23456\n" + self.assert_execute_outputs(OpenBugs(), ["12345\n23456"], expected_logs=expected_logs) diff --git a/Tools/Scripts/webkitpy/tool/commands/perfalizer.py b/Tools/Scripts/webkitpy/tool/commands/perfalizer.py index ae9f63a65..ed0e01548 100644 --- a/Tools/Scripts/webkitpy/tool/commands/perfalizer.py +++ b/Tools/Scripts/webkitpy/tool/commands/perfalizer.py @@ -26,7 +26,8 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -from webkitpy.common.system.deprecated_logging import log +import logging + from webkitpy.tool.bot.expectedfailures import ExpectedFailures from webkitpy.tool.bot.irc_command import IRCCommand from webkitpy.tool.bot.irc_command import Help @@ -38,6 +39,8 @@ from webkitpy.tool.bot.sheriff import Sheriff from webkitpy.tool.commands.queues import AbstractQueue from webkitpy.tool.commands.stepsequence import StepSequenceErrorHandler +_log = logging.getLogger(__name__) + class PerfalizerTask(PatchAnalysisTask): def __init__(self, tool, patch, logger): @@ -204,7 +207,7 @@ class Perfalizer(AbstractQueue, StepSequenceErrorHandler): return True def handle_unexpected_error(self, failure_map, message): - log(message) + _log.error(message) # StepSequenceErrorHandler methods diff --git a/Tools/Scripts/webkitpy/tool/commands/queries.py b/Tools/Scripts/webkitpy/tool/commands/queries.py index cbdb903ee..7cc846715 100644 --- a/Tools/Scripts/webkitpy/tool/commands/queries.py +++ b/Tools/Scripts/webkitpy/tool/commands/queries.py @@ -29,6 +29,7 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. import fnmatch +import logging import re from datetime import datetime @@ -45,10 +46,11 @@ from webkitpy.common.system.crashlogs import CrashLogs from webkitpy.common.system.user import User from webkitpy.tool.grammar import pluralize from webkitpy.tool.multicommandtool import AbstractDeclarativeCommand -from webkitpy.common.system.deprecated_logging import log from webkitpy.layout_tests.models.test_expectations import TestExpectations from webkitpy.layout_tests.port import platform_options, configuration_options +_log = logging.getLogger(__name__) + class SuggestReviewers(AbstractDeclarativeCommand): name = "suggest-reviewers" @@ -82,7 +84,7 @@ class PatchesInCommitQueue(AbstractDeclarativeCommand): def execute(self, options, args, tool): patches = tool.bugs.queries.fetch_patches_from_commit_queue() - log("Patches in commit queue:") + _log.info("Patches in commit queue:") for patch in patches: print patch.url() @@ -99,13 +101,13 @@ class PatchesToCommitQueue(AbstractDeclarativeCommand): @staticmethod def _needs_commit_queue(patch): if patch.commit_queue() == "+": # If it's already cq+, ignore the patch. - log("%s already has cq=%s" % (patch.id(), patch.commit_queue())) + _log.info("%s already has cq=%s" % (patch.id(), patch.commit_queue())) return False # We only need to worry about patches from contributers who are not yet committers. committer_record = CommitterList().committer_by_email(patch.attacher_email()) if committer_record: - log("%s committer = %s" % (patch.id(), committer_record)) + _log.info("%s committer = %s" % (patch.id(), committer_record)) return not committer_record def execute(self, options, args, tool): diff --git a/Tools/Scripts/webkitpy/tool/commands/queries_unittest.py b/Tools/Scripts/webkitpy/tool/commands/queries_unittest.py index ef1e0b387..b252c0b0e 100644 --- a/Tools/Scripts/webkitpy/tool/commands/queries_unittest.py +++ b/Tools/Scripts/webkitpy/tool/commands/queries_unittest.py @@ -65,24 +65,24 @@ class MockPortFactory(object): class QueryCommandsTest(CommandsTest): def test_bugs_to_commit(self): - expected_stderr = "Warning, attachment 10001 on bug 50000 has invalid committer (non-committer@example.com)\n" - self.assert_execute_outputs(BugsToCommit(), None, "50000\n50003\n", expected_stderr) + expected_logs = "Warning, attachment 10001 on bug 50000 has invalid committer (non-committer@example.com)\n" + self.assert_execute_outputs(BugsToCommit(), None, "50000\n50003\n", expected_logs=expected_logs) def test_patches_in_commit_queue(self): expected_stdout = "http://example.com/10000\nhttp://example.com/10002\n" - expected_stderr = "Warning, attachment 10001 on bug 50000 has invalid committer (non-committer@example.com)\nPatches in commit queue:\n" - self.assert_execute_outputs(PatchesInCommitQueue(), None, expected_stdout, expected_stderr) + expected_logs = "Warning, attachment 10001 on bug 50000 has invalid committer (non-committer@example.com)\nPatches in commit queue:\n" + self.assert_execute_outputs(PatchesInCommitQueue(), None, expected_stdout, expected_logs=expected_logs) def test_patches_to_commit_queue(self): expected_stdout = "http://example.com/10003&action=edit\n" - expected_stderr = "10000 already has cq=+\n10001 already has cq=+\n10004 committer = \"Eric Seidel\" <eric@webkit.org>\n" + expected_logs = "10000 already has cq=+\n10001 already has cq=+\n10004 committer = \"Eric Seidel\" <eric@webkit.org>\n" options = Mock() options.bugs = False - self.assert_execute_outputs(PatchesToCommitQueue(), None, expected_stdout, expected_stderr, options=options) + self.assert_execute_outputs(PatchesToCommitQueue(), None, expected_stdout, expected_logs=expected_logs, options=options) expected_stdout = "http://example.com/50003\n" options.bugs = True - self.assert_execute_outputs(PatchesToCommitQueue(), None, expected_stdout, expected_stderr, options=options) + self.assert_execute_outputs(PatchesToCommitQueue(), None, expected_stdout, expected_logs=expected_logs, options=options) def test_patches_to_review(self): options = Mock() diff --git a/Tools/Scripts/webkitpy/tool/commands/queues.py b/Tools/Scripts/webkitpy/tool/commands/queues.py index 62d429d88..edfbee402 100644 --- a/Tools/Scripts/webkitpy/tool/commands/queues.py +++ b/Tools/Scripts/webkitpy/tool/commands/queues.py @@ -28,6 +28,7 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. import codecs +import logging import os import sys import time @@ -41,7 +42,6 @@ from webkitpy.common.config.committervalidator import CommitterValidator from webkitpy.common.config.ports import DeprecatedPort from webkitpy.common.net.bugzilla import Attachment from webkitpy.common.net.statusserver import StatusServer -from webkitpy.common.system.deprecated_logging import error, log from webkitpy.common.system.executive import ScriptError from webkitpy.tool.bot.botinfo import BotInfo from webkitpy.tool.bot.commitqueuetask import CommitQueueTask, CommitQueueTaskDelegate @@ -55,6 +55,8 @@ from webkitpy.tool.bot.stylequeuetask import StyleQueueTask, StyleQueueTaskDeleg from webkitpy.tool.commands.stepsequence import StepSequenceErrorHandler from webkitpy.tool.multicommandtool import Command, TryAgain +_log = logging.getLogger(__name__) + class AbstractQueue(Command, QueueEngineDelegate): watchers = [ @@ -78,7 +80,7 @@ class AbstractQueue(Command, QueueEngineDelegate): self._tool.bugs.add_cc_to_bug(bug_id, self.watchers) except Exception, e: traceback.print_exc() - log("Failed to CC watchers.") + _log.error("Failed to CC watchers.") def run_webkit_patch(self, args): webkit_patch_args = [self._tool.path()] @@ -111,12 +113,13 @@ class AbstractQueue(Command, QueueEngineDelegate): raise NotImplementedError, "subclasses must implement" def begin_work_queue(self): - log("CAUTION: %s will discard all local changes in \"%s\"" % (self.name, self._tool.scm().checkout_root)) + _log.info("CAUTION: %s will discard all local changes in \"%s\"" % (self.name, self._tool.scm().checkout_root)) if self._options.confirm: response = self._tool.user.prompt("Are you sure? Type \"yes\" to continue: ") if (response != "yes"): - error("User declined.") - log("Running WebKit %s." % self.name) + _log.error("User declined.") + sys.exit(1) + _log.info("Running WebKit %s." % self.name) self._tool.status_server.update_status(self.name, "Starting Queue") def stop_work_queue(self, reason): @@ -193,7 +196,7 @@ class FeederQueue(AbstractQueue): return None def handle_unexpected_error(self, work_item, message): - log(message) + _log.error(message) class AbstractPatchQueue(AbstractQueue): @@ -357,7 +360,7 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler, CommitQueueTaskD # Hitting this error handler should be pretty rare. It does occur, # however, when a patch no longer applies to top-of-tree in the final # land step. - log(script_error.message_with_output()) + _log.error(script_error.message_with_output()) @classmethod def handle_checkout_needs_update(cls, tool, state, options, error): @@ -405,13 +408,13 @@ class AbstractReviewQueue(AbstractPatchQueue, StepSequenceErrorHandler): raise e def handle_unexpected_error(self, patch, message): - log(message) + _log.error(message) # StepSequenceErrorHandler methods @classmethod def handle_script_error(cls, tool, state, script_error): - log(script_error.output) + _log.error(script_error.output) class StyleQueue(AbstractReviewQueue, StyleQueueTaskDelegate): diff --git a/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py b/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py index 88ab3de06..0a32f29be 100644 --- a/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py +++ b/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py @@ -130,9 +130,8 @@ class FeederQueueTest(QueuesTest): def test_feeder_queue(self): queue = TestFeederQueue() tool = MockTool(log_executive=True) - expected_stderr = { - "begin_work_queue": self._default_begin_work_queue_stderr("feeder-queue"), - "next_work_item": "", + expected_logs = { + "begin_work_queue": self._default_begin_work_queue_logs("feeder-queue"), "process_work_item": """Warning, attachment 10001 on bug 50000 has invalid committer (non-committer@example.com) Warning, attachment 10001 on bug 50000 has invalid committer (non-committer@example.com) MOCK setting flag 'commit-queue' to '-' on attachment '10001' with comment 'Rejecting attachment 10001 from commit-queue.' and additional comment 'non-committer@example.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py. @@ -147,7 +146,7 @@ MOCK: submit_to_ews: 10002 """, "handle_unexpected_error": "Mock error message\n", } - self.assert_queue_outputs(queue, tool=tool, expected_stderr=expected_stderr) + self.assert_queue_outputs(queue, tool=tool, expected_logs=expected_logs) class AbstractPatchQueueTest(CommandsTest): @@ -160,8 +159,8 @@ class AbstractPatchQueueTest(CommandsTest): self.assertEqual(queue._next_patch(), None) tool.status_server = MockStatusServer(work_items=[2, 10000, 10001]) expected_stdout = "MOCK: fetch_attachment: 2 is not a known attachment id\n" # A mock-only message to prevent us from making mistakes. - expected_stderr = "MOCK: release_work_item: None 2\n" - patch = OutputCapture().assert_outputs(self, queue._next_patch, expected_stdout=expected_stdout, expected_stderr=expected_stderr) + expected_logs = "MOCK: release_work_item: None 2\n" + patch = OutputCapture().assert_outputs(self, queue._next_patch, expected_stdout=expected_stdout, expected_logs=expected_logs) # The patch.id() == 2 is ignored because it doesn't exist. self.assertEqual(patch.id(), 10000) self.assertEqual(queue._next_patch().id(), 10001) @@ -175,13 +174,13 @@ class AbstractPatchQueueTest(CommandsTest): queue._options = Mock() queue._options.port = None patch = queue._tool.bugs.fetch_attachment(10001) - expected_stderr = """MOCK add_attachment_to_bug: bug_id=50000, description=Archive of layout-test-results from bot filename=layout-test-results.zip mimetype=None + expected_logs = """MOCK add_attachment_to_bug: bug_id=50000, description=Archive of layout-test-results from bot filename=layout-test-results.zip mimetype=None -- Begin comment -- The attached test failures were seen while running run-webkit-tests on the mock-queue. Port: MockPort Platform: MockPlatform 1.0 -- End comment -- """ - OutputCapture().assert_outputs(self, queue._upload_results_archive_for_patch, [patch, Mock()], expected_stderr=expected_stderr) + OutputCapture().assert_outputs(self, queue._upload_results_archive_for_patch, [patch, Mock()], expected_logs=expected_logs) class NeedsUpdateSequence(StepSequence): @@ -235,9 +234,8 @@ class CommitQueueTest(QueuesTest): tool = MockTool() tool.filesystem.write_text_file('/tmp/layout-test-results/full_results.json', '') # Otherwise the commit-queue will hit a KeyError trying to read the results from the MockFileSystem. tool.filesystem.write_text_file('/tmp/layout-test-results/webkit_unit_tests_output.xml', '') - expected_stderr = { - "begin_work_queue": self._default_begin_work_queue_stderr("commit-queue"), - "next_work_item": "", + expected_logs = { + "begin_work_queue": self._default_begin_work_queue_logs("commit-queue"), "process_work_item": """MOCK: update_status: commit-queue Cleaned working directory MOCK: update_status: commit-queue Updated working directory MOCK: update_status: commit-queue Applied patch @@ -248,15 +246,14 @@ MOCK: update_status: commit-queue Landed patch MOCK: update_status: commit-queue Pass MOCK: release_work_item: commit-queue 10000 """, - "handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '10000' with comment 'Rejecting attachment 10000 from commit-queue.' and additional comment 'Mock error message'\n", "handle_script_error": "ScriptError error message\n\nMOCK output\n", + "handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '10000' with comment 'Rejecting attachment 10000 from commit-queue.' and additional comment 'Mock error message'\n", } - self.assert_queue_outputs(CommitQueue(), tool=tool, expected_stderr=expected_stderr) + self.assert_queue_outputs(CommitQueue(), tool=tool, expected_logs=expected_logs) def test_commit_queue_failure(self): - expected_stderr = { - "begin_work_queue": self._default_begin_work_queue_stderr("commit-queue"), - "next_work_item": "", + expected_logs = { + "begin_work_queue": self._default_begin_work_queue_logs("commit-queue"), "process_work_item": """MOCK: update_status: commit-queue Cleaned working directory MOCK: update_status: commit-queue Updated working directory MOCK: update_status: commit-queue Patch does not apply @@ -265,8 +262,8 @@ Full output: http://dummy_url' MOCK: update_status: commit-queue Fail MOCK: release_work_item: commit-queue 10000 """, - "handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '10000' with comment 'Rejecting attachment 10000 from commit-queue.' and additional comment 'Mock error message'\n", "handle_script_error": "ScriptError error message\n\nMOCK output\n", + "handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '10000' with comment 'Rejecting attachment 10000 from commit-queue.' and additional comment 'Mock error message'\n", } queue = CommitQueue() @@ -278,12 +275,11 @@ MOCK: release_work_item: commit-queue 10000 raise ScriptError('MOCK script error') queue.run_webkit_patch = mock_run_webkit_patch - self.assert_queue_outputs(queue, expected_stderr=expected_stderr) + self.assert_queue_outputs(queue, expected_logs=expected_logs) def test_commit_queue_failure_with_failing_tests(self): - expected_stderr = { - "begin_work_queue": self._default_begin_work_queue_stderr("commit-queue"), - "next_work_item": "", + expected_logs = { + "begin_work_queue": self._default_begin_work_queue_logs("commit-queue"), "process_work_item": """MOCK: update_status: commit-queue Cleaned working directory MOCK: update_status: commit-queue Updated working directory MOCK: update_status: commit-queue Patch does not apply @@ -294,8 +290,8 @@ Full output: http://dummy_url' MOCK: update_status: commit-queue Fail MOCK: release_work_item: commit-queue 10000 """, - "handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '10000' with comment 'Rejecting attachment 10000 from commit-queue.' and additional comment 'Mock error message'\n", "handle_script_error": "ScriptError error message\n\nMOCK output\n", + "handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '10000' with comment 'Rejecting attachment 10000 from commit-queue.' and additional comment 'Mock error message'\n", } queue = CommitQueue() @@ -308,16 +304,15 @@ MOCK: release_work_item: commit-queue 10000 raise ScriptError('MOCK script error') queue.run_webkit_patch = mock_run_webkit_patch - self.assert_queue_outputs(queue, expected_stderr=expected_stderr) + self.assert_queue_outputs(queue, expected_logs=expected_logs) def test_rollout(self): tool = MockTool(log_executive=True) tool.filesystem.write_text_file('/tmp/layout-test-results/full_results.json', '') # Otherwise the commit-queue will hit a KeyError trying to read the results from the MockFileSystem. tool.filesystem.write_text_file('/tmp/layout-test-results/webkit_unit_tests_output.xml', '') tool.buildbot.light_tree_on_fire() - expected_stderr = { - "begin_work_queue": self._default_begin_work_queue_stderr("commit-queue"), - "next_work_item": "", + expected_logs = { + "begin_work_queue": self._default_begin_work_queue_logs("commit-queue"), "process_work_item": """MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'clean', '--port=%(port_name)s'], cwd=/mock-checkout MOCK: update_status: commit-queue Cleaned working directory MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'update', '--port=%(port_name)s'], cwd=/mock-checkout @@ -335,19 +330,18 @@ MOCK: update_status: commit-queue Landed patch MOCK: update_status: commit-queue Pass MOCK: release_work_item: commit-queue 10000 """ % {"port_name": CommitQueue.port_name}, - "handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '10000' with comment 'Rejecting attachment 10000 from commit-queue.' and additional comment 'Mock error message'\n", "handle_script_error": "ScriptError error message\n\nMOCK output\n", + "handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '10000' with comment 'Rejecting attachment 10000 from commit-queue.' and additional comment 'Mock error message'\n", } - self.assert_queue_outputs(CommitQueue(), tool=tool, expected_stderr=expected_stderr) + self.assert_queue_outputs(CommitQueue(), tool=tool, expected_logs=expected_logs) def test_rollout_lands(self): tool = MockTool(log_executive=True) tool.buildbot.light_tree_on_fire() rollout_patch = tool.bugs.fetch_attachment(10005) # _patch6, a rollout patch. assert(rollout_patch.is_rollout()) - expected_stderr = { - "begin_work_queue": self._default_begin_work_queue_stderr("commit-queue"), - "next_work_item": "", + expected_logs = { + "begin_work_queue": self._default_begin_work_queue_logs("commit-queue"), "process_work_item": """MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'clean', '--port=%(port_name)s'], cwd=/mock-checkout MOCK: update_status: commit-queue Cleaned working directory MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'update', '--port=%(port_name)s'], cwd=/mock-checkout @@ -361,10 +355,10 @@ MOCK: update_status: commit-queue Landed patch MOCK: update_status: commit-queue Pass MOCK: release_work_item: commit-queue 10005 """ % {"port_name": CommitQueue.port_name}, - "handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '10005' with comment 'Rejecting attachment 10005 from commit-queue.' and additional comment 'Mock error message'\n", "handle_script_error": "ScriptError error message\n\nMOCK output\n", + "handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '10005' with comment 'Rejecting attachment 10005 from commit-queue.' and additional comment 'Mock error message'\n", } - self.assert_queue_outputs(CommitQueue(), tool=tool, work_item=rollout_patch, expected_stderr=expected_stderr) + self.assert_queue_outputs(CommitQueue(), tool=tool, work_item=rollout_patch, expected_logs=expected_logs) def test_auto_retry(self): queue = CommitQueue() @@ -373,9 +367,11 @@ MOCK: release_work_item: commit-queue 10005 tool = AlwaysCommitQueueTool() sequence = NeedsUpdateSequence(None) - expected_stderr = "Commit failed because the checkout is out of date. Please update and try again.\nMOCK: update_status: commit-queue Tests passed, but commit failed (checkout out of date). Updating, then landing without building or re-running tests.\n" + expected_logs = """Commit failed because the checkout is out of date. Please update and try again. +MOCK: update_status: commit-queue Tests passed, but commit failed (checkout out of date). Updating, then landing without building or re-running tests. +""" state = {'patch': None} - OutputCapture().assert_outputs(self, sequence.run_and_handle_errors, [tool, options, state], expected_exception=TryAgain, expected_stderr=expected_stderr) + OutputCapture().assert_outputs(self, sequence.run_and_handle_errors, [tool, options, state], expected_exception=TryAgain, expected_logs=expected_logs) self.assertEqual(options.update, True) self.assertEqual(options.build, False) @@ -388,7 +384,7 @@ MOCK: release_work_item: commit-queue 10005 queue._tool.filesystem.write_text_file('/tmp/layout-test-results/webkit_unit_tests_output.xml', '') queue._options = Mock() queue._options.port = None - expected_stderr = """MOCK: update_status: commit-queue Cleaned working directory + expected_logs = """MOCK: update_status: commit-queue Cleaned working directory MOCK: update_status: commit-queue Updated working directory MOCK: update_status: commit-queue Applied patch MOCK: update_status: commit-queue ChangeLog validated @@ -397,11 +393,11 @@ MOCK: update_status: commit-queue Passed tests MOCK: update_status: commit-queue Retry MOCK: release_work_item: commit-queue 10000 """ - OutputCapture().assert_outputs(self, queue.process_work_item, [QueuesTest.mock_work_item], expected_stderr=expected_stderr) + OutputCapture().assert_outputs(self, queue.process_work_item, [QueuesTest.mock_work_item], expected_logs=expected_logs) def test_report_flaky_tests(self): queue = TestCommitQueue(MockTool()) - expected_stderr = """MOCK bug comment: bug_id=50002, cc=None + expected_logs = """MOCK bug comment: bug_id=50002, cc=None --- Begin comment --- The commit-queue just saw foo/bar.html flake (text diff) while processing attachment 10000 on bug 50000. Port: MockPort Platform: MockPlatform 1.0 @@ -414,6 +410,7 @@ The commit-queue just saw bar/baz.html flake (text diff) while processing attach Port: MockPort Platform: MockPlatform 1.0 --- End comment --- +bar/baz-diffs.txt does not exist in results archive, uploading entire archive. MOCK add_attachment_to_bug: bug_id=50002, description=Archive of layout-test-results from bot filename=layout-test-results.zip mimetype=None MOCK bug comment: bug_id=50000, cc=None --- Begin comment --- @@ -439,7 +436,7 @@ The commit-queue is continuing to process your patch. # This is intentionally missing one diffs.txt to exercise the "upload the whole zip" codepath. return ['foo/bar-diffs.txt'] - OutputCapture().assert_outputs(self, queue.report_flaky_tests, [QueuesTest.mock_work_item, test_results, MockZipFile()], expected_stderr=expected_stderr) + OutputCapture().assert_outputs(self, queue.report_flaky_tests, [QueuesTest.mock_work_item, test_results, MockZipFile()], expected_logs=expected_logs) def test_did_pass_testing_ews(self): tool = MockTool() @@ -450,9 +447,8 @@ The commit-queue is continuing to process your patch. class StyleQueueTest(QueuesTest): def test_style_queue_with_style_exception(self): - expected_stderr = { - "begin_work_queue": self._default_begin_work_queue_stderr("style-queue"), - "next_work_item": "", + expected_logs = { + "begin_work_queue": self._default_begin_work_queue_logs("style-queue"), "process_work_item": """MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'clean'], cwd=/mock-checkout MOCK: update_status: style-queue Cleaned working directory MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'update'], cwd=/mock-checkout @@ -470,12 +466,11 @@ MOCK: release_work_item: style-queue 10000 "handle_script_error": "MOCK output\n", } tool = MockTool(log_executive=True, executive_throws_when_run=set(['check-style'])) - self.assert_queue_outputs(StyleQueue(), expected_stderr=expected_stderr, tool=tool) + self.assert_queue_outputs(StyleQueue(), expected_logs=expected_logs, tool=tool) def test_style_queue_with_watch_list_exception(self): - expected_stderr = { - "begin_work_queue": self._default_begin_work_queue_stderr("style-queue"), - "next_work_item": "", + expected_logs = { + "begin_work_queue": self._default_begin_work_queue_logs("style-queue"), "process_work_item": """MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'clean'], cwd=/mock-checkout MOCK: update_status: style-queue Cleaned working directory MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'update'], cwd=/mock-checkout @@ -493,4 +488,4 @@ MOCK: release_work_item: style-queue 10000 "handle_script_error": "MOCK output\n", } tool = MockTool(log_executive=True, executive_throws_when_run=set(['apply-watchlist-local'])) - self.assert_queue_outputs(StyleQueue(), expected_stderr=expected_stderr, tool=tool) + self.assert_queue_outputs(StyleQueue(), expected_logs=expected_logs, tool=tool) diff --git a/Tools/Scripts/webkitpy/tool/commands/queuestest.py b/Tools/Scripts/webkitpy/tool/commands/queuestest.py index b99302c8d..314a64021 100644 --- a/Tools/Scripts/webkitpy/tool/commands/queuestest.py +++ b/Tools/Scripts/webkitpy/tool/commands/queuestest.py @@ -48,24 +48,33 @@ class QueuesTest(unittest.TestCase): # This is _patch1 in mocktool.py mock_work_item = MockTool().bugs.fetch_attachment(10000) - def assert_outputs(self, func, func_name, args, expected_stdout, expected_stderr, expected_exceptions): + def assert_outputs(self, func, func_name, args, expected_stdout, expected_stderr, expected_exceptions, expected_logs): exception = None if expected_exceptions and func_name in expected_exceptions: exception = expected_exceptions[func_name] + logs = None + if expected_logs and func_name in expected_logs: + logs = expected_logs[func_name] + OutputCapture().assert_outputs(self, func, args=args, expected_stdout=expected_stdout.get(func_name, ""), expected_stderr=expected_stderr.get(func_name, ""), - expected_exception=exception) + expected_exception=exception, + expected_logs=logs) def _default_begin_work_queue_stderr(self, name): + string_replacements = {"name": name} + return "MOCK: update_status: %(name)s Starting Queue\n" % string_replacements + + def _default_begin_work_queue_logs(self, name): checkout_dir = '/mock-checkout' string_replacements = {"name": name, 'checkout_dir': checkout_dir} return "CAUTION: %(name)s will discard all local changes in \"%(checkout_dir)s\"\nRunning WebKit %(name)s.\nMOCK: update_status: %(name)s Starting Queue\n" % string_replacements - def assert_queue_outputs(self, queue, args=None, work_item=None, expected_stdout=None, expected_stderr=None, expected_exceptions=None, options=None, tool=None): + def assert_queue_outputs(self, queue, args=None, work_item=None, expected_stdout=None, expected_stderr=None, expected_exceptions=None, expected_logs=None, options=None, tool=None): if not tool: tool = MockTool() # This is a hack to make it easy for callers to not have to setup a custom MockFileSystem just to test the commit-queue @@ -86,13 +95,13 @@ class QueuesTest(unittest.TestCase): queue.execute(options, args, tool, engine=MockQueueEngine) - self.assert_outputs(queue.queue_log_path, "queue_log_path", [], expected_stdout, expected_stderr, expected_exceptions) - self.assert_outputs(queue.work_item_log_path, "work_item_log_path", [work_item], expected_stdout, expected_stderr, expected_exceptions) - self.assert_outputs(queue.begin_work_queue, "begin_work_queue", [], expected_stdout, expected_stderr, expected_exceptions) - self.assert_outputs(queue.should_continue_work_queue, "should_continue_work_queue", [], expected_stdout, expected_stderr, expected_exceptions) - self.assert_outputs(queue.next_work_item, "next_work_item", [], expected_stdout, expected_stderr, expected_exceptions) - self.assert_outputs(queue.process_work_item, "process_work_item", [work_item], expected_stdout, expected_stderr, expected_exceptions) - self.assert_outputs(queue.handle_unexpected_error, "handle_unexpected_error", [work_item, "Mock error message"], expected_stdout, expected_stderr, expected_exceptions) + self.assert_outputs(queue.queue_log_path, "queue_log_path", [], expected_stdout, expected_stderr, expected_exceptions, expected_logs) + self.assert_outputs(queue.work_item_log_path, "work_item_log_path", [work_item], expected_stdout, expected_stderr, expected_exceptions, expected_logs) + self.assert_outputs(queue.begin_work_queue, "begin_work_queue", [], expected_stdout, expected_stderr, expected_exceptions, expected_logs) + self.assert_outputs(queue.should_continue_work_queue, "should_continue_work_queue", [], expected_stdout, expected_stderr, expected_exceptions, expected_logs) + self.assert_outputs(queue.next_work_item, "next_work_item", [], expected_stdout, expected_stderr, expected_exceptions, expected_logs) + self.assert_outputs(queue.process_work_item, "process_work_item", [work_item], expected_stdout, expected_stderr, expected_exceptions, expected_logs) + self.assert_outputs(queue.handle_unexpected_error, "handle_unexpected_error", [work_item, "Mock error message"], expected_stdout, expected_stderr, expected_exceptions, expected_logs) # 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": self.mock_work_item}, ScriptError(message="ScriptError error message", script_args="MockErrorCommand", output="MOCK output")], 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", output="MOCK output")], expected_stdout, expected_stderr, expected_exceptions, expected_logs) diff --git a/Tools/Scripts/webkitpy/tool/commands/roll_unittest.py b/Tools/Scripts/webkitpy/tool/commands/roll_unittest.py index 237a1c94f..1dae497bc 100644 --- a/Tools/Scripts/webkitpy/tool/commands/roll_unittest.py +++ b/Tools/Scripts/webkitpy/tool/commands/roll_unittest.py @@ -34,20 +34,20 @@ from webkitpy.tool.mocktool import MockOptions, MockTool class RollCommandsTest(CommandsTest): def test_update_chromium_deps(self): - expected_stderr = """Updating Chromium DEPS to 6764 + expected_logs = """Updating Chromium DEPS to 6764 MOCK: MockDEPS.write_variable(chromium_rev, 6764) MOCK: user.open_url: file://... Was that diff correct? Committed r49824: <http://trac.webkit.org/changeset/49824> """ - self.assert_execute_outputs(RollChromiumDEPS(), [6764], expected_stderr=expected_stderr) + self.assert_execute_outputs(RollChromiumDEPS(), [6764], expected_logs=expected_logs) def test_update_chromium_deps_older_revision(self): options = MockOptions(non_interactive=False) - expected_stderr = """Current Chromium DEPS revision 6564 is newer than 5764. -ERROR: Unable to update Chromium DEPS + expected_logs = """Current Chromium DEPS revision 6564 is newer than 5764. +Unable to update Chromium DEPS """ - self.assert_execute_outputs(RollChromiumDEPS(), [5764], options=options, expected_stderr=expected_stderr, expected_exception=SystemExit) + self.assert_execute_outputs(RollChromiumDEPS(), [5764], options=options, expected_logs=expected_logs, expected_exception=SystemExit) class PostRollCommandsTest(CommandsTest): diff --git a/Tools/Scripts/webkitpy/tool/commands/sheriffbot.py b/Tools/Scripts/webkitpy/tool/commands/sheriffbot.py index d30da395b..0f91be3ef 100644 --- a/Tools/Scripts/webkitpy/tool/commands/sheriffbot.py +++ b/Tools/Scripts/webkitpy/tool/commands/sheriffbot.py @@ -26,13 +26,16 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -from webkitpy.common.system.deprecated_logging import log +import logging + from webkitpy.tool.bot.sheriff import Sheriff from webkitpy.tool.bot.irc_command import commands as irc_commands from webkitpy.tool.bot.ircbot import IRCBot from webkitpy.tool.commands.queues import AbstractQueue from webkitpy.tool.commands.stepsequence import StepSequenceErrorHandler +_log = logging.getLogger(__name__) + class SheriffBot(AbstractQueue, StepSequenceErrorHandler): name = "sheriff-bot" @@ -63,7 +66,7 @@ class SheriffBot(AbstractQueue, StepSequenceErrorHandler): return True def handle_unexpected_error(self, failure_map, message): - log(message) + _log.error(message) # StepSequenceErrorHandler methods diff --git a/Tools/Scripts/webkitpy/tool/commands/stepsequence.py b/Tools/Scripts/webkitpy/tool/commands/stepsequence.py index b66655446..1668cdb63 100644 --- a/Tools/Scripts/webkitpy/tool/commands/stepsequence.py +++ b/Tools/Scripts/webkitpy/tool/commands/stepsequence.py @@ -26,13 +26,16 @@ # (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 logging + from webkitpy.tool import steps from webkitpy.common.checkout.scm import CheckoutNeedsUpdate -from webkitpy.common.system.deprecated_logging import log from webkitpy.common.system.executive import ScriptError from webkitpy.tool.bot.queueengine import QueueEngine +_log = logging.getLogger(__name__) + class StepSequenceErrorHandler(): @classmethod @@ -69,14 +72,14 @@ class StepSequence(object): try: self._run(tool, options, state) except CheckoutNeedsUpdate, e: - log("Commit failed because the checkout is out of date. Please update and try again.") + _log.info("Commit failed because the checkout is out of date. Please update and try again.") if options.parent_command: command = tool.command_by_name(options.parent_command) command.handle_checkout_needs_update(tool, state, options, e) QueueEngine.exit_after_handled_error(e) except ScriptError, e: if not options.quiet: - log(e.message_with_output()) + _log.error(e.message_with_output()) if options.parent_command: command = tool.command_by_name(options.parent_command) command.handle_script_error(tool, state, e) diff --git a/Tools/Scripts/webkitpy/tool/commands/upload.py b/Tools/Scripts/webkitpy/tool/commands/upload.py index 6b52e6c83..5cd0de9e0 100644 --- a/Tools/Scripts/webkitpy/tool/commands/upload.py +++ b/Tools/Scripts/webkitpy/tool/commands/upload.py @@ -28,6 +28,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 logging import os import re import sys @@ -38,7 +39,6 @@ from webkitpy.tool import steps from webkitpy.common.checkout.changelog import parse_bug_id_from_changelog from webkitpy.common.config.committers import CommitterList -from webkitpy.common.system.deprecated_logging import error, log from webkitpy.common.system.user import User from webkitpy.thirdparty.mock import Mock from webkitpy.tool.commands.abstractsequencedcommand import AbstractSequencedCommand @@ -46,6 +46,8 @@ from webkitpy.tool.comments import bug_comment_from_svn_revision from webkitpy.tool.grammar import pluralize, join_with_separators from webkitpy.tool.multicommandtool import AbstractDeclarativeCommand +_log = logging.getLogger(__name__) + class CommitMessageForCurrentDiff(AbstractDeclarativeCommand): name = "commit-message" @@ -133,24 +135,24 @@ class AssignToCommitter(AbstractDeclarativeCommand): bug = self._tool.bugs.fetch_bug(bug_id) if not bug.is_unassigned(): assigned_to_email = bug.assigned_to_email() - log(u"Bug %s is already assigned to %s (%s)." % (bug_id, assigned_to_email, committers.committer_by_email(assigned_to_email))) + _log.info(u"Bug %s is already assigned to %s (%s)." % (bug_id, assigned_to_email, committers.committer_by_email(assigned_to_email))) return reviewed_patches = bug.reviewed_patches() if not reviewed_patches: - log("Bug %s has no non-obsolete patches, ignoring." % bug_id) + _log.info("Bug %s has no non-obsolete patches, ignoring." % bug_id) return # We only need to do anything with this bug if one of the r+'d patches does not have a valid committer (cq+ set). if self._patches_have_commiters(reviewed_patches): - log("All reviewed patches on bug %s already have commit-queue+, ignoring." % bug_id) + _log.info("All reviewed patches on bug %s already have commit-queue+, ignoring." % bug_id) return latest_patch = reviewed_patches[-1] attacher_email = latest_patch.attacher_email() committer = committers.committer_by_email(attacher_email) if not committer: - log("Attacher %s is not a committer. Bug %s likely needs commit-queue+." % (attacher_email, bug_id)) + _log.info("Attacher %s is not a committer. Bug %s likely needs commit-queue+." % (attacher_email, bug_id)) return reassign_message = u"Attachment %s was posted by a committer and has review+, assigning to %s for commit." % (latest_patch.id(), committer.full_name) @@ -202,7 +204,8 @@ class AbstractPatchUploadingCommand(AbstractSequencedCommand): state = {} state["bug_id"] = self._bug_id(options, args, tool, state) if not state["bug_id"]: - error("No bug id passed and no bug url found in ChangeLogs.") + _log.error("No bug id passed and no bug url found in ChangeLogs.") + sys.exit(1) return state @@ -322,7 +325,8 @@ class PostCommits(AbstractDeclarativeCommand): def execute(self, options, args, tool): commit_ids = tool.scm().commit_ids_from_commitish_arguments(args) if len(commit_ids) > 10: # We could lower this limit, 10 is too many for one bug as-is. - error("webkit-patch does not support attaching %s at once. Are you sure you passed the right commit range?" % (pluralize("patch", len(commit_ids)))) + _log.error("webkit-patch does not support attaching %s at once. Are you sure you passed the right commit range?" % (pluralize("patch", len(commit_ids)))) + sys.exit(1) have_obsoleted_patches = set() for commit_id in commit_ids: @@ -331,7 +335,7 @@ class PostCommits(AbstractDeclarativeCommand): # Prefer --bug-id=, then a bug url in the commit message, then a bug url in the entire commit diff (i.e. ChangeLogs). bug_id = options.bug_id or parse_bug_id_from_changelog(commit_message.message()) or parse_bug_id_from_changelog(tool.scm().create_patch(git_commit=commit_id)) if not bug_id: - log("Skipping %s: No bug id found in commit or specified with --bug-id." % commit_id) + _log.info("Skipping %s: No bug id found in commit or specified with --bug-id." % commit_id) continue if options.obsolete_patches and bug_id not in have_obsoleted_patches: @@ -382,8 +386,9 @@ class MarkBugFixed(AbstractDeclarativeCommand): not_found.append("bug id") if not svn_revision: not_found.append("svn revision") - error("Could not find %s on command-line or in %s." + _log.error("Could not find %s on command-line or in %s." % (" or ".join(not_found), "r%s" % svn_revision if svn_revision else "last commit")) + sys.exit(1) return (bug_id, svn_revision) @@ -395,15 +400,16 @@ class MarkBugFixed(AbstractDeclarativeCommand): if re.match("^r[0-9]+$", svn_revision, re.IGNORECASE): svn_revision = svn_revision[1:] if not re.match("^[0-9]+$", svn_revision): - error("Invalid svn revision: '%s'" % svn_revision) + _log.error("Invalid svn revision: '%s'" % svn_revision) + sys.exit(1) needs_prompt = False if not bug_id or not svn_revision: needs_prompt = True (bug_id, svn_revision) = self._determine_bug_id_and_svn_revision(tool, bug_id, svn_revision) - log("Bug: <%s> %s" % (tool.bugs.bug_url_for_bug_id(bug_id), tool.bugs.fetch_bug_dictionary(bug_id)["title"])) - log("Revision: %s" % svn_revision) + _log.info("Bug: <%s> %s" % (tool.bugs.bug_url_for_bug_id(bug_id), tool.bugs.fetch_bug_dictionary(bug_id)["title"])) + _log.info("Revision: %s" % svn_revision) if options.open_bug: tool.user.open_url(tool.bugs.bug_url_for_bug_id(bug_id)) @@ -417,10 +423,10 @@ class MarkBugFixed(AbstractDeclarativeCommand): bug_comment = "%s\n\n%s" % (options.comment, bug_comment) if options.update_only: - log("Adding comment to Bug %s." % bug_id) + _log.info("Adding comment to Bug %s." % bug_id) tool.bugs.post_comment_to_bug(bug_id, bug_comment) else: - log("Adding comment to Bug %s and marking as Resolved/Fixed." % bug_id) + _log.info("Adding comment to Bug %s and marking as Resolved/Fixed." % bug_id) tool.bugs.close_bug_as_fixed(bug_id, bug_comment) @@ -443,7 +449,8 @@ class CreateBug(AbstractDeclarativeCommand): def create_bug_from_commit(self, options, args, tool): commit_ids = tool.scm().commit_ids_from_commitish_arguments(args) if len(commit_ids) > 3: - error("Are you sure you want to create one bug with %s patches?" % len(commit_ids)) + _log.error("Are you sure you want to create one bug with %s patches?" % len(commit_ids)) + sys.exit(1) commit_id = commit_ids[0] @@ -499,7 +506,8 @@ class CreateBug(AbstractDeclarativeCommand): def execute(self, options, args, tool): if len(args): if (not tool.scm().supports_local_commits()): - error("Extra arguments not supported; patch is taken from working directory.") + _log.error("Extra arguments not supported; patch is taken from working directory.") + sys.exit(1) self.create_bug_from_commit(options, args, tool) else: self.create_bug_from_patch(options, args, tool) diff --git a/Tools/Scripts/webkitpy/tool/commands/upload_unittest.py b/Tools/Scripts/webkitpy/tool/commands/upload_unittest.py index 185bb97f3..ad1b591e0 100644 --- a/Tools/Scripts/webkitpy/tool/commands/upload_unittest.py +++ b/Tools/Scripts/webkitpy/tool/commands/upload_unittest.py @@ -42,7 +42,7 @@ class UploadCommandsTest(CommandsTest): def test_assign_to_committer(self): tool = MockTool() - expected_stderr = """Warning, attachment 10001 on bug 50000 has invalid committer (non-committer@example.com) + expected_logs = """Warning, attachment 10001 on bug 50000 has invalid committer (non-committer@example.com) MOCK reassign_bug: bug_id=50000, assignee=eric@webkit.org -- Begin comment -- Attachment 10001 was posted by a committer and has review+, assigning to Eric Seidel for commit. @@ -50,11 +50,11 @@ Attachment 10001 was posted by a committer and has review+, assigning to Eric Se Bug 50003 is already assigned to foo@foo.com (None). Bug 50002 has no non-obsolete patches, ignoring. """ - self.assert_execute_outputs(AssignToCommitter(), [], expected_stderr=expected_stderr, tool=tool) + self.assert_execute_outputs(AssignToCommitter(), [], expected_logs=expected_logs, tool=tool) def test_obsolete_attachments(self): - expected_stderr = "Obsoleting 2 old patches on bug 50000\n" - self.assert_execute_outputs(ObsoleteAttachments(), [50000], expected_stderr=expected_stderr) + expected_logs = "Obsoleting 2 old patches on bug 50000\n" + self.assert_execute_outputs(ObsoleteAttachments(), [50000], expected_logs=expected_logs) def test_post(self): options = MockOptions() @@ -66,44 +66,46 @@ Bug 50002 has no non-obsolete patches, ignoring. options.request_commit = False options.review = True options.suggest_reviewers = False - expected_stderr = """MOCK: user.open_url: file://... + expected_logs = """MOCK: user.open_url: file://... Was that diff correct? Obsoleting 2 old patches on bug 50000 MOCK reassign_bug: bug_id=50000, assignee=None MOCK add_patch_to_bug: bug_id=50000, description=MOCK description, mark_for_review=True, mark_for_commit_queue=False, mark_for_landing=False MOCK: user.open_url: http://example.com/50000 """ - self.assert_execute_outputs(Post(), [50000], options=options, expected_stderr=expected_stderr) + self.assert_execute_outputs(Post(), [50000], options=options, expected_logs=expected_logs) def test_attach_to_bug(self): options = MockOptions() options.comment = "extra comment" options.description = "file description" - expected_stderr = """MOCK add_attachment_to_bug: bug_id=50000, description=file description filename=None mimetype=None + expected_logs = """MOCK add_attachment_to_bug: bug_id=50000, description=file description filename=None mimetype=None -- Begin comment -- extra comment -- End comment -- """ - self.assert_execute_outputs(AttachToBug(), [50000, "path/to/file.txt", "file description"], options=options, expected_stderr=expected_stderr) + self.assert_execute_outputs(AttachToBug(), [50000, "path/to/file.txt", "file description"], options=options, expected_logs=expected_logs) def test_attach_to_bug_no_description_or_comment(self): options = MockOptions() options.comment = None options.description = None - expected_stderr = """MOCK add_attachment_to_bug: bug_id=50000, description=file.txt filename=None mimetype=None -""" - self.assert_execute_outputs(AttachToBug(), [50000, "path/to/file.txt"], options=options, expected_stderr=expected_stderr) + expected_logs = "MOCK add_attachment_to_bug: bug_id=50000, description=file.txt filename=None mimetype=None\n" + self.assert_execute_outputs(AttachToBug(), [50000, "path/to/file.txt"], options=options, expected_logs=expected_logs) def test_land_safely(self): - expected_stderr = "Obsoleting 2 old patches on bug 50000\nMOCK reassign_bug: bug_id=50000, assignee=None\nMOCK add_patch_to_bug: bug_id=50000, description=Patch for landing, mark_for_review=False, mark_for_commit_queue=False, mark_for_landing=True\n" - self.assert_execute_outputs(LandSafely(), [50000], expected_stderr=expected_stderr) + expected_logs = """Obsoleting 2 old patches on bug 50000 +MOCK reassign_bug: bug_id=50000, assignee=None +MOCK add_patch_to_bug: bug_id=50000, description=Patch for landing, mark_for_review=False, mark_for_commit_queue=False, mark_for_landing=True +""" + self.assert_execute_outputs(LandSafely(), [50000], expected_logs=expected_logs) def test_prepare_diff_with_arg(self): self.assert_execute_outputs(Prepare(), [50000]) def test_prepare(self): - expected_stderr = "MOCK create_bug\nbug_title: Mock user response\nbug_description: Mock user response\ncomponent: MOCK component\ncc: MOCK cc\n" - self.assert_execute_outputs(Prepare(), [], expected_stderr=expected_stderr) + expected_logs = "MOCK create_bug\nbug_title: Mock user response\nbug_description: Mock user response\ncomponent: MOCK component\ncc: MOCK cc\n" + self.assert_execute_outputs(Prepare(), [], expected_logs=expected_logs) def test_upload(self): options = MockOptions() @@ -115,14 +117,14 @@ extra comment options.request_commit = False options.review = True options.suggest_reviewers = False - expected_stderr = """MOCK: user.open_url: file://... + expected_logs = """MOCK: user.open_url: file://... Was that diff correct? Obsoleting 2 old patches on bug 50000 MOCK reassign_bug: bug_id=50000, assignee=None MOCK add_patch_to_bug: bug_id=50000, description=MOCK description, mark_for_review=True, mark_for_commit_queue=False, mark_for_landing=False MOCK: user.open_url: http://example.com/50000 """ - self.assert_execute_outputs(Upload(), [50000], options=options, expected_stderr=expected_stderr) + self.assert_execute_outputs(Upload(), [50000], options=options, expected_logs=expected_logs) def test_mark_bug_fixed(self): tool = MockTool() @@ -130,7 +132,7 @@ MOCK: user.open_url: http://example.com/50000 options = Mock() options.bug_id = 50000 options.comment = "MOCK comment" - expected_stderr = """Bug: <http://example.com/50000> Bug with two r+'d and cq+'d patches, one of which has an invalid commit-queue setter. + expected_logs = """Bug: <http://example.com/50000> Bug with two r+'d and cq+'d patches, one of which has an invalid commit-queue setter. Revision: 9876 MOCK: user.open_url: http://example.com/50000 Is this correct? @@ -143,7 +145,7 @@ Committed r9876: <http://trac.webkit.org/changeset/9876> --- End comment --- """ - self.assert_execute_outputs(MarkBugFixed(), [], expected_stderr=expected_stderr, tool=tool, options=options) + self.assert_execute_outputs(MarkBugFixed(), [], expected_logs=expected_logs, tool=tool, options=options) def test_edit_changelog(self): self.assert_execute_outputs(EditChangeLogs(), []) diff --git a/Tools/Scripts/webkitpy/tool/multicommandtool.py b/Tools/Scripts/webkitpy/tool/multicommandtool.py index 38c410cf8..e2f91a7da 100644 --- a/Tools/Scripts/webkitpy/tool/multicommandtool.py +++ b/Tools/Scripts/webkitpy/tool/multicommandtool.py @@ -31,12 +31,14 @@ # which are called with the following format: # tool-name [global options] command-name [command options] +import logging import sys from optparse import OptionParser, IndentedHelpFormatter, SUPPRESS_USAGE, make_option from webkitpy.tool.grammar import pluralize -from webkitpy.common.system.deprecated_logging import log + +_log = logging.getLogger(__name__) class TryAgain(Exception): @@ -109,13 +111,13 @@ class Command(object): def check_arguments_and_execute(self, options, args, tool=None): if len(args) < len(self.required_arguments): - log("%s required, %s provided. Provided: %s Required: %s\nSee '%s help %s' for usage." % ( - pluralize("argument", len(self.required_arguments)), - pluralize("argument", len(args)), - "'%s'" % " ".join(args), - " ".join(self.required_arguments), - tool.name(), - self.name)) + _log.error("%s required, %s provided. Provided: %s Required: %s\nSee '%s help %s' for usage." % ( + pluralize("argument", len(self.required_arguments)), + pluralize("argument", len(args)), + "'%s'" % " ".join(args), + " ".join(self.required_arguments), + tool.name(), + self.name)) return 1 return self.execute(options, args, tool) or 0 @@ -303,7 +305,7 @@ class MultiCommandTool(object): (should_execute, failure_reason) = self.should_execute_command(command) if not should_execute: - log(failure_reason) + _log.error(failure_reason) return 0 # FIXME: Should this really be 0? while True: diff --git a/Tools/Scripts/webkitpy/tool/multicommandtool_unittest.py b/Tools/Scripts/webkitpy/tool/multicommandtool_unittest.py index c19095c3e..ecb1df007 100644 --- a/Tools/Scripts/webkitpy/tool/multicommandtool_unittest.py +++ b/Tools/Scripts/webkitpy/tool/multicommandtool_unittest.py @@ -81,8 +81,8 @@ class CommandTest(unittest.TestCase): def test_required_arguments(self): two_required_arguments = TrivialCommand(argument_names="ARG1 ARG2 [ARG3]") - expected_missing_args_error = "2 arguments required, 1 argument provided. Provided: 'foo' Required: ARG1 ARG2\nSee 'trivial-tool help trivial' for usage.\n" - exit_code = OutputCapture().assert_outputs(self, two_required_arguments.check_arguments_and_execute, [None, ["foo"], TrivialTool()], expected_stderr=expected_missing_args_error) + expected_logs = "2 arguments required, 1 argument provided. Provided: 'foo' Required: ARG1 ARG2\nSee 'trivial-tool help trivial' for usage.\n" + exit_code = OutputCapture().assert_outputs(self, two_required_arguments.check_arguments_and_execute, [None, ["foo"], TrivialTool()], expected_logs=expected_logs) self.assertEqual(exit_code, 1) diff --git a/Tools/Scripts/webkitpy/tool/servers/gardeningserver.py b/Tools/Scripts/webkitpy/tool/servers/gardeningserver.py index 41b81663e..77068acf4 100644 --- a/Tools/Scripts/webkitpy/tool/servers/gardeningserver.py +++ b/Tools/Scripts/webkitpy/tool/servers/gardeningserver.py @@ -54,6 +54,8 @@ class GardeningHTTPServer(SocketServer.ThreadingMixIn, BaseHTTPServer.HTTPServer class GardeningHTTPRequestHandler(ReflectionHandler): STATIC_FILE_NAMES = frozenset() + STATIC_FILE_EXTENSIONS = ('.js', '.css', '.html', '.gif', '.png') + STATIC_FILE_DIRECTORY = os.path.join( os.path.dirname(__file__), '..', diff --git a/Tools/Scripts/webkitpy/tool/servers/reflectionhandler.py b/Tools/Scripts/webkitpy/tool/servers/reflectionhandler.py index 2148b4eb2..954fb56a9 100644 --- a/Tools/Scripts/webkitpy/tool/servers/reflectionhandler.py +++ b/Tools/Scripts/webkitpy/tool/servers/reflectionhandler.py @@ -78,7 +78,7 @@ class ReflectionHandler(BaseHTTPServer.BaseHTTPRequestHandler): self.query = {} function_or_file_name = path[1:] or "index.html" - _, extension = os.path.splitext(self.path) + _, extension = os.path.splitext(function_or_file_name) if extension in self.STATIC_FILE_EXTENSIONS: self._serve_static_file(function_or_file_name) return diff --git a/Tools/Scripts/webkitpy/tool/steps/addsvnmimetypeforpng.py b/Tools/Scripts/webkitpy/tool/steps/addsvnmimetypeforpng.py index 73bec15db..0ef0fed46 100644 --- a/Tools/Scripts/webkitpy/tool/steps/addsvnmimetypeforpng.py +++ b/Tools/Scripts/webkitpy/tool/steps/addsvnmimetypeforpng.py @@ -21,12 +21,15 @@ # (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 logging + from webkitpy.tool.steps.abstractstep import AbstractStep from webkitpy.common import checksvnconfigfile -from webkitpy.common.system.deprecated_logging import log from webkitpy.common.checkout.scm.detection import SCMDetector from webkitpy.common.system.systemhost import SystemHost +_log = logging.getLogger(__name__) + class AddSvnMimetypeForPng(AbstractStep): def __init__(self, tool, options, host=None, scm=None): @@ -47,19 +50,19 @@ class AddSvnMimetypeForPng(AbstractStep): config_file_path = checksvnconfigfile.config_file_path(self._host, self._fs) if file_missing: - log("There is no SVN config file. The svn:mime-type of pngs won't set.") + _log.info("There is no SVN config file. The svn:mime-type of pngs won't set.") if not self._tool.user.confirm("Are you sure you want to continue?", default="n"): self._exit(1) elif autoprop_missing and png_missing: - log(checksvnconfigfile.errorstr_autoprop(config_file_path) + checksvnconfigfile.errorstr_png(config_file_path)) + _log.info(checksvnconfigfile.errorstr_autoprop(config_file_path) + checksvnconfigfile.errorstr_png(config_file_path)) if not self._tool.user.confirm("Do you want to continue?", default="n"): self._exit(1) elif autoprop_missing: - log(checksvnconfigfile.errorstr_autoprop(config_file_path)) + _log.info(checksvnconfigfile.errorstr_autoprop(config_file_path)) if not self._tool.user.confirm("Do you want to continue?", default="n"): self._exit(1) elif png_missing: - log(checksvnconfigfile.errorstr_png(config_file_path)) + _log.info(checksvnconfigfile.errorstr_png(config_file_path)) if not self._tool.user.confirm("Do you want to continue?", default="n"): self._exit(1) diff --git a/Tools/Scripts/webkitpy/tool/steps/applypatch.py b/Tools/Scripts/webkitpy/tool/steps/applypatch.py index 5c36169fd..50ee1f7ed 100644 --- a/Tools/Scripts/webkitpy/tool/steps/applypatch.py +++ b/Tools/Scripts/webkitpy/tool/steps/applypatch.py @@ -26,9 +26,13 @@ # (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 logging + from webkitpy.tool.steps.abstractstep import AbstractStep from webkitpy.tool.steps.options import Options -from webkitpy.common.system.deprecated_logging import log + +_log = logging.getLogger(__name__) + class ApplyPatch(AbstractStep): @classmethod @@ -38,5 +42,5 @@ class ApplyPatch(AbstractStep): ] def run(self, state): - log("Processing patch %s from bug %s." % (state["patch"].id(), state["patch"].bug_id())) + _log.info("Processing patch %s from bug %s." % (state["patch"].id(), state["patch"].bug_id())) self._tool.checkout().apply_patch(state["patch"]) diff --git a/Tools/Scripts/webkitpy/tool/steps/applywatchlist_unittest.py b/Tools/Scripts/webkitpy/tool/steps/applywatchlist_unittest.py index bdaaf758a..a978f4164 100644 --- a/Tools/Scripts/webkitpy/tool/steps/applywatchlist_unittest.py +++ b/Tools/Scripts/webkitpy/tool/steps/applywatchlist_unittest.py @@ -41,11 +41,11 @@ class ApplyWatchListTest(unittest.TestCase): 'bug_id': '50001', 'diff': 'The diff', } - expected_stderr = """MockWatchList: determine_cc_and_messages + expected_logs = """MockWatchList: determine_cc_and_messages MOCK bug comment: bug_id=50001, cc=set(['levin@chromium.org']) --- Begin comment --- Message2. --- End comment --- """ - capture.assert_outputs(self, step.run, [state], expected_stderr=expected_stderr) + capture.assert_outputs(self, step.run, [state], expected_logs=expected_logs) diff --git a/Tools/Scripts/webkitpy/tool/steps/build.py b/Tools/Scripts/webkitpy/tool/steps/build.py index 7f7dd9f36..a2a627229 100644 --- a/Tools/Scripts/webkitpy/tool/steps/build.py +++ b/Tools/Scripts/webkitpy/tool/steps/build.py @@ -26,9 +26,12 @@ # (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 logging + from webkitpy.tool.steps.abstractstep import AbstractStep from webkitpy.tool.steps.options import Options -from webkitpy.common.system.deprecated_logging import log + +_log = logging.getLogger(__name__) class Build(AbstractStep): @@ -52,7 +55,7 @@ class Build(AbstractStep): def run(self, state): if not self._options.build: return - log("Building WebKit") + _log.info("Building WebKit") if self._options.build_style == "both": self.build("debug") self.build("release") diff --git a/Tools/Scripts/webkitpy/tool/steps/checkstyle.py b/Tools/Scripts/webkitpy/tool/steps/checkstyle.py index 3304f016f..0cb15f4c1 100644 --- a/Tools/Scripts/webkitpy/tool/steps/checkstyle.py +++ b/Tools/Scripts/webkitpy/tool/steps/checkstyle.py @@ -29,7 +29,6 @@ from webkitpy.common.system.executive import ScriptError from webkitpy.tool.steps.abstractstep import AbstractStep from webkitpy.tool.steps.options import Options -from webkitpy.common.system.deprecated_logging import error class CheckStyle(AbstractStep): @classmethod diff --git a/Tools/Scripts/webkitpy/tool/steps/closebug.py b/Tools/Scripts/webkitpy/tool/steps/closebug.py index b33e373bf..e58be5468 100644 --- a/Tools/Scripts/webkitpy/tool/steps/closebug.py +++ b/Tools/Scripts/webkitpy/tool/steps/closebug.py @@ -26,9 +26,12 @@ # (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 logging + from webkitpy.tool.steps.abstractstep import AbstractStep from webkitpy.tool.steps.options import Options -from webkitpy.common.system.deprecated_logging import log + +_log = logging.getLogger(__name__) class CloseBug(AbstractStep): @@ -48,6 +51,6 @@ class CloseBug(AbstractStep): patches = self._tool.bugs.fetch_bug(state["patch"].bug_id()).patches() for patch in patches: if patch.review() == "?" or patch.review() == "+": - log("Not closing bug %s as attachment %s has review=%s. Assuming there are more patches to land from this bug." % (patch.bug_id(), patch.id(), patch.review())) + _log.info("Not closing bug %s as attachment %s has review=%s. Assuming there are more patches to land from this bug." % (patch.bug_id(), patch.id(), patch.review())) return self._tool.bugs.close_bug_as_fixed(state["patch"].bug_id(), "All reviewed patches have been landed. Closing bug.") diff --git a/Tools/Scripts/webkitpy/tool/steps/closebugforlanddiff.py b/Tools/Scripts/webkitpy/tool/steps/closebugforlanddiff.py index e5a68dbf1..1662d6a5d 100644 --- a/Tools/Scripts/webkitpy/tool/steps/closebugforlanddiff.py +++ b/Tools/Scripts/webkitpy/tool/steps/closebugforlanddiff.py @@ -26,10 +26,13 @@ # (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 logging + from webkitpy.tool.comments import bug_comment_from_commit_text from webkitpy.tool.steps.abstractstep import AbstractStep from webkitpy.tool.steps.options import Options -from webkitpy.common.system.deprecated_logging import log + +_log = logging.getLogger(__name__) class CloseBugForLandDiff(AbstractStep): @@ -46,7 +49,7 @@ class CloseBugForLandDiff(AbstractStep): bug_id = state.get("patch").bug_id() if bug_id: - log("Updating bug %s" % bug_id) + _log.info("Updating bug %s" % bug_id) if self._options.close_bug: self._tool.bugs.close_bug_as_fixed(bug_id, comment_text) else: @@ -54,5 +57,5 @@ class CloseBugForLandDiff(AbstractStep): # to the bug, and if so obsolete it. self._tool.bugs.post_comment_to_bug(bug_id, comment_text) else: - log(comment_text) - log("No bug id provided.") + _log.info(comment_text) + _log.info("No bug id provided.") diff --git a/Tools/Scripts/webkitpy/tool/steps/closebugforlanddiff_unittest.py b/Tools/Scripts/webkitpy/tool/steps/closebugforlanddiff_unittest.py index 0a56564dd..6969c4e9a 100644 --- a/Tools/Scripts/webkitpy/tool/steps/closebugforlanddiff_unittest.py +++ b/Tools/Scripts/webkitpy/tool/steps/closebugforlanddiff_unittest.py @@ -36,5 +36,5 @@ class CloseBugForLandDiffTest(unittest.TestCase): def test_empty_state(self): capture = OutputCapture() step = CloseBugForLandDiff(MockTool(), MockOptions()) - expected_stderr = "Committed r49824: <http://trac.webkit.org/changeset/49824>\nNo bug id provided.\n" - capture.assert_outputs(self, step.run, [{"commit_text" : "Mock commit text"}], expected_stderr=expected_stderr) + expected_logs = "Committed r49824: <http://trac.webkit.org/changeset/49824>\nNo bug id provided.\n" + capture.assert_outputs(self, step.run, [{"commit_text": "Mock commit text"}], expected_logs=expected_logs) diff --git a/Tools/Scripts/webkitpy/tool/steps/commit.py b/Tools/Scripts/webkitpy/tool/steps/commit.py index 0e5ca9157..2bffa4c2a 100644 --- a/Tools/Scripts/webkitpy/tool/steps/commit.py +++ b/Tools/Scripts/webkitpy/tool/steps/commit.py @@ -26,16 +26,18 @@ # (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 logging import sys from webkitpy.common.checkout.scm import AuthenticationError, AmbiguousCommitError from webkitpy.common.config import urls -from webkitpy.common.system.deprecated_logging import log from webkitpy.common.system.executive import ScriptError from webkitpy.common.system.user import User from webkitpy.tool.steps.abstractstep import AbstractStep from webkitpy.tool.steps.options import Options +_log = logging.getLogger(__name__) + class Commit(AbstractStep): # FIXME: This option exists only to make sure we don't break scripts which include --ignore-builders @@ -89,7 +91,7 @@ class Commit(AbstractStep): scm = self._tool.scm() commit_text = scm.commit_with_message(self._commit_message, git_commit=self._options.git_commit, username=username, password=password, force_squash=force_squash, changed_files=self._changed_files(state)) svn_revision = scm.svn_revision_from_commit_text(commit_text) - log("Committed r%s: <%s>" % (svn_revision, urls.view_revision_url(svn_revision))) + _log.info("Committed r%s: <%s>" % (svn_revision, urls.view_revision_url(svn_revision))) self._state["commit_text"] = commit_text break; except AmbiguousCommitError, e: diff --git a/Tools/Scripts/webkitpy/tool/steps/commit_unittest.py b/Tools/Scripts/webkitpy/tool/steps/commit_unittest.py index 25d9b61a1..936e3ebab 100644 --- a/Tools/Scripts/webkitpy/tool/steps/commit_unittest.py +++ b/Tools/Scripts/webkitpy/tool/steps/commit_unittest.py @@ -50,13 +50,16 @@ class CommitTest(unittest.TestCase): } tool.executive = MockExecutive(should_log=True, should_throw_when_run=False) - capture.assert_outputs(self, step.run, [state], expected_stderr="Committed r49824: <http://trac.webkit.org/changeset/49824>\n") + expected_logs = "Committed r49824: <http://trac.webkit.org/changeset/49824>\n" + capture.assert_outputs(self, step.run, [state], expected_logs=expected_logs) state = { "changed_files": ["platform/chromium/" + filename], } - capture.assert_outputs(self, step.run, [state], expected_stderr="MOCK run_and_throw_if_fail: ['mock-check-webkit-style', '--diff-files', 'platform/chromium/" - + filename + "'], cwd=/mock-checkout\nCommitted r49824: <http://trac.webkit.org/changeset/49824>\n") + expected_logs = """MOCK run_and_throw_if_fail: ['mock-check-webkit-style', '--diff-files', 'platform/chromium/%s'], cwd=/mock-checkout +Committed r49824: <http://trac.webkit.org/changeset/49824> +""" % filename + capture.assert_outputs(self, step.run, [state], expected_logs=expected_logs) tool.executive = MockExecutive(should_log=True, should_throw_when_run=set(["platform/chromium/" + filename])) self.assertRaises(ScriptError, capture.assert_outputs, self, step.run, [state]) diff --git a/Tools/Scripts/webkitpy/tool/steps/ensurelocalcommitifneeded.py b/Tools/Scripts/webkitpy/tool/steps/ensurelocalcommitifneeded.py index 2167351e2..778d7ae56 100644 --- a/Tools/Scripts/webkitpy/tool/steps/ensurelocalcommitifneeded.py +++ b/Tools/Scripts/webkitpy/tool/steps/ensurelocalcommitifneeded.py @@ -26,9 +26,13 @@ # (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 logging +import sys + from webkitpy.tool.steps.abstractstep import AbstractStep from webkitpy.tool.steps.options import Options -from webkitpy.common.system.deprecated_logging import error + +_log = logging.getLogger(__name__) class EnsureLocalCommitIfNeeded(AbstractStep): @@ -40,4 +44,5 @@ class EnsureLocalCommitIfNeeded(AbstractStep): def run(self, state): if self._options.local_commit and not self._tool.scm().supports_local_commits(): - error("--local-commit passed, but %s does not support local commits" % self._tool.scm().display_name()) + _log.error("--local-commit passed, but %s does not support local commits" % self._tool.scm().display_name()) + sys.exit(1) diff --git a/Tools/Scripts/webkitpy/tool/steps/obsoletepatches.py b/Tools/Scripts/webkitpy/tool/steps/obsoletepatches.py index de508c6cc..dfed959cb 100644 --- a/Tools/Scripts/webkitpy/tool/steps/obsoletepatches.py +++ b/Tools/Scripts/webkitpy/tool/steps/obsoletepatches.py @@ -26,10 +26,13 @@ # (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 logging + from webkitpy.tool.grammar import pluralize from webkitpy.tool.steps.abstractstep import AbstractStep from webkitpy.tool.steps.options import Options -from webkitpy.common.system.deprecated_logging import log + +_log = logging.getLogger(__name__) class ObsoletePatches(AbstractStep): @@ -46,6 +49,6 @@ class ObsoletePatches(AbstractStep): patches = self._tool.bugs.fetch_bug(bug_id).patches() if not patches: return - log("Obsoleting %s on bug %s" % (pluralize("old patch", len(patches)), bug_id)) + _log.info("Obsoleting %s on bug %s" % (pluralize("old patch", len(patches)), bug_id)) for patch in patches: self._tool.bugs.obsolete_attachment(patch.id()) diff --git a/Tools/Scripts/webkitpy/tool/steps/preparechangelog.py b/Tools/Scripts/webkitpy/tool/steps/preparechangelog.py index 19caace01..4d80ab61f 100644 --- a/Tools/Scripts/webkitpy/tool/steps/preparechangelog.py +++ b/Tools/Scripts/webkitpy/tool/steps/preparechangelog.py @@ -26,11 +26,15 @@ # (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 logging +import sys + from webkitpy.common.checkout.changelog import ChangeLog from webkitpy.common.system.executive import ScriptError from webkitpy.tool.steps.abstractstep import AbstractStep from webkitpy.tool.steps.options import Options -from webkitpy.common.system.deprecated_logging import error + +_log = logging.getLogger(__name__) class PrepareChangeLog(AbstractStep): @@ -73,5 +77,6 @@ class PrepareChangeLog(AbstractStep): try: self._tool.executive.run_and_throw_if_fail(args, self._options.quiet, cwd=self._tool.scm().checkout_root) except ScriptError, e: - error("Unable to prepare ChangeLogs.") + _log.error("Unable to prepare ChangeLogs.") + sys.exit(1) self.did_modify_checkout(state) diff --git a/Tools/Scripts/webkitpy/tool/steps/reopenbugafterrollout.py b/Tools/Scripts/webkitpy/tool/steps/reopenbugafterrollout.py index f369ca925..39388f62e 100644 --- a/Tools/Scripts/webkitpy/tool/steps/reopenbugafterrollout.py +++ b/Tools/Scripts/webkitpy/tool/steps/reopenbugafterrollout.py @@ -26,9 +26,12 @@ # (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 logging + from webkitpy.tool.comments import bug_comment_from_commit_text from webkitpy.tool.steps.abstractstep import AbstractStep -from webkitpy.common.system.deprecated_logging import log + +_log = logging.getLogger(__name__) class ReopenBugAfterRollout(AbstractStep): @@ -38,7 +41,7 @@ class ReopenBugAfterRollout(AbstractStep): bug_id = state["bug_id"] if not bug_id: - log(comment_text) - log("No bugs were updated.") + _log.info(comment_text) + _log.info("No bugs were updated.") return self._tool.bugs.reopen_bug(bug_id, comment_text) diff --git a/Tools/Scripts/webkitpy/tool/steps/runtests.py b/Tools/Scripts/webkitpy/tool/steps/runtests.py index aa8729123..6dc90f92c 100644 --- a/Tools/Scripts/webkitpy/tool/steps/runtests.py +++ b/Tools/Scripts/webkitpy/tool/steps/runtests.py @@ -26,11 +26,14 @@ # (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 logging + from webkitpy.tool.steps.abstractstep import AbstractStep from webkitpy.tool.steps.options import Options -from webkitpy.common.system.deprecated_logging import log from webkitpy.common.system.executive import ScriptError +_log = logging.getLogger(__name__) + class RunTests(AbstractStep): # FIXME: This knowledge really belongs in the commit-queue. NON_INTERACTIVE_FAILURE_LIMIT_COUNT = 30 @@ -52,31 +55,31 @@ class RunTests(AbstractStep): python_unittests_command = self._tool.port().run_python_unittests_command() if python_unittests_command: - log("Running Python unit tests") + _log.info("Running Python unit tests") self._tool.executive.run_and_throw_if_fail(python_unittests_command, cwd=self._tool.scm().checkout_root) perl_unittests_command = self._tool.port().run_perl_unittests_command() if perl_unittests_command: - log("Running Perl unit tests") + _log.info("Running Perl unit tests") self._tool.executive.run_and_throw_if_fail(perl_unittests_command, cwd=self._tool.scm().checkout_root) javascriptcore_tests_command = self._tool.port().run_javascriptcore_tests_command() if javascriptcore_tests_command: - log("Running JavaScriptCore tests") + _log.info("Running JavaScriptCore tests") self._tool.executive.run_and_throw_if_fail(javascriptcore_tests_command, quiet=True, cwd=self._tool.scm().checkout_root) webkit_unit_tests_command = self._tool.port().run_webkit_unit_tests_command() if webkit_unit_tests_command: - log("Running WebKit unit tests") + _log.info("Running WebKit unit tests") args = webkit_unit_tests_command if self._options.non_interactive: args.append("--gtest_output=xml:%s/webkit_unit_tests_output.xml" % self._tool.port().results_directory) try: self._tool.executive.run_and_throw_if_fail(args, cwd=self._tool.scm().checkout_root) except ScriptError, e: - log("Error running webkit_unit_tests: %s" % e.message_with_output()) + _log.info("Error running webkit_unit_tests: %s" % e.message_with_output()) - log("Running run-webkit-tests") + _log.info("Running run-webkit-tests") args = self._tool.port().run_webkit_tests_command() if self._options.non_interactive: args.extend([ diff --git a/Tools/Scripts/webkitpy/tool/steps/runtests_unittest.py b/Tools/Scripts/webkitpy/tool/steps/runtests_unittest.py index bf888e505..78a867b36 100644 --- a/Tools/Scripts/webkitpy/tool/steps/runtests_unittest.py +++ b/Tools/Scripts/webkitpy/tool/steps/runtests_unittest.py @@ -38,9 +38,9 @@ class RunTestsTest(unittest.TestCase): tool._deprecated_port.run_python_unittests_command = lambda: None tool._deprecated_port.run_perl_unittests_command = lambda: None step = RunTests(tool, MockOptions(test=True, non_interactive=True, quiet=False)) - expected_stderr = """Running WebKit unit tests + expected_logs = """Running WebKit unit tests MOCK run_and_throw_if_fail: ['mock-run-webkit-unit-tests', '--gtest_output=xml:/mock-results/webkit_unit_tests_output.xml'], cwd=/mock-checkout Running run-webkit-tests MOCK run_and_throw_if_fail: ['mock-run-webkit-tests', '--no-new-test-results', '--no-launch-safari', '--skip-failing-tests', '--exit-after-n-failures=30', '--results-directory=/mock-results', '--quiet'], cwd=/mock-checkout """ - OutputCapture().assert_outputs(self, step.run, [{}], expected_stderr=expected_stderr) + OutputCapture().assert_outputs(self, step.run, [{}], expected_logs=expected_logs) diff --git a/Tools/Scripts/webkitpy/tool/steps/steps_unittest.py b/Tools/Scripts/webkitpy/tool/steps/steps_unittest.py index 99f174932..c4ea47b4d 100644 --- a/Tools/Scripts/webkitpy/tool/steps/steps_unittest.py +++ b/Tools/Scripts/webkitpy/tool/steps/steps_unittest.py @@ -56,8 +56,8 @@ class StepsTest(unittest.TestCase): tool = MockTool() options = self._step_options() options.update = True - expected_stderr = "Updating working directory\n" - OutputCapture().assert_outputs(self, self._run_step, [steps.Update, tool, options], expected_stderr=expected_stderr) + expected_logs = "Updating working directory\n" + OutputCapture().assert_outputs(self, self._run_step, [steps.Update, tool, options], expected_logs=expected_logs) def test_prompt_for_bug_or_title_step(self): tool = MockTool() @@ -74,26 +74,26 @@ class StepsTest(unittest.TestCase): options.open_bug = True return options - def _assert_step_output_with_bug(self, step, bug_id, expected_stderr, options=None): + def _assert_step_output_with_bug(self, step, bug_id, expected_logs, options=None): state = {'bug_id': bug_id} - OutputCapture().assert_outputs(self, self._run_step, [step, MockTool(), options, state], expected_stderr=expected_stderr) + OutputCapture().assert_outputs(self, self._run_step, [step, MockTool(), options, state], expected_logs=expected_logs) - def _assert_post_diff_output_for_bug(self, step, bug_id, expected_stderr): - self._assert_step_output_with_bug(step, bug_id, expected_stderr, self._post_diff_options()) + def _assert_post_diff_output_for_bug(self, step, bug_id, expected_logs): + self._assert_step_output_with_bug(step, bug_id, expected_logs, self._post_diff_options()) def test_post_diff(self): - expected_stderr = "MOCK add_patch_to_bug: bug_id=78, description=Patch, mark_for_review=True, mark_for_commit_queue=False, mark_for_landing=False\nMOCK: user.open_url: http://example.com/78\n" - self._assert_post_diff_output_for_bug(steps.PostDiff, 78, expected_stderr) + expected_logs = "MOCK add_patch_to_bug: bug_id=78, description=Patch, mark_for_review=True, mark_for_commit_queue=False, mark_for_landing=False\nMOCK: user.open_url: http://example.com/78\n" + self._assert_post_diff_output_for_bug(steps.PostDiff, 78, expected_logs) def test_post_diff_for_commit(self): - expected_stderr = "MOCK add_patch_to_bug: bug_id=78, description=Patch for landing, mark_for_review=False, mark_for_commit_queue=False, mark_for_landing=True\n" - self._assert_post_diff_output_for_bug(steps.PostDiffForCommit, 78, expected_stderr) + expected_logs = "MOCK add_patch_to_bug: bug_id=78, description=Patch for landing, mark_for_review=False, mark_for_commit_queue=False, mark_for_landing=True\n" + self._assert_post_diff_output_for_bug(steps.PostDiffForCommit, 78, expected_logs) def test_ensure_bug_is_open_and_assigned(self): - expected_stderr = "MOCK reopen_bug 50004 with comment 'Reopening to attach new patch.'\n" - self._assert_step_output_with_bug(steps.EnsureBugIsOpenAndAssigned, 50004, expected_stderr) - expected_stderr = "MOCK reassign_bug: bug_id=50002, assignee=None\n" - self._assert_step_output_with_bug(steps.EnsureBugIsOpenAndAssigned, 50002, expected_stderr) + expected_logs = "MOCK reopen_bug 50004 with comment 'Reopening to attach new patch.'\n" + self._assert_step_output_with_bug(steps.EnsureBugIsOpenAndAssigned, 50004, expected_logs) + expected_logs = "MOCK reassign_bug: bug_id=50002, assignee=None\n" + self._assert_step_output_with_bug(steps.EnsureBugIsOpenAndAssigned, 50002, expected_logs) def test_runtests_args(self): mock_options = self._step_options() @@ -104,7 +104,7 @@ class StepsTest(unittest.TestCase): tool = MockTool(log_executive=True) tool.port = lambda: mock_port step = steps.RunTests(tool, mock_options) - expected_stderr = """Running Python unit tests + expected_logs = """Running Python unit tests MOCK run_and_throw_if_fail: ['Tools/Scripts/test-webkitpy'], cwd=/mock-checkout Running Perl unit tests MOCK run_and_throw_if_fail: ['Tools/Scripts/test-webkitperl'], cwd=/mock-checkout @@ -113,4 +113,4 @@ MOCK run_and_throw_if_fail: ['Tools/Scripts/run-javascriptcore-tests'], cwd=/moc Running run-webkit-tests MOCK run_and_throw_if_fail: ['Tools/Scripts/run-webkit-tests', '--quiet'], cwd=/mock-checkout """ - OutputCapture().assert_outputs(self, step.run, [{}], expected_stderr=expected_stderr) + OutputCapture().assert_outputs(self, step.run, [{}], expected_logs=expected_logs) diff --git a/Tools/Scripts/webkitpy/tool/steps/suggestreviewers_unittest.py b/Tools/Scripts/webkitpy/tool/steps/suggestreviewers_unittest.py index e99566326..42254c86b 100644 --- a/Tools/Scripts/webkitpy/tool/steps/suggestreviewers_unittest.py +++ b/Tools/Scripts/webkitpy/tool/steps/suggestreviewers_unittest.py @@ -42,5 +42,5 @@ class SuggestReviewersTest(unittest.TestCase): capture = OutputCapture() step = SuggestReviewers(MockTool(), MockOptions(suggest_reviewers=True, git_commit=None)) expected_stdout = "The following reviewers have recently modified files in your patch:\nFoo Bar\n" - expected_stderr = "Would you like to CC them?\n" - capture.assert_outputs(self, step.run, [{"bug_id": "123"}], expected_stdout=expected_stdout, expected_stderr=expected_stderr) + expected_logs = "Would you like to CC them?\n" + capture.assert_outputs(self, step.run, [{"bug_id": "123"}], expected_stdout=expected_stdout, expected_logs=expected_logs) diff --git a/Tools/Scripts/webkitpy/tool/steps/update.py b/Tools/Scripts/webkitpy/tool/steps/update.py index cae2bbd8d..0737ebcd0 100644 --- a/Tools/Scripts/webkitpy/tool/steps/update.py +++ b/Tools/Scripts/webkitpy/tool/steps/update.py @@ -26,9 +26,12 @@ # (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 logging + from webkitpy.tool.steps.abstractstep import AbstractStep from webkitpy.tool.steps.options import Options -from webkitpy.common.system.deprecated_logging import log + +_log = logging.getLogger(__name__) class Update(AbstractStep): @@ -43,7 +46,7 @@ class Update(AbstractStep): def run(self, state): if not self._options.update: return - log("Updating working directory") + _log.info("Updating working directory") self._tool.executive.run_and_throw_if_fail(self._update_command(), quiet=self._options.quiet, cwd=self._tool.scm().checkout_root) def _update_command(self): diff --git a/Tools/Scripts/webkitpy/tool/steps/updatechangelogswithreview_unittest.py b/Tools/Scripts/webkitpy/tool/steps/updatechangelogswithreview_unittest.py index 8ec8891f9..3182cf3ab 100644 --- a/Tools/Scripts/webkitpy/tool/steps/updatechangelogswithreview_unittest.py +++ b/Tools/Scripts/webkitpy/tool/steps/updatechangelogswithreview_unittest.py @@ -36,14 +36,14 @@ class UpdateChangeLogsWithReviewerTest(unittest.TestCase): def test_guess_reviewer_from_bug(self): capture = OutputCapture() step = UpdateChangeLogsWithReviewer(MockTool(), MockOptions()) - expected_stderr = "No reviewed patches on bug 50001, cannot infer reviewer.\n" - capture.assert_outputs(self, step._guess_reviewer_from_bug, [50001], expected_stderr=expected_stderr) + expected_logs = "No reviewed patches on bug 50001, cannot infer reviewer.\n" + capture.assert_outputs(self, step._guess_reviewer_from_bug, [50001], expected_logs=expected_logs) def test_guess_reviewer_from_multipatch_bug(self): capture = OutputCapture() step = UpdateChangeLogsWithReviewer(MockTool(), MockOptions()) - expected_stderr = "Guessing \"Reviewer2\" as reviewer from attachment 10001 on bug 50000.\n" - capture.assert_outputs(self, step._guess_reviewer_from_bug, [50000], expected_stderr=expected_stderr) + expected_logs = "Guessing \"Reviewer2\" as reviewer from attachment 10001 on bug 50000.\n" + capture.assert_outputs(self, step._guess_reviewer_from_bug, [50000], expected_logs=expected_logs) def test_empty_state(self): capture = OutputCapture() diff --git a/Tools/Scripts/webkitpy/tool/steps/updatechangelogswithreviewer.py b/Tools/Scripts/webkitpy/tool/steps/updatechangelogswithreviewer.py index cc3e96525..ef210a02f 100644 --- a/Tools/Scripts/webkitpy/tool/steps/updatechangelogswithreviewer.py +++ b/Tools/Scripts/webkitpy/tool/steps/updatechangelogswithreviewer.py @@ -26,11 +26,15 @@ # (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 logging + from webkitpy.common.checkout.changelog import ChangeLog from webkitpy.tool.grammar import pluralize from webkitpy.tool.steps.abstractstep import AbstractStep from webkitpy.tool.steps.options import Options -from webkitpy.common.system.deprecated_logging import log, error + +_log = logging.getLogger(__name__) + class UpdateChangeLogsWithReviewer(AbstractStep): @classmethod @@ -45,10 +49,10 @@ class UpdateChangeLogsWithReviewer(AbstractStep): # here as we don't currently have a way to invalidate a bug after making changes (like ObsoletePatches does). patches = self._tool.bugs.fetch_bug(bug_id).reviewed_patches() if not patches: - log("%s on bug %s, cannot infer reviewer." % ("No reviewed patches", bug_id)) + _log.info("%s on bug %s, cannot infer reviewer." % ("No reviewed patches", bug_id)) return None patch = patches[-1] - log("Guessing \"%s\" as reviewer from attachment %s on bug %s." % (patch.reviewer().full_name, patch.id(), bug_id)) + _log.info("Guessing \"%s\" as reviewer from attachment %s on bug %s." % (patch.reviewer().full_name, patch.id(), bug_id)) return patch.reviewer().full_name def run(self, state): @@ -59,12 +63,12 @@ class UpdateChangeLogsWithReviewer(AbstractStep): reviewer = self._options.reviewer if not reviewer: if not bug_id: - log("No bug id provided and --reviewer= not provided. Not updating ChangeLogs with reviewer.") + _log.info("No bug id provided and --reviewer= not provided. Not updating ChangeLogs with reviewer.") return reviewer = self._guess_reviewer_from_bug(bug_id) if not reviewer: - log("Failed to guess reviewer from bug %s and --reviewer= not provided. Not updating ChangeLogs with reviewer." % bug_id) + _log.info("Failed to guess reviewer from bug %s and --reviewer= not provided. Not updating ChangeLogs with reviewer." % bug_id) return # cached_lookup("changelogs") is always absolute paths. diff --git a/Tools/Scripts/webkitpy/tool/steps/updatechromiumdeps.py b/Tools/Scripts/webkitpy/tool/steps/updatechromiumdeps.py index c9fc63179..23d861bfc 100644 --- a/Tools/Scripts/webkitpy/tool/steps/updatechromiumdeps.py +++ b/Tools/Scripts/webkitpy/tool/steps/updatechromiumdeps.py @@ -26,12 +26,15 @@ # (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 logging +import sys import urllib2 from webkitpy.tool.steps.abstractstep import AbstractStep from webkitpy.tool.steps.options import Options from webkitpy.common.config import urls -from webkitpy.common.system.deprecated_logging import log, error + +_log = logging.getLogger(__name__) class UpdateChromiumDEPS(AbstractStep): @@ -49,16 +52,17 @@ class UpdateChromiumDEPS(AbstractStep): if new_chromium_revision < current_chromium_revision: message = "Current Chromium DEPS revision %s is newer than %s." % (current_chromium_revision, new_chromium_revision) if self._options.non_interactive: - error(message) # Causes the process to terminate. - log(message) + _log.error(message) + sys.exit(1) + _log.info(message) new_chromium_revision = self._tool.user.prompt("Enter new chromium revision (enter nothing to cancel):\n") try: new_chromium_revision = int(new_chromium_revision) except ValueError, TypeError: new_chromium_revision = None if not new_chromium_revision: - error("Unable to update Chromium DEPS") - + _log.error("Unable to update Chromium DEPS") + sys.exit(1) def run(self, state): # Note that state["chromium_revision"] must be defined, but can be None. @@ -69,5 +73,5 @@ class UpdateChromiumDEPS(AbstractStep): deps = self._tool.checkout().chromium_deps() current_chromium_revision = deps.read_variable("chromium_rev") self._validate_revisions(current_chromium_revision, new_chromium_revision) - log("Updating Chromium DEPS to %s" % new_chromium_revision) + _log.info("Updating Chromium DEPS to %s" % new_chromium_revision) deps.write_variable("chromium_rev", new_chromium_revision) diff --git a/Tools/Scripts/webkitpy/tool/steps/validatechangelogs.py b/Tools/Scripts/webkitpy/tool/steps/validatechangelogs.py index b6b33c0b6..061baa5ec 100644 --- a/Tools/Scripts/webkitpy/tool/steps/validatechangelogs.py +++ b/Tools/Scripts/webkitpy/tool/steps/validatechangelogs.py @@ -26,10 +26,14 @@ # (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 logging +import sys + from webkitpy.tool.steps.abstractstep import AbstractStep from webkitpy.tool.steps.options import Options from webkitpy.common.checkout.diff_parser import DiffParser -from webkitpy.common.system.deprecated_logging import error, log + +_log = logging.getLogger(__name__) # This is closely related to the ValidateReviewer step and the CommitterValidator class. @@ -53,7 +57,7 @@ class ValidateChangeLogs(AbstractStep): if self._options.non_interactive: return False - log("The diff to %s looks wrong. Are you sure your ChangeLog entry is at the top of the file?" % (diff_file.filename)) + _log.info("The diff to %s looks wrong. Are you sure your ChangeLog entry is at the top of the file?" % (diff_file.filename)) # FIXME: Do we need to make the file path absolute? self._tool.scm().diff_for_file(diff_file.filename) if self._tool.user.confirm("OK to continue?", default='n'): @@ -73,4 +77,5 @@ class ValidateChangeLogs(AbstractStep): parsed_diff = DiffParser(diff.splitlines()) for filename, diff_file in parsed_diff.files.items(): if not self._check_changelog_diff(diff_file): - error("ChangeLog entry in %s is not at the top of the file." % diff_file.filename) + _log.error("ChangeLog entry in %s is not at the top of the file." % diff_file.filename) + sys.exit(1) diff --git a/Tools/Scripts/webkitpy/tool/steps/validatechangelogs_unittest.py b/Tools/Scripts/webkitpy/tool/steps/validatechangelogs_unittest.py index 96bae9fa8..c3b723ed1 100644 --- a/Tools/Scripts/webkitpy/tool/steps/validatechangelogs_unittest.py +++ b/Tools/Scripts/webkitpy/tool/steps/validatechangelogs_unittest.py @@ -43,10 +43,10 @@ class ValidateChangeLogsTest(unittest.TestCase): diff_file = Mock() diff_file.filename = "mock/ChangeLog" diff_file.lines = [(start_line, start_line, "foo")] - expected_stdout = expected_stderr = "" + expected_stdout = expected_stderr = expected_logs = "" if should_fail and not non_interactive: - expected_stderr = "The diff to mock/ChangeLog looks wrong. Are you sure your ChangeLog entry is at the top of the file?\nOK to continue?\n" - result = OutputCapture().assert_outputs(self, step._check_changelog_diff, [diff_file], expected_stderr=expected_stderr) + expected_logs = "The diff to mock/ChangeLog looks wrong. Are you sure your ChangeLog entry is at the top of the file?\nOK to continue?\n" + result = OutputCapture().assert_outputs(self, step._check_changelog_diff, [diff_file], expected_logs=expected_logs) self.assertEqual(not result, should_fail) def test_check_changelog_diff(self): diff --git a/Tools/Scripts/webkitpy/tool/steps/validatereviewer.py b/Tools/Scripts/webkitpy/tool/steps/validatereviewer.py index 5e93821ce..90ddf5be3 100644 --- a/Tools/Scripts/webkitpy/tool/steps/validatereviewer.py +++ b/Tools/Scripts/webkitpy/tool/steps/validatereviewer.py @@ -26,10 +26,14 @@ # (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 logging +import sys + from webkitpy.common.checkout.changelog import ChangeLog from webkitpy.tool.steps.abstractstep import AbstractStep from webkitpy.tool.steps.options import Options -from webkitpy.common.system.deprecated_logging import error, log + +_log = logging.getLogger(__name__) # FIXME: Some of this logic should probably be unified with CommitterValidator? @@ -51,5 +55,6 @@ class ValidateReviewer(AbstractStep): continue reviewer_text = changelog_entry.reviewer_text() if reviewer_text: - log("%s found in %s does not appear to be a valid reviewer according to committers.py." % (reviewer_text, changelog_path)) - error('%s neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).' % changelog_path) + _log.info("%s found in %s does not appear to be a valid reviewer according to committers.py." % (reviewer_text, changelog_path)) + _log.error('%s neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).' % changelog_path) + sys.exit(1) |