From a6c091b8549d1be3cd39543fc93a257bd4c84c02 Mon Sep 17 00:00:00 2001 From: Tobias Kaupat Date: Tue, 20 Oct 2026 09:17:00 +0200 Subject: [PATCH] test: isolate scenario steps, share helpers, use restore_latest, HEAD dir probe Scenario fixtures record each step; a failing step fails only the tests that assert on it. running() and raise_interrupt() live in helpers.py. put_file probes parent directories with a HEAD request instead of a get_archive stream. stream_run uses restore_latest and asserts its result. Docstrings no longer refer to the removed Bash suite. Co-Authored-By: Claude Sonnet 5.5 --- tests/conftest.py | 8 +-- tests/helpers.py | 63 ++++++++++++++++++-- tests/test_compat.py | 129 ++++++++++++++++++++--------------------- tests/test_features.py | 82 +++++++++++++------------- tests/test_scaffold.py | 12 ---- 5 files changed, 167 insertions(+), 127 deletions(-) delete mode 100644 tests/test_scaffold.py diff --git a/tests/conftest.py b/tests/conftest.py index 629a295..9295a09 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -12,7 +12,7 @@ from pathlib import Path import docker import pytest -from helpers import Container, remove_quietly, run_once, start_container +from helpers import Container, raise_interrupt, remove_quietly, run_once, start_container ROOT = Path(__file__).resolve().parent.parent IMAGE = os.environ.get("IMAGE", "restic-backup-docker:test") @@ -39,13 +39,9 @@ def required_features() -> set[str]: return {line for line in lines if line and not line.startswith("#")} -def _raise_interrupt(signum: int, _frame: object) -> None: - raise KeyboardInterrupt(f"received {signal.Signals(signum).name}") - - def pytest_configure(config: pytest.Config) -> None: """Turn SIGTERM into KeyboardInterrupt so session teardown (cleanup) runs.""" - signal.signal(signal.SIGTERM, _raise_interrupt) + signal.signal(signal.SIGTERM, raise_interrupt) def pytest_addoption(parser: pytest.Parser) -> None: diff --git a/tests/helpers.py b/tests/helpers.py index 8964272..a2c1095 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -10,13 +10,16 @@ from __future__ import annotations import contextlib import io import json +import signal import tarfile import time +from collections.abc import Callable from dataclasses import dataclass from pathlib import PurePosixPath from typing import Any import docker +import pytest from docker.models.containers import Container as DockerContainer START_TIMEOUT = 60 @@ -33,6 +36,49 @@ class CommandError(AssertionError): pass +def raise_interrupt(signum: int, _frame: object) -> None: + """Signal handler: turn SIGTERM into KeyboardInterrupt so cleanup code runs.""" + raise KeyboardInterrupt(f"received {signal.Signals(signum).name}") + + +class StepFailed(AssertionError): + pass + + +class Scenario: + """Results of the actions of one scenario fixture. + + `record(name, fn)` runs one step and stores its result or its error. Reading an + attribute of a failed step raises, so only the tests that assert on that step fail; + the other tests of the class still run against what the scenario did produce. + """ + + def __init__(self) -> None: + self._steps: dict[str, tuple[Any, BaseException | None]] = {} + + def record(self, name: str, step: Callable[[], Any]) -> None: + try: + self._steps[name] = (step(), None) + except Exception as exc: # noqa: BLE001 - the error is re-raised on access + self._steps[name] = (None, exc) + + def __getattr__(self, name: str) -> Any: + try: + value, error = self.__dict__["_steps"][name] + except KeyError: + raise AttributeError(name) from None + if error is not None: + raise StepFailed(f"scenario step '{name}' failed: {error}") from error + return value + + +def running(container: Container) -> Container: + """Scenarios that act on the container need it started; the start tests do not.""" + if not container.started: + pytest.fail(f"container did not start:\n{container.start_log}", pytrace=False) + return container + + class Container: """A started image container with exec/file helpers.""" @@ -95,11 +141,20 @@ class Container: def _dir_exists(raw: DockerContainer, path: str) -> bool: + """Ask the daemon with a HEAD request on the archive endpoint: stat only, no tar stream. + + A container that is not started cannot exec `test -d`, and `get_archive` would leave + its streaming response open. + """ + api = raw.client.api + response = api.head(api._url("/containers/{0}/archive", raw.id), params={"path": path}) try: - raw.get_archive(path, chunk_size=1) - except docker.errors.NotFound: - return False - return True + if response.status_code == 404: + return False + response.raise_for_status() + return True + finally: + response.close() def put_file(raw: DockerContainer, path: str, content: str, *, executable: bool = False) -> None: diff --git a/tests/test_compat.py b/tests/test_compat.py index 0910bd5..5ea1749 100644 --- a/tests/test_compat.py +++ b/tests/test_compat.py @@ -5,7 +5,8 @@ A change that turns one of these red is a breaking change, whether or not intend Layout: every scenario fixture starts one throwaway container, performs the actions once and records the results; the tests only assert on what was recorded, so they do -not depend on each other. Each docstring is the name of the matching Bash test. +not depend on each other. A scenario step that fails fails only the tests that assert +on its result. Each docstring states the behaviour the test pins. Tests run in a container with the image defaults: repository /mnt/restic, tag `testtag`, cron far in the future. """ @@ -14,12 +15,11 @@ from __future__ import annotations from collections.abc import Iterator from dataclasses import dataclass -from types import SimpleNamespace import docker import pytest from conftest import LABELS -from helpers import Container, run_once +from helpers import Container, Scenario, run_once, running HELLO = "backup me" PRE_BACKUP_HOOK = '#!/bin/sh\necho "PRE-HOOK-RAN"\n' @@ -36,13 +36,6 @@ def started(request: pytest.FixtureRequest, run_container) -> Iterator[Container yield container -def _running(container: Container) -> Container: - """Scenarios that act on the container need it started; the start tests do not.""" - if not container.started: - pytest.fail(f"container did not start:\n{container.start_log}", pytrace=False) - return container - - def _crontab_lines(container: Container) -> list[str]: return container.read_file("/var/spool/cron/crontabs/root").splitlines() @@ -53,21 +46,21 @@ def _crontab_lines(container: Container) -> list[str]: @pytest.fixture(scope="class") -def default_run(started: Container) -> SimpleNamespace: - c = _running(started) +def default_run(started: Container) -> Scenario: + c = running(started) c.write_file("/data/hello.txt", HELLO) - run = SimpleNamespace(container=c) - run.backup = c.exec("/bin/backup") - run.snapshots_after_first = c.snapshots() - run.restore = c.restore_latest("/tmp/restore") - run.restored = c.exec("cat", "/tmp/restore/data/hello.txt") - run.backup_last_log = c.read_file("/var/log/backup-last.log") - c.run("/bin/backup") - run.snapshots_after_second = c.snapshots() - run.check = c.exec("/bin/check") + run = Scenario() + run.record("backup", lambda: c.exec("/bin/backup")) + run.record("snapshots_after_first", c.snapshots) + run.record("restore", lambda: c.restore_latest("/tmp/restore")) + run.record("restored", lambda: c.exec("cat", "/tmp/restore/data/hello.txt")) + run.record("backup_last_log", lambda: c.read_file("/var/log/backup-last.log")) + run.record("second_backup", lambda: c.run("/bin/backup")) + run.record("snapshots_after_second", c.snapshots) + run.record("check", lambda: c.exec("/bin/check")) c.write_file("/hooks/pre-backup.sh", PRE_BACKUP_HOOK, executable=True) c.write_file("/hooks/post-backup.sh", POST_BACKUP_HOOK, executable=True) - run.hook_backup = c.exec("/bin/backup") + run.record("hook_backup", lambda: c.exec("/bin/backup")) return run @@ -173,14 +166,15 @@ class TestDefaultBackup: @pytest.fixture(scope="class") -def checkcron_run(started: Container) -> SimpleNamespace: - c = _running(started) - run = SimpleNamespace(container=c, crontab=_crontab_lines(c)) +def checkcron_run(started: Container) -> Scenario: + c = running(started) + run = Scenario() + run.record("crontab", lambda: _crontab_lines(c)) c.write_file("/data/hello.txt", HELLO) - c.run("/bin/backup") + run.record("backup", lambda: c.run("/bin/backup")) c.write_file("/hooks/pre-check.sh", PRE_CHECK_HOOK, executable=True) c.write_file("/hooks/post-check.sh", POST_CHECK_HOOK, executable=True) - run.check = c.exec("/bin/check") + run.record("check", lambda: c.exec("/bin/check")) return run @@ -227,14 +221,15 @@ CORRUPT_CMD = 'f=$(find /mnt/restic/data -type f | head -n1); chmod u+w "$f"; pr @pytest.fixture(scope="class") -def corrupt_run(started: Container) -> SimpleNamespace: - c = _running(started) +def corrupt_run(started: Container) -> Scenario: + c = running(started) c.write_file("/data/hello.txt", HELLO) - c.run("/bin/backup") - run = SimpleNamespace(container=c, snapshots_before=c.snapshots()) - c.run("sh", "-c", CORRUPT_CMD) - run.check = c.exec("/bin/check") - run.error_log_exists = c.file_exists("/var/log/check-error-last.log") + run = Scenario() + run.record("backup", lambda: c.run("/bin/backup")) + run.record("snapshots_before", c.snapshots) + run.record("corrupt", lambda: c.run("sh", "-c", CORRUPT_CMD)) + run.record("check", lambda: c.exec("/bin/check")) + run.record("error_log_exists", lambda: c.file_exists("/var/log/check-error-last.log")) return run @@ -264,14 +259,14 @@ class TestCorruptedRepository: @pytest.fixture(scope="class") -def forget_run(started: Container) -> SimpleNamespace: - c = _running(started) +def forget_run(started: Container) -> Scenario: + c = running(started) c.write_file("/data/hello.txt", HELLO) - c.run("/bin/backup") - first = c.snapshots() - run = SimpleNamespace(container=c, first=first) - run.second_backup = c.exec("/bin/backup") - run.snapshots = c.snapshots() + run = Scenario() + run.record("first_backup", lambda: c.run("/bin/backup")) + run.record("first", c.snapshots) + run.record("second_backup", lambda: c.exec("/bin/backup")) + run.record("snapshots", c.snapshots) return run @@ -299,14 +294,15 @@ class TestForgetArgs: @pytest.fixture(scope="class") -def jobargs_run(started: Container) -> SimpleNamespace: - c = _running(started) +def jobargs_run(started: Container) -> Scenario: + c = running(started) c.write_file("/data/keep.txt", "keep") c.write_file("/data/skipme.txt", "skip") - run = SimpleNamespace(container=c, backup=c.exec("/bin/backup")) - run.restore = c.restore_latest("/tmp/restore") - run.restored_keep = c.exec("cat", "/tmp/restore/data/keep.txt") - run.paths = c.backed_up_paths() + run = Scenario() + run.record("backup", lambda: c.exec("/bin/backup")) + run.record("restore", lambda: c.restore_latest("/tmp/restore")) + run.record("restored_keep", lambda: c.exec("cat", "/tmp/restore/data/keep.txt")) + run.record("paths", c.backed_up_paths) return run @@ -334,14 +330,15 @@ class TestJobArgs: @pytest.fixture(scope="class") -def sources_run(started: Container) -> SimpleNamespace: - c = _running(started) +def sources_run(started: Container) -> Scenario: + c = running(started) c.write_file("/opt/custom/custom.txt", "custom source") c.write_file("/data/ignored.txt", "should not be backed up") - c.run("/bin/backup") - run = SimpleNamespace(container=c, restore=c.restore_latest("/tmp/restore")) - run.restored = c.exec("cat", "/tmp/restore/opt/custom/custom.txt") - run.paths = c.backed_up_paths() + run = Scenario() + run.record("backup", lambda: c.run("/bin/backup")) + run.record("restore", lambda: c.restore_latest("/tmp/restore")) + run.record("restored", lambda: c.exec("cat", "/tmp/restore/opt/custom/custom.txt")) + run.record("paths", c.backed_up_paths) return run @@ -364,16 +361,15 @@ class TestBackupSources: @pytest.fixture(scope="class") -def failure_run(started: Container) -> SimpleNamespace: - c = _running(started) +def failure_run(started: Container) -> Scenario: + c = running(started) c.write_file("/data/hello.txt", HELLO) # Back up a source that does not exist. The repository stays the initialised # default one, so the snapshot list below is of the repo the run really used. - run = SimpleNamespace( - container=c, backup=c.exec("/bin/backup", env={"BACKUP_SOURCES": "/does-not-exist"}) - ) - run.error_log_exists = c.file_exists("/var/log/backup-error-last.log") - run.snapshots = c.snapshots() + run = Scenario() + run.record("backup", lambda: c.exec("/bin/backup", env={"BACKUP_SOURCES": "/does-not-exist"})) + run.record("error_log_exists", lambda: c.file_exists("/var/log/backup-error-last.log")) + run.record("snapshots", c.snapshots) return run @@ -440,13 +436,14 @@ class TestTimezone: @pytest.fixture(scope="class") -def tag_run(started: Container) -> SimpleNamespace: - c = _running(started) +def tag_run(started: Container) -> Scenario: + c = running(started) c.write_file("/data/hello.txt", HELLO) - run = SimpleNamespace(container=c, empty=c.exec("/bin/backup", env={"RESTIC_TAG": ""})) - run.snapshots_after_empty = c.snapshots() - run.unset = c.exec("env", "-u", "RESTIC_TAG", "/bin/backup") - run.snapshots_after_unset = c.snapshots() + run = Scenario() + run.record("empty", lambda: c.exec("/bin/backup", env={"RESTIC_TAG": ""})) + run.record("snapshots_after_empty", c.snapshots) + run.record("unset", lambda: c.exec("env", "-u", "RESTIC_TAG", "/bin/backup")) + run.record("snapshots_after_unset", c.snapshots) return run diff --git a/tests/test_features.py b/tests/test_features.py index a29096c..79658ae 100644 --- a/tests/test_features.py +++ b/tests/test_features.py @@ -2,8 +2,8 @@ Each class is guarded by `@pytest.mark.feature()`: it runs as soon as the built image ships the feature (detected in conftest.py) and is skipped before that. A gate -listed in tests/required-features.txt turns the skip into a failure. Each docstring is -the name of the matching Bash test. +listed in tests/required-features.txt turns the skip into a failure. A scenario step +that fails fails only the tests that assert on its result. Scenario fixtures request `feature_gate_class` first, so a gated class never starts a container on an image without the feature. @@ -12,11 +12,10 @@ container on an image without the feature. from __future__ import annotations from collections.abc import Iterator -from types import SimpleNamespace import pytest from conftest import LABELS -from helpers import Container, run_once +from helpers import Container, Scenario, run_once, running STREAM_ENV = { "RESTIC_STREAM": "1", @@ -30,60 +29,60 @@ if [[ "${arr[1]}" == "two" ]]; then echo "BASH-HOOK-RAN"; fi MSMTP_CONFIG = "defaults\naccount default\nhost localhost\n" -def _running(container: Container) -> Container: - if not container.started: - pytest.fail(f"container did not start:\n{container.start_log}", pytrace=False) - return container - - # --------------------------------------------------------------------------- # stream backup (PR #109) # --------------------------------------------------------------------------- @pytest.fixture(scope="class") -def stream_run(feature_gate_class, run_container) -> Iterator[SimpleNamespace]: +def stream_run(feature_gate_class, run_container) -> Iterator[Scenario]: with run_container("stream", env=STREAM_ENV) as c: - _running(c) - run = SimpleNamespace(container=c) - run.backup = c.exec("/bin/backup") - run.snapshots = c.snapshots() - c.exec("restic", "restore", "latest", "--target", "/tmp/restore") - run.restored = c.exec("cat", "/tmp/restore/stream.txt") + running(c) + run = Scenario() + run.record("backup", lambda: c.exec("/bin/backup")) + run.record("snapshots", c.snapshots) + run.record("restore", lambda: c.restore_latest("/tmp/restore")) + run.record("restored", lambda: c.exec("cat", "/tmp/restore/stream.txt")) yield run @pytest.fixture(scope="class") -def stream_jobargs_run(feature_gate_class, run_container) -> Iterator[SimpleNamespace]: +def stream_jobargs_run(feature_gate_class, run_container) -> Iterator[Scenario]: # --host is used because it is visible in the snapshot metadata of a stdin backup. with run_container( "stream-jobargs", env={**STREAM_ENV, "RESTIC_JOB_ARGS": "--host=jobargshost"} ) as c: - _running(c) - c.exec("/bin/backup") + running(c) + run = Scenario() + run.record("backup", lambda: c.run("/bin/backup")) # restic's own --host filter only returns snapshots whose hostname equals the # value; both directions are checked so the filter is known to discriminate. - yield SimpleNamespace( - matching=c.restic_json("snapshots", "--host", "jobargshost") or [], - unrelated=c.restic_json("snapshots", "--host", "some-other-host") or [], + run.record("matching", lambda: c.restic_json("snapshots", "--host", "jobargshost") or []) + run.record( + "unrelated", lambda: c.restic_json("snapshots", "--host", "some-other-host") or [] ) + yield run @pytest.fixture(scope="class") -def stream_misconfigured_run(feature_gate_class, run_container) -> Iterator[SimpleNamespace]: +def stream_misconfigured_run(feature_gate_class, run_container) -> Iterator[Scenario]: with run_container("stream-misconfigured", env={"RESTIC_STREAM": "1"}) as c: - _running(c) - backup = c.exec("/bin/backup") - yield SimpleNamespace(backup=backup, snapshots=c.snapshots()) + running(c) + run = Scenario() + run.record("backup", lambda: c.exec("/bin/backup")) + run.record("snapshots", c.snapshots) + yield run @pytest.fixture(scope="class") -def stream_disabled_run(feature_gate_class, run_container) -> Iterator[SimpleNamespace]: +def stream_disabled_run(feature_gate_class, run_container) -> Iterator[Scenario]: with run_container("stream-disabled") as c: - _running(c) + running(c) c.write_file("/data/hello.txt", "backup me") - c.exec("/bin/backup") - yield SimpleNamespace(snapshots=c.snapshots()) + run = Scenario() + run.record("backup", lambda: c.run("/bin/backup")) + run.record("snapshots", c.snapshots) + yield run @pytest.mark.feature("stream") @@ -98,6 +97,7 @@ class TestStream: def test_stream_content_stored_under_filename(self, stream_run): """stream content is stored under RESTIC_STREAM_FILENAME""" + assert stream_run.restore.rc == 0, stream_run.restore.out assert stream_run.restored.rc == 0, stream_run.restored.out assert stream_run.restored.out == "hello-stream\n" @@ -142,12 +142,14 @@ def test_docker_cli_usable(docker_client, image): @pytest.fixture(scope="class") -def bash_hook_run(feature_gate_class, run_container) -> Iterator[SimpleNamespace]: +def bash_hook_run(feature_gate_class, run_container) -> Iterator[Scenario]: with run_container("bash-hooks") as c: - _running(c) + running(c) c.write_file("/hooks/pre-backup.sh", BASH_HOOK, executable=True) c.write_file("/data/hello.txt", "backup me") - yield SimpleNamespace(backup=c.exec("/bin/backup")) + run = Scenario() + run.record("backup", lambda: c.exec("/bin/backup")) + yield run @pytest.mark.feature("bash") @@ -166,18 +168,20 @@ class TestBashHooks: def msmtp_config_run(feature_gate_class, run_container) -> Iterator[Container]: # entry.sh copies the config at startup, so it must exist before the container starts. with run_container("msmtp-config", files={"/config/msmtprc": MSMTP_CONFIG}) as c: - yield _running(c) + yield running(c) @pytest.fixture(scope="class") -def msmtp_missing_config_run(feature_gate_class, run_container) -> Iterator[SimpleNamespace]: +def msmtp_missing_config_run(feature_gate_class, run_container) -> Iterator[Scenario]: # Users who configured mail before msmtp existed must not lose their backups. env = {"MAILX_ARGS": "-r from@example.com -s subject to@example.com"} with run_container("msmtp-missing-config", env=env) as c: - _running(c) + running(c) c.write_file("/data/hello.txt", "backup me") - backup = c.exec("/bin/backup") - yield SimpleNamespace(backup=backup, snapshots=c.snapshots()) + run = Scenario() + run.record("backup", lambda: c.exec("/bin/backup")) + run.record("snapshots", c.snapshots) + yield run @pytest.mark.feature("msmtp") diff --git a/tests/test_scaffold.py b/tests/test_scaffold.py deleted file mode 100644 index f6e4a48..0000000 --- a/tests/test_scaffold.py +++ /dev/null @@ -1,12 +0,0 @@ -"""Scaffold checks: prove the gating logic itself. Real tests follow in later commits.""" - -import pytest - - -def test_image_is_available(image): - assert image - - -@pytest.mark.feature("bash") -def test_feature_gate_probe(detected_features): - assert "bash" in detected_features