[webkit-changes] [111034] trunk/Tools

Fri, 16 Mar 2012 11:31:52 -0700

Title: [111034] trunk/Tools
Revision
111034
Author
o...@chromium.org
Date
2012-03-16 11:31:33 -0700 (Fri, 16 Mar 2012)

Log Message

Specifier collapsing when writing test expectations lines gets a number of cases wrong
https://bugs.webkit.org/show_bug.cgi?id=81309

Reviewed by Dimitri Glazkov.

I've run this over all the lines in the current Chromium test_expectations.txt file,
so I'm relatively confident we now cover all the cases.

* Scripts/webkitpy/layout_tests/models/test_configuration.py:
(TestConfigurationConverter.__init__):
(TestConfigurationConverter.collapse_macros):
(TestConfigurationConverter.collapse_macros.collapse_individual_specifier_set):
(TestConfigurationConverter):
(TestConfigurationConverter.intersect_combination):
(TestConfigurationConverter.symmetric_difference):
(TestConfigurationConverter.to_specifiers_list):
(TestConfigurationConverter.to_specifiers_list.try_collapsing):
(TestConfigurationConverter.to_specifiers_list.try_abbreviating):
* Scripts/webkitpy/layout_tests/models/test_configuration_unittest.py:
(make_mock_all_test_configurations_set):
(TestConfigurationConverterTest.test_symmetric_difference):
(TestConfigurationConverterTest.test_to_config_set):
(TestConfigurationConverterTest.test_macro_expansion):
(TestConfigurationConverterTest.test_to_specifier_lists):
(TestConfigurationConverterTest.test_converter_macro_collapsing):
* Scripts/webkitpy/layout_tests/port/chromium.py:
(ChromiumPort):
* Scripts/webkitpy/layout_tests/port/chromium_android.py:
(ChromiumAndroidPort.__init__):
The android port uses "arm" as it's architecture, which is technically correct,
but considerably complicates making collapsing work. We probably should kill
the concept of architecture entirely. The benefits are not worth the code
complexity.

* Scripts/webkitpy/layout_tests/port/chromium_unittest.py:
(ChromiumPortTest.test_all_test_configurations):
* Scripts/webkitpy/tool/commands/rebaseline_unittest.py:
(TestRebaseline.test_rebaseline_updates_expectations_file_noop):
(test_rebaseline_updates_expectations_file):

Modified Paths

Diff

Modified: trunk/Tools/ChangeLog (111033 => 111034)


--- trunk/Tools/ChangeLog	2012-03-16 18:27:21 UTC (rev 111033)
+++ trunk/Tools/ChangeLog	2012-03-16 18:31:33 UTC (rev 111034)
@@ -1,3 +1,45 @@
+2012-03-15  Ojan Vafai  <o...@chromium.org>
+
+        Specifier collapsing when writing test expectations lines gets a number of cases wrong
+        https://bugs.webkit.org/show_bug.cgi?id=81309
+
+        Reviewed by Dimitri Glazkov.
+
+        I've run this over all the lines in the current Chromium test_expectations.txt file,
+        so I'm relatively confident we now cover all the cases.
+
+        * Scripts/webkitpy/layout_tests/models/test_configuration.py:
+        (TestConfigurationConverter.__init__):
+        (TestConfigurationConverter.collapse_macros):
+        (TestConfigurationConverter.collapse_macros.collapse_individual_specifier_set):
+        (TestConfigurationConverter):
+        (TestConfigurationConverter.intersect_combination):
+        (TestConfigurationConverter.symmetric_difference):
+        (TestConfigurationConverter.to_specifiers_list):
+        (TestConfigurationConverter.to_specifiers_list.try_collapsing):
+        (TestConfigurationConverter.to_specifiers_list.try_abbreviating):
+        * Scripts/webkitpy/layout_tests/models/test_configuration_unittest.py:
+        (make_mock_all_test_configurations_set):
+        (TestConfigurationConverterTest.test_symmetric_difference):
+        (TestConfigurationConverterTest.test_to_config_set):
+        (TestConfigurationConverterTest.test_macro_expansion):
+        (TestConfigurationConverterTest.test_to_specifier_lists):
+        (TestConfigurationConverterTest.test_converter_macro_collapsing):
+        * Scripts/webkitpy/layout_tests/port/chromium.py:
+        (ChromiumPort):
+        * Scripts/webkitpy/layout_tests/port/chromium_android.py:
+        (ChromiumAndroidPort.__init__):
+        The android port uses "arm" as it's architecture, which is technically correct,
+        but considerably complicates making collapsing work. We probably should kill
+        the concept of architecture entirely. The benefits are not worth the code
+        complexity.
+
+        * Scripts/webkitpy/layout_tests/port/chromium_unittest.py:
+        (ChromiumPortTest.test_all_test_configurations):
+        * Scripts/webkitpy/tool/commands/rebaseline_unittest.py:
+        (TestRebaseline.test_rebaseline_updates_expectations_file_noop):
+        (test_rebaseline_updates_expectations_file):
+
 2012-03-16  Dinu Jacob  <dinu.ja...@nokia.com>
 
         [Qt][Wk2] Assertion Failure and crash on file upload

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/models/test_configuration.py (111033 => 111034)


--- trunk/Tools/Scripts/webkitpy/layout_tests/models/test_configuration.py	2012-03-16 18:27:21 UTC (rev 111033)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/models/test_configuration.py	2012-03-16 18:31:33 UTC (rev 111034)
@@ -115,19 +115,19 @@
         self._specifier_sorter = SpecifierSorter()
         self._collapsing_sets_by_size = {}
         self._junk_specifier_combinations = {}
-        collapsing_sets_by_category = {}
+        self._collapsing_sets_by_category = {}
         matching_sets_by_category = {}
         for configuration in all_test_configurations:
             for category, specifier in configuration.items():
                 self._specifier_to_configuration_set.setdefault(specifier, set()).add(configuration)
                 self._specifier_sorter.add_specifier(category, specifier)
-                collapsing_sets_by_category.setdefault(category, set()).add(specifier)
+                self._collapsing_sets_by_category.setdefault(category, set()).add(specifier)
                 # FIXME: This seems extra-awful.
                 for cat2, spec2 in configuration.items():
                     if category == cat2:
                         continue
                     matching_sets_by_category.setdefault(specifier, {}).setdefault(cat2, set()).add(spec2)
-        for collapsing_set in collapsing_sets_by_category.values():
+        for collapsing_set in self._collapsing_sets_by_category.values():
             self._collapsing_sets_by_size.setdefault(len(collapsing_set), set()).add(frozenset(collapsing_set))
 
         for specifier, sets_by_category in matching_sets_by_category.items():
@@ -165,16 +165,49 @@
 
     @classmethod
     def collapse_macros(cls, macros_dict, specifiers_list):
-        for i in range(len(specifiers_list)):
-            for macro_specifier, macro in macros_dict.items():
-                specifiers_set = set(specifiers_list[i])
+        for macro_specifier, macro in macros_dict.items():
+            if len(macro) == 1:
+                continue
+
+            for combination in itertools.combinations(specifiers_list, len(macro)):
+                if cls.symmetric_difference(combination) == set(macro):
+                    for item in combination:
+                        specifiers_list.remove(item)
+                    new_specifier_set = cls.intersect_combination(combination)
+                    new_specifier_set.add(macro_specifier)
+                    specifiers_list.append(frozenset(new_specifier_set))
+
+        def collapse_individual_specifier_set(macro_specifier, macro):
+            specifiers_to_remove = []
+            specifiers_to_add = []
+            for specifier_set in specifiers_list:
                 macro_set = set(macro)
-                if specifiers_set >= macro_set:
-                    specifiers_list[i] = frozenset((specifiers_set - macro_set) | set([macro_specifier]))
+                if macro_set.intersection(specifier_set) == macro_set:
+                    specifiers_to_remove.append(specifier_set)
+                    specifiers_to_add.append(frozenset((set(specifier_set) - macro_set) | set([macro_specifier])))
+            for specifier in specifiers_to_remove:
+                specifiers_list.remove(specifier)
+            for specifier in specifiers_to_add:
+                specifiers_list.append(specifier)
 
+        for macro_specifier, macro in macros_dict.items():
+            collapse_individual_specifier_set(macro_specifier, macro)
+
+    @classmethod
+    def intersect_combination(cls, combination):
+        return reduce(set.intersection, [set(specifiers) for specifiers in combination])
+
+    @classmethod
+    def symmetric_difference(cls, iterable):
+        union = set()
+        intersection = iterable[0]
+        for item in iterable:
+            union = union | item
+            intersection = intersection.intersection(item)
+        return union - intersection
+
     def to_specifiers_list(self, test_configuration_set):
         """Convert a set of TestConfiguration instances into one or more list of specifiers."""
-
         # Easy out: if the set is all configurations, the modifier is empty.
         if len(test_configuration_set) == len(self._all_test_configurations):
             return [[]]
