+2012-01-25 Dirk Pranke <dpranke@chromium.org>
+
+ webkitpy: print nicer errors while linting expectations files, remove redundant tests
+ https://bugs.webkit.org/show_bug.cgi?id=76955
+
+ Reviewed by Adam Barth.
+
+ This change cleans up the errors that are printed so that the
+ are more "quickfix" compatible (path:lineno).
+
+ Also, there were a bunch of redundant tests between
+ layout_tests.models.test_expectations_unittest and
+ style.checkers.test_expectations_unittest. The latter should
+ just expect the former to do most of the testing.
+
+ * Scripts/webkitpy/layout_tests/models/test_expectations.py:
+ (TestExpectationParser._parse_modifiers):
+ (TestExpectations._report_errors):
+ (TestExpectations._add_skipped_tests):
+ * Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py:
+ (test_parse_error_nonfatal):
+ (SemanticTests.test_bad_bugid):
+ * Scripts/webkitpy/style/checkers/test_expectations.py:
+ (TestExpectationsChecker.__init__):
+ * Scripts/webkitpy/style/checkers/test_expectations_unittest.py:
+ (TestExpectationsTestCase.assert_lines_lint):
+ (TestExpectationsTestCase.test_valid_expectations):
+ (TestExpectationsTestCase.test_invalid_expectations):
+ (TestExpectationsTestCase.test_tab):
+
2012-01-25 Eric Seidel <eric@webkit.org>
webkit-patch apply-* should always continue after failures
def _parse_modifiers(self, expectation_line):
has_wontfix = False
+ has_bugid = False
parsed_specifiers = set()
for modifier in expectation_line.modifiers:
if modifier in TestExpectations.MODIFIERS:
if modifier == self.WONTFIX_MODIFIER:
has_wontfix = True
elif modifier.startswith(self.BUG_MODIFIER_PREFIX):
+ has_bugid = True
if re.match(self.BUG_MODIFIER_REGEX, modifier):
expectation_line.errors.append('BUG\d+ is not allowed, must be one of BUGCR\d+, BUGWK\d+, BUGV8_\d+, or a non-numeric bug identifier.')
else:
else:
parsed_specifiers.add(modifier)
- if not expectation_line.parsed_bug_modifiers and not has_wontfix:
+ if not expectation_line.parsed_bug_modifiers and not has_wontfix and not has_bugid:
expectation_line.warnings.append('Test lacks BUG modifier.')
if self._allow_rebaseline_modifier and self.REBASELINE_MODIFIER in expectation_line.modifiers:
def _report_errors(self):
errors = []
warnings = []
+ test_expectation_path = self._port.path_to_test_expectations_file()
+ if test_expectation_path.startswith(self._port.path_from_webkit_base()):
+ test_expectation_path = self._port.host.filesystem.relpath(test_expectation_path, self._port.path_from_webkit_base())
for expectation in self._expectations:
for error in expectation.errors:
- errors.append("Line:%s %s %s" % (expectation.line_number, error, expectation.name if expectation.expectations else expectation.original_string))
+ errors.append('%s:%d %s %s' % (test_expectation_path, expectation.line_number, error, expectation.name if expectation.expectations else expectation.original_string))
for warning in expectation.warnings:
- warnings.append("Line:%s %s %s" % (expectation.line_number, warning, expectation.name if expectation.expectations else expectation.original_string))
+ warnings.append('%s:%d %s %s' % (test_expectation_path, expectation.line_number, warning, expectation.name if expectation.expectations else expectation.original_string))
for warning in self._skipped_tests_warnings:
- warnings.append(warning)
+ warnings.append('%s%s' % (test_expectation_path, warning))
if errors or warnings:
- test_expectation_path = self._port.path_to_test_expectations_file()
- failure_title = "FAILURES IN %s" % test_expectation_path
-
if errors:
- raise ParseError(fatal=True, errors=[failure_title] + errors)
+ raise ParseError(fatal=True, errors=sorted(errors + warnings))
if warnings:
self._has_warnings = True
if self._is_lint_mode:
- raise ParseError(fatal=False, errors=[failure_title] + warnings)
+ raise ParseError(fatal=False, errors=warnings)
def _process_tests_without_expectations(self):
if self._full_test_list:
return
for index, test in enumerate(self._expectations, start=1):
if test.name and test.name in tests_to_skip:
- self._skipped_tests_warnings.append('The %s test from test_expectations.txt in line %d is also in Skipped!' %
- (test.name, index))
+ 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)
self.assertFalse(True, "ParseError wasn't raised")
except ParseError, e:
self.assertTrue(e.fatal)
- exp_errors = [u"FAILURES IN %s" % self._port.path_to_test_expectations_file(),
- u"Line:1 Unrecognized modifier 'foo' failures/expected/text.html",
- u"Line:2 Missing expectations SKIP : failures/expected/image.html"]
- self.assertEqual(str(e), '\n'.join(map(str, exp_errors)))
- self.assertEqual(e.errors, exp_errors)
+ exp_errors = [u":1 Test lacks BUG modifier. failures/expected/text.html",
+ u":1 Unrecognized modifier 'foo' failures/expected/text.html",
+ u":2 Missing expectations SKIP : failures/expected/image.html"]
+ self.assertEqual(str(e), '\n'.join(self._port.path_to_test_expectations_file() + str(error) for error in exp_errors))
def test_parse_error_nonfatal(self):
try:
self.assertFalse(True, "ParseError wasn't raised")
except ParseError, e:
self.assertFalse(e.fatal)
- exp_errors = [u'FAILURES IN %s' % self._port.path_to_test_expectations_file(),
- u'Line:1 Test lacks BUG modifier. failures/expected/text.html']
- self.assertEqual(str(e), '\n'.join(map(str, exp_errors)))
- self.assertEqual(e.errors, exp_errors)
+ exp_errors = [u':1 Test lacks BUG modifier. failures/expected/text.html']
+ self.assertEqual(str(e), '\n'.join(self._port.path_to_test_expectations_file() + str(error) for error in exp_errors))
def test_error_on_different_platform(self):
# parse_exp uses a Windows port. Assert errors on Mac show up in lint mode.
def test_bug_format(self):
self.assertRaises(ParseError, self.parse_exp, 'BUG1234 : failures/expected/text.html = TEXT')
+ def test_bad_bugid(self):
+ try:
+ self.parse_exp('BUG1234 SLOW : failures/expected/text.html = TEXT')
+ self.fail('should have raised an error about a bad bug identifier')
+ except ParseError, exp:
+ self.assertEquals(exp.fatal, True)
+ self.assertEquals(len(exp.errors), 1)
+
def test_missing_bugid(self):
# This should log a non-fatal error.
self.parse_exp('SLOW : failures/expected/text.html = TEXT')
self._handle_style_error = handle_style_error
self._handle_style_error.turn_off_line_filtering()
self._tab_checker = TabChecker(file_path, handle_style_error)
- self._output_regex = re.compile('Line:(?P<line>\d+)\s*(?P<message>.+)')
+ self._output_regex = re.compile('.*test_expectations.txt:(?P<line>\d+)\s*(?P<message>.+)')
# FIXME: host should be a required parameter, not an optional one.
host = host or Host()
self._expect_port_for_expectations_path(None, "/")
self._expect_port_for_expectations_path("ChromiumMacPort", "/mock-checkout/LayoutTests/chromium-mac/test_expectations.txt")
- def assert_lines_lint(self, lines, expected):
+ def assert_lines_lint(self, lines, should_pass, expected_output=None):
self._error_collector.reset_errors()
checker = TestExpectationsChecker('test/test_expectations.txt',
self._error_collector, host=MockHost())
tests=[self._test_file],
overrides=None)
checker.check_tabs(lines)
- self.assertEqual(expected, self._error_collector.get_errors())
+ if should_pass:
+ self.assertEqual('', self._error_collector.get_errors())
+ elif expected_output:
+ self.assertEquals(expected_output, self._error_collector.get_errors())
+ else:
+ self.assertNotEquals('', self._error_collector.get_errors())
self.assertTrue(self._error_collector.turned_off_filtering)
def test_valid_expectations(self):
- self.assert_lines_lint(
- ["BUGCR1234 MAC : passes/text.html = PASS FAIL"],
- "")
- self.assert_lines_lint(
- ["SKIP BUGCR1234 : passes/text.html = TIMEOUT PASS"],
- "")
- self.assert_lines_lint(
- ["BUGCR1234 DEBUG : passes/text.html = TIMEOUT PASS"],
- "")
- self.assert_lines_lint(
- ["BUGCR1234 DEBUG SKIP : passes/text.html = TIMEOUT PASS"],
- "")
- self.assert_lines_lint(
- ["BUGCR1234 MAC DEBUG SKIP : passes/text.html = TIMEOUT PASS"],
- "")
- self.assert_lines_lint(
- ["BUGCR1234 DEBUG MAC : passes/text.html = TIMEOUT PASS"],
- "")
- self.assert_lines_lint(
- ["SLOW BUGCR1234 : passes/text.html = PASS"],
- "")
- self.assert_lines_lint(
- ["WONTFIX SKIP : passes/text.html = TIMEOUT"],
- "")
-
- def test_modifier_errors(self):
- self.assert_lines_lint(
- ["BUG1234 : passes/text.html = FAIL"],
- "BUG\\d+ is not allowed, must be one of BUGCR\\d+, BUGWK\\d+, BUGV8_\\d+, or a non-numeric bug identifier. passes/text.html [test/expectations] [5]")
-
- def test_valid_modifiers(self):
- self.assert_lines_lint(
- ["INVALID-MODIFIER : passes/text.html = PASS"],
- "Unrecognized modifier 'invalid-modifier' "
- "passes/text.html [test/expectations] [5]")
- self.assert_lines_lint(
- ["SKIP : passes/text.html = PASS"],
- "Test lacks BUG modifier. "
- "passes/text.html [test/expectations] [2]")
-
- def test_expectation_errors(self):
- self.assert_lines_lint(
- ["missing expectations"],
- "Missing a ':' missing expectations [test/expectations] [5]")
- self.assert_lines_lint(
- ["SLOW : passes/text.html = TIMEOUT"],
- "A test can not be both SLOW and TIMEOUT. "
- "If it times out indefinitely, then it should be just TIMEOUT. "
- "passes/text.html [test/expectations] [5]")
- self.assert_lines_lint(
- ["BUGWK1 : does/not/exist.html = FAIL"],
- "Path does not exist. does/not/exist.html [test/expectations] [2]")
-
- def test_parse_expectations(self):
- self.assert_lines_lint(
- ["BUGWK1 : passes/text.html = PASS"],
- "")
- self.assert_lines_lint(
- ["BUGWK1 : passes/text.html = UNSUPPORTED"],
- "Unsupported expectation: unsupported "
- "passes/text.html [test/expectations] [5]")
- self.assert_lines_lint(
- ["BUGWK1 : passes/text.html = PASS UNSUPPORTED"],
- "Unsupported expectation: unsupported "
- "passes/text.html [test/expectations] [5]")
-
- def test_already_seen_test(self):
- self.assert_lines_lint(
- ["BUGWK1 : passes/text.html = PASS",
- "BUGWK1 : passes/text.html = TIMEOUT"],
- "Duplicate or ambiguous expectation. %s [test/expectations] [5]" % self._test_file)
-
- self.assert_lines_lint(
- ["BUGWK1 LEOPARD : passes/text.html = PASS",
- "BUGWK1 MAC : passes/text.html = TIMEOUT"],
- "More specific entry on line 1 overrides line 2 passes/text.html [test/expectations] [5]")
-
- self.assert_lines_lint(
- ["BUGWK1 LEOPARD : passes/text.html = PASS",
- "BUGWK1 LEOPARD RELEASE : passes/text.html = TIMEOUT"],
- "More specific entry on line 2 overrides line 1 passes/text.html [test/expectations] [5]")
-
- self.assert_lines_lint(
- ["BUGWK1 RELEASE : passes/text.html = PASS",
- "BUGWK1 CPU : passes/text.html = TIMEOUT"],
- "Entries on line 1 and line 2 match overlapping sets of configurations passes/text.html [test/expectations] [5]")
-
- self.assert_lines_lint(
- ["BUGWK1 WIN : passes/text.html = PASS",
- "BUGWK1 MAC : passes/text.html = TIMEOUT"],
- "")
-
- self.assert_lines_lint(
- ["BUGWK1 LEOPARD DEBUG : passes/text.html = PASS",
- "BUGWK1 LEOPARD RELEASE : passes/text.html = TIMEOUT"],
- "")
+ self.assert_lines_lint(["BUGCR1234 MAC : passes/text.html = PASS FAIL"], should_pass=True)
+ def test_invalid_expectations(self):
+ self.assert_lines_lint(["BUG1234 : passes/text.html = GIVE UP"], should_pass=False)
def test_tab(self):
- self.assert_lines_lint(
- ["\tBUGWK1 : passes/text.html = PASS"],
- "Line contains tab character. [whitespace/tab] [5]")
+ self.assert_lines_lint(["\tBUGWK1 : passes/text.html = PASS"], should_pass=False, expected_output="Line contains tab character. [whitespace/tab] [5]")
projects / profile / ivi / webkit-efl.git / commitdiff