diff --git a/src/ffpuppet/core.py b/src/ffpuppet/core.py index fc19ca8..59b2c9d 100644 --- a/src/ffpuppet/core.py +++ b/src/ffpuppet/core.py @@ -26,13 +26,11 @@ CREATE_SUSPENDED = 0x00000004 -with suppress(ImportError): - from xvfbwrapper import Xvfb - from typing import TYPE_CHECKING from .bootstrapper import Bootstrapper from .checks import CheckLogContents, CheckLogSize, CheckMemoryUsage +from .display import DISPLAYS, DisplayMode from .exceptions import BrowserExecutionError, InvalidPrefs, LaunchError from .helpers import prepare_environment, wait_on_files from .minidump_parser import MDSW_URL, MinidumpParser @@ -87,13 +85,6 @@ class Reason(IntEnum): class FFPuppet: """FFPuppet manages launching and monitoring the browser process(es). This includes setting up the environment, collecting logs and some debugger support. - - Attributes: - debugger: Debugger to use. - headless: Headless mode to use. - use_profile: Path to existing user profile. - use_xvfb: Use Xvfb (DEPRECATED). - working_path: Path to use as base directory for temporary files. """ LAUNCH_TIMEOUT_MIN = 10 # minimum amount of time to wait for the browser to launch @@ -104,13 +95,12 @@ class FFPuppet: "_bin_path", "_checks", "_dbg", - "_headless", + "_display", "_launches", "_logs", "_proc_tree", "_profile_template", "_working_path", - "_xvfb", "marionette", "profile", "reason", @@ -119,42 +109,34 @@ class FFPuppet: def __init__( self, debugger: Debugger = Debugger.NONE, - headless: str | None = None, + display_mode: DisplayMode = DisplayMode.DEFAULT, use_profile: Path | None = None, - use_xvfb: bool = False, working_path: str | None = None, ) -> None: + """ + Args: + debugger: Debugger to use. + display_mode: Display mode to use. + use_profile: Path to existing profile to use. + working_path: Path to use as base directory for temporary files. + """ # tokens used to notify log scanner to kill the browser process self._abort_tokens: set[Pattern[str]] = set() self._bin_path: Path | None = None self._checks: list[CheckLogContents | CheckLogSize | CheckMemoryUsage] = [] self._dbg = debugger self._dbg_sanity_check(self._dbg) - self._headless = headless - self._launches = 0 # number of successful browser launches - self._logs = PuppetLogger(base_path=working_path) + self._display = DISPLAYS[display_mode]() + # number of successful browser launches + self._launches = 0 self._proc_tree: ProcessTree | None = None self._profile_template = use_profile - self._xvfb: Xvfb | None = None self._working_path = working_path self.marionette: int | None = None self.profile: Profile | None = None self.reason: Reason | None = Reason.CLOSED - if use_xvfb: - self._headless = "xvfb" - - if self._headless == "xvfb": - try: - self._xvfb = Xvfb(width=1280, height=1024) - except NameError: - self._logs.clean_up(ignore_errors=True) - raise OSError( - "Please install xvfbwrapper (Only supported on Linux)" - ) from None - self._xvfb.start() - else: - assert self._headless in (None, "default") + self._logs = PuppetLogger(base_path=working_path) def __enter__(self) -> FFPuppet: return self @@ -322,8 +304,7 @@ def build_launch_cmd( if bin_path.lower().endswith(".py"): cmd.append(executable) cmd += [bin_path, "-new-instance"] - if self._headless == "default": - cmd.append("-headless") + cmd.extend(self._display.args) if self.profile is not None: cmd += ["-profile", str(self.profile)] @@ -437,16 +418,13 @@ def clean_up(self) -> None: Returns: None """ + self._display.close() if self._launches < 0: LOG.debug("clean_up() call ignored") return LOG.debug("clean_up() called") self.close(force_close=True) self._logs.clean_up(ignore_errors=True) - # close Xvfb - if self._xvfb is not None: - self._xvfb.stop() - self._xvfb = None # at this point everything should be cleaned up assert self.reason is not None assert self._logs.closed @@ -738,8 +716,7 @@ def launch( # https://developer.gimp.org/api/2.0/glib/glib-running.html#G_DEBUG env_mod["G_DEBUG"] = "gc-friendly" env_mod["MOZ_CRASHREPORTER_DISABLE"] = "1" - if self._headless == "xvfb": - env_mod["MOZ_ENABLE_WAYLAND"] = "0" + env_mod.update(self._display.env) # create a profile self.profile = Profile( diff --git a/src/ffpuppet/display.py b/src/ffpuppet/display.py new file mode 100644 index 0000000..77af243 --- /dev/null +++ b/src/ffpuppet/display.py @@ -0,0 +1,101 @@ +# 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/. +"""ffpuppet display module""" + +from __future__ import annotations + +from contextlib import suppress +from enum import Enum, auto, unique +from logging import getLogger +from platform import system +from types import MappingProxyType + +with suppress(ImportError): + from xvfbwrapper import Xvfb + +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from collections.abc import Iterable, Mapping + + +LOG = getLogger(__name__) + + +@unique +class DisplayMode(Enum): + """Supported display modes.""" + + DEFAULT = auto() + HEADLESS = auto() + if system() == "Linux": + XVFB = auto() + + +class Display: + """Default display mode. + + Attributes: + args: Extra Firefox command line arguments to use. + env: Extra environment variables to use. + mode: DisplayMode enum name. + """ + + __slots__ = ("args", "env", "mode") + + def __init__(self) -> None: + self.args: Iterable[str] = () + self.env: Mapping[str, str] = MappingProxyType({}) + self.mode: str = DisplayMode.DEFAULT.name + + def close(self) -> None: + """Perform any required operations to shutdown and cleanup. + + Args: + None + + Returns: + None + """ + + +class HeadlessDisplay(Display): + """Headless display mode.""" + + def __init__(self) -> None: + super().__init__() + self.args = ("-headless",) + self.mode = DisplayMode.HEADLESS.name + + +class XvfbDisplay(Display): + """Xvfb display mode.""" + + __slots__ = ("_xvfb",) + + def __init__(self) -> None: + super().__init__() + self.env = MappingProxyType({"MOZ_ENABLE_WAYLAND": "0"}) + self.mode = DisplayMode.XVFB.name + try: + self._xvfb: Xvfb | None = Xvfb(width=1280, height=1024) + except NameError: + LOG.error("Missing xvfbwrapper") + raise + self._xvfb.start() + + def close(self) -> None: + if self._xvfb is not None: + self._xvfb.stop() + self._xvfb = None + + +_displays: dict[DisplayMode, type[Display]] = { + DisplayMode.DEFAULT: Display, + DisplayMode.HEADLESS: HeadlessDisplay, +} +if system() == "Linux": + _displays[DisplayMode.XVFB] = XvfbDisplay + +DISPLAYS = MappingProxyType(_displays) diff --git a/src/ffpuppet/main.py b/src/ffpuppet/main.py index 2886fc0..1205309 100644 --- a/src/ffpuppet/main.py +++ b/src/ffpuppet/main.py @@ -15,6 +15,7 @@ from .bootstrapper import Bootstrapper from .core import Debugger, FFPuppet, Reason +from .display import DisplayMode from .exceptions import BrowserExecutionError from .helpers import certutil_available, certutil_find from .profile import Profile @@ -116,6 +117,12 @@ def parse_args(argv: list[str] | None = None) -> Namespace: type=Path, help="Install trusted certificates.", ) + cfg_group.add_argument( + "--display", + choices=sorted(x.name.lower() for x in DisplayMode), + default=DisplayMode.DEFAULT.name, + help="Display mode.", + ) cfg_group.add_argument( "-e", "--extension", @@ -124,17 +131,6 @@ def parse_args(argv: list[str] | None = None) -> Namespace: help="Install extensions. Specify the path to the xpi or the directory " "containing the unpacked extension.", ) - headless_choices = ["default"] - if system() == "Linux": - headless_choices.append("xvfb") - cfg_group.add_argument( - "--headless", - choices=headless_choices, - const="default", - default=None, - nargs="?", - help="Headless mode. 'default' uses browser's built-in headless mode.", - ) cfg_group.add_argument( "--marionette", const=0, @@ -160,14 +156,6 @@ def parse_args(argv: list[str] | None = None) -> Namespace: cfg_group.add_argument( "-u", "--url", help="Server URL or path to local file to load." ) - if system() == "Linux": - cfg_group.add_argument( - "--xvfb", - action="store_true", - help="DEPRECATED! Please use '--headless xvfb'", - ) - else: - cfg_group.set_defaults(xvfb=False) report_group = parser.add_argument_group("Issue Detection & Reporting") report_group.add_argument( @@ -290,19 +278,12 @@ def parse_args(argv: list[str] | None = None) -> Namespace: args.memory *= 1_048_576 if args.prefs is not None and not args.prefs.is_file(): parser.error(f"Invalid prefs.js file '{args.prefs}'") - if args.xvfb: - LOG.warning("'--xvfb' is DEPRECATED. Please use '--headless xvfb'") - args.headless = "xvfb" return args def main(argv: list[str] | None = None) -> None: - """ - FFPuppet main entry point - - Run with --help for usage - """ + """FFPuppet main entry point.""" args = parse_args(argv) # set output verbosity if args.log_level == DEBUG: @@ -315,7 +296,7 @@ def main(argv: list[str] | None = None) -> None: ffp = FFPuppet( debugger=args.debugger, - headless=args.headless, + display_mode=DisplayMode[args.display.upper()], use_profile=args.profile, ) for a_token in args.abort_token: diff --git a/src/ffpuppet/test_display.py b/src/ffpuppet/test_display.py new file mode 100644 index 0000000..44a3bad --- /dev/null +++ b/src/ffpuppet/test_display.py @@ -0,0 +1,28 @@ +# 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/. +"""display.py tests""" +from platform import system + +from pytest import mark, raises + +from .display import DISPLAYS, DisplayMode, XvfbDisplay + + +@mark.parametrize("mode", tuple(x for x in DisplayMode)) +def test_displays(mocker, mode): + """test Displays()""" + if system() == "Linux": + mocker.patch("ffpuppet.display.Xvfb", autospec=True) + display = DISPLAYS[mode]() + assert display + assert display.mode == mode.name + display.close() + + +@mark.skipif(system() != "Linux", reason="Only supported on Linux") +def test_xvfb_missing_deps(mocker): + """test XvfbDisplay() missing deps""" + mocker.patch("ffpuppet.display.Xvfb", side_effect=NameError("test")) + with raises(NameError): + XvfbDisplay() diff --git a/src/ffpuppet/test_ffpuppet.py b/src/ffpuppet/test_ffpuppet.py index 444c9d4..f0faad0 100644 --- a/src/ffpuppet/test_ffpuppet.py +++ b/src/ffpuppet/test_ffpuppet.py @@ -320,39 +320,7 @@ def test_ffpuppet_12(): ffp.close() -def test_ffpuppet_13(mocker): - """test launching under Xvfb""" - mocker.patch("ffpuppet.core.Popen", autospec=True) - mocker.patch("ffpuppet.core.ProcessTree", autospec=True) - fake_bts = mocker.patch("ffpuppet.core.Bootstrapper", autospec=True) - fake_bts.create.return_value.location = "http://test:123" - fake_system = mocker.patch("ffpuppet.core.system", autospec=True) - is_linux = system() == "Linux" - fake_xvfb = mocker.patch( - "ffpuppet.core.Xvfb", autospec=is_linux, create=not is_linux - ) - # success - fake_system.return_value = "Linux" - with FFPuppet(headless="xvfb") as ffp: - ffp.launch(TESTFF_BIN) - assert fake_xvfb.call_count == 1 - assert fake_xvfb.return_value.start.call_count == 1 - fake_xvfb.reset_mock() - # success - legacy - fake_system.return_value = "Linux" - with FFPuppet(use_xvfb=True): - pass - assert fake_xvfb.call_count == 1 - assert fake_xvfb.return_value.start.call_count == 1 - fake_xvfb.reset_mock() - # not installed - fake_xvfb.side_effect = NameError - with raises(OSError, match="Please install xvfbwrapper"): - FFPuppet(headless="xvfb") - assert fake_xvfb.start.call_count == 0 - - -def test_ffpuppet_14(tmp_path): +def test_ffpuppet_13(tmp_path): """test passing a file and a non existing file to launch() via location""" with FFPuppet() as ffp: with raises(OSError, match="Cannot find"): @@ -393,7 +361,7 @@ def test_ffpuppet_14(tmp_path): (Debugger.VALGRIND, b"valgrind", b"valgrind-99.0"), ], ) -def test_ffpuppet_15(mocker, tmp_path, debugger, dbg_bin, version): +def test_ffpuppet_14(mocker, tmp_path, debugger, dbg_bin, version): """test launching with debuggers""" mocker.patch("ffpuppet.core.check_output", autospec=True, return_value=version) mocker.patch("ffpuppet.core.Popen", autospec=True) @@ -412,7 +380,7 @@ def test_ffpuppet_15(mocker, tmp_path, debugger, dbg_bin, version): assert b"[ffpuppet] Reason code:" in log_data -def test_ffpuppet_16(tmp_path): +def test_ffpuppet_15(tmp_path): """test calling save_logs() before close()""" with FFPuppet() as ffp, HTTPTestServer() as srv: ffp.launch(TESTFF_BIN, location=srv.get_addr()) @@ -420,7 +388,7 @@ def test_ffpuppet_16(tmp_path): ffp.save_logs(tmp_path / "logs") -def test_ffpuppet_17(tmp_path): +def test_ffpuppet_16(tmp_path): """test detecting invalid prefs file""" prefs = tmp_path / "prefs.js" prefs.write_bytes(b"//fftest_invalid_js\n") @@ -432,7 +400,7 @@ def test_ffpuppet_17(tmp_path): ffp.launch(TESTFF_BIN, location=srv.get_addr(), prefs_js=prefs) -def test_ffpuppet_18(): +def test_ffpuppet_17(): """test log_length()""" with FFPuppet() as ffp: assert ffp.log_length("INVALID") is None @@ -450,7 +418,7 @@ def test_ffpuppet_18(): assert ffp.log_length("stderr") is None -def test_ffpuppet_19(): +def test_ffpuppet_18(): """test running multiple instances in parallel""" # use test pool size of 10 with HTTPTestServer() as srv: @@ -466,7 +434,7 @@ def test_ffpuppet_19(): ffp.clean_up() -def test_ffpuppet_20(tmp_path): +def test_ffpuppet_19(tmp_path): """test hitting log size limit""" prefs = tmp_path / "prefs.js" prefs.write_bytes(b"//fftest_big_log\n") @@ -490,7 +458,7 @@ def test_ffpuppet_20(tmp_path): ) -def test_ffpuppet_21(tmp_path): +def test_ffpuppet_20(tmp_path): """test collecting and cleaning up ASan logs""" with FFPuppet() as ffp: ffp.launch(TESTFF_BIN) @@ -549,7 +517,7 @@ def test_ffpuppet_21(tmp_path): @mark.parametrize("mdsw_available", [True, False]) -def test_ffpuppet_22(mocker, tmp_path, mdsw_available): +def test_ffpuppet_21(mocker, tmp_path, mdsw_available): """test multiple minidumps""" def _fake_create_log(src, filename, _timeout: int = 90): @@ -589,7 +557,7 @@ def _fake_create_log(src, filename, _timeout: int = 90): assert not any(logs.glob("log_minidump_*")) -def test_ffpuppet_23(tmp_path): +def test_ffpuppet_22(tmp_path): """test using a readonly prefs.js and extension""" prefs = tmp_path / "prefs.js" prefs.touch() @@ -605,7 +573,7 @@ def test_ffpuppet_23(tmp_path): assert not prof_path.is_dir() -def test_ffpuppet_24(mocker, tmp_path): +def test_ffpuppet_23(mocker, tmp_path): """test _crashreports()""" mocker.patch( "ffpuppet.core.check_output", autospec=True, return_value=b"valgrind-99.0" @@ -667,21 +635,13 @@ def close(self, force_close=False): assert not ffp._logs.watching -def test_ffpuppet_25(mocker, tmp_path): +def test_ffpuppet_24(mocker, tmp_path): """test build_launch_cmd()""" with FFPuppet() as ffp: cmd = ffp.build_launch_cmd("bin_path", ["test"]) assert len(cmd) == 3 assert cmd[0] == "bin_path" assert cmd[-1] == "test" - assert "-headless" not in cmd - # headless - ffp._headless = "default" - cmd = ffp.build_launch_cmd("bin_path", ["test"]) - assert len(cmd) == 4 - assert cmd[0] == "bin_path" - assert cmd[-1] == "test" - assert "-headless" in cmd # GDB ffp._dbg = Debugger.GDB cmd = ffp.build_launch_cmd("bin_path") @@ -715,7 +675,7 @@ def test_ffpuppet_25(mocker, tmp_path): assert cmd[0] == "valgrind" -def test_ffpuppet_26(): +def test_ffpuppet_25(): """test cpu_usage()""" with FFPuppet() as ffp: assert not any(ffp.cpu_usage()) @@ -728,7 +688,7 @@ def test_ffpuppet_26(): assert ffp.wait(timeout=10) -def test_ffpuppet_27(mocker): +def test_ffpuppet_26(mocker): """test _dbg_sanity_check()""" fake_system = mocker.patch("ffpuppet.core.system", autospec=True) fake_chkout = mocker.patch("ffpuppet.core.check_output", autospec=True) @@ -787,7 +747,7 @@ def test_ffpuppet_27(mocker): FFPuppet._dbg_sanity_check(Debugger.VALGRIND) -def test_ffpuppet_28(mocker, tmp_path): +def test_ffpuppet_27(mocker, tmp_path): """test FFPuppet.close() setting reason""" class StubbedProc(FFPuppet): @@ -886,7 +846,7 @@ def launch(self): assert fake_wait_files.call_count == 0 -def test_ffpuppet_29(): +def test_ffpuppet_28(): """test ignoring benign sanitizer logs""" with FFPuppet() as ffp: ffp.launch(TESTFF_BIN) @@ -911,7 +871,7 @@ def test_ffpuppet_29(): (False, FileNotFoundError), ], ) -def test_ffpuppet_30(mocker, tmp_path, bin_exists, expect_exc): +def test_ffpuppet_29(mocker, tmp_path, bin_exists, expect_exc): """test Popen failure during launch""" bin_fake = tmp_path / "fake_bin" if bin_exists: @@ -929,7 +889,7 @@ def test_ffpuppet_30(mocker, tmp_path, bin_exists, expect_exc): @mark.skipif(system() != "Windows", reason="Only supported on Windows") -def test_ffpuppet_31(mocker): +def test_ffpuppet_30(mocker): """test FFPuppet.launch() config_job_object code path""" mocker.patch("ffpuppet.core.ProcessTree", autospec=True) fake_bts = mocker.patch("ffpuppet.core.Bootstrapper", autospec=True) @@ -949,7 +909,7 @@ def test_ffpuppet_31(mocker): assert resume_suspended.mock_calls[0] == mocker.call(789) -def test_ffpuppet_32(mocker): +def test_ffpuppet_31(mocker): """test FFPuppet.dump_coverage()""" with FFPuppet() as ffp: ffp._proc_tree = mocker.Mock(spec_set=ProcessTree) @@ -965,7 +925,7 @@ def test_ffpuppet_32(mocker): ffp._proc_tree = None -def test_ffpuppet_33(): +def test_ffpuppet_32(): """test FFPuppet.launch() with marionette""" with FFPuppet() as ffp, HTTPTestServer() as srv: assert ffp.marionette is None @@ -976,7 +936,7 @@ def test_ffpuppet_33(): @mark.parametrize("port", [0, 123]) -def test_ffpuppet_34(mocker, port): +def test_ffpuppet_33(mocker, port): """test FFPuppet.launch() with marionette failure""" fake_bts = mocker.patch("ffpuppet.core.Bootstrapper", autospec=True) fake_bts.create.return_value.location = "" diff --git a/src/ffpuppet/test_main.py b/src/ffpuppet/test_main.py index 617e88c..b436fad 100644 --- a/src/ffpuppet/test_main.py +++ b/src/ffpuppet/test_main.py @@ -122,26 +122,6 @@ def test_parse_args_01(capsys, mocker, tmp_path): assert parse_args([str(fake_bin)]) -def test_parse_args_02(mocker, tmp_path): - """test parse_args() - headless""" - fake_system = mocker.patch("ffpuppet.main.system", autospec=True) - fake_bin = tmp_path / "fake.bin" - fake_bin.touch() - # no headless - assert parse_args([str(fake_bin)]).headless is None - # headless (default) - assert parse_args([str(fake_bin), "--headless"]).headless == "default" - # headless via Xvfb - fake_system.return_value = "Linux" - assert parse_args([str(fake_bin), "--headless", "xvfb"]).headless == "xvfb" - # --xvfb flag - assert parse_args([str(fake_bin), "--xvfb"]).headless == "xvfb" - # headless Xvfb unsupported - fake_system.return_value = "Windows" - with raises(SystemExit): - parse_args([str(fake_bin), "--headless", "xvfb"]) - - def test_dump_to_console_01(tmp_path): """test dump_to_console()""" # call with no logs