Bug 1354232 - Fix updating assert count when there's an exising value, r=maja_zf
authorJames Graham <james@hoppipolla.co.uk>
Thu, 07 Jun 2018 10:40:58 +0100
changeset 1555231 af7bf94091db83843e68dc648fa6df509cf55796
parent 1555230 a14c73b1ed2846f8d386d29fa274fcab9318dd15
child 1555232 c2f10dbc353da438c73a4354380bc6437fb9e2d8
push id282197
push userjames@hoppipolla.co.uk
push dateTue, 03 Jul 2018 20:12:38 +0000
treeherdertry@03ac4554ef3e [default view] [failures only]
reviewersmaja_zf
bugs1354232
milestone63.0a1
Bug 1354232 - Fix updating assert count when there's an exising value, r=maja_zf In this case we want to take the existing value into account, and update to 1 more than the new value (in the max-asserts case). MozReview-Commit-ID: 1RtJ2gU1ZbH
testing/web-platform/tests/tools/wptrunner/wptrunner/manifestexpected.py
testing/web-platform/tests/tools/wptrunner/wptrunner/manifestupdate.py
--- a/testing/web-platform/tests/tools/wptrunner/wptrunner/manifestexpected.py
+++ b/testing/web-platform/tests/tools/wptrunner/wptrunner/manifestexpected.py
@@ -297,16 +297,17 @@ def get_manifest(metadata_root, test_pat
             return static.compile(f,
                                   run_info,
                                   data_cls_getter=data_cls_getter,
                                   test_path=test_path,
                                   url_base=url_base)
     except IOError:
         return None
 
+
 def get_dir_manifest(path, run_info):
     """Get the ExpectedManifest for a particular test path, or None if there is no
     metadata stored for that test path.
 
     :param path: Full path to the ini file
     :param run_info: Dictionary of properties of the test run for which the expectation
                      values should be computed.
     """
--- a/testing/web-platform/tests/tools/wptrunner/wptrunner/manifestupdate.py
+++ b/testing/web-platform/tests/tools/wptrunner/wptrunner/manifestupdate.py
@@ -342,17 +342,19 @@ class PropertyUpdate(object):
                                 if result.value != unconditional_value)
 
         # It is an invariant that nothing in new matches an existing
         # condition except for the default condition
         if self.new:
             update_default, new_default_value = self.update_default()
             if update_default:
                 if new_default_value != self.default_value:
-                    self.node.set(self.property_name, self.update_value(None, new_default_value), condition=None)
+                    self.node.set(self.property_name,
+                                  self.update_value(unconditional_value, new_default_value),
+                                  condition=None)
             else:
                 self.add_new(unconditional_value)
 
         # Remove cases where the value matches the default
         if (self.property_name in self.node._data and
             len(self.node._data[self.property_name]) > 0 and
             self.node._data[self.property_name][-1].condition_node is None and
             self.node._data[self.property_name][-1].value_as(self.value_type) == self.default_value):
@@ -424,84 +426,86 @@ class ExpectedUpdate(PropertyUpdate):
 
 
 class MaxAssertsUpdate(PropertyUpdate):
     property_name = "max-asserts"
     cls_default_value = 0
     value_type = int
 
     def update_value(self, old_value, new_value):
+        new_value = self.value_type(new_value)
         if old_value is not None:
             old_value = self.value_type(old_value)
-        if old_value and old_value < new_value:
-            return new_value
+        if old_value is not None and old_value < new_value:
+            return new_value + 1
         if old_value is None:
-            return new_value
+            return new_value + 1
         return old_value
 
     def update_default(self):
         """For asserts we always update the default value and never add new conditionals.
         The value we set as the default is the maximum the current default or one more than the
         number of asserts we saw in any configuration."""
         # Current values
         values = []
         current_default = None
         if self.property_name in self.node._data:
             current_default = [item for item in
                                self.node._data[self.property_name]
                                if item.condition_node is None]
             if current_default:
                 values.append(int(current_default[0].value))
-        values.extend(item.value + 1 for item in self.new)
-        values.extend(item.value + 1 for item in
+        values.extend(item.value for item in self.new)
+        values.extend(item.value for item in
                       itertools.chain.from_iterable(results for _, results in self.updated))
         new_value = max(values)
         return True, new_value
 
 
 class MinAssertsUpdate(PropertyUpdate):
     property_name = "min-asserts"
     cls_default_value = 0
     value_type = int
 
     def update_value(self, old_value, new_value):
+        new_value = self.value_type(new_value)
         if old_value is not None:
             old_value = self.value_type(old_value)
-        if old_value and new_value < old_value:
+        if old_value is not None and new_value < old_value:
             return 0
         if old_value is None:
             # If we are getting some asserts for the first time, set the minimum to 0
-            return 0
+            return new_value
         return old_value
 
     def update_default(self):
         """For asserts we always update the default value and never add new conditionals.
         This is either set to the current value or one less than the number of asserts
         we saw, whichever is lower."""
         values = []
         current_default = None
         if self.property_name in self.node._data:
             current_default = [item for item in
                                self.node._data[self.property_name]
                                if item.condition_node is None]
         if current_default:
             values.append(current_default[0].value_as(self.value_type))
-        values.extend(max(0, item.value - 1) for item in self.new)
-        values.extend(max(0, item.value - 1) for item in
+        values.extend(max(0, item.value) for item in self.new)
+        values.extend(max(0, item.value) for item in
                       itertools.chain.from_iterable(results for _, results in self.updated))
         new_value = min(values)
         return True, new_value
 
 
 class LsanUpdate(PropertyUpdate):
     property_name = "lsan-allowed"
     cls_default_value = None
 
     def get_value(self, result):
-        # IF we have an allowed_match that matched, return None
+        # If we have an allowed_match that matched, return None
         # This value is ignored later (because it matches the default)
         # We do that because then if we allow a failure in foo/__dir__.ini
         # we don't want to update foo/bar/__dir__.ini with the same rule
         if result[1]:
             return None
         # Otherwise return the topmost stack frame
         # TODO: there is probably some improvement to be made by looking for a "better" stack frame
         return result[0][0]