Bug 1643103 - Fix how system layers work with the test layers. r=mozperftest-reviewers,tarek

This patch fixes how the system and browser/test layers are run. With this fix, the system layer no longer fully runs its setup and teardown stages before the browser layer has started and finished. Now the setup/teardown happens before/after the full test layer run.

Depends on D78016

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

python/mozperftest/mozperftest/environment.py

@@ -15,7 +15,7 @@ from mozperftest.layers import Layers
 from mozperftest.utils import download_file
 
 
-SYSTEM, BROWSER, METRICS = 0, 1, 2
+SYSTEM, TEST, METRICS = 0, 1, 2
 
 
 class MachEnvironment:
@@ -90,40 +90,32 @@ class MachEnvironment:
         has_exc_handler = self.has_hook("on_exception")
 
         # run the system and test layers
-        for layer in self.layers[:-1]:
-            with layer:
-                try:
-                    metadata = layer(metadata)
-                except Exception as e:
-                    if has_exc_handler:
-                        # if the hook returns True, we abort and return
-                        # without error. If it returns False, we continue
-                        # the loop. The hook can also raise an exception or
-                        # re-raise this exception.
-                        if not self.run_hook("on_exception", layer, e):
-                            return metadata
-                    else:
-                        raise
-        # then run the metrics layers
-        with self.layers[METRICS] as metrics:
+        with self.layers[SYSTEM] as syslayer, self.layers[TEST] as testlayer:
+            metadata = syslayer(metadata)
             try:
-                metadata = metrics(metadata)
+                metadata = testlayer(metadata)
             except Exception as e:
                 if has_exc_handler:
-                    if not self.run_hook("on_exception", layer, e):
+                    # if the hook returns True, we abort and return
+                    # without error. If it returns False, we continue
+                    # the loop. The hook can also raise an exception or
+                    # re-raise this exception.
+                    if not self.run_hook("on_exception", testlayer, e):
                         return metadata
                 else:
                     raise
+
+        # then run the metrics layers
+        with self.layers[METRICS] as metrics:
+            metadata = metrics(metadata)
+
         return metadata
 
     def __enter__(self):
-        for layer in self.layers:
-            layer.__enter__()
         return self
 
     def __exit__(self, type, value, traceback):
-        for layer in self.layers:
-            layer.__exit__(type, value, traceback)
+        return
 
     def cleanup(self):
         if self.tmp_dir is None:

python/mozperftest/mozperftest/metrics/common.py

@@ -135,9 +135,7 @@ class MetricsStorage(object):
                 "file_groups": {data_type: data_info["files"]},
             }
 
-            ptnb = PerftestETL(
-                config["file_groups"], config, data_info["transformer"]
-            )
+            ptnb = PerftestETL(config["file_groups"], config, data_info["transformer"])
             r = ptnb.process(**data_info["options"])
             self.stddata[data_type] = r["data"]
 

python/mozperftest/mozperftest/metrics/perfherder.py

@@ -112,9 +112,7 @@ class Perfherder(Layer):
             subtests = {}
             for r in res:
                 vals = [
-                    v["value"]
-                    for v in r["data"]
-                    if type(v["value"]) in (int, float)
+                    v["value"] for v in r["data"] if type(v["value"]) in (int, float)
                 ]
                 if vals:
                     subtests[r["subtest"]] = vals

python/mozperftest/mozperftest/runner.py

@@ -90,7 +90,7 @@ def run_tests(mach_cmd, **kwargs):
 
     # trying to get the arguments from the task params
     if on_try:
-        try_options = json.loads(os.environ['PERFTEST_OPTIONS'])
+        try_options = json.loads(os.environ["PERFTEST_OPTIONS"])
         kwargs.update(try_options)
 
     from mozperftest.utils import build_test_list, install_package

python/mozperftest/mozperftest/tests/test_browsertime.py

@@ -9,7 +9,7 @@ import random
 import pytest
 
 from mozperftest.tests.support import get_running_env, EXAMPLE_TEST
-from mozperftest.environment import BROWSER
+from mozperftest.environment import TEST
 from mozperftest.test.browsertime import add_options
 from mozperftest.test.browsertime.runner import (
     NodeException,
@@ -44,7 +44,7 @@ def test_browser(*mocked):
         browsertime_extra_options="one=1,two=2",
     )
 
-    browser = env.layers[BROWSER]
+    browser = env.layers[TEST]
     env.set_arg("tests", [EXAMPLE_TEST])
 
     try:
@@ -89,7 +89,7 @@ def test_browser_failed(*mocked):
     )
     # set the return value to 1 to simulate a node failure
     mach_cmd.run_process.return_value = 1
-    browser = env.layers[BROWSER]
+    browser = env.layers[TEST]
     env.set_arg("tests", [EXAMPLE_TEST])
 
     with browser as b, silence(), pytest.raises(NodeException):
@@ -109,7 +109,7 @@ def test_browser_desktop(*mocked):
     mach_cmd, metadata, env = get_running_env(
         browsertime_iterations=1, browsertime_extra_options="one=1,two=2",
     )
-    browser = env.layers[BROWSER]
+    browser = env.layers[TEST]
     env.set_arg("tests", [EXAMPLE_TEST])
 
     try:
@@ -151,7 +151,7 @@ def test_install_url(*mocked):
         [random.choice(string.hexdigits[:-6]) for c in range(40)]
     )
     mach, metadata, env = get_running_env(browsertime_install_url=url)
-    browser = env.layers[BROWSER]
+    browser = env.layers[TEST]
     env.set_arg("tests", [EXAMPLE_TEST])
 
     try:
@@ -174,7 +174,7 @@ def test_install_url(*mocked):
 )
 def test_install_url_bad(*mocked):
     mach, metadata, env = get_running_env(browsertime_install_url="meh")
-    browser = env.layers[BROWSER]
+    browser = env.layers[TEST]
     env.set_arg("tests", [EXAMPLE_TEST])
 
     with pytest.raises(ValueError):

python/mozperftest/mozperftest/tests/test_environment.py

@@ -119,18 +119,19 @@ def test_metrics_last():
     system = create_mock()
     browser = create_mock()
 
-    # check that the metrics layer is called after
+    # check that the metrics layer is entered after
     # other have finished
     class M:
         def __enter__(self):
+            system.teardown.assert_called()
+            browser.teardown.assert_called()
             return self
 
         def __exit__(self, *args, **kw):
             return
 
         def __call__(self, metadata):
-            system.teardown.assert_called()
-            browser.teardown.assert_called()
+            return
 
     env.layers = [system, browser, M()]
     with env:

python/mozperftest/mozperftest/tests/test_perftestnotebook.py

@@ -19,13 +19,11 @@ def test_get_notebook_section(ptnb):
 
 def test_get_notebook_section_unknown_analysis(ptnb):
     func = "unknown"
-    with mock.patch(
-        "mozperftest.metrics.notebook.perftestnotebook.logger"
-    ) as logger:
+    with mock.patch("mozperftest.metrics.notebook.perftestnotebook.logger") as logger:
         assert ptnb.get_notebook_section(func) == ""
-        logger.assert_has_calls(mock.call.warning(
-                'Could not find the notebook-section called unknown'
-            ))
+        logger.assert_has_calls(
+            mock.call.warning("Could not find the notebook-section called unknown")
+        )
 
 
 @pytest.mark.parametrize("analysis", [["scatterplot"], None])