Changeset 122384 in webkit


Ignore:
Timestamp:
Jul 11, 2012 4:11:14 PM (12 years ago)
Author:
dpranke@chromium.org
Message:

nrwt: clean up names in worker.py
​https://bugs.webkit.org/show_bug.cgi?id=90510

Reviewed by Ojan Vafai.

This is the last patch in the series of refactoring worker.py;
all this does is change some names of methods, instance
variables, and method parameters to be clearer (it also changes
some code in manager.py and manager_worker.py to be consistent).

There are no functional changes in this patch and the existing
tests should cover everything.

  • Scripts/webkitpy/layout_tests/controllers/manager.py:

(Manager.init):
(Manager.prepare_lists_and_print_output):
(Manager._run_tests.worker_factory):
(Manager._run_tests):
(Manager._show_results_html_file):
(Manager.handle_finished_test_list):
(_WorkerState.init):

  • Scripts/webkitpy/layout_tests/controllers/manager_worker_broker.py:

(_Broker.post_message):
(_Broker._dispatch_message):
(AbstractWorker.init):
(AbstractWorker.run):
(AbstractWorker.yield_to_caller):
(AbstractWorker.post):
(_WorkerConnection.init):

  • Scripts/webkitpy/layout_tests/controllers/manager_worker_broker_unittest.py:

(_TestWorker.init):
(_TestWorker.start):
(_TestWorker.handle):
(_TestWorker.stop):
(_TestsMixin):
(_TestsMixin.test_name):

  • Scripts/webkitpy/layout_tests/controllers/worker.py:

(Worker.init):
(Worker.del):
(Worker.start):
(Worker.handle):
(Worker._run_test):
(Worker.stop):
(Worker._timeout):
(Worker._kill_driver):
(Worker._run_test_with_timeout):
(Worker._clean_up_after_test):
(Worker._run_test_in_another_thread.SingleTestThread.run):
(Worker._run_test_in_this_thread):
(Worker._run_single_test):

