Changeset 90796 in webkit


Ignore:
Timestamp:
Jul 11, 2011 5:10:19 PM (13 years ago)
Author:
dpranke@chromium.org
Message:

nrwt: linting fixes
​https://bugs.webkit.org/show_bug.cgi?id=64225

Reviewed by Eric Siedel.

Miscellaneous linting fixes. The most notable change is that
we add public attributes for user, executive, filesystem, and
options on the Port object, so we don't have to refer to the
"protected" versions all over the place".

  • Scripts/webkitpy/layout_tests/controllers/manager.py:
  • Scripts/webkitpy/layout_tests/controllers/manager_worker_broker.py:
  • Scripts/webkitpy/layout_tests/controllers/message_broker.py:
  • Scripts/webkitpy/layout_tests/controllers/message_broker_unittest.py:
  • Scripts/webkitpy/layout_tests/controllers/worker.py:
  • Scripts/webkitpy/layout_tests/port/base.py:
  • Scripts/webkitpy/layout_tests/run_webkit_tests.py:
Location:
trunk/Tools
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/Tools/ChangeLog

    r90793 r90796  
     12011-07-08  Dirk Pranke  <dpranke@chromium.org>
     2
     3        nrwt: linting fixes
     4        https://bugs.webkit.org/show_bug.cgi?id=64225
     5
     6        Reviewed by Eric Siedel.
     7
     8        Miscellaneous linting fixes. The most notable change is that
     9        we add public attributes for user, executive, filesystem, and
     10        options on the Port object, so we don't have to refer to the
     11        "protected" versions all over the place".
     12
     13        * Scripts/webkitpy/layout_tests/controllers/manager.py:
     14        * Scripts/webkitpy/layout_tests/controllers/manager_worker_broker.py:
     15        * Scripts/webkitpy/layout_tests/controllers/message_broker.py:
     16        * Scripts/webkitpy/layout_tests/controllers/message_broker_unittest.py:
     17        * Scripts/webkitpy/layout_tests/controllers/worker.py:
     18        * Scripts/webkitpy/layout_tests/port/base.py:
     19        * Scripts/webkitpy/layout_tests/run_webkit_tests.py:
     20
    1212011-07-11  Ryosuke Niwa  <rniwa@webkit.org>
    222

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

    r90679 r90796  
    3838from __future__ import with_statement
    3939
    40 import copy
    4140import errno
    4241import logging
    … …  
    5453from webkitpy.layout_tests.layout_package import json_layout_results_generator
    5554from webkitpy.layout_tests.layout_package import json_results_generator
    56 from webkitpy.layout_tests.layout_package import test_results_uploader
    5755from webkitpy.layout_tests.models import test_expectations
    5856from webkitpy.layout_tests.models import test_failures
    … …  
    6260from webkitpy.layout_tests.views import printing
    6361
    64 from webkitpy.thirdparty import simplejson
    6562from webkitpy.tool import grammar
    6663
    … …  
    224221    """Raised when a test run should be stopped immediately."""
    225222    def __init__(self, reason):
     223        Exception.__init__(self)
    226224        self.reason = reason
    227225        self.msg = reason
    … …  
    270268        """
    271269        self._port = port
    272         self._fs = port._filesystem
     270        self._fs = port.filesystem
    273271        self._options = options
    274272        self._printer = printer
    275273        self._message_broker = None
     274        self._expectations = None
    276275
    277276        self.HTTP_SUBDIR = port.TEST_PATH_SEPARATOR + 'http' + port.TEST_PATH_SEPARATOR
    … …  
    323322            return path[len(self.LAYOUT_TESTS_DIRECTORY + self._port.TEST_PATH_SEPARATOR):]
    324323        if path.startswith(self.LAYOUT_TESTS_DIRECTORY + self._fs.sep):
    325             return path[len(self.LAYOUT_TEST_DIRECTORY + self._fs.sep):]
     324            return path[len(self.LAYOUT_TESTS_DIRECTORY + self._fs.sep):]
    326325        return path
    327326
    … …  
    409408                test_size = int(chunk_len)
    410409                assert(test_size > 0)
    411             except:
     410            except AssertionError:
    412411                _log.critical("invalid chunk '%s'" % chunk_value)
    413412                return None
    … …  
    755754        # reading messsages after receiving a stop, we can be sure each
    756755        # worker will get a stop message and hence they will all shut down.
    757         for i in xrange(num_workers):
     756        for _ in xrange(num_workers):
    758757            manager_connection.post_message('stop')
    759758
    … …  
    761760            while not self.is_done():
    762761                # Temporarily disabled to see how this code effect performance on the buildbots.
    763                 # if self._port.executive().running_pids(self._port.is_crash_reporter):
     762                # if self._port.executive.running_pids(self._port.is_crash_reporter):
    764763                #     self._printer.print_update("Waiting for crash reporter ...")
    765                 #     self._port.executive().wait_newest(self._port.is_crash_reporter)
     764                #     self._port.executive.wait_newest(self._port.is_crash_reporter)
    766765                manager_connection.run_message_loop(delay_secs=1.0)
    767766
    … …  
    10921091        generator.upload_json_files(json_files)
    10931092
    1094     def _print_config(self):
     1093    def print_config(self):
    10951094        """Prints the configuration for the test run."""
    10961095        p = self._printer
    … …  
    12921291        mean = sum(timings) / num_tests
    12931292
    1294         for time in timings:
    1295             sum_of_deviations = math.pow(time - mean, 2)
     1293        for timing in timings:
     1294            sum_of_deviations = math.pow(timing - mean, 2)
    12961295
    12971296        std_deviation = math.sqrt(sum_of_deviations / num_tests)
    … …  
    14381437
    14391438    def _log_worker_stack(self, stack):
    1440         webkitpydir = self._port.path_from_webkit_base('Tools', 'Scripts', 'webkitpy') + self._port._filesystem.sep
     1439        webkitpydir = self._port.path_from_webkit_base('Tools', 'Scripts', 'webkitpy') + self._port.filesystem.sep
    14411440        for filename, line_number, function_name, text in stack:
    14421441            if filename.startswith(webkitpydir):
    … …  
    14461445
    14471446
    1448 def read_test_files(fs, files, test_path_separator):
     1447def read_test_files(fs, filenames, test_path_separator):
    14491448    tests = []
    1450     for file in files:
     1449    for filename in filenames:
    14511450        try:
    14521451            if test_path_separator != fs.sep:
    1453                 file = file.replace(test_path_separator, fs.sep)
    1454             file_contents = fs.read_text_file(file).split('\n')
     1452                filename = filename.replace(test_path_separator, fs.sep)
     1453            file_contents = fs.read_text_file(filename).split('\n')
    14551454            for line in file_contents:
    14561455                line = test_expectations.strip_comments(line)
    … …  
    14891488        try:
    14901489            return int(val)
    1491         except:
     1490        except ValueError:
    14921491            return val
    14931492

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

    r90679 r90796  
    4646import Queue
    4747import sys
    48 import thread
    49 import threading
    50 import time
    5148
    5249# Handle Python < 2.6 where multiprocessing isn't available.
    … …  
    5653    multiprocessing = None
    5754
    58 from webkitpy.common.system import stack_utils
    59 from webkitpy.layout_tests import port
     55from webkitpy import layout_tests
    6056from webkitpy.layout_tests.controllers import message_broker
    6157from webkitpy.layout_tests.views import printing
    … …  
    112108
    113109class AbstractWorker(message_broker.BrokerClient):
    114     def __init__(self, broker_connection, worker_number, options):
     110    def __init__(self, worker_connection, worker_number, options):
    115111        """The constructor should be used to do any simple initialization
    116112        necessary, but should not do anything that creates data structures
    … …  
    120116
    121117        Args:
    122             broker_connection - handle to the BrokerConnection object creating
     118            worker_connection - handle to the BrokerConnection object creating
    123119                the worker and that can be used for messaging.
    124120            worker_number - identifier for this particular worker
    125121            options - command-line argument object from optparse"""
    126 
    127         raise NotImplementedError
     122        message_broker.BrokerClient.__init__(self)
     123        self._worker_connection = worker_connection
     124        self._options = options
     125        self._worker_number = worker_number
     126        self._name = 'worker/%d' % worker_number
    128127
    129128    def run(self, port):
    … …  
    214213class _InlineWorkerConnection(_WorkerConnection):
    215214    def __init__(self, broker, port, manager_client, worker_class, worker_number):
    216         _WorkerConnection.__init__(self, broker, worker_class, worker_number, port._options)
     215        _WorkerConnection.__init__(self, broker, worker_class, worker_number, port.options)
    217216        self._alive = False
    218217        self._port = port
    … …  
    255254        def run(self):
    256255            options = self._options
    257             port_obj = port.get(self._platform_name, options)
     256            port_obj = layout_tests.port.get(self._platform_name, options)
    258257
    259258            # The unix multiprocessing implementation clones the

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

    r90679 r90796  
    5858BrokerConnection).
    5959"""
     60import sys
     61import traceback
    6062
    6163import cPickle
    6264import logging
    6365import Queue
    64 import sys
    65 import time
    66 import traceback
    6766
    6867from webkitpy.common.system import stack_utils
    … …  
    8180    src indicates the name of the sender. If the message contains values in
    8281    the message body, those will be provided as optparams."""
    83 
    84     def __init__(self, *optargs, **kwargs):
    85         raise NotImplementedError
    8682
    8783    def is_done(self):
    … …  
    157153class _Message(object):
    158154    @staticmethod
    159     def loads(str):
    160         obj = cPickle.loads(str)
     155    def loads(string_value):
     156        obj = cPickle.loads(string_value)
    161157        assert(isinstance(obj, _Message))
    162158        return obj

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

    r90523 r90796  
    6666
    6767    def test_brokerclient_is_abstract(self):
    68         # Test that we can't create an instance directly.
    69         self.assertRaises(NotImplementedError, message_broker.BrokerClient)
    70 
    71         class TestClient(message_broker.BrokerClient):
    72             def __init__(self):
    73                 pass
    74 
    7568        # Test that all the base class methods are abstract and have the
    7669        # signature we expect.
    77         obj = TestClient()
     70        obj = message_broker.BrokerClient()
    7871        self.assertRaises(NotImplementedError, obj.is_done)
    7972        self.assertRaises(NotImplementedError, obj.name)

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

    r90679 r90796  
    4545class Worker(manager_worker_broker.AbstractWorker):
    4646    def __init__(self, worker_connection, worker_number, options):
    47         self._worker_connection = worker_connection
    48         self._worker_number = worker_number
    49         self._options = options
    50         self._name = 'worker/%d' % worker_number
     47        manager_worker_broker.AbstractWorker.__init__(self, worker_connection, worker_number, options)
    5148        self._done = False
    5249        self._canceled = False
    5350        self._port = None
     51        self._batch_size = 0
     52        self._batch_count = 0
     53        self._filesystem = None
     54        self._driver = None
     55        self._tests_run_file = None
     56        self._tests_run_filename = None
    5457
    5558    def __del__(self):
    … …  
    6366        across into a child process."""
    6467        self._port = port
    65         self._filesystem = port._filesystem
     68        self._filesystem = port.filesystem
    6669        self._batch_count = 0
    6770        self._batch_size = self._options.batch_size
    68         self._driver = None
    6971        tests_run_filename = self._filesystem.join(port.results_directory(),
    7072                                                   "tests_run%d.txt" % self._worker_number)
    … …  
    125127        #
    126128        # Temporarily disabled to see how this code effect performance on the buildbots.
    127         # self._port.executive().wait_newest(self._port.is_crash_reporter)
     129        # self._port.executive.wait_newest(self._port.is_crash_reporter)
    128130
    129131        test_timeout_sec = self.timeout(test_input)
    … …  
    169171        if self._options.run_singly:
    170172            return self._run_test_in_another_thread(test_input, timeout)
    171         else:
    172             return self._run_test_in_this_thread(test_input)
    173         return result
     173        return self._run_test_in_this_thread(test_input)
    174174
    175175    def clean_up_after_test(self, test_input, result):
    … …  
    214214        worker = self
    215215
    216         driver = worker._port.create_driver(worker._worker_number)
     216        driver = self._port.create_driver(self._worker_number)
    217217        driver.start()
    218218
    219219        class SingleTestThread(threading.Thread):
     220            def __init__(self):
     221                threading.Thread.__init__(self)
     222                self.result = None
     223
    220224            def run(self):
    221                 self.result = worker._run_single_test(driver, test_input)
     225                self.result = worker.run_single_test(driver, test_input)
    222226
    223227        thread = SingleTestThread()
    224228        thread.start()
    225229        thread.join(thread_timeout_sec)
    226         result = getattr(thread, 'result', None)
     230        result = thread.result
    227231        if thread.isAlive():
    228232            # If join() returned with the thread still running, the
    … …  
    253257            self._driver = self._port.create_driver(self._worker_number)
    254258            self._driver.start()
    255         return self._run_single_test(self._driver, test_input)
    256 
    257     def _run_single_test(self, driver, test_input):
     259        return self.run_single_test(self._driver, test_input)
     260
     261    def run_single_test(self, driver, test_input):
    258262        return single_test_runner.run_single_test(self._port, self._options,
    259263            test_input, driver, self._name)

  • trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py

    r90607 r90796  
    3737import errno
    3838import os
    39 import sys
    40 import time
    4139
    4240from webkitpy.common.net.testoutputset import AggregateTestOutputSet
    … …  
    4947
    5048from webkitpy.common import system
    51 from webkitpy.common.system import filesystem
    5249from webkitpy.common.system import logutils
    5350from webkitpy.common.system import path
    … …  
    6461_log = logutils.get_logger(__file__)
    6562
    66 
    6763class DummyOptions(object):
    6864    """Fake implementation of optparse.Values. Cloned from webkitpy.tool.mocktool.MockOptions."""
    … …  
    104100        # well-formed options object that had all of the necessary
    105101        # options defined on it.
    106         self._options = options or DummyOptions()
    107 
    108         self._executive = executive or Executive()
    109         self._user = user or User()
    110         self._filesystem = filesystem or system.filesystem.FileSystem()
    111         self._config = config or port_config.Config(self._executive, self._filesystem)
     102        self.options = options or DummyOptions()
     103
     104        self.executive = executive or Executive()
     105        self.user = user or User()
     106        self.filesystem = filesystem or system.filesystem.FileSystem()
     107        self.config = config or port_config.Config(self.executive, self.filesystem)
     108
     109        # FIXME: Remove all of the old "protected" versions when we can.
     110        self._options = self.options
     111        self._executive = self.executive
     112        self._filesystem = self.filesystem
     113        self._user = self.user
     114        self._config = self.config
    112115
    113116        self._helper = None
    … …  
    140143        self._results_directory = None
    141144
    142     def executive(self):
    143         return self._executive
    144 
    145145    def wdiff_available(self):
    146146        if self._wdiff_available is None:
    … …  
    194194        """Checks whether we can use the PrettyPatch ruby script."""
    195195        try:
    196             result = self._executive.run_command(['ruby', '--version'])
     196            _ = self._executive.run_command(['ruby', '--version'])
    197197        except OSError, e:
    198198            if e.errno in [errno.ENOENT, errno.EACCES, errno.ECHILD]:
    … …  
    216216
    217217        try:
    218             result = self._executive.run_command([self._path_to_wdiff(), '--help'])
     218            _ = self._executive.run_command([self._path_to_wdiff(), '--help'])
    219219        except OSError:
    220220            if logging:
    … …  
    226226    def check_httpd(self):
    227227        if self._uses_apache():
    228             path = self._path_to_apache()
     228            httpd_path = self._path_to_apache()
    229229        else:
    230             path = self._path_to_lighttpd()
     230            httpd_path = self._path_to_lighttpd()
    231231
    232232        try:
    233233            env = self.setup_environ_for_server()
    234             return self._executive.run_command([path, "-v"], env=env, return_exit_code=True) == 0
    235         except OSError, e:
     234            return self._executive.run_command([httpd_path, "-v"], env=env, return_exit_code=True) == 0
     235        except OSError:
    236236            _log.error("No httpd found. Cannot run http tests.")
    237237            return False
    … …  
    270270        # raw bytes and not unicode, so that they don't trigger join()
    271271        # trying to decode the input.
    272         def to_raw_bytes(str):
    273             if isinstance(str, unicode):
    274                 return str.encode('utf-8')
    275             return str
     272        def to_raw_bytes(string_value):
     273            if isinstance(string_value, unicode):
     274                return string_value.encode('utf-8')
     275            return string_value
    276276        expected_filename = to_raw_bytes(expected_filename)
    277277        actual_filename = to_raw_bytes(actual_filename)
    … …  
    378378    def expected_image(self, test_name):
    379379        """Returns the image we expect the test to produce."""
    380         path = self.expected_filename(test_name, '.png')
    381         if not self._filesystem.exists(path):
     380        baseline_path = self.expected_filename(test_name, '.png')
     381        if not self._filesystem.exists(baseline_path):
    382382            return None
    383         return self._filesystem.read_binary_file(path)
     383        return self._filesystem.read_binary_file(baseline_path)
    384384
    385385    def expected_audio(self, test_name):
    386         path = self.expected_filename(test_name, '.wav')
    387         if not self._filesystem.exists(path):
     386        baseline_path = self.expected_filename(test_name, '.wav')
     387        if not self._filesystem.exists(baseline_path):
    388388            return None
    389         return self._filesystem.read_binary_file(path)
     389        return self._filesystem.read_binary_file(baseline_path)
    390390
    391391    def expected_text(self, test_name):
    … …  
    396396        # output from DRT (instead treating it as a binary string), we read the
    397397        # baselines as a binary string, too.
    398         path = self.expected_filename(test_name, '.txt')
    399         if not self._filesystem.exists(path):
    400             path = self.expected_filename(test_name, '.webarchive')
    401             if not self._filesystem.exists(path):
     398        baseline_path = self.expected_filename(test_name, '.txt')
     399        if not self._filesystem.exists(baseline_path):
     400            baseline_path = self.expected_filename(test_name, '.webarchive')
     401            if not self._filesystem.exists(baseline_path):
    402402                return None
    403         text = self._filesystem.read_binary_file(path)
     403        text = self._filesystem.read_binary_file(baseline_path)
    404404        return text.replace("\r\n", "\n")
    405405
    … …  
    418418
    419419        port = None
    420         use_ssl = False
    421420
    422421        relative_path = test_name
    … …  
    454453        """Return True if the test name refers to a directory of tests."""
    455454        # Used by test_expectations.py to apply rules to whole directories.
    456         path = self.abspath_for_test(test_name)
    457         return self._filesystem.isdir(path)
     455        test_path = self.abspath_for_test(test_name)
     456        return self._filesystem.isdir(test_path)
    458457
    459458    def test_exists(self, test_name):
    … …  
    461460        # Used by test_expectations.py to determine if an entry refers to a
    462461        # valid test and by printing.py to determine if baselines exist.
    463         path = self.abspath_for_test(test_name)
    464         return self._filesystem.exists(path)
     462        test_path = self.abspath_for_test(test_name)
     463        return self._filesystem.exists(test_path)
    465464
    466465    def split_test(self, test_name):
    … …  
    482481        return driver.cmd_line()
    483482
    484     def update_baseline(self, path, data):
     483    def update_baseline(self, baseline_path, data):
    485484        """Updates the baseline for a test.
    486485
    487486        Args:
    488             path: the actual path to use for baseline, not the path to
     487            baseline_path: the actual path to use for baseline, not the path to
    489488              the test. This function is used to update either generic or
    490489              platform-specific baselines, but we can't infer which here.
    491490            data: contents of the baseline.
    492491        """
    493         self._filesystem.write_binary_file(path, data)
     492        self._filesystem.write_binary_file(baseline_path, data)
    494493
    495494    def uri_to_test_name(self, uri):
    … …  
    522521        """Return the absolute path to the top of the LayoutTests directory."""
    523522        return self.path_from_webkit_base('LayoutTests')
     523
     524    def skipped_layout_tests(self):
     525        return []
    524526
    525527    def skips_layout_test(self, test_name):
    … …  
    537539        return False
    538540
    539     def maybe_make_directory(self, *path):
     541    def maybe_make_directory(self, *comps):
    540542        """Creates the specified directory if it doesn't already exist."""
    541         self._filesystem.maybe_make_directory(*path)
     543        self._filesystem.maybe_make_directory(*comps)
    542544
    543545    def name(self):
    … …  
    922924        self.version = version or port.version()
    923925        self.architecture = architecture or port.architecture()
    924         self.build_type = build_type or port._options.configuration.lower()
     926        self.build_type = build_type or port.options.configuration.lower()
    925927        self.graphics_type = graphics_type or port.graphics_type()
    926928
    … …  
    947949        # every configuration on every system.
    948950        test_configurations = []
    949         for system in self.all_systems():
     951        for version, architecture in self.all_systems():
    950952            for build_type in self.all_build_types():
    951953                for graphics_type in self.all_graphics_types():
    952954                    test_configurations.append(TestConfiguration(
    953                         version=system[0],
    954                         architecture=system[1],
     955                        version=version,
     956                        architecture=architecture,
    955957                        build_type=build_type,
    956958                        graphics_type=graphics_type))

  • trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py

    r90546 r90796  
    3939import sys
    4040
    41 from webkitpy.common.net import resultsjsonparser
    42 from webkitpy.layout_tests import layout_package
     41from webkitpy import layout_tests
     42
    4343from webkitpy.layout_tests.controllers.manager import Manager, WorkerException
    44 from webkitpy.layout_tests.layout_package import json_results_generator
    4544from webkitpy.layout_tests.views import printing
    4645
    47 from webkitpy.common.system import user
    48 from webkitpy.thirdparty import simplejson
    49 
    50 import port
    5146
    5247_log = logging.getLogger(__name__)
    … …  
    8782    try:
    8883        manager = Manager(port, options, printer)
    89         manager._print_config()
     84        manager.print_config()
    9085
    9186        printer.print_update("Collecting tests ...")
    … …  
    120115
    121116
    122 def _set_up_derived_options(port_obj, options):
     117def _set_up_derived_options(port, options):
    123118    """Sets the options values that depend on other options values."""
    124119    # We return a list of warnings to print after the printer is initialized.
    … …  
    126121
    127122    if options.worker_model is None:
    128         options.worker_model = port_obj.default_worker_model()
     123        options.worker_model = port.default_worker_model()
    129124
    130125    if options.worker_model == 'inline':
    … …  
    134129    if not options.child_processes:
    135130        options.child_processes = os.environ.get("WEBKIT_TEST_CHILD_PROCESSES",
    136                                                  str(port_obj.default_child_processes()))
     131                                                 str(port.default_child_processes()))
    137132
    138133    if not options.configuration:
    139         options.configuration = port_obj.default_configuration()
     134        options.configuration = port.default_configuration()
    140135
    141136    if options.pixel_tests is None:
    … …  
    153148        normalized_platform_directories = []
    154149        for path in options.additional_platform_directory:
    155             if not port_obj._filesystem.isabs(path):
     150            if not port.filesystem.isabs(path):
    156151                warnings.append("--additional-platform-directory=%s is ignored since it is not absolute" % path)
    157152                continue
    158             normalized_platform_directories.append(port_obj._filesystem.normpath(path))
     153            normalized_platform_directories.append(port.filesystem.normpath(path))
    159154        options.additional_platform_directory = normalized_platform_directories
    160155
    … …  
    430425def main():
    431426    options, args = parse_args()
    432     port_obj = port.get(options.platform, options)
    433     return run(port_obj, options, args)
     427    port = layout_tests.port.get(options.platform, options)
     428    return run(port, options, args)
    434429
    435430
    … …  
    441436        INTERRUPTED_EXIT_STATUS = signal.SIGINT + 128
    442437        sys.exit(INTERRUPTED_EXIT_STATUS)
    443     except WorkerException, e:
     438    except WorkerException:
    444439        # This is a randomly chosen exit code that can be tested against to
    445440        # indicate that an unexpected exception occurred.
Note: See TracChangeset for help on using the changeset viewer.