Changeset 268811 in webkit

Timestamp:

Oct 21, 2020, 11:33:17 AM (6 years ago)

Author:

Lauro Moura

Message:

webkitpy: Check for duplicated keys in json expectation files
​https://bugs.webkit.org/show_bug.cgi?id=218032

Reviewed by Carlos Alberto Lopez Perez.

Tools:

When JSON contains repeated keys, Python only uses the last key/value
pair in the resulting dict, without warnings. This may cause some
expectations to be ignored, like what r267323 fixed.

This change also makes MockTestExpectations use the same loading
validation as the actual expectation instead of calling json.loads
directly.

• Scripts/webkitpy/common/test_expectations.py:

(check_repeated_keys): Added.
(TestExpectations.init): Use helper function.
(TestExpectations._load_expectation_string): Moved validation here.

• Scripts/webkitpy/common/test_expectations_unittest.py:

(MockTestExpectations.init): Use helper function.
(test_repeated_keys): Added test when there are repeated keys.

WebDriverTests:

• TestExpectations.json: Fix duplicated test key.

Location:

trunk

Files:

• Tools/ChangeLog (modified) (1 diff)
• Tools/Scripts/webkitpy/common/test_expectations.py (modified) (2 diffs)
• Tools/Scripts/webkitpy/common/test_expectations_unittest.py (modified) (3 diffs)
• WebDriverTests/ChangeLog (modified) (1 diff)
• WebDriverTests/TestExpectations.json (modified) (2 diffs)

trunk/Tools/ChangeLog

@@ +1 @@
+2020-10-21  Lauro Moura  <lmoura@igalia.com>
+
+        webkitpy: Check for duplicated keys in json expectation files
+        https://bugs.webkit.org/show_bug.cgi?id=218032
+
+        Reviewed by Carlos Alberto Lopez Perez.
+
+        When JSON contains repeated keys, Python only uses the last key/value
+        pair in the resulting dict, without warnings. This may cause some
+        expectations to be ignored, like what r267323 fixed.
+
+        This change also makes MockTestExpectations use the same loading
+        validation as the actual expectation instead of calling json.loads
+        directly.
+
+        * Scripts/webkitpy/common/test_expectations.py:
+        (check_repeated_keys): Added.
+        (TestExpectations.__init__): Use helper function.
+        (TestExpectations._load_expectation_string): Moved validation here.
+        * Scripts/webkitpy/common/test_expectations_unittest.py:
+        (MockTestExpectations.__init__): Use helper function.
+        (test_repeated_keys): Added test when there are repeated keys.
+
 2020-10-21  Alex Christensen  <achristensen@webkit.org>
 

trunk/Tools/Scripts/webkitpy/common/test_expectations.py

@@ -25 +25 @@
 
 
+def check_repeated_keys(args):
+    seen = {}
+    for key, val in args:
+        if key in seen:
+            raise ValueError("Key %s appears more than once" % key)
+        else:
+            seen[key] = val
+
+    return seen
+
+
 class TestExpectations(object):
 
@@ -32 +43 @@
         if os.path.isfile(expectations_file):
             with open(expectations_file, 'r') as fd:
-                self._expectations = json.load(fd)
+                self._expectations = self._load_expectation_string(fd.read())
         else:
             self._expectations = {}
+
+    def _load_expectation_string(self, expectations):
+        return json.loads(expectations, object_pairs_hook=check_repeated_keys)
 
     def _port_name_for_expected(self, expected):

trunk/Tools/Scripts/webkitpy/common/test_expectations_unittest.py

@@ -38 +38 @@
         self._port_name = port.name()
         self._build_type = build_type
-        self._expectations = json.loads(expectations)
+        self._expectations = self._load_expectation_string(expectations)
 
     def is_skip(self, test, subtest):
@@ -183 +183 @@
             }
         }
+    }
+}"""
+
+    REPEATED_KEYS = """
+{
+    "TestDummy": {
+        "a": 1
+    },
+    "TestAnother": {
+    },
+    "TestDummy": {
+        "a": 2
     }
 }"""
@@ -294 +306 @@
         self.assert_slow('TestWebKit', 'WebKit.MouseMoveAfterCrash', True)
         self.assert_slow('TestWebKit', 'WebKit.WKConnection', False)
+
+    def test_repeated_keys(self):
+        self.assertRaises(ValueError, lambda: MockTestExpectations('gtk', self.REPEATED_KEYS))

trunk/WebDriverTests/ChangeLog

@@ +1 @@
+2020-10-21  Lauro Moura  <lmoura@igalia.com>
+
+        webkitpy: Check for duplicated keys in json expectation files
+        https://bugs.webkit.org/show_bug.cgi?id=218032
+
+        Reviewed by Carlos Alberto Lopez Perez.
+
+        * TestExpectations.json: Fix duplicated test key.
+
 2020-10-21  Carlos Garcia Campos  <cgarcia@igalia.com>
 

trunk/WebDriverTests/TestExpectations.json

@@ -515 +515 @@
             "test_duplicated_cookie": {
                 "expected": {"all": {"status": ["FAIL"], "bug": "webkit.org/b/182330"}}
+            },
+            "test_no_top_browsing_context": {
+                "expected": {"wpe": {"status": ["FAIL"], "bug": "webkit.org/b/212950"}}
+            },
+            "test_no_browsing_context": {
+                "expected": {"wpe": {"status": ["FAIL"], "bug": "webkit.org/b/212950"}}
             }
         }
@@ -924 +930 @@
 
     "imported/w3c/webdriver/tests/get_element_text/get.py": {
-        "subtests": {
-            "test_no_top_browsing_context": {
-                "expected": {"wpe": {"status": ["FAIL"], "bug": "webkit.org/b/212950"}}
-            },
-            "test_no_browsing_context": {
-                "expected": {"wpe": {"status": ["FAIL"], "bug": "webkit.org/b/212950"}}
-            }
-        }
-    },
-
-    "imported/w3c/webdriver/tests/get_named_cookie/get.py": {
         "subtests": {
             "test_no_top_browsing_context": {