Bug 71403 – META: strip the Port class of its godlike powers

• Status: NEW
• Component: Tools / Tests

[Dirk Pranke]

the Port class in webkitpy.layout_tests.port predates much of the other infrastructure in webkitpy and now does too many things that should probably be done elsewhere. This bug is intended to track that cleanup.

Some possible improvements:

* the 'filesystem', 'executive', and 'config' arguments should be passed into the constructors, either explicitly or via a host object. They should not be public properties and other parts of the code should not retrieve references to filesystem, executive, or config objects from the Port class

* at least the following routines should probably be moved elsewhere or reworked to push more of the logic out of the Port directory:
  * wdiff_available(), check_image_diff(), check_pretty_patch(), check_wdiff(), check_httpd()
  * webkit_base(), path_from_webkit_base(), layout_tests_dir(), webkit_base(), path_from_webkit_base()
  * maybe_make_directory(), split_test()
  * operating_system(), version(), graphics_type(), architecture(), test_configuration(), all_test_configurations(), configuration_specifier_macros(), all_baseline_variants() - these should somehow be merged into the test_configuration() objects and split out somehow
  * get_option(), set_option() - these should be better integrated into the options parameter that is passed to the constructor and we need a better way to initialize port-specific default options.
  * results_directory() and default_results_directory()
  * show_results_html_file()

This list is merely meant as a first draft, and not intended to be exhaustive or definitive.

Here's the rationale I have in mind for what should stay in the port package. These principles are open for debate.

1) "Ports" in the webkitpy sense capture the combination of an implementation of a "webkit port" in the Apple/Chromium/Qt/Gtk sense and the "port" in the operating system sense (mac/win/linux/etc. plus architecture like arm/x86/x86_64). A single concrete port specifies a single combination of those attributes, but of course mixes in behaviour that is shared (e.g., WebKitPort, ChromiumPort are really mix-ins). Most if not all of the operating-system-type functionality should actually be implemented in webkit.common.system, however.

1) Anything needed to be able to implement the Test Port properly belongs on the Port interface, so that unit and integration tests can run quickly, reliably, and portably. The primary reason for the Test port to exist is to demonstrate the routines that may need to be overridden to implement a port that works radically differently than the "real" ports.

2) Anything that manages how tests are found and named (this allows for handling filesystem path differences like unix/win separators, and also allows for in-memory constructs like those used in the Test port. They ensure that more esoteric ports that don't necessarily have a full suite of tests embedded under a single LayoutTests directory  in the filesystem can be supported without having to change the rest of NRWT.

3) Anything that is actually overridden in one of the Port subclasses.

[Eric Seidel (no email)]

I think this is a great start.  This will be a great meta-bug for doing this work.  Thanks.

[Dirk Pranke]

I will note that cleaning up the way that port objects are constructed (all the name parsing and handling of defaults and so on) is probably tangled up in this somehow but I would like to leave this out of scope as much as possible.

[Dirk Pranke]

resetting the owner in case someone else wants to take a look, as these bugs aren't on my immediate to-do list.

[Dirk Pranke]

I now think the stuff in 2) should probably be a separate object, mostly to make the Port conceptually smaller and make it clearer what tends to vary between ports. I've filed bug 103827 for that effort.