test(packaging): expand removal semantics tests for #49

Replace the thin test_removal_semantics with comprehensive per-operation
tests covering all five acceptance criteria:

- deb remove keeps config, store (DB + WAL sidecars + backup), and group
- deb purge removes config, store, backup, and group
- rpm erase preserves modified config as .rpmsave
- rpm erase removes unmodified config
- store files never deleted except by purge
- dedicated test: sanctioned disable never runs on upgrade (parametrized
  across all deb + rpm targets)

Co-authored-by: CommandCodeBot <noreply@commandcode.ai>

tests/test_packaging.py

@@ -418,36 +418,115 @@ def _assert_rpm_upgrade_full(container: str, pkg_name: str) -> None:
     )
 
 
-def _assert_removal_semantics(container: str, fmt: str) -> None:
+def _setup_store_and_config(container: str) -> None:
-    """Assert removal mapping (spec §7)."""
+    """Plant observation store and config for removal semantics testing."""
-    if fmt == "deb":
+    _container_exec(
-        # remove (not purge) — config and store survive
+        container,
-        rc, out = _container_exec(
+        "mkdir -p /var/lib/fenris && "
-            container, "dpkg --purge fenris 2>&1 || true"
+        "python3 -c \""
-        )
+        "import sqlite3; "
-        # After purge: config gone, store gone (our postrm removes on purge)
+        "c = sqlite3.connect('/var/lib/fenris/observations.db'); "
-        # But the store dir may survive since it's not package-owned
+        "c.execute('PRAGMA user_version=1'); "
-    else:
+        "c.commit(); c.close()\" && "
-        # rpm erase
+        "touch /var/lib/fenris/observations.db-wal && "
-        rc, out = _container_exec(
+        "touch /var/lib/fenris/observations.db-shm && "
-            container, "rpm -e fenris 2>&1 || true"
+        "cp /var/lib/fenris/observations.db /var/lib/fenris/observations.db.bak",
-        )
+    )
+    _container_exec(
+        container,
+        "echo 'devices = /dev/nvme0n1' > /etc/fenris/fenris.conf",
+    )
 
-    # Units removed
+
+def _assert_package_files_removed(container: str) -> None:
+    """Assert package-owned files are removed after removal."""
     rc, _ = _container_exec(
         container, "test -f /usr/lib/systemd/system/fenris-collect.timer"
     )
-    assert rc != 0, "Timer unit should be removed after uninstall"
+    assert rc != 0, "Timer should be removed"
 
-    # Helpers removed
     rc, _ = _container_exec(
-        container, "test -f /usr/libexec/fenris/fenris-monitor"
+        container, "test -x /usr/libexec/fenris/fenris-monitor"
     )
-    assert rc != 0, "Helper should be removed after uninstall"
+    assert rc != 0, "Helper should be removed"
 
-    # Venv key files removed (directories may remain if non-empty)
     rc, _ = _container_exec(container, "test -f /opt/fenris/bin/python3")
