Bug 1401613 - Don't require test type in wpt expectation manifest, r=maja_zf
☠☠ backed out by 5b815c394dcb ☠ ☠
authorJames Graham <james@hoppipolla.co.uk>
Wed, 20 Sep 2017 15:50:06 +0100
changeset 382343 9192c6646cec6809c0afcbed592457b182471814
parent 382342 abead884f8fbb18c60c035b5326b1445b8b8508b
child 382344 55541ed45819e54e8a6c08c4525e263c80bd35ab
push id32555
push userarchaeopteryx@coole-files.de
push dateFri, 22 Sep 2017 09:43:20 +0000
treeherdermozilla-central@82b2ae0b03ca [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1401613 - Don't require test type in wpt expectation manifest, r=maja_zf The test type is already in the MANIFEST.json file and requiring it in the expectation data doesn't make much sense. It isn't used execpt during updates and so people often forget to add it. Therefore it makes a lot of sense to just use the data from the main manifest. MozReview-Commit-ID: HyOoN6T28qc
--- a/testing/web-platform/tests/tools/wptrunner/README.rst
+++ b/testing/web-platform/tests/tools/wptrunner/README.rst
@@ -141,29 +141,26 @@ example a test file `html/test1.html` co
 have an expectation file called `html/test1.html.ini` in the
 `metadata` directory.
 An example of an expectation file is::
   example_default_key: example_value
-    type: testharness
       expected: FAIL
         if platform == 'win': TIMEOUT
         if platform == 'osx': ERROR
-    type: testharness
     disabled: bug12345
 The file consists of two elements, key-value pairs and
 Sections are delimited by headings enclosed in square brackets. Any
 closing square bracket in the heading itself my be escaped with a
 backslash. Each section may then contain any number of key-value pairs
@@ -225,18 +222,15 @@ The web-platform-test harness knows abou
   Must evaluate to a possible test status indicating the expected
   result of the test. The implicit default is PASS or OK when the
   field isn't present.
   Any value indicates that the test is disabled.
-  The test type e.g. `testharness`, `reftest`, or `wdspec`.
   The type of comparison for reftests; either `==` or `!=`.
   The reference url for reftests.
 .. _`web-platform-tests testsuite`: https://github.com/w3c/web-platform-tests
--- a/testing/web-platform/tests/tools/wptrunner/wptrunner/manifestupdate.py
+++ b/testing/web-platform/tests/tools/wptrunner/wptrunner/manifestupdate.py
@@ -111,35 +111,34 @@ class TestNode(ManifestItem):
         ManifestItem.__init__(self, node)
         self.updated_expected = []
         self.new_expected = []
         self.subtests = {}
         self.default_status = None
         self._from_file = True
-    def create(cls, test_type, test_id):
+    def create(cls, test_id):
         """Create a TestNode corresponding to a given test
         :param test_type: The type of the test
         :param test_id: The id of the test"""
         url = test_id
         name = url.split("/")[-1]
         node = DataNode(name)
         self = cls(node)
-        self.set("type", test_type)
         self._from_file = False
         return self
     def is_empty(self):
-        required_keys = set(["type"])
-        if set(self._data.keys()) != required_keys:
+        ignore_keys = set(["type"])
+        if set(self._data.keys()) - ignore_keys:
             return False
         return all(child.is_empty for child in self.children)
     def test_type(self):
         """The type of the test represented by this TestNode"""
         return self.get("type", None)
--- a/testing/web-platform/tests/tools/wptrunner/wptrunner/metadata.py
+++ b/testing/web-platform/tests/tools/wptrunner/wptrunner/metadata.py
@@ -226,61 +226,62 @@ class ExpectedUpdater(object):
     def update_from_log(self, log_file):
         self.run_info = None
         log_reader = reader.read(log_file)
         reader.each_log(log_reader, self.action_map)
     def suite_start(self, data):
         self.run_info = data["run_info"]
-    def test_id(self, id):
-        if type(id) in types.StringTypes:
-            return id
-        else:
-            return tuple(id)
+    def test_type(self, path):
+        for manifest in self.test_manifests.iterkeys():
+            tests = list(manifest.iterpath(path))
+            if len(tests):
+                assert all(test.item_type == tests[0].item_type for test in tests)
+                return tests[0].item_type
+        assert False
     def test_start(self, data):
-        test_id = self.test_id(data["test"])
+        test_id = data["test"]
             test_manifest, test = self.id_path_map[test_id]
             expected_node = self.expected_tree[test_manifest][test].get_test(test_id)
         except KeyError:
             print "Test not found %s, skipping" % test_id
         self.test_cache[test_id] = expected_node
         if test_id not in self.tests_visited:
             if self.ignore_existing:
             self.tests_visited[test_id] = set()
     def test_status(self, data):
-        test_id = self.test_id(data["test"])
-        test = self.test_cache.get(test_id)
+        test = self.test_cache.get(data["test"])
         if test is None:
-        test_cls = wpttest.manifest_test_cls[test.test_type]
+        test_cls = wpttest.manifest_test_cls[self.test_type(test.root.test_path)]
         subtest = test.get_subtest(data["subtest"])
         result = test_cls.subtest_result_cls(
         subtest.set_result(self.run_info, result)
     def test_end(self, data):
-        test_id = self.test_id(data["test"])
+        test_id = data["test"]
         test = self.test_cache.get(test_id)
         if test is None:
-        test_cls = wpttest.manifest_test_cls[test.test_type]
+        test_cls = wpttest.manifest_test_cls[self.test_type(test.root.test_path)]
         if data["status"] == "SKIP":
         result = test_cls.result_cls(
@@ -317,17 +318,17 @@ def create_test_tree(metadata_path, test
 def create_expected(test_manifest, test_path, tests, property_order=None,
     expected = manifestupdate.ExpectedManifest(None, test_path, test_manifest.url_base,
     for test in tests:
-        expected.append(manifestupdate.TestNode.create(test.item_type, test.id))
+        expected.append(manifestupdate.TestNode.create(test.id))
     return expected
 def load_expected(test_manifest, metadata_path, test_path, tests, property_order=None,
     expected_manifest = manifestupdate.get_manifest(metadata_path,
@@ -341,11 +342,11 @@ def load_expected(test_manifest, metadat
     # Remove expected data for tests that no longer exist
     for test in expected_manifest.iterchildren():
         if not test.id in tests_by_id:
     # Add tests that don't have expected data
     for test in tests:
         if not expected_manifest.has_test(test.id):
-            expected_manifest.append(manifestupdate.TestNode.create(test.item_type, test.id))
+            expected_manifest.append(manifestupdate.TestNode.create(test.id))
     return expected_manifest
--- a/testing/web-platform/tests/tools/wptrunner/wptrunner/wptmanifest/serializer.py
+++ b/testing/web-platform/tests/tools/wptrunner/wptrunner/wptmanifest/serializer.py
@@ -23,16 +23,18 @@ def escape(string, extras=""):
 class ManifestSerializer(NodeVisitor):
     def __init__(self, skip_empty_data=False):
         self.skip_empty_data = skip_empty_data
     def serialize(self, root):
         self.indent = 2
         rv = "\n".join(self.visit(root))
+        if not rv:
+            return rv
         if rv[-1] != "\n":
             rv = rv + "\n"
         return rv
     def visit_DataNode(self, node):
         rv = []
         if not self.skip_empty_data or node.children:
             if node.data: