diff options
Diffstat (limited to 'Tools/Scripts/webkitpy')
36 files changed, 792 insertions, 1241 deletions
diff --git a/Tools/Scripts/webkitpy/common/config/committers.py b/Tools/Scripts/webkitpy/common/config/committers.py index f31510022..4a55d495c 100644 --- a/Tools/Scripts/webkitpy/common/config/committers.py +++ b/Tools/Scripts/webkitpy/common/config/committers.py @@ -123,6 +123,7 @@ contributors_who_are_not_committers = [ Contributor("David Barr", "davidbarr@chromium.org", "barrbrain"), Contributor("David Dorwin", "ddorwin@chromium.org", "ddorwin"), Contributor("David Reveman", "reveman@chromium.org", "reveman"), + Contributor("Dongsung Huang", "luxtella@company100.net", "Huang"), Contributor("Douglas Davidson", "ddavidso@apple.com"), Contributor("Edward O'Connor", "eoconnor@apple.com", "hober"), Contributor("Eric Penner", "epenner@chromium.org", "epenner"), @@ -255,6 +256,7 @@ committers_unable_to_review = [ Committer("Hironori Bono", "hbono@chromium.org", "hbono"), Committer("Helder Correia", "helder.correia@nokia.com", "helder"), Committer("Hin-Chung Lam", ["hclam@google.com", "hclam@chromium.org"]), + Committer("Hugo Parente Lima", "hugo.lima@openbossa.org", "hugopl"), Committer("Ian Vollick", "vollick@chromium.org", "vollick"), Committer("Igor Trindade Oliveira", ["igor.oliveira@webkit.org", "igor.o@sisa.samsung.com"], "igoroliveira"), Committer("Ilya Sherman", "isherman@chromium.org", "isherman"), @@ -303,7 +305,7 @@ committers_unable_to_review = [ Committer("Konrad Piascik", "kpiascik@rim.com", "kpiascik"), Committer("Kristof Kosztyo", "kkristof@inf.u-szeged.hu", "kkristof"), Committer("Krzysztof Kowalczyk", "kkowalczyk@gmail.com"), - Committer("Kwang Yul Seo", ["skyul@company100.net", "kseo@webkit.org"], "kwangseo"), + Committer("Kwang Yul Seo", ["skyul@company100.net", "kseo@webkit.org"], "kseo"), Committer("Leandro Gracia Gil", "leandrogracia@chromium.org", "leandrogracia"), Committer("Leandro Pereira", ["leandro@profusion.mobi", "leandro@webkit.org"], "acidx"), Committer("Leo Yang", ["leo.yang@torchmobile.com.cn", "leoyang@webkit.org", "leoyang.webkit@gmail.com", "leo.yang.c@gmail.com"], "leoyang"), diff --git a/Tools/Scripts/webkitpy/common/message_pool.py b/Tools/Scripts/webkitpy/common/message_pool.py new file mode 100644 index 000000000..2d90cbe0b --- /dev/null +++ b/Tools/Scripts/webkitpy/common/message_pool.py @@ -0,0 +1,311 @@ +# Copyright (C) 2011 Google Inc. All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +"""Module for handling messages and concurrency for run-webkit-tests +and test-webkitpy. This module follows the design for multiprocessing.Pool +and concurrency.futures.ProcessPoolExecutor, with the following differences: + +* Tasks are executed in stateful subprocesses via objects that implement the + Worker interface - this allows the workers to share state across tasks. +* The pool provides an asynchronous event-handling interface so the caller + may receive events as tasks are processed. + +If you don't need these features, use multiprocessing.Pool or concurrency.futures +intead. + +""" + +import cPickle +import logging +import multiprocessing +import Queue +import sys +import time +import traceback + + +from webkitpy.common.host import Host +from webkitpy.common.system import stack_utils + + +_log = logging.getLogger(__name__) + + +def get(caller, worker_factory, num_workers, worker_startup_delay_secs=0.0, host=None): + """Returns an object that exposes a run() method that takes a list of test shards and runs them in parallel.""" + return _MessagePool(caller, worker_factory, num_workers, worker_startup_delay_secs, host) + + +class _MessagePool(object): + def __init__(self, caller, worker_factory, num_workers, worker_startup_delay_secs=0.0, host=None): + self._caller = caller + self._worker_factory = worker_factory + self._num_workers = num_workers + self._worker_startup_delay_secs = worker_startup_delay_secs + self._workers = [] + self._workers_stopped = set() + self._host = host + self._name = 'manager' + self._running_inline = (self._num_workers == 1) + if self._running_inline: + self._messages_to_worker = Queue.Queue() + self._messages_to_manager = Queue.Queue() + else: + self._messages_to_worker = multiprocessing.Queue() + self._messages_to_manager = multiprocessing.Queue() + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_value, exc_traceback): + self._close() + return False + + def run(self, shards): + """Posts a list of messages to the pool and waits for them to complete.""" + for message in shards: + self._messages_to_worker.put(_Message(self._name, message[0], message[1:], from_user=True, logs=())) + + for _ in xrange(self._num_workers): + self._messages_to_worker.put(_Message(self._name, 'stop', message_args=(), from_user=False, logs=())) + + self.wait() + + def _start_workers(self): + assert not self._workers + self._workers_stopped = set() + host = None + if self._running_inline or self._can_pickle(self._host): + host = self._host + + for worker_number in xrange(self._num_workers): + worker = _Worker(host, self._messages_to_manager, self._messages_to_worker, self._worker_factory, worker_number, self._running_inline, self if self._running_inline else None) + self._workers.append(worker) + worker.start() + if self._worker_startup_delay_secs: + time.sleep(self._worker_startup_delay_secs) + + def wait(self): + try: + self._start_workers() + if self._running_inline: + self._workers[0].run() + self._loop(block=False) + else: + self._loop(block=True) + finally: + self._close() + + def _close(self): + for worker in self._workers: + if worker.is_alive(): + worker.terminate() + worker.join() + self._workers = [] + if not self._running_inline: + # FIXME: This is a hack to get multiprocessing to not log tracebacks during shutdown :(. + multiprocessing.util._exiting = True + if self._messages_to_worker: + self._messages_to_worker.close() + self._messages_to_worker = None + if self._messages_to_manager: + self._messages_to_manager.close() + self._messages_to_manager = None + + def _log_messages(self, messages): + for message in messages: + logging.root.handle(message) + + def _handle_done(self, source): + self._workers_stopped.add(source) + + @staticmethod + def _handle_worker_exception(source, exception_type, exception_value, _): + if exception_type == KeyboardInterrupt: + raise exception_type(exception_value) + raise WorkerException(str(exception_value)) + + def _can_pickle(self, host): + try: + cPickle.dumps(host) + return True + except TypeError: + return False + + def _loop(self, block): + try: + while True: + if len(self._workers_stopped) == len(self._workers): + block = False + message = self._messages_to_manager.get(block) + self._log_messages(message.logs) + if message.from_user: + self._caller.handle(message.name, message.src, *message.args) + continue + method = getattr(self, '_handle_' + message.name) + assert method, 'bad message %s' % repr(message) + method(message.src, *message.args) + except Queue.Empty: + pass + + +class WorkerException(Exception): + """Raised when we receive an unexpected/unknown exception from a worker.""" + pass + + +class _Message(object): + def __init__(self, src, message_name, message_args, from_user, logs): + self.src = src + self.name = message_name + self.args = message_args + self.from_user = from_user + self.logs = logs + + def __repr__(self): + return '_Message(src=%s, name=%s, args=%s, from_user=%s, logs=%s)' % (self.src, self.name, self.args, self.from_user, self.logs) + + +class _Worker(multiprocessing.Process): + def __init__(self, host, messages_to_manager, messages_to_worker, worker_factory, worker_number, running_inline, manager): + super(_Worker, self).__init__() + self.host = host + self.worker_number = worker_number + self.name = 'worker/%d' % worker_number + self.log_messages = [] + self._running_inline = running_inline + self._manager = manager + + self._messages_to_manager = messages_to_manager + self._messages_to_worker = messages_to_worker + self._worker = worker_factory(self) + self._logger = None + self._log_handler = None + + def terminate(self): + if self._worker: + if hasattr(self._worker, 'stop'): + self._worker.stop() + self._worker = None + if self.is_alive(): + super(_Worker, self).terminate() + + def _close(self): + if self._log_handler and self._logger: + self._logger.removeHandler(self._log_handler) + self._log_handler = None + self._logger = None + + def start(self): + if not self._running_inline: + super(_Worker, self).start() + + def run(self): + if not self.host: + self.host = Host() + if not self._running_inline: + self._set_up_logging() + + worker = self._worker + exception_msg = "" + _log.debug("%s starting" % self.name) + + try: + if hasattr(worker, 'start'): + worker.start() + while True: + message = self._messages_to_worker.get() + if message.from_user: + worker.handle(message.name, message.src, *message.args) + self._yield_to_manager() + else: + assert message.name == 'stop', 'bad message %s' % repr(message) + break + + _log.debug("%s exiting" % self.name) + except Queue.Empty: + assert False, '%s: ran out of messages in worker queue.' % self.name + except KeyboardInterrupt, e: + self._raise(sys.exc_info()) + except Exception, e: + self._raise(sys.exc_info()) + finally: + try: + if hasattr(worker, 'stop'): + worker.stop() + finally: + self._post(name='done', args=(), from_user=False) + self._close() + + def post(self, name, *args): + self._post(name, args, from_user=True) + self._yield_to_manager() + + def _yield_to_manager(self): + if self._running_inline: + self._manager._loop(block=False) + + def _post(self, name, args, from_user): + log_messages = self.log_messages + self.log_messages = [] + self._messages_to_manager.put(_Message(self.name, name, args, from_user, log_messages)) + + def _raise(self, exc_info): + exception_type, exception_value, exception_traceback = exc_info + if self._running_inline: + raise exception_type, exception_value, exception_traceback + + if exception_type == KeyboardInterrupt: + _log.debug("%s: interrupted, exiting" % self.name) + stack_utils.log_traceback(_log.debug, exception_traceback) + else: + _log.error("%s: %s('%s') raised:" % (self.name, exception_value.__class__.__name__, str(exception_value))) + stack_utils.log_traceback(_log.error, exception_traceback) + # Since tracebacks aren't picklable, send the extracted stack instead. + stack = traceback.extract_tb(exception_traceback) + self._post(name='worker_exception', args=(exception_type, exception_value, stack), from_user=False) + + def _set_up_logging(self): + self._logger = logging.getLogger() + + # The unix multiprocessing implementation clones any log handlers into the child process, + # so we remove them to avoid duplicate logging. + for h in self._logger.handlers: + self._logger.removeHandler(h) + + self._log_handler = _WorkerLogHandler(self) + self._logger.addHandler(self._log_handler) + + +class _WorkerLogHandler(logging.Handler): + def __init__(self, worker): + logging.Handler.__init__(self) + self._worker = worker + + def emit(self, record): + self._worker.log_messages.append(record) diff --git a/Tools/Scripts/webkitpy/common/system/user.py b/Tools/Scripts/webkitpy/common/system/user.py index 262b97944..c49429c0d 100644 --- a/Tools/Scripts/webkitpy/common/system/user.py +++ b/Tools/Scripts/webkitpy/common/system/user.py @@ -90,13 +90,21 @@ class User(object): def _wait_on_list_response(cls, list_items, can_choose_multiple, raw_input): while True: if can_choose_multiple: - response = cls.prompt("Enter one or more numbers (comma-separated), or \"all\": ", raw_input=raw_input) + response = cls.prompt("Enter one or more numbers (comma-separated) or ranges (e.g. 3-7), or \"all\": ", raw_input=raw_input) if not response.strip() or response == "all": return list_items + try: - indices = [int(r) - 1 for r in re.split("\s*,\s*", response)] + indices = [] + for value in re.split("\s*,\s*", response): + parts = value.split('-') + if len(parts) == 2: + indices += range(int(parts[0]) - 1, int(parts[1])) + else: + indices.append(int(value) - 1) except ValueError, err: continue + return [list_items[i] for i in indices] else: try: diff --git a/Tools/Scripts/webkitpy/common/system/user_unittest.py b/Tools/Scripts/webkitpy/common/system/user_unittest.py index 8b7cc1c0c..86b9db7d1 100644 --- a/Tools/Scripts/webkitpy/common/system/user_unittest.py +++ b/Tools/Scripts/webkitpy/common/system/user_unittest.py @@ -59,9 +59,9 @@ class UserTest(unittest.TestCase): actual_result = output_capture.assert_outputs( self, User.prompt_with_multiple_lists, - args=["title", ["subtitle1", "subtitle2"], [["foo", "bar"], ["foobar", "barbaz"]]], + args=["title", ["subtitle1", "subtitle2"], [["foo", "bar"], ["foobar", "barbaz", "foobaz"]]], kwargs={"can_choose_multiple": can_choose_multiple, "raw_input": mock_raw_input}, - expected_stdout="title\n\nsubtitle1\n 1. foo\n 2. bar\n\nsubtitle2\n 3. foobar\n 4. barbaz\n") + expected_stdout="title\n\nsubtitle1\n 1. foo\n 2. bar\n\nsubtitle2\n 3. foobar\n 4. barbaz\n 5. foobaz\n") self.assertEqual(actual_result, expected_result) self.assertEqual(len(inputs), 0) @@ -69,13 +69,17 @@ class UserTest(unittest.TestCase): run_prompt_test(["badinput", "2"], "bar") run_prompt_test(["3"], "foobar") run_prompt_test(["4"], "barbaz") + run_prompt_test(["5"], "foobaz") run_prompt_test(["1,2"], ["foo", "bar"], can_choose_multiple=True) + run_prompt_test(["1-3"], ["foo", "bar", "foobar"], can_choose_multiple=True) + run_prompt_test(["1-2,3"], ["foo", "bar", "foobar"], can_choose_multiple=True) + run_prompt_test(["2-1,3"], ["foobar"], can_choose_multiple=True) run_prompt_test([" 1, 2 "], ["foo", "bar"], can_choose_multiple=True) - run_prompt_test(["all"], ["foo", "bar", 'foobar', 'barbaz'], can_choose_multiple=True) - run_prompt_test([""], ["foo", "bar", 'foobar', 'barbaz'], can_choose_multiple=True) - run_prompt_test([" "], ["foo", "bar", 'foobar', 'barbaz'], can_choose_multiple=True) - run_prompt_test(["badinput", "all"], ["foo", "bar", 'foobar', 'barbaz'], can_choose_multiple=True) + run_prompt_test(["all"], ["foo", "bar", 'foobar', 'barbaz', 'foobaz'], can_choose_multiple=True) + run_prompt_test([""], ["foo", "bar", 'foobar', 'barbaz', 'foobaz'], can_choose_multiple=True) + run_prompt_test([" "], ["foo", "bar", 'foobar', 'barbaz', 'foobaz'], can_choose_multiple=True) + run_prompt_test(["badinput", "all"], ["foo", "bar", 'foobar', 'barbaz', 'foobaz'], can_choose_multiple=True) def test_prompt_with_list(self): def run_prompt_test(inputs, expected_result, can_choose_multiple=False): diff --git a/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py b/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py index f2fed3f4a..7aee0c2fb 100644 --- a/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py +++ b/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py @@ -44,7 +44,7 @@ import re import sys import time -from webkitpy.layout_tests.controllers import manager_worker_broker +from webkitpy.common import message_pool from webkitpy.layout_tests.controllers import worker from webkitpy.layout_tests.controllers.test_result_writer import TestResultWriter from webkitpy.layout_tests.layout_package import json_layout_results_generator @@ -88,11 +88,9 @@ def interpret_test_failures(port, test_name, failures): test_dict['image_diff_percent'] = failure.diff_percent elif isinstance(failure, test_failures.FailureReftestMismatch): test_dict['is_reftest'] = True - test_dict['ref_file'] = port.relative_test_filename(failure.reference_filename) test_dict['image_diff_percent'] = failure.diff_percent elif isinstance(failure, test_failures.FailureReftestMismatchDidNotOccur): test_dict['is_mismatch_reftest'] = True - test_dict['ref_file'] = port.relative_test_filename(failure.reference_filename) if test_failures.FailureMissingResult in failure_types: test_dict['is_missing_text'] = True @@ -266,7 +264,8 @@ class TestRunInterruptedException(Exception): return self.__class__, (self.reason,) -WorkerException = manager_worker_broker.WorkerException +# Export this so callers don't need to know about message pools. +WorkerException = message_pool.WorkerException class TestShard(object): @@ -300,7 +299,6 @@ class Manager(object): self._filesystem = port.host.filesystem self._options = options self._printer = printer - self._message_broker = None self._expectations = None self.HTTP_SUBDIR = 'http' + port.TEST_PATH_SEPARATOR @@ -328,11 +326,9 @@ class Manager(object): self._all_results = [] self._group_stats = {} + self._worker_stats = {} self._current_result_summary = None - # This maps worker names to the state we are tracking for each of them. - self._worker_states = {} - def collect_tests(self, args): """Find all the files to test. @@ -484,20 +480,13 @@ class Manager(object): # now make sure we're explicitly running any tests passed on the command line. self._test_files.update(found_test_files.intersection(self._paths)) - if not num_all_test_files: + num_to_run = len(self._test_files) + num_skipped = num_all_test_files - num_to_run + + if not num_to_run: _log.critical('No tests to run.') return None - num_skipped = num_all_test_files - len(self._test_files) - if num_skipped: - self._printer.print_expected("Running %s (found %d, skipping %d)." % ( - grammar.pluralize('test', num_all_test_files - num_skipped), - num_all_test_files, num_skipped)) - elif len(self._test_files) > 1: - self._printer.print_expected("Running all %d tests." % len(self._test_files)) - else: - self._printer.print_expected("Running %1 test.") - # Create a sorted list of test files so the subset chunk, # if used, contains alphabetically consecutive tests. self._test_files_list = list(self._test_files) @@ -522,6 +511,8 @@ class Manager(object): (self._options.repeat_each if self._options.repeat_each else 1) * \ (self._options.iterations if self._options.iterations else 1) result_summary = ResultSummary(self._expectations, self._test_files | skipped, iterations) + + self._printer.print_expected('Found %s.' % grammar.pluralize('test', num_all_test_files)) self._print_expected_results_of_type(result_summary, test_expectations.PASS, "passes") self._print_expected_results_of_type(result_summary, test_expectations.FAIL, "failures") self._print_expected_results_of_type(result_summary, test_expectations.FLAKY, "flaky") @@ -534,17 +525,16 @@ class Manager(object): for test in skipped: result = test_results.TestResult(test) result.type = test_expectations.SKIP - iterations = \ - (self._options.repeat_each if self._options.repeat_each else 1) * \ - (self._options.iterations if self._options.iterations else 1) for iteration in range(iterations): result_summary.add(result, expected=True) self._printer.print_expected('') - # Check to make sure we didn't filter out all of the tests. - if not len(self._test_files): - _log.info("All tests are being skipped") - return None + if self._options.repeat_each > 1: + self._printer.print_expected('Running each test %d times.' % self._options.repeat_each) + if self._options.iterations > 1: + self._printer.print_expected('Running %d iterations of the tests.' % self._options.iterations) + if iterations > 1: + self._printer.print_expected('') return result_summary @@ -719,11 +709,12 @@ class Manager(object): def _log_num_workers(self, num_workers, num_shards, num_locked_shards): driver_name = self._port.driver_name() if num_workers == 1: - self._printer.print_config("Running 1 %s over %s" % + self._printer.print_config("Running 1 %s over %s." % (driver_name, grammar.pluralize('shard', num_shards))) else: - self._printer.print_config("Running %d %ss in parallel over %d shards (%d locked)" % + self._printer.print_config("Running %d %ss in parallel over %d shards (%d locked)." % (num_workers, driver_name, num_shards, num_locked_shards)) + self._printer.print_config('') def _run_tests(self, file_list, result_summary, num_workers): """Runs the tests in the file_list. @@ -744,11 +735,10 @@ class Manager(object): self._current_result_summary = result_summary self._all_results = [] self._group_stats = {} - self._worker_states = {} + self._worker_stats = {} keyboard_interrupted = False interrupted = False - thread_timings = [] self._printer.print_update('Sharding tests ...') locked_shards, unlocked_shards = self._shard_tests(file_list, int(self._options.child_processes), self._options.fully_parallel) @@ -769,71 +759,32 @@ class Manager(object): num_workers = min(num_workers, len(all_shards)) self._log_num_workers(num_workers, len(all_shards), len(locked_shards)) - def worker_factory(worker_connection, worker_number): - return worker.Worker(worker_connection, worker_number, self.results_directory(), self._options) - - manager_connection = manager_worker_broker.get(num_workers, self, worker_factory, self._port.host) + def worker_factory(worker_connection): + return worker.Worker(worker_connection, self.results_directory(), self._options) if self._options.dry_run: - return (keyboard_interrupted, interrupted, thread_timings, self._group_stats, self._all_results) + return (keyboard_interrupted, interrupted, self._worker_stats.values(), self._group_stats, self._all_results) self._printer.print_update('Starting %s ...' % grammar.pluralize('worker', num_workers)) - for worker_number in xrange(num_workers): - worker_connection = manager_connection.start_worker(worker_number) - worker_state = _WorkerState(worker_number, worker_connection) - self._worker_states[worker_connection.name()] = worker_state - - time.sleep(self._port.worker_startup_delay_secs()) - - self._printer.print_update("Starting testing ...") - for shard in all_shards: - # FIXME: Change 'test_list' to 'shard', make sharding public. - manager_connection.post_message('test_list', shard.name, shard.test_inputs) - - # We post one 'stop' message for each worker. Because the stop message - # are sent after all of the tests, and because each worker will stop - # reading messsages after receiving a stop, we can be sure each - # worker will get a stop message and hence they will all shut down. - for _ in xrange(num_workers): - manager_connection.post_message('stop') try: - while not self.is_done(): - manager_connection.run_message_loop(delay_secs=1.0) - - # Make sure all of the workers have shut down (if possible). - for worker_state in self._worker_states.values(): - if worker_state.worker_connection.is_alive(): - _log.debug('Waiting for worker %d to exit' % worker_state.number) - worker_state.worker_connection.join(5.0) - if worker_state.worker_connection.is_alive(): - _log.error('Worker %d did not exit in time.' % worker_state.number) - + with message_pool.get(self, worker_factory, num_workers, self._port.worker_startup_delay_secs(), self._port.host) as pool: + pool.run(('test_list', shard.name, shard.test_inputs) for shard in all_shards) except KeyboardInterrupt: self._printer.flush() self._printer.write('Interrupted, exiting ...') - self.cancel_workers() keyboard_interrupted = True except TestRunInterruptedException, e: _log.warning(e.reason) - self.cancel_workers() interrupted = True - except WorkerException: - self.cancel_workers() - raise - except: - # Unexpected exception; don't try to clean up workers. - _log.error("Exception raised, exiting") - self.cancel_workers() + except Exception, e: + _log.debug('%s("%s") raised, exiting' % (e.__class__.__name__, str(e))) raise finally: - manager_connection.cleanup() self.stop_servers_with_lock() - thread_timings = [worker_state.stats for worker_state in self._worker_states.values()] - # FIXME: should this be a class instead of a tuple? - return (interrupted, keyboard_interrupted, thread_timings, self._group_stats, self._all_results) + return (interrupted, keyboard_interrupted, self._worker_stats.values(), self._group_stats, self._all_results) def results_directory(self): if not self._retrying: @@ -934,7 +885,7 @@ class Manager(object): self._print_timing_statistics(end_time - start_time, thread_timings, test_timings, individual_test_timings, result_summary) self._print_result_summary(result_summary) - self._printer.print_one_line_summary(result_summary.total, result_summary.expected, result_summary.unexpected) + self._printer.print_one_line_summary(result_summary.total - result_summary.expected_skips, result_summary.expected - result_summary.expected_skips, result_summary.unexpected) unexpected_results = summarize_results(self._port, self._expectations, result_summary, retry_summary, individual_test_timings, only_unexpected=True, interrupted=interrupted) self._printer.print_unexpected_results(unexpected_results) @@ -1375,9 +1326,8 @@ class Manager(object): result_summary: information to log """ failed = result_summary.total_failures - skipped = result_summary.total_tests_by_expectation[test_expectations.SKIP] - total = result_summary.total - passed = total - failed - skipped + total = result_summary.total - result_summary.expected_skips + passed = total - failed pct_passed = 0.0 if total > 0: pct_passed = float(passed) * 100 / total @@ -1442,42 +1392,17 @@ class Manager(object): results_filename = self._filesystem.join(self._results_directory, "results.html") self._port.show_results_html_file(results_filename) - def name(self): - return 'Manager' - - def is_done(self): - worker_states = self._worker_states.values() - return worker_states and all(self._worker_is_done(worker_state) for worker_state in worker_states) - - # FIXME: Inline this function. - def _worker_is_done(self, worker_state): - return worker_state.done - - def cancel_workers(self): - for worker_state in self._worker_states.values(): - worker_state.worker_connection.cancel() - - def handle_started_test(self, source, test_info, hang_timeout): - worker_state = self._worker_states[source] - worker_state.current_test_name = test_info.test_name - worker_state.next_timeout = time.time() + hang_timeout - - def handle_done(self, source, log_messages=None): - worker_state = self._worker_states[source] - worker_state.done = True - self._log_messages(log_messages) - - def handle_exception(self, source, exception_type, exception_value, stack): - if exception_type in (KeyboardInterrupt, TestRunInterruptedException): - raise exception_type(exception_value) - _log.error("%s raised %s('%s'):" % ( - source, - exception_value.__class__.__name__, - str(exception_value))) - self._log_worker_stack(stack) - raise WorkerException(str(exception_value)) - - def handle_finished_list(self, source, list_name, num_tests, elapsed_time): + def handle(self, name, source, *args): + method = getattr(self, '_handle_' + name) + if method: + return method(source, *args) + raise AssertionError('unknown message %s received from %s, args=%s' % (name, source, repr(args))) + + def _handle_started_test(self, worker_name, test_input, test_timeout_sec): + # FIXME: log that we've started another test. + pass + + def _handle_finished_test_list(self, worker_name, list_name, num_tests, elapsed_time): self._group_stats[list_name] = (num_tests, elapsed_time) def find(name, test_lists): @@ -1492,29 +1417,13 @@ class Manager(object): if not self._remaining_locked_shards: self.stop_servers_with_lock() - def handle_finished_test(self, source, result, elapsed_time, log_messages=None): - worker_state = self._worker_states[source] - worker_state.next_timeout = None - worker_state.current_test_name = None - worker_state.stats['total_time'] += elapsed_time - worker_state.stats['num_tests'] += 1 - - self._log_messages(log_messages) + 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) self._update_summary_with_result(self._current_result_summary, result) - def _log_messages(self, messages): - for message in messages: - logging.root.handle(message) - - def _log_worker_stack(self, stack): - webkitpydir = self._port.path_from_webkit_base('Tools', 'Scripts', 'webkitpy') + self._filesystem.sep - for filename, line_number, function_name, text in stack: - if filename.startswith(webkitpydir): - filename = filename.replace(webkitpydir, '') - _log.error(' %s:%u (in %s)' % (filename, line_number, function_name)) - _log.error(' %s' % text) - def read_test_files(fs, filenames, test_path_separator): tests = [] @@ -1563,20 +1472,3 @@ def natural_sort_key(string_to_split): return val return [tryint(chunk) for chunk in re.split('(\d+)', string_to_split)] - - -class _WorkerState(object): - """A class for the manager to use to track the current state of the workers.""" - def __init__(self, number, worker_connection): - self.worker_connection = worker_connection - self.number = number - self.done = False - self.current_test_name = None - self.next_timeout = None - self.stats = {} - self.stats['name'] = worker_connection.name() - self.stats['num_tests'] = 0 - self.stats['total_time'] = 0 - - def __repr__(self): - return "_WorkerState(" + str(self.__dict__) + ")" diff --git a/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py b/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py index 3496322a9..27f06a70e 100644 --- a/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py +++ b/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py @@ -452,7 +452,6 @@ class ResultSummaryTest(unittest.TestCase): [test_failures.FailureReftestMismatch(self.port.abspath_for_test('foo/common.html'))]) self.assertTrue('is_reftest' in test_dict) self.assertFalse('is_mismatch_reftest' in test_dict) - self.assertEqual(test_dict['ref_file'], 'foo/common.html') test_dict = interpret_test_failures(self.port, 'foo/reftest.html', [test_failures.FailureReftestMismatchDidNotOccur(self.port.abspath_for_test('foo/reftest-expected-mismatch.html'))]) @@ -463,7 +462,6 @@ class ResultSummaryTest(unittest.TestCase): [test_failures.FailureReftestMismatchDidNotOccur(self.port.abspath_for_test('foo/common.html'))]) self.assertFalse('is_reftest' in test_dict) self.assertTrue(test_dict['is_mismatch_reftest']) - self.assertEqual(test_dict['ref_file'], 'foo/common.html') def get_result(self, test_name, result_type=test_expectations.PASS, run_time=0): failures = [] diff --git a/Tools/Scripts/webkitpy/layout_tests/controllers/manager_worker_broker.py b/Tools/Scripts/webkitpy/layout_tests/controllers/manager_worker_broker.py deleted file mode 100755 index f7baced0a..000000000 --- a/Tools/Scripts/webkitpy/layout_tests/controllers/manager_worker_broker.py +++ /dev/null @@ -1,515 +0,0 @@ -# Copyright (C) 2011 Google Inc. All rights reserved. -# -# Redistribution and use in source and binary forms, with or without -# modification, are permitted provided that the following conditions are -# met: -# -# * Redistributions of source code must retain the above copyright -# notice, this list of conditions and the following disclaimer. -# * Redistributions in binary form must reproduce the above -# copyright notice, this list of conditions and the following disclaimer -# in the documentation and/or other materials provided with the -# distribution. -# * Neither the name of Google Inc. nor the names of its -# contributors may be used to endorse or promote products derived from -# this software without specific prior written permission. -# -# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - -"""Module for handling messages and concurrency for run-webkit-tests. - -This module implements a message broker that connects the manager to the -workers: it provides a messaging abstraction and message loops (building on -top of message_broker), and handles starting workers by launching processes. - -There are a lot of classes and objects involved in a fully connected system. -They interact more or less like: - - Manager --> _InlineManager ---> _InlineWorker <-> Worker - ^ \ / ^ - | v v | - \----------------------- Broker ----------------/ - -The broker simply distributes messages onto topics (named queues); the actual -queues themselves are provided by the caller, as the queue's implementation -requirements varies vary depending on the desired concurrency model -(none/threads/processes). - -In order for shared-nothing messaging between processing to be possible, -Messages must be picklable. - -The module defines one interface and two classes. Callers of this package -must implement the BrokerClient interface, and most callers will create -_BrokerConnections as well as Brokers. - -The classes relate to each other as: - - BrokerClient ------> _BrokerConnection - ^ | - | v - \---------------- _Broker - -(The BrokerClient never calls broker directly after it is created, only -_BrokerConnection. _BrokerConnection passes a reference to BrokerClient to -_Broker, and _Broker only invokes that reference, never talking directly to -BrokerConnection). -""" - -import cPickle -import logging -import multiprocessing -import optparse -import os -import Queue -import sys -import traceback - - -from webkitpy.common.host import Host -from webkitpy.common.system import stack_utils -from webkitpy.layout_tests.views import metered_stream - - -_log = logging.getLogger(__name__) - - -# -# Topic names for Manager <-> Worker messaging -# -MANAGER_TOPIC = 'managers' -ANY_WORKER_TOPIC = 'workers' - - -def get(max_workers, client, worker_factory, host=None): - """Return a connection to a manager/worker message_broker - - Args: - max_workers - max # of workers to run concurrently. - client - BrokerClient implementation to dispatch - replies to. - worker_factory: factory method for creating objects that implement the Worker interface. - host: optional picklable host object that can be passed to workers for testing. - Returns: - A handle to an object that will talk to a message broker configured - for the normal manager/worker communication.""" - if max_workers == 1: - queue_class = Queue.Queue - manager_class = _InlineManager - else: - queue_class = multiprocessing.Queue - manager_class = _MultiProcessManager - - broker = _Broker(queue_class) - return manager_class(broker, client, worker_factory, host) - - -class WorkerException(Exception): - """Raised when we receive an unexpected/unknown exception from a worker.""" - pass - - -class BrokerClient(object): - """Abstract base class / interface that all message broker clients must - implement. In addition to the methods below, by convention clients - implement routines of the signature type - - handle_MESSAGE_NAME(self, src, ...): - - where MESSAGE_NAME matches the string passed to post_message(), and - src indicates the name of the sender. If the message contains values in - the message body, those will be provided as optparams.""" - - def is_done(self): - """Called from inside run_message_loop() to indicate whether to exit.""" - raise NotImplementedError - - def name(self): - """Return a name that identifies the client.""" - raise NotImplementedError - - -class _Broker(object): - """Brokers provide the basic model of a set of topics. Clients can post a - message to any topic using post_message(), and can process messages on one - topic at a time using run_message_loop().""" - - def __init__(self, queue_maker): - """Args: - queue_maker: a factory method that returns objects implementing a - Queue interface (put()/get()). - """ - self._queue_maker = queue_maker - self._topics = {} - - def __del__(self): - self.cleanup() - - def cleanup(self): - for queue in self._topics.values(): - if hasattr(queue, 'close'): - queue.close() - self._topics = {} - - def add_topic(self, topic_name): - if topic_name not in self._topics: - self._topics[topic_name] = self._queue_maker() - - def _get_queue_for_topic(self, topic_name): - return self._topics[topic_name] - - def post_message(self, client, topic_name, message_name, *message_args): - """Post a message to the appropriate topic name. - - Messages have a name and a tuple of optional arguments. Both must be picklable.""" - message = _Message(client.name(), topic_name, message_name, message_args) - queue = self._get_queue_for_topic(topic_name) - queue.put(_Message.dumps(message)) - - def run_message_loop(self, topic_name, client, delay_secs=None): - """Loop processing messages until client.is_done() or delay passes. - - To run indefinitely, set delay_secs to None.""" - assert delay_secs is None or delay_secs > 0 - self._run_loop(topic_name, client, block=True, delay_secs=delay_secs) - - def run_all_pending(self, topic_name, client): - """Process messages until client.is_done() or caller would block.""" - self._run_loop(topic_name, client, block=False, delay_secs=None) - - def _run_loop(self, topic_name, client, block, delay_secs): - queue = self._get_queue_for_topic(topic_name) - while not client.is_done(): - try: - s = queue.get(block, delay_secs) - except Queue.Empty: - return - msg = _Message.loads(s) - self._dispatch_message(msg, client) - - def _dispatch_message(self, message, client): - if not hasattr(client, 'handle_' + message.name): - raise ValueError( - "%s: received message '%s' it couldn't handle" % - (client.name(), message.name)) - optargs = message.args - message_handler = getattr(client, 'handle_' + message.name) - message_handler(message.src, *optargs) - - -class _Message(object): - @staticmethod - def loads(string_value): - obj = cPickle.loads(string_value) - assert(isinstance(obj, _Message)) - return obj - - def __init__(self, src, topic_name, message_name, message_args): - self.src = src - self.topic_name = topic_name - self.name = message_name - self.args = message_args - - def dumps(self): - return cPickle.dumps(self) - - def __repr__(self): - return ("_Message(from='%s', topic_name='%s', message_name='%s')" % - (self.src, self.topic_name, self.name)) - - -class _BrokerConnection(object): - """_BrokerConnection provides a connection-oriented facade on top of a - Broker, so that callers don't have to repeatedly pass the same topic - names over and over.""" - - def __init__(self, broker, client, run_topic, post_topic): - """Create a _BrokerConnection on top of a _Broker. Note that the _Broker - is passed in rather than created so that a single _Broker can be used - by multiple _BrokerConnections.""" - self._broker = broker - self._client = client - self._post_topic = post_topic - self._run_topic = run_topic - broker.add_topic(run_topic) - broker.add_topic(post_topic) - - def cleanup(self): - self._broker.cleanup() - self._broker = None - - def run_message_loop(self, delay_secs=None): - self._broker.run_message_loop(self._run_topic, self._client, delay_secs) - - def post_message(self, message_name, *message_args): - self._broker.post_message(self._client, self._post_topic, - message_name, *message_args) - - def raise_exception(self, exc_info): - # Since tracebacks aren't picklable, send the extracted stack instead, - # but at least log the full traceback. - exception_type, exception_value, exception_traceback = sys.exc_info() - stack_utils.log_traceback(_log.error, exception_traceback) - stack = traceback.extract_tb(exception_traceback) - self._broker.post_message(self._client, self._post_topic, 'exception', exception_type, exception_value, stack) - - -class AbstractWorker(BrokerClient): - def __init__(self, worker_connection, worker_number): - BrokerClient.__init__(self) - self.worker = None - self._worker_connection = worker_connection - self._worker_number = worker_number - self._name = 'worker/%d' % worker_number - self._done = False - self._canceled = False - self._options = optparse.Values({'verbose': False}) - self.host = None - - def name(self): - return self._name - - def is_done(self): - return self._done or self._canceled - - def stop_handling_messages(self): - self._done = True - - def run(self, host): - """Callback for the worker to start executing. Typically does any - remaining initialization and then calls broker_connection.run_message_loop().""" - exception_msg = "" - self.host = host - - self.worker.safe_init() - _log.debug('%s starting' % self._name) - - try: - self._worker_connection.run_message_loop() - if not self.is_done(): - raise AssertionError("%s: ran out of messages in worker queue." - % self._name) - except KeyboardInterrupt: - exception_msg = ", interrupted" - self._worker_connection.raise_exception(sys.exc_info()) - except: - exception_msg = ", exception raised" - self._worker_connection.raise_exception(sys.exc_info()) - finally: - _log.debug("%s done with message loop%s" % (self._name, exception_msg)) - try: - self.worker.cleanup() - finally: - # Make sure we post a done so that we can flush the log messages - # and clean up properly even if we raise an exception in worker.cleanup(). - self._worker_connection.post_message('done') - - def handle_stop(self, source): - self._done = True - - def handle_test_list(self, source, list_name, test_list): - self.worker.handle('test_list', source, list_name, test_list) - - def cancel(self): - """Called when possible to indicate to the worker to stop processing - messages and shut down. Note that workers may be stopped without this - method being called, so clients should not rely solely on this.""" - self._canceled = True - - def yield_to_broker(self): - self._worker_connection.yield_to_broker() - - def post_message(self, *args): - self._worker_connection.post_message(*args) - - -class _ManagerConnection(_BrokerConnection): - def __init__(self, broker, client, worker_factory, host): - _BrokerConnection.__init__(self, broker, client, MANAGER_TOPIC, ANY_WORKER_TOPIC) - self._worker_factory = worker_factory - self._host = host - - def start_worker(self, worker_number): - raise NotImplementedError - - -class _InlineManager(_ManagerConnection): - def __init__(self, broker, client, worker_factory, host): - _ManagerConnection.__init__(self, broker, client, worker_factory, host) - self._inline_worker = None - - def start_worker(self, worker_number): - host = self._host - self._inline_worker = _InlineWorkerConnection(host, self._broker, self._client, self._worker_factory, worker_number) - return self._inline_worker - - def run_message_loop(self, delay_secs=None): - # Note that delay_secs is ignored in this case since we can't easily - # implement it. - self._inline_worker.run() - self._broker.run_all_pending(MANAGER_TOPIC, self._client) - - -class _MultiProcessManager(_ManagerConnection): - def _can_pickle_host(self): - try: - cPickle.dumps(self._host) - return True - except TypeError: - return False - - def start_worker(self, worker_number): - host = None - if self._can_pickle_host(): - host = self._host - worker_connection = _MultiProcessWorkerConnection(host, self._broker, self._worker_factory, worker_number) - worker_connection.start() - return worker_connection - - -class _WorkerConnection(_BrokerConnection): - def __init__(self, host, broker, worker_factory, worker_number): - # FIXME: keeping track of the differences between the WorkerConnection, the AbstractWorker, and the - # actual Worker (created by worker_factory) is very confusing, but this all gets better when - # _WorkerConnection and AbstractWorker get merged. - self._client = AbstractWorker(self, worker_number) - self._worker = worker_factory(self._client, worker_number) - self._client.worker = self._worker - self._host = host - self._log_messages = [] - self._logger = None - self._log_handler = None - _BrokerConnection.__init__(self, broker, self._client, ANY_WORKER_TOPIC, MANAGER_TOPIC) - - def name(self): - return self._client.name() - - def cancel(self): - raise NotImplementedError - - def is_alive(self): - raise NotImplementedError - - def join(self, timeout): - raise NotImplementedError - - def yield_to_broker(self): - pass - - def post_message(self, *args): - # FIXME: This is a hack until we can remove the log_messages arg from the manager. - if args[0] in ('finished_test', 'done'): - log_messages = self._log_messages - self._log_messages = [] - args = args + tuple([log_messages]) - super(_WorkerConnection, self).post_message(*args) - - def set_up_logging(self): - self._logger = logging.root - # The unix multiprocessing implementation clones the MeteredStream log handler - # into the child process, so we need to remove it to avoid duplicate logging. - for h in self._logger.handlers: - # log handlers don't have names until python 2.7. - if getattr(h, 'name', '') == metered_stream.LOG_HANDLER_NAME: - self._logger.removeHandler(h) - break - self._logger.setLevel(logging.DEBUG if self._client._options.verbose else logging.INFO) - self._log_handler = _WorkerLogHandler(self) - self._logger.addHandler(self._log_handler) - - def clean_up_logging(self): - if self._log_handler and self._logger: - self._logger.removeHandler(self._log_handler) - self._log_handler = None - self._logger = None - - -class _InlineWorkerConnection(_WorkerConnection): - def __init__(self, host, broker, manager_client, worker_factory, worker_number): - _WorkerConnection.__init__(self, host, broker, worker_factory, worker_number) - self._alive = False - self._manager_client = manager_client - - def cancel(self): - self._client.cancel() - - def is_alive(self): - return self._alive - - def join(self, timeout): - assert not self._alive - - def run(self): - self._alive = True - try: - self._client.run(self._host) - finally: - self._alive = False - - def yield_to_broker(self): - self._broker.run_all_pending(MANAGER_TOPIC, self._manager_client) - - def raise_exception(self, exc_info): - # Since the worker is in the same process as the manager, we can - # raise the exception directly, rather than having to send it through - # the queue. This allows us to preserve the traceback, but we log - # it anyway for consistency with the multiprocess case. - exception_type, exception_value, exception_traceback = sys.exc_info() - stack_utils.log_traceback(_log.error, exception_traceback) - raise exception_type, exception_value, exception_traceback - - -class _Process(multiprocessing.Process): - def __init__(self, worker_connection, client): - multiprocessing.Process.__init__(self) - self._worker_connection = worker_connection - self._client = client - - def run(self): - if not self._worker_connection._host: - self._worker_connection._host = Host() - self._worker_connection.run() - - -class _MultiProcessWorkerConnection(_WorkerConnection): - def __init__(self, host, broker, worker_factory, worker_number): - _WorkerConnection.__init__(self, host, broker, worker_factory, worker_number) - self._proc = _Process(self, self._client) - - def cancel(self): - return self._proc.terminate() - - def is_alive(self): - return self._proc.is_alive() - - def join(self, timeout): - return self._proc.join(timeout) - - def start(self): - self._proc.start() - - def run(self): - self.set_up_logging() - try: - self._client.run(self._host) - finally: - self.clean_up_logging() - - -class _WorkerLogHandler(logging.Handler): - def __init__(self, worker): - logging.Handler.__init__(self) - self._worker = worker - self._pid = os.getpid() - - def emit(self, record): - self._worker._log_messages.append(record) diff --git a/Tools/Scripts/webkitpy/layout_tests/controllers/manager_worker_broker_unittest.py b/Tools/Scripts/webkitpy/layout_tests/controllers/manager_worker_broker_unittest.py deleted file mode 100644 index d7c3714d8..000000000 --- a/Tools/Scripts/webkitpy/layout_tests/controllers/manager_worker_broker_unittest.py +++ /dev/null @@ -1,213 +0,0 @@ -# Copyright (C) 2010 Google Inc. All rights reserved. -# -# Redistribution and use in source and binary forms, with or without -# modification, are permitted provided that the following conditions are -# met: -# -# * Redistributions of source code must retain the above copyright -# notice, this list of conditions and the following disclaimer. -# * Redistributions in binary form must reproduce the above -# copyright notice, this list of conditions and the following disclaimer -# in the documentation and/or other materials provided with the -# distribution. -# * Neither the name of Google Inc. nor the names of its -# contributors may be used to endorse or promote products derived from -# this software without specific prior written permission. -# -# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - -import optparse -import Queue -import sys -import unittest - -from webkitpy.common.system import outputcapture -from webkitpy.layout_tests.controllers import manager_worker_broker - - -# In order to reliably control when child workers are starting and stopping, -# we use a pair of global variables to hold queues used for messaging. Ideally -# we wouldn't need globals, but we can't pass these through a lexical closure -# because those can't be Pickled and sent to a subprocess, and we'd prefer not -# to have to pass extra arguments to the worker in the start_worker() call. -starting_queue = None -stopping_queue = None - - -WORKER_NAME = 'worker/1' - -def make_broker(manager, max_workers, start_queue=None, stop_queue=None): - global starting_queue - global stopping_queue - starting_queue = start_queue - stopping_queue = stop_queue - return manager_worker_broker.get(max_workers, manager, _TestWorker) - - -class _TestWorker(object): - def __init__(self, caller, worker_number): - self._caller = caller - self._thing_to_greet = 'everybody' - self._starting_queue = starting_queue - self._stopping_queue = stopping_queue - self._options = optparse.Values({'verbose': False}) - - def name(self): - return WORKER_NAME - - def cleanup(self): - pass - - def handle(self, message, src, an_int, a_str): - assert an_int == 1 - assert a_str == "hello, world" - self._caller.post_message('finished_test', 2) - - def safe_init(self): - if self._starting_queue: - self._starting_queue.put('') - - if self._stopping_queue: - self._stopping_queue.get() - - def stop(self): - self._caller.post_message('done') - - -class FunctionTests(unittest.TestCase): - def test_get__inline(self): - self.assertTrue(make_broker(self, 1) is not None) - - def test_get__processes(self): - # This test sometimes fails on Windows. See <http://webkit.org/b/55087>. - if sys.platform in ('cygwin', 'win32'): - return - self.assertTrue(make_broker(self, 2) is not None) - - -class _TestsMixin(object): - """Mixin class that implements a series of tests to enforce the - contract all implementations must follow.""" - - def name(self): - return 'TesterManager' - - def is_done(self): - return self._done - - def handle_done(self, src, log_messages): - self._done = True - - def handle_finished_test(self, src, an_int, log_messages): - self._an_int = an_int - - def handle_exception(self, src, exception_type, exception_value, stack): - raise exception_type(exception_value) - - def setUp(self): - self._an_int = None - self._broker = None - self._done = False - self._exception = None - self._max_workers = None - - def make_broker(self, starting_queue=None, stopping_queue=None): - self._broker = make_broker(self, self._max_workers, starting_queue, - stopping_queue) - - def test_name(self): - self.make_broker() - worker = self._broker.start_worker(1) - self.assertEquals(worker.name(), WORKER_NAME) - worker.cancel() - worker.join(0.1) - self.assertFalse(worker.is_alive()) - self._broker.cleanup() - - def test_cancel(self): - self.make_broker() - worker = self._broker.start_worker(1) - self._broker.post_message('test_list', 1, 'hello, world') - worker.cancel() - worker.join(0.1) - self.assertFalse(worker.is_alive()) - self._broker.cleanup() - - def test_done(self): - self.make_broker() - worker = self._broker.start_worker(1) - self._broker.post_message('test_list', 1, 'hello, world') - self._broker.post_message('stop') - self._broker.run_message_loop() - worker.join(0.5) - self.assertFalse(worker.is_alive()) - self.assertTrue(self.is_done()) - self.assertEqual(self._an_int, 2) - self._broker.cleanup() - - def test_unknown_message(self): - self.make_broker() - worker = self._broker.start_worker(1) - self._broker.post_message('unknown') - try: - self._broker.run_message_loop() - self.fail() - except ValueError, e: - self.assertEquals(str(e), - "%s: received message 'unknown' it couldn't handle" % WORKER_NAME) - finally: - worker.join(0.5) - self.assertFalse(worker.is_alive()) - self._broker.cleanup() - - -class InlineBrokerTests(_TestsMixin, unittest.TestCase): - def setUp(self): - _TestsMixin.setUp(self) - self._max_workers = 1 - - -# FIXME: https://bugs.webkit.org/show_bug.cgi?id=54520. -if sys.platform not in ('cygwin', 'win32'): - - class MultiProcessBrokerTests(_TestsMixin, unittest.TestCase): - def setUp(self): - _TestsMixin.setUp(self) - self._max_workers = 2 - - -class MessageTest(unittest.TestCase): - def test__no_body(self): - msg = manager_worker_broker._Message('src', 'topic_name', 'message_name', None) - self.assertTrue(repr(msg)) - s = msg.dumps() - new_msg = manager_worker_broker._Message.loads(s) - self.assertEqual(new_msg.name, 'message_name') - self.assertEqual(new_msg.args, None) - self.assertEqual(new_msg.topic_name, 'topic_name') - self.assertEqual(new_msg.src, 'src') - - def test__body(self): - msg = manager_worker_broker._Message('src', 'topic_name', 'message_name', ('body', 0)) - self.assertTrue(repr(msg)) - s = msg.dumps() - new_msg = manager_worker_broker._Message.loads(s) - self.assertEqual(new_msg.name, 'message_name') - self.assertEqual(new_msg.args, ('body', 0)) - self.assertEqual(new_msg.topic_name, 'topic_name') - self.assertEqual(new_msg.src, 'src') - - - -if __name__ == '__main__': - unittest.main() diff --git a/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py b/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py index 62d214ee9..243a11d8d 100644 --- a/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py +++ b/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py @@ -40,6 +40,7 @@ def write_test_result(filesystem, port, test_name, driver_output, """Write the test result to the result output directory.""" root_output_dir = port.results_directory() writer = TestResultWriter(filesystem, port, root_output_dir, test_name) + if driver_output.error: writer.write_stderr(driver_output.error) @@ -74,10 +75,10 @@ def write_test_result(filesystem, port, test_name, driver_output, failure.diff_percent = diff_percent else: _log.warn('Can not get image diff. ImageDiff program might not work correctly.') - writer.copy_file(failure.reference_filename) + writer.write_reftest(failure.reference_filename) elif isinstance(failure, test_failures.FailureReftestMismatchDidNotOccur): writer.write_image_files(driver_output.image, expected_image=None) - writer.copy_file(failure.reference_filename) + writer.write_reftest(failure.reference_filename) else: assert isinstance(failure, (test_failures.FailureTimeout, test_failures.FailureReftestNoImagesGenerated)) @@ -124,6 +125,16 @@ class TestResultWriter(object): output_filename = fs.join(self._root_output_dir, self._test_name) return fs.splitext(output_filename)[0] + modifier + def _write_binary_file(self, path, contents): + if contents is not None: + self._make_output_directory() + self._filesystem.write_binary_file(path, contents) + + def _write_text_file(self, path, contents): + if contents is not None: + self._make_output_directory() + self._filesystem.write_text_file(path, contents) + def _output_testname(self, modifier): fs = self._filesystem return fs.splitext(fs.basename(self._test_name))[0] + modifier @@ -141,28 +152,19 @@ class TestResultWriter(object): output: A string containing the test output expected: A string containing the expected test output """ - self._make_output_directory() actual_filename = self.output_filename(self.FILENAME_SUFFIX_ACTUAL + file_type) expected_filename = self.output_filename(self.FILENAME_SUFFIX_EXPECTED + file_type) - fs = self._filesystem - if output is not None: - fs.write_binary_file(actual_filename, output) - if expected is not None: - fs.write_binary_file(expected_filename, expected) + self._write_binary_file(actual_filename, output) + self._write_binary_file(expected_filename, expected) def write_stderr(self, error): - fs = self._filesystem filename = self.output_filename(self.FILENAME_SUFFIX_STDERR + ".txt") - fs.maybe_make_directory(fs.dirname(filename)) - fs.write_binary_file(filename, error) + self._write_binary_file(filename, error) def write_crash_log(self, crash_log): - fs = self._filesystem filename = self.output_filename(self.FILENAME_SUFFIX_CRASH_LOG + ".txt") - fs.maybe_make_directory(fs.dirname(filename)) - if crash_log is not None: - fs.write_text_file(filename, crash_log) + self._write_text_file(filename, crash_log) def write_text_files(self, actual_text, expected_text): self.write_output_files(".txt", actual_text, expected_text) @@ -173,28 +175,26 @@ class TestResultWriter(object): if not actual_text or not expected_text: return - self._make_output_directory() file_type = '.txt' actual_filename = self.output_filename(self.FILENAME_SUFFIX_ACTUAL + file_type) expected_filename = self.output_filename(self.FILENAME_SUFFIX_EXPECTED + file_type) - fs = self._filesystem # We treat diff output as binary. Diff output may contain multiple files # in conflicting encodings. diff = self._port.diff_text(expected_text, actual_text, expected_filename, actual_filename) diff_filename = self.output_filename(self.FILENAME_SUFFIX_DIFF + file_type) - fs.write_binary_file(diff_filename, diff) + self._write_binary_file(diff_filename, diff) # Shell out to wdiff to get colored inline diffs. if self._port.wdiff_available(): wdiff = self._port.wdiff_text(expected_filename, actual_filename) wdiff_filename = self.output_filename(self.FILENAME_SUFFIX_WDIFF) - fs.write_binary_file(wdiff_filename, wdiff) + self._write_binary_file(wdiff_filename, wdiff) # Use WebKit's PrettyPatch.rb to get an HTML diff. if self._port.pretty_patch_available(): pretty_patch = self._port.pretty_patch_text(diff_filename) pretty_patch_filename = self.output_filename(self.FILENAME_SUFFIX_PRETTY_PATCH) - fs.write_binary_file(pretty_patch_filename, pretty_patch) + self._write_binary_file(pretty_patch_filename, pretty_patch) def write_audio_files(self, actual_audio, expected_audio): self.write_output_files('.wav', actual_audio, expected_audio) @@ -204,8 +204,7 @@ class TestResultWriter(object): def write_image_diff_files(self, image_diff): diff_filename = self.output_filename(self.FILENAME_SUFFIX_IMAGE_DIFF) - fs = self._filesystem - fs.write_binary_file(diff_filename, image_diff) + self._write_binary_file(diff_filename, image_diff) diffs_html_filename = self.output_filename(self.FILENAME_SUFFIX_IMAGE_DIFFS_HTML) # FIXME: old-run-webkit-tests shows the diff percentage as the text contents of the "diff" link. @@ -263,9 +262,8 @@ Difference between images: <a href="%(diff_filename)s">diff</a><br> } self._filesystem.write_text_file(diffs_html_filename, html) - def copy_file(self, src_filepath): + def write_reftest(self, src_filepath): fs = self._filesystem - assert fs.exists(src_filepath), 'src_filepath: %s' % src_filepath - dst_filepath = fs.join(self._root_output_dir, self._port.relative_test_filename(src_filepath)) - self._make_output_directory() - fs.copyfile(src_filepath, dst_filepath) + dst_dir = fs.dirname(fs.join(self._root_output_dir, self._test_name)) + dst_filepath = fs.join(dst_dir, fs.basename(src_filepath)) + self._write_text_file(dst_filepath, fs.read_text_file(src_filepath)) diff --git a/Tools/Scripts/webkitpy/layout_tests/controllers/worker.py b/Tools/Scripts/webkitpy/layout_tests/controllers/worker.py index c68915916..837aea86b 100644 --- a/Tools/Scripts/webkitpy/layout_tests/controllers/worker.py +++ b/Tools/Scripts/webkitpy/layout_tests/controllers/worker.py @@ -41,14 +41,14 @@ _log = logging.getLogger(__name__) class Worker(object): - def __init__(self, worker_connection, worker_number, results_directory, options): - self._worker_connection = worker_connection - self._worker_number = worker_number - self._name = 'worker/%d' % worker_number + def __init__(self, caller, results_directory, options): + self._caller = caller + self._worker_number = caller.worker_number + self._name = caller.name self._results_directory = results_directory self._options = options - # The remaining fields are initialized in safe_init() + # The remaining fields are initialized in start() self._host = None self._port = None self._batch_size = None @@ -59,12 +59,13 @@ class Worker(object): self._tests_run_filename = None def __del__(self): - self.cleanup() + self.stop() - def safe_init(self): - """This method is called when it is safe for the object to create state that - does not need to be pickled (usually this means it is called in a child process).""" - self._host = self._worker_connection.host + def start(self): + """This method is called when the object is starting to be used and it is safe + for the object to create state that does not need to be pickled (usually this means + it is called in a child process).""" + self._host = self._caller.host self._filesystem = self._host.filesystem self._port = self._host.port_factory.get(self._options.platform, self._options) @@ -73,18 +74,13 @@ class Worker(object): tests_run_filename = self._filesystem.join(self._results_directory, "tests_run%d.txt" % self._worker_number) self._tests_run_file = self._filesystem.open_text_file_for_writing(tests_run_filename) - def handle(self, name, source, list_name, test_list): + def handle(self, name, source, test_list_name, test_inputs): assert name == 'test_list' start_time = time.time() - num_tests = 0 - for test_input in test_list: - self._update_test_input(test_input) + for test_input in test_inputs: self._run_test(test_input) - num_tests += 1 - self._worker_connection.yield_to_broker() - elapsed_time = time.time() - start_time - self._worker_connection.post_message('finished_list', list_name, num_tests, elapsed_time) + self._caller.post('finished_test_list', test_list_name, len(test_inputs), elapsed_time) def _update_test_input(self, test_input): test_input.reference_files = self._port.reference_files(test_input.test_name) @@ -99,25 +95,26 @@ class Worker(object): test_input.should_run_pixel_test = False def _run_test(self, test_input): - test_timeout_sec = self.timeout(test_input) + self._update_test_input(test_input) + test_timeout_sec = self._timeout(test_input) start = time.time() - self._worker_connection.post_message('started_test', test_input, test_timeout_sec) + self._caller.post('started_test', test_input, test_timeout_sec) - result = self.run_test_with_timeout(test_input, test_timeout_sec) + result = self._run_test_with_timeout(test_input, test_timeout_sec) elapsed_time = time.time() - start - self._worker_connection.post_message('finished_test', result, elapsed_time) + self._caller.post('finished_test', result, elapsed_time) - self.clean_up_after_test(test_input, result) + self._clean_up_after_test(test_input, result) - def cleanup(self): + def stop(self): _log.debug("%s cleaning up" % self._name) - self.kill_driver() + self._kill_driver() if self._tests_run_file: self._tests_run_file.close() self._tests_run_file = None - def timeout(self, test_input): + def _timeout(self, test_input): """Compute the appropriate timeout value for a test.""" # The DumpRenderTree watchdog uses 2.5x the timeout; we want to be # larger than that. We also add a little more padding if we're @@ -133,7 +130,7 @@ class Worker(object): thread_timeout_sec = driver_timeout_sec + thread_padding_sec return thread_timeout_sec - def kill_driver(self): + def _kill_driver(self): # Be careful about how and when we kill the driver; if driver.stop() # raises an exception, this routine may get re-entered via __del__. driver = self._driver @@ -142,12 +139,12 @@ class Worker(object): _log.debug("%s killing driver" % self._name) driver.stop() - def run_test_with_timeout(self, test_input, timeout): + def _run_test_with_timeout(self, test_input, timeout): if self._options.run_singly: return self._run_test_in_another_thread(test_input, timeout) return self._run_test_in_this_thread(test_input) - def clean_up_after_test(self, test_input, result): + def _clean_up_after_test(self, test_input, result): self._batch_count += 1 test_name = test_input.test_name self._tests_run_file.write(test_name + "\n") @@ -155,7 +152,7 @@ class Worker(object): if result.failures: # Check and kill DumpRenderTree if we need to. if any([f.driver_needs_restart() for f in result.failures]): - self.kill_driver() + self._kill_driver() # Reset the batch count since the shell just bounced. self._batch_count = 0 @@ -169,7 +166,7 @@ class Worker(object): _log.debug("%s %s passed" % (self._name, test_name)) if self._batch_size > 0 and self._batch_count >= self._batch_size: - self.kill_driver() + self._kill_driver() self._batch_count = 0 def _run_test_in_another_thread(self, test_input, thread_timeout_sec): @@ -195,7 +192,7 @@ class Worker(object): self.result = None def run(self): - self.result = worker.run_single_test(driver, test_input) + self.result = worker._run_single_test(driver, test_input) thread = SingleTestThread() thread.start() @@ -227,11 +224,11 @@ class Worker(object): Returns: a TestResult object. """ if self._driver and self._driver.has_crashed(): - self.kill_driver() + self._kill_driver() if not self._driver: self._driver = self._port.create_driver(self._worker_number) - return self.run_single_test(self._driver, test_input) + return self._run_single_test(self._driver, test_input) - def run_single_test(self, driver, test_input): + def _run_single_test(self, driver, test_input): return single_test_runner.run_single_test(self._port, self._options, test_input, driver, self._name) diff --git a/Tools/Scripts/webkitpy/layout_tests/models/result_summary.py b/Tools/Scripts/webkitpy/layout_tests/models/result_summary.py index 27c5452c0..d46703e8f 100644 --- a/Tools/Scripts/webkitpy/layout_tests/models/result_summary.py +++ b/Tools/Scripts/webkitpy/layout_tests/models/result_summary.py @@ -47,6 +47,7 @@ class ResultSummary(object): self.unexpected_results = {} self.failures = {} self.total_failures = 0 + self.expected_skips = 0 self.total_tests_by_expectation[SKIP] = 0 self.tests_by_expectation[SKIP] = set() for expectation in TestExpectations.EXPECTATIONS.values(): @@ -65,6 +66,8 @@ class ResultSummary(object): self.failures[test_result.test_name] = test_result.failures if expected: self.expected += 1 + if test_result.type == SKIP: + self.expected_skips += 1 else: self.unexpected_results[test_result.test_name] = test_result self.unexpected += 1 diff --git a/Tools/Scripts/webkitpy/layout_tests/models/test_configuration.py b/Tools/Scripts/webkitpy/layout_tests/models/test_configuration.py index e9607279b..95d0f2b87 100644 --- a/Tools/Scripts/webkitpy/layout_tests/models/test_configuration.py +++ b/Tools/Scripts/webkitpy/layout_tests/models/test_configuration.py @@ -26,7 +26,6 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -import itertools class TestConfiguration(object): def __init__(self, version, architecture, build_type): @@ -212,8 +211,8 @@ class TestConfigurationConverter(object): break else: return - indices[i] += 1 - for j in range(i + 1, r): + indices[i] += 1 # pylint: disable=W0631 + for j in range(i + 1, r): # pylint: disable=W0631 indices[j] = indices[j - 1] + 1 yield tuple(pool[i] for i in indices) diff --git a/Tools/Scripts/webkitpy/layout_tests/models/test_configuration_unittest.py b/Tools/Scripts/webkitpy/layout_tests/models/test_configuration_unittest.py index c367b7591..5c43b6ac6 100644 --- a/Tools/Scripts/webkitpy/layout_tests/models/test_configuration_unittest.py +++ b/Tools/Scripts/webkitpy/layout_tests/models/test_configuration_unittest.py @@ -28,8 +28,6 @@ import unittest -from webkitpy.common.host_mock import MockHost - from webkitpy.layout_tests.models.test_configuration import * @@ -77,7 +75,7 @@ class TestConfigurationTest(unittest.TestCase): self.assertTrue(config_dict[TestConfiguration('xp', 'x86', 'release')]) def query_unknown_key(): - config_dict[TestConfiguration('xp', 'x86', 'debug')] + return config_dict[TestConfiguration('xp', 'x86', 'debug')] self.assertRaises(KeyError, query_unknown_key) self.assertTrue(TestConfiguration('xp', 'x86', 'release') in config_dict) @@ -88,8 +86,6 @@ class TestConfigurationTest(unittest.TestCase): def test_eq(self): self.assertEquals(TestConfiguration('xp', 'x86', 'release'), TestConfiguration('xp', 'x86', 'release')) - host = MockHost() - test_port = host.port_factory.get('test-win-xp', None) self.assertNotEquals(TestConfiguration('xp', 'x86', 'release'), TestConfiguration('xp', 'x86', 'debug')) def test_values(self): diff --git a/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py b/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py index 93ef517f4..9c6d478d4 100644 --- a/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py +++ b/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py @@ -31,12 +31,10 @@ for layout tests. """ -import itertools -import json import logging import re -from webkitpy.layout_tests.models.test_configuration import TestConfiguration, TestConfigurationConverter +from webkitpy.layout_tests.models.test_configuration import TestConfigurationConverter _log = logging.getLogger(__name__) @@ -117,6 +115,7 @@ def strip_comments(line): class ParseError(Exception): def __init__(self, warnings): + super(ParseError, self).__init__() self.warnings = warnings def __str__(self): @@ -400,9 +399,10 @@ class TestExpectationLine(object): def __init__(self): """Initializes a blank-line equivalent of an expectation.""" self.original_string = None + self.filename = None # this is the path to the expectations file for this line self.line_number = None - self.name = None - self.path = None + self.name = None # this is the path in the line itself + self.path = None # this is the normpath of self.name self.modifiers = [] self.parsed_modifiers = [] self.parsed_bug_modifiers = [] @@ -515,6 +515,25 @@ class TestExpectationsModel(object): def get_expectations(self, test): return self._test_to_expectations[test] + def get_expectations_string(self, test): + """Returns the expectatons for the given test as an uppercase string. + If there are no expectations for the test, then "PASS" is returned.""" + expectations = self.get_expectations(test) + retval = [] + + for expectation in expectations: + retval.append(self.expectation_to_string(expectation)) + + return " ".join(retval) + + def expectation_to_string(self, expectation): + """Return the uppercased string equivalent of a given expectation.""" + for item in TestExpectations.EXPECTATIONS.items(): + if item[1] == expectation: + return item[0].upper() + raise ValueError(expectation) + + def add_expectation_line(self, expectation_line, in_skipped=False): """Returns a list of warnings encountered while matching modifiers.""" @@ -525,7 +544,7 @@ class TestExpectationsModel(object): if not in_skipped and self._already_seen_better_match(test, expectation_line): continue - self._clear_expectations_for_test(test, expectation_line) + self._clear_expectations_for_test(test) self._test_to_expectation_line[test] = expectation_line self._add_test(test, expectation_line) @@ -559,7 +578,7 @@ class TestExpectationsModel(object): # FIXME: What is this? self._result_type_to_tests[FAIL].add(test) - def _clear_expectations_for_test(self, test, expectation_line): + def _clear_expectations_for_test(self, test): """Remove prexisting expectations for this test. This happens if we are seeing a more precise path than a previous listing. @@ -571,13 +590,13 @@ class TestExpectationsModel(object): self._remove_from_sets(test, self._timeline_to_tests) self._remove_from_sets(test, self._result_type_to_tests) - def _remove_from_sets(self, test, dict): + def _remove_from_sets(self, test, dict_of_sets_of_tests): """Removes the given test from the sets in the dictionary. Args: test: test to look for dict: dict of sets of files""" - for set_of_tests in dict.itervalues(): + for set_of_tests in dict_of_sets_of_tests.itervalues(): if test in set_of_tests: set_of_tests.remove(test) @@ -782,22 +801,10 @@ class TestExpectations(object): return self._model.get_tests_with_timeline(timeline) def get_expectations_string(self, test): - """Returns the expectatons for the given test as an uppercase string. - If there are no expectations for the test, then "PASS" is returned.""" - expectations = self._model.get_expectations(test) - retval = [] - - for expectation in expectations: - retval.append(self.expectation_to_string(expectation)) - - return " ".join(retval) + return self._model.get_expectations_string(test) def expectation_to_string(self, expectation): - """Return the uppercased string equivalent of a given expectation.""" - for item in self.EXPECTATIONS.items(): - if item[1] == expectation: - return item[0].upper() - raise ValueError(expectation) + return self._model.expectation_to_string(expectation) def matches_an_expected_result(self, test, result, pixel_tests_are_enabled): expected_results = self._model.get_expectations(test) @@ -820,7 +827,8 @@ class TestExpectations(object): warnings = [] for expectation in self._expectations: for warning in expectation.warnings: - warnings.append('%s:%d %s %s' % (self._shorten_filename(expectation.filename), expectation.line_number, warning, expectation.name if expectation.expectations else expectation.original_string)) + warnings.append('%s:%d %s %s' % (self._shorten_filename(expectation.filename), expectation.line_number, + warning, expectation.name if expectation.expectations else expectation.original_string)) if warnings: self._has_warnings = True diff --git a/Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py b/Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py index 7b589b501..c780dac23 100644 --- a/Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py +++ b/Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py @@ -95,7 +95,7 @@ class Base(unittest.TestCase): # Note that all of these tests are written assuming the configuration # being tested is Windows XP, Release build. - def __init__(self, testFunc, setUp=None, tearDown=None, description=None): + def __init__(self, testFunc): host = MockHost() self._port = host.port_factory.get('test-win-xp', None) self._exp = None @@ -123,11 +123,11 @@ BUG_TEST WONTFIX MAC : failures/expected/image.html = IMAGE """ def parse_exp(self, expectations, overrides=None, is_lint_mode=False): - self._expectations_dict = OrderedDict() - self._expectations_dict['expectations'] = expectations + expectations_dict = OrderedDict() + expectations_dict['expectations'] = expectations if overrides: - self._expectations_dict['overrides'] = overrides - self._port.expectations_dict = lambda: self._expectations_dict + expectations_dict['overrides'] = overrides + self._port.expectations_dict = lambda: expectations_dict self._exp = TestExpectations(self._port, self.get_basic_tests(), is_lint_mode) def assert_exp(self, test, result): @@ -274,11 +274,11 @@ class SkippedTests(Base): def check(self, expectations, overrides, skips, lint=False): port = MockHost().port_factory.get('qt') port._filesystem.write_text_file(port._filesystem.join(port.layout_tests_dir(), 'failures/expected/text.html'), 'foo') - self._expectations_dict = OrderedDict() - self._expectations_dict['expectations'] = expectations + expectations_dict = OrderedDict() + expectations_dict['expectations'] = expectations if overrides: - self._expectations_dict['overrides'] = overrides - port.expectations_dict = lambda: self._expectations_dict + expectations_dict['overrides'] = overrides + port.expectations_dict = lambda: expectations_dict port.skipped_layout_tests = lambda tests: set(skips) exp = TestExpectations(port, ['failures/expected/text.html'], lint) @@ -532,7 +532,6 @@ class TestExpectationParserTests(unittest.TestCase): host = MockHost() test_port = host.port_factory.get('test-win-xp', None) test_port.test_exists = lambda test: True - test_config = test_port.test_configuration() full_test_list = [] expectation_line = self._tokenize('') parser = TestExpectationParser(test_port, full_test_list, allow_rebaseline_modifier=False) diff --git a/Tools/Scripts/webkitpy/layout_tests/models/test_failures.py b/Tools/Scripts/webkitpy/layout_tests/models/test_failures.py index 029094ec4..afea52e60 100644 --- a/Tools/Scripts/webkitpy/layout_tests/models/test_failures.py +++ b/Tools/Scripts/webkitpy/layout_tests/models/test_failures.py @@ -112,6 +112,7 @@ class TestFailure(object): class FailureTimeout(TestFailure): """Test timed out. We also want to restart DumpRenderTree if this happens.""" def __init__(self, is_reftest=False): + super(FailureTimeout, self).__init__() self.is_reftest = is_reftest def message(self): @@ -124,6 +125,7 @@ class FailureTimeout(TestFailure): class FailureCrash(TestFailure): """DumpRenderTree/WebKitTestRunner crashed.""" def __init__(self, is_reftest=False, process_name='DumpRenderTree', pid=None): + super(FailureCrash, self).__init__() self.process_name = process_name self.pid = pid self.is_reftest = is_reftest @@ -168,6 +170,7 @@ class FailureMissingImage(TestFailure): class FailureImageHashMismatch(TestFailure): """Image hashes didn't match.""" def __init__(self, diff_percent=0): + super(FailureImageHashMismatch, self).__init__() self.diff_percent = diff_percent def message(self): @@ -185,6 +188,7 @@ class FailureReftestMismatch(TestFailure): """The result didn't match the reference rendering.""" def __init__(self, reference_filename=None): + super(FailureReftestMismatch, self).__init__() self.reference_filename = reference_filename self.diff_percent = None @@ -196,6 +200,7 @@ class FailureReftestMismatchDidNotOccur(TestFailure): """Unexpected match between the result and the reference rendering.""" def __init__(self, reference_filename=None): + super(FailureReftestMismatchDidNotOccur, self).__init__() self.reference_filename = reference_filename def message(self): @@ -206,6 +211,7 @@ class FailureReftestNoImagesGenerated(TestFailure): """Both the reftest and the -expected html file didn't generate pixel results.""" def __init__(self, reference_filename=None): + super(FailureReftestNoImagesGenerated, self).__init__() self.reference_filename = reference_filename def message(self): diff --git a/Tools/Scripts/webkitpy/layout_tests/models/test_failures_unittest.py b/Tools/Scripts/webkitpy/layout_tests/models/test_failures_unittest.py index 3b9ba33d0..e096b171f 100644 --- a/Tools/Scripts/webkitpy/layout_tests/models/test_failures_unittest.py +++ b/Tools/Scripts/webkitpy/layout_tests/models/test_failures_unittest.py @@ -45,10 +45,14 @@ class TestFailuresTest(unittest.TestCase): def test_unknown_failure_type(self): class UnknownFailure(TestFailure): - pass + def message(self): + return '' failure_obj = UnknownFailure() self.assertRaises(ValueError, determine_result_type, [failure_obj]) + + def test_message_is_virtual(self): + failure_obj = TestFailure() self.assertRaises(NotImplementedError, failure_obj.message) def test_loads(self): diff --git a/Tools/Scripts/webkitpy/layout_tests/models/test_results.py b/Tools/Scripts/webkitpy/layout_tests/models/test_results.py index 51ac505c2..346d5a640 100644 --- a/Tools/Scripts/webkitpy/layout_tests/models/test_results.py +++ b/Tools/Scripts/webkitpy/layout_tests/models/test_results.py @@ -35,8 +35,8 @@ class TestResult(object): """Data object containing the results of a single test.""" @staticmethod - def loads(str): - return cPickle.loads(str) + def loads(string): + return cPickle.loads(string) def __init__(self, test_name, failures=None, test_run_time=None, has_stderr=False): self.test_name = test_name @@ -54,9 +54,9 @@ class TestResult(object): def __ne__(self, other): return not (self == other) - def has_failure_matching_types(self, *args, **kargs): + def has_failure_matching_types(self, *failure_classes): for failure in self.failures: - if type(failure) in args: + if type(failure) in failure_classes: return True return False diff --git a/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py b/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py index 2a1877407..2240657c1 100644 --- a/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py @@ -518,7 +518,7 @@ class ChromiumAndroidDriver(chromium.ChromiumDriver): def _start_once(self, pixel_tests, per_test_args): self._port._run_adb_command(['logcat', '-c']) self._port._run_adb_command(['shell', 'echo'] + self.cmd_line(pixel_tests, per_test_args) + ['>', COMMAND_LINE_FILE]) - start_result = self._port._run_adb_command(['shell', 'am', 'start', '-n', DRT_ACTIVITY_FULL_NAME]) + start_result = self._port._run_adb_command(['shell', 'am', 'start', '-e', 'RunInSubThread', '-n', DRT_ACTIVITY_FULL_NAME]) if start_result.find('Exception') != -1: _log.error('Failed to start DumpRenderTree application. Exception:\n' + start_result) return False diff --git a/Tools/Scripts/webkitpy/layout_tests/port/efl.py b/Tools/Scripts/webkitpy/layout_tests/port/efl.py index 5964dfe52..4e43f8b6e 100644 --- a/Tools/Scripts/webkitpy/layout_tests/port/efl.py +++ b/Tools/Scripts/webkitpy/layout_tests/port/efl.py @@ -45,6 +45,7 @@ class EflPort(WebKitPort, PulseAudioSanitizer): self._jhbuild_wrapper_path = self.path_from_webkit_base('Tools', 'efl', 'run-with-jhbuild') self.set_option_default('wrapper', self._jhbuild_wrapper_path) + self.webprocess_cmd_prefix = self.get_option('webprocess_cmd_prefix') def _port_flag_for_scripts(self): return "--efl" @@ -56,6 +57,8 @@ class EflPort(WebKitPort, PulseAudioSanitizer): env = super(EflPort, self).setup_environ_for_server(server_name) env['TEST_RUNNER_INJECTED_BUNDLE_FILENAME'] = self._build_path('lib', 'libTestRunnerInjectedBundle.so') env['TEST_RUNNER_PLUGIN_PATH'] = self._build_path('lib') + if self.webprocess_cmd_prefix: + env['WEB_PROCESS_CMD_PREFIX'] = self.webprocess_cmd_prefix return env def clean_up_test_run(self): diff --git a/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py b/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py index e3a13c20f..691c8456b 100755 --- a/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py +++ b/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py @@ -35,6 +35,7 @@ import optparse import os import signal import sys +import traceback from webkitpy.common.host import Host from webkitpy.common.system import stack_utils @@ -131,6 +132,7 @@ def run(port, options, args, regular_output=sys.stderr, buildbot_output=sys.stdo except Exception: exception_type, exception_value, exception_traceback = sys.exc_info() if exception_type not in (KeyboardInterrupt, TestRunInterruptedException, WorkerException): + print >> sys.stderr, '\n%s raised: %s' % (exception_type.__name__, exception_value) stack_utils.log_traceback(_log.error, exception_traceback) raise finally: @@ -254,6 +256,11 @@ def parse_args(args=None): help="Arguments parsed to Android adb, to select device, etc."), ])) + option_group_definitions.append(("EFL-specific Options", [ + optparse.make_option("--webprocess-cmd-prefix", type="string", + default=False, help="Prefix used when spawning the Web process (Debug mode only)"), + ])) + option_group_definitions.append(("WebKit Options", [ optparse.make_option("--gc-between-tests", action="store_true", default=False, help="Force garbage collection between each test"), @@ -464,6 +471,10 @@ def main(argv=None): # FIXME: is this the best way to handle unsupported port names? print >> sys.stderr, str(e) return EXCEPTIONAL_EXIT_STATUS + except Exception, e: + print >> sys.stderr, '\n%s raised: %s' % (e.__class__.__name__, str(e)) + traceback.print_exc(file=sys.stderr) + raise logging.getLogger().setLevel(logging.DEBUG if options.verbose else logging.INFO) return run(port, options, args) @@ -472,7 +483,7 @@ def main(argv=None): if '__main__' == __name__: try: sys.exit(main()) - except Exception, e: + except BaseException, e: if e.__class__ in (KeyboardInterrupt, TestRunInterruptedException): sys.exit(INTERRUPTED_EXIT_STATUS) sys.exit(EXCEPTIONAL_EXIT_STATUS) diff --git a/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py b/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py index c106fbf47..ad14bf4ef 100755 --- a/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py +++ b/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py @@ -51,7 +51,7 @@ from webkitpy.common.host_mock import MockHost from webkitpy.layout_tests import port from webkitpy.layout_tests import run_webkit_tests -from webkitpy.layout_tests.controllers.manager_worker_broker import WorkerException +from webkitpy.layout_tests.controllers.manager import WorkerException from webkitpy.layout_tests.port import Port from webkitpy.layout_tests.port.test import TestPort, TestDriver from webkitpy.test.skip import skip_if @@ -951,11 +951,11 @@ class EndToEndTest(unittest.TestCase): self.assertTrue("multiple-mismatch-success.html" not in json["tests"]["reftests"]["foo"]) self.assertTrue("multiple-both-success.html" not in json["tests"]["reftests"]["foo"]) self.assertEqual(json["tests"]["reftests"]["foo"]["multiple-match-failure.html"], - {"expected": "PASS", "ref_file": "reftests/foo/second-mismatching-ref.html", "actual": "IMAGE", "image_diff_percent": 1, 'is_reftest': True}) + {"expected": "PASS", "actual": "IMAGE", "image_diff_percent": 1, 'is_reftest': True}) self.assertEqual(json["tests"]["reftests"]["foo"]["multiple-mismatch-failure.html"], - {"expected": "PASS", "ref_file": "reftests/foo/matching-ref.html", "actual": "IMAGE", "is_mismatch_reftest": True}) + {"expected": "PASS", "actual": "IMAGE", "is_mismatch_reftest": True}) self.assertEqual(json["tests"]["reftests"]["foo"]["multiple-both-failure.html"], - {"expected": "PASS", "ref_file": "reftests/foo/matching-ref.html", "actual": "IMAGE", "is_mismatch_reftest": True}) + {"expected": "PASS", "actual": "IMAGE", "is_mismatch_reftest": True}) class RebaselineTest(unittest.TestCase, StreamTestingMixin): diff --git a/Tools/Scripts/webkitpy/layout_tests/views/printing.py b/Tools/Scripts/webkitpy/layout_tests/views/printing.py index 3d98c6c59..2dd909930 100644 --- a/Tools/Scripts/webkitpy/layout_tests/views/printing.py +++ b/Tools/Scripts/webkitpy/layout_tests/views/printing.py @@ -31,6 +31,7 @@ import optparse +from webkitpy.tool import grammar from webkitpy.common.net import resultsjsonparser from webkitpy.layout_tests.models.test_expectations import TestExpectations from webkitpy.layout_tests.views.metered_stream import MeteredStream @@ -217,23 +218,21 @@ class Printer(object): return incomplete = total - expected - unexpected + incomplete_str = '' if incomplete: self._write("") incomplete_str = " (%d didn't run)" % incomplete - expected_str = str(expected) - else: - incomplete_str = "" - expected_str = "All %d" % expected if unexpected == 0: - self._write("%s tests ran as expected%s." % - (expected_str, incomplete_str)) - elif expected == 1: - self._write("1 test ran as expected, %d didn't%s:" % - (unexpected, incomplete_str)) + if expected == total: + if expected > 1: + self._write("All %d tests ran as expected." % expected) + else: + self._write("The test ran as expected.") + else: + self._write("%s ran as expected%s." % (grammar.pluralize('test', expected), incomplete_str)) else: - self._write("%d tests ran as expected, %d didn't%s:" % - (expected, unexpected, incomplete_str)) + self._write("%s ran as expected, %d didn't%s:" % (grammar.pluralize('test', expected), unexpected, incomplete_str)) self._write("") def print_test_result(self, result, expected, exp_str, got_str): diff --git a/Tools/Scripts/webkitpy/layout_tests/views/printing_unittest.py b/Tools/Scripts/webkitpy/layout_tests/views/printing_unittest.py index 56970a863..1312050e9 100644 --- a/Tools/Scripts/webkitpy/layout_tests/views/printing_unittest.py +++ b/Tools/Scripts/webkitpy/layout_tests/views/printing_unittest.py @@ -206,11 +206,11 @@ class Testprinter(unittest.TestCase): printer, err, out = self.get_printer(['--print', 'one-line-summary']) printer.print_one_line_summary(1, 1, 0) - self.assertWritten(err, ["All 1 tests ran as expected.\n", "\n"]) + self.assertWritten(err, ["The test ran as expected.\n", "\n"]) printer, err, out = self.get_printer(['--print', 'everything']) printer.print_one_line_summary(1, 1, 0) - self.assertWritten(err, ["All 1 tests ran as expected.\n", "\n"]) + self.assertWritten(err, ["The test ran as expected.\n", "\n"]) printer, err, out = self.get_printer(['--print', 'everything']) printer.print_one_line_summary(2, 1, 1) diff --git a/Tools/Scripts/webkitpy/pylintrc b/Tools/Scripts/webkitpy/pylintrc index dae778d63..bdd040415 100644 --- a/Tools/Scripts/webkitpy/pylintrc +++ b/Tools/Scripts/webkitpy/pylintrc @@ -85,13 +85,15 @@ load-plugins= # W0141: Used builtin function '' # W0212: Access to a protected member X of a client class # W0142: Used * or ** magic +# W0401: Wildcard import X # W0402: Uses of a deprecated module 'string' # W0404: 41: Reimport 'XX' (imported line NN) # W0511: TODO # W0603: Using the global statement +# W0614: Unused import X from wildcard import # W0703: Catch "Exception" # W1201: Specify string format arguments as logging function parameters -disable=C0103,C0111,C0302,I0010,I0011,R0201,R0801,R0901,R0902,R0903,R0904,R0911,R0912,R0913,R0914,R0915,R0921,R0922,W0122,W0141,W0142,W0212,W0402,W0404,W0511,W0603,W0703,W1201 +disable=C0103,C0111,C0302,I0010,I0011,R0201,R0801,R0901,R0902,R0903,R0904,R0911,R0912,R0913,R0914,R0915,R0921,R0922,W0122,W0141,W0142,W0212,W0401,W0402,W0404,W0511,W0603,W0614,W0703,W1201 [REPORTS] diff --git a/Tools/Scripts/webkitpy/test/test_finder.py b/Tools/Scripts/webkitpy/test/finder.py index 3a90197e9..132072d82 100644 --- a/Tools/Scripts/webkitpy/test/test_finder.py +++ b/Tools/Scripts/webkitpy/test/finder.py @@ -31,7 +31,7 @@ import sys _log = logging.getLogger(__name__) -class TestDirectoryTree(object): +class _DirectoryTree(object): def __init__(self, filesystem, top_directory, starting_subdirectory): self.filesystem = filesystem self.top_directory = filesystem.realpath(top_directory) @@ -63,7 +63,6 @@ class TestDirectoryTree(object): return realpath.replace(self.top_directory + self.filesystem.sep, '') return None - def clean(self): """Delete all .pyc files in the tree that have no matching .py file.""" _log.debug("Cleaning orphaned *.pyc files from: %s" % self.search_directory) @@ -74,13 +73,13 @@ class TestDirectoryTree(object): self.filesystem.remove(filename) -class TestFinder(object): +class Finder(object): def __init__(self, filesystem): self.filesystem = filesystem self.trees = [] def add_tree(self, top_directory, starting_subdirectory=None): - self.trees.append(TestDirectoryTree(self.filesystem, top_directory, starting_subdirectory)) + self.trees.append(_DirectoryTree(self.filesystem, top_directory, starting_subdirectory)) def additional_paths(self, paths): return [tree.top_directory for tree in self.trees if tree.top_directory not in paths] diff --git a/Tools/Scripts/webkitpy/test/test_finder_unittest.py b/Tools/Scripts/webkitpy/test/finder_unittest.py index 5b6b3b030..09048b159 100644 --- a/Tools/Scripts/webkitpy/test/test_finder_unittest.py +++ b/Tools/Scripts/webkitpy/test/finder_unittest.py @@ -25,10 +25,10 @@ import unittest from webkitpy.common.system.filesystem_mock import MockFileSystem from webkitpy.common.system.outputcapture import OutputCapture -from webkitpy.test.test_finder import TestFinder +from webkitpy.test.finder import Finder -class TestFinderTest(unittest.TestCase): +class FinderTest(unittest.TestCase): def setUp(self): files = { '/foo/bar/baz.py': '', @@ -40,7 +40,7 @@ class TestFinderTest(unittest.TestCase): '/tmp/another_unittest.py': '', } self.fs = MockFileSystem(files) - self.finder = TestFinder(self.fs) + self.finder = Finder(self.fs) self.finder.add_tree('/foo', 'bar') self.finder.add_tree('/foo2') @@ -49,7 +49,7 @@ class TestFinderTest(unittest.TestCase): self.root_logger = logging.getLogger() self.log_handler = None for h in self.root_logger.handlers: - if getattr(h, 'name', None) == 'webkitpy.test.main': + if getattr(h, 'name', None) == 'webkitpy.test.printer': self.log_handler = h break if self.log_handler: diff --git a/Tools/Scripts/webkitpy/test/main.py b/Tools/Scripts/webkitpy/test/main.py index c5dc39433..2048d9e59 100644 --- a/Tools/Scripts/webkitpy/test/main.py +++ b/Tools/Scripts/webkitpy/test/main.py @@ -25,24 +25,24 @@ import logging import optparse -import os import StringIO import sys import traceback import unittest from webkitpy.common.system.filesystem import FileSystem -from webkitpy.common.system import outputcapture -from webkitpy.test.test_finder import TestFinder -from webkitpy.test.runner import TestRunner +from webkitpy.test.finder import Finder +from webkitpy.test.printer import Printer +from webkitpy.test.runner import Runner _log = logging.getLogger(__name__) class Tester(object): def __init__(self, filesystem=None): - self.finder = TestFinder(filesystem or FileSystem()) - self.stream = sys.stderr + self.finder = Finder(filesystem or FileSystem()) + self.printer = Printer(sys.stderr) + self._options = None def add_tree(self, top_directory, starting_subdirectory=None): self.finder.add_tree(top_directory, starting_subdirectory) @@ -50,13 +50,13 @@ class Tester(object): def _parse_args(self): parser = optparse.OptionParser(usage='usage: %prog [options] [args...]') parser.add_option('-a', '--all', action='store_true', default=False, - help='run all the tests'), + help='run all the tests') parser.add_option('-c', '--coverage', action='store_true', default=False, - help='generate code coverage info (requires http://pypi.python.org/pypi/coverage)'), + help='generate code coverage info (requires http://pypi.python.org/pypi/coverage)') parser.add_option('-q', '--quiet', action='store_true', default=False, - help='run quietly (errors, warnings, and progress only)'), + help='run quietly (errors, warnings, and progress only)') parser.add_option('-t', '--timing', action='store_true', default=False, - help='display per-test execution time (implies --verbose)'), + help='display per-test execution time (implies --verbose)') parser.add_option('-v', '--verbose', action='count', default=0, help='verbose output (specify once for individual test results, twice for debug messages)') parser.add_option('--skip-integrationtests', action='store_true', default=False, @@ -69,72 +69,9 @@ class Tester(object): return parser.parse_args() - def _configure(self, options): - self._options = options - - if options.timing: - # --timing implies --verbose - options.verbose = max(options.verbose, 1) - - log_level = logging.INFO - if options.quiet: - log_level = logging.WARNING - elif options.verbose == 2: - log_level = logging.DEBUG - self._configure_logging(log_level) - - def _configure_logging(self, log_level): - """Configure the root logger. - - Configure the root logger not to log any messages from webkitpy -- - except for messages from the autoinstall module. Also set the - logging level as described below. - """ - handler = logging.StreamHandler(self.stream) - # We constrain the level on the handler rather than on the root - # logger itself. This is probably better because the handler is - # configured and known only to this module, whereas the root logger - # is an object shared (and potentially modified) by many modules. - # Modifying the handler, then, is less intrusive and less likely to - # interfere with modifications made by other modules (e.g. in unit - # tests). - handler.name = __name__ - handler.setLevel(log_level) - formatter = logging.Formatter("%(message)s") - handler.setFormatter(formatter) - - logger = logging.getLogger() - logger.addHandler(handler) - logger.setLevel(logging.NOTSET) - - # Filter out most webkitpy messages. - # - # Messages can be selectively re-enabled for this script by updating - # this method accordingly. - def filter(record): - """Filter out autoinstall and non-third-party webkitpy messages.""" - # FIXME: Figure out a way not to use strings here, for example by - # using syntax like webkitpy.test.__name__. We want to be - # sure not to import any non-Python 2.4 code, though, until - # after the version-checking code has executed. - if (record.name.startswith("webkitpy.common.system.autoinstall") or - record.name.startswith("webkitpy.test")): - return True - if record.name.startswith("webkitpy"): - return False - return True - - testing_filter = logging.Filter() - testing_filter.filter = filter - - # Display a message so developers are not mystified as to why - # logging does not work in the unit tests. - _log.info("Suppressing most webkitpy logging while running unit tests.") - handler.addFilter(testing_filter) - def run(self): - options, args = self._parse_args() - self._configure(options) + self._options, args = self._parse_args() + self.printer.configure(self._options) self.finder.clean_trees() @@ -149,7 +86,7 @@ class Tester(object): if self._options.coverage: try: import webkitpy.thirdparty.autoinstalled.coverage as coverage - except ImportError, e: + except ImportError: _log.error("Failed to import 'coverage'; can't generate coverage numbers.") return False cov = coverage.coverage() @@ -169,7 +106,7 @@ class Tester(object): # produces lousy error messages for bad modules. try: __import__(name) - except ImportError, e: + except ImportError: _log.fatal('Failed to import %s:' % name) self._log_exception() return False @@ -177,11 +114,9 @@ class Tester(object): suites.append(loader.loadTestsFromName(name, None)) test_suite = unittest.TestSuite(suites) - test_runner = TestRunner(self.stream, self._options, loader) + test_runner = Runner(self.printer, self._options, loader) _log.debug("Running the tests.") - if self._options.pass_through: - outputcapture.OutputCapture.stream_wrapper = _CaptureAndPassThroughStream result = test_runner.run(test_suite) if self._options.coverage: cov.stop() @@ -194,32 +129,3 @@ class Tester(object): traceback.print_exc(file=s) for l in s.buflist: _log.error(' ' + l.rstrip()) - - -class _CaptureAndPassThroughStream(object): - def __init__(self, stream): - self._buffer = StringIO.StringIO() - self._stream = stream - - def write(self, msg): - self._stream.write(msg) - - # Note that we don't want to capture any output generated by the debugger - # because that could cause the results of capture_output() to be invalid. - if not self._message_is_from_pdb(): - self._buffer.write(msg) - - def _message_is_from_pdb(self): - # We will assume that if the pdb module is in the stack then the output - # is being generated by the python debugger (or the user calling something - # from inside the debugger). - import inspect - import pdb - stack = inspect.stack() - return any(frame[1] == pdb.__file__.replace('.pyc', '.py') for frame in stack) - - def flush(self): - self._stream.flush() - - def getvalue(self): - return self._buffer.getvalue() diff --git a/Tools/Scripts/webkitpy/test/main_unittest.py b/Tools/Scripts/webkitpy/test/main_unittest.py index 1a60beef3..2cf6df4a2 100644 --- a/Tools/Scripts/webkitpy/test/main_unittest.py +++ b/Tools/Scripts/webkitpy/test/main_unittest.py @@ -40,7 +40,7 @@ class TesterTest(unittest.TestCase): root_handlers = root_logger.handlers root_logger.handlers = [] - tester.stream = errors + tester.printer.stream = errors tester.finder.find_names = lambda args, skip_integration, run_all: [] oc = OutputCapture() try: diff --git a/Tools/Scripts/webkitpy/test/printer.py b/Tools/Scripts/webkitpy/test/printer.py new file mode 100644 index 000000000..77e28b8d1 --- /dev/null +++ b/Tools/Scripts/webkitpy/test/printer.py @@ -0,0 +1,182 @@ +# Copyright (C) 2012 Google, Inc. +# Copyright (C) 2010 Chris Jerdonek (cjerdonek@webkit.org) +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions +# are met: +# 1. Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# 2. Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' AND +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED +# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +# DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS BE LIABLE FOR +# ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +import logging +import re +import StringIO + +from webkitpy.common.system import outputcapture + +_log = logging.getLogger(__name__) + + +class Printer(object): + def __init__(self, stream, options=None): + self.stream = stream + self.options = options + self.test_description = re.compile("(\w+) \(([\w.]+)\)") + + def test_name(self, test): + m = self.test_description.match(str(test)) + return "%s.%s" % (m.group(2), m.group(1)) + + def configure(self, options): + self.options = options + + if options.timing: + # --timing implies --verbose + options.verbose = max(options.verbose, 1) + + log_level = logging.INFO + if options.quiet: + log_level = logging.WARNING + elif options.verbose == 2: + log_level = logging.DEBUG + + handler = logging.StreamHandler(self.stream) + # We constrain the level on the handler rather than on the root + # logger itself. This is probably better because the handler is + # configured and known only to this module, whereas the root logger + # is an object shared (and potentially modified) by many modules. + # Modifying the handler, then, is less intrusive and less likely to + # interfere with modifications made by other modules (e.g. in unit + # tests). + handler.name = __name__ + handler.setLevel(log_level) + formatter = logging.Formatter("%(message)s") + handler.setFormatter(formatter) + + logger = logging.getLogger() + logger.addHandler(handler) + logger.setLevel(logging.NOTSET) + + # Filter out most webkitpy messages. + # + # Messages can be selectively re-enabled for this script by updating + # this method accordingly. + def filter_records(record): + """Filter out autoinstall and non-third-party webkitpy messages.""" + # FIXME: Figure out a way not to use strings here, for example by + # using syntax like webkitpy.test.__name__. We want to be + # sure not to import any non-Python 2.4 code, though, until + # after the version-checking code has executed. + if (record.name.startswith("webkitpy.common.system.autoinstall") or + record.name.startswith("webkitpy.test")): + return True + if record.name.startswith("webkitpy"): + return False + return True + + testing_filter = logging.Filter() + testing_filter.filter = filter_records + + # Display a message so developers are not mystified as to why + # logging does not work in the unit tests. + _log.info("Suppressing most webkitpy logging while running unit tests.") + handler.addFilter(testing_filter) + + if self.options.pass_through: + outputcapture.OutputCapture.stream_wrapper = _CaptureAndPassThroughStream + + def print_started_test(self, test_name): + if self.options.verbose: + self.stream.write(test_name) + + def print_finished_test(self, result, test_name, test_time, failure, err): + timing = '' + if self.options.timing: + timing = ' %.4fs' % test_time + if self.options.verbose: + if failure: + msg = ' failed' + elif err: + msg = ' erred' + else: + msg = ' passed' + self.stream.write(msg + timing + '\n') + else: + if failure: + msg = 'F' + elif err: + msg = 'E' + else: + msg = '.' + self.stream.write(msg) + + def print_result(self, result, run_time): + self.stream.write('\n') + + for (test, err) in result.errors: + self.stream.write("=" * 80 + '\n') + self.stream.write("ERROR: " + self.test_name(test) + '\n') + self.stream.write("-" * 80 + '\n') + for line in err.splitlines(): + self.stream.write(line + '\n') + self.stream.write('\n') + + for (test, failure) in result.failures: + self.stream.write("=" * 80 + '\n') + self.stream.write("FAILURE: " + self.test_name(test) + '\n') + self.stream.write("-" * 80 + '\n') + for line in failure.splitlines(): + self.stream.write(line + '\n') + self.stream.write('\n') + + self.stream.write('-' * 80 + '\n') + self.stream.write('Ran %d test%s in %.3fs\n' % + (result.testsRun, result.testsRun != 1 and "s" or "", run_time)) + + if result.wasSuccessful(): + self.stream.write('\nOK\n') + else: + self.stream.write('FAILED (failures=%d, errors=%d)\n' % + (len(result.failures), len(result.errors))) + + +class _CaptureAndPassThroughStream(object): + def __init__(self, stream): + self._buffer = StringIO.StringIO() + self._stream = stream + + def write(self, msg): + self._stream.write(msg) + + # Note that we don't want to capture any output generated by the debugger + # because that could cause the results of capture_output() to be invalid. + if not self._message_is_from_pdb(): + self._buffer.write(msg) + + def _message_is_from_pdb(self): + # We will assume that if the pdb module is in the stack then the output + # is being generated by the python debugger (or the user calling something + # from inside the debugger). + import inspect + import pdb + stack = inspect.stack() + return any(frame[1] == pdb.__file__.replace('.pyc', '.py') for frame in stack) + + def flush(self): + self._stream.flush() + + def getvalue(self): + return self._buffer.getvalue() diff --git a/Tools/Scripts/webkitpy/test/runner.py b/Tools/Scripts/webkitpy/test/runner.py index e190f2cd4..9c952075e 100644 --- a/Tools/Scripts/webkitpy/test/runner.py +++ b/Tools/Scripts/webkitpy/test/runner.py @@ -23,7 +23,6 @@ """code to actually run a list of python tests.""" import logging -import re import time import unittest @@ -31,16 +30,11 @@ import unittest _log = logging.getLogger(__name__) -class TestRunner(object): - def __init__(self, stream, options, loader): +class Runner(object): + def __init__(self, printer, options, loader): self.options = options - self.stream = stream + self.printer = printer self.loader = loader - self.test_description = re.compile("(\w+) \(([\w.]+)\)") - - def test_name(self, test): - m = self.test_description.match(str(test)) - return "%s.%s" % (m.group(2), m.group(1)) def all_test_names(self, suite): names = [] @@ -48,7 +42,7 @@ class TestRunner(object): for t in suite._tests: names.extend(self.all_test_names(t)) else: - names.append(self.test_name(suite)) + names.append(self.printer.test_name(suite)) return names def run(self, suite): @@ -57,8 +51,7 @@ class TestRunner(object): result = unittest.TestResult() stop = run_start_time for test_name in all_test_names: - if self.options.verbose: - self.stream.write(test_name) + self.printer.print_started_test(test_name) num_failures = len(result.failures) num_errors = len(result.errors) @@ -75,58 +68,8 @@ class TestRunner(object): failure = result.failures[num_failures][1] elif len(result.errors) > num_errors: err = result.errors[num_errors][1] - self.write_result(result, test_name, stop - start, failure, err) + self.printer.print_finished_test(result, test_name, stop - start, failure, err) - self.write_summary(result, stop - run_start_time) + self.printer.print_result(result, stop - run_start_time) return result - - def write_result(self, result, test_name, test_time, failure=None, err=None): - timing = '' - if self.options.timing: - timing = ' %.4fs' % test_time - if self.options.verbose: - if failure: - msg = ' failed' - elif err: - msg = ' erred' - else: - msg = ' passed' - self.stream.write(msg + timing + '\n') - else: - if failure: - msg = 'F' - elif err: - msg = 'E' - else: - msg = '.' - self.stream.write(msg) - - def write_summary(self, result, run_time): - self.stream.write('\n') - - for (test, err) in result.errors: - self.stream.write("=" * 80 + '\n') - self.stream.write("ERROR: " + self.test_name(test) + '\n') - self.stream.write("-" * 80 + '\n') - for line in err.splitlines(): - self.stream.write(line + '\n') - self.stream.write('\n') - - for (test, failure) in result.failures: - self.stream.write("=" * 80 + '\n') - self.stream.write("FAILURE: " + self.test_name(test) + '\n') - self.stream.write("-" * 80 + '\n') - for line in failure.splitlines(): - self.stream.write(line + '\n') - self.stream.write('\n') - - self.stream.write('-' * 80 + '\n') - self.stream.write('Ran %d test%s in %.3fs\n' % - (result.testsRun, result.testsRun != 1 and "s" or "", run_time)) - - if result.wasSuccessful(): - self.stream.write('\nOK\n') - else: - self.stream.write('FAILED (failures=%d, errors=%d)\n' % - (len(result.failures), len(result.errors))) diff --git a/Tools/Scripts/webkitpy/test/runner_unittest.py b/Tools/Scripts/webkitpy/test/runner_unittest.py index e2ea31aa1..1cf0146fb 100644 --- a/Tools/Scripts/webkitpy/test/runner_unittest.py +++ b/Tools/Scripts/webkitpy/test/runner_unittest.py @@ -25,7 +25,8 @@ import StringIO import unittest from webkitpy.tool.mocktool import MockOptions -from webkitpy.test.runner import TestRunner +from webkitpy.test.printer import Printer +from webkitpy.test.runner import Runner class FakeModuleSuite(object): @@ -74,7 +75,7 @@ class RunnerTest(unittest.TestCase): loader = FakeLoader(('test1 (Foo)', '.', ''), ('test2 (Foo)', 'F', 'test2\nfailed'), ('test3 (Foo)', 'E', 'test3\nerred')) - result = TestRunner(stream, options, loader).run(loader.top_suite()) + result = Runner(Printer(stream, options), options, loader).run(loader.top_suite()) self.assertFalse(result.wasSuccessful()) self.assertEquals(result.testsRun, 3) self.assertEquals(len(result.failures), 1) @@ -87,7 +88,7 @@ class RunnerTest(unittest.TestCase): loader = FakeLoader(('test1 (Foo)', '.', ''), ('test2 (Foo)', 'F', 'test2\nfailed'), ('test3 (Foo)', 'E', 'test3\nerred')) - result = TestRunner(stream, options, loader).run(loader.top_suite()) + result = Runner(Printer(stream, options), options, loader).run(loader.top_suite()) self.assertFalse(result.wasSuccessful()) self.assertEquals(result.testsRun, 3) self.assertEquals(len(result.failures), 1) @@ -100,7 +101,7 @@ class RunnerTest(unittest.TestCase): loader = FakeLoader(('test1 (Foo)', '.', ''), ('test2 (Foo)', 'F', 'test2\nfailed'), ('test3 (Foo)', 'E', 'test3\nerred')) - result = TestRunner(stream, options, loader).run(loader.top_suite()) + result = Runner(Printer(stream, options), options, loader).run(loader.top_suite()) self.assertFalse(result.wasSuccessful()) self.assertEquals(result.testsRun, 3) self.assertEquals(len(result.failures), 1) diff --git a/Tools/Scripts/webkitpy/tool/commands/queues.py b/Tools/Scripts/webkitpy/tool/commands/queues.py index 2af08b718..e8db17c7b 100644 --- a/Tools/Scripts/webkitpy/tool/commands/queues.py +++ b/Tools/Scripts/webkitpy/tool/commands/queues.py @@ -38,6 +38,7 @@ from optparse import make_option from StringIO import StringIO from webkitpy.common.config.committervalidator import CommitterValidator +from webkitpy.common.config.ports import DeprecatedPort from webkitpy.common.net.bugzilla import Attachment from webkitpy.common.net.statusserver import StatusServer from webkitpy.common.system.deprecated_logging import error, log @@ -257,10 +258,17 @@ class AbstractPatchQueue(AbstractQueue): class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler, CommitQueueTaskDelegate): name = "commit-queue" + port_name = "chromium-xvfb" + + def __init__(self): + AbstractPatchQueue.__init__(self) + self.port = DeprecatedPort.port(self.port_name) # AbstractPatchQueue methods def begin_work_queue(self): + # FIXME: This violates abstraction + self._tool._deprecated_port = self.port AbstractPatchQueue.begin_work_queue(self) self.committer_validator = CommitterValidator(self._tool) self._expected_failures = ExpectedFailures() @@ -305,7 +313,7 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler, CommitQueueTaskD # CommitQueueTaskDelegate methods def run_command(self, command): - self.run_webkit_patch(command) + self.run_webkit_patch(command + [self.port.flag()]) def command_passed(self, message, patch): self._update_status(message, patch=patch) diff --git a/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py b/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py index 1c2d57b1c..1914ccd4b 100644 --- a/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py +++ b/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py @@ -49,7 +49,7 @@ class TestCommitQueue(CommitQueue): CommitQueue.__init__(self) if tool: self.bind_to_tool(tool) - self._options = MockOptions(confirm=False, parent_command="commit-queue") + self._options = MockOptions(confirm=False, parent_command="commit-queue", port=None) def begin_work_queue(self): output_capture = OutputCapture() @@ -231,8 +231,8 @@ class CommitQueueTest(QueuesTest): def test_commit_queue(self): tool = MockTool() - tool.filesystem.write_text_file('/mock-results/full_results.json', '') # Otherwise the commit-queue will hit a KeyError trying to read the results from the MockFileSystem. - tool.filesystem.write_text_file('/mock-results/webkit_unit_tests_output.xml', '') + tool.filesystem.write_text_file('/tmp/layout-test-results/full_results.json', '') # Otherwise the commit-queue will hit a KeyError trying to read the results from the MockFileSystem. + tool.filesystem.write_text_file('/tmp/layout-test-results/webkit_unit_tests_output.xml', '') expected_stderr = { "begin_work_queue": self._default_begin_work_queue_stderr("commit-queue"), "next_work_item": "", @@ -269,7 +269,7 @@ MOCK: release_work_item: commit-queue 10000 queue = CommitQueue() def mock_run_webkit_patch(command): - if command == ['clean'] or command == ['update']: + if command[0] == 'clean' or command[0] == 'update': # We want cleaning to succeed so we can error out on a step # that causes the commit-queue to reject the patch. return @@ -298,7 +298,7 @@ MOCK: release_work_item: commit-queue 10000 queue = CommitQueue() def mock_run_webkit_patch(command): - if command == ['clean'] or command == ['update']: + if command[0] == 'clean' or command[0] == 'update': # We want cleaning to succeed so we can error out on a step # that causes the commit-queue to reject the patch. return @@ -310,29 +310,29 @@ MOCK: release_work_item: commit-queue 10000 def test_rollout(self): tool = MockTool(log_executive=True) - tool.filesystem.write_text_file('/mock-results/full_results.json', '') # Otherwise the commit-queue will hit a KeyError trying to read the results from the MockFileSystem. - tool.filesystem.write_text_file('/mock-results/webkit_unit_tests_output.xml', '') + tool.filesystem.write_text_file('/tmp/layout-test-results/full_results.json', '') # Otherwise the commit-queue will hit a KeyError trying to read the results from the MockFileSystem. + tool.filesystem.write_text_file('/tmp/layout-test-results/webkit_unit_tests_output.xml', '') tool.buildbot.light_tree_on_fire() expected_stderr = { "begin_work_queue": self._default_begin_work_queue_stderr("commit-queue"), "next_work_item": "", - "process_work_item": """MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'clean'], cwd=/mock-checkout + "process_work_item": """MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'clean', '--port=%(port_name)s'], cwd=/mock-checkout MOCK: update_status: commit-queue Cleaned working directory -MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'update'], cwd=/mock-checkout +MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'update', '--port=%(port_name)s'], cwd=/mock-checkout MOCK: update_status: commit-queue Updated working directory -MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'apply-attachment', '--no-update', '--non-interactive', 10000], cwd=/mock-checkout +MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'apply-attachment', '--no-update', '--non-interactive', 10000, '--port=%(port_name)s'], cwd=/mock-checkout MOCK: update_status: commit-queue Applied patch -MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'validate-changelog', '--non-interactive', 10000], cwd=/mock-checkout +MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'validate-changelog', '--non-interactive', 10000, '--port=%(port_name)s'], cwd=/mock-checkout MOCK: update_status: commit-queue ChangeLog validated -MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'build', '--no-clean', '--no-update', '--build-style=both'], cwd=/mock-checkout +MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'build', '--no-clean', '--no-update', '--build-style=both', '--port=%(port_name)s'], cwd=/mock-checkout MOCK: update_status: commit-queue Built patch -MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive'], cwd=/mock-checkout +MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'build-and-test', '--no-clean', '--no-update', '--test', '--non-interactive', '--port=%(port_name)s'], cwd=/mock-checkout MOCK: update_status: commit-queue Passed tests -MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 10000], cwd=/mock-checkout +MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 10000, '--port=%(port_name)s'], cwd=/mock-checkout MOCK: update_status: commit-queue Landed patch MOCK: update_status: commit-queue Pass MOCK: release_work_item: commit-queue 10000 -""", +""" % {"port_name": CommitQueue.port_name}, "handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '10000' with comment 'Rejecting attachment 10000 from commit-queue.' and additional comment 'Mock error message'\n", "handle_script_error": "ScriptError error message\n\nMOCK output\n", } @@ -346,19 +346,19 @@ MOCK: release_work_item: commit-queue 10000 expected_stderr = { "begin_work_queue": self._default_begin_work_queue_stderr("commit-queue"), "next_work_item": "", - "process_work_item": """MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'clean'], cwd=/mock-checkout + "process_work_item": """MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'clean', '--port=%(port_name)s'], cwd=/mock-checkout MOCK: update_status: commit-queue Cleaned working directory -MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'update'], cwd=/mock-checkout +MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'update', '--port=%(port_name)s'], cwd=/mock-checkout MOCK: update_status: commit-queue Updated working directory -MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'apply-attachment', '--no-update', '--non-interactive', 10005], cwd=/mock-checkout +MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'apply-attachment', '--no-update', '--non-interactive', 10005, '--port=%(port_name)s'], cwd=/mock-checkout MOCK: update_status: commit-queue Applied patch -MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'validate-changelog', '--non-interactive', 10005], cwd=/mock-checkout +MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'validate-changelog', '--non-interactive', 10005, '--port=%(port_name)s'], cwd=/mock-checkout MOCK: update_status: commit-queue ChangeLog validated -MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 10005], cwd=/mock-checkout +MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 10005, '--port=%(port_name)s'], cwd=/mock-checkout MOCK: update_status: commit-queue Landed patch MOCK: update_status: commit-queue Pass MOCK: release_work_item: commit-queue 10005 -""", +""" % {"port_name": CommitQueue.port_name}, "handle_unexpected_error": "MOCK setting flag 'commit-queue' to '-' on attachment '10005' with comment 'Rejecting attachment 10005 from commit-queue.' and additional comment 'Mock error message'\n", "handle_script_error": "ScriptError error message\n\nMOCK output\n", } @@ -382,8 +382,8 @@ MOCK: release_work_item: commit-queue 10005 def test_manual_reject_during_processing(self): queue = SecondThoughtsCommitQueue(MockTool()) queue.begin_work_queue() - queue._tool.filesystem.write_text_file('/mock-results/full_results.json', '') # Otherwise the commit-queue will hit a KeyError trying to read the results from the MockFileSystem. - queue._tool.filesystem.write_text_file('/mock-results/webkit_unit_tests_output.xml', '') + queue._tool.filesystem.write_text_file('/tmp/layout-test-results/full_results.json', '') # Otherwise the commit-queue will hit a KeyError trying to read the results from the MockFileSystem. + queue._tool.filesystem.write_text_file('/tmp/layout-test-results/webkit_unit_tests_output.xml', '') queue._options = Mock() queue._options.port = None expected_stderr = """MOCK: update_status: commit-queue Cleaned working directory diff --git a/Tools/Scripts/webkitpy/tool/commands/rebaseline.py b/Tools/Scripts/webkitpy/tool/commands/rebaseline.py index cb7254ba2..c214a339c 100644 --- a/Tools/Scripts/webkitpy/tool/commands/rebaseline.py +++ b/Tools/Scripts/webkitpy/tool/commands/rebaseline.py @@ -398,10 +398,10 @@ class Rebaseline(AbstractParallelRebaselineCommand): return self._tool.user.prompt_with_list("Which test(s) to rebaseline for %s:" % builder.name(), failing_tests, can_choose_multiple=True) def _suffixes_to_update(self, options): - suffixes = [] + suffixes = set() for suffix_list in options.suffixes: - suffixes += suffix_list.split(",") - return suffixes + suffixes |= set(suffix_list.split(",")) + return list(suffixes) def execute(self, options, args, tool): if options.builders: diff --git a/Tools/Scripts/webkitpy/tool/commands/rebaseline_unittest.py b/Tools/Scripts/webkitpy/tool/commands/rebaseline_unittest.py index a3b9efaeb..433906b8c 100644 --- a/Tools/Scripts/webkitpy/tool/commands/rebaseline_unittest.py +++ b/Tools/Scripts/webkitpy/tool/commands/rebaseline_unittest.py @@ -465,18 +465,18 @@ MOCK run_command: ['echo', 'optimize-baselines', '--suffixes', 'txt', 'mock/path tool.executive = MockExecutive(should_log=True) - expected_stdout = """rebaseline-json: {'mock/path/to/test.html': {'MOCK builder2': ['txt', 'png', 'wav'], 'MOCK builder': ['txt', 'png', 'wav'], 'MOCK builder3': ['txt', 'png', 'wav']}, 'mock/path/to/test2.html': {'MOCK builder2': ['txt', 'png', 'wav'], 'MOCK builder': ['txt', 'png', 'wav'], 'MOCK builder3': ['txt', 'png', 'wav']}} + expected_stdout = """rebaseline-json: {'mock/path/to/test.html': {'MOCK builder2': ['wav', 'txt', 'png'], 'MOCK builder': ['wav', 'txt', 'png'], 'MOCK builder3': ['wav', 'txt', 'png']}, 'mock/path/to/test2.html': {'MOCK builder2': ['wav', 'txt', 'png'], 'MOCK builder': ['wav', 'txt', 'png'], 'MOCK builder3': ['wav', 'txt', 'png']}} """ - expected_stderr = """MOCK run_command: ['echo', 'rebaseline-test-internal', '--suffixes', 'txt,png,wav', '--builder', 'MOCK builder2', '--test', 'mock/path/to/test.html'], cwd=/mock-checkout -MOCK run_command: ['echo', 'rebaseline-test-internal', '--suffixes', 'txt,png,wav', '--builder', 'MOCK builder', '--test', 'mock/path/to/test.html'], cwd=/mock-checkout -MOCK run_command: ['echo', 'rebaseline-test-internal', '--suffixes', 'txt,png,wav', '--builder', 'MOCK builder2', '--test', 'mock/path/to/test2.html'], cwd=/mock-checkout -MOCK run_command: ['echo', 'rebaseline-test-internal', '--suffixes', 'txt,png,wav', '--builder', 'MOCK builder', '--test', 'mock/path/to/test2.html'], cwd=/mock-checkout + expected_stderr = """MOCK run_command: ['echo', 'rebaseline-test-internal', '--suffixes', 'wav,txt,png', '--builder', 'MOCK builder2', '--test', 'mock/path/to/test.html'], cwd=/mock-checkout +MOCK run_command: ['echo', 'rebaseline-test-internal', '--suffixes', 'wav,txt,png', '--builder', 'MOCK builder', '--test', 'mock/path/to/test.html'], cwd=/mock-checkout +MOCK run_command: ['echo', 'rebaseline-test-internal', '--suffixes', 'wav,txt,png', '--builder', 'MOCK builder2', '--test', 'mock/path/to/test2.html'], cwd=/mock-checkout +MOCK run_command: ['echo', 'rebaseline-test-internal', '--suffixes', 'wav,txt,png', '--builder', 'MOCK builder', '--test', 'mock/path/to/test2.html'], cwd=/mock-checkout MOCK run_command: ['echo', 'optimize-baselines', '--suffixes', 'wav,txt,png', 'mock/path/to/test.html'], cwd=/mock-checkout MOCK run_command: ['echo', 'optimize-baselines', '--suffixes', 'wav,txt,png', 'mock/path/to/test2.html'], cwd=/mock-checkout """ - OutputCapture().assert_outputs(self, command.execute, [MockOptions(optimize=True, builders=["MOCK builder,MOCK builder2", "MOCK builder3"], suffixes=["txt", "png,wav"], verbose=True), ["mock/path/to/test.html", "mock/path/to/test2.html"], tool], expected_stdout=expected_stdout, expected_stderr=expected_stderr) + OutputCapture().assert_outputs(self, command.execute, [MockOptions(optimize=True, builders=["MOCK builder,MOCK builder2", "MOCK builder3"], suffixes=["txt,png", "png,wav,txt"], verbose=True), ["mock/path/to/test.html", "mock/path/to/test2.html"], tool], expected_stdout=expected_stdout, expected_stderr=expected_stderr) finally: builders._exact_matches = old_exact_matches |