import-w3c-tests doesn't prefix property values
authorrego@igalia.com <rego@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Oct 2014 08:08:05 +0000 (08:08 +0000)
committerrego@igalia.com <rego@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Oct 2014 08:08:05 +0000 (08:08 +0000)
https://bugs.webkit.org/show_bug.cgi?id=137600

Reviewed by Bem Jones-Bey.

Some property values are prefixed in WebKit. Modified the W3C import
script in order to prefix property values and not only properties.
The patch re-uses most of the already existent logic to prefix
properties.

* Scripts/webkitpy/w3c/test_converter.py: Read prefixed property values
from CSSValueKeywords.in and adapt converter to modify both properties
and property values.
(convert_for_webkit):
(_W3CTestConverter.__init__):
(_W3CTestConverter.output):
(_W3CTestConverter.read_webkit_prefixed_css_property_list):
(_W3CTestConverter):
(_W3CTestConverter.add_webkit_prefix_to_unprefixed_properties_and_values):
(_W3CTestConverter.add_webkit_prefix_following_regex):
(_W3CTestConverter.convert_reference_relpaths):
(_W3CTestConverter.convert_style_data):
(_W3CTestConverter.convert_attributes_if_needed):
(_W3CTestConverter.add_webkit_prefix_to_unprefixed_properties): Deleted.
* Scripts/webkitpy/w3c/test_converter_unittest.py: Updated unit test to
include checks for property values too.
(W3CTestConverterTest.test_read_prefixed_property_list):
(verify_no_conversion_happened):
(verify_prefixed_properties):
(verify_prefixed_property_values):
(generate_test_content_properties_and_values):
(generate_test_content):
* Scripts/webkitpy/w3c/test_importer.py: Modified importer to manage
prefixed property values and inform about them.
(TestImporter.import_tests):
(TestImporter.write_import_log):

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

Tools/ChangeLog
Tools/Scripts/webkitpy/w3c/test_converter.py
Tools/Scripts/webkitpy/w3c/test_converter_unittest.py
Tools/Scripts/webkitpy/w3c/test_importer.py

index b45f59e..c74309e 100644 (file)
@@ -1,3 +1,42 @@
+2014-10-14  Manuel Rego Casasnovas  <rego@igalia.com>
+
+        import-w3c-tests doesn't prefix property values
+        https://bugs.webkit.org/show_bug.cgi?id=137600
+
+        Reviewed by Bem Jones-Bey.
+
+        Some property values are prefixed in WebKit. Modified the W3C import
+        script in order to prefix property values and not only properties.
+        The patch re-uses most of the already existent logic to prefix
+        properties.
+
+        * Scripts/webkitpy/w3c/test_converter.py: Read prefixed property values
+        from CSSValueKeywords.in and adapt converter to modify both properties
+        and property values.
+        (convert_for_webkit):
+        (_W3CTestConverter.__init__):
+        (_W3CTestConverter.output):
+        (_W3CTestConverter.read_webkit_prefixed_css_property_list):
+        (_W3CTestConverter):
+        (_W3CTestConverter.add_webkit_prefix_to_unprefixed_properties_and_values):
+        (_W3CTestConverter.add_webkit_prefix_following_regex):
+        (_W3CTestConverter.convert_reference_relpaths):
+        (_W3CTestConverter.convert_style_data):
+        (_W3CTestConverter.convert_attributes_if_needed):
+        (_W3CTestConverter.add_webkit_prefix_to_unprefixed_properties): Deleted.
+        * Scripts/webkitpy/w3c/test_converter_unittest.py: Updated unit test to
+        include checks for property values too.
+        (W3CTestConverterTest.test_read_prefixed_property_list):
+        (verify_no_conversion_happened):
+        (verify_prefixed_properties):
+        (verify_prefixed_property_values):
+        (generate_test_content_properties_and_values):
+        (generate_test_content):
+        * Scripts/webkitpy/w3c/test_importer.py: Modified importer to manage
+        prefixed property values and inform about them.
+        (TestImporter.import_tests):
+        (TestImporter.write_import_log):
+
 2014-10-13  Jer Noble  <jer.noble@apple.com>
 
         MediaPlayer::characteristicChanged() is not called when new tracks are found in SourceBufferPrivateAVFObjC
index 5ccbd6f..d463596 100644 (file)
@@ -44,7 +44,7 @@ def convert_for_webkit(new_path, filename, reference_support_info, host=Host()):
     contents = host.filesystem.read_binary_file(filename)
     converter = _W3CTestConverter(new_path, filename, reference_support_info, host)
     if filename.endswith('.css'):
-        return converter.add_webkit_prefix_to_unprefixed_properties(contents)
+        return converter.add_webkit_prefix_to_unprefixed_properties_and_values(contents)
     else:
         converter.feed(contents)
         converter.close()
@@ -61,6 +61,7 @@ class _W3CTestConverter(HTMLParser):
 
         self.converted_data = []
         self.converted_properties = []
+        self.converted_property_values = []
         self.in_style_tag = False
         self.style_data = []
         self.filename = filename
@@ -72,25 +73,30 @@ class _W3CTestConverter(HTMLParser):
 
         # These settings might vary between WebKit and Blink
         self._css_property_file = self.path_from_webkit_root('Source', 'WebCore', 'css', 'CSSPropertyNames.in')
+        self._css_property_value_file = self.path_from_webkit_root('Source', 'WebCore', 'css', 'CSSValueKeywords.in')
         self._css_property_split_string = '='
 
         self.test_harness_re = re.compile('/resources/testharness')
 
-        self.prefixed_properties = self.read_webkit_prefixed_css_property_list()
+        self.prefixed_properties = self.read_webkit_prefixed_css_property_list(self._css_property_file)
         prop_regex = '([\s{]|^)(' + "|".join(prop.replace('-webkit-', '') for prop in self.prefixed_properties) + ')(\s+:|:)'
         self.prop_re = re.compile(prop_regex)
 
+        self.prefixed_property_values = self.read_webkit_prefixed_css_property_list(self._css_property_value_file)
+        prop_value_regex = '(:\s*|^\s*)(' + "|".join(value.replace('-webkit-', '') for value in self.prefixed_property_values) + ')(\s*;|\s*}|\s*$)'
+        self.prop_value_re = re.compile(prop_value_regex)
+
     def output(self):
-        return (self.converted_properties, ''.join(self.converted_data))
+        return (self.converted_properties, self.converted_property_values, ''.join(self.converted_data))
 
     def path_from_webkit_root(self, *comps):
         return self._filesystem.abspath(self._filesystem.join(self._webkit_root, *comps))
 
-    def read_webkit_prefixed_css_property_list(self):
+    def read_webkit_prefixed_css_property_list(self, file_name):
+        contents = self._filesystem.read_text_file(file_name)
         prefixed_properties = []
         unprefixed_properties = set()
 
-        contents = self._filesystem.read_text_file(self._css_property_file)
         for line in contents.splitlines():
             if re.match('^(#|//)', line):
                 # skip comments and preprocessor directives
@@ -107,25 +113,31 @@ class _W3CTestConverter(HTMLParser):
         # Ignore any prefixed properties for which an unprefixed version is supported
         return [prop for prop in prefixed_properties if prop not in unprefixed_properties]
 
-    def add_webkit_prefix_to_unprefixed_properties(self, text):
-        """ Searches |text| for instances of properties requiring the -webkit- prefix and adds the prefix to them.
+    def add_webkit_prefix_to_unprefixed_properties_and_values(self, text):
+        """ Searches |text| for instances of properties and values requiring the -webkit- prefix and adds the prefix to them.
+
+        Returns the list of converted properties, values and the modified text."""
+
+        converted_properties = self.add_webkit_prefix_following_regex(text, self.prop_re)
+        converted_property_values = self.add_webkit_prefix_following_regex(converted_properties[1], self.prop_value_re)
 
-        Returns the list of converted properties and the modified text."""
+        # FIXME: Handle the JS versions of these properties and values and GetComputedStyle, too.
+        return (converted_properties[0], converted_property_values[0], converted_property_values[1])
 
-        converted_properties = set()
+    def add_webkit_prefix_following_regex(self, text, regex):
+        converted_list = set()
         text_chunks = []
         cur_pos = 0
-        for m in self.prop_re.finditer(text):
+        for m in regex.finditer(text):
             text_chunks.extend([text[cur_pos:m.start()], m.group(1), '-webkit-', m.group(2), m.group(3)])
-            converted_properties.add(m.group(2))
+            converted_list.add(m.group(2))
             cur_pos = m.end()
         text_chunks.append(text[cur_pos:])
 
-        for prop in converted_properties:
-            _log.info('  converting %s', prop)
+        for item in converted_list:
+            _log.info('  converting %s', item)
 
-        # FIXME: Handle the JS versions of these properties and GetComputedStyle, too.
-        return (converted_properties, ''.join(text_chunks))
+        return (converted_list, ''.join(text_chunks))
 
     def convert_reference_relpaths(self, text):
         """ Searches |text| for instances of files in reference_support_info and updates the relative path to be correct for the new ref file location"""
@@ -140,14 +152,16 @@ class _W3CTestConverter(HTMLParser):
         return converted
 
     def convert_style_data(self, data):
-        converted = self.add_webkit_prefix_to_unprefixed_properties(data)
+        converted = self.add_webkit_prefix_to_unprefixed_properties_and_values(data)
         if converted[0]:
             self.converted_properties.extend(list(converted[0]))
+        if converted[1]:
+            self.converted_property_values.extend(list(converted[1]))
 
         if self.reference_support_info is None or self.reference_support_info == {}:
-            return converted[1]
+            return converted[2]
 
-        return self.convert_reference_relpaths(converted[1])
+        return self.convert_reference_relpaths(converted[2])
 
     def convert_attributes_if_needed(self, tag, attrs):
         converted = self.get_starttag_text()
index 896bb6f..173bdd6 100644 (file)
@@ -54,6 +54,8 @@ class W3CTestConverterTest(unittest.TestCase):
         converter = _W3CTestConverter(DUMMY_PATH, DUMMY_FILENAME, None)
         prop_list = converter.prefixed_properties
         self.assertTrue(prop_list, 'No prefixed properties found')
+        property_values_list = converter.prefixed_property_values
+        self.assertTrue(property_values_list, 'No prefixed property values found')
 
     def test_convert_for_webkit_nothing_to_convert(self):
         """ Tests convert_for_webkit() using a basic test that has nothing to convert """
@@ -104,8 +106,9 @@ CONTENT OF TEST
         converted = converter.output()
 
         self.verify_conversion_happened(converted)
-        self.verify_test_harness_paths(converter, converted[1], fake_dir_path, 1, 1)
+        self.verify_test_harness_paths(converter, converted[2], fake_dir_path, 1, 1)
         self.verify_prefixed_properties(converted, [])
+        self.verify_prefixed_property_values(converted, [])
 
     def test_convert_for_webkit_properties_only(self):
         """ Tests convert_for_webkit() using a test that has 2 prefixed properties: 1 in a style block + 1 inline style """
@@ -116,31 +119,32 @@ CONTENT OF TEST
 <script src="/resources/testharness.js"></script>
 <style type="text/css">
 
-#block1 { @test0@: propvalue; }
+#block1 { @test0@: @propvalue0@; }
 
 </style>
 </head>
 <body>
