webkitpy: lint code in webkitpy.layout_tests.models
authordpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 11 Jul 2012 23:17:18 +0000 (23:17 +0000)
committerdpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 11 Jul 2012 23:17:18 +0000 (23:17 +0000)
https://bugs.webkit.org/show_bug.cgi?id=90416

Reviewed by Ojan Vafai.

Cleaning up errors reported from lint-webkitpy.

Also, suppress the warnings about wildcard imports in pylintrc;
we have nothing particularly against them.

* Scripts/webkitpy/layout_tests/models/test_configuration.py:
(TestConfigurationConverter.combinations):
* Scripts/webkitpy/layout_tests/models/test_configuration_unittest.py:
(TestConfigurationTest.test_hash.query_unknown_key):
(TestConfigurationTest.test_eq):
* Scripts/webkitpy/layout_tests/models/test_expectations.py:
(ParseError.__init__):
(TestExpectationLine.__init__):
(TestExpectationsModel.get_expectations_string):
(TestExpectationsModel):
(TestExpectationsModel.expectation_to_string):
(TestExpectationsModel.add_expectation_line):
(TestExpectationsModel._clear_expectations_for_test):
(TestExpectationsModel._remove_from_sets):
(TestExpectations.get_expectations_string):
(TestExpectations.expectation_to_string):
(TestExpectations._report_warnings):
* Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py:
(Base.__init__):
(parse_exp):
(SkippedTests.check):
(TestExpectationParserTests.test_parse_empty_string):
* Scripts/webkitpy/layout_tests/models/test_failures.py:
(FailureTimeout.__init__):
(FailureCrash.__init__):
(FailureImageHashMismatch.__init__):
(FailureReftestMismatch.__init__):
(FailureReftestMismatchDidNotOccur.__init__):
(FailureReftestNoImagesGenerated.__init__):
* Scripts/webkitpy/layout_tests/models/test_failures_unittest.py:
(TestFailuresTest.test_unknown_failure_type.UnknownFailure.message):
(TestFailuresTest.test_unknown_failure_type):
(TestFailuresTest):
(TestFailuresTest.test_message_is_virtual):
* Scripts/webkitpy/layout_tests/models/test_results.py:
(TestResult.loads):
(TestResult.has_failure_matching_types):
* Scripts/webkitpy/pylintrc:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@122387 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog
Tools/Scripts/webkitpy/layout_tests/models/test_configuration.py
Tools/Scripts/webkitpy/layout_tests/models/test_configuration_unittest.py
Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py
Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py
Tools/Scripts/webkitpy/layout_tests/models/test_failures.py
Tools/Scripts/webkitpy/layout_tests/models/test_failures_unittest.py
Tools/Scripts/webkitpy/layout_tests/models/test_results.py
Tools/Scripts/webkitpy/pylintrc

index 13bed43..3c4473b 100644 (file)
@@ -1,5 +1,56 @@
 2012-07-11  Dirk Pranke  <dpranke@chromium.org>
 
+        webkitpy: lint code in webkitpy.layout_tests.models
+        https://bugs.webkit.org/show_bug.cgi?id=90416
+
+        Reviewed by Ojan Vafai.
+
+        Cleaning up errors reported from lint-webkitpy.
+
+        Also, suppress the warnings about wildcard imports in pylintrc;
+        we have nothing particularly against them.
+
+        * Scripts/webkitpy/layout_tests/models/test_configuration.py:
+        (TestConfigurationConverter.combinations):
+        * Scripts/webkitpy/layout_tests/models/test_configuration_unittest.py:
+        (TestConfigurationTest.test_hash.query_unknown_key):
+        (TestConfigurationTest.test_eq):
+        * Scripts/webkitpy/layout_tests/models/test_expectations.py:
+        (ParseError.__init__):
+        (TestExpectationLine.__init__):
+        (TestExpectationsModel.get_expectations_string):
+        (TestExpectationsModel):
+        (TestExpectationsModel.expectation_to_string):
+        (TestExpectationsModel.add_expectation_line):
+        (TestExpectationsModel._clear_expectations_for_test):
+        (TestExpectationsModel._remove_from_sets):
+        (TestExpectations.get_expectations_string):
+        (TestExpectations.expectation_to_string):
+        (TestExpectations._report_warnings):
+        * Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py:
+        (Base.__init__):
+        (parse_exp):
+        (SkippedTests.check):
+        (TestExpectationParserTests.test_parse_empty_string):
+        * Scripts/webkitpy/layout_tests/models/test_failures.py:
+        (FailureTimeout.__init__):
+        (FailureCrash.__init__):
+        (FailureImageHashMismatch.__init__):
+        (FailureReftestMismatch.__init__):
+        (FailureReftestMismatchDidNotOccur.__init__):
+        (FailureReftestNoImagesGenerated.__init__):
+        * Scripts/webkitpy/layout_tests/models/test_failures_unittest.py:
+        (TestFailuresTest.test_unknown_failure_type.UnknownFailure.message):
+        (TestFailuresTest.test_unknown_failure_type):
+        (TestFailuresTest):
+        (TestFailuresTest.test_message_is_virtual):
+        * Scripts/webkitpy/layout_tests/models/test_results.py:
+        (TestResult.loads):
+        (TestResult.has_failure_matching_types):
+        * Scripts/webkitpy/pylintrc:
+
+2012-07-11  Dirk Pranke  <dpranke@chromium.org>
+
         nrwt: clean up names in worker.py
         https://bugs.webkit.org/show_bug.cgi?id=90510
 
index e960727..95d0f2b 100644 (file)
@@ -26,7 +26,6 @@
 # (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 itertools
 
 class TestConfiguration(object):
     def __init__(self, version, architecture, build_type):
@@ -212,8 +211,8 @@ class TestConfigurationConverter(object):
                     break
             else:
                 return
-            indices[i] += 1
-            for j in range(i + 1, r):
+            indices[i] += 1  # pylint: disable=W0631
+            for j in range(i + 1, r):  # pylint: disable=W0631
                 indices[j] = indices[j - 1] + 1
             yield tuple(pool[i] for i in indices)
 
index c367b75..5c43b6a 100644 (file)
@@ -28,8 +28,6 @@
 
 import unittest
 
-from webkitpy.common.host_mock import MockHost
-
 from webkitpy.layout_tests.models.test_configuration import *
 
 
@@ -77,7 +75,7 @@ class TestConfigurationTest(unittest.TestCase):
         self.assertTrue(config_dict[TestConfiguration('xp', 'x86', 'release')])
 
         def query_unknown_key():
-            config_dict[TestConfiguration('xp', 'x86', 'debug')]
+            return config_dict[TestConfiguration('xp', 'x86', 'debug')]
 
         self.assertRaises(KeyError, query_unknown_key)
         self.assertTrue(TestConfiguration('xp', 'x86', 'release') in config_dict)
@@ -88,8 +86,6 @@ class TestConfigurationTest(unittest.TestCase):
 
     def test_eq(self):
         self.assertEquals(TestConfiguration('xp', 'x86', 'release'), TestConfiguration('xp', 'x86', 'release'))
-        host = MockHost()
-        test_port = host.port_factory.get('test-win-xp', None)
         self.assertNotEquals(TestConfiguration('xp', 'x86', 'release'), TestConfiguration('xp', 'x86', 'debug'))
 
     def test_values(self):
index 93ef517..9c6d478 100644 (file)
 for layout tests.
 """
 
-import itertools
-import json
 import logging
 import re
 
-from webkitpy.layout_tests.models.test_configuration import TestConfiguration, TestConfigurationConverter
+from webkitpy.layout_tests.models.test_configuration import TestConfigurationConverter
 
 _log = logging.getLogger(__name__)
 
@@ -117,6 +115,7 @@ def strip_comments(line):
 
 class ParseError(Exception):
     def __init__(self, warnings):
+        super(ParseError, self).__init__()
         self.warnings = warnings
 
     def __str__(self):
@@ -400,9 +399,10 @@ class TestExpectationLine(object):
     def __init__(self):
         """Initializes a blank-line equivalent of an expectation."""
         self.original_string = None
+        self.filename = None  # this is the path to the expectations file for this line
         self.line_number = None
-        self.name = None
-        self.path = None
+        self.name = None  # this is the path in the line itself
+        self.path = None  # this is the normpath of self.name
         self.modifiers = []
         self.parsed_modifiers = []
         self.parsed_bug_modifiers = []
@@ -515,6 +515,25 @@ class TestExpectationsModel(object):
     def get_expectations(self, test):
         return self._test_to_expectations[test]
 
+    def get_expectations_string(self, test):
+        """Returns the expectatons for the given test as an uppercase string.
+        If there are no expectations for the test, then "PASS" is returned."""
+        expectations = self.get_expectations(test)
+        retval = []
+
+        for expectation in expectations:
+            retval.append(self.expectation_to_string(expectation))
+
+        return " ".join(retval)
+
+    def expectation_to_string(self, expectation):
+        """Return the uppercased string equivalent of a given expectation."""
+        for item in TestExpectations.EXPECTATIONS.items():
+            if item[1] == expectation:
+                return item[0].upper()
+        raise ValueError(expectation)
+
+
     def add_expectation_line(self, expectation_line, in_skipped=False):
         """Returns a list of warnings encountered while matching modifiers."""
 
@@ -525,7 +544,7 @@ class TestExpectationsModel(object):
             if not in_skipped and self._already_seen_better_match(test, expectation_line):
                 continue
 
-            self._clear_expectations_for_test(test, expectation_line)
+            self._clear_expectations_for_test(test)
             self._test_to_expectation_line[test] = expectation_line
             self._add_test(test, expectation_line)
 
@@ -559,7 +578,7 @@ class TestExpectationsModel(object):
             # FIXME: What is this?
             self._result_type_to_tests[FAIL].add(test)
 
-    def _clear_expectations_for_test(self, test, expectation_line):
+    def _clear_expectations_for_test(self, test):
         """Remove prexisting expectations for this test.
         This happens if we are seeing a more precise path
         than a previous listing.
@@ -571,13 +590,13 @@ class TestExpectationsModel(object):
             self._remove_from_sets(test, self._timeline_to_tests)
             self._remove_from_sets(test, self._result_type_to_tests)
 
-    def _remove_from_sets(self, test, dict):
+    def _remove_from_sets(self, test, dict_of_sets_of_tests):
         """Removes the given test from the sets in the dictionary.
 
         Args:
           test: test to look for
           dict: dict of sets of files"""
-        for set_of_tests in dict.itervalues():
+        for set_of_tests in dict_of_sets_of_tests.itervalues():
             if test in set_of_tests:
                 set_of_tests.remove(test)
 
@@ -782,22 +801,10 @@ class TestExpectations(object):
         return self._model.get_tests_with_timeline(timeline)
 
     def get_expectations_string(self, test):
-        """Returns the expectatons for the given test as an uppercase string.
-        If there are no expectations for the test, then "PASS" is returned."""
-        expectations = self._model.get_expectations(test)
-        retval = []
-
-        for expectation in expectations:
-            retval.append(self.expectation_to_string(expectation))
-
-        return " ".join(retval)
+        return self._model.get_expectations_string(test)
 
     def expectation_to_string(self, expectation):
-        """Return the uppercased string equivalent of a given expectation."""
-        for item in self.EXPECTATIONS.items():
-            if item[1] == expectation:
-                return item[0].upper()
-        raise ValueError(expectation)
+        return self._model.expectation_to_string(expectation)
 
     def matches_an_expected_result(self, test, result, pixel_tests_are_enabled):
         expected_results = self._model.get_expectations(test)
@@ -820,7 +827,8 @@ class TestExpectations(object):
         warnings = []
         for expectation in self._expectations:
             for warning in expectation.warnings:
-                warnings.append('%s:%d %s %s' % (self._shorten_filename(expectation.filename), expectation.line_number, warning, expectation.name if expectation.expectations else expectation.original_string))
+                warnings.append('%s:%d %s %s' % (self._shorten_filename(expectation.filename), expectation.line_number,
+                                warning, expectation.name if expectation.expectations else expectation.original_string))
 
         if warnings:
             self._has_warnings = True
index 7b589b5..c780dac 100644 (file)
@@ -95,7 +95,7 @@ class Base(unittest.TestCase):
     # Note that all of these tests are written assuming the configuration
     # being tested is Windows XP, Release build.
 
-    def __init__(self, testFunc, setUp=None, tearDown=None, description=None):
+    def __init__(self, testFunc):
         host = MockHost()
         self._port = host.port_factory.get('test-win-xp', None)
         self._exp = None
@@ -123,11 +123,11 @@ BUG_TEST WONTFIX MAC : failures/expected/image.html = IMAGE
 """
 
     def parse_exp(self, expectations, overrides=None, is_lint_mode=False):
-        self._expectations_dict = OrderedDict()
-        self._expectations_dict['expectations'] = expectations
+        expectations_dict = OrderedDict()
+        expectations_dict['expectations'] = expectations
         if overrides:
-            self._expectations_dict['overrides'] = overrides
-        self._port.expectations_dict = lambda: self._expectations_dict
+            expectations_dict['overrides'] = overrides
+        self._port.expectations_dict = lambda: expectations_dict
         self._exp = TestExpectations(self._port, self.get_basic_tests(), is_lint_mode)
 
     def assert_exp(self, test, result):
@@ -274,11 +274,11 @@ class SkippedTests(Base):
     def check(self, expectations, overrides, skips, lint=False):
         port = MockHost().port_factory.get('qt')
         port._filesystem.write_text_file(port._filesystem.join(port.layout_tests_dir(), 'failures/expected/text.html'), 'foo')
-        self._expectations_dict = OrderedDict()
-        self._expectations_dict['expectations'] = expectations
+        expectations_dict = OrderedDict()
+        expectations_dict['expectations'] = expectations
         if overrides:
-            self._expectations_dict['overrides'] = overrides
-        port.expectations_dict = lambda: self._expectations_dict
+            expectations_dict['overrides'] = overrides
+        port.expectations_dict = lambda: expectations_dict
         port.skipped_layout_tests = lambda tests: set(skips)
         exp = TestExpectations(port, ['failures/expected/text.html'], lint)
 
@@ -532,7 +532,6 @@ class TestExpectationParserTests(unittest.TestCase):
         host = MockHost()
         test_port = host.port_factory.get('test-win-xp', None)
         test_port.test_exists = lambda test: True
-        test_config = test_port.test_configuration()
         full_test_list = []
         expectation_line = self._tokenize('')
         parser = TestExpectationParser(test_port, full_test_list, allow_rebaseline_modifier=False)
index 029094e..afea52e 100644 (file)
@@ -112,6 +112,7 @@ class TestFailure(object):
 class FailureTimeout(TestFailure):
     """Test timed out.  We also want to restart DumpRenderTree if this happens."""
     def __init__(self, is_reftest=False):
+        super(FailureTimeout, self).__init__()
         self.is_reftest = is_reftest
 
     def message(self):
@@ -124,6 +125,7 @@ class FailureTimeout(TestFailure):
 class FailureCrash(TestFailure):
     """DumpRenderTree/WebKitTestRunner crashed."""
     def __init__(self, is_reftest=False, process_name='DumpRenderTree', pid=None):
+        super(FailureCrash, self).__init__()
         self.process_name = process_name
         self.pid = pid
         self.is_reftest = is_reftest
@@ -168,6 +170,7 @@ class FailureMissingImage(TestFailure):
 class FailureImageHashMismatch(TestFailure):
     """Image hashes didn't match."""
     def __init__(self, diff_percent=0):
