Bug 1678044 - Switch to explicit profile cleanup rather than using __…

…del__, r=webdriver-reviewers,whimboo This requires passing the test_environment into the get_executor_kwargs function so that in the firefox wdspec case we can add a cleanup function to the environment when running wdspec tests. That seems reasonable since we were previously using a variety of data in the environment to setup the kwargs anyway Depends on D99698 Differential Revision: https://phabricator.services.mozilla.com/D101768
dothq · May 19, 2021 · 3f74c6c · 3f74c6c
1 parent a7fdba5
commit 3f74c6c
Show file tree

Hide file tree

Showing 20 changed files with 71 additions and 73 deletions.
diff --git a/testing/mozbase/mozrunner/mozrunner/base/runner.py b/testing/mozbase/mozrunner/mozrunner/base/runner.py
@@ -78,7 +78,8 @@ def __init__(
 
     def __del__(self):
         if not self.explicit_cleanup:
-            self.cleanup()
+            # If we're relying on the gc for cleanup do the same with the profile
+            self.cleanup(keep_profile=True)
 
     @abstractproperty
     def command(self):
@@ -274,8 +275,10 @@ def check_for_crashes(
 
         return crash_count
 
-    def cleanup(self):
+    def cleanup(self, keep_profile=False):
         """
         Cleanup all runner state
         """
         self.stop()
+        if not keep_profile:
+            self.profile.cleanup()
diff --git a/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/android_weblayer.py b/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/android_weblayer.py
@@ -35,16 +35,15 @@ def browser_kwargs(logger, test_type, run_info_data, config, **kwargs):
             "symbols_path": kwargs.get("symbols_path")}
 
 
-def executor_kwargs(logger, test_type, server_config, cache_manager, run_info_data,
+def executor_kwargs(logger, test_type, test_environment, run_info_data,
                     **kwargs):
     # Use update() to modify the global list in place.
     _wptserve_ports.update(set(
-        server_config['ports']['http'] + server_config['ports']['https'] +
-        server_config['ports']['ws'] + server_config['ports']['wss']
+        test_environment.config['ports']['http'] + test_environment.config['ports']['https'] +
+        test_environment.config['ports']['ws'] + test_environment.config['ports']['wss']
     ))
 
-    executor_kwargs = chrome_executor_kwargs(logger, test_type, server_config,
-                                             cache_manager, run_info_data,
+    executor_kwargs = chrome_executor_kwargs(logger, test_type, test_environment, run_info_data,
                                              **kwargs)
     del executor_kwargs["capabilities"]["goog:chromeOptions"]["prefs"]
     capabilities = executor_kwargs["capabilities"]

diff --git a/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/android_webview.py b/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/android_webview.py
@@ -35,16 +35,15 @@ def browser_kwargs(logger, test_type, run_info_data, config, **kwargs):
             "symbols_path": kwargs.get("symbols_path")}
 
 
-def executor_kwargs(logger, test_type, server_config, cache_manager, run_info_data,
+def executor_kwargs(logger, test_type, test_environment, run_info_data,
                     **kwargs):
     # Use update() to modify the global list in place.
     _wptserve_ports.update(set(
-        server_config['ports']['http'] + server_config['ports']['https'] +
-        server_config['ports']['ws'] + server_config['ports']['wss']
+        test_environment.config['ports']['http'] + test_environment.config['ports']['https'] +
+        test_environment.config['ports']['ws'] + test_environment.config['ports']['wss']
     ))
 
-    executor_kwargs = chrome_executor_kwargs(logger, test_type, server_config,
-                                             cache_manager, run_info_data,
+    executor_kwargs = chrome_executor_kwargs(logger, test_type, test_environment, run_info_data,
                                              **kwargs)
     del executor_kwargs["capabilities"]["goog:chromeOptions"]["prefs"]
     capabilities = executor_kwargs["capabilities"]

diff --git a/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/chrome.py b/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/chrome.py
@@ -34,10 +34,9 @@ def browser_kwargs(logger, test_type, run_info_data, config, **kwargs):
             "webdriver_args": kwargs.get("webdriver_args")}
 
 
-def executor_kwargs(logger, test_type, server_config, cache_manager, run_info_data,
+def executor_kwargs(logger, test_type, test_environment, run_info_data,
                     **kwargs):
-    executor_kwargs = base_executor_kwargs(test_type, server_config,
-                                           cache_manager, run_info_data,
+    executor_kwargs = base_executor_kwargs(test_type, test_environment, run_info_data,
                                            **kwargs)
     executor_kwargs["close_after_done"] = True
     executor_kwargs["supports_eager_pageload"] = False

diff --git a/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/chrome_android.py b/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/chrome_android.py
@@ -39,16 +39,15 @@ def browser_kwargs(logger, test_type, run_info_data, config, **kwargs):
             "symbols_path": kwargs.get("symbols_path")}
 
 
-def executor_kwargs(logger, test_type, server_config, cache_manager, run_info_data,
+def executor_kwargs(logger, test_type, test_environment, run_info_data,
                     **kwargs):
     # Use update() to modify the global list in place.
     _wptserve_ports.update(set(
-        server_config['ports']['http'] + server_config['ports']['https'] +
-        server_config['ports']['ws'] + server_config['ports']['wss']
+        test_environment.config['ports']['http'] + test_environment.config['ports']['https'] +
+        test_environment.config['ports']['ws'] + test_environment.config['ports']['wss']
     ))
 
-    executor_kwargs = chrome_executor_kwargs(logger, test_type, server_config,
-                                             cache_manager, run_info_data,
+    executor_kwargs = chrome_executor_kwargs(logger, test_type, test_environment, run_info_data,
                                              **kwargs)
     # Remove unsupported options on mobile.
     del executor_kwargs["capabilities"]["goog:chromeOptions"]["prefs"]

diff --git a/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/chrome_ios.py b/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/chrome_ios.py
@@ -27,9 +27,9 @@ def browser_kwargs(logger, test_type, run_info_data, config, **kwargs):
             "webdriver_args": kwargs.get("webdriver_args")}
 
 
-def executor_kwargs(logger, test_type, server_config, cache_manager, run_info_data,
+def executor_kwargs(logger, test_type, test_environment, run_info_data,
                     **kwargs):
-    executor_kwargs = base_executor_kwargs(test_type, server_config, cache_manager, run_info_data,
+    executor_kwargs = base_executor_kwargs(test_type, test_environment, run_info_data,
                                            **kwargs)
     executor_kwargs["close_after_done"] = True
     executor_kwargs["capabilities"] = {}

diff --git a/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/edge.py b/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/edge.py
@@ -42,10 +42,9 @@ def browser_kwargs(logger, test_type, run_info_data, config, **kwargs):
                                                          **kwargs)}
 
 
-def executor_kwargs(logger, test_type, server_config, cache_manager, run_info_data,
+def executor_kwargs(logger, test_type, test_environment, run_info_data,
                     **kwargs):
-    executor_kwargs = base_executor_kwargs(test_type, server_config,
-                                           cache_manager, run_info_data, **kwargs)
+    executor_kwargs = base_executor_kwargs(test_type, test_environment, run_info_data, **kwargs)
     executor_kwargs["close_after_done"] = True
     executor_kwargs["timeout_multiplier"] = get_timeout_multiplier(test_type,
                                                                    run_info_data,

diff --git a/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/edgechromium.py b/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/edgechromium.py
@@ -30,9 +30,10 @@ def browser_kwargs(logger, test_type, run_info_data, config, **kwargs):
             "webdriver_args": kwargs.get("webdriver_args")}
 
 
-def executor_kwargs(logger, test_type, server_config, cache_manager, run_info_data,
+def executor_kwargs(logger, test_type,test_environment, cache_manager, run_info_data,
                     **kwargs):
-    executor_kwargs = base_executor_kwargs(test_type, server_config,
+    executor_kwargs = base_executor_kwargs(test_type,
+                                           test_environment,
                                            cache_manager, run_info_data,
                                            **kwargs)
     executor_kwargs["close_after_done"] = True

diff --git a/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/epiphany.py b/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/epiphany.py
@@ -49,12 +49,11 @@ def capabilities(server_config, **kwargs):
             "certificates": certificate_domain_list(server_config.domains_set, kwargs["host_cert_path"])}}
 
 
-def executor_kwargs(logger, test_type, server_config, cache_manager, run_info_data,
+def executor_kwargs(logger, test_type, test_environment, run_info_data,
                     **kwargs):
-    executor_kwargs = base_executor_kwargs(test_type, server_config,
-                                           cache_manager, run_info_data, **kwargs)
+    executor_kwargs = base_executor_kwargs(test_type, test_environment, run_info_data, **kwargs)
     executor_kwargs["close_after_done"] = True
-    executor_kwargs["capabilities"] = capabilities(server_config, **kwargs)
+    executor_kwargs["capabilities"] = capabilities(test_environment.config, **kwargs)
     return executor_kwargs
 
 

diff --git a/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/firefox.py b/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/firefox.py
@@ -106,10 +106,20 @@ def browser_kwargs(logger, test_type, run_info_data, config, **kwargs):
             "specialpowers_path": kwargs["specialpowers_path"]}
 
 
-def executor_kwargs(logger, test_type, server_config, cache_manager, run_info_data,
+class WdSpecProfile(object):
+    def __init__(self, profile):
+        self.profile = profile
+
+    def __enter__(self):
+        return self
+
+    def __exit__(self, *args, **kwargs):
+        self.profile.cleanup()
+
+
+def executor_kwargs(logger, test_type, test_environment, run_info_data,
                     **kwargs):
-    executor_kwargs = base_executor_kwargs(test_type, server_config,
-                                           cache_manager, run_info_data,
+    executor_kwargs = base_executor_kwargs(test_type, test_environment, run_info_data,
                                            **kwargs)
     executor_kwargs["close_after_done"] = test_type != "reftest"
     executor_kwargs["timeout_multiplier"] = get_timeout_multiplier(test_type,
@@ -131,15 +141,15 @@ def executor_kwargs(logger, test_type, server_config, cache_manager, run_info_da
 
         profile_creator = ProfileCreator(logger,
                                          kwargs["prefs_root"],
-                                         server_config,
+                                         test_environment.config,
                                          test_type,
                                          kwargs["extra_prefs"],
                                          kwargs["gecko_e10s"],
                                          kwargs["enable_fission"],
                                          kwargs["browser_channel"],
                                          kwargs["binary"],
                                          kwargs["certutil_binary"],
-                                         server_config.ssl_config["ca_cert_path"])
+                                         test_environment.config.ssl_config["ca_cert_path"])
         if kwargs["processes"] > 1:
             # With multiple processes, we would need a profile directory per process, but we
             # don't have an easy way to do that, so include the profile in the capabilties
@@ -148,8 +158,7 @@ def executor_kwargs(logger, test_type, server_config, cache_manager, run_info_da
         else:
             profile = profile_creator.create()
             options["args"].extend(["--profile", profile.profile])
-            # Prevent the profile being deleted
-            executor_kwargs["profile"] = profile
+            test_environment.env_extras_cms.append(WdSpecProfile(profile))
 
         capabilities["moz:firefoxOptions"] = options
 
@@ -380,6 +389,7 @@ def teardown(self, force=False):
             if instance:
                 instance.stop(force)
                 instance.cleanup()
+        self.base_profile.cleanup()
 
 
 class PreloadInstanceManager(FirefoxInstanceManager):
@@ -408,6 +418,7 @@ def teardown(self, force=False):
             if instance:
                 instance.stop(force, skip_marionette=skip_marionette)
                 instance.cleanup()
+        self.base_profile.cleanup()
 
 
 class BrowserInstance(object):
@@ -469,7 +480,7 @@ def is_alive(self):
         return False
 
     def cleanup(self):
-        # mozprofile handles deleting the profile when the refcount reaches 0
+        self.runner.cleanup()
         self.runner = None
 
 
@@ -603,6 +614,7 @@ def create(self, **kwargs):
         preferences = self._load_prefs()
 
         profile = FirefoxProfile(preferences=preferences,
+                                 restore=False,
                                  **kwargs)
         self._set_required_prefs(profile)
         if self.ca_certificate_path is not None:

diff --git a/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/ie.py b/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/ie.py
@@ -22,14 +22,13 @@ def browser_kwargs(logger, test_type, run_info_data, config, **kwargs):
     return {"webdriver_binary": kwargs["webdriver_binary"],
             "webdriver_args": kwargs.get("webdriver_args")}
 
-def executor_kwargs(logger, test_type, server_config, cache_manager, run_info_data,
+def executor_kwargs(logger, test_type, test_environment, run_info_data,
                     **kwargs):
     options = {}
     options["requireWindowFocus"] = True
     capabilities = {}
     capabilities["se:ieOptions"] = options
-    executor_kwargs = base_executor_kwargs(test_type, server_config,
-                                           cache_manager, run_info_data, **kwargs)
+    executor_kwargs = base_executor_kwargs(test_type, test_environment, run_info_data, **kwargs)
     executor_kwargs["close_after_done"] = True
     executor_kwargs["capabilities"] = capabilities
     return executor_kwargs

diff --git a/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/opera.py b/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/opera.py
@@ -30,12 +30,11 @@ def browser_kwargs(logger, test_type, run_info_data, config, **kwargs):
             "webdriver_args": kwargs.get("webdriver_args")}
 
 
-def executor_kwargs(logger, test_type, server_config, cache_manager, run_info_data,
+def executor_kwargs(logger, test_type, test_environment, run_info_data,
                     **kwargs):
     from selenium.webdriver import DesiredCapabilities
 
-    executor_kwargs = base_executor_kwargs(test_type, server_config,
-                                           cache_manager, run_info_data, **kwargs)
+    executor_kwargs = base_executor_kwargs(test_type, test_environment, run_info_data, **kwargs)
     executor_kwargs["close_after_done"] = True
     capabilities = dict(DesiredCapabilities.OPERA.items())
     capabilities.setdefault("operaOptions", {})["prefs"] = {

diff --git a/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/safari.py b/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/safari.py
@@ -38,10 +38,8 @@ def browser_kwargs(logger, test_type, run_info_data, config, **kwargs):
             "kill_safari": kwargs.get("kill_safari", False)}
 
 
-def executor_kwargs(logger, test_type, server_config, cache_manager, run_info_data,
-                    **kwargs):
-    executor_kwargs = base_executor_kwargs(test_type, server_config,
-                                           cache_manager, run_info_data, **kwargs)
+def executor_kwargs(logger, test_type, test_environment, run_info_data, **kwargs):
+    executor_kwargs = base_executor_kwargs(test_type, test_environment, run_info_data, **kwargs)
     executor_kwargs["close_after_done"] = True
     executor_kwargs["capabilities"] = {}
     if test_type == "testharness":

diff --git a/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/sauce.py b/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/sauce.py
@@ -101,10 +101,9 @@ def browser_kwargs(logger, test_type, run_info_data, config, **kwargs):
     return {"sauce_config": sauce_config}
 
 
-def executor_kwargs(logger, test_type, server_config, cache_manager, run_info_data,
+def executor_kwargs(logger, test_type, test_environment, run_info_data,
                     **kwargs):
-    executor_kwargs = base_executor_kwargs(test_type, server_config,
-                                           cache_manager, run_info_data, **kwargs)
+    executor_kwargs = base_executor_kwargs(test_type, test_environment, run_info_data, **kwargs)
 
     executor_kwargs["capabilities"] = get_capabilities(**kwargs)
 

diff --git a/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/servo.py b/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/servo.py
@@ -43,10 +43,9 @@ def browser_kwargs(logger, test_type, run_info_data, config, **kwargs):
     }
 
 
-def executor_kwargs(logger, test_type, server_config, cache_manager, run_info_data,
+def executor_kwargs(logger, test_type, test_environment, run_info_data,
                     **kwargs):
-    rv = base_executor_kwargs(test_type, server_config,
-                              cache_manager, run_info_data, **kwargs)
+    rv = base_executor_kwargs(test_type, test_environment, run_info_data, **kwargs)
     rv["pause_after_test"] = kwargs["pause_after_test"]
     if test_type == "wdspec":
         rv["capabilities"] = {}

diff --git a/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/servodriver.py b/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/servodriver.py
@@ -46,9 +46,8 @@ def browser_kwargs(logger, test_type, run_info_data, config, **kwargs):
     }
 
 
-def executor_kwargs(logger, test_type, server_config, cache_manager, run_info_data, **kwargs):
-    rv = base_executor_kwargs(test_type, server_config,
-                              cache_manager, run_info_data, **kwargs)
+def executor_kwargs(logger, test_type, test_environment, run_info_data, **kwargs):
+    rv = base_executor_kwargs(test_type, test_environment, run_info_data, **kwargs)
     return rv
 
 

diff --git a/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/webkit.py b/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/webkit.py
@@ -54,12 +54,11 @@ def capabilities_for_port(server_config, **kwargs):
     return {}
 
 
-def executor_kwargs(logger, test_type, server_config, cache_manager, run_info_data,
+def executor_kwargs(logger, test_type, test_environment, run_info_data,
                     **kwargs):
-    executor_kwargs = base_executor_kwargs(test_type, server_config,
-                                           cache_manager, run_info_data, **kwargs)
+    executor_kwargs = base_executor_kwargs(test_type, test_environment, run_info_data, **kwargs)
     executor_kwargs["close_after_done"] = True
-    executor_kwargs["capabilities"] = capabilities_for_port(server_config,
+    executor_kwargs["capabilities"] = capabilities_for_port(test_environment.config,
                                                             **kwargs)
     return executor_kwargs
 

diff --git a/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/webkitgtk_minibrowser.py b/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/webkitgtk_minibrowser.py
@@ -53,12 +53,11 @@ def capabilities(server_config, **kwargs):
             "certificates": certificate_domain_list(server_config.domains_set, kwargs["host_cert_path"])}}
 
 
-def executor_kwargs(logger, test_type, server_config, cache_manager, run_info_data,
+def executor_kwargs(logger, test_type, test_environment, run_info_data,
                     **kwargs):
-    executor_kwargs = base_executor_kwargs(test_type, server_config,
-                                           cache_manager, run_info_data, **kwargs)
+    executor_kwargs = base_executor_kwargs(test_type, test_environment, run_info_data, **kwargs)
     executor_kwargs["close_after_done"] = True
-    executor_kwargs["capabilities"] = capabilities(server_config, **kwargs)
+    executor_kwargs["capabilities"] = capabilities(test_environment.config, **kwargs)
     return executor_kwargs
 
 

diff --git a/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/base.py b/testing/web-platform/tests/tools/wptrunner/wptrunner/executors/base.py
@@ -17,19 +17,17 @@
 here = os.path.dirname(__file__)
 
 
-
-def executor_kwargs(test_type, server_config, cache_manager, run_info_data,
-                    **kwargs):
+def executor_kwargs(test_type, test_environment, run_info_data, **kwargs):
     timeout_multiplier = kwargs["timeout_multiplier"]
     if timeout_multiplier is None:
         timeout_multiplier = 1
 
-    executor_kwargs = {"server_config": server_config,
+    executor_kwargs = {"server_config": test_environment.config,
                        "timeout_multiplier": timeout_multiplier,
                        "debug_info": kwargs["debug_info"]}
 
     if test_type in ("reftest", "print-reftest"):
-        executor_kwargs["screenshot_cache"] = cache_manager.dict()
+        executor_kwargs["screenshot_cache"] = test_environment.cache_manager.dict()
 
     if test_type == "wdspec":
         executor_kwargs["binary"] = kwargs.get("binary")

diff --git a/testing/web-platform/tests/tools/wptrunner/wptrunner/wptrunner.py b/testing/web-platform/tests/tools/wptrunner/wptrunner/wptrunner.py
@@ -296,8 +296,7 @@ def run_tests(config, test_paths, product, **kwargs):
                     executor_cls = product.executor_classes.get(test_type)
                     executor_kwargs = product.get_executor_kwargs(logger,
                                                                   test_type,
-                                                                  test_environment.config,
-                                                                  test_environment.cache_manager,
+                                                                  test_environment,
                                                                   run_info,
                                                                   **kwargs)