Skip to content

Commit

Permalink
Bug 1744091 - Update test harnesses to run with fission by default, e…
Browse files Browse the repository at this point in the history
…xcept on android; r=perftest-reviewers,releng-reviewers,jmaher,AlexandruIonescu

Run with fission by default in test harnesses, with --disable-fission available as an
option.
Android mach commands automatically set --disable-fission; this can be over-ridden by
--setpref=fission.autostart=true.
fission.autostart is removed from all test profiles.
No changes to wpt, handled already in bug 1743714.

Differential Revision: https://phabricator.services.mozilla.com/D135137
  • Loading branch information
gbrownmozilla committed Jan 7, 2022
1 parent 22b54fb commit 618f09c
Show file tree
Hide file tree
Showing 25 changed files with 83 additions and 93 deletions.
4 changes: 4 additions & 0 deletions layout/tools/reftest/mach_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,10 @@ def run_android_test(self, **kwargs):
args.e10s = False
print("using e10s=False for non-geckoview app")

# Disable fission until geckoview supports fission by default.
# Need fission on Android? Use '--setpref fission.autostart=true'
args.fission = False

# A symlink and some path manipulations are required so that test
# manifests can be found both locally and remotely (via a url)
# using the same relative path.
Expand Down
8 changes: 4 additions & 4 deletions layout/tools/reftest/reftestcommandline.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,11 +249,11 @@ def __init__(self, **kwargs):
)

self.add_argument(
"--enable-fission",
action="store_true",
default=False,
"--disable-fission",
action="store_false",
default=True,
dest="fission",
help="Run tests with fission (site isolation) enabled.",
help="Run tests with fission (site isolation) disabled.",
)