-<div id="elem1" style="@test1@: propvalue;"></div>
+<div id="elem1" style="@test1@: @propvalue1@;"></div>
 </body>
 </html>
 """
         fake_dir_path = self.fake_dir_path('harnessandprops')
         converter = _W3CTestConverter(fake_dir_path, DUMMY_FILENAME, None)
-        test_content = self.generate_test_content(converter.prefixed_properties, 1, test_html)
+        test_content = self.generate_test_content_properties_and_values(converter.prefixed_properties, converter.prefixed_property_values, 1, test_html)
 
         oc = OutputCapture()
         oc.capture_output()
         try:
-            converter.feed(test_content[1])
+            converter.feed(test_content[2])
             converter.close()
             converted = converter.output()
         finally:
             oc.restore_output()
 
         self.verify_conversion_happened(converted)
-        self.verify_test_harness_paths(converter, converted[1], fake_dir_path, 1, 1)
+        self.verify_test_harness_paths(converter, converted[2], fake_dir_path, 1, 1)
         self.verify_prefixed_properties(converted, test_content[0])
+        self.verify_prefixed_property_values(converted, test_content[1])
 
     def test_convert_for_webkit_harness_and_properties(self):
         """ Tests convert_for_webkit() using a basic JS test that uses testharness.js and testharness.css and has 4 prefixed properties: 3 in a style block + 1 inline style """
@@ -151,14 +155,14 @@ CONTENT OF TEST
 <script src="/resources/testharness.js"></script>
 <style type="text/css">
 
-#block1 { @test0@: propvalue; }
-#block2 { @test1@: propvalue; }
-#block3 { @test2@: propvalue; }
+#block1 { @test0@: @propvalue0@; }
+#block2 { @test1@: @propvalue1@; }
+#block3 { @test2@: @propvalue2@; }
 
 </style>
 </head>
 <body>
-<div id="elem1" style="@test3@: propvalue;"></div>
+<div id="elem1" style="@test3@: @propvalue3@;"></div>
 </body>
 </html>
 """
@@ -168,16 +172,17 @@ CONTENT OF TEST
         oc = OutputCapture()
         oc.capture_output()
         try:
-            test_content = self.generate_test_content(converter.prefixed_properties, 2, test_html)
-            converter.feed(test_content[1])
+            test_content = self.generate_test_content_properties_and_values(converter.prefixed_properties, converter.prefixed_property_values, 2, test_html)
+            converter.feed(test_content[2])
             converter.close()
             converted = converter.output()
         finally:
             oc.restore_output()
 
         self.verify_conversion_happened(converted)
-        self.verify_test_harness_paths(converter, converted[1], fake_dir_path, 1, 1)
+        self.verify_test_harness_paths(converter, converted[2], fake_dir_path, 1, 1)
         self.verify_prefixed_properties(converted, test_content[0])
+        self.verify_prefixed_property_values(converted, test_content[1])
 
     def test_convert_test_harness_paths(self):
         """ Tests convert_testharness_paths() with a test that uses all three testharness files """
@@ -201,7 +206,7 @@ CONTENT OF TEST
             oc.restore_output()
 
         self.verify_conversion_happened(converted)
-        self.verify_test_harness_paths(converter, converted[1], fake_dir_path, 2, 1)
+        self.verify_test_harness_paths(converter, converted[2], fake_dir_path, 2, 1)
 
     def test_convert_prefixed_properties(self):
         """ Tests convert_prefixed_properties() file that has 20 properties requiring the -webkit- prefix:
@@ -219,60 +224,62 @@ CONTENT OF TEST
 }
 
 .block2 {
-    @test0@: propvalue;
+    @test0@: @propvalue0@;
 }
 
-.block3{@test1@: propvalue;}
+.block3{@test1@: @propvalue1@;}
 
-.block4 { @test2@:propvalue; }
+.block4 { @test2@:@propvalue2@; }
 
-.block5{ @test3@ :propvalue; }
+.block5{ @test3@ :@propvalue3@; }
 
-#block6 {    @test4@   :   propvalue;  }
+#block6 {    @test4@   :   @propvalue4@   ;  }
 
 #block7
 {
-    @test5@: propvalue;
+    @test5@: @propvalue5@;
 }
 
-#block8 { @test6@: propvalue; }
+#block8 { @test6@: @propvalue6@ }
 
 #block9:pseudo
 {
 
-    @test7@: propvalue;
+    @test7@: @propvalue7@;
     @test8@:  propvalue propvalue propvalue;;
+    propname:
+@propvalue8@;
 }
 
 ]]></style>
 </head>
 <body>
-    <div id="elem1" style="@test9@: propvalue;"></div>
-    <div id="elem2" style="propname: propvalue; @test10@ : propvalue; propname:propvalue;"></div>
-    <div id="elem2" style="@test11@: propvalue; @test12@ : propvalue; @test13@   :propvalue;"></div>
-    <div id="elem3" style="@test14@:propvalue"></div>
+    <div id="elem1" style="@test9@: @propvalue9@;"></div>
+    <div id="elem2" style="propname: propvalue; @test10@ : @propvalue10@; propname:propvalue;"></div>
+    <div id="elem2" style="@test11@: @propvalue11@; @test12@ : @propvalue12@; @test13@   :   @propvalue13@   ;"></div>
+    <div id="elem3" style="@test14@:@propvalue14@"></div>
 </body>
 <style type="text/css"><![CDATA[
 
-.block10{ @test15@: propvalue; }
-.block11{ @test16@: propvalue; }
-.block12{ @test17@: propvalue; }
+.block10{ @test15@: @propvalue15@; }
+.block11{ @test16@: @propvalue16@; }
+.block12{ @test17@: @propvalue17@; }
 #block13:pseudo
 {
-    @test18@: propvalue;
-    @test19@: propvalue;
+    @test18@: @propvalue18@;
+    @test19@: @propvalue19@;
 }
 
 ]]></style>
 </html>
 """
         converter = _W3CTestConverter(DUMMY_PATH, DUMMY_FILENAME, None)
-        test_content = self.generate_test_content(converter.prefixed_properties, 20, test_html)
+        test_content = self.generate_test_content_properties_and_values(converter.prefixed_properties, converter.prefixed_property_values, 20, test_html)
 
         oc = OutputCapture()
         oc.capture_output()
         try:
-            converter.feed(test_content[1])
+            converter.feed(test_content[2])
             converter.close()
             converted = converter.output()
         finally:
@@ -280,12 +287,13 @@ CONTENT OF TEST
 
         self.verify_conversion_happened(converted)
         self.verify_prefixed_properties(converted, test_content[0])
+        self.verify_prefixed_property_values(converted, test_content[1])
 
     def verify_conversion_happened(self, converted):
         self.assertTrue(converted, "conversion didn't happen")
 
     def verify_no_conversion_happened(self, converted, original):
-        self.assertEqual(converted[1], original, 'test should not have been converted')
+        self.assertEqual(converted[2], original, 'test should not have been converted')
 
     def verify_test_harness_paths(self, converter, converted, test_path, num_src_paths, num_href_paths):
         if isinstance(converted, basestring):
@@ -306,24 +314,34 @@ CONTENT OF TEST
     def verify_prefixed_properties(self, converted, test_properties):
         self.assertEqual(len(set(converted[0])), len(set(test_properties)), 'Incorrect number of properties converted')
         for test_prop in test_properties:
-            self.assertTrue((test_prop in converted[1]), 'Property ' + test_prop + ' not found in converted doc')
+            self.assertTrue((test_prop in converted[2]), 'Property ' + test_prop + ' not found in converted doc')
 
-    def generate_test_content(self, full_property_list, num_test_properties, html):
-        """Inserts properties requiring a -webkit- prefix into the content, replacing \'@testXX@\' with a property."""
-        test_properties = []
+    def verify_prefixed_property_values(self, converted, test_property_values):
+        self.assertEqual(len(set(converted[1])), len(set(test_property_values)), 'Incorrect number of property values converted ' + str(len(set(converted[1]))) + ' vs ' + str(len(set(test_property_values))))
+        for test_value in test_property_values:
+            self.assertTrue((test_value in converted[2]), 'Property value ' + test_value + ' not found in converted doc')
+
+    def generate_test_content_properties_and_values(self, full_property_list, fully_property_values_list, num_test_properties_and_values, html):
+        """Inserts properties requiring a -webkit- prefix into the content, replacing \'@testXX@\' with a property and \'@propvalueXX@\' with a value."""
+        test_content_properties = self.generate_test_content(full_property_list, num_test_properties_and_values, 'test', html)
+        test_content_property_values = self.generate_test_content(fully_property_values_list, num_test_properties_and_values, 'propvalue', test_content_properties[1])
+        return (test_content_properties[0], test_content_property_values[0], test_content_property_values[1])
+
+    def generate_test_content(self, full_list, num_test, suffix, html):
+        test_list = []
         count = 0
-        while count < num_test_properties:
-            test_properties.append(full_property_list[count])
+        while count < num_test:
+            test_list.append(full_list[count])
             count += 1
 
-        # Replace the tokens in the testhtml with the test properties. Walk backward
-        # through the list to replace the double-digit tokens first
-        index = len(test_properties) - 1
+        # Replace the tokens in the testhtml with the test properties or values.
+        # Walk backward through the list to replace the double-digit tokens first.
+        index = len(test_list) - 1
         while index >= 0:
             # Use the unprefixed version
-            test_prop = test_properties[index].replace('-webkit-', '')
+            test = test_list[index].replace('-webkit-', '')
             # Replace the token
-            html = html.replace('@test' + str(index) + '@', test_prop)
+            html = html.replace('@' + suffix + str(index) + '@', test)
             index -= 1
 
-        return (test_properties, html)
+        return (test_list, html)
index 751e8b3..0017779 100644 (file)
@@ -239,6 +239,7 @@ class TestImporter(object):
         total_imported_reftests = 0
         total_imported_jstests = 0
         total_prefixed_properties = {}
+        total_prefixed_property_values = {}
 
         failed_conversion_files = []
 
@@ -248,6 +249,7 @@ class TestImporter(object):
             total_imported_jstests += dir_to_copy['jstests']
 
             prefixed_properties = []
+            prefixed_property_values = []
 
             if not dir_to_copy['copy_list']:
                 continue
@@ -312,8 +314,15 @@ class TestImporter(object):
                             total_prefixed_properties[prefixed_property] += 1
 
                         prefixed_properties.extend(set(converted_file[0]) - set(prefixed_properties))
+
+                        for prefixed_value in converted_file[1]:
+                            total_prefixed_property_values.setdefault(prefixed_value, 0)
+                            total_prefixed_property_values[prefixed_value] += 1
+
+                        prefixed_property_values.extend(set(converted_file[1]) - set(prefixed_property_values))
+
                         outfile = open(new_filepath, 'wb')
-                        outfile.write(converted_file[1])
+                        outfile.write(converted_file[2])
                         outfile.close()
                 else:
                     shutil.copyfile(orig_filepath, new_filepath)
@@ -321,7 +330,7 @@ class TestImporter(object):
                 copied_files.append(new_filepath.replace(self._webkit_root, ''))
 
             self.remove_deleted_files(new_path, copied_files)
-            self.write_import_log(new_path, copied_files, prefixed_properties)
+            self.write_import_log(new_path, copied_files, prefixed_properties, prefixed_property_values)
 
         _log.info('Import complete')
 
@@ -336,6 +345,11 @@ class TestImporter(object):
 
         for prefixed_property in sorted(total_prefixed_properties, key=lambda p: total_prefixed_properties[p]):
             _log.info('  %s: %s', prefixed_property, total_prefixed_properties[prefixed_property])
+        _log.info('')
+        _log.info('Property values needing prefixes (by count):')
+
+        for prefixed_value in sorted(total_prefixed_property_values, key=lambda p: total_prefixed_property_values[p]):
+            _log.info('  %s: %s', prefixed_value, total_prefixed_property_values[prefixed_value])
 
     def remove_deleted_files(self, import_directory, new_file_list):
         """ Reads an import log in |import_directory|, compares it to the |new_file_list|, and removes files not in the new list."""
@@ -361,7 +375,7 @@ class TestImporter(object):
 
         import_log.close()
 
-    def write_import_log(self, import_directory, file_list, prop_list):
+    def write_import_log(self, import_directory, file_list, prop_list, property_values_list):
         """ Writes a w3c-import.log file in each directory with imported files. """
 
         now = datetime.datetime.now()
@@ -384,6 +398,12 @@ class TestImporter(object):
                 import_log.write(prop + '\n')
         else:
             import_log.write('None\n')
+        import_log.write('Property values requiring vendor prefixes:\n')
+        if property_values_list:
+            for value in property_values_list:
+                import_log.write(value + '\n')
+        else:
+            import_log.write('None\n')
         import_log.write('------------------------------------------------------------------------\n')
         import_log.write('List of files:\n')
         for item in file_list: