Refactor test.py to be easier to fake.

third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/test.py

Index: third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/test.py
diff --git a/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/test.py b/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/test.py
index ccd37dfed664224a532896865a1861be0de5eba8..7036bc9453e1d82935d0e57efe3cfa83e8d7f6ce 100644
--- a/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/test.py
+++ b/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/port/test.py
@@ -388,6 +388,23 @@ class TestPort(Port):
         'linux32': ['test-linux-x86', 'test-linux-precise', 'test-linux-trusty', 'test-win-win7'],
     }
 
+    _all_systems = (('mac10.10', 'x86'),
+                    ('mac10.11', 'x86'),
+                    ('win7', 'x86'),
+                    ('win10', 'x86'),
+                    ('linux32', 'x86'),
+                    ('precise', 'x86_64'),
+                    ('trusty', 'x86_64'))
+
+    _all_build_types = ('debug', 'release')
+
+    """To avoid surprises when introducing new macros, these are intentionally fixed in time."""

[qyearsley, 2016/07/06 18:44:48]

Strings are usually only used for comments if they're at the top of a class or
function or module -- because in these circumstances they get set as the __doc__
attribute of the class/function/module.

In other places in the code, it's conventional to just use line comments that
start with #.

[bokan, 2016/07/06 21:29:19]

Thanks, done.

+    _configuration_specifier_macros = {
+        'mac': ['mac10.10', 'mac10.11'],
+        'win': ['win7', 'win10'],
+        'linux': ['linux32', 'precise', 'trusty']
+    }
+
     @classmethod
     def determine_full_port_name(cls, host, options, port_name):
         if port_name == 'test':
@@ -528,33 +545,16 @@ class TestPort(Port):
         # By default, we assume we want to test every graphics type in
         # every configuration on every system.
         test_configurations = []
-        for version, architecture in self._all_systems():
-            for build_type in self._all_build_types():
+        for version, architecture in self._all_systems:
+            for build_type in self._all_build_types:
                 test_configurations.append(TestConfiguration(
                     version=version,
                     architecture=architecture,
                     build_type=build_type))
         return test_configurations
 
-    def _all_systems(self):
-        return (('mac10.10', 'x86'),
-                ('mac10.11', 'x86'),
-                ('win7', 'x86'),
-                ('win10', 'x86'),
-                ('linux32', 'x86'),
-                ('precise', 'x86_64'),
-                ('trusty', 'x86_64'))

[qyearsley, 2016/07/06 18:44:48]

Could this also be faked as-is without this CL?

E.g. in a test method where you want to set `port._all_systems = (...)`, could
you instead use `port._all_systems = lambda: (...)`?

[bokan, 2016/07/06 21:29:19]

Right, (Python isn't my native tongue :P) if you think that's better I can go
ahead and change the tests and drop this CL.

[qyearsley, 2016/07/06 22:40:53]

I think this is fine -- I can't think any particular advantage of having these
as functions rather than attributes, so this CL LGTM (once you've made the
change Dirk suggested).

-
-    def _all_build_types(self):
-        return ('debug', 'release')
-
     def configuration_specifier_macros(self):
-        """To avoid surprises when introducing new macros, these are intentionally fixed in time."""
-        return {
-            'mac': ['mac10.10', 'mac10.11'],
-            'win': ['win7', 'win10'],
-            'linux': ['linux32', 'precise', 'trusty']
-        }
+        return self._configuration_specifier_macros
 
     def virtual_test_suites(self):
         return [