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 427391 0f2a7154454ceec763bd64597ad9e3155c8d091c
parent 427390 0e161374b583a7637db664473e441a1567e82a5b
child 427392 6bf4a20193b801d342ff662b5a9712e516561d9e
push id66623
push userbtara@mozilla.com
push dateThu, 19 Jul 2018 21:54:02 +0000
treeherderautoland@bb9c6d756b8b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmaja_zf
bugs1354232
milestone63.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 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, stability)
 
         # 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]