Bug 1279813 - Handle edge cases in wpt update better, r=Ms2ger draft
authorJames Graham <james@hoppipolla.co.uk>
Sun, 12 Jun 2016 17:46:51 +0100
changeset 377834 192a72364cdddb99848d12c978b833d666ae1fdf
parent 377833 a73c5c0e188d1cfcdc1bf32f686405f8b5e569bb
child 377836 15ac011b34fc127c4322ba235bbf9ebe5e2803ed
push id20865
push userbmo:james@hoppipolla.co.uk
push dateSun, 12 Jun 2016 19:04:37 +0000
reviewersMs2ger
bugs1279813
milestone50.0a1
Bug 1279813 - Handle edge cases in wpt update better, r=Ms2ger Better support for cases where tests get conflicting results and where there is no expected value in a manifest. MozReview-Commit-ID: EHYpPa64JJL
testing/web-platform/harness/wptrunner/manifestupdate.py
testing/web-platform/harness/wptrunner/wptmanifest/backends/conditional.py
--- a/testing/web-platform/harness/wptrunner/manifestupdate.py
+++ b/testing/web-platform/harness/wptrunner/manifestupdate.py
@@ -28,16 +28,19 @@ at runtime.
 When a result for a test is to be updated set_result on the
 [Sub]TestNode is called to store the new result, alongside the
 existing conditional that result's run info matched, if any. Once all
 new results are known, coalesce_expected is called to compute the new
 set of results and conditionals. The AST of the underlying parsed manifest
 is updated with the changes, and the result is serialised to a file.
 """
 
+class ConditionError(Exception):
+    pass
+
 Result = namedtuple("Result", ["run_info", "status"])
 
 
 def data_cls_getter(output_node, visited_node):
     # visited_node is intentionally unused
     if output_node is None:
         return ExpectedManifest
     elif isinstance(output_node, ExpectedManifest):
@@ -205,17 +208,18 @@ class TestNode(ManifestItem):
             if not results:
                 # The conditional didn't match anything in these runs so leave it alone
                 final_conditionals.append(conditional_value)
             elif all(results[0].status == result.status for result in results):
                 # All the new values for this conditional matched, so update the node
                 result = results[0]
                 if (result.status == unconditional_status and
                     conditional_value.condition_node is not None):
-                    self.remove_value("expected", conditional_value)
+                    if "expected" in self:
+                        self.remove_value("expected", conditional_value)
                 else:
                     conditional_value.value = result.status
                     final_conditionals.append(conditional_value)
             elif conditional_value.condition_node is not None:
                 # Blow away the existing condition and rebuild from scratch
                 # This isn't sure to work if we have a conditional later that matches
                 # these values too, but we can hope, verify that we get the results
                 # we expect, and if not let a human sort it out
@@ -231,20 +235,25 @@ class TestNode(ManifestItem):
         if self.new_expected:
             if all(self.new_expected[0].status == result.status
                    for result in self.new_expected) and not self.updated_expected:
                 status = self.new_expected[0].status
                 if status != self.default_status:
                     self.set("expected", status, condition=None)
                     final_conditionals.append(self._data["expected"][-1])
             else:
-                for conditional_node, status in group_conditionals(
+                try:
+                    conditionals = group_conditionals(
                         self.new_expected,
                         property_order=self.root.property_order,
-                        boolean_properties=self.root.boolean_properties):
+                        boolean_properties=self.root.boolean_properties)
+                except ConditionError:
+                    print "Conflicting test results for %s, cannot update" % self.root.test_path
+                    return
+                for conditional_node, status in conditionals:
                     if status != unconditional_status:
                         self.set("expected", status, condition=conditional_node.children[0])
                         final_conditionals.append(self._data["expected"][-1])
 
         if ("expected" in self._data and
             len(self._data["expected"]) > 0 and
             self._data["expected"][-1].condition_node is None and
             self._data["expected"][-1].value == self.default_status):
@@ -343,16 +352,18 @@ def group_conditionals(values, property_
         boolean_properties = set(boolean_properties)
 
     # If we have more than one value, remove any properties that are common
     # for all the values
     if len(values) > 1:
         for key, statuses in by_property.copy().iteritems():
             if len(statuses) == len(values):
                 del by_property[key]
+        if not by_property:
+            raise ConditionError
 
     properties = set(item[0] for item in by_property.iterkeys())
     include_props = []
 
     for prop in property_order:
         if prop in properties:
             include_props.append(prop)
 
--- a/testing/web-platform/harness/wptrunner/wptmanifest/backends/conditional.py
+++ b/testing/web-platform/harness/wptrunner/wptmanifest/backends/conditional.py
@@ -190,16 +190,19 @@ class ManifestItem(object):
         return "<ManifestItem %s>" % (self.node.data)
 
     def __str__(self):
         rv = [repr(self)]
         for item in self.children:
             rv.extend("  %s" % line for line in str(item).split("\n"))
         return "\n".join(rv)
 
+    def __contains__(self, key):
+        return key in self._data
+
     @property
     def is_empty(self):
         if self._data:
             return False
         return all(child.is_empty for child in self.children)
 
     @property
     def root(self):