Log Message
nrwt: clean up TestInputs in preparation for cleaning up sharding https://bugs.webkit.org/show_bug.cgi?id=92784
Reviewed by Ryosuke Niwa. Currently, in order to shard the tests you need to refer to state in the manager as well as the state in the TestInputs; this change embeds the necessary state into the TestInputs so sharding them can be a standalone operation. The actual clean up of the sharding will follow in a subsequent patch. Covered by existing tests; no new functionality. However, I did rework the sharding tests to be less dependent on the test scaffolding and easier to follow. * Scripts/webkitpy/layout_tests/controllers/manager.py: (Manager._test_input_for_file): (Manager._shard_in_two): (Manager._shard_every_file): (Manager._shard_by_directory): * Scripts/webkitpy/layout_tests/controllers/manager_unittest.py: (ManagerWrapper._test_input_for_file): (ShardingTests.assert_shards): (ShardingTests.test_shard_by_dir): (ShardingTests.test_shard_by_dir_sharding_ref_tests): (ShardingTests.test_shard_every_file): (ShardingTests.test_shard_in_two): (ShardingTests.test_shard_in_two_sharding_ref_tests): (ShardingTests.test_shard_in_two_has_no_unlocked_shards): (ShardingTests.test_multiple_locked_shards): * Scripts/webkitpy/layout_tests/models/test_input.py: (TestInput.__init__): (TestInput.__repr__):
Modified Paths
Diff
Modified: trunk/Tools/ChangeLog (124245 => 124246)
--- trunk/Tools/ChangeLog 2012-07-31 20:46:08 UTC (rev 124245)
+++ trunk/Tools/ChangeLog 2012-07-31 20:48:56 UTC (rev 124246)
@@ -1,3 +1,40 @@
+2012-07-31 Dirk Pranke <dpra...@chromium.org>
+
+ nrwt: clean up TestInputs in preparation for cleaning up sharding
+ https://bugs.webkit.org/show_bug.cgi?id=92784
+
+ Reviewed by Ryosuke Niwa.
+
+ Currently, in order to shard the tests you need to refer to
+ state in the manager as well as the state in the TestInputs;
+ this change embeds the necessary state into the TestInputs so
+ sharding them can be a standalone operation.
+
+ The actual clean up of the sharding will follow in a subsequent patch.
+
+ Covered by existing tests; no new functionality. However, I did
+ rework the sharding tests to be less dependent on the test
+ scaffolding and easier to follow.
+
+ * Scripts/webkitpy/layout_tests/controllers/manager.py:
+ (Manager._test_input_for_file):
+ (Manager._shard_in_two):
+ (Manager._shard_every_file):
+ (Manager._shard_by_directory):
+ * Scripts/webkitpy/layout_tests/controllers/manager_unittest.py:
+ (ManagerWrapper._test_input_for_file):
+ (ShardingTests.assert_shards):
+ (ShardingTests.test_shard_by_dir):
+ (ShardingTests.test_shard_by_dir_sharding_ref_tests):
+ (ShardingTests.test_shard_every_file):
+ (ShardingTests.test_shard_in_two):
+ (ShardingTests.test_shard_in_two_sharding_ref_tests):
+ (ShardingTests.test_shard_in_two_has_no_unlocked_shards):
+ (ShardingTests.test_multiple_locked_shards):
+ * Scripts/webkitpy/layout_tests/models/test_input.py:
+ (TestInput.__init__):
+ (TestInput.__repr__):
+
2012-07-31 Thiago Marcos P. Santos <thiago.san...@intel.com>
[EFL] Dump a backtrace in case of a crash
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py (124245 => 124246)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py 2012-07-31 20:46:08 UTC (rev 124245)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py 2012-07-31 20:48:56 UTC (rev 124246)
@@ -382,13 +382,11 @@
# what made them stable on linux/mac.
return directory
- def _get_test_input_for_file(self, test_file):
- """Returns the appropriate TestInput object for the file. Mostly this
- is used for looking up the timeout value (in ms) to use for the given
- test."""
- if self._test_is_slow(test_file):
- return TestInput(test_file, self._options.slow_time_out_ms)
- return TestInput(test_file, self._options.time_out_ms)
+ def _test_input_for_file(self, test_file):
+ return TestInput(test_file,
+ self._options.slow_time_out_ms if self._test_is_slow(test_file) else self._options.time_out_ms,
+ self._test_requires_lock(test_file),
+ self._port.reference_files(test_file) if self._options.shard_ref_tests else None)
def _test_requires_lock(self, test_file):
"""Return True if the test needs to be locked when
@@ -434,14 +432,14 @@
unlocked_inputs = []
unlocked_ref_test_inputs = []
for test_file in test_files:
- test_input = self._get_test_input_for_file(test_file)
+ test_input = self._test_input_for_file(test_file)
if self._test_requires_lock(test_file):
- if shard_ref_tests and self._is_ref_test(test_input):
+ if shard_ref_tests and test_input.reference_files:
locked_ref_test_inputs.append(test_input)
else:
locked_inputs.append(test_input)
else:
- if shard_ref_tests and self._is_ref_test(test_input):
+ if shard_ref_tests and test_input.reference_files:
unlocked_ref_test_inputs.append(test_input)
else:
unlocked_inputs.append(test_input)
@@ -464,7 +462,7 @@
locked_shards = []
unlocked_shards = []
for test_file in test_files:
- test_input = self._get_test_input_for_file(test_file)
+ test_input = self._test_input_for_file(test_file)
# Note that we use a '.' for the shard name; the name doesn't really
# matter, and the only other meaningful value would be the filename,
@@ -489,8 +487,8 @@
# we can probably rewrite this to be clearer and faster.
for test_file in test_files:
directory = self._get_dir_for_test_file(test_file)
- test_input = self._get_test_input_for_file(test_file)
- if shard_ref_tests and self._is_ref_test(test_input):
+ test_input = self._test_input_for_file(test_file)
+ if shard_ref_tests and test_input.reference_files:
ref_tests_by_dir.setdefault(directory, [])
ref_tests_by_dir[directory].append(test_input)
else:
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py (124245 => 124246)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py 2012-07-31 20:46:08 UTC (rev 124245)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py 2012-07-31 20:48:56 UTC (rev 124246)
@@ -51,6 +51,7 @@
from webkitpy.layout_tests.models.result_summary import ResultSummary
from webkitpy.layout_tests.models.test_expectations import TestExpectations
from webkitpy.layout_tests.models.test_results import TestResult
+from webkitpy.layout_tests.models.test_input import TestInput
from webkitpy.layout_tests.views import printing
from webkitpy.tool.mocktool import MockOptions
from webkitpy.common.system.executive_mock import MockExecutive
@@ -62,13 +63,10 @@
Manager.__init__(self, **kwargs)
self._ref_tests = ref_tests
- def _get_test_input_for_file(self, test_file):
- return test_file
+ def _test_input_for_file(self, test_file):
+ return TestInput(test_file, reference_files=(['foo'] if test_file in self._ref_tests else None))
- def _is_ref_test(self, test_input):
- return test_input in self._ref_tests
-
class ShardingTests(unittest.TestCase):
test_list = [
"http/tests/websocket/tests/unicode.htm",
@@ -99,29 +97,33 @@
self.manager = ManagerWrapper(self.ref_tests, port=port, options=options, printer=Mock())
return self.manager._shard_tests(test_list, num_workers, fully_parallel, shard_ref_tests)
+ def assert_shards(self, actual_shards, expected_shard_names):
+ self.assertEquals(len(actual_shards), len(expected_shard_names))
+ for i, shard in enumerate(actual_shards):
+ expected_shard_name, expected_test_names = expected_shard_names[i]
+ self.assertEquals(shard.name, expected_shard_name)
+ self.assertEquals([test_input.test_name for test_input in shard.test_inputs],
+ expected_test_names)
+
def test_shard_by_dir(self):
locked, unlocked = self.get_shards(num_workers=2, fully_parallel=False)
# Note that although there are tests in multiple dirs that need locks,
# they are crammed into a single shard in order to reduce the # of
# workers hitting the server at once.
- self.assertEquals(locked,
- [TestShard('locked_shard_1',
- ['http/tests/security/view-source-no-refresh.html',
- 'http/tests/websocket/tests/unicode.htm',
- 'http/tests/websocket/tests/websocket-protocol-ignored.html',
- 'http/tests/xmlhttprequest/supported-xml-content-types.html',
- 'perf/object-keys.html'])])
- self.assertEquals(unlocked,
- [TestShard('animations',
- ['animations/keyframes.html']),
- TestShard('dom/html/level2/html',
- ['dom/html/level2/html/HTMLAnchorElement03.html',
- 'dom/html/level2/html/HTMLAnchorElement06.html']),
- TestShard('fast/css',
- ['fast/css/display-none-inline-style-change-crash.html']),
- TestShard('ietestcenter/_javascript_',
- ['ietestcenter/_javascript_/11.1.5_4-4-c-1.html'])])
+ self.assert_shards(locked,
+ [('locked_shard_1',
+ ['http/tests/security/view-source-no-refresh.html',
+ 'http/tests/websocket/tests/unicode.htm',
+ 'http/tests/websocket/tests/websocket-protocol-ignored.html',
+ 'http/tests/xmlhttprequest/supported-xml-content-types.html',
+ 'perf/object-keys.html'])])
+ self.assert_shards(unlocked,
+ [('animations', ['animations/keyframes.html']),
+ ('dom/html/level2/html', ['dom/html/level2/html/HTMLAnchorElement03.html',
+ 'dom/html/level2/html/HTMLAnchorElement06.html']),
+ ('fast/css', ['fast/css/display-none-inline-style-change-crash.html']),
+ ('ietestcenter/_javascript_', ['ietestcenter/_javascript_/11.1.5_4-4-c-1.html'])])
def test_shard_by_dir_sharding_ref_tests(self):
locked, unlocked = self.get_shards(num_workers=2, fully_parallel=False, shard_ref_tests=True)
@@ -129,73 +131,68 @@
# Note that although there are tests in multiple dirs that need locks,
# they are crammed into a single shard in order to reduce the # of
# workers hitting the server at once.
- self.assertEquals(locked,
- [TestShard('locked_shard_1',
+ self.assert_shards(locked,
+ [('locked_shard_1',
['http/tests/websocket/tests/unicode.htm',
'http/tests/xmlhttprequest/supported-xml-content-types.html',
'perf/object-keys.html',
'http/tests/security/view-source-no-refresh.html',
'http/tests/websocket/tests/websocket-protocol-ignored.html'])])
- self.assertEquals(unlocked,
- [TestShard('animations',
- ['animations/keyframes.html']),
- TestShard('dom/html/level2/html',
- ['dom/html/level2/html/HTMLAnchorElement03.html']),
- TestShard('fast/css',
- ['fast/css/display-none-inline-style-change-crash.html']),
- TestShard('~ref:dom/html/level2/html',
- ['dom/html/level2/html/HTMLAnchorElement06.html']),
- TestShard('~ref:ietestcenter/_javascript_',
- ['ietestcenter/_javascript_/11.1.5_4-4-c-1.html'])])
+ self.assert_shards(unlocked,
+ [('animations', ['animations/keyframes.html']),
+ ('dom/html/level2/html', ['dom/html/level2/html/HTMLAnchorElement03.html']),
+ ('fast/css', ['fast/css/display-none-inline-style-change-crash.html']),
+ ('~ref:dom/html/level2/html', ['dom/html/level2/html/HTMLAnchorElement06.html']),
+ ('~ref:ietestcenter/_javascript_', ['ietestcenter/_javascript_/11.1.5_4-4-c-1.html'])])
def test_shard_every_file(self):
locked, unlocked = self.get_shards(num_workers=2, fully_parallel=True)
- self.assertEquals(locked,
- [TestShard('.', ['http/tests/websocket/tests/unicode.htm']),
- TestShard('.', ['http/tests/security/view-source-no-refresh.html']),
- TestShard('.', ['http/tests/websocket/tests/websocket-protocol-ignored.html']),
- TestShard('.', ['http/tests/xmlhttprequest/supported-xml-content-types.html']),
- TestShard('.', ['perf/object-keys.html'])]),
- self.assertEquals(unlocked,
- [TestShard('.', ['animations/keyframes.html']),
- TestShard('.', ['fast/css/display-none-inline-style-change-crash.html']),
- TestShard('.', ['dom/html/level2/html/HTMLAnchorElement03.html']),
- TestShard('.', ['ietestcenter/_javascript_/11.1.5_4-4-c-1.html']),
- TestShard('.', ['dom/html/level2/html/HTMLAnchorElement06.html'])])
+ self.assert_shards(locked,
+ [('.', ['http/tests/websocket/tests/unicode.htm']),
+ ('.', ['http/tests/security/view-source-no-refresh.html']),
+ ('.', ['http/tests/websocket/tests/websocket-protocol-ignored.html']),
+ ('.', ['http/tests/xmlhttprequest/supported-xml-content-types.html']),
+ ('.', ['perf/object-keys.html'])]),
+ self.assert_shards(unlocked,
+ [('.', ['animations/keyframes.html']),
+ ('.', ['fast/css/display-none-inline-style-change-crash.html']),
+ ('.', ['dom/html/level2/html/HTMLAnchorElement03.html']),
+ ('.', ['ietestcenter/_javascript_/11.1.5_4-4-c-1.html']),
+ ('.', ['dom/html/level2/html/HTMLAnchorElement06.html'])])
def test_shard_in_two(self):
locked, unlocked = self.get_shards(num_workers=1, fully_parallel=False)
- self.assertEquals(locked,
- [TestShard('locked_tests',
- ['http/tests/websocket/tests/unicode.htm',
- 'http/tests/security/view-source-no-refresh.html',
- 'http/tests/websocket/tests/websocket-protocol-ignored.html',
- 'http/tests/xmlhttprequest/supported-xml-content-types.html',
- 'perf/object-keys.html'])])
- self.assertEquals(unlocked,
- [TestShard('unlocked_tests',
- ['animations/keyframes.html',
- 'fast/css/display-none-inline-style-change-crash.html',
- 'dom/html/level2/html/HTMLAnchorElement03.html',
- 'ietestcenter/_javascript_/11.1.5_4-4-c-1.html',
- 'dom/html/level2/html/HTMLAnchorElement06.html'])])
+ self.assert_shards(locked,
+ [('locked_tests',
+ ['http/tests/websocket/tests/unicode.htm',
+ 'http/tests/security/view-source-no-refresh.html',
+ 'http/tests/websocket/tests/websocket-protocol-ignored.html',
+ 'http/tests/xmlhttprequest/supported-xml-content-types.html',
+ 'perf/object-keys.html'])])
+ self.assert_shards(unlocked,
+ [('unlocked_tests',
+ ['animations/keyframes.html',
+ 'fast/css/display-none-inline-style-change-crash.html',
+ 'dom/html/level2/html/HTMLAnchorElement03.html',
+ 'ietestcenter/_javascript_/11.1.5_4-4-c-1.html',
+ 'dom/html/level2/html/HTMLAnchorElement06.html'])])
def test_shard_in_two_sharding_ref_tests(self):
locked, unlocked = self.get_shards(num_workers=1, fully_parallel=False, shard_ref_tests=True)
- self.assertEquals(locked,
- [TestShard('locked_tests',
- ['http/tests/websocket/tests/unicode.htm',
- 'http/tests/xmlhttprequest/supported-xml-content-types.html',
- 'perf/object-keys.html',
- 'http/tests/security/view-source-no-refresh.html',
- 'http/tests/websocket/tests/websocket-protocol-ignored.html'])])
- self.assertEquals(unlocked,
- [TestShard('unlocked_tests',
- ['animations/keyframes.html',
- 'fast/css/display-none-inline-style-change-crash.html',
- 'dom/html/level2/html/HTMLAnchorElement03.html',
- 'ietestcenter/_javascript_/11.1.5_4-4-c-1.html',
- 'dom/html/level2/html/HTMLAnchorElement06.html'])])
+ self.assert_shards(locked,
+ [('locked_tests',
+ ['http/tests/websocket/tests/unicode.htm',
+ 'http/tests/xmlhttprequest/supported-xml-content-types.html',
+ 'perf/object-keys.html',
+ 'http/tests/security/view-source-no-refresh.html',
+ 'http/tests/websocket/tests/websocket-protocol-ignored.html'])])
+ self.assert_shards(unlocked,
+ [('unlocked_tests',
+ ['animations/keyframes.html',
+ 'fast/css/display-none-inline-style-change-crash.html',
+ 'dom/html/level2/html/HTMLAnchorElement03.html',
+ 'ietestcenter/_javascript_/11.1.5_4-4-c-1.html',
+ 'dom/html/level2/html/HTMLAnchorElement06.html'])])
def test_shard_in_two_has_no_locked_shards(self):
locked, unlocked = self.get_shards(num_workers=1, fully_parallel=False,
@@ -205,29 +202,29 @@
def test_shard_in_two_has_no_unlocked_shards(self):
locked, unlocked = self.get_shards(num_workers=1, fully_parallel=False,
- test_list=['http/tests/webcoket/tests/unicode.htm'])
+ test_list=['http/tests/websocket/tests/unicode.htm'])
self.assertEquals(len(locked), 1)
self.assertEquals(len(unlocked), 0)
def test_multiple_locked_shards(self):
locked, unlocked = self.get_shards(num_workers=4, fully_parallel=False, max_locked_shards=2)
- self.assertEqual(locked,
- [TestShard('locked_shard_1',
- ['http/tests/security/view-source-no-refresh.html',
- 'http/tests/websocket/tests/unicode.htm',
- 'http/tests/websocket/tests/websocket-protocol-ignored.html']),
- TestShard('locked_shard_2',
- ['http/tests/xmlhttprequest/supported-xml-content-types.html',
- 'perf/object-keys.html'])])
+ self.assert_shards(locked,
+ [('locked_shard_1',
+ ['http/tests/security/view-source-no-refresh.html',
+ 'http/tests/websocket/tests/unicode.htm',
+ 'http/tests/websocket/tests/websocket-protocol-ignored.html']),
+ ('locked_shard_2',
+ ['http/tests/xmlhttprequest/supported-xml-content-types.html',
+ 'perf/object-keys.html'])])
locked, unlocked = self.get_shards(num_workers=4, fully_parallel=False)
- self.assertEquals(locked,
- [TestShard('locked_shard_1',
- ['http/tests/security/view-source-no-refresh.html',
- 'http/tests/websocket/tests/unicode.htm',
- 'http/tests/websocket/tests/websocket-protocol-ignored.html',
- 'http/tests/xmlhttprequest/supported-xml-content-types.html',
- 'perf/object-keys.html'])])
+ self.assert_shards(locked,
+ [('locked_shard_1',
+ ['http/tests/security/view-source-no-refresh.html',
+ 'http/tests/websocket/tests/unicode.htm',
+ 'http/tests/websocket/tests/websocket-protocol-ignored.html',
+ 'http/tests/xmlhttprequest/supported-xml-content-types.html',
+ 'perf/object-keys.html'])])
class LockCheckingManager(Manager):
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/models/test_input.py (124245 => 124246)
--- trunk/Tools/Scripts/webkitpy/layout_tests/models/test_input.py 2012-07-31 20:46:08 UTC (rev 124245)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/models/test_input.py 2012-07-31 20:48:56 UTC (rev 124246)
@@ -32,21 +32,15 @@
class TestInput(object):
"""Groups information about a test for easy passing of data."""
- def __init__(self, test_name, timeout):
- """Holds the input parameters for a test.
- Args:
- test: name of test (not an absolute path!)
- timeout: Timeout in msecs the driver should use while running the test
- """
+ def __init__(self, test_name, timeout=None, test_requires_lock=None, reference_files=None, should_run_pixel_tests=None):
+ # TestInput objects are normally constructed by the manager and passed
+ # to the workers, but these some fields are set lazily in the workers where possible
+ # because they require us to look at the filesystem and we want to be able to do that in parallel.
self.test_name = test_name
- self.timeout = timeout
+ self.timeout = timeout # in msecs; should rename this for consistency
+ self.test_requires_lock = test_requires_lock
+ self.should_run_pixel_tests = should_run_pixel_tests
+ self.reference_files = reference_files
- # TestInput objects are normally constructed by the manager and passed
- # to the workers, but these two fields are set lazily in the workers
- # because they require us to figure out if the test is a reftest or not
- # and we want to be able to do that in parallel.
- self.should_run_pixel_tests = None
- self.reference_files = None
-
def __repr__(self):
- return "TestInput('%s', %d, %s, %s)" % (self.test_name, self.timeout, self.should_run_pixel_tests, self.reference_files)
+ return "TestInput('%s', %s, %s, %s, %s)" % (self.test_name, self.timeout, self.test_requires_lock, self.should_run_pixel_tests, self.reference_files)
_______________________________________________ webkit-changes mailing list webkit-changes@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-changes
