mirror of
https://github.com/kevinveenbirkenbach/docker-volume-backup.git
synced 2026-08-20 21:22:54 +00:00
build(lint): gate make test on a clean ruff run
ruff was never wired into this repository: no target, no CI step, no pin. It reported 45 findings across sources and tests, so nothing enforced what the codebase already mostly followed. Adds `make ruff` (check + format --check), `make ruff-fix`, and `make lint` as its alias, and makes `make test` run lint as a fourth parallel spur. The CI workflow calls `make test`, so it is covered there too. The linter is pinned in a `lint` extra: a ruff minor bump changes which rules fire, and with the suite gating on a clean run an unpinned linter would fail it on an unrelated day. The 45 findings are fixed rather than configured away. Three needed a decision instead of the mechanical fix: - The generation timestamp keeps its local wall clock (DTZ005 waived). Generations sort by that name, and UTC would order new ones before the existing ones wherever the offset is positive - "newest generation" is what every restore path selects on. - The per-volume `copy` closure now binds volume_name and vol_dir as default arguments (B023). It only worked because it is called inside the same iteration. - The two CLI top-level handlers keep their blind except (BLE001 waived): turning any failure into exit 1 is what a CLI boundary is for. The two in run.py did not need it and were narrowed to what they actually catch. Also drops the comments that restate the code: the section banners in restore/__main__.py, the filename repeated as line 1 of nine test files, step narration above the statement it narrates, and a block in app.py documenting parameters that had moved to another module. What names a trip-wire stays - the snapshot destination rule, the mysql-binary absence in MariaDB 11 images, the session-scoped FOREIGN_KEY_CHECKS, the spooled temp file for multi-GB dumps, and the negative control that loses its discriminating power if it ever passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -9,7 +9,6 @@ import pandas as pd
|
||||
# Adjust if your package name/import path differs.
|
||||
from baudolo.backup.dumps import load_databases_df
|
||||
|
||||
|
||||
EXPECTED_COLUMNS = ["instance", "database", "username", "password"]
|
||||
|
||||
|
||||
@@ -33,7 +32,6 @@ class TestLoadDatabasesDf(unittest.TestCase):
|
||||
def test_empty_csv_is_handled_with_warning_and_empty_df(self) -> None:
|
||||
with tempfile.TemporaryDirectory() as td:
|
||||
empty_path = os.path.join(td, "databases.csv")
|
||||
# Create an empty file (0 bytes)
|
||||
with open(empty_path, "w", encoding="utf-8") as f:
|
||||
f.write("")
|
||||
|
||||
|
||||
@@ -69,14 +69,18 @@ class TestSnapshotFlags(unittest.TestCase):
|
||||
|
||||
class TestRequiredFlags(unittest.TestCase):
|
||||
def test_backups_dir_is_required(self) -> None:
|
||||
with mock.patch("sys.argv", ["baudolo", "--compose-dir", "/compose"]):
|
||||
with self.assertRaises(SystemExit):
|
||||
parse_args()
|
||||
with (
|
||||
mock.patch("sys.argv", ["baudolo", "--compose-dir", "/compose"]),
|
||||
self.assertRaises(SystemExit),
|
||||
):
|
||||
parse_args()
|
||||
|
||||
def test_compose_dir_is_required(self) -> None:
|
||||
with mock.patch("sys.argv", ["baudolo", "--backups-dir", "/backups"]):
|
||||
with self.assertRaises(SystemExit):
|
||||
parse_args()
|
||||
with (
|
||||
mock.patch("sys.argv", ["baudolo", "--backups-dir", "/backups"]),
|
||||
self.assertRaises(SystemExit),
|
||||
):
|
||||
parse_args()
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
|
||||
@@ -3,7 +3,6 @@ from __future__ import annotations
|
||||
import tempfile
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
from typing import List
|
||||
from unittest.mock import patch
|
||||
|
||||
from .compose_fixture import setup_compose_dir as _setup_compose_dir
|
||||
@@ -99,7 +98,7 @@ class TestCompose(unittest.TestCase):
|
||||
str(d), ["up", "-d", "--force-recreate"]
|
||||
)
|
||||
|
||||
expected: List[str] = [
|
||||
expected: list[str] = [
|
||||
"/usr/bin/docker",
|
||||
"compose",
|
||||
"--chdir",
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import unittest
|
||||
from typing import List
|
||||
from unittest.mock import patch
|
||||
|
||||
|
||||
@@ -11,7 +10,7 @@ class HardRestartArgTests(unittest.TestCase):
|
||||
the dir is a stack whose overlay network collides with compose up, pass
|
||||
nothing."""
|
||||
|
||||
def _parse(self, extra: List[str]):
|
||||
def _parse(self, extra: list[str]):
|
||||
import sys
|
||||
|
||||
from baudolo.backup import cli
|
||||
|
||||
@@ -20,15 +20,17 @@ def _capture_commands(*, db_type, rows, container):
|
||||
captured.append(cmd)
|
||||
return []
|
||||
|
||||
with tempfile.TemporaryDirectory() as td:
|
||||
with patch.object(db_mod, "execute_shell_command", side_effect=_capture):
|
||||
db_mod.backup_database(
|
||||
container=container,
|
||||
volume_dir=td,
|
||||
db_type=db_type,
|
||||
databases_df=_df(rows),
|
||||
database_containers=[container],
|
||||
)
|
||||
with (
|
||||
tempfile.TemporaryDirectory() as td,
|
||||
patch.object(db_mod, "execute_shell_command", side_effect=_capture),
|
||||
):
|
||||
db_mod.backup_database(
|
||||
container=container,
|
||||
volume_dir=td,
|
||||
db_type=db_type,
|
||||
databases_df=_df(rows),
|
||||
database_containers=[container],
|
||||
)
|
||||
return captured
|
||||
|
||||
|
||||
|
||||
@@ -61,9 +61,11 @@ class TestBtrfs(unittest.TestCase):
|
||||
|
||||
def test_it_removes_the_snapshot_even_when_the_body_raises(self) -> None:
|
||||
run = Runner()
|
||||
with self.assertRaises(ZeroDivisionError):
|
||||
with volume_snapshot("btrfs", "/var/lib/docker", "20260731", run=run):
|
||||
raise ZeroDivisionError
|
||||
with (
|
||||
self.assertRaises(ZeroDivisionError),
|
||||
volume_snapshot("btrfs", "/var/lib/docker", "20260731", run=run),
|
||||
):
|
||||
raise ZeroDivisionError
|
||||
self.assertTrue(run.calls[-1].startswith("btrfs subvolume delete"))
|
||||
|
||||
|
||||
@@ -93,26 +95,30 @@ class TestZfs(unittest.TestCase):
|
||||
|
||||
def test_an_unmounted_dataset_is_an_error(self) -> None:
|
||||
run = Runner({"zfs list": [""]})
|
||||
with self.assertRaises(SnapshotError):
|
||||
with volume_snapshot("zfs", "/var/lib/docker", "20260731", run=run):
|
||||
pass
|
||||
with (
|
||||
self.assertRaises(SnapshotError),
|
||||
volume_snapshot("zfs", "/var/lib/docker", "20260731", run=run),
|
||||
):
|
||||
pass
|
||||
|
||||
|
||||
class TestRejections(unittest.TestCase):
|
||||
def test_an_unknown_kind_is_rejected(self) -> None:
|
||||
run = Runner()
|
||||
with self.assertRaises(SnapshotError):
|
||||
with volume_snapshot("ext4", "/var/lib/docker", "20260731", run=run):
|
||||
pass
|
||||
with (
|
||||
self.assertRaises(SnapshotError),
|
||||
volume_snapshot("ext4", "/var/lib/docker", "20260731", run=run),
|
||||
):
|
||||
pass
|
||||
self.assertEqual(run.calls, [])
|
||||
|
||||
def test_a_path_outside_the_subject_is_rejected(self) -> None:
|
||||
run = Runner()
|
||||
with volume_snapshot(
|
||||
"btrfs", "/var/lib/docker", "20260731", run=run
|
||||
) as resolve:
|
||||
with self.assertRaises(SnapshotError):
|
||||
resolve("/etc/passwd")
|
||||
with (
|
||||
volume_snapshot("btrfs", "/var/lib/docker", "20260731", run=run) as resolve,
|
||||
self.assertRaises(SnapshotError),
|
||||
):
|
||||
resolve("/etc/passwd")
|
||||
|
||||
def test_the_subject_itself_resolves_to_the_snapshot_root(self) -> None:
|
||||
run = Runner()
|
||||
@@ -137,9 +143,11 @@ class TestRemovalFailure(unittest.TestCase):
|
||||
pass
|
||||
|
||||
def test_a_failed_removal_does_not_mask_the_body(self) -> None:
|
||||
with self.assertRaises(ZeroDivisionError):
|
||||
with volume_snapshot("btrfs", "/var/lib/docker", "20260731", run=Busy()):
|
||||
raise ZeroDivisionError
|
||||
with (
|
||||
self.assertRaises(ZeroDivisionError),
|
||||
volume_snapshot("btrfs", "/var/lib/docker", "20260731", run=Busy()),
|
||||
):
|
||||
raise ZeroDivisionError
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
|
||||
@@ -65,10 +65,7 @@ class TestClusterReplay(unittest.TestCase):
|
||||
self.assertIn("rolname <> current_user", preclean)
|
||||
|
||||
def test_only_the_connecting_role_loses_its_create(self) -> None:
|
||||
# Captured from pg_dumpall 17: the bootstrap superuser is recreated like
|
||||
# any other role, and the pre-clean cannot drop the one holding the
|
||||
# session - so that single CREATE always collides while its ALTER, which
|
||||
# carries the attributes and the password, must survive.
|
||||
# Captured from pg_dumpall 17.
|
||||
dump = [
|
||||
b"CREATE ROLE app;\n",
|
||||
b"ALTER ROLE app WITH NOSUPERUSER INHERIT LOGIN PASSWORD 'SCRAM-SHA-256$...';\n",
|
||||
|
||||
@@ -131,7 +131,6 @@ class TestSeedMain(unittest.TestCase):
|
||||
warning_calls,
|
||||
"Expected a WARNING print when databases.csv is empty, but none was found.",
|
||||
)
|
||||
# Ensure the warning goes to stderr
|
||||
_, warn_kwargs = warning_calls[0]
|
||||
self.assertEqual(warn_kwargs.get("file"), seed_main.sys.stderr)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user