Skip to content

Commit

Permalink
Switch to explicit profile cleanup rather than using __del
Browse files Browse the repository at this point in the history
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

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1678044
gecko-commit: 5281b7443a899dcd538380b4f7c15c46300f9bdd
gecko-reviewers: webdriver-reviewers, whimboo
  • Loading branch information
jgraham committed May 21, 2021
1 parent 8c9153b commit 3127e83
Show file tree
Hide file tree
Showing 19 changed files with 66 additions and 71 deletions.
9 changes: 4 additions & 5 deletions tools/wptrunner/wptrunner/browsers/android_weblayer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
9 changes: 4 additions & 5 deletions tools/wptrunner/wptrunner/browsers/android_webview.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
5 changes: 2 additions & 3 deletions tools/wptrunner/wptrunner/browsers/chrome.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 4 additions & 5 deletions tools/wptrunner/wptrunner/browsers/chrome_android.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
4 changes: 2 additions & 2 deletions tools/wptrunner/wptrunner/browsers/chrome_ios.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"] = {}
Expand Down
5 changes: 2 additions & 3 deletions tools/wptrunner/wptrunner/browsers/edge.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 3 additions & 2 deletions tools/wptrunner/wptrunner/browsers/edgechromium.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 3 additions & 4 deletions tools/wptrunner/wptrunner/browsers/epiphany.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
28 changes: 20 additions & 8 deletions tools/wptrunner/wptrunner/browsers/firefox.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -380,6 +389,7 @@ def teardown(self, force=False):
if instance:
instance.stop(force)
instance.cleanup()
self.base_profile.cleanup()


class PreloadInstanceManager(FirefoxInstanceManager):
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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


Expand Down Expand Up @@ -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:
Expand Down
5 changes: 2 additions & 3 deletions tools/wptrunner/wptrunner/browsers/ie.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions tools/wptrunner/wptrunner/browsers/opera.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"] = {
Expand Down
6 changes: 2 additions & 4 deletions tools/wptrunner/wptrunner/browsers/safari.py
Original file line number Diff line number Diff line change
Expand Up @@ -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":
Expand Down
5 changes: 2 additions & 3 deletions tools/wptrunner/wptrunner/browsers/sauce.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
5 changes: 2 additions & 3 deletions tools/wptrunner/wptrunner/browsers/servo.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"] = {}
Expand Down
5 changes: 2 additions & 3 deletions tools/wptrunner/wptrunner/browsers/servodriver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
7 changes: 3 additions & 4 deletions tools/wptrunner/wptrunner/browsers/webkit.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
7 changes: 3 additions & 4 deletions tools/wptrunner/wptrunner/browsers/webkitgtk_minibrowser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
8 changes: 3 additions & 5 deletions tools/wptrunner/wptrunner/executors/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
3 changes: 1 addition & 2 deletions tools/wptrunner/wptrunner/wptrunner.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down

0 comments on commit 3127e83

Please sign in to comment.