From: ojan@chromium.org Date: Mon, 30 Jan 2012 23:16:08 +0000 (+0000) Subject: Parsing test_expecations.txt + Skipped lists takes too long X-Git-Tag: 070512121124~14201 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=c1c1bea6dcd1a0c635e36b579e83da906ab93d04;p=profile%2Fivi%2Fwebkit-efl.git 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: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@106293 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- diff --git a/Tools/ChangeLog b/Tools/ChangeLog index 2d18676..48bc56fd 100644 --- a/Tools/ChangeLog +++ b/Tools/ChangeLog @@ -1,3 +1,34 @@ +2012-01-30 Ojan Vafai + + 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 Unreviewed. Add myself to CSS, GStreamer, Qt related watchlists. diff --git a/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py b/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py index 7fe7301..010ddd8 100644 --- a/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py +++ b/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py @@ -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) 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 a294f36..98b91ae 100644 --- a/Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py +++ b/Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py @@ -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): diff --git a/Tools/Scripts/webkitpy/layout_tests/port/base.py b/Tools/Scripts/webkitpy/layout_tests/port/base.py index edd6166..a8b98fd 100755 --- a/Tools/Scripts/webkitpy/layout_tests/port/base.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/base.py @@ -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)."""