Bug 1655031 - Fix how mozperftest metrics are parsed. r=tarek

Differential Revision: https://phabricator.services.mozilla.com/D85318

python/mozperftest/mozperftest/metrics/utils.py

@@ -1,9 +1,11 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
+import ast
 import os
 import json
 import pathlib
+import re
 
 from jsonschema import validate
 from jsonschema.exceptions import ValidationError
@@ -28,6 +30,11 @@ KNOWN_SUITE_PROPS = set(
 KNOWN_SINGLE_MEASURE_PROPS = set(set(["values"]) | KNOWN_PERFHERDER_PROPS)
 
 
+# Regex splitter for the metric fields - used to handle
+# the case when `,` is found within the options values.
+METRIC_SPLITTER = re.compile(r",\s*(?![^\[\]]*\])")
+
+
 def is_number(value):
     """Determines if the value is an int/float."""
     return isinstance(value, (int, float)) and not isinstance(value, bool)
@@ -107,17 +114,29 @@ def metric_fields(value):
         return {"name": value}
 
     def _check(field):
-        sfield = field.strip().split(":")
-        if len(sfield) != 2:
+        sfield = field.strip().partition(":")
+        if len(sfield) != 3 or not (sfield[1] and sfield[2]):
             raise ValueError(f"Unexpected metrics definition {field}")
-        if sfield[0] not in KNOWN_PERFHERDER_PROPS:
+        if sfield[0] not in KNOWN_SUITE_PROPS:
             raise ValueError(
-                f"Unknown field '{sfield[0]}', should be in "
-                f"{KNOWN_PERFHERDER_PROPS}"
+                f"Unknown field '{sfield[0]}', should be in " f"{KNOWN_SUITE_PROPS}"
             )
+
+        sfield = [sfield[0], sfield[2]]
+
+        try:
+            # This handles dealing with parsing lists
+            # from a string
+            sfield[1] = ast.literal_eval(sfield[1])
+        except (ValueError, SyntaxError):
+            # Ignore failures, those are from instances
+            # which don't need to be converted from a python
+            # representation
+            pass
+
         return sfield
 
-    fields = [field.strip() for field in value.split(",")]
+    fields = [field.strip() for field in METRIC_SPLITTER.split(value)]
     res = dict([_check(field) for field in fields])
     if "name" not in res:
         raise ValueError(f"{value} misses the 'name' field")

python/mozperftest/mozperftest/tests/test_argparser.py

@@ -49,7 +49,7 @@ def test_perfherder_metrics():
 
     res = parser.parse_args(args)
     assert res.perfherder_metrics[0]["name"] == "foo"
-    assert res.perfherder_metrics[1]["alertThreshold"] == "2"
+    assert res.perfherder_metrics[1]["alertThreshold"] == 2
 
     args = [
         "test_one.js",

python/mozperftest/mozperftest/tests/test_metrics_utils.py

@@ -4,8 +4,9 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 import mozunit
 import json
+import pytest
 
-from mozperftest.metrics.utils import open_file
+from mozperftest.metrics.utils import open_file, metric_fields
 from mozperftest.tests.support import temp_file
 
 
@@ -20,5 +21,69 @@ def test_open_file():
         assert open_file(f) == "yeah"
 
 
+def test_metric_fields_old_format():
+    assert metric_fields("firstPaint") == {"name": "firstPaint"}
+
+
+def test_metric_fields_new_format():
+    assert metric_fields("name:foo,extraOptions:bar") == {
+        "name": "foo",
+        "extraOptions": "bar",
+    }
+
+
+@pytest.mark.parametrize(
+    "metrics, expected",
+    [
+        [
+            "name:foo,extraOptions:['1', '2', '3', 2]",
+            {"name": "foo", "extraOptions": ["1", "2", "3", 2]},
+        ],
+        [
+            """name:foo,extraOptions:['1', '2', '3', 2, "3", "hello,world"] """,
+            {"name": "foo", "extraOptions": ["1", "2", "3", 2, "3", "hello,world"]},
+        ],
+        [
+            """name:foo,extraOptions:['1', '2', '3', 2, "3", "hello,world"],"""
+            """alertThreshold:['1',2,"hello"] """,
+            {
+                "name": "foo",
+                "extraOptions": ["1", "2", "3", 2, "3", "hello,world"],
+                "alertThreshold": ["1", 2, "hello"],
+            },
+        ],
+        [
+            """name:foo,extraOptions:['1', '2', '3', 2, "3", "hello,world"],"""
+            """value:foo,alertThreshold:['1',2,"hello"],framework:99 """,
+            {
+                "name": "foo",
+                "extraOptions": ["1", "2", "3", 2, "3", "hello,world"],
+                "alertThreshold": ["1", 2, "hello"],
+                "value": "foo",
+                "framework": 99,
+            },
+        ],
+    ],
+)
+def test_metric_fields_complex(metrics, expected):
+    assert metric_fields(metrics) == expected
+
+
+@pytest.mark.parametrize(
+    "metrics",
+    [
+        """name:foo,extraOptions:['1', '2', '3', 2, "3", "hello,world"],"""
+        """value:foo,alertThreshold:['1',2,"hello"],framework:99,"""
+        """shouldAlert:[99,100,["hello", "world"],0] """,
+        """name:foo,extraOptions:['1', '2', '3', 2, "3", "hello,world"],"""
+        """value:foo,alertThreshold:['1',2,"hello"],framework:99,"""
+        """shouldAlert:[99,100,["hello:", "world:"],0] """,
+    ],
+)
+def test_metric_fields_complex_failures(metrics):
+    with pytest.raises(Exception):
+        metric_fields(metrics)
+
+
 if __name__ == "__main__":
     mozunit.main()

python/mozperftest/setup.py

@@ -9,7 +9,7 @@ from setuptools import setup
 PACKAGE_NAME = "mozperftest"
 PACKAGE_VERSION = "0.2"
 
-deps = ["jsonschema", "mozlog >= 6.0", "mozdevice >= 4.0.0", "mozproxy", "mozinfo"]
+deps = ["regex", "jsonschema", "mozlog >= 6.0", "mozdevice >= 4.0.0", "mozproxy", "mozinfo"]
 
 setup(
     name=PACKAGE_NAME,