investigate use of 'mac' and 'win' as fully-specified port names for the apple ports
authordpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Jan 2012 01:29:12 +0000 (01:29 +0000)
committerdpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Jan 2012 01:29:12 +0000 (01:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=76475

Reviewed by Eric Seidel.

This patch cleans up the internal usage of port names for the
Apple mac and Apple win ports so that 'mac' and 'win are never
considered "fully specified" port names: they are still legal
input to the PortFactory.get() routine, but only if run on the given
platform (i.e. trying to PortFactory.get('mac') on windows or
linux will return an error, since we don't know which version of
the port is desired.

This also cleans up handling of the webkit2 variants, so that
the full port names consistently follow the
<operating_system>-<version>[-<wk2>] convention.

Lastly this patch adds some assertions and removes some
try/catch logic to catch more programming and usage errors;
previously mac-tiger might've been allowed -- and it would've
translated into using just the baselines in platform/mac -- but
now it should fail.

* Scripts/webkitpy/layout_tests/port/apple.py:
(ApplePort.determine_full_port_name):
(ApplePort.__init__):
(ApplePort):
(ApplePort._port_name_with_version):
* Scripts/webkitpy/layout_tests/port/builders.py:
* Scripts/webkitpy/layout_tests/port/factory_unittest.py:
(FactoryTest.test_mac):
(FactoryTest.test_win):
* Scripts/webkitpy/layout_tests/port/mac.py:
(MacPort.baseline_search_path):
* Scripts/webkitpy/layout_tests/port/mac_unittest.py:
(MacTest.test_versions):
* Scripts/webkitpy/layout_tests/port/win.py:
(WinPort.baseline_search_path):
* Scripts/webkitpy/layout_tests/port/win_unittest.py:
(WinTest.test_versions):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@105951 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog
Tools/Scripts/webkitpy/layout_tests/port/apple.py
Tools/Scripts/webkitpy/layout_tests/port/builders.py
Tools/Scripts/webkitpy/layout_tests/port/factory_unittest.py
Tools/Scripts/webkitpy/layout_tests/port/mac.py
Tools/Scripts/webkitpy/layout_tests/port/mac_unittest.py
Tools/Scripts/webkitpy/layout_tests/port/win.py
Tools/Scripts/webkitpy/layout_tests/port/win_unittest.py

index b9f93bd..14d5a76 100644 (file)
@@ -1,3 +1,46 @@
+2012-01-23  Dirk Pranke  <dpranke@chromium.org>
+
+        investigate use of 'mac' and 'win' as fully-specified port names for the apple ports
+        https://bugs.webkit.org/show_bug.cgi?id=76475
+
+        Reviewed by Eric Seidel.
+
+        This patch cleans up the internal usage of port names for the
+        Apple mac and Apple win ports so that 'mac' and 'win are never
+        considered "fully specified" port names: they are still legal
+        input to the PortFactory.get() routine, but only if run on the given
+        platform (i.e. trying to PortFactory.get('mac') on windows or
+        linux will return an error, since we don't know which version of
+        the port is desired.
+
+        This also cleans up handling of the webkit2 variants, so that
+        the full port names consistently follow the
+        <operating_system>-<version>[-<wk2>] convention.
+
+        Lastly this patch adds some assertions and removes some
+        try/catch logic to catch more programming and usage errors;
+        previously mac-tiger might've been allowed -- and it would've
+        translated into using just the baselines in platform/mac -- but
+        now it should fail.
+
+        * Scripts/webkitpy/layout_tests/port/apple.py:
+        (ApplePort.determine_full_port_name):
+        (ApplePort.__init__):
+        (ApplePort):
+        (ApplePort._port_name_with_version):
+        * Scripts/webkitpy/layout_tests/port/builders.py:
+        * Scripts/webkitpy/layout_tests/port/factory_unittest.py:
+        (FactoryTest.test_mac):
+        (FactoryTest.test_win):
+        * Scripts/webkitpy/layout_tests/port/mac.py:
+        (MacPort.baseline_search_path):
+        * Scripts/webkitpy/layout_tests/port/mac_unittest.py:
+        (MacTest.test_versions):
+        * Scripts/webkitpy/layout_tests/port/win.py:
+        (WinPort.baseline_search_path):
+        * Scripts/webkitpy/layout_tests/port/win_unittest.py:
+        (WinTest.test_versions):
+
 2012-01-25  Dirk Pranke  <dpranke@chromium.org>
 
         webkitpy: print nicer errors while linting expectations files, remove redundant tests
index f0befe9..ac425c9 100644 (file)
@@ -51,20 +51,12 @@ class ApplePort(WebKitPort):
 
     @classmethod
     def determine_full_port_name(cls, host, options, port_name):
-        if port_name.endswith('-wk2'):
-            # FIXME: This may be wrong, since options is a global property, and the port_name is specific to this object?
-            if options and not hasattr(options, 'webkit_test_runner'):
-                options.webkit_test_runner = True
-            port_name = port_name.replace('-wk2', '')
         if port_name == cls.port_name:
-            # FIXME: This is almost certainly wrong; we shouldn't be returning a port name at all
-            # if we're on the wrong platform and it isn't fully specified. Also, 'mac' and 'win'
-            # shouldn't be legal port names, since we can't distinguish them from underspecified
-            # abbreviations (i.e., does 'mac' refer to 'mac-future' or 'the version of mac matching the
-            # version of the operating system on the host machine'?).
-            if port_name != host.platform.os_name:
-                return port_name
-            return port_name + '-' + host.platform.os_version
+            assert port_name == host.platform.os_name
+            return cls.port_name + '-' + host.platform.os_version
+        if port_name == cls.port_name + '-wk2':
+            assert port_name == host.platform.os_name
+            return cls.port_name + '-' + host.platform.os_version + '-wk2'
         return port_name
 
     def _strip_port_name_prefix(self, port_name):
@@ -78,15 +70,14 @@ class ApplePort(WebKitPort):
         WebKitPort.__init__(self, host, port_name, **kwargs)
 
         allowed_port_names = self.VERSION_FALLBACK_ORDER + [self.operating_system() + "-future"]
+        port_name = port_name.replace('-wk2', '')
         assert port_name in allowed_port_names, "%s is not in %s" % (port_name, allowed_port_names)
         self._version = self._strip_port_name_prefix(port_name)
 
-    # FIXME: A more sophisitcated version of this function should move to WebKitPort and replace all calls to name().
+    # FIXME: A more sophisticated version of this function should move to WebKitPort and replace all calls to name().
+    # This is also a misleading name, since 'mac-future' gets remapped to 'mac'.
     def _port_name_with_version(self):
-        components = [self.port_name]
-        if self._version != self.FUTURE_VERSION:
-            components.append(self._version)
-        return '-'.join(components)
+        return self.name().replace('-future', '').replace('-wk2', '')
 
     def _generate_all_test_configurations(self):
         configurations = []
index f05baf9..42fed50 100644 (file)
@@ -63,10 +63,10 @@ _exact_matches = {
     "GTK Linux 32-bit Debug": {"port_name": "gtk", "specifiers": set(["gtk"])},
     "Leopard Intel Debug (Tests)": {"port_name": "mac-leopard", "specifiers": set(["leopard"])},
     "SnowLeopard Intel Release (Tests)": {"port_name": "mac-snowleopard", "specifiers": set(["snowleopard"])},
-    "SnowLeopard Intel Release (WebKit2 Tests)": {"port_name": "mac-wk2", "specifiers": set(["wk2"])},
+    "SnowLeopard Intel Release (WebKit2 Tests)": {"port_name": "mac-snowleopard-wk2", "specifiers": set(["wk2"])},
     "Qt Linux Release": {"port_name": "qt-linux", "specifiers": set(["win", "linux", "mac"])},
     "Windows XP Debug (Tests)": {"port_name": "win-xp", "specifiers": set(["win"])},
-    "Windows 7 Release (WebKit2 Tests)": {"port_name": "win-wk2", "specifiers": set(["wk2"])},
+    "Windows 7 Release (WebKit2 Tests)": {"port_name": "win-future-wk2", "specifiers": set(["wk2"])},
 }
 
 
index c199626..8828518 100644 (file)
@@ -59,11 +59,15 @@ class FactoryTest(unittest.TestCase):
         self.assertTrue(isinstance(port, cls))
 
     def test_mac(self):
-        self.assert_port(port_name='mac', cls=mac.MacPort)
+        self.assert_port(port_name='mac-leopard', cls=mac.MacPort)
+        self.assert_port(port_name='mac-leopard-wk2', cls=mac.MacPort)
+        self.assert_port(port_name='mac', os_name='mac', os_version='leopard', cls=mac.MacPort)
         self.assert_port(port_name=None,  os_name='mac', os_version='leopard', cls=mac.MacPort)
 
     def test_win(self):
-        self.assert_port(port_name='win', cls=win.WinPort)
+        self.assert_port(port_name='win-xp', cls=win.WinPort)
+        self.assert_port(port_name='win-xp-wk2', cls=win.WinPort)
+        self.assert_port(port_name='win', os_name='win', os_version='xp', cls=win.WinPort)
         self.assert_port(port_name=None, os_name='win', os_version='xp', cls=win.WinPort)
         self.assert_port(port_name=None, os_name='win', os_version='xp', options=self.webkit_options, cls=win.WinPort)
 
index fc1925c..e23fb82 100644 (file)
@@ -51,12 +51,8 @@ class MacPort(ApplePort):
             self.set_option_default("batch_size", 1000)
 
     def baseline_search_path(self):
-        try:
-            fallback_index = self.VERSION_FALLBACK_ORDER.index(self._port_name_with_version())
-            fallback_names = list(self.VERSION_FALLBACK_ORDER[fallback_index:])
-        except ValueError:
-            # Unknown versions just fall back to the base port results.
-            fallback_names = [self.port_name]
+        fallback_index = self.VERSION_FALLBACK_ORDER.index(self._port_name_with_version())
+        fallback_names = list(self.VERSION_FALLBACK_ORDER[fallback_index:])
         if self.get_option('webkit_test_runner'):
             fallback_names.insert(0, self._wk2_port_name())
             # Note we do not add 'wk2' here, even though it's included in _skipped_search_paths().
index 7376629..43a8008 100644 (file)
@@ -114,6 +114,8 @@ java/
         self.assert_name('mac', 'future', 'mac-future')
         self.assert_name('mac-future', 'future', 'mac-future')
 
+        self.assertRaises(AssertionError, self.assert_name, 'mac-tiger', 'leopard', 'mac-leopard')
+
 
     def test_is_version_methods(self):
         leopard_port = self.make_port(port_name='mac-leopard')
index 323bc1d..ab7103c 100644 (file)
@@ -60,12 +60,8 @@ class WinPort(ApplePort):
         return expected_text != actual_text
 
     def baseline_search_path(self):
-        try:
-            fallback_index = self.VERSION_FALLBACK_ORDER.index(self._port_name_with_version())
-            fallback_names = list(self.VERSION_FALLBACK_ORDER[fallback_index:])
-        except ValueError:
-            # Unknown versions just fall back to the base port results.
-            fallback_names = [self.port_name]
+        fallback_index = self.VERSION_FALLBACK_ORDER.index(self._port_name_with_version())
+        fallback_names = list(self.VERSION_FALLBACK_ORDER[fallback_index:])
         # FIXME: The AppleWin port falls back to AppleMac for some results.  Eventually we'll have a shared 'apple' port.
         if self.get_option('webkit_test_runner'):
             fallback_names.insert(0, 'win-wk2')
index c016083..760f98d 100644 (file)
@@ -80,6 +80,7 @@ class WinPortTest(port_testcase.PortTestCase):
         self._assert_version('win-xp', 'xp')
         self._assert_version('win-vista', 'vista')
         self._assert_version('win-7sp0', '7sp0')
+        self.assertRaises(AssertionError, self._assert_version, 'win-me', 'xp')
 
     def test_compare_text(self):
         expected = "EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification\nfoo\nEDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification\n"