@@ -188,20 +221,14 @@
                     values -= junk_specifier_set
             specifiers_list.append(frozenset(values))
 
-        def intersect_combination(combination):
-            return reduce(set.intersection, [set(specifiers) for specifiers in combination])
-
-        def symmetric_difference(iterable):
-            return reduce(lambda x, y: x ^ y, iterable)
-
         def try_collapsing(size, collapsing_sets):
             if len(specifiers_list) < size:
                 return False
             for combination in itertools.combinations(specifiers_list, size):
-                if symmetric_difference(combination) in collapsing_sets:
+                if self.symmetric_difference(combination) in collapsing_sets:
                     for item in combination:
                         specifiers_list.remove(item)
-                    specifiers_list.append(frozenset(intersect_combination(combination)))
+                    specifiers_list.append(frozenset(self.intersect_combination(combination)))
                     return True
             return False
 
@@ -211,14 +238,14 @@
             while try_collapsing(size, collapsing_sets):
                 pass
 
-        def try_abbreviating():
+        def try_abbreviating(collapsing_sets):
             if len(specifiers_list) < 2:
                 return False
             for combination in itertools.combinations(specifiers_list, 2):
                 for collapsing_set in collapsing_sets:
-                    diff = symmetric_difference(combination)
+                    diff = self.symmetric_difference(combination)
                     if diff <= collapsing_set:
-                        common = intersect_combination(combination)
+                        common = self.intersect_combination(combination)
                         for item in combination:
                             specifiers_list.remove(item)
                         specifiers_list.append(frozenset(common | diff))
@@ -227,11 +254,30 @@
 
         # 3) Abbreviate specifier sets by combining specifiers across categories.
         #   (xp, release), (win7, release) --> (xp, win7, release)
-        while try_abbreviating():
+        while try_abbreviating(self._collapsing_sets_by_size.values()):
             pass
 
+
         # 4) Substitute specifier subsets that match macros witin each set:
         #   (xp, vista, win7, release) -> (win, release)
         self.collapse_macros(self._configuration_macros, specifiers_list)
 
+        macro_keys = set(self._configuration_macros.keys())
+
+        # 5) Collapsing macros may have created combinations the can now be abbreviated.
+        #   (xp, release), (linux, x86, release), (linux, x86_64, release) --> (xp, release), (linux, release) --> (xp, linux, release)
+        while try_abbreviating([self._collapsing_sets_by_category['version'] | macro_keys]):
+            pass
+
+        # 6) Remove cases where we have collapsed but have all macros.
+        #   (android, win, mac, linux, release) --> (release)
+        specifiers_to_remove = []
+        for specifier_set in specifiers_list:
+            if macro_keys <= specifier_set:
+                specifiers_to_remove.append(specifier_set)
+
+        for specifier_set in specifiers_to_remove:
+            specifiers_list.remove(specifier_set)
+            specifiers_list.append(frozenset(specifier_set - macro_keys))
+
         return specifiers_list

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/models/test_configuration_unittest.py (111033 => 111034)


--- trunk/Tools/Scripts/webkitpy/layout_tests/models/test_configuration_unittest.py	2012-03-16 18:27:21 UTC (rev 111033)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/models/test_configuration_unittest.py	2012-03-16 18:31:33 UTC (rev 111034)
@@ -35,14 +35,14 @@
 
 def make_mock_all_test_configurations_set():
     all_test_configurations = set()
-    for version, architecture in (('snowleopard', 'x86'), ('xp', 'x86'), ('win7', 'x86'), ('lucid', 'x86'), ('lucid', 'x86_64')):
+    for version, architecture in (('snowleopard', 'x86'), ('xp', 'x86'), ('win7', 'x86'), ('vista', 'x86'), ('lucid', 'x86'), ('lucid', 'x86_64')):
         for build_type in ('debug', 'release'):
             all_test_configurations.add(TestConfiguration(version, architecture, build_type))
     return all_test_configurations
 
 MOCK_MACROS = {
     'mac': ['snowleopard'],
-    'win': ['xp', 'win7'],
+    'win': ['xp', 'vista', 'win7'],
     'linux': ['lucid'],
 }
 
@@ -156,6 +156,10 @@
         self._all_test_configurations = make_mock_all_test_configurations_set()
         unittest.TestCase.__init__(self, testFunc)
 