Location:
trunk/Tools
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Tools/ChangeLog

    r122381 r122384  
     12012-07-11  Dirk Pranke  <dpranke@chromium.org>
     2
     3        nrwt: clean up names in worker.py
     4        https://bugs.webkit.org/show_bug.cgi?id=90510
     5
     6        Reviewed by Ojan Vafai.
     7
     8        This is the last patch in the series of refactoring worker.py;
     9        all this does is change some names of methods, instance
     10        variables, and method parameters to be clearer (it also changes
     11        some code in manager.py and manager_worker.py to be consistent).
     12
     13        There are no functional changes in this patch and the existing
     14        tests should cover everything.
     15
     16        * Scripts/webkitpy/layout_tests/controllers/manager.py:
     17        (Manager.__init__):
     18        (Manager.prepare_lists_and_print_output):
     19        (Manager._run_tests.worker_factory):
     20        (Manager._run_tests):
     21        (Manager._show_results_html_file):
     22        (Manager.handle_finished_test_list):
     23        (_WorkerState.__init__):
     24        * Scripts/webkitpy/layout_tests/controllers/manager_worker_broker.py:
     25        (_Broker.post_message):
     26        (_Broker._dispatch_message):
     27        (AbstractWorker.__init__):
     28        (AbstractWorker.run):
     29        (AbstractWorker.yield_to_caller):
     30        (AbstractWorker.post):
     31        (_WorkerConnection.__init__):
     32        * Scripts/webkitpy/layout_tests/controllers/manager_worker_broker_unittest.py:
     33        (_TestWorker.__init__):
     34        (_TestWorker.start):
     35        (_TestWorker.handle):
     36        (_TestWorker.stop):
     37        (_TestsMixin):
     38        (_TestsMixin.test_name):
     39        * Scripts/webkitpy/layout_tests/controllers/worker.py:
     40        (Worker.__init__):
     41        (Worker.__del__):
     42        (Worker.start):
     43        (Worker.handle):
     44        (Worker._run_test):
     45        (Worker.stop):
     46        (Worker._timeout):
     47        (Worker._kill_driver):
     48        (Worker._run_test_with_timeout):
     49        (Worker._clean_up_after_test):
     50        (Worker._run_test_in_another_thread.SingleTestThread.run):
     51        (Worker._run_test_in_this_thread):
     52        (Worker._run_single_test):
     53
    1542012-07-11  Adam Barth  <abarth@webkit.org>
    255

  • trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py

    r121509 r122384  
    334334        self._worker_states = {}
    335335
     336        self.name = 'manager'
     337
    336338    def collect_tests(self, args):
    337339        """Find all the files to test.
    … …  
    497499            self._printer.print_expected("Running all %d tests." % len(self._test_files))
    498500        else:
    499             self._printer.print_expected("Running %1 test.")
     501            self._printer.print_expected("Running 1 test.")
    500502
    501503        # Create a sorted list of test files so the subset chunk,
    … …  
    770772        self._log_num_workers(num_workers, len(all_shards), len(locked_shards))
    771773
    772         def worker_factory(worker_connection, worker_number):
    773             return worker.Worker(worker_connection, worker_number, self.results_directory(), self._options)
     774        def worker_factory(worker_connection):
     775            return worker.Worker(worker_connection, self.results_directory(), self._options)
    774776
    775777        manager_connection = manager_worker_broker.get(num_workers, self, worker_factory, self._port.host)
    … …  
    782784            worker_connection = manager_connection.start_worker(worker_number)
    783785            worker_state = _WorkerState(worker_number, worker_connection)
    784             self._worker_states[worker_connection.name()] = worker_state
     786            self._worker_states[worker_connection.name] = worker_state
    785787
    786788            time.sleep(self._port.worker_startup_delay_secs())
    … …  
    14431445        self._port.show_results_html_file(results_filename)
    14441446
    1445     def name(self):
    1446         return 'Manager'
    14471447
    14481448    def is_done(self):
    … …  
    14781478        raise WorkerException(str(exception_value))
    14791479
    1480     def handle_finished_list(self, source, list_name, num_tests, elapsed_time):
     1480    def handle_finished_test_list(self, source, list_name, num_tests, elapsed_time):
    14811481        self._group_stats[list_name] = (num_tests, elapsed_time)
    14821482
    … …  
    15751575        self.next_timeout = None
    15761576        self.stats = {}
    1577         self.stats['name'] = worker_connection.name()
     1577        self.stats['name'] = worker_connection.name
    15781578        self.stats['num_tests'] = 0
    15791579        self.stats['total_time'] = 0

  • trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_worker_broker.py

    r121816 r122384  
    172172
    173173        Messages have a name and a tuple of optional arguments. Both must be picklable."""
    174         message = _Message(client.name(), topic_name, message_name, message_args)
     174        message = _Message(client.name, topic_name, message_name, message_args)
    175175        queue = self._get_queue_for_topic(topic_name)
    176176        queue.put(_Message.dumps(message))
    … …  
    201201            raise ValueError(
    202202               "%s: received message '%s' it couldn't handle" %
    203                (client.name(), message.name))
     203               (client.name, message.name))
    204204        optargs = message.args
    205205        message_handler = getattr(client, 'handle_' + message.name)
    … …  
    269269        self.worker = None
    270270        self._worker_connection = worker_connection
    271         self._worker_number = worker_number
    272         self._name = 'worker/%d' % worker_number
     271        self.worker_number = worker_number
     272        self.name = 'worker/%d' % worker_number
    273273        self._done = False
    274274        self._canceled = False
    275275        self._options = optparse.Values({'verbose': False})
    276276        self.host = None
    277 
    278     def name(self):
    279         return self._name
    280277
    281278    def is_done(self):
    … …  
    291288        self.host = host
    292289
    293         self.worker.safe_init()
    294         _log.debug('%s starting' % self._name)
     290        self.worker.start()
     291        _log.debug('%s starting' % self.name)
    295292
    296293        try:
    … …  
    298295            if not self.is_done():
    299296                raise AssertionError("%s: ran out of messages in worker queue."
    300                                      % self._name)
     297                                     % self.name)
    301298        except KeyboardInterrupt:
    302299            exception_msg = ", interrupted"
    … …  
    306303            self._worker_connection.raise_exception(sys.exc_info())
    307304        finally:
    308             _log.debug("%s done with message loop%s" % (self._name, exception_msg))
     305            _log.debug("%s done with message loop%s" % (self.name, exception_msg))
    309306            try:
    310                 self.worker.cleanup()
     307                self.worker.stop()
    311308            finally:
    312309                # Make sure we post a done so that we can flush the log messages
    … …  
    326323        self._canceled = True
    327324
    328     def yield_to_broker(self):
     325    def yield_to_caller(self):
    329326        self._worker_connection.yield_to_broker()
    330327
    331     def post_message(self, *args):
     328    def post(self, *args):
    332329        self._worker_connection.post_message(*args)
    333330
    … …  
    383380        # _WorkerConnection and AbstractWorker get merged.
    384381        self._client = AbstractWorker(self, worker_number)
    385         self._worker = worker_factory(self._client, worker_number)
     382        self._worker = worker_factory(self._client)
    386383        self._client.worker = self._worker
    387384        self._host = host
    … …  
    389386        self._logger = None
    390387        self._log_handler = None
     388        self.name = self._client.name
    391389        _BrokerConnection.__init__(self, broker, self._client, ANY_WORKER_TOPIC, MANAGER_TOPIC)
    392 
    393     def name(self):
    394         return self._client.name()
    395390
    396391    def cancel(self):

  • trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_worker_broker_unittest.py

    r121809 r122384  
    5656
    5757class _TestWorker(object):
    58     def __init__(self, caller, worker_number):
     58    def __init__(self, caller):
    5959        self._caller = caller
    6060        self._thing_to_greet = 'everybody'
    … …  
    6363        self._options = optparse.Values({'verbose': False})
    6464
    65     def name(self):
    66         return WORKER_NAME
    67 
    68     def cleanup(self):
    69         pass
     65    def start(self):
     66        if self._starting_queue:
     67            self._starting_queue.put('')
     68
     69        if self._stopping_queue:
     70            self._stopping_queue.get()
    7071
    7172    def handle(self, message, src, an_int, a_str):
    7273        assert an_int == 1
    7374        assert a_str == "hello, world"
    74         self._caller.post_message('finished_test', 2)
    75 
    76     def safe_init(self):
    77         if self._starting_queue:
    78             self._starting_queue.put('')
    79 
    80         if self._stopping_queue:
    81             self._stopping_queue.get()
     75        self._caller.post('finished_test', 2)
    8276
    8377    def stop(self):
    84         self._caller.post_message('done')
     78        pass
    8579
    8680
    … …  
    10094    contract all implementations must follow."""
    10195
    102     def name(self):
    103         return 'TesterManager'
     96    name = 'TesterManager'
    10497
    10598    def is_done(self):
    … …  
    129122        self.make_broker()
    130123        worker = self._broker.start_worker(1)
    131         self.assertEquals(worker.name(), WORKER_NAME)
     124        self.assertEquals(worker.name, WORKER_NAME)
    132125        worker.cancel()
    133126        worker.join(0.1)

  • trunk/Tools/Scripts/webkitpy/layout_tests/controllers/worker.py

    r121816 r122384  
    4242
    4343class Worker(object):
    44     def __init__(self, worker_connection, worker_number, results_directory, options):
    45         self._worker_connection = worker_connection
    46         self._worker_number = worker_number
    47         self._name = 'worker/%d' % worker_number
     44    def __init__(self, caller, results_directory, options):
     45        self._caller = caller
     46        self._worker_number = caller.worker_number
     47        self._name = caller.name
    4848        self._results_directory = results_directory
    4949        self._options = options
    5050
    51         # The remaining fields are initialized in safe_init()
     51        # The remaining fields are initialized in start()
    5252        self._host = None
    5353        self._port = None
    … …  
    6060
    6161    def __del__(self):
    62         self.cleanup()
    63 
    64     def safe_init(self):
    65         """This method is called when it is safe for the object to create state that
    66         does not need to be pickled (usually this means it is called in a child process)."""
    67         self._host = self._worker_connection.host
     62        self.stop()
     63
     64    def start(self):
     65        """This method is called when the object is starting to be used and it is safe
     66        for the object to create state that does not need to be pickled (usually this means
     67        it is called in a child process)."""
     68        self._host = self._caller.host
    6869        self._filesystem = self._host.filesystem
    6970        self._port = self._host.port_factory.get(self._options.platform, self._options)
    … …  
    7475        self._tests_run_file = self._filesystem.open_text_file_for_writing(tests_run_filename)
    7576
    76     def handle(self, name, source, list_name, test_list):
     77    def handle(self, name, source, test_list_name, test_inputs):
    7778        assert name == 'test_list'
    7879        start_time = time.time()
    79         num_tests = 0
    80         for test_input in test_list:
    81             self._update_test_input(test_input)
     80        for test_input in test_inputs:
    8281            self._run_test(test_input)
    83             num_tests += 1
    84             self._worker_connection.yield_to_broker()
    85 
     82            self._caller.yield_to_caller()
    8683        elapsed_time = time.time() - start_time
    87         self._worker_connection.post_message('finished_list', list_name, num_tests, elapsed_time)
     84        self._caller.post('finished_test_list', test_list_name, len(test_inputs), elapsed_time)
    8885
    8986    def _update_test_input(self, test_input):
    … …  
    10097
    10198    def _run_test(self, test_input):
    102         test_timeout_sec = self.timeout(test_input)
     99        self._update_test_input(test_input)
     100        test_timeout_sec = self._timeout(test_input)
    103101        start = time.time()
    104         self._worker_connection.post_message('started_test', test_input, test_timeout_sec)
    105 
    106         result = self.run_test_with_timeout(test_input, test_timeout_sec)
     102        self._caller.post('started_test', test_input, test_timeout_sec)
     103
     104        result = self._run_test_with_timeout(test_input, test_timeout_sec)
    107105
    108106        elapsed_time = time.time() - start
    109         self._worker_connection.post_message('finished_test', result, elapsed_time)
    110 
    111         self.clean_up_after_test(test_input, result)
    112 
    113     def cleanup(self):
     107        self._caller.post('finished_test', result, elapsed_time)
     108
     109        self._clean_up_after_test(test_input, result)
     110
     111    def stop(self):
    114112        _log.debug("%s cleaning up" % self._name)
    115         self.kill_driver()
     113        self._kill_driver()
    116114        if self._tests_run_file:
    117115            self._tests_run_file.close()
    118116            self._tests_run_file = None
    119117
    120     def timeout(self, test_input):
     118    def _timeout(self, test_input):
    121119        """Compute the appropriate timeout value for a test."""
    122120        # The DumpRenderTree watchdog uses 2.5x the timeout; we want to be
    … …  
    134132        return thread_timeout_sec
    135133
    136     def kill_driver(self):
     134    def _kill_driver(self):
    137135        # Be careful about how and when we kill the driver; if driver.stop()
    138136        # raises an exception, this routine may get re-entered via __del__.
    … …  
    143141            driver.stop()
    144142
    145     def run_test_with_timeout(self, test_input, timeout):
     143    def _run_test_with_timeout(self, test_input, timeout):
    146144        if self._options.run_singly:
    147145            return self._run_test_in_another_thread(test_input, timeout)
    148146        return self._run_test_in_this_thread(test_input)
    149147
    150     def clean_up_after_test(self, test_input, result):
     148    def _clean_up_after_test(self, test_input, result):
    151149        self._batch_count += 1
    152150        test_name = test_input.test_name
    … …  
    156154            # Check and kill DumpRenderTree if we need to.
    157155            if any([f.driver_needs_restart() for f in result.failures]):
    158                 self.kill_driver()
     156                self._kill_driver()
    159157                # Reset the batch count since the shell just bounced.
    160158                self._batch_count = 0
    … …  
    170168
    171169        if self._batch_size > 0 and self._batch_count >= self._batch_size:
    172             self.kill_driver()
     170            self._kill_driver()
    173171            self._batch_count = 0
    174172
    … …  
    196194
    197195            def run(self):
    198                 self.result = worker.run_single_test(driver, test_input)
     196                self.result = worker._run_single_test(driver, test_input)
    199197
    200198        thread = SingleTestThread()
    … …  
    228226        """
    229227        if self._driver and self._driver.has_crashed():
    230             self.kill_driver()
     228            self._kill_driver()
    231229        if not self._driver:
    232230            self._driver = self._port.create_driver(self._worker_number)
    233         return self.run_single_test(self._driver, test_input)
    234 
    235     def run_single_test(self, driver, test_input):
     231        return self._run_single_test(self._driver, test_input)
     232
     233    def _run_single_test(self, driver, test_input):
    236234        return single_test_runner.run_single_test(self._port, self._options,
    237235            test_input, driver, self._name)
Note: See TracChangeset for help on using the changeset viewer.