- Revision
- 268811
- Author
- [email protected]
- Date
- 2020-10-21 11:33:17 -0700 (Wed, 21 Oct 2020)
Log 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.
Modified Paths
Diff
Modified: trunk/Tools/ChangeLog (268810 => 268811)
--- trunk/Tools/ChangeLog 2020-10-21 18:21:47 UTC (rev 268810)
+++ trunk/Tools/ChangeLog 2020-10-21 18:33:17 UTC (rev 268811)
@@ -1,3 +1,26 @@
+2020-10-21 Lauro Moura <[email protected]>
+
+ 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 <[email protected]>
Don't crash when deallocating WKWebView during TLS handshake
Modified: trunk/Tools/Scripts/webkitpy/common/test_expectations.py (268810 => 268811)
--- trunk/Tools/Scripts/webkitpy/common/test_expectations.py 2020-10-21 18:21:47 UTC (rev 268810)
+++ trunk/Tools/Scripts/webkitpy/common/test_expectations.py 2020-10-21 18:33:17 UTC (rev 268811)
@@ -24,6 +24,17 @@
import os
+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):
def __init__(self, port_name, expectations_file, build_type='Release'):
@@ -31,10 +42,13 @@
self._build_type = build_type
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):
if self._port_name in expected:
return self._port_name
Modified: trunk/Tools/Scripts/webkitpy/common/test_expectations_unittest.py (268810 => 268811)
--- trunk/Tools/Scripts/webkitpy/common/test_expectations_unittest.py 2020-10-21 18:21:47 UTC (rev 268810)
+++ trunk/Tools/Scripts/webkitpy/common/test_expectations_unittest.py 2020-10-21 18:33:17 UTC (rev 268811)
@@ -37,7 +37,7 @@
port = host.port_factory.get(port)
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):
if test in self.skipped_tests():
@@ -185,6 +185,18 @@
}
}"""
+ REPEATED_KEYS = """
+{
+ "TestDummy": {
+ "a": 1
+ },
+ "TestAnother": {
+ },
+ "TestDummy": {
+ "a": 2
+ }
+}"""
+
def assert_exp(self, test, subtest, result):
self.assertIn(result, self.expectations.get_expectation(test, subtest))
@@ -293,3 +305,6 @@
self.assert_slow('TestWebKit', 'WebKit.WKView', False)
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))
Modified: trunk/WebDriverTests/ChangeLog (268810 => 268811)
--- trunk/WebDriverTests/ChangeLog 2020-10-21 18:21:47 UTC (rev 268810)
+++ trunk/WebDriverTests/ChangeLog 2020-10-21 18:33:17 UTC (rev 268811)
@@ -1,3 +1,12 @@
+2020-10-21 Lauro Moura <[email protected]>
+
+ 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 <[email protected]>
WebDriver: add support for wheel actions
Modified: trunk/WebDriverTests/TestExpectations.json (268810 => 268811)
--- trunk/WebDriverTests/TestExpectations.json 2020-10-21 18:21:47 UTC (rev 268810)
+++ trunk/WebDriverTests/TestExpectations.json 2020-10-21 18:33:17 UTC (rev 268811)
@@ -514,6 +514,12 @@
"subtests": {
"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"}}
}
}
},
@@ -933,17 +939,6 @@
}
},
- "imported/w3c/webdriver/tests/get_named_cookie/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_page_source/source.py": {
"subtests": {
"test_no_top_browsing_context": {
