mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-28 19:01:34 +00:00
test: the sys.modules guard now sees the write, not its spelling (#398)
The guard introduced by #394 matched the literal sys.modules['custom_components... and therefore never looked at pure_imports.py, which writes through a variable — the third instance of the #389 class walked straight past the check created for it. The guard now inspects the write itself and decides by the key: a whole literal or the literal head of an f-string is safe unless it starts with custom_components (that is how tests register homeassistant.*, hp_pure.* and houseplan.trails); anything else — a variable, a concatenation, setdefault/update — counts as a violation whenever the file is able to name the package at all, i.e. contains a custom_components. literal. A file that never names the package cannot poison it through a variable, so restoring a snapshot stays legal. load_pure now removes what it registered. Removing its own name is not enough: relative imports pull neighbours in, so junction_limits leaves wall_segment_model and coordinate_canonicalization behind. It removes the whole custom_components difference accumulated during exec_module, in a finally, and a repeated call still works. pure_imports.py is a named exemption of the static guard precisely because that guard cannot see the cleanup — so the cleanup is proven by an executable test instead, and the mutant pure-imports-stops-cleaning reddens it. Both mutants were run by hand. User-Visible: no Issue: #398
This commit is contained in:
@@ -1567,6 +1567,13 @@ their passports; `scripts/config-audit.mjs` treats both as `current`.
|
||||
- `pyproject.toml` configures ruff (E/F/B/I, E501 excluded by decision) and
|
||||
mypy strict for a grow-only allowlist of pure modules; the completeness
|
||||
guard lives in `tests_backend/test_backend_quality.py`.
|
||||
- Writing into `sys.modules` from a backend test is refused by
|
||||
`test/backend-test-hygiene.test.mjs` — by the fact of the write, not by its
|
||||
spelling (#398). Two files are named exemptions: `conftest.py`, whose stub is
|
||||
conditional on Home Assistant being absent, and `pure_imports.py`, which
|
||||
registers a module only for the duration of `exec_module` and removes the
|
||||
whole `custom_components` difference afterwards — the removal itself is
|
||||
proven by an executable test, because the static guard cannot see it.
|
||||
- Both linters RUN in the backend CI job: the typing step derives its module
|
||||
list from the `pyproject.toml` allowlist rather than repeating it, refuses an
|
||||
empty list, and is itself guarded by a test plus the `typing-gate-stops-
|
||||
|
||||
@@ -746,6 +746,28 @@ const MUTANT_DEFINITIONS = [
|
||||
replace: " this._persistDecorStyle();\n }, 0);",
|
||||
}],
|
||||
},
|
||||
{
|
||||
id: 'sysmodules-guard-blind-to-variable',
|
||||
guard: 'node --test test/backend-test-hygiene.test.mjs',
|
||||
because: 'a guard that only sees string literals let the third instance of '
|
||||
+ 'the #389 class walk past it (#398 AC1)',
|
||||
patches: [{
|
||||
file: 'test/backend-test-hygiene.test.mjs',
|
||||
find: ' if (start === null) { if (namesThePackage) hits.push(match[0]); continue; }',
|
||||
replace: ' if (start === null) { continue; }',
|
||||
}],
|
||||
},
|
||||
{
|
||||
id: 'pure-imports-stops-cleaning',
|
||||
guard: 'python3 -m pytest tests_backend/test_backend_quality.py -q -p no:cacheprovider',
|
||||
because: 'pure_imports is allowed to write into sys.modules only because it '
|
||||
+ 'removes the entries again; without that the exemption is a hole (#398 AC8)',
|
||||
patches: [{
|
||||
file: 'tests_backend/pure_imports.py',
|
||||
find: ' del sys.modules[key]',
|
||||
replace: ' pass',
|
||||
}],
|
||||
},
|
||||
{
|
||||
id: 'device-echo-keeps-local-noncanonical',
|
||||
guard: 'node demo/smoke_device_position_history.mjs',
|
||||
|
||||
@@ -14,9 +14,63 @@ import { fileURLToPath } from 'node:url';
|
||||
// а исходник ловит намерение.
|
||||
|
||||
const BACKEND = fileURLToPath(new URL('../tests_backend/', import.meta.url));
|
||||
|
||||
/** Файлы, которым запись в `sys.modules` разрешена поимённо (#398 AC3).
|
||||
*
|
||||
* Список короткий не случайно. `conftest.py` ставит пустышки родительских
|
||||
* пакетов, когда Home Assistant недоступен, — по условию и осознанно (#394).
|
||||
* `pure_imports.py` регистрирует загружаемый модуль ровно на время
|
||||
* `exec_module` и снимает регистрацию в `finally`; статический разбор этого
|
||||
* знать не может, поэтому файл назван здесь, а то, что он действительно
|
||||
* убирает за собой, доказывает исполняемая проверка ниже. */
|
||||
const SYS_MODULES_WRITERS = ['conftest.py', 'pure_imports.py'];
|
||||
|
||||
const files = () => readdirSync(BACKEND).filter((name) => name.endsWith('.py'));
|
||||
const read = (name) => readFileSync(new URL(name, `file://${BACKEND}`), 'utf8');
|
||||
|
||||
/** Записи в `sys.modules`, про которые нельзя доказать, что пакет интеграции
|
||||
* они не трогают (#398).
|
||||
*
|
||||
* Прежняя проверка искала строковый литерал `sys.modules['custom_components`
|
||||
* и не видела `sys.modules[name] = module` — так третий заход класса #389
|
||||
* проехал мимо гварда, заведённого ради него. Здесь разбирается сам факт
|
||||
* записи, а решение принимается по ключу:
|
||||
*
|
||||
* - ключ — литерал или f-строка с литеральным началом: безопасно, если это
|
||||
* начало не `custom_components` (так тесты ставят `homeassistant.*`,
|
||||
* `hp_pure.*`, `houseplan.trails` — свои имена, к пакету не относящиеся);
|
||||
* - ключ — выражение, либо это `setdefault`/`update`: судить о содержимом
|
||||
* статически нельзя, поэтому запись считается нарушением, КОГДА файл вообще
|
||||
* способен назвать пакет интеграции — то есть содержит литерал вида
|
||||
* `custom_components.` (с точкой: это имя модуля, а не кусок пути).
|
||||
*
|
||||
* Последнее и есть fail-closed в осмысленной форме: файл, который нигде не
|
||||
* упоминает имя пакета, отравить его переменной не может, а файл, который
|
||||
* упоминает, обязан делать это разрешённым способом. */
|
||||
function sysModulesWrites(source) {
|
||||
const code = source.replace(/#[^\n]*/g, '');
|
||||
const namesThePackage = /(['"])custom_components\./.test(code);
|
||||
// Начало ключа, если оно доказуемо литеральное: целая строка либо f-строка
|
||||
// до первой подстановки. Всё прочее (конкатенация, вызов, переменная) — не
|
||||
// литерал: доказать по нему ничего нельзя.
|
||||
const literalStart = (key) => {
|
||||
const whole = /^(['"])((?:[^'"\\]|\\.)*)\1$/.exec(key.trim());
|
||||
if (whole) return whole[2];
|
||||
const fstring = /^f(['"])((?:[^'"\\{]|\\.)*)/.exec(key.trim());
|
||||
return fstring ? fstring[2] : null;
|
||||
};
|
||||
const hits = [];
|
||||
for (const match of code.matchAll(/sys\.modules\s*\[([^\]]*)\]\s*=/g)) {
|
||||
const start = literalStart(match[1]);
|
||||
if (start === null) { if (namesThePackage) hits.push(match[0]); continue; }
|
||||
if (start.startsWith('custom_components')) hits.push(match[0]);
|
||||
}
|
||||
for (const match of code.matchAll(/sys\.modules\.(?:setdefault|update)\s*\(/g)) {
|
||||
if (namesThePackage) hits.push(match[0]);
|
||||
}
|
||||
return hits;
|
||||
}
|
||||
|
||||
test('backend-тесты не правят sys.path (#393)', () => {
|
||||
const offenders = files().filter((name) => /^\s*sys\.path\b/m.test(read(name)));
|
||||
assert.deepEqual(offenders, [],
|
||||
@@ -36,12 +90,22 @@ test('пакет интеграции подменяет только conftest
|
||||
// она не срабатывала лишь потому, что настоящий пакет успевал импортироваться
|
||||
// из файла, который идёт раньше по алфавиту. Корректность харнесса держалась
|
||||
// на именах файлов; чем это кончается, показал #389.
|
||||
const assigns = /sys\.modules\[\s*(['"])custom_components/;
|
||||
const offenders = files().filter((name) => name !== 'conftest.py' && assigns.test(read(name)));
|
||||
// #398: проверка смотрит на ФАКТ записи в `sys.modules`, а не на её запись
|
||||
// строковым литералом. Прежняя регулярка искала `sys.modules['custom_...`
|
||||
// и не видела `sys.modules[name] = module` в pure_imports.py — третий заход
|
||||
// того же класса проехал мимо гварда, заведённого ради него.
|
||||
//
|
||||
// Разбор грубый и fail-closed: ключ, про который нельзя доказать, что он не
|
||||
// из `custom_components`, считается нарушением. Догонять формы записи
|
||||
// регуляркой — бесконечная гонка, отказывать на непонятном — нет.
|
||||
const offenders = files()
|
||||
.filter((name) => !SYS_MODULES_WRITERS.includes(name))
|
||||
.filter((name) => sysModulesWrites(read(name)).length > 0);
|
||||
assert.deepEqual(offenders, [],
|
||||
'подмена пакета интеграции живёт в tests_backend/conftest.py и только там:'
|
||||
+ ' там она условная (нет Home Assistant — нечего ломать), а в тесте она'
|
||||
+ ' переживает свой тест и достаётся всей сессии (#389, #394).');
|
||||
'запись в sys.modules живёт только в tests_backend/conftest.py (условная'
|
||||
+ ' подмена пакета) и tests_backend/pure_imports.py (регистрация, снимаемая'
|
||||
+ ' тем же вызовом). В остальных файлах она переживает свой тест и'
|
||||
+ ' достаётся всей сессии (#389, #394, #398).');
|
||||
|
||||
const conftest = read('conftest.py');
|
||||
assert.match(conftest, /if not HAS_HA:/,
|
||||
@@ -49,3 +113,64 @@ test('пакет интеграции подменяет только conftest
|
||||
const stub = conftest.slice(conftest.indexOf('if not HAS_HA:'));
|
||||
assert.match(stub, /sys\.modules\[_name\] = _module/);
|
||||
});
|
||||
|
||||
// --- #398: сам гвард обязан ловить свой класс ----------------------------
|
||||
// Проверка, которая не умеет краснеть, — это не проверка. Формы записи
|
||||
// перечислены поимённо, потому что каждая из них уже встречалась в проекте
|
||||
// или на расстоянии одной правки от встречавшейся.
|
||||
|
||||
const guardSees = (code) => sysModulesWrites(code).length > 0;
|
||||
// Файл, который где-то называет пакет интеграции как модуль — именно то
|
||||
// условие, при котором запись «неизвестно чем» становится опасной.
|
||||
const PACKAGE_NAMED = 'PKG = "custom_components.houseplan"\n';
|
||||
|
||||
test('#398 AC1: запись через переменную не проходит мимо гварда', () => {
|
||||
assert.equal(guardSees(`${PACKAGE_NAMED}sys.modules[name] = module\n`), true,
|
||||
'ключ-переменная — ровно та форма, которую прежняя регулярка не видела');
|
||||
});
|
||||
|
||||
test('#398 AC2: остальные формы записи тоже ловятся или отклоняются', () => {
|
||||
for (const line of [
|
||||
'sys.modules[f"custom_components.{name}"] = module',
|
||||
'sys.modules["custom_" + "components.houseplan"] = module',
|
||||
'sys.modules.setdefault("custom_components.houseplan", module)',
|
||||
'sys.modules.update({"custom_components.houseplan": module})',
|
||||
'sys.modules[ key ] = module',
|
||||
]) {
|
||||
assert.equal(guardSees(`${PACKAGE_NAMED}${line}\n`), true, `не поймана форма: ${line}`);
|
||||
}
|
||||
});
|
||||
|
||||
test('#398 AC2: безопасное не объявляется нарушением', () => {
|
||||
for (const line of [
|
||||
'sys.modules["homeassistant.core"] = core',
|
||||
'sys.modules[f"hp_pure.{dep}"] = mod',
|
||||
'sys.modules["houseplan.trails"] = mod',
|
||||
'# sys.modules["custom_components.houseplan"] = stub # так писать нельзя',
|
||||
]) {
|
||||
assert.equal(guardSees(`${line}\n`), false, `ложное срабатывание на: ${line}`);
|
||||
}
|
||||
// Файл, который нигде не называет пакет интеграции, отравить его
|
||||
// переменной не может — восстановление снимка остаётся законным.
|
||||
assert.equal(guardSees('sys.modules.update(saved)\n'), false);
|
||||
});
|
||||
|
||||
test('#398 AC3: список файлов с правом записи закрыт', () => {
|
||||
assert.deepEqual(SYS_MODULES_WRITERS, ['conftest.py', 'pure_imports.py'],
|
||||
'третий файл в списке — это новое исключение, а не правка: оно требует'
|
||||
+ ' своего обоснования и отдельного решения, а не молчаливого добавления');
|
||||
});
|
||||
|
||||
test('#398 AC8: pure_imports снимает регистрацию, а не обещает', () => {
|
||||
// Файл в списке исключений не потому, что ему доверяют, а потому, что
|
||||
// статический разбор не видит очистки. Значит очистку проверяет этот тест —
|
||||
// иначе исключение стало бы дырой ровно того размера, что #389.
|
||||
const source = read('pure_imports.py');
|
||||
assert.match(source, /before = frozenset\(sys\.modules\)/,
|
||||
'снимок берётся до загрузки');
|
||||
assert.match(source, /finally:[\s\S]*del sys\.modules\[key\]/,
|
||||
'разница снимается в finally — даже если exec_module бросил');
|
||||
assert.match(source, /key\.startswith\(PACKAGE_ROOT\.name\)/,
|
||||
'снимается вся разница под префиксом пакета, а не одно собственное имя:'
|
||||
+ ' относительные импорты подтягивают соседей');
|
||||
});
|
||||
|
||||
@@ -21,13 +21,33 @@ HOUSEPLAN_ROOT = PACKAGE_ROOT / "houseplan"
|
||||
|
||||
|
||||
def load_pure(name: str, file: Path):
|
||||
"""Загрузить модуль по пути под указанным именем.
|
||||
"""Загрузить модуль по пути под указанным именем и убрать за собой.
|
||||
|
||||
Имя значимо: относительные импорты внутри модуля резолвятся только тогда,
|
||||
когда модуль знает, какому пакету принадлежит.
|
||||
|
||||
Регистрация в `sys.modules` обязательна на время `exec_module` и вредна
|
||||
после (#398). Она переживала вызов и доставалась всей сессии — тот же
|
||||
класс, что #389: загрузчик Home Assistant получал бы модуль, собранный
|
||||
мимо него, а объекты классов одного файла оказывались бы разными. Снимать
|
||||
только собственное имя мало: относительные импорты подтягивают соседей
|
||||
(`junction_limits` тянет `wall_segment_model` и
|
||||
`coordinate_canonicalization`), поэтому снимается вся разница под
|
||||
префиксом `custom_components`, появившаяся за время загрузки.
|
||||
|
||||
`conftest.py` ставит пустышки родительских пакетов, когда Home Assistant
|
||||
недоступен, — они принадлежат ему и здесь не трогаются.
|
||||
"""
|
||||
before = frozenset(sys.modules)
|
||||
spec = importlib.util.spec_from_file_location(name, file)
|
||||
module = importlib.util.module_from_spec(spec)
|
||||
sys.modules[name] = module
|
||||
spec.loader.exec_module(module)
|
||||
try:
|
||||
spec.loader.exec_module(module)
|
||||
finally:
|
||||
for key in [
|
||||
key for key in sys.modules
|
||||
if key not in before and key.startswith(PACKAGE_ROOT.name)
|
||||
]:
|
||||
del sys.modules[key]
|
||||
return module
|
||||
|
||||
@@ -157,3 +157,42 @@ def test_issue_42_every_noqa_carries_a_reason():
|
||||
explanation = comment.split(" ", 1)[1] if " " in comment.strip() else ""
|
||||
assert len(explanation.strip()) >= 10, (
|
||||
f"{path.name}:{index}: a bare noqa hides a decision — add the reason")
|
||||
|
||||
|
||||
def test_issue_398_pure_imports_leaves_sys_modules_as_it_found_it():
|
||||
"""`load_pure` не оставляет следов в `sys.modules` (#398 AC4).
|
||||
|
||||
Статический гвард (`test/backend-test-hygiene.test.mjs`) разрешает этому
|
||||
файлу писать в `sys.modules`, потому что не видит очистки. Значит очистку
|
||||
обязан доказывать исполняемый тест — иначе разрешение стало бы дырой
|
||||
ровно того размера, что #389: пустышка переживала свой тест, Home
|
||||
Assistant не мог поднять интеграцию, и 85 тестов харнесса падали с голым
|
||||
`assert False`.
|
||||
|
||||
Проверяется именно РАЗНИЦА, а не пустота: пустышки родительских пакетов
|
||||
ставит conftest, они принадлежат ему и остаются.
|
||||
"""
|
||||
import sys
|
||||
|
||||
from tests_backend.pure_imports import HOUSEPLAN_ROOT, load_pure
|
||||
|
||||
before = sorted(k for k in sys.modules if k.startswith("custom_components"))
|
||||
module = load_pure(
|
||||
"custom_components.houseplan.junction_limits",
|
||||
HOUSEPLAN_ROOT / "junction_limits.py",
|
||||
)
|
||||
assert dir(module), "модуль обязан быть рабочим после очистки"
|
||||
after = sorted(k for k in sys.modules if k.startswith("custom_components"))
|
||||
assert after == before, (
|
||||
f"load_pure оставил в sys.modules: {sorted(set(after) - set(before))} — "
|
||||
"относительные импорты подтягивают соседей, снимать нужно всю разницу"
|
||||
)
|
||||
|
||||
# Повторный вызов обязан работать так же: очистка не должна ломать
|
||||
# следующий заход (тесты вызывают load_pure по нескольку раз за сессию).
|
||||
again = load_pure(
|
||||
"custom_components.houseplan.junction_limits",
|
||||
HOUSEPLAN_ROOT / "junction_limits.py",
|
||||
)
|
||||
assert dir(again)
|
||||
assert sorted(k for k in sys.modules if k.startswith("custom_components")) == before
|
||||
|
||||
Reference in New Issue
Block a user