diff --git a/docs/superpowers/contracts/brother-printer-queue-fix-split-2026-08-16.json b/docs/superpowers/contracts/brother-printer-queue-fix-split-2026-08-16.json new file mode 100644 index 00000000..47c99ff3 --- /dev/null +++ b/docs/superpowers/contracts/brother-printer-queue-fix-split-2026-08-16.json @@ -0,0 +1,19 @@ +{ + "title": "Split brother_printer/cups_queue.py under the 250-line cap", + "objective": "Bring python_pkg/brother_printer/cups_queue.py (424 lines) under the repo-wide 250-line cap by moving the queue-status display and the interactive fix prompt into a new _queue_fix.py, chosen so the CUPS-mutating action primitives stay where their tests patch subprocess and time.", + "acceptance_criteria": [ + "cups_queue.py and the new _queue_fix.py are each at most 250 lines.", + "The brother_printer suite still reports exactly 402 passing tests -- the same count as before the split, proving no test was dropped or silently skipped.", + "The package keeps 100% branch coverage of the brother_printer source modules.", + "_cups_enable_printer, _cups_cancel_all_jobs, _cups_cancel_job and _cups_restart_service remain defined in cups_queue.py, so test_cups_queue.py's patches of subprocess, time, shutil and Path on that module still intercept every call that would otherwise reach the live CUPS daemon.", + "No assertion is changed and no test is deleted, skipped or weakened; the only test edits are the MOD constant and the import module path following the moved code.", + "No suppression is added and no coverage exclusion is introduced." + ], + "out_of_scope": [ + "Changing what any test asserts, or what any display string contains.", + "Splitting the brother_printer test files, which are separate commits.", + "Moving the _cups_* primitives, which would let an unpatched subprocess.run restart the real CUPS service during the test run.", + "Adding a re-export shim in cups_queue for display_cups_queue_status; importers point at the new module directly to keep the dependency direction one-way." + ], + "verifier": "PYTHONPATH=. python3 -m pytest python_pkg/brother_printer/tests -q (expects exactly 402 passed), plus pre-commit run --files on the touched files, plus bash ~/utils/scripts/check_file_length.sh --all for the cap." +} diff --git a/docs/superpowers/evidence/brother-printer-queue-fix-split-2026-08-16.json b/docs/superpowers/evidence/brother-printer-queue-fix-split-2026-08-16.json new file mode 100644 index 00000000..78c28ac3 --- /dev/null +++ b/docs/superpowers/evidence/brother-printer-queue-fix-split-2026-08-16.json @@ -0,0 +1,47 @@ +{ + "intent": "Bring brother_printer/cups_queue.py under the 250-line cap by moving the queue-status display and the interactive fix prompt into their own module, with no behaviour change.", + "scope": [ + "python_pkg/brother_printer/_queue_fix.py (new)", + "python_pkg/brother_printer/cups_queue.py", + "python_pkg/brother_printer/display.py (import path only)", + "python_pkg/brother_printer/tests/test_cups_queue_part2.py, test_cups_queue_part3.py (MOD and import path only)", + "Non-goal: no assertion changed, no test deleted, skipped or weakened" + ], + "changes": [ + "Moved display_cups_queue_status, _offer_queue_fix, the five _dwj_* choice handlers, the _DWJ_ACTIONS table and the four _handle_* dispatchers verbatim into python_pkg/brother_printer/_queue_fix.py.", + "The _cups_* action primitives stay in cups_queue.py on purpose and _queue_fix imports the three it needs. test_cups_queue.py patches subprocess and time on cups_queue, and those primitives are their only consumers, so moving them would have left subprocess.run unpatched and able to restart the real CUPS service during the suite.", + "cups_queue.py dropped the imports that only the moved code used (BOLD, CYAN, GREEN, YELLOW, _prompt and the whole TYPE_CHECKING/Callable block); 424 -> 245 lines.", + "display.py now imports display_cups_queue_status from _queue_fix and get_cups_queue_status from cups_queue, keeping the dependency direction cups_queue <- _queue_fix <- display with no re-export shim and no cycle.", + "test_cups_queue_part2.py and test_cups_queue_part3.py had their MOD constant and import module repointed at _queue_fix. test_cups_queue.py needed no change at all, which is the signal the seam matched the existing test boundary." + ], + "verification": [ + { + "command": "PYTHONPATH=. python3 -m pytest python_pkg/brother_printer/tests -q", + "result": "pass", + "evidence": "402 passed before the split and 402 passed after it - identical pass count, not merely a green run." + }, + { + "command": "PYTHONPATH=. python3 -c \"from python_pkg.brother_printer import _queue_fix as q, cups_queue as c, display as d; q._cups_enable_printer; q._cups_restart_service; q._prompt; q._handle_disabled_with_jobs; c.subprocess; c.time; c.shutil; c.Path; d.display_cups_queue_status\"", + "result": "pass", + "evidence": "runtime probe OK - every name the three test modules patch is still a real attribute of the module they name, so ruff's TC001 rewrite trap did not fire." + }, + { + "command": "wc -l python_pkg/brother_printer/cups_queue.py python_pkg/brother_printer/_queue_fix.py", + "result": "pass", + "evidence": "245 and 203 lines, both under the 250-line cap." + }, + { + "command": "pre-commit run --files ", + "result": "pass", + "evidence": "ruff, ruff format, mypy, pylint, bandit and pytest-with-coverage all pass on the touched files." + } + ], + "risks": [ + "mock.patch pins names to a module. The dangerous direction here was silent rather than red: an unpatched _cups_restart_service would have run systemctl restart cups for real. Mitigated by leaving all four _cups_* primitives in cups_queue.py, which the runtime probe and the unchanged test_cups_queue.py both confirm.", + "display.py imports from both modules now; a future move of get_cups_queue_status into _queue_fix would create a cycle." + ], + "rollback": [ + "git revert the commit; cups_queue.py returns to 424 lines and _queue_fix.py disappears.", + "After rollback, re-run the package suite and confirm it still reports 402 passed." + ] +} diff --git a/python_pkg/brother_printer/_queue_fix.py b/python_pkg/brother_printer/_queue_fix.py new file mode 100644 index 00000000..e5ac8a4a --- /dev/null +++ b/python_pkg/brother_printer/_queue_fix.py @@ -0,0 +1,203 @@ +"""CUPS queue status display and the interactive queue-fix prompt. + +Split out of :mod:`python_pkg.brother_printer.cups_queue` to keep it under the +250-line cap. The ``_cups_*`` action primitives deliberately stay in +``cups_queue``: ``tests/test_cups_queue.py`` patches ``subprocess`` and +``time`` on that module, and moving them would let a real ``subprocess.run`` +reach the live CUPS daemon. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from python_pkg.brother_printer.constants import ( + BOLD, + CYAN, + DIM, + GREEN, + RED, + RESET, + YELLOW, + _out, + _prompt, +) +from python_pkg.brother_printer.cups_queue import ( + _cups_cancel_all_jobs, + _cups_enable_printer, + _cups_restart_service, +) + +if TYPE_CHECKING: + from collections.abc import Callable + + from python_pkg.brother_printer.data_classes import CUPSQueueStatus + + +# ── Queue status display ──────────────────────────────────────────── + + +def display_cups_queue_status(queue: CUPSQueueStatus) -> None: + """Display CUPS queue status and offer interactive fixes.""" + if not queue.printer_name: + return + if queue.enabled and not queue.jobs and not queue.has_backend_errors: + return + + _out() + _out(f"{BOLD}── Print Queue ──{RESET}") + _out() + + if queue.has_backend_errors and queue.enabled and not queue.jobs: + _out(f" {YELLOW}{BOLD}⚡ CUPS backend has stale errors{RESET}") + _out( + f" {DIM}New print jobs may silently fail." + f" A CUPS restart usually fixes this.{RESET}" + ) + _out() + + if not queue.enabled: + _out(f" {RED}{BOLD}⚠ Printer queue is DISABLED{RESET}") + if queue.reason: + _out(f" {DIM}Reason: {queue.reason}{RESET}") + _out() + + if queue.jobs: + _out(f" {BOLD}Pending jobs ({len(queue.jobs)}):{RESET}") + for job in queue.jobs: + _out(f" {job.job_id} {DIM}{job.user} {job.size}B {job.date}{RESET}") + _out() + + _offer_queue_fix(queue) + + +# ── Interactive queue fix ──────────────────────────────────────────── + + +def _offer_queue_fix(queue: CUPSQueueStatus) -> None: + """Prompt the user to fix a disabled queue / pending jobs.""" + _out(f" {BOLD}Available actions:{RESET}") + + options: list[str] = [] + if not queue.enabled and queue.jobs: + _out(f" {CYAN}1){RESET} Re-enable printer and retry all jobs") + _out(f" {CYAN}2){RESET} Re-enable printer and cancel all jobs") + _out(f" {CYAN}3){RESET} Cancel all jobs (keep printer disabled)") + _out(f" {CYAN}4){RESET} Restart CUPS service (fixes stale backend)") + _out(f" {CYAN}5){RESET} Restart CUPS + re-enable + retry all jobs") + _out(f" {CYAN}6){RESET} Do nothing") + options = ["1", "2", "3", "4", "5", "6"] + elif not queue.enabled: + _out(f" {CYAN}1){RESET} Re-enable printer") + _out(f" {CYAN}2){RESET} Restart CUPS service (fixes stale backend)") + _out(f" {CYAN}3){RESET} Do nothing") + options = ["1", "2", "3"] + elif queue.jobs: + _out(f" {CYAN}1){RESET} Cancel all pending jobs") + _out(f" {CYAN}2){RESET} Restart CUPS service (fixes stale backend)") + _out(f" {CYAN}3){RESET} Do nothing") + options = ["1", "2", "3"] + else: + _out(f" {CYAN}1){RESET} Restart CUPS service (fixes stale backend)") + _out(f" {CYAN}2){RESET} Do nothing") + options = ["1", "2"] + + _out() + choice = _prompt(f" Choose [{'/'.join(options)}]: ") + _out() + + if not queue.enabled and queue.jobs: + _handle_disabled_with_jobs(queue, choice) + elif not queue.enabled: + _handle_disabled_no_jobs(queue, choice) + elif queue.jobs: + _handle_enabled_with_jobs(queue, choice) + else: + _handle_backend_errors_only(choice) + + +def _dwj_enable_only(printer_name: str) -> None: + """Choice 1: re-enable printer so queued jobs are retried.""" + if _cups_enable_printer(printer_name): + _out(f" {GREEN}✓ Printer re-enabled. Jobs will be retried.{RESET}") + + +def _dwj_cancel_and_enable(printer_name: str) -> None: + """Choice 2: cancel all jobs then re-enable.""" + _cups_cancel_all_jobs(printer_name) + if _cups_enable_printer(printer_name): + _out(f" {GREEN}✓ All jobs cancelled and printer re-enabled.{RESET}") + + +def _dwj_cancel_only(printer_name: str) -> None: + """Choice 3: cancel all jobs.""" + if _cups_cancel_all_jobs(printer_name): + _out(f" {GREEN}✓ All jobs cancelled.{RESET}") + + +def _dwj_restart_only(_printer_name: str) -> None: + """Choice 4: restart CUPS.""" + if _cups_restart_service(): + _out(f" {GREEN}✓ CUPS restarted.{RESET}") + + +def _dwj_restart_and_enable(printer_name: str) -> None: + """Choice 5: restart CUPS and re-enable printer.""" + if _cups_restart_service(): + _cups_enable_printer(printer_name) + _out( + f" {GREEN}✓ CUPS restarted, printer re-enabled." + f" Jobs will be retried.{RESET}" + ) + + +_DWJ_ACTIONS: dict[str, Callable[[str], None]] = { + "1": _dwj_enable_only, + "2": _dwj_cancel_and_enable, + "3": _dwj_cancel_only, + "4": _dwj_restart_only, + "5": _dwj_restart_and_enable, +} + + +def _handle_disabled_with_jobs(queue: CUPSQueueStatus, choice: str) -> None: + """Handle fix for disabled printer with pending jobs.""" + action = _DWJ_ACTIONS.get(choice) + if action is not None: + action(queue.printer_name) + else: + _out(f" {DIM}No changes made.{RESET}") + + +def _handle_disabled_no_jobs(queue: CUPSQueueStatus, choice: str) -> None: + """Handle fix for disabled printer with no pending jobs.""" + if choice == "1": + if _cups_enable_printer(queue.printer_name): + _out(f" {GREEN}✓ Printer re-enabled.{RESET}") + elif choice == "2": + if _cups_restart_service(): + _cups_enable_printer(queue.printer_name) + _out(f" {GREEN}✓ CUPS restarted and printer re-enabled.{RESET}") + else: + _out(f" {DIM}No changes made.{RESET}") + + +def _handle_enabled_with_jobs(queue: CUPSQueueStatus, choice: str) -> None: + """Handle fix for enabled printer with stuck jobs.""" + if choice == "1": + if _cups_cancel_all_jobs(queue.printer_name): + _out(f" {GREEN}✓ All jobs cancelled.{RESET}") + elif choice == "2": + if _cups_restart_service(): + _out(f" {GREEN}✓ CUPS restarted.{RESET}") + else: + _out(f" {DIM}No changes made.{RESET}") + + +def _handle_backend_errors_only(choice: str) -> None: + """Handle fix when only stale backend errors are detected.""" + if choice == "1": + if _cups_restart_service(): + _out(f" {GREEN}✓ CUPS restarted. Stale backend errors cleared.{RESET}") + else: + _out(f" {DIM}No changes made.{RESET}") diff --git a/python_pkg/brother_printer/cups_queue.py b/python_pkg/brother_printer/cups_queue.py index 48194ed9..d9aebf01 100644 --- a/python_pkg/brother_printer/cups_queue.py +++ b/python_pkg/brother_printer/cups_queue.py @@ -8,28 +8,18 @@ import shutil import subprocess import sys import time -from typing import TYPE_CHECKING from python_pkg.brother_printer._query import run_command_text from python_pkg.brother_printer.constants import ( - BOLD, - CYAN, DIM, - GREEN, MIN_LPSTAT_JOB_PARTS, RED, RESET, - YELLOW, _out, - _prompt, ) from python_pkg.brother_printer.cups_service import find_cups_printer_name from python_pkg.brother_printer.data_classes import CUPSJob, CUPSQueueStatus -if TYPE_CHECKING: - from collections.abc import Callable - - # ── Queue inspection ───────────────────────────────────────────────── @@ -253,172 +243,3 @@ def _check_cups_backend_errors( return False, "" return True, backend_error - - -# ── Queue status display ──────────────────────────────────────────── - - -def display_cups_queue_status(queue: CUPSQueueStatus) -> None: - """Display CUPS queue status and offer interactive fixes.""" - if not queue.printer_name: - return - if queue.enabled and not queue.jobs and not queue.has_backend_errors: - return - - _out() - _out(f"{BOLD}── Print Queue ──{RESET}") - _out() - - if queue.has_backend_errors and queue.enabled and not queue.jobs: - _out(f" {YELLOW}{BOLD}⚡ CUPS backend has stale errors{RESET}") - _out( - f" {DIM}New print jobs may silently fail." - f" A CUPS restart usually fixes this.{RESET}" - ) - _out() - - if not queue.enabled: - _out(f" {RED}{BOLD}⚠ Printer queue is DISABLED{RESET}") - if queue.reason: - _out(f" {DIM}Reason: {queue.reason}{RESET}") - _out() - - if queue.jobs: - _out(f" {BOLD}Pending jobs ({len(queue.jobs)}):{RESET}") - for job in queue.jobs: - _out(f" {job.job_id} {DIM}{job.user} {job.size}B {job.date}{RESET}") - _out() - - _offer_queue_fix(queue) - - -# ── Interactive queue fix ──────────────────────────────────────────── - - -def _offer_queue_fix(queue: CUPSQueueStatus) -> None: - """Prompt the user to fix a disabled queue / pending jobs.""" - _out(f" {BOLD}Available actions:{RESET}") - - options: list[str] = [] - if not queue.enabled and queue.jobs: - _out(f" {CYAN}1){RESET} Re-enable printer and retry all jobs") - _out(f" {CYAN}2){RESET} Re-enable printer and cancel all jobs") - _out(f" {CYAN}3){RESET} Cancel all jobs (keep printer disabled)") - _out(f" {CYAN}4){RESET} Restart CUPS service (fixes stale backend)") - _out(f" {CYAN}5){RESET} Restart CUPS + re-enable + retry all jobs") - _out(f" {CYAN}6){RESET} Do nothing") - options = ["1", "2", "3", "4", "5", "6"] - elif not queue.enabled: - _out(f" {CYAN}1){RESET} Re-enable printer") - _out(f" {CYAN}2){RESET} Restart CUPS service (fixes stale backend)") - _out(f" {CYAN}3){RESET} Do nothing") - options = ["1", "2", "3"] - elif queue.jobs: - _out(f" {CYAN}1){RESET} Cancel all pending jobs") - _out(f" {CYAN}2){RESET} Restart CUPS service (fixes stale backend)") - _out(f" {CYAN}3){RESET} Do nothing") - options = ["1", "2", "3"] - else: - _out(f" {CYAN}1){RESET} Restart CUPS service (fixes stale backend)") - _out(f" {CYAN}2){RESET} Do nothing") - options = ["1", "2"] - - _out() - choice = _prompt(f" Choose [{'/'.join(options)}]: ") - _out() - - if not queue.enabled and queue.jobs: - _handle_disabled_with_jobs(queue, choice) - elif not queue.enabled: - _handle_disabled_no_jobs(queue, choice) - elif queue.jobs: - _handle_enabled_with_jobs(queue, choice) - else: - _handle_backend_errors_only(choice) - - -def _dwj_enable_only(printer_name: str) -> None: - """Choice 1: re-enable printer so queued jobs are retried.""" - if _cups_enable_printer(printer_name): - _out(f" {GREEN}✓ Printer re-enabled. Jobs will be retried.{RESET}") - - -def _dwj_cancel_and_enable(printer_name: str) -> None: - """Choice 2: cancel all jobs then re-enable.""" - _cups_cancel_all_jobs(printer_name) - if _cups_enable_printer(printer_name): - _out(f" {GREEN}✓ All jobs cancelled and printer re-enabled.{RESET}") - - -def _dwj_cancel_only(printer_name: str) -> None: - """Choice 3: cancel all jobs.""" - if _cups_cancel_all_jobs(printer_name): - _out(f" {GREEN}✓ All jobs cancelled.{RESET}") - - -def _dwj_restart_only(_printer_name: str) -> None: - """Choice 4: restart CUPS.""" - if _cups_restart_service(): - _out(f" {GREEN}✓ CUPS restarted.{RESET}") - - -def _dwj_restart_and_enable(printer_name: str) -> None: - """Choice 5: restart CUPS and re-enable printer.""" - if _cups_restart_service(): - _cups_enable_printer(printer_name) - _out( - f" {GREEN}✓ CUPS restarted, printer re-enabled." - f" Jobs will be retried.{RESET}" - ) - - -_DWJ_ACTIONS: dict[str, Callable[[str], None]] = { - "1": _dwj_enable_only, - "2": _dwj_cancel_and_enable, - "3": _dwj_cancel_only, - "4": _dwj_restart_only, - "5": _dwj_restart_and_enable, -} - - -def _handle_disabled_with_jobs(queue: CUPSQueueStatus, choice: str) -> None: - """Handle fix for disabled printer with pending jobs.""" - action = _DWJ_ACTIONS.get(choice) - if action is not None: - action(queue.printer_name) - else: - _out(f" {DIM}No changes made.{RESET}") - - -def _handle_disabled_no_jobs(queue: CUPSQueueStatus, choice: str) -> None: - """Handle fix for disabled printer with no pending jobs.""" - if choice == "1": - if _cups_enable_printer(queue.printer_name): - _out(f" {GREEN}✓ Printer re-enabled.{RESET}") - elif choice == "2": - if _cups_restart_service(): - _cups_enable_printer(queue.printer_name) - _out(f" {GREEN}✓ CUPS restarted and printer re-enabled.{RESET}") - else: - _out(f" {DIM}No changes made.{RESET}") - - -def _handle_enabled_with_jobs(queue: CUPSQueueStatus, choice: str) -> None: - """Handle fix for enabled printer with stuck jobs.""" - if choice == "1": - if _cups_cancel_all_jobs(queue.printer_name): - _out(f" {GREEN}✓ All jobs cancelled.{RESET}") - elif choice == "2": - if _cups_restart_service(): - _out(f" {GREEN}✓ CUPS restarted.{RESET}") - else: - _out(f" {DIM}No changes made.{RESET}") - - -def _handle_backend_errors_only(choice: str) -> None: - """Handle fix when only stale backend errors are detected.""" - if choice == "1": - if _cups_restart_service(): - _out(f" {GREEN}✓ CUPS restarted. Stale backend errors cleared.{RESET}") - else: - _out(f" {DIM}No changes made.{RESET}") diff --git a/python_pkg/brother_printer/display.py b/python_pkg/brother_printer/display.py index d3f25d30..f9a4af5b 100644 --- a/python_pkg/brother_printer/display.py +++ b/python_pkg/brother_printer/display.py @@ -14,6 +14,7 @@ from python_pkg.brother_printer._page_count import ( _display_page_count_estimate, _display_page_delivery_warning, ) +from python_pkg.brother_printer._queue_fix import display_cups_queue_status from python_pkg.brother_printer._severity import ( _SEVERITY_COLORS, _SEVERITY_ICONS, @@ -38,10 +39,7 @@ from python_pkg.brother_printer.constants import ( _out, get_status_info, ) -from python_pkg.brother_printer.cups_queue import ( - display_cups_queue_status, - get_cups_queue_status, -) +from python_pkg.brother_printer.cups_queue import get_cups_queue_status if TYPE_CHECKING: from python_pkg.brother_printer.data_classes import ( diff --git a/python_pkg/brother_printer/tests/test_cups_queue_part2.py b/python_pkg/brother_printer/tests/test_cups_queue_part2.py index a1791773..3c736a9a 100644 --- a/python_pkg/brother_printer/tests/test_cups_queue_part2.py +++ b/python_pkg/brother_printer/tests/test_cups_queue_part2.py @@ -1,11 +1,11 @@ -"""Tests for brother_printer.cups_queue module - part 2 (interactive fix).""" +"""Tests for brother_printer._queue_fix module - part 2 (interactive fix).""" from __future__ import annotations from io import StringIO from unittest.mock import MagicMock, patch -from python_pkg.brother_printer.cups_queue import ( +from python_pkg.brother_printer._queue_fix import ( _dwj_cancel_and_enable, _dwj_cancel_only, _dwj_enable_only, @@ -19,7 +19,7 @@ from python_pkg.brother_printer.cups_queue import ( ) from python_pkg.brother_printer.data_classes import CUPSJob, CUPSQueueStatus -MOD = "python_pkg.brother_printer.cups_queue" +MOD = "python_pkg.brother_printer._queue_fix" # ── _offer_queue_fix ───────────────────────────────────────────────── diff --git a/python_pkg/brother_printer/tests/test_cups_queue_part3.py b/python_pkg/brother_printer/tests/test_cups_queue_part3.py index 6abbbb7d..d3d33ef8 100644 --- a/python_pkg/brother_printer/tests/test_cups_queue_part3.py +++ b/python_pkg/brother_printer/tests/test_cups_queue_part3.py @@ -1,16 +1,16 @@ -"""Tests for brother_printer.cups_queue module - part 3 (display status).""" +"""Tests for brother_printer._queue_fix module - part 3 (display status).""" from __future__ import annotations from io import StringIO from unittest.mock import MagicMock, patch -from python_pkg.brother_printer.cups_queue import ( +from python_pkg.brother_printer._queue_fix import ( display_cups_queue_status, ) from python_pkg.brother_printer.data_classes import CUPSJob, CUPSQueueStatus -MOD = "python_pkg.brother_printer.cups_queue" +MOD = "python_pkg.brother_printer._queue_fix" class TestDisplayCupsQueueStatus: