Diff
Modified: trunk/Tools/ChangeLog (129183 => 129184)
--- trunk/Tools/ChangeLog 2012-09-21 01:24:03 UTC (rev 129183)
+++ trunk/Tools/ChangeLog 2012-09-21 01:33:38 UTC (rev 129184)
@@ -1,3 +1,50 @@
+2012-09-20 Dirk Pranke <[email protected]>
+
+ make Skip, WontFix be the only expectations on a line
+ https://bugs.webkit.org/show_bug.cgi?id=97225
+
+ Reviewed by Ojan Vafai.
+
+ It is now incorrect in the new syntax to have a line like:
+
+ foo.html [ WontFix Crash ]
+
+ This will generate a lint warning and be treated as an invalid
+ line. Fixing this caused a whole bunch of unit tests to need updating
+ to no longer be marked as WontFix :). Also, this patch adjusts
+ the warnings so that missing Bug() identifiers will cause lint
+ warnings but will *not* cause the line to be treated as invalid.
+ Fixing these issues also revealed that test_hung_thread was no
+ longer testing the right logic, so I adjusted the timeouts in
+ test.py to make that test work again.
+
+ * Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:
+ (Worker._run_test_in_another_thread):
+ * Scripts/webkitpy/layout_tests/controllers/manager_unittest.py:
+ (ResultSummaryTest.test_summarized_results_wontfix):
+ * Scripts/webkitpy/layout_tests/models/test_expectations.py:
+ (TestExpectationParser._parse_modifiers):
+ (TestExpectationParser._tokenize_line_using_new_format):
+ (TestExpectationLine.is_invalid):
+ * Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py:
+ (BasicTests.test_basic):
+ (test_get_test_set):
+ (test_parse_warning):
+ (test_pixel_tests_flag):
+ (SemanticTests.test_missing_bugid):
+ (SemanticTests):
+ (SemanticTests.test_skip_and_wontfix):
+ * Scripts/webkitpy/layout_tests/port/test.py:
+ (TestDriver.run_test):
+ * Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py:
+ (MainTest.test_hung_thread):
+ * Scripts/webkitpy/tool/commands/queries_unittest.py:
+ (PrintExpectationsTest.test_basic):
+ (PrintExpectationsTest.test_multiple):
+ (PrintExpectationsTest.test_full):
+ (PrintExpectationsTest.test_exclude):
+ (PrintExpectationsTest.test_csv):
+
2012-09-20 Lucas Forschler <[email protected]>
Unreviewed. Start running tests on the mac-ews.
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py (129183 => 129184)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py 2012-09-21 01:24:03 UTC (rev 129183)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py 2012-09-21 01:33:38 UTC (rev 129184)
@@ -420,6 +420,7 @@
thread.start()
thread.join(thread_timeout_sec)
result = thread.result
+ failures = []
if thread.isAlive():
# If join() returned with the thread still running, the
# DumpRenderTree is completely hung and there's nothing
@@ -430,11 +431,12 @@
# that tradeoff in order to avoid losing the rest of this
# thread's results.
_log.error('Test thread hung: killing all DumpRenderTrees')
+ failures = [test_failures.FailureTimeout()]
driver.stop()
if not result:
- result = test_results.TestResult(test_input.test_name, failures=[], test_run_time=0)
+ result = test_results.TestResult(test_input.test_name, failures=failures, test_run_time=0)
return result
def _run_test_in_this_thread(self, test_input, stop_when_done):
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py (129183 => 129184)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py 2012-09-21 01:24:03 UTC (rev 129183)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py 2012-09-21 01:33:38 UTC (rev 129184)
@@ -192,5 +192,5 @@
port = host.port_factory.get('test')
port._options.builder_name = 'dummy builder'
port._filesystem.write_text_file(port._filesystem.join(port.layout_tests_dir(), "failures/expected/wontfix.html"), "Dummy test contents")
- expected_results, unexpected_results = self.summarized_results(port, expected=False, passing=False, flaky=False, extra_tests=['failures/expected/wontfix.html'], extra_expectations='BUGX WONTFIX : failures/expected/wontfix.html = FAIL\n')
+ expected_results, unexpected_results = self.summarized_results(port, expected=False, passing=False, flaky=False, extra_tests=['failures/expected/wontfix.html'], extra_expectations='Bug(x) failures/expected/wontfix.html [ WontFix ]\n')
self.assertTrue(expected_results['tests']['failures']['expected']['wontfix.html']['wontfix'])
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py (129183 => 129184)
--- trunk/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py 2012-09-21 01:24:03 UTC (rev 129183)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py 2012-09-21 01:33:38 UTC (rev 129184)
@@ -76,6 +76,8 @@
TIMEOUT_EXPECTATION = 'timeout'
+ MISSING_BUG_WARNING = 'Test lacks BUG modifier.'
+
def __init__(self, port, full_test_list, allow_rebaseline_modifier):
self._port = port
self._test_configuration_converter = TestConfigurationConverter(set(port.all_test_configurations()), port.configuration_specifier_macros())
@@ -154,7 +156,7 @@
parsed_specifiers.add(modifier)
if not expectation_line.parsed_bug_modifiers and not has_wontfix and not has_bugid:
- expectation_line.warnings.append('Test lacks BUG modifier.')
+ expectation_line.warnings.append(self.MISSING_BUG_WARNING)
if self._allow_rebaseline_modifier and self.REBASELINE_MODIFIER in modifiers:
expectation_line.warnings.append('REBASELINE should only be used for running rebaseline.py. Cannot be checked in.')
@@ -392,7 +394,13 @@
elif state not in ('name_found', 'done'):
warnings.append('Missing a "]"')
- if not expectations:
+ if 'WONTFIX' in modifiers and 'SKIP' not in modifiers:
+ modifiers.append('SKIP')
+
+ if 'SKIP' in modifiers and expectations:
+ # FIXME: This is really a semantic warning and shouldn't be here. Remove when we drop the old syntax.
+ warnings.append('A test marked Skip or WontFix must not have other expectations.')
+ elif not expectations:
if 'SKIP' not in modifiers and 'REBASELINE' not in modifiers and 'SLOW' not in modifiers:
modifiers.append('SKIP')
expectations = ['PASS']
@@ -431,7 +439,7 @@
self.warnings = []
def is_invalid(self):
- return len(self.warnings) > 0
+ return self.warnings and self.warnings != [TestExpectationParser.MISSING_BUG_WARNING]
def is_flaky(self):
return len(self.parsed_expectations) > 1
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py (129183 => 129184)
--- trunk/Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py 2012-09-21 01:24:03 UTC (rev 129183)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py 2012-09-21 01:33:38 UTC (rev 129184)
@@ -68,10 +68,10 @@
def get_basic_expectations(self):
return """
BUG_TEST : failures/expected/text.html = FAIL
-BUG_TEST WONTFIX SKIP : failures/expected/crash.html = CRASH
+BUG_TEST WONTFIX : failures/expected/crash.html = PASS
BUG_TEST REBASELINE : failures/expected/missing_image.html = MISSING
-BUG_TEST WONTFIX : failures/expected/image_checksum.html = IMAGE
-BUG_TEST WONTFIX MAC : failures/expected/image.html = IMAGE
+BUG_TEST WONTFIX : failures/expected/image_checksum.html = PASS
+BUG_TEST WONTFIX MAC : failures/expected/image.html = PASS
"""
def parse_exp(self, expectations, overrides=None, is_lint_mode=False):
@@ -94,7 +94,7 @@
def test_basic(self):
self.parse_exp(self.get_basic_expectations())
self.assert_exp('failures/expected/text.html', FAIL)
- self.assert_exp('failures/expected/image_checksum.html', IMAGE)
+ self.assert_exp('failures/expected/image_checksum.html', PASS)
self.assert_exp('passes/text.html', PASS)
self.assert_exp('failures/expected/image.html', PASS)
@@ -137,14 +137,14 @@
# expectations and that known test part of a test category is
# present in the expectations.
exp_str = """
-BUGX WONTFIX : failures/expected = IMAGE
+BUGX WONTFIX : failures/expected = PASS
"""
self.parse_exp(exp_str)
test_name = 'failures/expected/unknown-test.html'
unknown_test = self.get_test(test_name)
self.assertRaises(KeyError, self._exp.get_expectations,
unknown_test)
- self.assert_exp('failures/expected/crash.html', IMAGE)
+ self.assert_exp('failures/expected/crash.html', PASS)
def test_get_modifiers(self):
self.parse_exp(self.get_basic_expectations())
@@ -170,11 +170,6 @@
self.assertEqual(s,
set([self.get_test('failures/expected/crash.html'),
self.get_test('failures/expected/image_checksum.html')]))
- s = self._exp.get_test_set(WONTFIX, CRASH)
- self.assertEqual(s,
- set([self.get_test('failures/expected/crash.html')]))
- s = self._exp.get_test_set(WONTFIX, CRASH, include_skips=False)
- self.assertEqual(s, set([]))
def test_parse_warning(self):
try:
@@ -191,13 +186,6 @@
"expectations:2 Path does not exist. non-existent-test.html")
self.assertEqual(str(e), warnings)
- try:
- self.parse_exp('SKIP : failures/expected/text.html = FAIL', is_lint_mode=True)
- self.assertFalse(True, "ParseError wasn't raised")
- except ParseError, e:
- warnings = u'expectations:1 Test lacks BUG modifier. failures/expected/text.html'
- self.assertEqual(str(e), warnings)
-
def test_error_on_different_platform(self):
# parse_exp uses a Windows port. Assert errors on Mac show up in lint mode.
self.assertRaises(ParseError, self.parse_exp,
@@ -236,11 +224,11 @@
self.assertTrue(match('failures/expected/text.html', FAIL, False))
self.assertFalse(match('failures/expected/text.html', CRASH, True))
self.assertFalse(match('failures/expected/text.html', CRASH, False))
- self.assertTrue(match('failures/expected/image_checksum.html', IMAGE,
+ self.assertTrue(match('failures/expected/image_checksum.html', PASS,
True))
self.assertTrue(match('failures/expected/image_checksum.html', PASS,
False))
- self.assertTrue(match('failures/expected/crash.html', SKIP, False))
+ self.assertTrue(match('failures/expected/crash.html', PASS, False))
self.assertTrue(match('passes/text.html', PASS, False))
def test_more_specific_override_resets_skip(self):
@@ -470,6 +458,24 @@
self.parse_exp('SLOW : failures/expected/text.html = FAIL')
self.assertTrue(self._exp.has_warnings())
+ self.parse_exp('failures/expected/text.html [ Failure ]')
+ line = self._exp._model.get_expectation_line('failures/expected/text.html')
+ self.assertFalse(line.is_invalid())
+ self.assertEquals(line.warnings, ['Test lacks BUG modifier.'])
+
+ def test_skip_and_wontfix(self):
+ # Skip and WontFix are not allowed to have other expectations as well, because those
+ # expectations won't be exercised and may become stale .
+ self.parse_exp('failures/expected/text.html [ Failure Skip ]')
+ self.assertTrue(self._exp.has_warnings())
+
+ self.parse_exp('failures/expected/text.html [ Crash WontFix ]')
+ self.assertTrue(self._exp.has_warnings())
+
+ # We can't warn against [ Pass WontFix ] until we get rid of the old syntax.
+ self.parse_exp('failures/expected/text.html [ Pass WontFix ]')
+ self.assertFalse(self._exp.has_warnings())
+
def test_slow_and_timeout(self):
# A test cannot be SLOW and expected to TIMEOUT.
self.assertRaises(ParseError, self.parse_exp,
@@ -503,19 +509,19 @@
# and tests expectations covering entire directories.
exp_str = """
BUGX : failures/expected/text.html = FAIL
-BUGX WONTFIX : failures/expected = IMAGE
+BUGX WONTFIX : failures/expected = PASS
"""
self.parse_exp(exp_str)
self.assert_exp('failures/expected/text.html', FAIL)
- self.assert_exp('failures/expected/crash.html', IMAGE)
+ self.assert_exp('failures/expected/crash.html', PASS)
exp_str = """
-BUGX WONTFIX : failures/expected = IMAGE
+BUGX WONTFIX : failures/expected = PASS
BUGX : failures/expected/text.html = FAIL
"""
self.parse_exp(exp_str)
self.assert_exp('failures/expected/text.html', FAIL)
- self.assert_exp('failures/expected/crash.html', IMAGE)
+ self.assert_exp('failures/expected/crash.html', PASS)
def test_ambiguous(self):
self.assert_bad_expectations("BUG_TEST RELEASE : passes/text.html = PASS\n"
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/test.py (129183 => 129184)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/test.py 2012-09-21 01:24:03 UTC (rev 129183)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/test.py 2012-09-21 01:33:38 UTC (rev 129184)
@@ -258,25 +258,25 @@
filesystem.maybe_make_directory(LAYOUT_TEST_DIR + '/platform/test')
if not filesystem.exists(LAYOUT_TEST_DIR + '/platform/test/TestExpectations'):
filesystem.write_text_file(LAYOUT_TEST_DIR + '/platform/test/TestExpectations', """
-WONTFIX : failures/expected/crash.html = CRASH
-WONTFIX : failures/expected/image.html = IMAGE
-WONTFIX : failures/expected/audio.html = FAIL
-WONTFIX : failures/expected/image_checksum.html = IMAGE
-WONTFIX : failures/expected/mismatch.html = IMAGE
-WONTFIX : failures/expected/missing_check.html = MISSING PASS
-WONTFIX : failures/expected/missing_image.html = MISSING PASS
-WONTFIX : failures/expected/missing_audio.html = MISSING PASS
-WONTFIX : failures/expected/missing_text.html = MISSING PASS
-WONTFIX : failures/expected/newlines_leading.html = FAIL
-WONTFIX : failures/expected/newlines_trailing.html = FAIL
-WONTFIX : failures/expected/newlines_with_excess_CR.html = FAIL
-WONTFIX : failures/expected/reftest.html = IMAGE
-WONTFIX : failures/expected/text.html = FAIL
-WONTFIX : failures/expected/timeout.html = TIMEOUT
-WONTFIX SKIP : failures/expected/hang.html = TIMEOUT
-WONTFIX SKIP : failures/expected/keyboard.html = CRASH
-WONTFIX SKIP : failures/expected/exception.html = CRASH
-WONTFIX SKIP : passes/skipped/skip.html = PASS
+Bug(test) failures/expected/crash.html [ Crash ]
+Bug(test) failures/expected/image.html [ ImageOnlyFailure ]
+Bug(test) failures/expected/audio.html [ Failure ]
+Bug(test) failures/expected/image_checksum.html [ ImageOnlyFailure ]
+Bug(test) failures/expected/mismatch.html [ ImageOnlyFailure ]
+Bug(test) failures/expected/missing_check.html [ Missing Pass ]
+Bug(test) failures/expected/missing_image.html [ Missing Pass ]
+Bug(test) failures/expected/missing_audio.html [ Missing Pass ]
+Bug(test) failures/expected/missing_text.html [ Missing Pass ]
+Bug(test) failures/expected/newlines_leading.html [ Failure ]
+Bug(test) failures/expected/newlines_trailing.html [ Failure ]
+Bug(test) failures/expected/newlines_with_excess_CR.html [ Failure ]
+Bug(test) failures/expected/reftest.html [ ImageOnlyFailure ]
+Bug(test) failures/expected/text.html [ Failure ]
+Bug(test) failures/expected/timeout.html [ Timeout ]
+Bug(test) failures/expected/hang.html [ WontFix ]
+Bug(test) failures/expected/keyboard.html [ WontFix ]
+Bug(test) failures/expected/exception.html [ WontFix ]
+Bug(test) passes/skipped/skip.html [ Skip ]
""")
filesystem.maybe_make_directory(LAYOUT_TEST_DIR + '/reftests/foo')
@@ -537,7 +537,7 @@
if test.exception:
raise ValueError('exception from ' + test_name)
if test.hang:
- time.sleep((float(test_input.timeout) * 4) / 1000.0)
+ time.sleep((float(test_input.timeout) * 4) / 1000.0 + 1.0) # The 1.0 comes from thread_padding_sec in layout_test_runnery.
audio = None
actual_text = test.actual_text
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py (129183 => 129184)
--- trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py 2012-09-21 01:24:03 UTC (rev 129183)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py 2012-09-21 01:33:38 UTC (rev 129184)
@@ -347,7 +347,9 @@
res, out, err, user = logging_run(['--run-singly', '--time-out-ms=50',
'failures/expected/hang.html'],
tests_included=True)
- self.assertEqual(res, 0)
+ # Note that hang.html is marked as WontFix and all WontFix tests are
+ # expected to Pass, so that actually running them generates an "unexpected" error.
+ self.assertEqual(res, 1)
self.assertNotEmpty(out)
self.assertNotEmpty(err)
Modified: trunk/Tools/Scripts/webkitpy/tool/commands/queries_unittest.py (129183 => 129184)
--- trunk/Tools/Scripts/webkitpy/tool/commands/queries_unittest.py 2012-09-21 01:24:03 UTC (rev 129183)
+++ trunk/Tools/Scripts/webkitpy/tool/commands/queries_unittest.py 2012-09-21 01:33:38 UTC (rev 129184)
@@ -178,35 +178,35 @@
def test_basic(self):
self.run_test(['failures/expected/text.html', 'failures/expected/image.html'],
('// For test-win-xp\n'
- 'failures/expected/image.html [ ImageOnlyFailure WontFix ]\n'
- 'failures/expected/text.html [ Failure WontFix ]\n'))
+ 'failures/expected/image.html [ ImageOnlyFailure ]\n'
+ 'failures/expected/text.html [ Failure ]\n'))
def test_multiple(self):
self.run_test(['failures/expected/text.html', 'failures/expected/image.html'],
('// For test-win-vista\n'
- 'failures/expected/image.html [ ImageOnlyFailure WontFix ]\n'
- 'failures/expected/text.html [ Failure WontFix ]\n'
+ 'failures/expected/image.html [ ImageOnlyFailure ]\n'
+ 'failures/expected/text.html [ Failure ]\n'
'\n'
'// For test-win-win7\n'
- 'failures/expected/image.html [ ImageOnlyFailure WontFix ]\n'
- 'failures/expected/text.html [ Failure WontFix ]\n'
+ 'failures/expected/image.html [ ImageOnlyFailure ]\n'
+ 'failures/expected/text.html [ Failure ]\n'
'\n'
'// For test-win-xp\n'
- 'failures/expected/image.html [ ImageOnlyFailure WontFix ]\n'
- 'failures/expected/text.html [ Failure WontFix ]\n'),
+ 'failures/expected/image.html [ ImageOnlyFailure ]\n'
+ 'failures/expected/text.html [ Failure ]\n'),
platform='test-win-*')
def test_full(self):
self.run_test(['failures/expected/text.html', 'failures/expected/image.html'],
('// For test-win-xp\n'
- 'failures/expected/image.html [ ImageOnlyFailure WontFix ]\n'
- 'failures/expected/text.html [ Failure WontFix ]\n'),
+ 'Bug(test) failures/expected/image.html [ ImageOnlyFailure ]\n'
+ 'Bug(test) failures/expected/text.html [ Failure ]\n'),
full=True)
def test_exclude(self):
self.run_test(['failures/expected/text.html', 'failures/expected/image.html'],
('// For test-win-xp\n'
- 'failures/expected/text.html [ Failure WontFix ]\n'),
+ 'failures/expected/text.html [ Failure ]\n'),
exclude_keyword=['image'])
def test_include(self):
@@ -217,8 +217,8 @@
def test_csv(self):
self.run_test(['failures/expected/text.html', 'failures/expected/image.html'],
- ('test-win-xp,failures/expected/image.html,WONTFIX,IMAGE\n'
- 'test-win-xp,failures/expected/text.html,WONTFIX,FAIL\n'),
+ ('test-win-xp,failures/expected/image.html,BUGTEST,IMAGE\n'
+ 'test-win-xp,failures/expected/text.html,BUGTEST,FAIL\n'),
csv=True)
