Split brother_printer queue display and fix prompt out of cups_queue

cups_queue.py 424 -> 245. display_cups_queue_status, _offer_queue_fix,
the _dwj_* choices and the _handle_* dispatchers move to _queue_fix.py.

The _cups_* action primitives stay put: 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
free to restart the real CUPS service mid-suite. test_cups_queue.py
therefore needs no edit; only part2/part3 repoint their MOD.

Pass count is 402 before and after.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011w2Pj4Ek3ZNMQVYcFHgXDY
This commit is contained in:
Krzysztof Rudnicki 2026-08-16 16:53:51 +02:00
parent 386ff25296
commit cf44e54215
7 changed files with 277 additions and 189 deletions

View File

@ -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."
}

View File

@ -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 <the six touched 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."
]
}

View File

@ -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}")

View File

@ -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}")

View File

@ -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 (

View File

@ -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 ─────────────────────────────────────────────────

View File

@ -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: