Parsing test_expecations.txt + Skipped lists takes too long
authorojan@chromium.org <ojan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 30 Jan 2012 23:16:08 +0000 (23:16 +0000)
committerojan@chromium.org <ojan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 30 Jan 2012 23:16:08 +0000 (23:16 +0000)
https://bugs.webkit.org/show_bug.cgi?id=77059

Reviewed by Dirk Pranke.

This saves ~100ms on the Apple Mac port.
-memoize a bunch of path methods.
-Avoid doing multiple disk accesses per line.
-Parse the skipped list directly instead of turning it into a test_expecations.txt
formatting string and parsing that.

* Scripts/webkitpy/layout_tests/models/test_expectations.py:
(TestExpectationParser):
(TestExpectationParser.expectation_for_skipped_test):
(TestExpectationParser._parse_line):
(TestExpectationParser._collect_matching_tests):
(TestExpectations.__init__):
(TestExpectations._add_skipped_tests):
* Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py:
(test_add_skipped_tests):
(test_add_skipped_tests_duplicate):
* Scripts/webkitpy/layout_tests/port/base.py:
(Port):
(Port.test_isfile):
(Port.normalize_test_name):
(Port.layout_tests_dir):
(Port.abspath_for_test):
* Scripts/webkitpy/layout_tests/run_webkit_tests.py:

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

Tools/ChangeLog
Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py
Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py
Tools/Scripts/webkitpy/layout_tests/port/base.py

index 2d186760612effacacc754e08e1743736b2d188e..48bc56fdaed7431785bfa3fbd65169ae127b0af3 100644 (file)
@@ -1,3 +1,34 @@
+2012-01-30  Ojan Vafai  <ojan@chromium.org>
+
+        Parsing test_expecations.txt + Skipped lists takes too long
+        https://bugs.webkit.org/show_bug.cgi?id=77059
+
+        Reviewed by Dirk Pranke.
+
+        This saves ~100ms on the Apple Mac port.
+        -memoize a bunch of path methods.
+        -Avoid doing multiple disk accesses per line.
+        -Parse the skipped list directly instead of turning it into a test_expecations.txt
+        formatting string and parsing that.
+
+        * Scripts/webkitpy/layout_tests/models/test_expectations.py:
+        (TestExpectationParser):
+        (TestExpectationParser.expectation_for_skipped_test):
+        (TestExpectationParser._parse_line):
+        (TestExpectationParser._collect_matching_tests):
+        (TestExpectations.__init__):
+        (TestExpectations._add_skipped_tests):
+        * Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py:
+        (test_add_skipped_tests):
+        (test_add_skipped_tests_duplicate):
+        * Scripts/webkitpy/layout_tests/port/base.py:
+        (Port):
+        (Port.test_isfile):
+        (Port.normalize_test_name):
+        (Port.layout_tests_dir):
+        (Port.abspath_for_test):
+        * Scripts/webkitpy/layout_tests/run_webkit_tests.py:
+
 2012-01-30  Alexis Menard  <alexis.menard@openbossa.org>
 
         Unreviewed. Add myself to CSS, GStreamer, Qt related watchlists.
index 7fe73012c35bfc7dbadd9b427664a6b5ba1c9799..010ddd84872154286a4ee6c3c76af50bfcb10262 100644 (file)
@@ -184,9 +184,11 @@ class TestExpectationSerializer(object):
 class TestExpectationParser(object):
     """Provides parsing facilities for lines in the test_expectation.txt file."""
 
+    DUMMY_BUG_MODIFIER = "bug_dummy"
     BUG_MODIFIER_PREFIX = 'bug'
     BUG_MODIFIER_REGEX = 'bug\d+'
     REBASELINE_MODIFIER = 'rebaseline'
+    FAIL_EXPECTATION = 'fail'
     SKIP_MODIFIER = 'skip'
     SLOW_MODIFIER = 'slow'
     WONTFIX_MODIFIER = 'wontfix'
@@ -205,15 +207,30 @@ class TestExpectationParser(object):
             self._parse_line(expectation_line)
         return expectations
 
+    def expectation_for_skipped_test(self, test_name):
+        expectation_line = TestExpectationLine()
+        expectation_line.original_string = test_name
+        expectation_line.modifiers = [TestExpectationParser.DUMMY_BUG_MODIFIER, TestExpectationParser.SKIP_MODIFIER]
+        expectation_line.name = test_name
+        expectation_line.expectations = [TestExpectationParser.FAIL_EXPECTATION]
+        self._parse_line(expectation_line)
+        return expectation_line
+
     def _parse_line(self, expectation_line):
         if not expectation_line.name:
             return
 
         self._check_modifiers_against_expectations(expectation_line)
-        if self._check_path_does_not_exist(expectation_line):
+
+        expectation_line.is_file = self._port.test_isfile(expectation_line.name)
+        if not expectation_line.is_file and self._check_path_does_not_exist(expectation_line):
             return
 
-        expectation_line.path = self._port.normalize_test_name(expectation_line.name)
+        if expectation_line.is_file:
+            expectation_line.path = expectation_line.name
+        else:
+            expectation_line.path = self._port.normalize_test_name(expectation_line.name)
+
         self._collect_matching_tests(expectation_line)
 
         self._parse_modifiers(expectation_line)
@@ -285,7 +302,7 @@ class TestExpectationParser(object):
             expectation_line.matching_tests = [expectation_line.path]
             return
 
-        if self._port.test_isdir(expectation_line.path):
+        if not expectation_line.is_file:
             # this is a test category, return all the tests of the category.
             expectation_line.matching_tests = [test for test in self._full_test_list if test.startswith(expectation_line.path)]
             return
@@ -702,7 +719,6 @@ class TestExpectations(object):
         self._model = TestExpectationsModel()
         self._parser = TestExpectationParser(port, tests, is_lint_mode)
         self._port = port
-        self._test_configuration_converter = TestConfigurationConverter(port.all_test_configurations(), port.configuration_specifier_macros())
         self._skipped_tests_warnings = []
 
         self._expectations = self._parser.parse(expectations)
@@ -835,6 +851,5 @@ class TestExpectations(object):
         for index, test in enumerate(self._expectations, start=1):
             if test.name and test.name in tests_to_skip:
                 self._skipped_tests_warnings.append(':%d %s is also in a Skipped file.' % (index, test.name))
-        skipped_tests = '\n'.join(map(lambda test_path: 'BUG_SKIPPED SKIP : %s = FAIL' % test_path, tests_to_skip))
-        for test in self._parser.parse(skipped_tests):
-            self._model.add_expectation_line(test, overrides_allowed=True)
+        for test_name in tests_to_skip:
+            self._model.add_expectation_line(self._parser.expectation_for_skipped_test(test_name), overrides_allowed=True)
index a294f3626a3c70920cd542f1221985291a437063..98b91aeed1f0df7c1a9dd2963cc593371c2af857 100644 (file)
@@ -273,7 +273,15 @@ BUG_OVERRIDE : failures/expected/text.html = CRASH
         port = MockHost().port_factory.get('qt')
         port._filesystem.files[port._filesystem.join(port.layout_tests_dir(), 'platform/qt/Skipped')] = 'failures/expected/text.html'
         port._filesystem.files[port._filesystem.join(port.layout_tests_dir(), 'failures/expected/text.html')] = 'foo'
-        self.assertRaises(ParseError, TestExpectations, port, 'failures/expected/text.html\n', 'BUGX : failures/expected/text.html = text\n', None, True)
+        expectations = TestExpectations(port, tests=['failures/expected/text.html'], expectations='', test_config=port.test_configuration())
+        self.assertEquals(expectations.get_modifiers('failures/expected/text.html'), [TestExpectationParser.DUMMY_BUG_MODIFIER, TestExpectationParser.SKIP_MODIFIER])
+        self.assertEquals(expectations.get_expectations('failures/expected/text.html'), set([FAIL]))
+
+    def test_add_skipped_tests_duplicate(self):
+        port = MockHost().port_factory.get('qt')
+        port._filesystem.files[port._filesystem.join(port.layout_tests_dir(), 'platform/qt/Skipped')] = 'failures/expected/text.html'
+        port._filesystem.files[port._filesystem.join(port.layout_tests_dir(), 'failures/expected/text.html')] = 'foo'
+        self.assertRaises(ParseError, TestExpectations, port, tests=['failures/expected/text.html'], expectations='BUGX : failures/expected/text.html = text\n', test_config=port.test_configuration(), is_lint_mode=True)
 
 
 class ExpectationSyntaxTests(Base):
index edd6166090ecb2a00d1d212b3afab9ecf457db9d..a8b98fda2c356f5403f7c562a94b6792e833d2b4 100755 (executable)
@@ -518,6 +518,14 @@ class Port(object):
         return filter(lambda x: self._filesystem.isdir(self._filesystem.join(layout_tests_dir, x)),
                       self._filesystem.listdir(layout_tests_dir))
 
+    @memoized
+    def test_isfile(self, test_name):
+        """Return True if the test name refers to a directory of tests."""
+        # Used by test_expectations.py to apply rules to whole directories.
+        test_path = self.abspath_for_test(test_name)
+        return self._filesystem.isfile(test_path)
+
+    @memoized
     def test_isdir(self, test_name):
         """Return True if the test name refers to a directory of tests."""
         # Used by test_expectations.py to apply rules to whole directories.
@@ -540,7 +548,9 @@ class Port(object):
 
     def normalize_test_name(self, test_name):
         """Returns a normalized version of the test name or test directory."""
-        if self.test_isdir(test_name) and not test_name.endswith('/'):
+        if test_name.endswith('/'):
+            return test_name
+        if self.test_isdir(test_name):
             return test_name + '/'
         return test_name
 
@@ -560,9 +570,10 @@ class Port(object):
         """
         self._filesystem.write_binary_file(baseline_path, data)
 
+    @memoized
     def layout_tests_dir(self):
         """Return the absolute path to the top of the LayoutTests directory."""
-        return self.path_from_webkit_base('LayoutTests')
+        return self._filesystem.normpath(self.path_from_webkit_base('LayoutTests'))
 
     def perf_tests_dir(self):
         """Return the absolute path to the top of the PerformanceTests directory."""
@@ -697,10 +708,11 @@ class Port(object):
         assert filename.startswith(self.perf_tests_dir()), "%s did not start with %s" % (filename, self.perf_tests_dir())
         return filename[len(self.perf_tests_dir()) + 1:]
 
+    @memoized
     def abspath_for_test(self, test_name):
         """Returns the full path to the file for a given test name. This is the
         inverse of relative_test_filename()."""
-        return self._filesystem.normpath(self._filesystem.join(self.layout_tests_dir(), test_name))
+        return self._filesystem.join(self.layout_tests_dir(), test_name)
 
     def results_directory(self):
         """Absolute path to the place to store the test results (uses --results-directory)."""