+    def test_symmetric_difference(self):
+        self.assertEquals(TestConfigurationConverter.symmetric_difference([set(['a', 'b']), set(['b', 'c'])]), set(['a', 'c']))
+        self.assertEquals(TestConfigurationConverter.symmetric_difference([set(['a', 'b']), set(['b', 'c']), set(['b', 'd'])]), set(['a', 'c', 'd']))
+
     def test_to_config_set(self):
         converter = TestConfigurationConverter(self._all_test_configurations)
 
@@ -178,6 +182,7 @@
 
         configs_to_match = set([
             TestConfiguration('snowleopard', 'x86', 'release'),
+            TestConfiguration('vista', 'x86', 'release'),
             TestConfiguration('win7', 'x86', 'release'),
             TestConfiguration('xp', 'x86', 'release'),
             TestConfiguration('lucid', 'x86', 'release'),
@@ -221,12 +226,14 @@
 
         configs_to_match = set([
             TestConfiguration('xp', 'x86', 'release'),
+            TestConfiguration('vista', 'x86', 'release'),
             TestConfiguration('win7', 'x86', 'release'),
         ])
         self.assertEquals(converter.to_config_set(set(['win', 'release'])), configs_to_match)
 
         configs_to_match = set([
             TestConfiguration('xp', 'x86', 'release'),
+            TestConfiguration('vista', 'x86', 'release'),
             TestConfiguration('win7', 'x86', 'release'),
             TestConfiguration('lucid', 'x86', 'release'),
             TestConfiguration('lucid', 'x86_64', 'release'),
@@ -235,13 +242,14 @@
 
         configs_to_match = set([
             TestConfiguration('xp', 'x86', 'release'),
+            TestConfiguration('vista', 'x86', 'release'),
             TestConfiguration('win7', 'x86', 'release'),
             TestConfiguration('snowleopard', 'x86', 'release'),
         ])
         self.assertEquals(converter.to_config_set(set(['win', 'mac', 'release'])), configs_to_match)
 
     def test_to_specifier_lists(self):
-        converter = TestConfigurationConverter(self._all_test_configurations)
+        converter = TestConfigurationConverter(self._all_test_configurations, MOCK_MACROS)
 
         self.assertEquals(converter.to_specifiers_list(set(self._all_test_configurations)), [[]])
         self.assertEquals(converter.to_specifiers_list(set()), [])
@@ -261,7 +269,7 @@
             TestConfiguration('lucid', 'x86_64', 'debug'),
             TestConfiguration('xp', 'x86', 'release'),
         ])
-        self.assertEquals(converter.to_specifiers_list(configs_to_match), [set(['debug', 'x86_64', 'lucid']), set(['release', 'xp'])])
+        self.assertEquals(converter.to_specifiers_list(configs_to_match), [set(['release', 'xp']), set(['debug', 'x86_64', 'linux'])])
 
         configs_to_match = set([
             TestConfiguration('xp', 'x86', 'release'),
@@ -271,11 +279,12 @@
             TestConfiguration('lucid', 'x86_64', 'debug'),
             TestConfiguration('lucid', 'x86', 'debug'),
         ])
-        self.assertEquals(converter.to_specifiers_list(configs_to_match), [set(['release', 'xp']), set(['debug', 'lucid'])])
+        self.assertEquals(converter.to_specifiers_list(configs_to_match), [set(['release', 'xp']), set(['debug', 'linux'])])
 
         configs_to_match = set([
             TestConfiguration('xp', 'x86', 'release'),
             TestConfiguration('snowleopard', 'x86', 'release'),
+            TestConfiguration('vista', 'x86', 'release'),
             TestConfiguration('win7', 'x86', 'release'),
             TestConfiguration('lucid', 'x86', 'release'),
             TestConfiguration('lucid', 'x86_64', 'release'),
@@ -286,7 +295,7 @@
             TestConfiguration('xp', 'x86', 'release'),
             TestConfiguration('snowleopard', 'x86', 'release'),
         ])
-        self.assertEquals(converter.to_specifiers_list(configs_to_match), [set(['xp', 'snowleopard', 'release'])])
+        self.assertEquals(converter.to_specifiers_list(configs_to_match), [set(['xp', 'mac', 'release'])])
 
         configs_to_match = set([
             TestConfiguration('xp', 'x86', 'release'),
@@ -295,7 +304,7 @@
             TestConfiguration('win7', 'x86', 'debug'),
             TestConfiguration('lucid', 'x86', 'release'),
         ])
-        self.assertEquals(converter.to_specifiers_list(configs_to_match), [set(['release', 'lucid', 'x86']), set(['win7']), set(['release', 'xp', 'snowleopard'])])
+        self.assertEquals(converter.to_specifiers_list(configs_to_match), [set(['win7']), set(['release', 'linux', 'x86']), set(['release', 'xp', 'mac'])])
 
     def test_macro_collapsing(self):
         macros = {'foo': ['bar', 'baz'], 'people': ['bob', 'alice', 'john']}
@@ -321,12 +330,14 @@
 
         configs_to_match = set([
             TestConfiguration('xp', 'x86', 'release'),
+            TestConfiguration('vista', 'x86', 'release'),
             TestConfiguration('win7', 'x86', 'release'),
         ])
         self.assertEquals(converter.to_specifiers_list(configs_to_match), [set(['win', 'release'])])
 
         configs_to_match = set([
             TestConfiguration('xp', 'x86', 'release'),
+            TestConfiguration('vista', 'x86', 'release'),
             TestConfiguration('win7', 'x86', 'release'),
             TestConfiguration('lucid', 'x86', 'release'),
             TestConfiguration('lucid', 'x86_64', 'release'),
@@ -335,11 +346,27 @@
 
         configs_to_match = set([
             TestConfiguration('xp', 'x86', 'release'),
+            TestConfiguration('vista', 'x86', 'release'),
             TestConfiguration('win7', 'x86', 'release'),
             TestConfiguration('snowleopard', 'x86', 'release'),
         ])
         self.assertEquals(converter.to_specifiers_list(configs_to_match), [set(['win', 'mac', 'release'])])
 
+        configs_to_match = set([
+            TestConfiguration('xp', 'x86', 'release'),
+            TestConfiguration('vista', 'x86', 'release'),
+            TestConfiguration('win7', 'x86', 'release'),
+            TestConfiguration('snowleopard', 'x86', 'release'),
+        ])
+        self.assertEquals(converter.to_specifiers_list(configs_to_match), [set(['win', 'mac', 'release'])])
+
+        configs_to_match = set([
+            TestConfiguration('xp', 'x86', 'release'),
+            TestConfiguration('vista', 'x86', 'release'),
+            TestConfiguration('win7', 'x86', 'release'),
+        ])
+        self.assertEquals(converter.to_specifiers_list(configs_to_match), [set(['win', 'release'])])
+
     def test_specifier_converter_access(self):
         specifier_sorter = TestConfigurationConverter(self._all_test_configurations, MOCK_MACROS).specifier_sorter()
         self.assertEquals(specifier_sorter.category_for_specifier('snowleopard'), 'version')

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium.py (111033 => 111034)


--- trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium.py	2012-03-16 18:27:21 UTC (rev 111033)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium.py	2012-03-16 18:31:33 UTC (rev 111034)
@@ -67,7 +67,9 @@
         ('win7', 'x86'),
         ('lucid', 'x86'),
         ('lucid', 'x86_64'),
-        ('icecreamsandwich', 'arm'))
+        # FIXME: Technically this should be 'arm', but adding a third architecture type breaks TestConfigurationConverter.
+        # If we need this to be 'arm' in the future, then we first have to fix TestConfigurationConverter.
+        ('icecreamsandwich', 'x86'))
 
     ALL_BASELINE_VARIANTS = [
         'chromium-mac-lion', 'chromium-mac-snowleopard', 'chromium-mac-leopard',

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py (111033 => 111034)


--- trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py	2012-03-16 18:27:21 UTC (rev 111033)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py	2012-03-16 18:31:33 UTC (rev 111034)
@@ -143,8 +143,6 @@
 
         self._operating_system = 'android'
         self._version = 'icecreamsandwich'
-        # FIXME: we may support other architectures in the future.
-        self._architecture = 'arm'
         self._original_governor = None
         self._android_base_dir = None

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py (111033 => 111034)


--- trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py	2012-03-16 18:27:21 UTC (rev 111033)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py	2012-03-16 18:31:33 UTC (rev 111034)
@@ -162,8 +162,8 @@
         """Validate the complete set of configurations this port knows about."""
         port = self.make_port()
         self.assertEquals(set(port.all_test_configurations()), set([
-            TestConfiguration('icecreamsandwich', 'arm', 'debug'),
-            TestConfiguration('icecreamsandwich', 'arm', 'release'),
+            TestConfiguration('icecreamsandwich', 'x86', 'debug'),
+            TestConfiguration('icecreamsandwich', 'x86', 'release'),
             TestConfiguration('leopard', 'x86', 'debug'),
             TestConfiguration('leopard', 'x86', 'release'),
             TestConfiguration('snowleopard', 'x86', 'debug'),

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to