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)
reviewersmaja_zf
bugs1401613
milestone58.0a1
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
testing/web-platform/tests/tools/wptrunner/README.rst
testing/web-platform/tests/tools/wptrunner/wptrunner/manifestupdate.py
testing/web-platform/tests/tools/wptrunner/wptrunner/metadata.py
testing/web-platform/tests/tools/wptrunner/wptrunner/wptmanifest/serializer.py
--- 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
 
   [filename.html]
-    type: testharness
-
     [subtest1]
       expected: FAIL
 
     [subtest2]
       expected:
         if platform == 'win': TIMEOUT
         if platform == 'osx': ERROR
         FAIL
 
   [filename.html?query=something]
-    type: testharness
     disabled: bug12345
 
 The file consists of two elements, key-value pairs and
 sections.
 
 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
 `expected`
   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.
 
 `disabled`
   Any value indicates that the test is disabled.
 
-`type`
-  The test type e.g. `testharness`, `reftest`, or `wdspec`.
-
 `reftype`
   The type of comparison for reftests; either `==` or `!=`.
 
 `refurl`
   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
 
     @classmethod
-    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
 
     @property
     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)
 
     @property
     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"]
         try:
             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
             return
         self.test_cache[test_id] = expected_node
 
         if test_id not in self.tests_visited:
             if self.ignore_existing:
                 expected_node.clear_expected()
             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:
             return
-        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"])
 
         self.tests_visited[test.id].add(data["subtest"])
 
         result = test_cls.subtest_result_cls(
             data["subtest"],
             data["status"],
             data.get("message"))
 
         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:
             return
-        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":
             return
 
         result = test_cls.result_cls(
             data["status"],
             data.get("message"))
 
@@ -317,17 +318,17 @@ def create_test_tree(metadata_path, test
 
 
 def create_expected(test_manifest, test_path, tests, property_order=None,
                     boolean_properties=None):
     expected = manifestupdate.ExpectedManifest(None, test_path, test_manifest.url_base,
                                                property_order=property_order,
                                                boolean_properties=boolean_properties)
     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,
                   boolean_properties=None):
     expected_manifest = manifestupdate.get_manifest(metadata_path,
                                                     test_path,
                                                     test_manifest.url_base,
@@ -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:
             test.remove()
 
     # 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: