From f04b0d0efd5914ddbb65f6dd72cea10d1d911516 Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Sun, 1 Nov 2026 17:27:27 +0100 Subject: [PATCH] Commit gate: pytest 67.7 s serial -> 9.4 s, screens-fit hook 3.6 s -> 0.7 s - addopts: -n 4 --dist worksteal (fixed, not auto: 24 workers would break capped.sh's 4 GiB). CI's own -n auto still wins. - coverage core = sysmon (PEP 669): ~40% less tracing overhead, same data. - shutdown ceiling invariant: one plain-function patch of earned_units per day instead of a MagicMock per unit (~5000 per case); every day and every case still checked. - poke/web server tests: serve_forever(poll_interval=0.01), so shutdown() no longer waits 0.5 s per test. - autouse _no_real_ntp: check_clock_skew sent real UDP to pool.ntp.org from every verification test, and a lost reply cost a random test 5 s. - temperature hard-timeout test: pins the default to 5 s and times the mechanism at 0.1 s instead of waiting 5 real seconds. - _xvfb_reexec: Xvfb -displayfd instead of xvfb-run's fixed 3 s sleep. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/python-tests.yml | 4 +- pyproject.toml | 11 ++++ screen_locker/tests/_real_state_fixtures.py | 14 +++++ screen_locker/tests/test_poke_server.py | 3 +- .../tests/test_shutdown_ceiling_invariant.py | 17 +++--- screen_locker/tests/test_temperature.py | 18 +++++-- screen_locker/tests/test_web_server.py | 3 +- scripts/_xvfb_reexec.py | 54 ++++++++++++++----- 8 files changed, 96 insertions(+), 28 deletions(-) diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml index 5bd844d..b9f579f 100644 --- a/.github/workflows/python-tests.yml +++ b/.github/workflows/python-tests.yml @@ -58,11 +58,11 @@ jobs: enable-cache: true cache-dependency-glob: requirements.txt - # No apt step: the runner image already ships xvfb-run, and + # No apt step: the runner image already ships Xvfb, and # setup-python's interpreter has its own tkinter (python3-tk is for the # system python, which this job never runs). The apt-get update + # install cost ~35 s. If either ever goes missing the check does not - # skip -- _xvfb_reexec returns 1 without xvfb-run, and the import fails + # skip -- _xvfb_reexec returns 1 without Xvfb, and the import fails # without tkinter. - name: Install dependencies run: | diff --git a/pyproject.toml b/pyproject.toml index 7ee3de6..7de24cb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -231,6 +231,14 @@ addopts = [ "--cov-branch", "--cov-report=term-missing", "--cov-report=lcov", + # The commit gate runs plain `python -m pytest -q`; serial it took 26 s of + # its 60 s budget, 4 workers 9 s. Fixed, not `auto`: auto is 24 workers + # here, past capped.sh's 4 GiB. CI's own `-n auto` comes later and wins; + # `-n0` gets a serial run back. + "-n", + "4", + "--dist", + "worksteal", # A fixture-only plugin module, not conftest.py (already at the # repo's 400-line-per-file cap) -- see its own docstring. "-p", @@ -245,6 +253,9 @@ filterwarnings = [ [tool.coverage.run] source = ["screen_locker"] branch = true +# sys.monitoring (PEP 669) rather than the C tracer: test_shutdown_ceiling_ +# invariant.py alone measured 13 s vs 22 s, for the same line+branch data. +core = "sysmon" omit = [ "*/__pycache__/*", "*/tests/*", diff --git a/screen_locker/tests/_real_state_fixtures.py b/screen_locker/tests/_real_state_fixtures.py index f1eb7fc..f8f2954 100644 --- a/screen_locker/tests/_real_state_fixtures.py +++ b/screen_locker/tests/_real_state_fixtures.py @@ -200,3 +200,17 @@ def _no_real_night_lockdown(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> "NIGHT_LOCKDOWN_STAMP", tmp_path / "locked_at_epoch", ) + + +@pytest.fixture(autouse=True) +def _no_real_ntp() -> Iterator[None]: + """Never send a real NTP query to pool.ntp.org from a test. + + ``check_clock_skew`` (every phone and RunnerUp verification) opens a UDP + socket, which ``_block_real_network``'s ``create_connection`` stub cannot + reach. A lost reply cost a test the full 5 s ``_NTP_TIMEOUT``, at random. + Stubbed as "unreachable", what the real query answers on any OSError. + ``test_time_check`` drives the real query through its own import. + """ + with patch("screen_locker._time_check._query_ntp_offset", return_value=None): + yield diff --git a/screen_locker/tests/test_poke_server.py b/screen_locker/tests/test_poke_server.py index 06da4c1..fb48b41 100644 --- a/screen_locker/tests/test_poke_server.py +++ b/screen_locker/tests/test_poke_server.py @@ -52,7 +52,8 @@ def port(monkeypatch: pytest.MonkeyPatch) -> Iterator[int]: """A running PokeServer on 127.0.0.1:0; shut down afterwards.""" monkeypatch.setattr(socket, "create_connection", _REAL_CREATE_CONNECTION) server = PokeServer(("127.0.0.1", 0), _KEY) - thread = threading.Thread(target=server.serve_forever, daemon=True) + # poll_interval: shutdown() waits out one poll, 0.5 s per test by default. + thread = threading.Thread(target=server.serve_forever, args=(0.01,), daemon=True) thread.start() try: yield server.server_address[1] diff --git a/screen_locker/tests/test_shutdown_ceiling_invariant.py b/screen_locker/tests/test_shutdown_ceiling_invariant.py index b672b29..9a79111 100644 --- a/screen_locker/tests/test_shutdown_ceiling_invariant.py +++ b/screen_locker/tests/test_shutdown_ceiling_invariant.py @@ -161,12 +161,17 @@ def _live_pass(day: date, case: tuple[str, bool, int, str]) -> int: for _ in range(further if extra else 0): host._adjust_shutdown_time_by(extra, at=when) state: dict[str, Any] = {} - for gate in (e for e in registry_for(day) if is_gate(e)): - if gate.kind == "flat": - host._adjust_shutdown_time_by(first_minutes(gate, day), at=when) - continue - for units in range(1, (gate.max_units or 1) + 1): - with patch.object(_gate_bonus, "earned_units", return_value=units): + # One plain-function patch per day, not a MagicMock per unit: building + # ~5000 mocks was half of this file's runtime. ``answered`` is the ledger's + # unit count at the moment each counted credit is applied. + answered = [0] + with patch.object(_gate_bonus, "earned_units", lambda _e, _d: answered[0]): + for gate in (e for e in registry_for(day) if is_gate(e)): + if gate.kind == "flat": + host._adjust_shutdown_time_by(first_minutes(gate, day), at=when) + continue + for units in range(1, (gate.max_units or 1) + 1): + answered[0] = units apply_counted_bonus( state, partial(host._adjust_shutdown_time_by, at=when), diff --git a/screen_locker/tests/test_temperature.py b/screen_locker/tests/test_temperature.py index 43e4eaf..56fb92b 100644 --- a/screen_locker/tests/test_temperature.py +++ b/screen_locker/tests/test_temperature.py @@ -3,6 +3,8 @@ from __future__ import annotations import http.client +from inspect import signature +import threading import time from unittest.mock import MagicMock, patch import urllib.error @@ -134,16 +136,24 @@ class TestNothingEscapesTheFetch: assert result.timed_out is False def test_the_whole_check_is_bounded_by_the_hard_timeout(self) -> None: - """The user's requirement: at most 5s, then lock regardless.""" + """The user's requirement: at most 5s, then lock regardless. + + The bound is the default argument, so it is pinned to 5s here and the + mechanism is timed at 0.1s: the same guarantee without 5 real seconds. + """ + default = signature(fetch_current_temp_with_status).parameters["hard_timeout"] + assert default.default == HARD_TIMEOUT_SECONDS == 5.0 + hang = threading.Event() with patch( "screen_locker._temperature._fetch_current_temp_celsius_unbounded", - side_effect=lambda _city: time.sleep(30), + side_effect=lambda _city: hang.wait(30), ): started = time.monotonic() - result = fetch_current_temp_with_status("Warsaw") + result = fetch_current_temp_with_status("Warsaw", hard_timeout=0.1) elapsed = time.monotonic() - started + hang.set() # release the abandoned worker thread assert result.timed_out is True - assert elapsed <= HARD_TIMEOUT_SECONDS + 0.5 + assert elapsed <= 0.1 + 0.5 class TestSubmitBackground: diff --git a/screen_locker/tests/test_web_server.py b/screen_locker/tests/test_web_server.py index ca3daa6..f11d02c 100644 --- a/screen_locker/tests/test_web_server.py +++ b/screen_locker/tests/test_web_server.py @@ -53,7 +53,8 @@ def _running() -> Iterator[int]: The bound port. """ server = create_server("127.0.0.1", 0) - thread = threading.Thread(target=server.serve_forever, daemon=True) + # poll_interval: shutdown() waits out one poll, 0.5 s per test by default. + thread = threading.Thread(target=server.serve_forever, args=(0.01,), daemon=True) thread.start() try: yield server.server_address[1] diff --git a/scripts/_xvfb_reexec.py b/scripts/_xvfb_reexec.py index 34a17c2..4f3ffb6 100644 --- a/scripts/_xvfb_reexec.py +++ b/scripts/_xvfb_reexec.py @@ -5,7 +5,7 @@ to keep every file under the 250-line cap. Both had their own near-identical copy; they differ only in the sentinel env var and how the inner process is addressed, so both are parameters here. -Each script runs itself twice: the outer run starts ``xvfb-run`` and re-execs, +Each script runs itself twice: the outer run starts ``Xvfb`` and re-execs, the inner run (marked by the sentinel) does the real work against that display. Without the sentinel the inner process would start another Xvfb forever. @@ -24,7 +24,7 @@ _logger = logging.getLogger(__name__) # Comfortably bigger than every size under test, so each surface is sized by # the geometry the harness sets rather than clamped by the display. -_SCREEN_ARG = "-screen 0 1600x1200x24" +_SCREEN_ARG = ("-screen", "0", "1600x1200x24") def reexec_under_xvfb( @@ -46,27 +46,53 @@ def reexec_under_xvfb( script: Absolute path of the script file to run. Returns: - The inner process's exit code, or 1 if ``xvfb-run`` is unavailable. + The inner process's exit code, or 1 if ``Xvfb`` is unavailable or + never became ready. """ if (module is None) == (script is None): msg = "pass exactly one of module= or script=" raise ValueError(msg) - xvfb_run = shutil.which("xvfb-run") - if xvfb_run is None: + xvfb = shutil.which("Xvfb") + if xvfb is None: _logger.error( - "xvfb-run not found. Install it with: sudo pacman -S --needed " - "xorg-server-xvfb" + "Xvfb not found. Install it with: sudo pacman -S --needed xorg-server-xvfb" ) return 1 - env = dict(os.environ, **{inner_env: "1"}) repo_root = Path(__file__).resolve().parent.parent # Absolute argv, no shell. target = ["-m", module] if module is not None else [str(script)] - return subprocess.run( - [xvfb_run, "-a", "-s", _SCREEN_ARG, sys.executable, *target], - check=False, - cwd=repo_root, - env=env, - ).returncode + server, display = _start_xvfb(xvfb) + try: + if display is None: + _logger.error("Xvfb exited before it was ready to accept clients") + return 1 + env = dict(os.environ, DISPLAY=display, **{inner_env: "1"}) + env.pop("XAUTHORITY", None) # a throwaway server has no cookie to check + return subprocess.run( + [sys.executable, *target], check=False, cwd=repo_root, env=env + ).returncode + finally: + server.terminate() + server.wait(timeout=5) + + +def _start_xvfb(xvfb: str) -> tuple[subprocess.Popen[bytes], str | None]: + """Start Xvfb on a free display; return it and ``:N`` once it is ready. + + ``-displayfd`` makes Xvfb pick a free display and write its number only + once it accepts clients. ``xvfb-run`` instead sleeps a fixed 3 s after + starting it, which was most of this pre-commit hook's runtime. + """ + read_fd, write_fd = os.pipe() + server = subprocess.Popen( + [xvfb, "-displayfd", str(write_fd), "-nolisten", "tcp", *_SCREEN_ARG], + pass_fds=(write_fd,), + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ) + os.close(write_fd) + with os.fdopen(read_fd) as ready: + number = ready.readline().strip() + return server, f":{number}" if number else None