-    assert rc != 0, "Venv Python should be removed after uninstall"
+    assert rc != 0, "Venv Python should be removed"
+
+
+def _assert_deb_remove_preserves(container: str) -> None:
+    """Assert deb remove keeps config, store, and group (spec §7)."""
+    # Config survives
+    rc, _ = _container_exec(container, "test -f /etc/fenris/fenris.conf")
+    assert rc == 0, "Config should survive deb remove"
+
+    # Store directory survives
+    rc, _ = _container_exec(container, "test -d /var/lib/fenris")
+    assert rc == 0, "Store directory should survive deb remove"
+
+    # Store DB, WAL sidecars, and backup survive
+    for name in ("observations.db", "observations.db-wal",
+                 "observations.db-shm", "observations.db.bak"):
+        rc, _ = _container_exec(
+            container, f"test -f /var/lib/fenris/{name}"
+        )
+        assert rc == 0, f"Store file {name} should survive deb remove"
+
+    # Service group survives
+    rc, _ = _container_exec(container, "getent group fenris")
+    assert rc == 0, "Group should survive deb remove"
+
+
+def _assert_deb_purge_removes(container: str) -> None:
+    """Assert deb purge removes config, store, backup, and group (spec §7)."""
+    rc, _ = _container_exec(container, "test -f /etc/fenris/fenris.conf")
+    assert rc != 0, "Config should be removed by purge"
+
+    rc, _ = _container_exec(container, "test -d /var/lib/fenris")
+    assert rc != 0, "Store directory should be removed by purge"
+
+    # Service group removed
+    rc, _ = _container_exec(container, "getent group fenris 2>/dev/null || true")
+    assert rc != 0, "Group should be removed by purge"
+
+
+def _assert_rpm_erase_removes_unmodified(container: str) -> None:
+    """Assert rpm erase removes unmodified config, keeps store (spec §7)."""
+    # Default config removed (unmodified by user)
+    rc, _ = _container_exec(
+        container, "test -f /etc/fenris/fenris.conf"
+    )
+    assert rc != 0, "Unmodified config should be removed by rpm erase"
+
+    # Store directory and files survive
+    rc, _ = _container_exec(container, "test -d /var/lib/fenris")
+    assert rc == 0, "Store directory should survive rpm erase"
+
+    for name in ("observations.db", "observations.db-wal",
+                 "observations.db-shm", "observations.db.bak"):
+        rc, _ = _container_exec(
+            container, f"test -f /var/lib/fenris/{name}"
+        )
+        assert rc == 0, f"Store file {name} should survive rpm erase"
+
+
+def _assert_rpm_erase_preserves_modified(container: str) -> None:
+    """Assert rpm erase preserves modified config as .rpmsave (spec §7)."""
+    rc, _ = _container_exec(
+        container, "test -f /etc/fenris/fenris.conf.rpmsave"
+    )
+    assert rc == 0, "Modified config should survive as .rpmsave"
+
+    # Store directory and files survive
+    rc, _ = _container_exec(container, "test -d /var/lib/fenris")
+    assert rc == 0, "Store directory should survive rpm erase"
+
+    for name in ("observations.db", "observations.db-wal",
+                 "observations.db-shm", "observations.db.bak"):
+        rc, _ = _container_exec(
+            container, f"test -f /var/lib/fenris/{name}"
+        )
+        assert rc == 0, f"Store file {name} should survive rpm erase"
 
 
 # ---------------------------------------------------------------------------
@@ -780,17 +859,108 @@ def test_upgrade_semantics(skip_no_docker, image, fmt, version):
 
 
 # ---------------------------------------------------------------------------
-# Tests — removal semantics
+# Tests — removal semantics (issue #49)
 # ---------------------------------------------------------------------------
 
 @pytest.mark.slow
-@pytest.mark.parametrize("image,fmt", ALL_TARGETS, ids=[t[0] for t in ALL_TARGETS])
+@pytest.mark.parametrize("image", [t[0] for t in DEB_TARGETS])
-def test_removal_semantics(skip_no_docker, image, fmt, version):
+def test_deb_remove_preserves_config_and_store(skip_no_docker, image, version):
-    """Assert removal cleans package-owned files."""
+    """deb remove keeps config, store, and group (spec §7, #49)."""
+    pkg = _find_package("deb")
+    pkg_name = pkg.name
+
+    build_dir = REPO_ROOT / "build" / "test-container-deb-remove"
+    build_dir.mkdir(parents=True, exist_ok=True)
+    (build_dir / "dist").mkdir(exist_ok=True)
+    subprocess.run(
+        ["cp", str(pkg), str(build_dir / "dist" / pkg_name)],
+        check=True,
+    )
+    (build_dir / "Dockerfile").write_text(
+        _build_deb_dockerfile(image, pkg_name)
+    )
+
+    tag = f"fenris-deb-remove-{image.replace(':', '-')}"
+    subprocess.run(
+        ["docker", "build", "-t", tag, str(build_dir)],
+        check=True, capture_output=True, timeout=300,
+    )
+
+    container = f"fenris-deb-remove-{os.getpid()}"
+    subprocess.run(
+        ["docker", "run", "-d", "--name", container,
+         "--tmpfs", "/tmp:exec,size=64m", tag, "sleep", "infinity"],
+        check=True, capture_output=True,
+    )
+
+    try:
+        _setup_store_and_config(container)
+        rc, out = _container_exec(container, "dpkg --remove fenris 2>&1")
+        assert rc == 0, f"dpkg --remove failed: {out}"
+        _assert_deb_remove_preserves(container)
+        _assert_package_files_removed(container)
+        # Sanctioned disable runs in prerm on remove — verified indirectly by
+        # the timer no longer being active.  "Never on upgrade" is covered by
+        # test_upgrade_semantics (criterion 4: removal scripts are no-ops).
+    finally:
+        subprocess.run(
+            ["docker", "rm", "-f", container], capture_output=True,
+        )
+
+
+@pytest.mark.slow
+@pytest.mark.parametrize("image", [t[0] for t in DEB_TARGETS])
+def test_deb_purge_removes_everything(skip_no_docker, image, version):
+    """deb purge removes config, store, backup, and group (spec §7, #49)."""
+    pkg = _find_package("deb")
+    pkg_name = pkg.name
+
+    build_dir = REPO_ROOT / "build" / "test-container-deb-purge"
+    build_dir.mkdir(parents=True, exist_ok=True)
+    (build_dir / "dist").mkdir(exist_ok=True)
+    subprocess.run(
+        ["cp", str(pkg), str(build_dir / "dist" / pkg_name)],
+        check=True,
+    )
+    (build_dir / "Dockerfile").write_text(
+        _build_deb_dockerfile(image, pkg_name)
+    )
+
+    tag = f"fenris-deb-purge-{image.replace(':', '-')}"
+    subprocess.run(
+        ["docker", "build", "-t", tag, str(build_dir)],
+        check=True, capture_output=True, timeout=300,
+    )
+
+    container = f"fenris-deb-purge-{os.getpid()}"
+    subprocess.run(
+        ["docker", "run", "-d", "--name", container,
+         "--tmpfs", "/tmp:exec,size=64m", tag, "sleep", "infinity"],
+        check=True, capture_output=True,
+    )
+
+    try:
+        _setup_store_and_config(container)
+        rc, out = _container_exec(container, "dpkg --purge fenris 2>&1")
+        assert rc == 0, f"dpkg --purge failed: {out}"
+        _assert_deb_purge_removes(container)
+        _assert_package_files_removed(container)
+    finally:
+        subprocess.run(
+            ["docker", "rm", "-f", container], capture_output=True,
+        )
+
+
+@pytest.mark.slow
+@pytest.mark.parametrize("image", [t[0] for t in ALL_TARGETS],
+                         ids=[t[0] for t in ALL_TARGETS])
+def test_sanctioned_disable_skipped_on_upgrade(skip_no_docker, image, fmt,
+                                               version):
+    """Sanctioned disable never runs during upgrade (spec §7, #49)."""
     pkg = _find_package(fmt)
     pkg_name = pkg.name
 
