Modified: trunk/Tools/Scripts/webkitpy/common/system/outputcapture.py (97819 => 97820)
--- trunk/Tools/Scripts/webkitpy/common/system/outputcapture.py 2011-10-19 00:40:30 UTC (rev 97819)
+++ trunk/Tools/Scripts/webkitpy/common/system/outputcapture.py 2011-10-19 00:47:26 UTC (rev 97820)
@@ -28,6 +28,7 @@
#
# Class for unittest support. Used for capturing stderr/stdout.
+import logging
import sys
import unittest
from StringIO import StringIO
@@ -50,20 +51,32 @@
return captured_output
def capture_output(self):
+ self._logs = StringIO()
+ self._logs_handler = logging.StreamHandler(self._logs)
+ self._logs_handler.setLevel(logging.INFO)
+ logging.getLogger().addHandler(self._logs_handler)
return (self._capture_output_with_name("stdout"), self._capture_output_with_name("stderr"))
def restore_output(self):
- return (self._restore_output_with_name("stdout"), self._restore_output_with_name("stderr"))
+ logging.getLogger().removeHandler(self._logs_handler)
+ self._logs_handler.flush()
+ self._logs.flush()
+ logs_string = self._logs.getvalue()
+ delattr(self, '_logs_handler')
+ delattr(self, '_logs')
+ return (self._restore_output_with_name("stdout"), self._restore_output_with_name("stderr"), logs_string)
- def assert_outputs(self, testcase, function, args=[], kwargs={}, expected_stdout="", expected_stderr="", expected_exception=None):
+ def assert_outputs(self, testcase, function, args=[], kwargs={}, expected_stdout="", expected_stderr="", expected_exception=None, expected_logs=None):
self.capture_output()
if expected_exception:
return_value = testcase.assertRaises(expected_exception, function, *args, **kwargs)
else:
return_value = function(*args, **kwargs)
- (stdout_string, stderr_string) = self.restore_output()
+ (stdout_string, stderr_string, logs_string) = self.restore_output()
testcase.assertEqual(stdout_string, expected_stdout)
testcase.assertEqual(stderr_string, expected_stderr)
+ if expected_logs is not None:
+ testcase.assertEqual(logs_string, expected_logs)
# This is a little strange, but I don't know where else to return this information.
return return_value
Modified: trunk/Tools/Scripts/webkitpy/common/watchlist/watchlistparser.py (97819 => 97820)
--- trunk/Tools/Scripts/webkitpy/common/watchlist/watchlistparser.py 2011-10-19 00:40:30 UTC (rev 97819)
+++ trunk/Tools/Scripts/webkitpy/common/watchlist/watchlistparser.py 2011-10-19 00:47:26 UTC (rev 97820)
@@ -28,6 +28,7 @@
import difflib
+import logging
import re
from webkitpy.common.watchlist.amountchangedpattern import AmountChangedPattern
@@ -38,13 +39,17 @@
from webkitpy.common.config.committers import CommitterList
+_log = logging.getLogger(__name__)
+
+
class WatchListParser(object):
_DEFINITIONS = 'DEFINITIONS'
_CC_RULES = 'CC_RULES'
_MESSAGE_RULES = 'MESSAGE_RULES'
_INVALID_DEFINITION_NAME_REGEX = r'\|'
- def __init__(self):
+ def __init__(self, log_error=None):
+ self._log_error = log_error or _log.error
self._section_parsers = {
self._DEFINITIONS: self._parse_definition_section,
self._CC_RULES: self._parse_cc_rules,
@@ -68,9 +73,10 @@
for section in dictionary:
parser = self._section_parsers.get(section)
if not parser:
- raise Exception(('Unknown section "%s" in watch list.'
- + self._suggest_words(section, self._section_parsers.keys()))
- % section)
+ self._log_error(('Unknown section "%s" in watch list.'
+ + self._suggest_words(section, self._section_parsers.keys()))
+ % section)
+ continue
parser(dictionary[section], watch_list)
self._validate(watch_list)
@@ -90,21 +96,24 @@
for name in definition_section:
invalid_character = re.search(self._INVALID_DEFINITION_NAME_REGEX, name)
if invalid_character:
- raise Exception('Invalid character "%s" in definition "%s".' % (invalid_character.group(0), name))
+ self._log_error('Invalid character "%s" in definition "%s".' % (invalid_character.group(0), name))
+ continue
definition = definition_section[name]
definitions[name] = []
for pattern_type in definition:
pattern_parser = self._definition_pattern_parsers.get(pattern_type)
if not pattern_parser:
- raise Exception(('Unknown pattern type "%s" in definition "%s".'
+ self._log_error(('Unknown pattern type "%s" in definition "%s".'
+ self._suggest_words(pattern_type, self._definition_pattern_parsers.keys()))
% (pattern_type, name))
+ continue
pattern = pattern_parser(definition[pattern_type])
definitions[name].append(pattern)
if not definitions[name]:
- raise Exception('The definition "%s" has no patterns, so it should be deleted.' % name)
+ self._log_error('The definition "%s" has no patterns, so it should be deleted.' % name)
+ continue
watch_list.definitions = definitions
def _parse_rules(self, rules_section):
@@ -112,7 +121,8 @@
for complex_definition in rules_section:
instructions = rules_section[complex_definition]
if not instructions:
- raise Exception('A rule for definition "%s" is empty, so it should be deleted.' % complex_definition)
+ self._log_error('A rule for definition "%s" is empty, so it should be deleted.' % complex_definition)
+ continue
rules.append(WatchListRule(complex_definition, instructions))
return rules
@@ -132,15 +142,20 @@
contributors = CommitterList()
for cc_rule in watch_list.cc_rules:
- for email in cc_rule.instructions():
+ # Copy the instructions since we'll be remove items from the original list and
+ # modifying a list while iterating through it leads to undefined behavior.
+ intructions_copy = cc_rule.instructions()[:]
+ for email in intructions_copy:
if not contributors.contributor_by_email(email):
- raise Exception("The email alias %s which is in the watchlist is not listed as a contributor in committers.py" % email)
+ cc_rule.remove_instruction(email)
+ self._log_error("The email alias %s which is in the watchlist is not listed as a contributor in committers.py" % email)
+ continue
def _verify_all_definitions_are_used(self, watch_list, used_definitions):
definitions_not_used = set(watch_list.definitions.keys())
definitions_not_used.difference_update(used_definitions)
if definitions_not_used:
- raise Exception('The following definitions are not used and should be removed: %s' % (', '.join(definitions_not_used)))
+ self._log_error('The following definitions are not used and should be removed: %s' % (', '.join(definitions_not_used)))
def _validate_definitions(self, definitions, rules_section_name, watch_list):
declared_definitions = watch_list.definitions.keys()
@@ -151,7 +166,7 @@
suggestions = ''
if len(definition_set) == 1:
suggestions = self._suggest_words(set().union(definition_set).pop(), declared_definitions)
- raise Exception('In section "%s", the following definitions are not used and should be removed: %s%s' % (rules_section_name, ', '.join(definition_set), suggestions))
+ self._log_error('In section "%s", the following definitions are not used and should be removed: %s%s' % (rules_section_name, ', '.join(definition_set), suggestions))
def _rule_definitions_as_set(self, rules):
definition_set = set()
Modified: trunk/Tools/Scripts/webkitpy/common/watchlist/watchlistparser_unittest.py (97819 => 97820)
--- trunk/Tools/Scripts/webkitpy/common/watchlist/watchlistparser_unittest.py 2011-10-19 00:40:30 UTC (rev 97819)
+++ trunk/Tools/Scripts/webkitpy/common/watchlist/watchlistparser_unittest.py 2011-10-19 00:47:26 UTC (rev 97820)
@@ -28,7 +28,13 @@
'''Unit tests for watchlistparser.py.'''
+
+import logging
+import sys
+
+
from webkitpy.common import webkitunittest
+from webkitpy.common.system.outputcapture import OutputCapture
from webkitpy.common.watchlist.watchlistparser import WatchListParser
@@ -38,19 +44,18 @@
self._watch_list_parser = WatchListParser()
def test_bad_section(self):
- watch_list_with_bad_section = ('{"FOO": {}}')
- self.assertRaisesRegexp(Exception, 'Unknown section "FOO" in watch list.',
- self._watch_list_parser.parse, watch_list_with_bad_section)
+ watch_list = ('{"FOO": {}}')
+ OutputCapture().assert_outputs(self, self._watch_list_parser.parse, args=[watch_list],
+ expected_logs='Unknown section "FOO" in watch list.\n')
def test_section_typo(self):
- watch_list_with_bad_section = ('{"DEFINTIONS": {}}')
- self.assertRaisesRegexp(Exception,
- r'Unknown section "DEFINTIONS" in watch list\.\s*'
- + r'Perhaps it should be DEFINITIONS\.',
- self._watch_list_parser.parse, watch_list_with_bad_section)
+ watch_list = ('{"DEFINTIONS": {}}')
+ OutputCapture().assert_outputs(self, self._watch_list_parser.parse, args=[watch_list],
+ expected_logs='Unknown section "DEFINTIONS" in watch list.'
+ + '\n\nPerhaps it should be DEFINITIONS.\n')
def test_bad_definition(self):
- watch_list_with_bad_definition = (
+ watch_list = (
'{'
' "DEFINITIONS": {'
' "WatchList1|A": {'
@@ -59,35 +64,43 @@
' },'
'}')
- self.assertRaisesRegexp(Exception, r'Invalid character "\|" in definition "WatchList1\|A"\.',
- self._watch_list_parser.parse, watch_list_with_bad_definition)
+ OutputCapture().assert_outputs(self, self._watch_list_parser.parse, args=[watch_list],
+ expected_logs='Invalid character "|" in definition "WatchList1|A".\n')
def test_bad_match_type(self):
- watch_list_with_bad_match_type = (
+ watch_list = (
'{'
' "DEFINITIONS": {'
' "WatchList1": {'
' "nothing_matches_this": r".*\\MyFileName\\.cpp",'
+ ' "filename": r".*\\MyFileName\\.cpp",'
' },'
' },'
+ ' "CC_RULES": {'
+ ' "WatchList1": ["[email protected]"],'
+ ' },'
'}')
- self.assertRaisesRegexp(Exception, 'Unknown pattern type "nothing_matches_this" in definition "WatchList1".',
- self._watch_list_parser.parse, watch_list_with_bad_match_type)
+ OutputCapture().assert_outputs(self, self._watch_list_parser.parse, args=[watch_list],
+ expected_logs='Unknown pattern type "nothing_matches_this" in definition "WatchList1".\n')
def test_match_type_typo(self):
- watch_list_with_bad_match_type = (
+ watch_list = (
'{'
' "DEFINITIONS": {'
' "WatchList1": {'
' "iflename": r".*\\MyFileName\\.cpp",'
+ ' "more": r"RefCounted",'
' },'
' },'
+ ' "CC_RULES": {'
+ ' "WatchList1": ["[email protected]"],'
+ ' },'
'}')
- self.assertRaisesRegexp(Exception, r'Unknown pattern type "iflename" in definition "WatchList1"\.\s*'
- + r'Perhaps it should be filename\.',
- self._watch_list_parser.parse, watch_list_with_bad_match_type)
+ OutputCapture().assert_outputs(self, self._watch_list_parser.parse, args=[watch_list],
+ expected_logs='Unknown pattern type "iflename" in definition "WatchList1".'
+ + '\n\nPerhaps it should be filename.\n')
def test_empty_definition(self):
watch_list = (
@@ -96,10 +109,13 @@
' "WatchList1": {'
' },'
' },'
+ ' "CC_RULES": {'
+ ' "WatchList1": ["[email protected]"],'
+ ' },'
'}')
- self.assertRaisesRegexp(Exception, r'The definition "WatchList1" has no patterns, so it should be deleted.',
- self._watch_list_parser.parse, watch_list)
+ OutputCapture().assert_outputs(self, self._watch_list_parser.parse, args=[watch_list],
+ expected_logs='The definition "WatchList1" has no patterns, so it should be deleted.\n')
def test_empty_cc_rule(self):
watch_list = (
@@ -114,8 +130,9 @@
' },'
'}')
- self.assertRaisesRegexp(Exception, r'A rule for definition "WatchList1" is empty, so it should be deleted.',
- self._watch_list_parser.parse, watch_list)
+ OutputCapture().assert_outputs(self, self._watch_list_parser.parse, args=[watch_list],
+ expected_logs='A rule for definition "WatchList1" is empty, so it should be deleted.\n'
+ + 'The following definitions are not used and should be removed: WatchList1\n')
def test_cc_rule_with_invalid_email(self):
watch_list = (
@@ -130,8 +147,9 @@
' },'
'}')
- self.assertRaisesRegexp(Exception, r'The email alias levin\+bad\[email protected] which is in the watchlist is not listed as a contributor in committers\.py',
- self._watch_list_parser.parse, watch_list)
+ OutputCapture().assert_outputs(self, self._watch_list_parser.parse, args=[watch_list],
+ expected_logs='The email alias [email protected] which is'
+ + ' in the watchlist is not listed as a contributor in committers.py\n')
def test_empty_message_rule(self):
watch_list = (
@@ -147,8 +165,9 @@
' },'
'}')
- self.assertRaisesRegexp(Exception, r'A rule for definition "WatchList1" is empty, so it should be deleted.',
- self._watch_list_parser.parse, watch_list)
+ OutputCapture().assert_outputs(self, self._watch_list_parser.parse, args=[watch_list],
+ expected_logs='A rule for definition "WatchList1" is empty, so it should be deleted.\n'
+ + 'The following definitions are not used and should be removed: WatchList1\n')
def test_unused_defintion(self):
watch_list = (
@@ -160,8 +179,8 @@
' },'
'}')
- self.assertRaisesRegexp(Exception, r'The following definitions are not used and should be removed: WatchList1',
- self._watch_list_parser.parse, watch_list)
+ OutputCapture().assert_outputs(self, self._watch_list_parser.parse, args=[watch_list],
+ expected_logs='The following definitions are not used and should be removed: WatchList1\n')
def test_cc_rule_with_undefined_defintion(self):
watch_list = (
@@ -171,8 +190,8 @@
' },'
'}')
- self.assertRaisesRegexp(Exception, r'In section "CC_RULES", the following definitions are not used and should be removed: WatchList1',
- self._watch_list_parser.parse, watch_list)
+ OutputCapture().assert_outputs(self, self._watch_list_parser.parse, args=[watch_list],
+ expected_logs='In section "CC_RULES", the following definitions are not used and should be removed: WatchList1\n')
def test_message_rule_with_undefined_defintion(self):
watch_list = (
@@ -182,8 +201,8 @@
' },'
'}')
- self.assertRaisesRegexp(Exception, r'In section "MESSAGE_RULES", the following definitions are not used and should be removed: WatchList1',
- self._watch_list_parser.parse, watch_list)
+ OutputCapture().assert_outputs(self, self._watch_list_parser.parse, args=[watch_list],
+ expected_logs='In section "MESSAGE_RULES", the following definitions are not used and should be removed: WatchList1\n')
def test_cc_rule_with_undefined_defintion_with_suggestion(self):
watch_list = (
@@ -201,6 +220,6 @@
' },'
'}')
- self.assertRaisesRegexp(Exception, r'In section "CC_RULES", the following definitions are not used and should be removed: WatchList\s*'
- r'Perhaps it should be WatchList1\.',
- self._watch_list_parser.parse, watch_list)
+ OutputCapture().assert_outputs(self, self._watch_list_parser.parse, args=[watch_list],
+ expected_logs='In section "CC_RULES", the following definitions are not used and should be removed: WatchList'
+ + '\n\nPerhaps it should be WatchList1.\n')
