mirror of
https://github.com/kevinveenbirkenbach/computer-playbook.git
synced 2026-11-11 21:26:44 +00:00
Extends ``[tool.ruff.lint].select`` with six additional rule families:
``RUF`` (ruff-specific), ``SIM`` (flake8-simplify), ``C4``
(flake8-comprehensions), ``PERF`` (perflint), ``RET`` (flake8-return),
``PIE`` (flake8-pie). Adds ``RUF001/002/003`` to ``ignore`` because
those exist for homoglyph-attack detection, not English typography —
the project uses em/en-dashes and ``ℹ`` as a UI-hint glyph in CLI help
output deliberately.
Auto-fixed (~225, mechanical & behavior-preserving)
====================================================
- RUF005: ``a + [b]`` → ``[*a, b]``
- C408: ``dict(k=v)`` → ``{"k": v}``
- C420: ``dict.fromkeys(seq)`` shape simplifications
- SIM103/SIM110/SIM118: truthy-return / dict-key / dict-method
idiom simplifications
- RET504/RET505: drop unneeded final assigns / ``else`` after
``return``
- PIE810: combine ``str.startswith(a) or .startswith(b)``
into one call with a tuple
- RUF010: ``f"{str(x)}"`` → ``f"{x\!s}"``
- RUF100: removed unused ``# noqa`` directives that
became stale (mostly old ``# noqa: E402``
markers that ruff's smart E402 implementation
no longer flags after ``sys.path`` shims)
- RUF015 / RUF013 / RUF022 / RUF046 / RUF059: misc small wins
- I001: re-sorted import blocks where the new
imports above shifted them
Hand-fixed (~140, scripted where mechanical)
============================================
PERF401 (42): scripted AST rewrite of
``for x in iter: target.append(expr)`` (with optional ``if`` guard)
→ ``target.extend(expr for x in iter [if cond])``.
``list.extend(genexp)`` consumes the generator immediately, so the
rewrite is semantically equivalent to the original eager loop.
SIM117 (41): scripted AST combiner for nested ``with`` blocks where
the inner is the only statement of the outer (38 sites). The remaining
3 multi-line ``patch.object(...)`` chains were combined by hand using
the parenthesised ``with (\\n A,\\n B,\\n):`` form.
SIM102 (14): scripted AST combiner for nested ``if`` (10 sites). The
remaining 4 carry an inline ``# noqa: SIM102`` with rationale —
combining would either change semantics (the outer body has siblings
beyond the inner ``if``, e.g. ``cli/encrypt/inventory/__main__.py``
where ``encrypted_value = …`` runs regardless of ``ask_confirmation``)
or hurt readability (long literal whitelists).
RUF012 (9): annotated mutable class attributes with ``ClassVar[…]``
and added ``from typing import ClassVar``.
SIM105 (6): ``try/except: pass`` → ``with contextlib.suppress(…):``
with the necessary ``import contextlib`` added. Verified on
``utils/templating.py`` (3 occurrences in a ``finally`` block) —
cleanup behaviour is identical.
SIM115 (4): wrapped ``tempfile.NamedTemporaryFile(..., delete=False)``
call patterns in a ``with`` block; ``delete=False`` keeps the file
on disk after the block, so callers' ``return f.name`` still works.
PERF402, PERF403, B007, SIM108, SIM103: 5 misc fixes (loop→list-copy,
loop→dict-comprehension, unused-loop-var rename, ternary, return
expression).
Verification
============
- ``ruff check .`` → All checks passed
- ``ruff format --check .`` → 894 files already formatted
- 1621 unit tests → all pass
- Integration suite: 44 baseline failures, 1 skipped (unchanged)
- Sample-audited PERF401 rewrites in ``resolver.py``, ``inventory.py``,
``cache/files.py``, ``ressources/__main__.py``; SIM117 in
``test_main.py``; SIM105 in ``utils/templating.py``; SIM102 noqa
rationales in ``cli/encrypt/inventory/__main__.py``.
182 files changed, +993 / −960. With this and the prior strict-rule
commit the branch satisfies 11 ruff rule families end-to-end.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
280 lines
8.8 KiB
Python
280 lines
8.8 KiB
Python
from __future__ import annotations
|
|
|
|
import unittest
|
|
from unittest.mock import patch
|
|
|
|
from ansible.errors import AnsibleError
|
|
|
|
from plugins.lookup.service import LookupModule
|
|
|
|
_SERVICE_REGISTRY = {
|
|
"matomo": {"role": "web-app-matomo"},
|
|
"cdn": {"role": "web-svc-cdn"},
|
|
"css": {"role": "web-svc-cdn", "canonical": "cdn"},
|
|
"logout": {"role": "web-svc-logout"},
|
|
"collab": {"role": "web-svc-collab"},
|
|
}
|
|
|
|
|
|
def _run(terms, applications, group_names, service_registry=None):
|
|
patches = [
|
|
patch(
|
|
"plugins.lookup.service.get_merged_applications",
|
|
return_value=applications,
|
|
)
|
|
]
|
|
if service_registry is not None:
|
|
patches.append(
|
|
patch(
|
|
"plugins.lookup.service.build_service_registry_from_applications",
|
|
return_value=service_registry,
|
|
)
|
|
)
|
|
for p in patches:
|
|
p.start()
|
|
try:
|
|
return LookupModule().run(
|
|
terms,
|
|
variables={"group_names": group_names},
|
|
)
|
|
finally:
|
|
for p in reversed(patches):
|
|
p.stop()
|
|
|
|
|
|
class TestServiceDirect(unittest.TestCase):
|
|
def setUp(self):
|
|
self.applications = {
|
|
"web-app-foo": {
|
|
"services": {
|
|
"matomo": {"enabled": True, "shared": True},
|
|
"css": {"enabled": True, "shared": False},
|
|
"logout": {"enabled": False, "shared": True},
|
|
}
|
|
},
|
|
"web-app-bar": {"services": {}},
|
|
}
|
|
|
|
def _get(self, term, group_names=None):
|
|
return _run(
|
|
[term],
|
|
self.applications,
|
|
group_names if group_names is not None else ["web-app-foo"],
|
|
service_registry=_SERVICE_REGISTRY,
|
|
)[0]
|
|
|
|
def test_required_true_when_enabled_and_shared(self):
|
|
self.assertTrue(self._get("matomo")["required"])
|
|
|
|
def test_required_false_when_enabled_only(self):
|
|
self.assertFalse(self._get("cdn")["required"])
|
|
|
|
def test_required_false_when_shared_only(self):
|
|
self.assertFalse(self._get("logout")["required"])
|
|
|
|
def test_required_false_when_service_absent(self):
|
|
self.assertFalse(self._get("collab")["required"])
|
|
|
|
def test_required_false_when_group_names_do_not_match(self):
|
|
self.assertFalse(self._get("matomo", ["web-app-unknown"])["required"])
|
|
|
|
def test_required_true_when_any_app_qualifies(self):
|
|
self.assertTrue(self._get("matomo", ["web-app-bar", "web-app-foo"])["required"])
|
|
|
|
def test_enabled_shared_flags_are_aggregated_via_canonical_alias(self):
|
|
result = self._get("cdn")
|
|
self.assertTrue(result["enabled"])
|
|
self.assertFalse(result["shared"])
|
|
|
|
def test_lookup_by_role_name_uses_same_primary_service(self):
|
|
by_key = self._get("cdn")
|
|
by_role = self._get("web-svc-cdn")
|
|
self.assertEqual(by_key, by_role)
|
|
|
|
def test_multiple_terms(self):
|
|
results = _run(
|
|
["matomo", "cdn", "logout"],
|
|
self.applications,
|
|
["web-app-foo"],
|
|
service_registry=_SERVICE_REGISTRY,
|
|
)
|
|
self.assertEqual(len(results), 3)
|
|
self.assertTrue(results[0]["required"])
|
|
self.assertFalse(results[1]["required"])
|
|
self.assertFalse(results[2]["required"])
|
|
|
|
def test_empty_terms_returns_empty(self):
|
|
self.assertEqual(
|
|
_run(
|
|
[],
|
|
self.applications,
|
|
["web-app-foo"],
|
|
service_registry=_SERVICE_REGISTRY,
|
|
),
|
|
[],
|
|
)
|
|
|
|
|
|
class TestServiceTransitive(unittest.TestCase):
|
|
def setUp(self):
|
|
self.applications = {
|
|
"web-app-nextcloud": {"services": {"collab": {"enabled": True}}},
|
|
"web-svc-collab": {
|
|
"services": {
|
|
"matomo": {"enabled": True, "shared": True},
|
|
}
|
|
},
|
|
}
|
|
|
|
def test_transitive_required_resolved(self):
|
|
result = _run(
|
|
["matomo"],
|
|
self.applications,
|
|
["web-app-nextcloud"],
|
|
service_registry=_SERVICE_REGISTRY,
|
|
)[0]
|
|
self.assertTrue(result["required"])
|
|
|
|
def test_transitive_requires_shared_at_target(self):
|
|
applications = {
|
|
"web-app-nextcloud": {"services": {"collab": {"enabled": True}}},
|
|
"web-svc-collab": {"services": {"matomo": {"enabled": True}}},
|
|
}
|
|
result = _run(
|
|
["matomo"],
|
|
applications,
|
|
["web-app-nextcloud"],
|
|
service_registry=_SERVICE_REGISTRY,
|
|
)[0]
|
|
self.assertFalse(result["required"])
|
|
|
|
def test_direct_provider_is_not_required_without_shared_flag(self):
|
|
result = _run(
|
|
["collab"],
|
|
self.applications,
|
|
["web-app-nextcloud"],
|
|
service_registry=_SERVICE_REGISTRY,
|
|
)[0]
|
|
self.assertFalse(result["required"])
|
|
|
|
|
|
class TestServiceDiscoveryWithoutExplicitRegistry(unittest.TestCase):
|
|
def test_role_local_provider_metadata_is_discovered_from_applications(self):
|
|
applications = {
|
|
"web-app-dashboard": {
|
|
"services": {
|
|
"dashboard": {"enabled": False, "shared": True},
|
|
}
|
|
},
|
|
"web-app-foo": {
|
|
"services": {
|
|
"dashboard": {"enabled": True, "shared": True},
|
|
}
|
|
},
|
|
}
|
|
|
|
result = _run(["dashboard"], applications, ["web-app-foo"])[0]
|
|
self.assertEqual(result["role"], "web-app-dashboard")
|
|
self.assertTrue(result["required"])
|
|
|
|
def test_provides_and_canonical_are_discovered_from_provider_configs(self):
|
|
applications = {
|
|
"web-app-keycloak": {
|
|
"services": {
|
|
"keycloak": {
|
|
"enabled": False,
|
|
"shared": True,
|
|
"provides": "oidc",
|
|
}
|
|
}
|
|
},
|
|
"web-svc-cdn": {
|
|
"services": {
|
|
"cdn": {"enabled": False, "shared": True},
|
|
"css": {"enabled": False, "shared": True, "canonical": "cdn"},
|
|
}
|
|
},
|
|
"web-app-foo": {
|
|
"services": {
|
|
"oidc": {"enabled": True, "shared": True},
|
|
"css": {"enabled": True, "shared": False},
|
|
}
|
|
},
|
|
}
|
|
|
|
oidc = _run(["oidc"], applications, ["web-app-foo"])[0]
|
|
cdn = _run(["cdn"], applications, ["web-app-foo"])[0]
|
|
|
|
self.assertEqual(oidc["role"], "web-app-keycloak")
|
|
self.assertTrue(oidc["required"])
|
|
self.assertEqual(cdn["role"], "web-svc-cdn")
|
|
self.assertTrue(cdn["enabled"])
|
|
self.assertFalse(cdn["required"])
|
|
|
|
|
|
class TestServiceCycleGuard(unittest.TestCase):
|
|
def test_cycle_does_not_loop(self):
|
|
applications = {
|
|
"svc-a": {"services": {"svc-b": {"enabled": True}}},
|
|
"svc-b": {"services": {"svc-a": {"enabled": True}}},
|
|
}
|
|
service_registry = {
|
|
"svc-a": {"role": "svc-a"},
|
|
"svc-b": {"role": "svc-b"},
|
|
"logout": {"role": "web-svc-logout"},
|
|
}
|
|
result = _run(
|
|
["logout"],
|
|
applications,
|
|
["svc-a"],
|
|
service_registry=service_registry,
|
|
)[0]
|
|
self.assertFalse(result["required"])
|
|
|
|
def test_cycle_found_if_service_present(self):
|
|
applications = {
|
|
"svc-a": {
|
|
"services": {
|
|
"svc-b": {"enabled": True},
|
|
"logout": {"enabled": True, "shared": True},
|
|
}
|
|
},
|
|
"svc-b": {"services": {"svc-a": {"enabled": True}}},
|
|
}
|
|
service_registry = {
|
|
"svc-a": {"role": "svc-a"},
|
|
"svc-b": {"role": "svc-b"},
|
|
"logout": {"role": "web-svc-logout"},
|
|
}
|
|
result = _run(
|
|
["logout"],
|
|
applications,
|
|
["svc-b"],
|
|
service_registry=service_registry,
|
|
)[0]
|
|
self.assertTrue(result["required"])
|
|
|
|
|
|
class TestServiceErrors(unittest.TestCase):
|
|
def test_raises_when_group_names_not_list(self):
|
|
with (
|
|
patch("plugins.lookup.service.get_merged_applications", return_value={}),
|
|
self.assertRaises(AnsibleError),
|
|
):
|
|
LookupModule().run(
|
|
["matomo"],
|
|
variables={"group_names": "not-a-list"},
|
|
)
|
|
|
|
def test_raises_when_term_empty(self):
|
|
with self.assertRaises(AnsibleError):
|
|
_run([" "], {}, [], service_registry=_SERVICE_REGISTRY)
|
|
|
|
def test_raises_when_term_unknown(self):
|
|
with self.assertRaises(AnsibleError):
|
|
_run(["totally-unknown-key"], {}, [], service_registry=_SERVICE_REGISTRY)
|
|
|
|
|
|
if __name__ == "__main__":
|
|
unittest.main()
|