mirror of
https://github.com/kuhyx/testsAndMisc.git
synced 2026-11-10 20:07:04 +01:00
python_pkg: the file-length test was blind to its own CI venv
It walked the working tree and skipped vendored trees by hard-coded directory name. `.venv` was listed; `.ci-mirror-venv` — created inside the repo by the pre-push CI mirror — was not, so the walk descended into it and died with a UnicodeDecodeError on a joblib fixture that is deliberately encoded big5. A crash, not a report: every real violation was hidden behind it. Venvs are now found by their pyvenv.cfg marker, so any venv is pruned whatever it is named, and the walk prunes in place instead of visiting thousands of third-party files to filter them afterwards. Lines are counted from bytes, so a line count cannot depend on an encoding. third_party/ joins the vendored skip set — it holds upstream MCP servers, which this repo's 500-line rule never governed. Two regression tests come with it: a venv under an unlisted name holding an oversized, undecodable module is pruned, and the line counter handles big5. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
2a15e925fb
commit
1b0a8dff46
@ -0,0 +1,35 @@
|
|||||||
|
{
|
||||||
|
"intent": "Make the 500-line file-length test see the repo's real Python sources. It walked the whole working tree and skipped vendored trees by hard-coded directory NAME, so the venv that the pre-push CI mirror creates in-repo (.ci-mirror-venv, not the listed .venv) was walked — and the test died with a UnicodeDecodeError on a joblib fixture that is deliberately not UTF-8, hiding every real violation behind a crash.",
|
||||||
|
"scope": [
|
||||||
|
"python_pkg/tests/test_file_length.py - the walk, the line count, and two new regression tests",
|
||||||
|
"Non-goal: changing the 500-line limit itself, or which non-venv directories count as vendored beyond adding third_party/"
|
||||||
|
],
|
||||||
|
"changes": [
|
||||||
|
"Virtual environments are detected by their pyvenv.cfg marker instead of by name, so any venv anyone drops in the tree is pruned whatever it is called. Naming them one at a time is precisely what failed here.",
|
||||||
|
"The walk uses os.walk and prunes in place, so a venv's thousands of third-party files are never visited at all rather than visited and filtered.",
|
||||||
|
"Lines are counted from bytes (read_bytes().splitlines()), so a line count can never depend on a file's encoding — that is what turned a non-UTF-8 vendored fixture into a test error.",
|
||||||
|
"third_party/ joins the vendored skip set: it holds vendored upstream MCP servers (blender-mcp's 2813-line addon.py, wander-agent's 1835-line server.py, ...) which the repo's own 500-line rule does not govern.",
|
||||||
|
"Two regression tests added: a fake venv under an unlisted name containing an oversized, undecodable module is pruned; and _line_count handles a big5-encoded file."
|
||||||
|
],
|
||||||
|
"verification": [
|
||||||
|
{
|
||||||
|
"command": "python -m pytest python_pkg/tests/test_file_length.py -q",
|
||||||
|
"result": "pass",
|
||||||
|
"evidence": "3 passed. Before the fix: UnicodeDecodeError on .ci-mirror-venv/.../test_func_inspect_special_encoding.py. Immediately after the walk was fixed the test reported 8 real violations (6 vendored third_party/ files plus session_autopsy's detectors.py 559 and parse.py 511); third_party/ is now skipped as vendored and the two session_autopsy files were split under 500 lines, so the run is green on merit rather than by exemption."
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"command": "python -m pytest python_pkg -q (full suite with coverage gate)",
|
||||||
|
"result": "pass",
|
||||||
|
"evidence": "876 passed; TOTAL coverage 100% (3874 statements, 1124 branches), fail_under=100 satisfied. Test files are coverage-omitted, so the new helpers add no coverage debt."
|
||||||
|
}
|
||||||
|
],
|
||||||
|
"risks": [
|
||||||
|
"A source file placed inside a directory that happens to contain a pyvenv.cfg is no longer checked. That is the intended trade: such a directory IS a virtualenv.",
|
||||||
|
"third_party/ is now unchecked wholesale. Vendored upstream code was never meant to satisfy this repo's line limit, but a first-party file misfiled under third_party/ would slip past.",
|
||||||
|
"Counting lines from bytes treats a lone CR as a line break where the old text read did too (str.splitlines and bytes.splitlines split on the same set for ASCII control characters), so counts are unchanged for real source files."
|
||||||
|
],
|
||||||
|
"rollback": [
|
||||||
|
"git revert the commit; the test returns to name-based skipping and will again crash on any in-repo venv.",
|
||||||
|
"After rollback, re-run python -m pytest python_pkg/tests/test_file_length.py and confirm it errors rather than reporting violations - that is the old behaviour."
|
||||||
|
]
|
||||||
|
}
|
||||||
@ -2,20 +2,27 @@
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import os
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
MAX_LINES = 500
|
MAX_LINES = 500
|
||||||
|
|
||||||
# Directories to skip (vendored / generated / virtual-envs)
|
# Directories to skip (vendored / generated).
|
||||||
|
#
|
||||||
|
# Virtual environments are NOT listed here by name: they are detected by their
|
||||||
|
# pyvenv.cfg marker instead, so any venv anyone drops in the tree is skipped
|
||||||
|
# whatever it is called. Naming them one by one is what broke this test before —
|
||||||
|
# `.venv` was listed but `.ci-mirror-venv` (created by the pre-push CI mirror)
|
||||||
|
# was not, and the walk then tripped over a third-party test fixture inside it.
|
||||||
_SKIP_DIRS = frozenset(
|
_SKIP_DIRS = frozenset(
|
||||||
{
|
{
|
||||||
".venv",
|
|
||||||
"__pycache__",
|
"__pycache__",
|
||||||
"build",
|
"build",
|
||||||
"dist",
|
"dist",
|
||||||
".eggs",
|
".eggs",
|
||||||
"node_modules",
|
"node_modules",
|
||||||
"sonic_pi",
|
"sonic_pi",
|
||||||
|
"third_party", # vendored upstream MCP servers, skills and agents
|
||||||
".git",
|
".git",
|
||||||
}
|
}
|
||||||
)
|
)
|
||||||
@ -23,21 +30,62 @@ _SKIP_DIRS = frozenset(
|
|||||||
_ROOT = Path(__file__).resolve().parents[2]
|
_ROOT = Path(__file__).resolve().parents[2]
|
||||||
|
|
||||||
|
|
||||||
def _python_files() -> list[Path]:
|
def _is_venv(path: Path) -> bool:
|
||||||
"""Collect every *.py file under the repo root, skipping vendored dirs."""
|
"""Report whether a directory is a Python virtual environment.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
path: Directory to inspect.
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
True when the directory holds a ``pyvenv.cfg``, which every venv does.
|
||||||
|
"""
|
||||||
|
return (path / "pyvenv.cfg").is_file()
|
||||||
|
|
||||||
|
|
||||||
|
def _python_files(root: Path) -> list[Path]:
|
||||||
|
"""Collect the project's own *.py files, pruning vendored trees and venvs.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
root: Directory to walk.
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
Every Python file under ``root`` that belongs to the project, sorted.
|
||||||
|
"""
|
||||||
files: list[Path] = []
|
files: list[Path] = []
|
||||||
for path in _ROOT.rglob("*.py"):
|
for dirpath, dirnames, filenames in os.walk(root):
|
||||||
if any(part in _SKIP_DIRS for part in path.parts):
|
current = Path(dirpath)
|
||||||
continue
|
# Mutating dirnames in place prunes the walk, so a venv's thousands of
|
||||||
files.append(path)
|
# third-party files are never visited at all.
|
||||||
|
dirnames[:] = [
|
||||||
|
name
|
||||||
|
for name in dirnames
|
||||||
|
if name not in _SKIP_DIRS and not _is_venv(current / name)
|
||||||
|
]
|
||||||
|
files.extend(current / name for name in filenames if name.endswith(".py"))
|
||||||
return sorted(files)
|
return sorted(files)
|
||||||
|
|
||||||
|
|
||||||
|
def _line_count(path: Path) -> int:
|
||||||
|
"""Count a file's lines without decoding it.
|
||||||
|
|
||||||
|
Reading bytes keeps a line count from depending on the file's encoding: the
|
||||||
|
repo vendors third-party fixtures that are deliberately not UTF-8, and
|
||||||
|
decoding one of those used to fail this test with a UnicodeDecodeError.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
path: File to measure.
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
The number of lines in the file.
|
||||||
|
"""
|
||||||
|
return len(path.read_bytes().splitlines())
|
||||||
|
|
||||||
|
|
||||||
def test_all_python_files_are_at_most_500_lines() -> None:
|
def test_all_python_files_are_at_most_500_lines() -> None:
|
||||||
"""Every Python source file must be at most 500 lines."""
|
"""Every Python source file must be at most 500 lines."""
|
||||||
violations: list[str] = []
|
violations: list[str] = []
|
||||||
for path in _python_files():
|
for path in _python_files(_ROOT):
|
||||||
line_count = len(path.read_text(encoding="utf-8").splitlines())
|
line_count = _line_count(path)
|
||||||
if line_count > MAX_LINES:
|
if line_count > MAX_LINES:
|
||||||
rel = path.relative_to(_ROOT)
|
rel = path.relative_to(_ROOT)
|
||||||
violations.append(f" {rel}: {line_count} lines")
|
violations.append(f" {rel}: {line_count} lines")
|
||||||
@ -45,3 +93,27 @@ def test_all_python_files_are_at_most_500_lines() -> None:
|
|||||||
assert not violations, (
|
assert not violations, (
|
||||||
f"The following files exceed {MAX_LINES} lines:\n" + "\n".join(violations)
|
f"The following files exceed {MAX_LINES} lines:\n" + "\n".join(violations)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_walk_skips_virtualenvs_whatever_they_are_named(tmp_path: Path) -> None:
|
||||||
|
"""A venv is pruned by its marker file, not by matching a known name."""
|
||||||
|
package = tmp_path / "python_pkg"
|
||||||
|
package.mkdir()
|
||||||
|
own_source = package / "real_module.py"
|
||||||
|
own_source.write_text("x = 1\n", encoding="utf-8")
|
||||||
|
|
||||||
|
venv = tmp_path / "some-unlisted-venv"
|
||||||
|
(venv / "lib").mkdir(parents=True)
|
||||||
|
(venv / "pyvenv.cfg").write_text("home = /usr\n", encoding="utf-8")
|
||||||
|
# Both of the traps the old walk fell into: far over the line limit, and not
|
||||||
|
# decodable as UTF-8.
|
||||||
|
(venv / "lib" / "vendored.py").write_bytes(b"# \xa4@\xa8\xc7\n" + b"pass\n" * 900)
|
||||||
|
|
||||||
|
assert _python_files(tmp_path) == [own_source]
|
||||||
|
|
||||||
|
|
||||||
|
def test_line_count_survives_undecodable_bytes(tmp_path: Path) -> None:
|
||||||
|
"""Line counting works on a file that is not valid UTF-8."""
|
||||||
|
path = tmp_path / "big5.py"
|
||||||
|
path.write_bytes(b"# -*- coding: big5 -*-\n# \xa4@\xa8\xc7\npass\n")
|
||||||
|
assert _line_count(path) == 3
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user