mirror of
https://github.com/kuhyx/testsAndMisc.git
synced 2026-11-10 18:47:04 +01:00
Reduce brother_printer/display.py and dedup the consumable bars
Partial: display.py 427 -> 342. Still a violation, not yet cleared.
Moved out only what no test patches:
_supply.py the supply-level arithmetic (imported by tests, not patched)
_severity.py the three _SEVERITY_* tables (pure data)
Also collapsed the toner and drum bars in _display_page_count_estimate into
_supply.render_life_bar. They were the same logic with different colours and
notes, so this removes duplication rather than relocating it.
The remaining ~92 lines are blocked on mock.patch pinning. This package's test
modules patch ~90 names via patch(f"{MOD}.<name>"), and a patched name only
resolves in the module the test names -- moving a function that calls one
breaks the mock. An attempt to move the page-count block was reverted for
exactly that (display has no attribute 'estimate_consumable_life'). Clearing
it needs either test edits or ~8 delegate stubs, which is a decision to take
rather than assume.
402 tests pass, unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011w2Pj4Ek3ZNMQVYcFHgXDY
This commit is contained in:
parent
5029d4685b
commit
2832b7841f
@ -0,0 +1,41 @@
|
||||
{
|
||||
"intent": "Reduce python_pkg/brother_printer/display.py by extracting the parts no test patches, and collapse the duplicated toner/drum bar rendering into one helper. This does NOT bring display.py under the 250-line cap -- it is 342 and needs a further ~92 lines removed, which is blocked on a decision about editing test files.",
|
||||
"scope": [
|
||||
"python_pkg/brother_printer/display.py",
|
||||
"python_pkg/brother_printer/_supply.py (new)",
|
||||
"python_pkg/brother_printer/_severity.py (new)",
|
||||
"Non-goal: no assertion changed, no test deleted or weakened"
|
||||
],
|
||||
"changes": [
|
||||
"Extracted the supply-level arithmetic into _supply.py: _classify_percentage_level, _classify_supply_level, _format_supply_bar, _process_supply_item, _parse_supply_value, _collect_supply_items. These are imported by the tests, not patched, so they were free to move.",
|
||||
"Extracted the three _SEVERITY_* lookup tables into _severity.py. Pure data, patched by nothing.",
|
||||
"Added _supply.render_life_bar and used it for both the toner and drum bars in _display_page_count_estimate. Those two blocks were the same logic with different colours and notes -- this removes the duplication rather than just relocating it.",
|
||||
"display.py re-exports the moved supply names through __all__, because tests/test_display.py imports them from display.",
|
||||
"display.py 427 -> 342 lines; _supply.py 131; _severity.py 44."
|
||||
],
|
||||
"verification": [
|
||||
{
|
||||
"command": "python3 -m pytest python_pkg/brother_printer/tests/ -q",
|
||||
"result": "pass",
|
||||
"evidence": "402 passed in 1.76s -- the same count as before the change, run after each of the three extractions."
|
||||
},
|
||||
{
|
||||
"command": "pre-commit run --files python_pkg/brother_printer/display.py python_pkg/brother_printer/_supply.py python_pkg/brother_printer/_severity.py",
|
||||
"result": "pass",
|
||||
"evidence": "ruff, ruff format, mypy, pylint, bandit and the coverage-gated pytest all pass. Only the evidence-artifact hook remained, which this file satisfies. No suppression added; a pylint C0104 on the name 'bar' was fixed by renaming it, not disabling the check."
|
||||
},
|
||||
{
|
||||
"command": "bash ~/utils/scripts/check_file_length.sh --all",
|
||||
"result": "fail",
|
||||
"evidence": "display.py is still 342 lines and remains a violation. Stated explicitly: this commit reduces it and removes real duplication, but does not clear it."
|
||||
}
|
||||
],
|
||||
"risks": [
|
||||
"The remaining ~92 lines cannot be extracted without either editing test files or adding ~8 pure-indirection delegate functions. brother_printer's test modules patch roughly 90 names across the package via patch(f'{MOD}.<name>'), and mock.patch pins a name to the module it is patched on: a function that moves to a new module resolves its collaborators through the new module's globals, so the patch no longer reaches it. An earlier attempt to move the page-count block was reverted for exactly this reason (AttributeError: module display does not have the attribute 'estimate_consumable_life').",
|
||||
"_display_report_header is patched from both test_display.py and test_display_part2.py, so any split of display.py into USB and network halves has to keep that function reachable from both patch sites."
|
||||
],
|
||||
"rollback": [
|
||||
"git revert the commit; display.py returns to 427 lines and the two new modules are deleted.",
|
||||
"After rollback, run python3 -m pytest python_pkg/brother_printer/tests/ -q and confirm it still reports 402 passed."
|
||||
]
|
||||
}
|
||||
44
python_pkg/brother_printer/_severity.py
Normal file
44
python_pkg/brother_printer/_severity.py
Normal file
@ -0,0 +1,44 @@
|
||||
"""The icon, colour and closing summary for each PJL severity.
|
||||
|
||||
Split out of :mod:`python_pkg.brother_printer.display` to keep it under the
|
||||
250-line cap. Pure data: no test patches these tables, so unlike most of
|
||||
``display`` they were free to move.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from python_pkg.brother_printer.constants import (
|
||||
BOLD,
|
||||
CYAN,
|
||||
GREEN,
|
||||
RED,
|
||||
RESET,
|
||||
YELLOW,
|
||||
)
|
||||
|
||||
_SEVERITY_ICONS: dict[str, str] = {
|
||||
"ok": "✓",
|
||||
"info": "i",
|
||||
"warn": "⚡",
|
||||
"critical": "⚠",
|
||||
}
|
||||
_SEVERITY_COLORS: dict[str, str] = {
|
||||
"ok": GREEN,
|
||||
"info": CYAN,
|
||||
"warn": YELLOW,
|
||||
"critical": RED,
|
||||
}
|
||||
_SEVERITY_SUMMARIES: dict[str, str] = {
|
||||
"ok": f"{GREEN}{BOLD}✓ Printer is healthy. No replacements needed.{RESET}",
|
||||
"info": (
|
||||
f"{CYAN}{BOLD}i Printer is busy/processing. No replacements needed.{RESET}"
|
||||
),
|
||||
"warn": (
|
||||
f"{YELLOW}{BOLD}⚡ WARNING: Maintenance will be needed"
|
||||
f" soon.{RESET}\n{YELLOW} Order replacement parts"
|
||||
f" now to avoid interruption.{RESET}"
|
||||
),
|
||||
"critical": (
|
||||
f"{RED}{BOLD}⚠ ACTION REQUIRED: Replacement or fix needed now!{RESET}"
|
||||
),
|
||||
}
|
||||
131
python_pkg/brother_printer/_supply.py
Normal file
131
python_pkg/brother_printer/_supply.py
Normal file
@ -0,0 +1,131 @@
|
||||
"""Turning a raw SNMP supply reading into a percentage, a bar and a verdict.
|
||||
|
||||
Split out of :mod:`python_pkg.brother_printer.display` to keep it under the
|
||||
250-line cap. Only the arithmetic lives here. The functions that *print*
|
||||
(``_display_supply_levels``, ``_display_supply_warnings``) stay in ``display``
|
||||
because ``tests/test_display_part2.py`` patches them there, and a patched name
|
||||
has to remain resolvable in the module the test names.
|
||||
|
||||
``display`` re-exports everything below, because the tests import these names
|
||||
from there rather than from here.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
from python_pkg.brother_printer.constants import (
|
||||
BOLD,
|
||||
GREEN,
|
||||
PROGRESS_BAR_WIDTH,
|
||||
RED,
|
||||
RESET,
|
||||
SNMP_LEVEL_LOW,
|
||||
SNMP_LEVEL_OK,
|
||||
SUPPLY_LOW_PCT,
|
||||
SUPPLY_WARN_PCT,
|
||||
YELLOW,
|
||||
_out,
|
||||
)
|
||||
from python_pkg.brother_printer.data_classes import SupplyStatus
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from python_pkg.brother_printer.data_classes import NetworkResult
|
||||
|
||||
|
||||
def _classify_percentage_level(desc: str, pct: int) -> tuple[int, str, str, str, bool]:
|
||||
"""Classify a supply by its calculated percentage."""
|
||||
if pct <= SUPPLY_LOW_PCT:
|
||||
return pct, f"{pct}%", RED, f"{desc} at {pct}%.", True
|
||||
if pct <= SUPPLY_WARN_PCT:
|
||||
return pct, f"{pct}%", YELLOW, f"{desc} at {pct}% -- order soon.", False
|
||||
return pct, f"{pct}%", GREEN, "", False
|
||||
|
||||
|
||||
def _classify_supply_level(
|
||||
desc: str, max_val: int, level: int
|
||||
) -> tuple[int, str, str, str, bool]:
|
||||
"""Classify a supply level. Returns (pct, status, color, warning, replace)."""
|
||||
if level == SNMP_LEVEL_OK:
|
||||
return -1, "OK", GREEN, "", False
|
||||
if level == SNMP_LEVEL_LOW:
|
||||
return -1, "LOW", RED, f"{desc} is LOW.", True
|
||||
if level == 0:
|
||||
return 0, "EMPTY", RED, f"{desc} is EMPTY -- replace now!", True
|
||||
if max_val > 0:
|
||||
pct = min(level * 100 // max_val, 100)
|
||||
return _classify_percentage_level(desc, pct)
|
||||
return -1, "", GREEN, "", False
|
||||
|
||||
|
||||
def _format_supply_bar(pct: int) -> str:
|
||||
"""Build a progress bar string for a supply percentage."""
|
||||
if pct < 0:
|
||||
return ""
|
||||
filled = pct * PROGRESS_BAR_WIDTH // 100
|
||||
empty = PROGRESS_BAR_WIDTH - filled
|
||||
return f"[{'█' * filled}{'░' * empty}]"
|
||||
|
||||
|
||||
def _process_supply_item(desc: str, max_val: int, level: int) -> SupplyStatus:
|
||||
"""Process a single supply item into display info."""
|
||||
pct, status_text, color, warning, needs_replacement = _classify_supply_level(
|
||||
desc, max_val, level
|
||||
)
|
||||
bar_text = _format_supply_bar(pct)
|
||||
return SupplyStatus(color, bar_text, status_text, warning, needs_replacement)
|
||||
|
||||
|
||||
def _parse_supply_value(values: list[str], index: int) -> int:
|
||||
"""Safely parse an integer from a supply value list."""
|
||||
try:
|
||||
return int(values[index])
|
||||
except (IndexError, ValueError):
|
||||
return 0
|
||||
|
||||
|
||||
def _collect_supply_items(
|
||||
result: NetworkResult,
|
||||
) -> tuple[list[SupplyStatus], list[str]]:
|
||||
"""Parse and collect supply items with their descriptions."""
|
||||
items: list[SupplyStatus] = []
|
||||
descs: list[str] = []
|
||||
for i, desc in enumerate(result.supplies.descriptions):
|
||||
max_val = _parse_supply_value(result.supplies.max_values, i)
|
||||
level = _parse_supply_value(result.supplies.levels, i)
|
||||
items.append(_process_supply_item(desc, max_val, level))
|
||||
descs.append(desc)
|
||||
return items, descs
|
||||
|
||||
|
||||
def render_life_bar(
|
||||
label: str,
|
||||
pct: int,
|
||||
*,
|
||||
exhausted: bool = False,
|
||||
low: bool = False,
|
||||
exhausted_note: str = "",
|
||||
low_note: str = "",
|
||||
) -> None:
|
||||
"""Print one consumable's remaining-life bar.
|
||||
|
||||
The toner and drum bars in the page-count estimate were byte-for-byte the
|
||||
same logic with different colours and notes, which is what this collapses.
|
||||
|
||||
Args:
|
||||
label: Displayed name, already padded by the caller if needed.
|
||||
pct: Percentage remaining, 0-100.
|
||||
exhausted: Render red with ``exhausted_note``.
|
||||
low: Render yellow with ``low_note``. Ignored when *exhausted*.
|
||||
exhausted_note: Suffix shown when *exhausted*.
|
||||
low_note: Suffix shown when *low*.
|
||||
"""
|
||||
filled = pct * PROGRESS_BAR_WIDTH // 100
|
||||
meter = f"[{'█' * filled}{'░' * (PROGRESS_BAR_WIDTH - filled)}]"
|
||||
if exhausted:
|
||||
color, note = RED, exhausted_note
|
||||
elif low:
|
||||
color, note = YELLOW, low_note
|
||||
else:
|
||||
color, note = GREEN, ""
|
||||
_out(f" {BOLD}{label}{RESET} {color}{meter} ~{pct}%{note}{RESET}")
|
||||
@ -3,20 +3,34 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
# ── Shared display helpers ───────────────────────────────────────────
|
||||
# Re-exported through __all__: tests/test_display.py imports these names from
|
||||
# this module. Only the un-patched supply *maths* moved to _supply -- everything
|
||||
# the tests reach with mock.patch has to stay resolvable here.
|
||||
from python_pkg.brother_printer._severity import (
|
||||
_SEVERITY_COLORS,
|
||||
_SEVERITY_ICONS,
|
||||
_SEVERITY_SUMMARIES,
|
||||
)
|
||||
from python_pkg.brother_printer._supply import (
|
||||
_classify_percentage_level,
|
||||
_classify_supply_level,
|
||||
_collect_supply_items,
|
||||
_format_supply_bar,
|
||||
_parse_supply_value,
|
||||
_process_supply_item,
|
||||
render_life_bar,
|
||||
)
|
||||
from python_pkg.brother_printer.constants import (
|
||||
BOLD,
|
||||
CYAN,
|
||||
DIM,
|
||||
DRUM_RATED_PAGES,
|
||||
GREEN,
|
||||
PROGRESS_BAR_WIDTH,
|
||||
RED,
|
||||
RESET,
|
||||
SNMP_LEVEL_LOW,
|
||||
SNMP_LEVEL_OK,
|
||||
SUPPLY_LOW_PCT,
|
||||
SUPPLY_WARN_PCT,
|
||||
TONER_RATED_PAGES,
|
||||
YELLOW,
|
||||
_out,
|
||||
@ -30,13 +44,23 @@ from python_pkg.brother_printer.cups_queue import (
|
||||
display_cups_queue_status,
|
||||
get_cups_queue_status,
|
||||
)
|
||||
from python_pkg.brother_printer.data_classes import (
|
||||
NetworkResult,
|
||||
SupplyStatus,
|
||||
USBResult,
|
||||
)
|
||||
|
||||
# ── Shared display helpers ───────────────────────────────────────────
|
||||
if TYPE_CHECKING:
|
||||
from python_pkg.brother_printer.data_classes import (
|
||||
NetworkResult,
|
||||
USBResult,
|
||||
)
|
||||
|
||||
__all__ = [
|
||||
"_classify_percentage_level",
|
||||
"_classify_supply_level",
|
||||
"_collect_supply_items",
|
||||
"_format_supply_bar",
|
||||
"_parse_supply_value",
|
||||
"_process_supply_item",
|
||||
"display_network_results",
|
||||
"display_usb_results",
|
||||
]
|
||||
|
||||
|
||||
def _display_report_header() -> None:
|
||||
@ -101,36 +125,20 @@ def _display_page_count_estimate(printer_total: int = 0) -> None:
|
||||
f" and reads high.{RESET}"
|
||||
)
|
||||
_out()
|
||||
# Toner bar
|
||||
toner_pct = estimate.toner_pct_remaining
|
||||
toner_filled = toner_pct * PROGRESS_BAR_WIDTH // 100
|
||||
toner_empty = PROGRESS_BAR_WIDTH - toner_filled
|
||||
toner_bar = f"[{'█' * toner_filled}{'░' * toner_empty}]"
|
||||
if estimate.toner_exhausted:
|
||||
toner_color = RED
|
||||
toner_note = " ← REPLACE NOW"
|
||||
elif estimate.toner_low:
|
||||
toner_color = YELLOW
|
||||
toner_note = " ← order soon"
|
||||
else:
|
||||
toner_color = GREEN
|
||||
toner_note = ""
|
||||
_out(
|
||||
f" {BOLD}Toner:{RESET} {toner_color}{toner_bar} ~{toner_pct}%"
|
||||
f"{toner_note}{RESET}"
|
||||
render_life_bar(
|
||||
"Toner:",
|
||||
estimate.toner_pct_remaining,
|
||||
exhausted=estimate.toner_exhausted,
|
||||
low=estimate.toner_low,
|
||||
exhausted_note=" ← REPLACE NOW",
|
||||
low_note=" ← order soon",
|
||||
)
|
||||
render_life_bar(
|
||||
"Drum: ",
|
||||
estimate.drum_pct_remaining,
|
||||
low=estimate.drum_near_end,
|
||||
low_note=" ← nearing end",
|
||||
)
|
||||
# Drum bar
|
||||
drum_pct = estimate.drum_pct_remaining
|
||||
drum_filled = drum_pct * PROGRESS_BAR_WIDTH // 100
|
||||
drum_empty = PROGRESS_BAR_WIDTH - drum_filled
|
||||
drum_bar = f"[{'█' * drum_filled}{'░' * drum_empty}]"
|
||||
if estimate.drum_near_end:
|
||||
drum_color = YELLOW
|
||||
drum_note = " ← nearing end"
|
||||
else:
|
||||
drum_color = GREEN
|
||||
drum_note = ""
|
||||
_out(f" {BOLD}Drum:{RESET} {drum_color}{drum_bar} ~{drum_pct}%{drum_note}{RESET}")
|
||||
_out(
|
||||
f" {DIM}Based on pages since last replacement"
|
||||
f" vs rated capacity (toner ~{TONER_RATED_PAGES},"
|
||||
@ -183,34 +191,6 @@ def _display_usb_device_info(result: USBResult) -> None:
|
||||
_out(f"{BOLD}Toner Save:{RESET} OFF")
|
||||
|
||||
|
||||
_SEVERITY_ICONS: dict[str, str] = {
|
||||
"ok": "✓",
|
||||
"info": "i",
|
||||
"warn": "⚡",
|
||||
"critical": "⚠",
|
||||
}
|
||||
_SEVERITY_COLORS: dict[str, str] = {
|
||||
"ok": GREEN,
|
||||
"info": CYAN,
|
||||
"warn": YELLOW,
|
||||
"critical": RED,
|
||||
}
|
||||
_SEVERITY_SUMMARIES: dict[str, str] = {
|
||||
"ok": f"{GREEN}{BOLD}✓ Printer is healthy. No replacements needed.{RESET}",
|
||||
"info": (
|
||||
f"{CYAN}{BOLD}i Printer is busy/processing. No replacements needed.{RESET}"
|
||||
),
|
||||
"warn": (
|
||||
f"{YELLOW}{BOLD}⚡ WARNING: Maintenance will be needed"
|
||||
f" soon.{RESET}\n{YELLOW} Order replacement parts"
|
||||
f" now to avoid interruption.{RESET}"
|
||||
),
|
||||
"critical": (
|
||||
f"{RED}{BOLD}⚠ ACTION REQUIRED: Replacement or fix needed now!{RESET}"
|
||||
),
|
||||
}
|
||||
|
||||
|
||||
def _format_status_detail(
|
||||
severity: str, short_text: str, action: str, result: USBResult
|
||||
) -> None:
|
||||
@ -291,49 +271,6 @@ def display_usb_results(result: USBResult) -> None:
|
||||
# ── Network supply level helpers ─────────────────────────────────────
|
||||
|
||||
|
||||
def _classify_percentage_level(desc: str, pct: int) -> tuple[int, str, str, str, bool]:
|
||||
"""Classify a supply by its calculated percentage."""
|
||||
if pct <= SUPPLY_LOW_PCT:
|
||||
return pct, f"{pct}%", RED, f"{desc} at {pct}%.", True
|
||||
if pct <= SUPPLY_WARN_PCT:
|
||||
return pct, f"{pct}%", YELLOW, f"{desc} at {pct}% -- order soon.", False
|
||||
return pct, f"{pct}%", GREEN, "", False
|
||||
|
||||
|
||||
def _classify_supply_level(
|
||||
desc: str, max_val: int, level: int
|
||||
) -> tuple[int, str, str, str, bool]:
|
||||
"""Classify a supply level. Returns (pct, status, color, warning, replace)."""
|
||||
if level == SNMP_LEVEL_OK:
|
||||
return -1, "OK", GREEN, "", False
|
||||
if level == SNMP_LEVEL_LOW:
|
||||
return -1, "LOW", RED, f"{desc} is LOW.", True
|
||||
if level == 0:
|
||||
return 0, "EMPTY", RED, f"{desc} is EMPTY -- replace now!", True
|
||||
if max_val > 0:
|
||||
pct = min(level * 100 // max_val, 100)
|
||||
return _classify_percentage_level(desc, pct)
|
||||
return -1, "", GREEN, "", False
|
||||
|
||||
|
||||
def _format_supply_bar(pct: int) -> str:
|
||||
"""Build a progress bar string for a supply percentage."""
|
||||
if pct < 0:
|
||||
return ""
|
||||
filled = pct * PROGRESS_BAR_WIDTH // 100
|
||||
empty = PROGRESS_BAR_WIDTH - filled
|
||||
return f"[{'█' * filled}{'░' * empty}]"
|
||||
|
||||
|
||||
def _process_supply_item(desc: str, max_val: int, level: int) -> SupplyStatus:
|
||||
"""Process a single supply item into display info."""
|
||||
pct, status_text, color, warning, needs_replacement = _classify_supply_level(
|
||||
desc, max_val, level
|
||||
)
|
||||
bar_text = _format_supply_bar(pct)
|
||||
return SupplyStatus(color, bar_text, status_text, warning, needs_replacement)
|
||||
|
||||
|
||||
def _display_supply_warnings(*, needs_replacement: bool, warnings: list[str]) -> None:
|
||||
"""Display supply level warnings summary."""
|
||||
_out()
|
||||
@ -349,28 +286,6 @@ def _display_supply_warnings(*, needs_replacement: bool, warnings: list[str]) ->
|
||||
_out(f"{GREEN}{BOLD}✓ All consumables are at healthy levels.{RESET}")
|
||||
|
||||
|
||||
def _parse_supply_value(values: list[str], index: int) -> int:
|
||||
"""Safely parse an integer from a supply value list."""
|
||||
try:
|
||||
return int(values[index])
|
||||
except (IndexError, ValueError):
|
||||
return 0
|
||||
|
||||
|
||||
def _collect_supply_items(
|
||||
result: NetworkResult,
|
||||
) -> tuple[list[SupplyStatus], list[str]]:
|
||||
"""Parse and collect supply items with their descriptions."""
|
||||
items: list[SupplyStatus] = []
|
||||
descs: list[str] = []
|
||||
for i, desc in enumerate(result.supplies.descriptions):
|
||||
max_val = _parse_supply_value(result.supplies.max_values, i)
|
||||
level = _parse_supply_value(result.supplies.levels, i)
|
||||
items.append(_process_supply_item(desc, max_val, level))
|
||||
descs.append(desc)
|
||||
return items, descs
|
||||
|
||||
|
||||
def _display_supply_levels(result: NetworkResult) -> None:
|
||||
"""Display consumable supply levels section."""
|
||||
_out()
|
||||
|
||||
Loading…
Reference in New Issue
Block a user