Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py (124246 => 124247)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py 2012-07-31 20:48:56 UTC (rev 124246)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py 2012-07-31 20:50:52 UTC (rev 124247)
@@ -276,9 +276,10 @@
def __init__(self, name, test_inputs):
self.name = name
self.test_inputs = test_inputs
+ self.requires_lock = test_inputs[0].requires_lock
def __repr__(self):
- return "TestShard(name='%s', test_inputs=%s'" % (self.name, self.test_inputs)
+ return "TestShard(name='%s', test_inputs=%s', requires_lock=%s)" % (self.name, self.test_inputs, self.requires_lock)
def __eq__(self, other):
return self.name == other.name and self.test_inputs == other.test_inputs
@@ -371,10 +372,10 @@
iterations = self._options.repeat_each * self._options.iterations
return ResultSummary(self._expectations, set(self._test_names), iterations, tests_to_skip)
- def _get_dir_for_test_file(self, test_file):
+ def _dir_for_test_input(self, test_input):
"""Returns the highest-level directory by which to shard the given
test file."""
- directory, test_file = self._port.split_test(test_file)
+ directory, test_file = self._port.split_test(test_input.test_name)
# The http tests are very stable on mac/linux.
# TODO(ojan): Make the http server on Windows be apache so we can
@@ -404,7 +405,7 @@
test_input.reference_files = self._port.reference_files(test_input.test_name)
return bool(test_input.reference_files)
- def _shard_tests(self, test_files, num_workers, fully_parallel, shard_ref_tests):
+ def _shard_tests(self, test_inputs, num_workers, fully_parallel, shard_ref_tests):
"""Groups tests into batches.
This helps ensure that tests that depend on each other (aka bad tests!)
continue to run together as most cross-tests dependencies tend to
@@ -418,12 +419,12 @@
# own class or module. Consider grouping it with the chunking logic
# in prepare_lists as well.
if num_workers == 1:
- return self._shard_in_two(test_files, shard_ref_tests)
+ return self._shard_in_two(test_inputs, shard_ref_tests)
elif fully_parallel:
- return self._shard_every_file(test_files)
- return self._shard_by_directory(test_files, num_workers, shard_ref_tests)
+ return self._shard_every_file(test_inputs)
+ return self._shard_by_directory(test_inputs, num_workers, shard_ref_tests)
- def _shard_in_two(self, test_files, shard_ref_tests):
+ def _shard_in_two(self, test_inputs, shard_ref_tests):
"""Returns two lists of shards, one with all the tests requiring a lock and one with the rest.
This is used when there's only one worker, to minimize the per-shard overhead."""
@@ -431,9 +432,8 @@
locked_ref_test_inputs = []
unlocked_inputs = []
unlocked_ref_test_inputs = []
- for test_file in test_files:
- test_input = self._test_input_for_file(test_file)
- if self._test_requires_lock(test_file):
+ for test_input in test_inputs:
+ if test_input.requires_lock:
if shard_ref_tests and test_input.reference_files:
locked_ref_test_inputs.append(test_input)
else:
@@ -455,26 +455,24 @@
return locked_shards, unlocked_shards
- def _shard_every_file(self, test_files):
+ def _shard_every_file(self, test_inputs):
"""Returns two lists of shards, each shard containing a single test file.
This mode gets maximal parallelism at the cost of much higher flakiness."""
locked_shards = []
unlocked_shards = []
- for test_file in test_files:
- test_input = self._test_input_for_file(test_file)
-
+ for test_input in test_inputs:
# 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,
# which would be really redundant.
- if self._test_requires_lock(test_file):
+ if test_input.requires_lock:
locked_shards.append(TestShard('.', [test_input]))
else:
unlocked_shards.append(TestShard('.', [test_input]))
return locked_shards, unlocked_shards
- def _shard_by_directory(self, test_files, num_workers, shard_ref_tests):
+ def _shard_by_directory(self, test_inputs, num_workers, shard_ref_tests):
"""Returns two lists of shards, each shard containing all the files in a directory.
This is the default mode, and gets as much parallelism as we can while
@@ -485,9 +483,8 @@
ref_tests_by_dir = {}
# FIXME: Given that the tests are already sorted by directory,
# 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._test_input_for_file(test_file)
+ for test_input in test_inputs:
+ directory = self._dir_for_test_input(test_input)
if shard_ref_tests and test_input.reference_files:
ref_tests_by_dir.setdefault(directory, [])
ref_tests_by_dir[directory].append(test_input)
@@ -497,7 +494,7 @@
for directory, test_inputs in tests_by_dir.iteritems():
shard = TestShard(directory, test_inputs)
- if self._test_requires_lock(directory):
+ if shard.requires_lock:
locked_shards.append(shard)
else:
unlocked_shards.append(shard)
@@ -505,7 +502,7 @@
for directory, test_inputs in ref_tests_by_dir.iteritems():
# '~' to place the ref tests after other tests after sorted.
shard = TestShard('~ref:' + directory, test_inputs)
- if self._test_requires_lock(directory):
+ if shard.requires_lock:
locked_shards.append(shard)
else:
unlocked_shards.append(shard)
@@ -591,7 +588,8 @@
interrupted = False
self._printer.write_update('Sharding tests ...')
- locked_shards, unlocked_shards = self._shard_tests(file_list, int(self._options.child_processes), self._options.fully_parallel, self._options.shard_ref_tests)
+ test_inputs = [self._test_input_for_file(file) for file in file_list]
+ locked_shards, unlocked_shards = self._shard_tests(test_inputs, int(self._options.child_processes), self._options.fully_parallel, self._options.shard_ref_tests)
# FIXME: We don't have a good way to coordinate the workers so that
# they don't try to run the shards that need a lock if we don't actually
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py (124246 => 124247)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py 2012-07-31 20:48:56 UTC (rev 124246)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py 2012-07-31 20:50:52 UTC (rev 124247)
@@ -64,7 +64,8 @@
self._ref_tests = ref_tests
def _test_input_for_file(self, test_file):
- return TestInput(test_file, reference_files=(['foo'] if test_file in self._ref_tests else None))
+ return TestInput(test_file, reference_files=(['foo'] if test_file in self._ref_tests else None),
+ requires_lock=(test_file.startswith('http') or test_file.startswith('perf')))
class ShardingTests(unittest.TestCase):
@@ -95,7 +96,7 @@
port._filesystem = MockFileSystem()
options = MockOptions(max_locked_shards=max_locked_shards)
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)
+ return self.manager._shard_tests([self.manager._test_input_for_file(test) for test in 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))
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/models/test_input.py (124246 => 124247)
--- trunk/Tools/Scripts/webkitpy/layout_tests/models/test_input.py 2012-07-31 20:48:56 UTC (rev 124246)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/models/test_input.py 2012-07-31 20:50:52 UTC (rev 124247)
@@ -32,15 +32,15 @@
class TestInput(object):
"""Groups information about a test for easy passing of data."""
- def __init__(self, test_name, timeout=None, test_requires_lock=None, reference_files=None, should_run_pixel_tests=None):
+ def __init__(self, test_name, timeout=None, 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 # in msecs; should rename this for consistency
- self.test_requires_lock = test_requires_lock
+ self.requires_lock = requires_lock
+ self.reference_files = reference_files
self.should_run_pixel_tests = should_run_pixel_tests
- self.reference_files = reference_files
def __repr__(self):
- return "TestInput('%s', %s, %s, %s, %s)" % (self.test_name, self.timeout, self.test_requires_lock, self.should_run_pixel_tests, self.reference_files)
+ return "TestInput('%s', timeout=%s, requires_lock=%s, reference_files=%s, should_run_pixel_tests=%s)" % (self.test_name, self.timeout, self.requires_lock, self.reference_files, self.should_run_pixel_tests)