self.add_argument(
Expand Down
3 changes: 0 additions & 3 deletions layout/tools/reftest/remotereftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -429,9 +429,6 @@ def runApp(
# browser environment
env = self.buildBrowserEnv(options, profile.profile)

self.log.info("Running with e10s: {}".format(options.e10s))
self.log.info("Running with fission: {}".format(options.fission))

rpm = RemoteProcessMonitor(
binary,
self.device,
Expand Down
9 changes: 6 additions & 3 deletions layout/tools/reftest/runreftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,12 @@ def createReftestProfile(
options.extraProfileFiles.append(os.path.join(here, "chrome"))

self.copyExtraFilesToProfile(options, profile)

self.log.info(
"Running with e10s: {}".format(prefs["browser.tabs.remote.autostart"])
)
self.log.info("Running with fission: {}".format(prefs["fission.autostart"]))

return profile

def environment(self, **kwargs):
Expand Down Expand Up @@ -879,9 +885,6 @@ def runApp(
# browser environment
env = self.buildBrowserEnv(options, profile.profile)

self.log.info("Running with e10s: {}".format(options.e10s))
self.log.info("Running with fission: {}".format(options.fission))

def timeoutHandler():
self.handleTimeout(timeout, proc, options.utilityPath, debuggerInfo)

Expand Down
4 changes: 4 additions & 0 deletions testing/mochitest/mach_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,10 @@ def run_android_test(self, command_context, tests, **kwargs):
options.e10s = False
print("using e10s=False for non-geckoview app")

# Disable fission until geckoview supports fission by default.
# Need fission on Android? Use '--setpref fission.autostart=true'
options.fission = False

return runtestsremote.run_test_harness(parser, options)

def run_geckoview_junit_test(self, context, **kwargs):
Expand Down
19 changes: 13 additions & 6 deletions testing/mochitest/mochitest_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -563,11 +563,12 @@ class MochitestArguments(ArgumentContainer):
},
],
[
["--enable-fission"],
["--disable-fission"],
{
"action": "store_true",
"default": False,
"help": "Run tests with fission (site isolation) enabled.",
"action": "store_false",
"default": True,
"dest": "fission",
"help": "Run tests with fission (site isolation) disabled.",
},
],
[
Expand Down Expand Up @@ -1155,8 +1156,14 @@ def validate(self, parser, options, context):
"--disable-e10s.".format(options.flavor)
)

if options.enable_fission:
options.extraPrefs.append("fission.autostart=true")
# If e10s explicitly disabled and no fission option specified, disable fission
if (
(not options.e10s)
and options.fission
and ("fission.autostart=true" not in options.extraPrefs)
and ("fission.autostart=false" not in options.extraPrefs)
):
options.fission = False

options.leakThresholds = {
"default": options.defaultLeakThreshold,
Expand Down
7 changes: 5 additions & 2 deletions testing/mochitest/runtests.py
Original file line number Diff line number Diff line change
Expand Up @@ -2847,12 +2847,15 @@ def runTests(self, options):
"""Prepare, configure, run tests and cleanup"""
self.extraPrefs = parse_preferences(options.extraPrefs)

if "fission.autostart" not in self.extraPrefs:
self.extraPrefs["fission.autostart"] = options.fission

# for test manifest parsing.
mozinfo.update(
{
"a11y_checks": options.a11y_checks,
"e10s": options.e10s,
"fission": self.extraPrefs.get("fission.autostart", False),
"fission": self.extraPrefs.get("fission.autostart", True),
"headless": options.headless,
# Until the test harness can understand default pref values,
# (https://bugzilla.mozilla.org/show_bug.cgi?id=1577912) this value
Expand All @@ -2865,7 +2868,7 @@ def runTests(self, options):
"sessionHistoryInParent": self.extraPrefs.get(
"fission.sessionHistoryInParent", False
)
or self.extraPrefs.get("fission.autostart", False),
or self.extraPrefs.get("fission.autostart", True),
"socketprocess_e10s": self.extraPrefs.get(
"network.process.enabled", False
),
Expand Down
14 changes: 7 additions & 7 deletions testing/mozharness/mozharness/mozilla/testing/raptor.py
Original file line number Diff line number Diff line change
Expand Up @@ -457,12 +457,12 @@ class Raptor(
},
],
[
["--enable-fission"],
["--disable-fission"],
{
"action": "store_true",
"dest": "enable_fission",
"default": False,
"help": "Enable Fission (site isolation) in Gecko.",
"action": "store_false",
"dest": "fission",
"default": True,
"help": "Disable Fission (site isolation) in Gecko.",
},
],
[
Expand Down Expand Up @@ -906,8 +906,8 @@ def raptor_options(self, args=None, **kw):
options.extend(["--disable-perf-tuning"])
if self.config.get("cold", False):
options.extend(["--cold"])
if self.config.get("enable_fission", False):
options.extend(["--enable-fission"])
if not self.config.get("fission", True):
options.extend(["--disable-fission"])
if self.config.get("verbose", False):
options.extend(["--verbose"])
if self.config.get("extra_prefs"):
Expand Down
21 changes: 10 additions & 11 deletions testing/mozharness/mozharness/mozilla/testing/talos.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,12 +208,12 @@ class Talos(
},
],
[
["--enable-fission"],
["--disable-fission"],
{
"action": "store_true",
"dest": "enable_fission",
"default": False,
"help": "Enable Fission (site isolation) in Gecko.",
"action": "store_false",
"dest": "fission",
"default": True,
"help": "Disable Fission (site isolation) in Gecko.",
},
],
[
Expand Down Expand Up @@ -527,13 +527,12 @@ def talos_options(self, args=None, **kw):
options.extend(
["--setpref={}".format(p) for p in self.config["extra_prefs"]]
)
# enabling fission can come from the --enable-fission cmd line argument; or in CI
# disabling fission can come from the --disable-fission cmd line argument; or in CI
# it comes from a taskcluster transform which adds a --setpref for fission.autostart
if (
self.config["enable_fission"]
or "fission.autostart=true" in self.config["extra_prefs"]
):
options.extend(["--enable-fission"])
if (not self.config["fission"]) or "fission.autostart=false" in self.config[
"extra_prefs"
]:
options.extend(["--disable-fission"])

return options

Expand Down
5 changes: 0 additions & 5 deletions testing/profiles/geckoview-junit/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,3 @@

// Always run in e10s
user_pref("browser.tabs.remote.autostart", true);

// Explicitly turn off fission so we don't accidentally use the wrong default
// value. This can be removed once harnesses and tasks assume fission by
// default.
user_pref("fission.autostart", false);
5 changes: 0 additions & 5 deletions testing/profiles/mochitest/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,3 @@ user_pref("browser.sessionstore.resume_from_crash", false);
// is suppressed, synthetic click events and co. go to the old page, which can
// be confusing for tests that send click events before the first paint.
user_pref("nglayout.initialpaint.unsuppress_with_no_background", true);

// Explicitly turn off fission so we don't accidentally use the wrong default
// value. This can be removed once harnesses and tasks assume fission by
// default.
user_pref("fission.autostart", false);
4 changes: 0 additions & 4 deletions testing/profiles/perf/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,3 @@ user_pref("startup.homepage_welcome_url.additional", "");
// tests (bug 1725270). Can be removed once non-about:blank intermediate pages
// are used instead (bug 1724261).
user_pref("browser.tabs.remote.systemTriggeredAboutBlankAnywhere", true);
// Explicitly turn off fission so we don't accidentally use the wrong default
// value. This can be removed once harnesses and tasks assume fission by
// default.
user_pref("fission.autostart", false);
5 changes: 0 additions & 5 deletions testing/profiles/profileserver/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,3 @@
/* globals user_pref */
// Turn off budget throttling for the profile server
user_pref("dom.timeout.enable_budget_timer_throttling", false);

// Explicitly turn off fission so we don't accidentally use the wrong default
// value. This can be removed once harnesses and tasks assume fission by
// default.
user_pref("fission.autostart", false);
5 changes: 0 additions & 5 deletions testing/profiles/raptor/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,6 @@ user_pref('datareporting.healthreport.uploadEnabled', false);
// https://bugzilla.mozilla.org/show_bug.cgi?id=1706180
user_pref('toolkit.telemetry.initDelay', 99999999);

// Explicitly turn off fission so we don't accidentally use the wrong default
// value. This can be removed once harnesses and tasks assume fission by
// default.
user_pref("fission.autostart", false);

// disable autoplay for raptor tests
user_pref('media.autoplay.default', 5);
user_pref('media.autoplay.ask-permission', true);
Expand Down
4 changes: 0 additions & 4 deletions testing/profiles/reftest/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,3 @@ user_pref("toolkit.telemetry.initDelay", 99999999);
user_pref("toolkit.legacyUserProfileCustomizations.stylesheets", true);
// Use a light color-scheme unless explicitly overriden.
user_pref("layout.css.prefers-color-scheme.content-override", 1);
// Explicitly turn off fission so we don't accidentally use the wrong default
// value. This can be removed once harnesses and tasks assume fission by
// default.
user_pref("fission.autostart", false);
4 changes: 0 additions & 4 deletions testing/profiles/xpcshell/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,3 @@ user_pref("gfx.color_management.force_srgb", true);
user_pref("gfx.color_management.mode", 1);
// Don't enable remote tiles on new-tab pages in xpcshell
user_pref("browser.topsites.contile.enabled", false);
// Explicitly turn off fission so we don't accidentally use the wrong default
// value. This can be removed once harnesses and tasks assume fission by
// default.
user_pref("fission.autostart", false);
3 changes: 3 additions & 0 deletions testing/raptor/mach_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,9 @@ def run_raptor(command_context, **kwargs):
xre=True,
): # Equivalent to 'run_local' = True.
return 1
# Disable fission until geckoview supports fission by default.
# Need fission on Android? Use '--setpref fission.autostart=true'
kwargs["fission"] = False

# Remove mach global arguments from sys.argv to prevent them
# from being consumed by raptor. Treat any item in sys.argv
Expand Down
10 changes: 5 additions & 5 deletions testing/raptor/raptor/cmdline.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,11 +284,11 @@ def create_parser(mach_interface=False):
help="Device name of mobile device.",
)
add_arg(
"--enable-fission",
dest="enable_fission",
action="store_true",
default=False,
help="Enable Fission (site isolation) in Gecko.",
"--disable-fission",
dest="fission",
action="store_false",
default=True,
help="Disable Fission (site isolation) in Gecko.",
)
add_arg(
"--setpref",
Expand Down
4 changes: 2 additions & 2 deletions testing/raptor/raptor/perftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ def __init__(
"enable_control_server_wait": memory_test or cpu_test,
"e10s": e10s,
"device_name": device_name,
"enable_fission": extra_prefs.get("fission.autostart", False),
"fission": extra_prefs.get("fission.autostart", True),
"disable_perf_tuning": disable_perf_tuning,
"conditioned_profile": conditioned_profile,
"chimera": chimera,
Expand Down Expand Up @@ -386,7 +386,7 @@ def build_browser_profile(self):
LOG.info("Merging profile: {}".format(path))
self.profile.merge(path)

if self.config["extra_prefs"].get("fission.autostart", False):
if self.config["extra_prefs"].get("fission.autostart", True):
LOG.info("Enabling fission via browser preferences")
LOG.info("Browser preferences: {}".format(self.config["extra_prefs"]))
self.profile.set_preferences(self.config["extra_prefs"])
Expand Down
15 changes: 9 additions & 6 deletions testing/raptor/raptor/raptor.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,22 +47,25 @@ def main(args=sys.argv[1:]):

args.extra_prefs = parse_preferences(args.extra_prefs or [])

if args.enable_fission:
if args.enable_marionette_trace:
args.extra_prefs.update(
{
"fission.autostart": True,
"marionette.log.level": "Trace",
}
)

if args.enable_marionette_trace:
# if fission.autostart not specified by --setpref, update pref to match
# args.fission
if args.extra_prefs and "fission.autostart" not in args.extra_prefs:
args.extra_prefs.update(
{
"marionette.log.level": "Trace",
"fission.autostart": args.fission,
}
)

if args.extra_prefs and args.extra_prefs.get("fission.autostart", False):
args.enable_fission = True
# if fission.autostart=False then make sure fission disabled
if args.extra_prefs and not args.extra_prefs.get("fission.autostart", True):
args.fission = False

args.environment = dict(parse_key_value(args.environment or [], context="--setenv"))

Expand Down
9 changes: 5 additions & 4 deletions testing/talos/talos/cmdline.py
Original file line number Diff line number Diff line change
Expand Up @@ -312,10 +312,11 @@ def create_parser(mach_interface=False):
" Currently only supported in production.",
)
add_arg(
"--enable-fission",
action="store_true",
default=False,
help="Enable Fission (site isolation) in Gecko.",
"--disable-fission",
action="store_false",
dest="fission",
default=True,
help="Disable Fission (site isolation) in Gecko.",
)

add_logging_group(parser)
Expand Down
2 changes: 1 addition & 1 deletion testing/talos/talos/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ def get_browser_config(config):
"debugger": None,
"debugger_args": None,
"develop": False,
"enable_fission": False,
"fission": True,
"process": "",
"framework": "talos",
"repository": None,
Expand Down
4 changes: 1 addition & 3 deletions testing/talos/talos/ffsetup.py
Original file line number Diff line number Diff line change
Expand Up @@ -340,9 +340,7 @@ def __enter__(self):
raise
self._init_gecko_profile()
LOG.info("Browser initialized.")
LOG.info(
"Fission enabled: %s" % self.browser_config.get("enable_fission", False)
)
LOG.info("Fission enabled: %s" % self.browser_config.get("fission", True))
# remove ccov files before actual tests start
if self.browser_config.get("code_coverage", False):
# if the Firefox build was instrumented for ccov, initializing the browser
Expand Down
6 changes: 3 additions & 3 deletions testing/talos/talos/run_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,8 @@ def run_tests(config, browser_config):
if browser_config["subtests"]:
browser_config["preferences"]["talos.subtests"] = browser_config["subtests"]

if browser_config.get("enable_fission", False):
browser_config["preferences"]["fission.autostart"] = True
if not browser_config.get("fission", True):
browser_config["preferences"]["fission.autostart"] = False

browser_config["preferences"]["network.proxy.type"] = 2
browser_config["preferences"]["network.proxy.autoconfig_url"] = (
Expand Down Expand Up @@ -268,7 +268,7 @@ def run_tests(config, browser_config):
talos_results.add_extra_option("gecko-profile")

# differentiate fission vs non-fission results in perfherder
if browser_config.get("enable_fission", False):
if browser_config.get("fission", True):
talos_results.add_extra_option("fission")

# differentiate webrender from non-webrender results
Expand Down
Loading

0 comments on commit 618f09c

Please sign in to comment.