-    build_dir = REPO_ROOT / "build" / "test-container-removal"
+    build_dir = REPO_ROOT / "build" / "test-container-upgrade-no-disable"
     build_dir.mkdir(parents=True, exist_ok=True)
     (build_dir / "dist").mkdir(exist_ok=True)
     subprocess.run(
@@ -804,29 +974,216 @@ def test_removal_semantics(skip_no_docker, image, fmt, version):
         dockerfile = _build_rpm_dockerfile(image, pkg_name)
     (build_dir / "Dockerfile").write_text(dockerfile)
 
-    tag = f"fenris-removal-{image.replace(':', '-').replace('/', '-')}"
+    tag = f"fenris-upgrade-no-disable-{image.replace(':', '-').replace('/', '-')}"
     subprocess.run(
         ["docker", "build", "-t", tag, str(build_dir)],
-        check=True,
+        check=True, capture_output=True, timeout=300,
-        capture_output=True,
-        timeout=300,
     )
 
-    container = f"fenris-removal-{os.getpid()}"
+    container = f"fenris-upgrade-no-disable-{os.getpid()}"
     subprocess.run(
-        [
+        ["docker", "run", "-d", "--name", container,
-            "docker", "run", "-d", "--name", container,
+         "--tmpfs", "/tmp:exec,size=64m", tag, "sleep", "infinity"],
-            "--tmpfs", "/tmp:exec,size=64m",
+        check=True, capture_output=True,
-            tag, "sleep", "infinity",
-        ],
-        check=True,
-        capture_output=True,
     )
 
     try:
-        _assert_removal_semantics(container, fmt)
+        _setup_store_and_config(container)
+
+        if fmt == "deb":
+            # Fake older version so dpkg treats reinstall as upgrade
+            _container_exec(
+                container,
+                f"sed -i 's/^Version: {version}$/Version: 0.2.0/' "
+                "/var/lib/dpkg/status",
+            )
+            rc, out = _container_exec(
+                container,
+                f"dpkg --force-confnew -i /pkg/{pkg_name} 2>&1 || "
+                "apt-get install -f -y 2>&1 || true",
+            )
+        else:
+            # RPM: invoke upgrade-path scriptlets ($1=1 for preun/postun)
+            _container_exec(
+                container,
+                f"rpm -q --scripts -p /pkg/{pkg_name} "
+                "| sed -n '/^preuninstall scriptlet/,/^postinstall scriptlet/"
+                "{/^postinstall scriptlet/d;/^preuninstall scriptlet/d;p}' "
+                "| sh -s 1 2",
+            )
+            _container_exec(
+                container,
+                f"rpm -q --scripts -p /pkg/{pkg_name} "
+                "| sed -n '/^postinstall scriptlet/,/^preuninstall/"
+                "{/^preuninstall/d;/^postinstall scriptlet/d;p}' "
+                "| sh -s 2 2",
+            )
+            _container_exec(
+                container,
+                f"rpm -q --scripts -p /pkg/{pkg_name} "
+                "| sed -n '/^postuninstall scriptlet/,/^preuninstall/"
+                "{/^preuninstall/d;/^postuninstall scriptlet/d;p}' "
+                "| sh -s 1",
+            )
+
+        # Timer still exists — removal scripts were no-ops, no disable
+        rc, _ = _container_exec(
+            container,
+            "test -f /usr/lib/systemd/system/fenris-collect.timer",
+        )
+        assert rc == 0, (
+            "Timer removed during upgrade — sanctioned disable may have run"
+        )
+
+        # Helper still exists
+        rc, _ = _container_exec(
+            container,
+            "test -x /usr/libexec/fenris/fenris-monitor",
+        )
+        assert rc == 0, "Helper removed during upgrade"
     finally:
         subprocess.run(
-            ["docker", "rm", "-f", container],
+            ["docker", "rm", "-f", container], capture_output=True,
-            capture_output=True,
+        )
+    """deb purge removes config, store, backup, and group (spec §7, #49)."""
+    pkg = _find_package("deb")
+    pkg_name = pkg.name
+
+    build_dir = REPO_ROOT / "build" / "test-container-deb-purge"
+    build_dir.mkdir(parents=True, exist_ok=True)
+    (build_dir / "dist").mkdir(exist_ok=True)
+    subprocess.run(
+        ["cp", str(pkg), str(build_dir / "dist" / pkg_name)],
+        check=True,
+    )
+    (build_dir / "Dockerfile").write_text(
+        _build_deb_dockerfile(image, pkg_name)
+    )
+
+    tag = f"fenris-deb-purge-{image.replace(':', '-')}"
+    subprocess.run(
+        ["docker", "build", "-t", tag, str(build_dir)],
+        check=True, capture_output=True, timeout=300,
+    )
+
+    container = f"fenris-deb-purge-{os.getpid()}"
+    subprocess.run(
+        ["docker", "run", "-d", "--name", container,
+         "--tmpfs", "/tmp:exec,size=64m", tag, "sleep", "infinity"],
+        check=True, capture_output=True,
+    )
+
+    try:
+        _setup_store_and_config(container)
+        rc, out = _container_exec(container, "dpkg --purge fenris 2>&1")
+        assert rc == 0, f"dpkg --purge failed: {out}"
+        _assert_deb_purge_removes(container)
+        _assert_package_files_removed(container)
+    finally:
+        subprocess.run(
+            ["docker", "rm", "-f", container], capture_output=True,
+        )
+
+
+@pytest.mark.slow
+def test_rpm_erase_modified_config_preserved(skip_no_docker, version):
+    """rpm erase preserves modified config as .rpmsave (spec §7, #49)."""
+    pkg = _find_package("rpm")
+    pkg_name = pkg.name
+
+    build_dir = REPO_ROOT / "build" / "test-container-rpm-erase-mod"
+    build_dir.mkdir(parents=True, exist_ok=True)
+    (build_dir / "dist").mkdir(exist_ok=True)
+    subprocess.run(
+        ["cp", str(pkg), str(build_dir / "dist" / pkg_name)],
+        check=True,
+    )
+    (build_dir / "Dockerfile").write_text(
+        _build_rpm_dockerfile("fedora:40", pkg_name)
+    )
+
+    tag = "fenris-rpm-erase-mod"
+    subprocess.run(
+        ["docker", "build", "-t", tag, str(build_dir)],
+        check=True, capture_output=True, timeout=300,
+    )
+
+    container = f"fenris-rpm-erase-{os.getpid()}"
+    subprocess.run(
+        ["docker", "run", "-d", "--name", container,
+         "--tmpfs", "/tmp:exec,size=64m", tag, "sleep", "infinity"],
+        check=True, capture_output=True,
+    )
+
+    try:
+        _setup_store_and_config(container)
+        # Install RPM (installs default fenris.conf, but we modified it above)
+        rc, out = _container_exec(
+            container, f"rpm -ivh /pkg/{pkg_name} 2>&1"
+        )
+        assert rc == 0, f"rpm -ivh failed: {out}"
+        # Modify config after install (simulate hand-edited device selector)
+        _container_exec(
+            container,
+            "echo 'devices = /dev/nvme0n1' > /etc/fenris/fenris.conf",
+        )
+        # Erase — RPM should save modified config as .rpmsave
+        rc, out = _container_exec(container, "rpm -e fenris 2>&1")
+        assert rc == 0, f"rpm -e failed: {out}"
+        _assert_rpm_erase_preserves_modified(container)
+        _assert_package_files_removed(container)
+        # Sanctioned disable runs in %preun on erase — verified indirectly by
+        # the timer no longer being active.  "Never on upgrade" is covered by
+        # test_upgrade_semantics (criterion 4: removal scripts are no-ops).
+    finally:
+        subprocess.run(
+            ["docker", "rm", "-f", container], capture_output=True,
+        )
+
+
+@pytest.mark.slow
+def test_rpm_erase_unmodified_config_removed(skip_no_docker, version):
+    """rpm erase removes unmodified config (spec §7, #49)."""
+    pkg = _find_package("rpm")
+    pkg_name = pkg.name
+
+    build_dir = REPO_ROOT / "build" / "test-container-rpm-erase-unmod"
+    build_dir.mkdir(parents=True, exist_ok=True)
+    (build_dir / "dist").mkdir(exist_ok=True)
+    subprocess.run(
+        ["cp", str(pkg), str(build_dir / "dist" / pkg_name)],
+        check=True,
+    )
+    (build_dir / "Dockerfile").write_text(
+        _build_rpm_dockerfile("fedora:40", pkg_name)
+    )
+
+    tag = "fenris-rpm-erase-unmod"
+    subprocess.run(
+        ["docker", "build", "-t", tag, str(build_dir)],
+        check=True, capture_output=True, timeout=300,
+    )
+
+    container = f"fenris-rpm-erase-unmod-{os.getpid()}"
+    subprocess.run(
+        ["docker", "run", "-d", "--name", container,
+         "--tmpfs", "/tmp:exec,size=64m", tag, "sleep", "infinity"],
+        check=True, capture_output=True,
+    )
+
+    try:
+        _setup_store_and_config(container)
+        # Install RPM (default config)
+        rc, out = _container_exec(
+            container, f"rpm -ivh /pkg/{pkg_name} 2>&1"
+        )
+        assert rc == 0, f"rpm -ivh failed: {out}"
+        # Erase without modifying config — default should be removed
+        rc, out = _container_exec(container, "rpm -e fenris 2>&1")
+        assert rc == 0, f"rpm -e failed: {out}"
+        _assert_rpm_erase_removes_unmodified(container)
+        _assert_package_files_removed(container)
+    finally:
+        subprocess.run(
+            ["docker", "rm", "-f", container], capture_output=True,
         )