Bug 1401613 - Don't require test type in wpt expectation manifest, r=maja_zf
authorJames Graham <james@hoppipolla.co.uk>
Wed, 20 Sep 2017 15:50:06 +0100
changeset 382607 f7aad92c4e7fddb3e1a7551cdcccae88d8cdd05e
parent 382606 698a9bb34b3d0b09465200421fedfb4a0f65fb23
child 382608 04fbb3a306ce7bf344f9229f40b6e8990cebd029
push id32566
push userphilringnalda@gmail.com
push dateSun, 24 Sep 2017 02:20:42 +0000
treeherdermozilla-central@ff40c5dcaa41 [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
@@ -116,35 +116,34 @@ class TestNode(ManifestItem):
         self.updated_expected = []
         self.new_expected = []
         self.new_disabled = False
         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
@@ -241,61 +241,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"))
         test.set_result(self.run_info, result)
@@ -331,17 +332,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,
@@ -355,11 +356,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: