mirror of
https://github.com/kevinveenbirkenbach/computer-playbook.git
synced 2026-11-10 04:36:40 +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>
226 lines
7.1 KiB
Python
226 lines
7.1 KiB
Python
import os
|
|
import subprocess
|
|
import sys
|
|
import unittest
|
|
|
|
from cli.deploy.container import command as deploy_container
|
|
|
|
|
|
class TestEnsureImage(unittest.TestCase):
|
|
@unittest.mock.patch("subprocess.run")
|
|
def test_ensure_image_skips_build_when_image_exists(self, mock_run):
|
|
"""
|
|
If the image already exists, ensure_image should only call
|
|
`docker image inspect` and NOT run `docker build`.
|
|
"""
|
|
# docker image inspect → rc=0 (image exists)
|
|
mock_run.return_value = subprocess.CompletedProcess(
|
|
args=["docker", "image", "inspect", "infinito:latest"],
|
|
returncode=0,
|
|
stdout="",
|
|
stderr="",
|
|
)
|
|
|
|
deploy_container.ensure_image("infinito:latest")
|
|
|
|
# Exactly one call: docker image inspect
|
|
self.assertEqual(mock_run.call_count, 1)
|
|
cmd = mock_run.call_args_list[0].args[0]
|
|
self.assertEqual(cmd[:3], ["docker", "image", "inspect"])
|
|
|
|
# Ensure docker build was never called
|
|
self.assertFalse(
|
|
any(
|
|
call.args[0][:2] == ["docker", "build"]
|
|
for call in mock_run.call_args_list
|
|
),
|
|
"docker build should not run when the image already exists",
|
|
)
|
|
|
|
@unittest.mock.patch("subprocess.run")
|
|
def test_ensure_image_builds_when_missing(self, mock_run):
|
|
"""
|
|
If the image does not exist, ensure_image should call
|
|
`docker image inspect` first and then `docker build`.
|
|
"""
|
|
calls: list[list[str]] = []
|
|
|
|
def _side_effect(cmd, *args, **kwargs):
|
|
calls.append(cmd)
|
|
|
|
# First: docker image inspect → rc=1 (missing)
|
|
if cmd[:3] == ["docker", "image", "inspect"]:
|
|
return subprocess.CompletedProcess(
|
|
args=cmd,
|
|
returncode=1,
|
|
stdout="",
|
|
stderr="missing",
|
|
)
|
|
|
|
# Then: docker build → rc=0 (success)
|
|
if cmd[:2] == ["docker", "build"]:
|
|
return subprocess.CompletedProcess(
|
|
args=cmd,
|
|
returncode=0,
|
|
stdout="",
|
|
stderr="",
|
|
)
|
|
|
|
# Any other commands (should not happen here)
|
|
return subprocess.CompletedProcess(
|
|
args=cmd,
|
|
returncode=0,
|
|
stdout="",
|
|
stderr="",
|
|
)
|
|
|
|
mock_run.side_effect = _side_effect
|
|
|
|
deploy_container.ensure_image("infinito:latest")
|
|
|
|
self.assertTrue(
|
|
any(c[:3] == ["docker", "image", "inspect"] for c in calls),
|
|
"Expected 'docker image inspect' to be called",
|
|
)
|
|
self.assertTrue(
|
|
any(c[:2] == ["docker", "build"] for c in calls),
|
|
"Expected 'docker build' to run when image is missing",
|
|
)
|
|
|
|
|
|
class TestMain(unittest.TestCase):
|
|
@unittest.mock.patch("cli.deploy.container.command.run_in_container")
|
|
def test_main_requires_forwarded_args(self, mock_run_in_container):
|
|
"""
|
|
In 'run' mode, main() must return 1 and not call run_in_container()
|
|
if no deploy arguments are provided after the first '--'.
|
|
"""
|
|
argv = [
|
|
"cli.deploy.container.command",
|
|
"run",
|
|
"--image",
|
|
"myimage:latest",
|
|
"--build",
|
|
"--inventory-dir",
|
|
"/tmp/inventory",
|
|
"--",
|
|
# no inventory/deploy args here
|
|
]
|
|
with unittest.mock.patch.object(sys, "argv", argv):
|
|
rc = deploy_container.main()
|
|
|
|
self.assertEqual(rc, 1)
|
|
mock_run_in_container.assert_not_called()
|
|
|
|
@unittest.mock.patch("cli.deploy.container.command.run_in_container")
|
|
def test_main_passes_arguments_to_run_in_container(self, mock_run_in_container):
|
|
"""
|
|
Ensure that main() correctly splits container args vs inventory/deploy
|
|
args and passes them to run_in_container(). Also ensure inventory_dir
|
|
is forwarded.
|
|
"""
|
|
argv = [
|
|
"cli.deploy.container",
|
|
"run",
|
|
"--image",
|
|
"myimage:latest",
|
|
"--build",
|
|
"--inventory-dir",
|
|
"/tmp/inventory",
|
|
"--",
|
|
"--exclude",
|
|
"foo,bar",
|
|
"--",
|
|
"--debug",
|
|
]
|
|
|
|
with unittest.mock.patch.object(sys, "argv", argv):
|
|
rc = deploy_container.main()
|
|
|
|
self.assertEqual(rc, 0)
|
|
mock_run_in_container.assert_called_once()
|
|
|
|
kwargs = mock_run_in_container.call_args.kwargs
|
|
|
|
# Container-level options
|
|
self.assertEqual(kwargs["image"], "myimage:latest")
|
|
self.assertTrue(kwargs["build"])
|
|
self.assertFalse(kwargs["rebuild"])
|
|
self.assertFalse(kwargs["no_cache"])
|
|
self.assertIsNone(kwargs["name"])
|
|
|
|
# Inventory dir (new)
|
|
self.assertEqual(kwargs["inventory_dir"], "/tmp/inventory")
|
|
|
|
# Inventory args: first segment after first '--' until second '--'
|
|
self.assertEqual(
|
|
kwargs["inventory_args"],
|
|
["--exclude", "foo,bar"],
|
|
)
|
|
|
|
# Deploy args: everything after the second '--'
|
|
self.assertEqual(
|
|
kwargs["deploy_args"],
|
|
["--debug"],
|
|
)
|
|
|
|
@unittest.mock.patch("cli.deploy.container.command.run_in_container")
|
|
def test_main_requires_inventory_dir_when_env_missing(self, mock_run_in_container):
|
|
"""
|
|
main() must fail if neither --inventory-dir is provided nor INVENTORY_DIR env is set.
|
|
"""
|
|
argv = [
|
|
"cli.deploy.container",
|
|
"run",
|
|
"--image",
|
|
"myimage:latest",
|
|
"--build",
|
|
"--",
|
|
"--exclude",
|
|
"foo,bar",
|
|
]
|
|
|
|
with (
|
|
unittest.mock.patch.dict(os.environ, {}, clear=True),
|
|
unittest.mock.patch.object(sys, "argv", argv),
|
|
self.assertRaises(SystemExit),
|
|
):
|
|
deploy_container.main()
|
|
|
|
mock_run_in_container.assert_not_called()
|
|
|
|
@unittest.mock.patch("cli.deploy.container.command.run_in_container")
|
|
def test_main_uses_inventory_dir_from_env_when_present(self, mock_run_in_container):
|
|
"""
|
|
If INVENTORY_DIR env is set, --inventory-dir should not be required and the env value
|
|
must be forwarded to run_in_container().
|
|
"""
|
|
argv = [
|
|
"cli.deploy.container",
|
|
"run",
|
|
"--image",
|
|
"myimage:latest",
|
|
"--build",
|
|
"--",
|
|
"--exclude",
|
|
"foo,bar",
|
|
]
|
|
|
|
with (
|
|
unittest.mock.patch.dict(
|
|
os.environ, {"INVENTORY_DIR": "/env/inventory"}, clear=True
|
|
),
|
|
unittest.mock.patch.object(sys, "argv", argv),
|
|
):
|
|
rc = deploy_container.main()
|
|
|
|
self.assertEqual(rc, 0)
|
|
mock_run_in_container.assert_called_once()
|
|
|
|
kwargs = mock_run_in_container.call_args.kwargs
|
|
self.assertEqual(kwargs["inventory_dir"], "/env/inventory")
|
|
|
|
|
|
if __name__ == "__main__":
|
|
unittest.main()
|