+        super(FailureImageHashMismatch, self).__init__()
         self.diff_percent = diff_percent
 
     def message(self):
@@ -185,6 +188,7 @@ class FailureReftestMismatch(TestFailure):
     """The result didn't match the reference rendering."""
 
     def __init__(self, reference_filename=None):
+        super(FailureReftestMismatch, self).__init__()
         self.reference_filename = reference_filename
         self.diff_percent = None
 
@@ -196,6 +200,7 @@ class FailureReftestMismatchDidNotOccur(TestFailure):
     """Unexpected match between the result and the reference rendering."""
 
     def __init__(self, reference_filename=None):
+        super(FailureReftestMismatchDidNotOccur, self).__init__()
         self.reference_filename = reference_filename
 
     def message(self):
@@ -206,6 +211,7 @@ class FailureReftestNoImagesGenerated(TestFailure):
     """Both the reftest and the -expected html file didn't generate pixel results."""
 
     def __init__(self, reference_filename=None):
+        super(FailureReftestNoImagesGenerated, self).__init__()
         self.reference_filename = reference_filename
 
     def message(self):
index 3b9ba33..e096b17 100644 (file)
@@ -45,10 +45,14 @@ class TestFailuresTest(unittest.TestCase):
 
     def test_unknown_failure_type(self):
         class UnknownFailure(TestFailure):
-            pass
+            def message(self):
+                return ''
 
         failure_obj = UnknownFailure()
         self.assertRaises(ValueError, determine_result_type, [failure_obj])
+
+    def test_message_is_virtual(self):
+        failure_obj = TestFailure()
         self.assertRaises(NotImplementedError, failure_obj.message)
 
     def test_loads(self):
index 51ac505..346d5a6 100644 (file)
@@ -35,8 +35,8 @@ class TestResult(object):
     """Data object containing the results of a single test."""
 
     @staticmethod
-    def loads(str):
-        return cPickle.loads(str)
+    def loads(string):
+        return cPickle.loads(string)
 
     def __init__(self, test_name, failures=None, test_run_time=None, has_stderr=False):
         self.test_name = test_name
@@ -54,9 +54,9 @@ class TestResult(object):
     def __ne__(self, other):
         return not (self == other)
 
-    def has_failure_matching_types(self, *args, **kargs):
+    def has_failure_matching_types(self, *failure_classes):
         for failure in self.failures:
-            if type(failure) in args:
+            if type(failure) in failure_classes:
                 return True
         return False
 
index dae778d..bdd0404 100644 (file)
@@ -85,13 +85,15 @@ load-plugins=
 # W0141: Used builtin function ''
 # W0212: Access to a protected member X of a client class
 # W0142: Used * or ** magic
+# W0401: Wildcard import X
 # W0402: Uses of a deprecated module 'string'
 # W0404: 41: Reimport 'XX' (imported line NN)
 # W0511: TODO
 # W0603: Using the global statement
+# W0614: Unused import X from wildcard import
 # W0703: Catch "Exception"
 # W1201: Specify string format arguments as logging function parameters
-disable=C0103,C0111,C0302,I0010,I0011,R0201,R0801,R0901,R0902,R0903,R0904,R0911,R0912,R0913,R0914,R0915,R0921,R0922,W0122,W0141,W0142,W0212,W0402,W0404,W0511,W0603,W0703,W1201
+disable=C0103,C0111,C0302,I0010,I0011,R0201,R0801,R0901,R0902,R0903,R0904,R0911,R0912,R0913,R0914,R0915,R0921,R0922,W0122,W0141,W0142,W0212,W0401,W0402,W0404,W0511,W0603,W0614,W0703,W1201
 
 
 [REPORTS]