Diff
Modified: trunk/Tools/ChangeLog (136578 => 136579)
--- trunk/Tools/ChangeLog 2012-12-04 22:47:59 UTC (rev 136578)
+++ trunk/Tools/ChangeLog 2012-12-04 22:50:24 UTC (rev 136579)
@@ -1,3 +1,55 @@
+2012-12-04 Dirk Pranke <[email protected]>
+
+ nrwt: derive per-thread, per-worker stats from result summaries
+ https://bugs.webkit.org/show_bug.cgi?id=103943
+
+ Reviewed by Ojan Vafai.
+
+ This patch changes the manager and printing code to pull more
+ information from the ResultSummary object rather than tracking
+ additional statistics on the side. This allows us to consolidate
+ a bunch of logic and remove a bunch of free-floating lists and dicts of
+ of values. We add the shard_name and worker_name to each TestResult,
+ and then modify print_timing_statistics() and upload_json_files()
+ to just need the result summaries.
+
+ Note that this patch also changes the timing statistics slightly
+ so that the individual test times are the times to run just the
+ tests themselves (TestResult.test_run_time) and do not include the
+ times taken to run the ref tests, compute diffs, etc. The aggregated
+ per-worker and per-shard statistics still use the total time to
+ process each test (TestResult.total_run_time).
+
+ * Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:
+ (LayoutTestRunner.__init__):
+ (LayoutTestRunner.run_tests):
+ (LayoutTestRunner._handle_finished_test_list):
+ (LayoutTestRunner._handle_finished_test):
+ (Worker.handle):
+ (Worker._run_test):
+ * Scripts/webkitpy/layout_tests/controllers/manager.py:
+ (summarize_results):
+ (Manager.run):
+ (Manager._upload_json_files):
+ * Scripts/webkitpy/layout_tests/controllers/manager_unittest.py:
+ (ResultSummaryTest.summarized_results):
+ * Scripts/webkitpy/layout_tests/layout_package/json_layout_results_generator.py:
+ (JSONLayoutResultsGenerator.__init__):
+ (JSONLayoutResultsGenerator._get_test_timing):
+ * Scripts/webkitpy/layout_tests/models/test_results.py:
+ (TestResult.__init__):
+ * Scripts/webkitpy/layout_tests/views/printing.py:
+ (Printer.print_results):
+ (Printer._print_timing_statistics):
+ (Printer):
+ (Printer._print_worker_statistics):
+ (Printer._print_aggregate_test_statistics):
+ (Printer._print_individual_test_times):
+ (Printer._print_directory_timings):
+ * Scripts/webkitpy/layout_tests/views/printing_unittest.py:
+ (Testprinter.test_print_unexpected_results.get_unexpected_results):
+ (Testprinter):
+
2012-12-04 Adam Barth <[email protected]>
Remove obsolete code from webkitpy
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py (136578 => 136579)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py 2012-12-04 22:47:59 UTC (rev 136578)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py 2012-12-04 22:50:24 UTC (rev 136579)
@@ -28,7 +28,6 @@
import logging
import math
-import re
import threading
import time
@@ -75,20 +74,11 @@
self._needs_websockets = None
self._retrying = False
self._test_files_list = []
- self._all_results = []
- self._group_stats = {}
- self._worker_stats = {}
+ self._remaining_locked_shards = []
+ self._has_http_lock = False
self._filesystem = self._port.host.filesystem
def run_tests(self, test_inputs, expectations, result_summary, num_workers, needs_http, needs_websockets, retrying):
- """Returns a tuple of (thread_timings, test_timings, individual_test_timings):
- thread_timings is a list of dicts with the total runtime
- of each thread with 'name', 'num_tests', 'total_time' properties
- test_timings is a list of timings for each sharded subdirectory
- of the form [time, directory_name, num_tests]
- individual_test_timings is a list of run times for each test
- in the form {filename:filename, test_run_time:test_run_time}
- """
self._current_result_summary = result_summary
self._expectations = expectations
self._needs_http = needs_http
@@ -98,9 +88,6 @@
self._printer.num_tests = len(self._test_files_list)
self._printer.num_completed = 0
- self._all_results = []
- self._group_stats = {}
- self._worker_stats = {}
self._has_http_lock = False
self._remaining_locked_shards = []
@@ -124,7 +111,7 @@
self._printer.print_workers_and_shards(num_workers, len(all_shards), len(locked_shards))
if self._options.dry_run:
- return (self._worker_stats.values(), self._group_stats, self._all_results)
+ return result_summary
self._printer.write_update('Starting %s ...' % grammar.pluralize('worker', num_workers))
@@ -144,8 +131,7 @@
finally:
self.stop_servers_with_lock()
- # FIXME: Move these stats into ResultSummary and return that.
- return (self._worker_stats.values(), self._group_stats, self._all_results)
+ return result_summary
def _worker_factory(self, worker_connection):
results_directory = self._results_directory
@@ -231,9 +217,7 @@
def _handle_started_test(self, worker_name, test_input, test_timeout_sec):
self._printer.print_started_test(test_input.test_name)
- def _handle_finished_test_list(self, worker_name, list_name, num_tests, elapsed_time):
- self._group_stats[list_name] = (num_tests, elapsed_time)
-
+ def _handle_finished_test_list(self, worker_name, list_name):
def find(name, test_lists):
for i in range(len(test_lists)):
if test_lists[i].name == name:
@@ -246,11 +230,7 @@
if not self._remaining_locked_shards and not self._port.requires_http_server():
self.stop_servers_with_lock()
- def _handle_finished_test(self, worker_name, result, elapsed_time, log_messages=[]):
- self._worker_stats.setdefault(worker_name, {'name': worker_name, 'num_tests': 0, 'total_time': 0})
- self._worker_stats[worker_name]['total_time'] += elapsed_time
- self._worker_stats[worker_name]['num_tests'] += 1
- self._all_results.append(result)
+ def _handle_finished_test(self, worker_name, result, log_messages=[]):
self._update_summary_with_result(self._current_result_summary, result)
@@ -290,11 +270,9 @@
def handle(self, name, source, test_list_name, test_inputs):
assert name == 'test_list'
- start_time = time.time()
for test_input in test_inputs:
- self._run_test(test_input)
- elapsed_time = time.time() - start_time
- self._caller.post('finished_test_list', test_list_name, len(test_inputs), elapsed_time)
+ self._run_test(test_input, test_list_name)
+ self._caller.post('finished_test_list', test_list_name)
def _update_test_input(self, test_input):
if test_input.reference_files is None:
@@ -305,7 +283,7 @@
else:
test_input.should_run_pixel_test = self._port.should_run_as_pixel_test(test_input)
- def _run_test(self, test_input):
+ def _run_test(self, test_input, shard_name):
self._batch_count += 1
stop_when_done = False
@@ -319,9 +297,11 @@
self._caller.post('started_test', test_input, test_timeout_sec)
result = self._run_test_with_timeout(test_input, test_timeout_sec, stop_when_done)
+ result.shard_name = shard_name
+ result.worker_name = self._name
+ result.total_run_time = time.time() - start
- elapsed_time = time.time() - start
- self._caller.post('finished_test', result, elapsed_time)
+ self._caller.post('finished_test', result)
self._clean_up_after_test(test_input, result)
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py (136578 => 136579)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py 2012-12-04 22:47:59 UTC (rev 136578)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py 2012-12-04 22:50:24 UTC (rev 136579)
@@ -106,7 +106,7 @@
# or split off from Manager onto another helper class, but should not be a free function.
# Most likely this should be made into its own class, and this super-long function
# split into many helper functions.
-def summarize_results(port_obj, expectations, result_summary, retry_summary, test_timings, only_unexpected):
+def summarize_results(port_obj, expectations, result_summary, retry_summary, only_unexpected):
"""Summarize failing results as a dict.
FIXME: split this data structure into a separate class?
@@ -116,7 +116,6 @@
expectations: test_expectations.TestExpectations object
result_summary: summary object from initial test runs
retry_summary: summary object from final test run of retried tests
- test_timings: a list of TestResult objects which contain test runtimes in seconds
only_unexpected: whether to return a summary only for the unexpected results
Returns:
A dictionary containing a summary of the unexpected results from the
@@ -401,14 +400,10 @@
return -1
start_time = time.time()
- retry_summary = result_summary
- # FIXME: move these fields into result_summary so we don't have to pass them around.
- thread_timings = test_timings = {}
- individual_test_timings = []
+ retry_summary = None
try:
- thread_timings, test_timings, individual_test_timings = \
- self._run_tests(self._test_names, result_summary, int(self._options.child_processes), retrying=False)
+ result_summary = self._run_tests(self._test_names, result_summary, int(self._options.child_processes), retrying=False)
# We exclude the crashes from the list of results to retry, because
# we want to treat even a potentially flaky crash as an error.
@@ -418,9 +413,7 @@
_log.info('')
_log.info("Retrying %d unexpected failure(s) ..." % len(failures))
_log.info('')
- # Note that we don't care about the values returned from the retry pass.
- retry_summary = ResultSummary(self._expectations, failures, 1, set())
- _ = self._run_tests(failures, retry_summary, 1, retrying=True)
+ retry_summary = self._run_tests(failures, ResultSummary(self._expectations, failures, 1, set()), 1, retrying=True)
finally:
# If we are ctrl-c'ed; we still want to try and print the results we got, and clean up,
# but we don't want to upload anything.
@@ -429,20 +422,25 @@
# Some crash logs can take a long time to be written out so look
# for new logs after the test run finishes.
self._look_for_new_crash_logs(result_summary, start_time)
- self._look_for_new_crash_logs(retry_summary, start_time)
+ if retry_summary:
+ self._look_for_new_crash_logs(retry_summary, start_time)
+ else:
+ # FIXME: Modify summarize_results to handle retry_summary == None.
+ retry_summary = result_summary
+
self._clean_up_run()
- unexpected_results = summarize_results(self._port, self._expectations, result_summary, retry_summary, individual_test_timings, _only_unexpected_=True)
+ unexpected_results = summarize_results(self._port, self._expectations, result_summary, retry_summary, _only_unexpected_=True)
- self._printer.print_results(end_time - start_time, thread_timings, test_timings, individual_test_timings, result_summary, unexpected_results)
+ self._printer.print_results(end_time - start_time, result_summary, unexpected_results)
# FIXME: remove record_results. It's just used for testing. There's no need
# for it to be a commandline argument.
if self._options.record_results and not self._options.dry_run:
self._port.print_leaks_summary()
# Write the same data to log files and upload generated JSON files to appengine server.
- summarized_results = summarize_results(self._port, self._expectations, result_summary, retry_summary, individual_test_timings, _only_unexpected_=False)
- self._upload_json_files(summarized_results, result_summary, individual_test_timings)
+ summarized_results = summarize_results(self._port, self._expectations, result_summary, retry_summary, _only_unexpected_=False)
+ self._upload_json_files(summarized_results, result_summary)
# Write the summary to disk (results.html) and display it if requested.
if not self._options.dry_run:
@@ -516,20 +514,17 @@
return failed_test_names
- def _upload_json_files(self, summarized_results, result_summary, individual_test_timings):
+ def _upload_json_files(self, summarized_results, result_summary):
"""Writes the results of the test run as JSON files into the results
dir and upload the files to the appengine server.
Args:
- unexpected_results: dict of unexpected results
summarized_results: dict of results
result_summary: full summary object
- individual_test_timings: list of test times (used by the flakiness
- dashboard).
"""
_log.debug("Writing JSON files in %s." % self._results_directory)
- times_trie = json_results_generator.test_timings_trie(self._port, individual_test_timings)
+ times_trie = json_results_generator.test_timings_trie(self._port, result_summary.results.values())
times_json_path = self._filesystem.join(self._results_directory, "times_ms.json")
json_results_generator.write_json(self._filesystem, times_trie, times_json_path)
@@ -540,7 +535,7 @@
generator = json_layout_results_generator.JSONLayoutResultsGenerator(
self._port, self._options.builder_name, self._options.build_name,
self._options.build_number, self._results_directory,
- BUILDER_BASE_URL, individual_test_timings,
+ BUILDER_BASE_URL,
self._expectations, result_summary, self._test_names,
self._options.test_results_server,
"layout-tests",
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py (136578 => 136579)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py 2012-12-04 22:47:59 UTC (rev 136578)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py 2012-12-04 22:50:24 UTC (rev 136579)
@@ -177,8 +177,8 @@
retry.add(self.get_result('passes/text.html'), True, test_is_slow)
retry.add(self.get_result('failures/expected/timeout.html'), True, test_is_slow)
retry.add(self.get_result('failures/expected/crash.html'), True, test_is_slow)
- unexpected_results = summarize_results(port, exp, rs, retry, test_timings={}, _only_unexpected_=True)
- expected_results = summarize_results(port, exp, rs, retry, test_timings={}, _only_unexpected_=False)
+ unexpected_results = summarize_results(port, exp, rs, retry, _only_unexpected_=True)
+ expected_results = summarize_results(port, exp, rs, retry, _only_unexpected_=False)
return expected_results, unexpected_results
def test_no_svn_revision(self):
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/json_layout_results_generator.py (136578 => 136579)
--- trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/json_layout_results_generator.py 2012-12-04 22:47:59 UTC (rev 136578)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/json_layout_results_generator.py 2012-12-04 22:50:24 UTC (rev 136579)
@@ -52,7 +52,7 @@
def __init__(self, port, builder_name, build_name, build_number,
results_file_base_path, builder_base_url,
- test_timings, expectations, result_summary, all_tests,
+ expectations, result_summary, all_tests,
test_results_server=None, test_type="", master_name=""):
"""Modifies the results.json file. Grabs it off the archive directory
if it is not found locally.
@@ -71,7 +71,7 @@
self._result_summary = result_summary
self._failures = dict((test_name, result_summary.results[test_name].type) for test_name in result_summary.failures)
self._all_tests = all_tests
- self._test_timings = dict((test_tuple.test_name, test_tuple.test_run_time) for test_tuple in test_timings)
+ self._test_timings = result_summary.results
self.generate_json_output()
@@ -99,7 +99,7 @@
def _get_test_timing(self, test_name):
if test_name in self._test_timings:
# Floor for now to get time in seconds.
- return int(self._test_timings[test_name])
+ return int(self._test_timings[test_name].test_run_time)
return 0
# override
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/models/test_results.py (136578 => 136579)
--- trunk/Tools/Scripts/webkitpy/layout_tests/models/test_results.py 2012-12-04 22:47:59 UTC (rev 136578)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/models/test_results.py 2012-12-04 22:50:24 UTC (rev 136579)
@@ -41,12 +41,18 @@
def __init__(self, test_name, failures=None, test_run_time=None, has_stderr=False, reftest_type=[]):
self.test_name = test_name
self.failures = failures or []
- self.test_run_time = test_run_time or 0
+ self.test_run_time = test_run_time or 0 # The time taken to execute the test itself.
self.has_stderr = has_stderr
self.reftest_type = reftest_type
+
# FIXME: Setting this in the constructor makes this class hard to mutate.
self.type = test_failures.determine_result_type(failures)
+ # These are set by the worker, not by the driver, so they are not passed to the constructor.
+ self.worker_name = ''
+ self.shard_name = ''
+ self.total_run_time = 0 # The time taken to run the test plus any references, compute diffs, etc.
+
def __eq__(self, other):
return (self.test_name == other.test_name and
self.failures == other.failures and
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/views/printing.py (136578 => 136579)
--- trunk/Tools/Scripts/webkitpy/layout_tests/views/printing.py 2012-12-04 22:47:59 UTC (rev 136578)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/views/printing.py 2012-12-04 22:50:24 UTC (rev 136579)
@@ -148,39 +148,47 @@
ndigits = int(math.log10(len(num))) + 1
return ndigits
- def print_results(self, run_time, thread_timings, test_timings, individual_test_timings, result_summary, unexpected_results):
- self._print_timing_statistics(run_time, thread_timings, test_timings, individual_test_timings, result_summary)
+ def print_results(self, run_time, result_summary, unexpected_results):
+ self._print_timing_statistics(run_time, result_summary)
self._print_result_summary(result_summary)
self._print_one_line_summary(result_summary.total - result_summary.expected_skips,
result_summary.expected - result_summary.expected_skips,
result_summary.unexpected)
self._print_unexpected_results(unexpected_results)
- def _print_timing_statistics(self, total_time, thread_timings,
- directory_test_timings, individual_test_timings,
- result_summary):
+ def _print_timing_statistics(self, total_time, result_summary):
self._print_debug("Test timing:")
self._print_debug(" %6.2f total testing time" % total_time)
self._print_debug("")
+
+ self._print_worker_statistics(result_summary, int(self._options.child_processes))
+ self._print_aggregate_test_statistics(result_summary)
+ self._print_individual_test_times(result_summary)
+ self._print_directory_timings(result_summary)
+
+ def _print_worker_statistics(self, result_summary, num_workers):
self._print_debug("Thread timing:")
+ stats = {}
cuml_time = 0
- for t in thread_timings:
- self._print_debug(" %10s: %5d tests, %6.2f secs" % (t['name'], t['num_tests'], t['total_time']))
- cuml_time += t['total_time']
- self._print_debug(" %6.2f cumulative, %6.2f optimal" % (cuml_time, cuml_time / int(self._options.child_processes)))
+ for result in result_summary.results.values():
+ stats.setdefault(result.worker_name, {'num_tests': 0, 'total_time': 0})
+ stats[result.worker_name]['num_tests'] += 1
+ stats[result.worker_name]['total_time'] += result.total_run_time
+ cuml_time += result.total_run_time
+
+ for worker_name in stats:
+ self._print_debug(" %10s: %5d tests, %6.2f secs" % (worker_name, stats[worker_name]['num_tests'], stats[worker_name]['total_time']))
+ self._print_debug(" %6.2f cumulative, %6.2f optimal" % (cuml_time, cuml_time / num_workers))
self._print_debug("")
- self._print_aggregate_test_statistics(individual_test_timings)
- self._print_individual_test_times(individual_test_timings, result_summary)
- self._print_directory_timings(directory_test_timings)
-
- def _print_aggregate_test_statistics(self, individual_test_timings):
- times_for_dump_render_tree = [test_stats.test_run_time for test_stats in individual_test_timings]
+ def _print_aggregate_test_statistics(self, result_summary):
+ times_for_dump_render_tree = [result.test_run_time for result in result_summary.results.values()]
self._print_statistics_for_test_timings("PER TEST TIME IN TESTSHELL (seconds):", times_for_dump_render_tree)
- def _print_individual_test_times(self, individual_test_timings, result_summary):
+ def _print_individual_test_times(self, result_summary):
# Reverse-sort by the time spent in DumpRenderTree.
- individual_test_timings.sort(lambda a, b: cmp(b.test_run_time, a.test_run_time))
+
+ individual_test_timings = sorted(result_summary.results.values(), key=lambda result: result.test_run_time, reverse=True)
num_printed = 0
slow_tests = []
timeout_or_crash_tests = []
@@ -218,18 +226,23 @@
test_run_time = round(test_tuple.test_run_time, 1)
self._print_debug(" %s took %s seconds" % (test_tuple.test_name, test_run_time))
- def _print_directory_timings(self, directory_test_timings):
+ def _print_directory_timings(self, result_summary):
+ stats = {}
+ for result in result_summary.results.values():
+ stats.setdefault(result.shard_name, {'num_tests': 0, 'total_time': 0})
+ stats[result.shard_name]['num_tests'] += 1
+ stats[result.shard_name]['total_time'] += result.total_run_time
+
timings = []
- for directory in directory_test_timings:
- num_tests, time_for_directory = directory_test_timings[directory]
- timings.append((round(time_for_directory, 1), directory, num_tests))
+ for directory in stats:
+ timings.append((directory, round(stats[directory]['total_time'], 1), stats[directory]['num_tests']))
timings.sort()
self._print_debug("Time to process slowest subdirectories:")
min_seconds_to_print = 10
for timing in timings:
if timing[0] > min_seconds_to_print:
- self._print_debug(" %s took %s seconds to run %s tests." % (timing[1], timing[0], timing[2]))
+ self._print_debug(" %s took %s seconds to run %s tests." % timing)
self._print_debug("")
def _print_statistics_for_test_timings(self, title, timings):
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/views/printing_unittest.py (136578 => 136579)
--- trunk/Tools/Scripts/webkitpy/layout_tests/views/printing_unittest.py 2012-12-04 22:47:59 UTC (rev 136578)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/views/printing_unittest.py 2012-12-04 22:50:24 UTC (rev 136579)
@@ -199,7 +199,7 @@
retry.add(self.get_result('passes/text.html'), True, test_is_slow)
retry.add(self.get_result('failures/expected/timeout.html'), True, test_is_slow)
retry.add(self.get_result('failures/expected/crash.html'), True, test_is_slow)
- unexpected_results = manager.summarize_results(self._port, exp, rs, retry, test_timings={}, _only_unexpected_=True)
+ unexpected_results = manager.summarize_results(self._port, exp, rs, retry, _only_unexpected_=True)
return unexpected_results
tests = ['passes/text.html', 'failures/expected/timeout.html', 'failures/expected/crash.html']
