Files
computer-playbook/tests/lint/filesystem/python/test_no_uncached_file_reads.py
T
kevinveenbirkenbachandClaude Opus 5 5137465c6f style: delete every comment the comments-valid policy rejects
3529 invalid comments in 528 files, grandfathered in while the linter
only saw the working-tree diff. Removal ran through the linter's own
collectors, so detection and deletion cannot drift: a comment whose
opener line is blank before it and whose closer line is blank after it
loses all its lines, anything else loses only the comment region, and a
line left holding a bare delimiter goes with it.

Five files keep their comments behind a nocheck marker instead. Four are
upstream configs whose inline docs are upstream's own (mautrix, systemd
journald, nextcloud-oidc-login, MediaWiki). In littlejs the
commented-out entries ARE the deactivation mechanism for an example
tile, so deleting them would delete configuration.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-18 03:22:02 +02:00

195 lines
7.0 KiB
Python

"""Lint guard: tests and the modules they import MUST route file reads
through ``utils.cache.files.read_text`` instead of calling
``Path.read_text(...)`` or ``Path.read_bytes(...)`` directly.
Background
==========
Pytest runs every test in one process. The test suite reads the
same role/task YAML, Dockerfile, packaging spec, compose template,
nginx conf, etc. dozens of times across sibling tests.
``utils.cache.files.read_text`` is an LRU-cached UTF-8 read keyed on
absolute path — every duplicate scan after the first one returns from
memory. A raw ``path.read_text(...)`` call bypasses that cache and
re-reads the file from disk every time.
The cost is invisible at the call site but measurable at suite level:
each non-cached read is one ``open() + read() + decode + close()``
syscall round-trip per test that asks for it. On the lint pass alone
(20+ scanners, ~250 role trees) the wasted reads add up to seconds of
cold-cache latency that no individual test owner notices. Integration
tests show the same pattern (e.g. one role's compose template read
across multiple shape tests).
The same logic applies to library code (``utils/``, ``cli/``,
``plugins/``, ``library/``) when tests import those modules and call
their functions directly: their raw reads run inside the same process
and bypass the cache the tests just populated.
See ``docs/agents/action/testing.md`` for the full rule.
Scope
=====
Every ``.py`` file under ``tests/``, ``utils/``, ``cli/``, ``plugins/``,
and ``library/`` is scanned. ``utils/cache/`` is excluded because it
holds the cache implementation itself (would self-reference).
``roles/`` is excluded because its scripts are deployed to target
hosts without ``utils/`` on PYTHONPATH. Legitimate exceptions — reads
from a tempdir created at runtime, synthetic fixtures written inside
the test, files mutated in the same flow that re-reads them — opt
out per call with ``# nocheck: cache-read``.
Detection
=========
AST-walks every ``.py`` file in scope and flags call expressions
matching::
<expr>.read_text(...)
<expr>.read_bytes(...)
The receiver is not type-inferred — every method call with that
attribute name is flagged, on the assumption that the receiver is a
``Path`` (which is the overwhelming convention in this codebase). False
positives on differently-typed receivers should opt out per call.
Plain attribute access without a call is NOT flagged. String literals,
docstrings and comments naming these methods in prose are also not
flagged because the AST sees only real call nodes.
Per-line opt-out
================
Add ``# nocheck: cache-read`` (or ``# nocheck: cache-read``;
case-insensitive) on the call's own line OR, for multi-line calls, on
any line spanned by the call expression. Use this for legitimate
exceptions — e.g. reading a file from a tempdir created at runtime, a
file outside the project tree, or a synthetic fixture written and
read back inside the same test. Do NOT use it to silence the lint for
"I want the convenience of ``path.read_text()``" — replace with
``read_text(str(path))`` from ``utils.cache.files`` instead.
The marker grammar lives in
``docs/contributing/actions/testing/suppression.md``; this lint
consumes it via :func:`utils.annotations.suppress.suppressed_line_numbers`.
"""
from __future__ import annotations
import ast
import unittest
from dataclasses import dataclass
from pathlib import Path
from utils.annotations.suppress import suppressed_line_numbers
from utils.cache.files import iter_project_files, read_text
from . import PROJECT_ROOT
_RULE = "cache-read"
_SCAN_PREFIXES: tuple[str, ...] = (
"tests/",
"utils/",
"cli/",
"plugins/",
"library/",
)
_SKIP_PREFIXES: tuple[str, ...] = ("utils/cache/",)
_FORBIDDEN_ATTRS: frozenset[str] = frozenset({"read_text", "read_bytes"})
@dataclass(frozen=True)
class Finding:
file: Path
line: int
pattern: str
snippet: str
def format(self, repo_root: Path) -> str:
rel = self.file.relative_to(repo_root).as_posix()
return f"{rel}:{self.line}: {self.pattern}: {self.snippet}"
def _scan_file(path: Path) -> list[Finding]:
findings: list[Finding] = []
try:
src = read_text(str(path))
except (OSError, UnicodeDecodeError):
return findings
try:
tree = ast.parse(src, filename=str(path))
except SyntaxError:
return findings
lines = src.splitlines()
noqa = suppressed_line_numbers(lines, _RULE)
def _is_noqa(node: ast.Call) -> bool:
start = node.lineno
end = getattr(node, "end_lineno", node.lineno)
return any(line in noqa for line in range(start, end + 1))
def _snippet(node: ast.Call) -> str:
idx = node.lineno - 1
if 0 <= idx < len(lines):
return lines[idx].strip()[:160]
return ""
for node in ast.walk(tree):
if not isinstance(node, ast.Call):
continue
func = node.func
if not isinstance(func, ast.Attribute):
continue
if func.attr not in _FORBIDDEN_ATTRS:
continue
if _is_noqa(node):
continue
findings.append(
Finding(
file=path,
line=node.lineno,
pattern=f".{func.attr}(...)",
snippet=_snippet(node),
)
)
return findings
class TestNoUncachedFileReads(unittest.TestCase):
def test_modules_in_scope_use_cached_read_text(self) -> None:
"""Forbidden ``Path.read_text`` / ``Path.read_bytes`` calls in
in-scope code without an explicit ``# nocheck: cache-read``
marker. Use :func:`utils.cache.files.read_text` so the LRU
cache is shared across the pytest session.
"""
repo_root = PROJECT_ROOT
findings: list[Finding] = []
for path_str in iter_project_files(extensions=(".py",)):
rel = Path(path_str).relative_to(repo_root).as_posix()
if not rel.startswith(_SCAN_PREFIXES):
continue
if rel.startswith(_SKIP_PREFIXES):
continue
findings.extend(_scan_file(Path(path_str)))
if findings:
formatted = "\n".join(f.format(repo_root) for f in findings)
self.fail(
f"{len(findings)} uncached file-read call(s) under "
f"{_SCAN_PREFIXES}:\n{formatted}\n\n"
"FIX: replace with `read_text` from "
"`utils.cache.files`:\n\n"
" from utils.cache.files import read_text\n"
" text = read_text(str(path))\n\n"
"This shares one decoded read per file across the whole "
"pytest session. See docs/agents/action/testing.md.\n\n"
"Only opt out per call with `# nocheck: cache-read` when "
"the read is genuinely outside the project tree "
"(tempdirs, synthetic fixtures written inside the same "
"test, files mutated in the same flow that re-reads "
"them, etc.) and a short inline comment explains why."
)
if __name__ == "__main__":
unittest.main()