Refactor TestExpectationsParser in preparation for caching the results

https://bugs.webkit.org/show_bug.cgi?id=76669

Reviewed by Dimitri Glazkov.

Make everything private expect for the parse method.
Eventually, we'll need the expectations lines to not be modified
outside of TestExpectationsParser so we can cache the results.
This makes check-webkit-style of the chromium test_expectations.txt file
go from ~17 seconds to ~12 seconds on my Mac Pro.

This patch is just a refactor in preparation, so no new tests.

* Scripts/webkitpy/layout_tests/controllers/test_expectations_editor_unittest.py:
(TestExpectationEditorTests.make_parsed_expectation_lines):
* Scripts/webkitpy/layout_tests/models/test_expectations.py:
(TestExpectationParser.parse):
(TestExpectationParser):
(TestExpectationParser._parse_line):
(TestExpectationParser._tokenize):
(TestExpectationParser._tokenize_list):
(TestExpectationsModel._clear_expectations_for_test):
(TestExpectations.__init__):
(TestExpectations._add_expectations):
(TestExpectations._add_skipped_tests):
* Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py:
(TestExpectationParserTests.test_tokenize_blank):
(TestExpectationParserTests.test_tokenize_missing_colon):
(TestExpectationParserTests.test_tokenize_extra_colon):
(TestExpectationParserTests.test_tokenize_empty_comment):
(TestExpectationParserTests.test_tokenize_comment):
(TestExpectationParserTests.test_tokenize_missing_equal):
(TestExpectationParserTests.test_tokenize_extra_equal):
(TestExpectationParserTests.test_tokenize_valid):
(TestExpectationParserTests.test_tokenize_valid_with_comment):
(TestExpectationParserTests.test_tokenize_valid_with_multiple_modifiers):
(TestExpectationParserTests.test_parse_empty_string):
(TestExpectationSerializerTests.assert_round_trip):
(TestExpectationSerializerTests.assert_list_round_trip):
* Scripts/webkitpy/tool/commands/expectations.py:
(OptimizeExpectations.execute):
* Scripts/webkitpy/tool/servers/gardeningserver.py:
(GardeningExpectationsUpdater.update_expectations):

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

diff --git a/Tools/ChangeLog b/Tools/ChangeLog
index 771b25e..f9427c1 100644 (file)
--- a/Tools/ChangeLog
+++ b/Tools/ChangeLog
@@ -1,3 +1,49 @@
+2012-01-19  Ojan Vafai  <ojan@chromium.org>
+
+        Refactor TestExpectationsParser in preparation for caching the results
+        https://bugs.webkit.org/show_bug.cgi?id=76669
+
+        Reviewed by Dimitri Glazkov.
+
+        Make everything private expect for the parse method. 
+        Eventually, we'll need the expectations lines to not be modified
+        outside of TestExpectationsParser so we can cache the results.
+        This makes check-webkit-style of the chromium test_expectations.txt file
+        go from ~17 seconds to ~12 seconds on my Mac Pro.
+
+        This patch is just a refactor in preparation, so no new tests.
+
+        * Scripts/webkitpy/layout_tests/controllers/test_expectations_editor_unittest.py:
+        (TestExpectationEditorTests.make_parsed_expectation_lines):
+        * Scripts/webkitpy/layout_tests/models/test_expectations.py:
+        (TestExpectationParser.parse):
+        (TestExpectationParser):
+        (TestExpectationParser._parse_line):
+        (TestExpectationParser._tokenize):
+        (TestExpectationParser._tokenize_list):
+        (TestExpectationsModel._clear_expectations_for_test):
+        (TestExpectations.__init__):
+        (TestExpectations._add_expectations):
+        (TestExpectations._add_skipped_tests):
+        * Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py:
+        (TestExpectationParserTests.test_tokenize_blank):
+        (TestExpectationParserTests.test_tokenize_missing_colon):
+        (TestExpectationParserTests.test_tokenize_extra_colon):
+        (TestExpectationParserTests.test_tokenize_empty_comment):
+        (TestExpectationParserTests.test_tokenize_comment):
+        (TestExpectationParserTests.test_tokenize_missing_equal):
+        (TestExpectationParserTests.test_tokenize_extra_equal):
+        (TestExpectationParserTests.test_tokenize_valid):
+        (TestExpectationParserTests.test_tokenize_valid_with_comment):
+        (TestExpectationParserTests.test_tokenize_valid_with_multiple_modifiers):
+        (TestExpectationParserTests.test_parse_empty_string):
+        (TestExpectationSerializerTests.assert_round_trip):
+        (TestExpectationSerializerTests.assert_list_round_trip):
+        * Scripts/webkitpy/tool/commands/expectations.py:
+        (OptimizeExpectations.execute):
+        * Scripts/webkitpy/tool/servers/gardeningserver.py:
+        (GardeningExpectationsUpdater.update_expectations):
+
 2012-01-20  Adam Barth  <abarth@webkit.org>
 
         Follow-up to previous patch: don't produce NaN when the revision number

diff --git a/Tools/Scripts/webkitpy/layout_tests/controllers/test_expectations_editor_unittest.py b/Tools/Scripts/webkitpy/layout_tests/controllers/test_expectations_editor_unittest.py
index f365d77..58a1835 100644 (file)
--- a/Tools/Scripts/webkitpy/layout_tests/controllers/test_expectations_editor_unittest.py
+++ b/Tools/Scripts/webkitpy/layout_tests/controllers/test_expectations_editor_unittest.py
@@ -76,11 +76,10 @@ class TestExpectationEditorTests(unittest.TestCase):
         unittest.TestCase.__init__(self, testFunc)
 
     def make_parsed_expectation_lines(self, in_string):
-        expectation_lines = TestExpectationParser.tokenize_list(in_string)
         parser = TestExpectationParser(self.test_port, self.full_test_list, allow_rebaseline_modifier=False)
+        expectation_lines = parser.parse(in_string)
         for expectation_line in expectation_lines:
             self.assertFalse(expectation_line.is_invalid())
-            parser.parse(expectation_line)
         return expectation_lines
 
     def assert_remove_roundtrip(self, in_string, test, expected_string, remove_flakes=False):

diff --git a/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py b/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py
index cf97b34..f9e0ff7 100644 (file)
--- a/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py
+++ b/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py
@@ -199,7 +199,13 @@ class TestExpectationParser(object):
         self._full_test_list = full_test_list
         self._allow_rebaseline_modifier = allow_rebaseline_modifier
 
-    def parse(self, expectation_line):
+    def parse(self, expectations_string):
+        expectations = TestExpectationParser._tokenize_list(expectations_string)
+        for expectation_line in expectations:
+            self._parse_line(expectation_line)
+        return expectations
+
+    def _parse_line(self, expectation_line):
         if not expectation_line.name:
             return
 
@@ -288,7 +294,7 @@ class TestExpectationParser(object):
             expectation_line.matching_tests.append(expectation_line.path)
 
     @classmethod
-    def tokenize(cls, expectation_string, line_number=None):
+    def _tokenize(cls, expectation_string, line_number=None):
         """Tokenizes a line from test_expectations.txt and returns an unparsed TestExpectationLine instance.
 
         The format of a test expectation line is:
@@ -327,13 +333,13 @@ class TestExpectationParser(object):
         return expectation_line
 
     @classmethod
-    def tokenize_list(cls, expectations_string):
+    def _tokenize_list(cls, expectations_string):
         """Returns a list of TestExpectationLines, one for each line in expectations_string."""
         expectation_lines = []
         line_number = 0
         for line in expectations_string.split("\n"):
             line_number += 1
-            expectation_lines.append(cls.tokenize(line, line_number))
+            expectation_lines.append(cls._tokenize(line, line_number))
         return expectation_lines
 
     @classmethod
@@ -514,8 +520,6 @@ class TestExpectationsModel(object):
             self._remove_from_sets(test, self._timeline_to_tests)
             self._remove_from_sets(test, self._result_type_to_tests)
 
-        self._test_to_expectation_line[test] = expectation_line
-
     def _remove_from_sets(self, test, dict):
         """Removes the given test from the sets in the dictionary.
 
@@ -699,12 +703,12 @@ class TestExpectations(object):
         self._test_configuration_converter = TestConfigurationConverter(port.all_test_configurations(), port.configuration_specifier_macros())
         self._skipped_tests_warnings = []
 
-        self._expectations = TestExpectationParser.tokenize_list(expectations)
+        self._expectations = self._parser.parse(expectations)
         self._add_expectations(self._expectations, overrides_allowed=False)
         self._add_skipped_tests(port.skipped_tests())
 
         if overrides:
-            overrides_expectations = TestExpectationParser.tokenize_list(overrides)
+            overrides_expectations = self._parser.parse(overrides)
             self._add_expectations(overrides_expectations, overrides_allowed=True)
             self._expectations += overrides_expectations
 
@@ -826,7 +830,6 @@ class TestExpectations(object):
             if not expectation_line.expectations:
                 continue
 
-            self._parser.parse(expectation_line)
             if self._test_config in expectation_line.matching_configurations:
                 self._model.add_expectation_line(expectation_line, overrides_allowed)
 
@@ -838,6 +841,5 @@ class TestExpectations(object):
                 self._skipped_tests_warnings.append('The %s test from test_expectations.txt in line %d is also in Skipped!' %
                             (test.name, index))
         skipped_tests = '\n'.join(map(lambda test_path: 'BUG_SKIPPED SKIP : %s = FAIL' % test_path, tests_to_skip))
-        for test in TestExpectationParser.tokenize_list(skipped_tests):
-            self._parser.parse(test)
+        for test in self._parser.parse(skipped_tests):
             self._model.add_expectation_line(test, overrides_allowed=True)

diff --git a/Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py b/Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py
index 6473053..ba687ab 100644 (file)
--- a/Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py
+++ b/Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py
@@ -400,51 +400,51 @@ class RebaseliningTest(Base):
 
 class TestExpectationParserTests(unittest.TestCase):
     def test_tokenize_blank(self):
-        expectation = TestExpectationParser.tokenize('')
+        expectation = TestExpectationParser._tokenize('')
         self.assertEqual(expectation.is_malformed(), False)
         self.assertEqual(expectation.comment, None)
         self.assertEqual(len(expectation.errors), 0)
 
     def test_tokenize_missing_colon(self):
-        expectation = TestExpectationParser.tokenize('Qux.')
+        expectation = TestExpectationParser._tokenize('Qux.')
         self.assertEqual(expectation.is_malformed(), True)
         self.assertEqual(str(expectation.errors), '["Missing a \':\'"]')
 
     def test_tokenize_extra_colon(self):
-        expectation = TestExpectationParser.tokenize('FOO : : bar')
+        expectation = TestExpectationParser._tokenize('FOO : : bar')
         self.assertEqual(expectation.is_malformed(), True)
         self.assertEqual(str(expectation.errors), '["Extraneous \':\'"]')
 
     def test_tokenize_empty_comment(self):
-        expectation = TestExpectationParser.tokenize('//')
+        expectation = TestExpectationParser._tokenize('//')
         self.assertEqual(expectation.is_malformed(), False)
         self.assertEqual(expectation.comment, '')
         self.assertEqual(len(expectation.errors), 0)
 
     def test_tokenize_comment(self):
-        expectation = TestExpectationParser.tokenize('//Qux.')
+        expectation = TestExpectationParser._tokenize('//Qux.')
         self.assertEqual(expectation.is_malformed(), False)
         self.assertEqual(expectation.comment, 'Qux.')
         self.assertEqual(len(expectation.errors), 0)
 
     def test_tokenize_missing_equal(self):
-        expectation = TestExpectationParser.tokenize('FOO : bar')
+        expectation = TestExpectationParser._tokenize('FOO : bar')
         self.assertEqual(expectation.is_malformed(), True)
         self.assertEqual(str(expectation.errors), "['Missing expectations\']")
 
     def test_tokenize_extra_equal(self):
-        expectation = TestExpectationParser.tokenize('FOO : bar = BAZ = Qux.')
+        expectation = TestExpectationParser._tokenize('FOO : bar = BAZ = Qux.')
         self.assertEqual(expectation.is_malformed(), True)
         self.assertEqual(str(expectation.errors), '["Extraneous \'=\'"]')
 
     def test_tokenize_valid(self):
-        expectation = TestExpectationParser.tokenize('FOO : bar = BAZ')
+        expectation = TestExpectationParser._tokenize('FOO : bar = BAZ')
         self.assertEqual(expectation.is_malformed(), False)
         self.assertEqual(expectation.comment, None)
         self.assertEqual(len(expectation.errors), 0)
 
     def test_tokenize_valid_with_comment(self):
-        expectation = TestExpectationParser.tokenize('FOO : bar = BAZ //Qux.')
+        expectation = TestExpectationParser._tokenize('FOO : bar = BAZ //Qux.')
         self.assertEqual(expectation.is_malformed(), False)
         self.assertEqual(expectation.comment, 'Qux.')
         self.assertEqual(str(expectation.modifiers), '[\'foo\']')
@@ -452,7 +452,7 @@ class TestExpectationParserTests(unittest.TestCase):
         self.assertEqual(len(expectation.errors), 0)
 
     def test_tokenize_valid_with_multiple_modifiers(self):
-        expectation = TestExpectationParser.tokenize('FOO1 FOO2 : bar = BAZ //Qux.')
+        expectation = TestExpectationParser._tokenize('FOO1 FOO2 : bar = BAZ //Qux.')
         self.assertEqual(expectation.is_malformed(), False)
         self.assertEqual(expectation.comment, 'Qux.')
         self.assertEqual(str(expectation.modifiers), '[\'foo1\', \'foo2\']')
@@ -465,9 +465,9 @@ class TestExpectationParserTests(unittest.TestCase):
         test_port.test_exists = lambda test: True
         test_config = test_port.test_configuration()
         full_test_list = []
-        expectation_line = TestExpectationParser.tokenize('')
+        expectation_line = TestExpectationParser._tokenize('')
         parser = TestExpectationParser(test_port, full_test_list, allow_rebaseline_modifier=False)
-        parser.parse(expectation_line)
+        parser._parse_line(expectation_line)
         self.assertFalse(expectation_line.is_invalid())
 
 
@@ -480,13 +480,13 @@ class TestExpectationSerializerTests(unittest.TestCase):
         unittest.TestCase.__init__(self, testFunc)
 
     def assert_round_trip(self, in_string, expected_string=None):
-        expectation = TestExpectationParser.tokenize(in_string)
+        expectation = TestExpectationParser._tokenize(in_string)
         if expected_string is None:
             expected_string = in_string
         self.assertEqual(expected_string, self._serializer.to_string(expectation))
 
     def assert_list_round_trip(self, in_string, expected_string=None):
-        expectations = TestExpectationParser.tokenize_list(in_string)
+        expectations = TestExpectationParser._tokenize_list(in_string)
         if expected_string is None:
             expected_string = in_string
         self.assertEqual(expected_string, TestExpectationSerializer.list_to_string(expectations, self._converter))

diff --git a/Tools/Scripts/webkitpy/tool/commands/expectations.py b/Tools/Scripts/webkitpy/tool/commands/expectations.py
index 575e80c..8af155c 100644 (file)
--- a/Tools/Scripts/webkitpy/tool/commands/expectations.py
+++ b/Tools/Scripts/webkitpy/tool/commands/expectations.py
@@ -37,9 +37,7 @@ class OptimizeExpectations(AbstractDeclarativeCommand):
 
     def execute(self, options, args, tool):
         port = tool.port_factory.get("chromium-win-win7")  # FIXME: This should be selectable.
-        expectation_lines = TestExpectationParser.tokenize_list(port.test_expectations())
         parser = TestExpectationParser(port, [], allow_rebaseline_modifier=False)
-        for expectation_line in expectation_lines:
-            parser.parse(expectation_line)
+        expectation_lines = parser.parse(port.test_expectations())
         converter = TestConfigurationConverter(port.all_test_configurations(), port.configuration_specifier_macros())
         tool.filesystem.write_text_file(port.path_to_test_expectations_file(), TestExpectationSerializer.list_to_string(expectation_lines, converter))

diff --git a/Tools/Scripts/webkitpy/tool/servers/gardeningserver.py b/Tools/Scripts/webkitpy/tool/servers/gardeningserver.py
index 425d1ce..b090af9 100644 (file)
--- a/Tools/Scripts/webkitpy/tool/servers/gardeningserver.py
+++ b/Tools/Scripts/webkitpy/tool/servers/gardeningserver.py
@@ -64,9 +64,7 @@ class GardeningExpectationsUpdater(BugManager):
         return "BUG_NEW"
 
     def update_expectations(self, failure_info_list):
-        expectation_lines = TestExpectationParser.tokenize_list(self._tool.filesystem.read_text_file(self._path_to_test_expectations_file))
-        for expectation_line in expectation_lines:
-            self._parser.parse(expectation_line)
+        expectation_lines = self._parser.parse(self._tool.filesystem.read_text_file(self._path_to_test_expectations_file))
         editor = TestExpectationsEditor(expectation_lines, self)
         updated_expectation_lines = []
         # FIXME: Group failures by testName+failureTypeList.