diff --git a/docs/superpowers/evidence/pre-commit-pytest-batch-2026-05-14.json b/docs/superpowers/evidence/pre-commit-pytest-batch-2026-05-14.json new file mode 100644 index 0000000..775c9ef --- /dev/null +++ b/docs/superpowers/evidence/pre-commit-pytest-batch-2026-05-14.json @@ -0,0 +1,39 @@ +{ + "intent": "Make pre-commit run a single batched, parallelized pytest invocation and eliminate the 15s real time.sleep() calls in steam_backlog_enforcer tests, so the pytest-coverage hook is fast enough to run on every commit instead of only on push.", + "scope": [ + "meta/.pre-commit-config.yaml", + "scripts/pytest_changed_packages.py", + "python_pkg/steam_backlog_enforcer/tests/conftest.py", + "Non-goal: changing what packages are covered or the 100% threshold", + "Non-goal: touching the .git/hooks/pre-push 4 GiB cgroup wrapper" + ], + "changes": [ + "Move pytest-coverage hook from stages:[pre-push] to stages:[pre-commit] in meta/.pre-commit-config.yaml.", + "Rewrite scripts/pytest_changed_packages.py: drop per-package subprocess loop and gc/tempfile bookkeeping; run a single batched pytest call with -n auto and one --cov flag per affected python_pkg subpackage; wrap in systemd-run --user --scope -p MemoryMax=4G -p MemorySwapMax=0 when available.", + "Add autouse fixture _no_real_sleep in steam_backlog_enforcer/tests/conftest.py that patches time.sleep in game_install / library_hider / steam_api / _enforce_loop. Fixes 3 TestFinalizeCompletion tests that were each waiting 15s in real time inside _ensure_steam_running()." + ], + "verification": [ + { + "command": "python -m pytest python_pkg/steam_backlog_enforcer/tests --no-cov -n auto -q", + "result": "pass", + "evidence": "535 passed in 5.61s (was 33.97s on main; ~6x speedup). Slowest test now 4.46s; previous top-3 at 15-18s gone." + }, + { + "command": "time python scripts/pytest_changed_packages.py python_pkg/shared/x.py python_pkg/wake_alarm/x.py python_pkg/brother_printer/x.py python_pkg/random_jpg/x.py python_pkg/screen_locker/x.py", + "result": "pass", + "evidence": "732 passed in 1.37s, 668% CPU; coverage 100%." + }, + { + "command": "pre-commit run --files python_pkg/steam_backlog_enforcer/tests/conftest.py scripts/pytest_changed_packages.py meta/.pre-commit-config.yaml", + "result": "pass", + "evidence": "All hooks passed including pytest-coverage at 100% on steam_backlog_enforcer." + } + ], + "risks": [ + "Tests that intentionally observe time.sleep behavior in the affected modules must override the autouse patch with their own with-patch (existing patterns already do).", + "systemd-run is optional; script falls back to direct pytest invocation if unavailable." + ], + "rollback": [ + "git revert the commit. Re-running pre-commit must show pytest-coverage hook back on pre-push stage and steam_backlog_enforcer test suite back to ~34s wall time." + ] +} diff --git a/meta/.pre-commit-config.yaml b/meta/.pre-commit-config.yaml index 186ab17..81e12a8 100644 --- a/meta/.pre-commit-config.yaml +++ b/meta/.pre-commit-config.yaml @@ -220,7 +220,7 @@ repos: # =========================================================================== # PYTEST + COVERAGE - Run tests and enforce 100% code coverage # Only tests for subpackages with changed files are run (see script). - # Runs on push only (slow); use --hook-stage pre-push to run manually. + # Uses pytest-xdist (-n auto) to parallelize across all CPUs. # =========================================================================== - repo: local hooks: @@ -230,7 +230,7 @@ repos: language: system types: [python] pass_filenames: true - stages: [pre-push] + stages: [pre-commit] # =========================================================================== # VULTURE - Dead code detection (disabled - doesn't work well with pre-commit) diff --git a/python_pkg/steam_backlog_enforcer/tests/conftest.py b/python_pkg/steam_backlog_enforcer/tests/conftest.py index 689ad37..cb72d86 100644 --- a/python_pkg/steam_backlog_enforcer/tests/conftest.py +++ b/python_pkg/steam_backlog_enforcer/tests/conftest.py @@ -116,3 +116,21 @@ def _block_real_subprocesses() -> Iterator[None]: ), ): yield + + +@pytest.fixture(autouse=True) +def _no_real_sleep() -> Iterator[None]: + """No-op every ``time.sleep`` used by the package. + + Several modules call ``time.sleep`` for Steam-launch / install-retry / + rate-limit pacing. Individual tests that need to observe sleep + behaviour can override these patches inside their own ``with`` block. + """ + noop = MagicMock() + with ( + patch("python_pkg.steam_backlog_enforcer.game_install.time.sleep", noop), + patch("python_pkg.steam_backlog_enforcer.library_hider.time.sleep", noop), + patch("python_pkg.steam_backlog_enforcer.steam_api.time.sleep", noop), + patch("python_pkg.steam_backlog_enforcer._enforce_loop.time.sleep", noop), + ): + yield diff --git a/scripts/pytest_changed_packages.py b/scripts/pytest_changed_packages.py index 3f176bf..ac6bb37 100755 --- a/scripts/pytest_changed_packages.py +++ b/scripts/pytest_changed_packages.py @@ -3,7 +3,8 @@ Used as a pre-commit hook entry point. Receives staged file paths as arguments, determines which ``python_pkg//`` directories are -affected, and runs pytest scoped to just those subpackages. +affected, and runs pytest scoped to just those subpackages in a single +invocation parallelized with pytest-xdist (-n auto). If a file outside any subpackage is changed (e.g. ``python_pkg/conftest.py``), all tests are run as a fallback. @@ -11,30 +12,21 @@ all tests are run as a fallback. from __future__ import annotations -import gc import os from pathlib import Path, PurePosixPath import shutil import subprocess import sys -import tempfile _MIN_SUBPACKAGE_DEPTH = 2 -_PER_PACKAGE_MEM = "2G" +_TOTAL_MEM = "4G" _RUN_ALL_TRIGGERS = frozenset({"conftest.py", "__init__.py"}) def _affected_packages(files: list[str]) -> set[str] | None: - """Return subpackage names touched by *files*, or ``None`` for all. - - Returns ``None`` only when a *currently existing* root-level - ``python_pkg/`` shared file (``conftest.py`` / ``__init__.py``) is - modified. Stray root-level files from rewritten history, or paths - pointing at deleted/non-existent subpackages, are silently skipped so - pre-push doesn't run the whole suite for irrelevant diffs. - """ + """Return subpackage names touched by *files*, or ``None`` for all.""" packages: set[str] = set() root = Path("python_pkg") for path in files: @@ -52,9 +44,9 @@ def _affected_packages(files: list[str]) -> set[str] | None: return packages -def _build_pytest_command(packages: set[str] | None) -> list[str]: - """Build the pytest invocation for the given *packages*.""" - base = [ +def _build_pytest_command(packages: set[str]) -> list[str]: + """Build a single pytest invocation covering *packages* in parallel.""" + cmd = [ sys.executable, "-m", "pytest", @@ -62,23 +54,16 @@ def _build_pytest_command(packages: set[str] | None) -> list[str]: "--cov-report=term-missing", "--cov-fail-under=100", "-q", - ] - if packages is None or not packages: - # Fallback: run everything. - return [*base, "--cov=python_pkg"] - - # Override addopts from pyproject.toml to remove the global --cov=python_pkg - # that would widen coverage measurement to the entire tree. - cmd = [ - *base, + "-n", + "auto", + # Override addopts from pyproject.toml to drop the global + # --cov=python_pkg that would widen coverage to the entire tree. "-o", - "addopts=-v --strict-markers --strict-config -ra", + "addopts=--strict-markers --strict-config -ra", ] for pkg in sorted(packages): cmd.extend(["--cov", f"python_pkg/{pkg}"]) - for pkg in sorted(packages): - test_dir = f"python_pkg/{pkg}/tests" - cmd.append(test_dir) + cmd.extend(f"python_pkg/{pkg}/tests" for pkg in sorted(packages)) return cmd @@ -90,10 +75,8 @@ def main() -> int: packages = _affected_packages(files) - # When many packages are affected, run each one in a separate subprocess - # to avoid accumulating memory across all test suites (OOM prevention). if packages is None: - # Discover all subpackages that have a tests/ directory. + # Root-level python_pkg file changed -> discover every subpackage. packages = { entry.name for entry in Path("python_pkg").iterdir() @@ -103,38 +86,21 @@ def main() -> int: if not packages: return 0 - # Run each package in its own subprocess so memory is freed between runs. - # Wrap each in a nested cgroup with MemorySwapMax=0 so it gets killed - # instantly at the limit instead of thrashing swap/zram. - use_cgroup = shutil.which("systemd-run") is not None - for pkg in sorted(packages): - # Each package gets its own isolated coverage data file so parallel - # cgroup subprocesses never stomp on each other's SQLite DB. - with tempfile.NamedTemporaryFile( - prefix=f".coverage_{pkg}_", dir=".", delete=False - ) as tmp: - cov_file = tmp.name - try: - cmd = _build_pytest_command({pkg}) - env = {**os.environ, "COVERAGE_FILE": cov_file} - if use_cgroup: - cmd = [ - "systemd-run", - "--user", - "--scope", - "-p", - f"MemoryMax={_PER_PACKAGE_MEM}", - "-p", - "MemorySwapMax=0", - *cmd, - ] - result = subprocess.run(cmd, check=False, env=env) - finally: - Path(cov_file).unlink(missing_ok=True) - gc.collect() - if result.returncode != 0: - return result.returncode - return 0 + cmd = _build_pytest_command(packages) + if shutil.which("systemd-run") is not None: + cmd = [ + "systemd-run", + "--user", + "--scope", + "--quiet", + "--collect", + "-p", + f"MemoryMax={_TOTAL_MEM}", + "-p", + "MemorySwapMax=0", + *cmd, + ] + return subprocess.run(cmd, check=False, env=os.environ).returncode if __name__ == "__main__":