refactor(assets): sweep dead code and dead parameters; expose protected tag bucket (review2-18)

This commit is contained in:
Simon Pinfold
2026-08-26 12:36:09 -07:00
parent 736e426eb9
commit e37f65a6a8
14 changed files with 57 additions and 127 deletions

View File

@@ -215,8 +215,9 @@ def _post_multipart_asset(
@pytest.fixture
def make_asset_bytes() -> Callable[[str, int], bytes]:
# Salt content per test so it never collides with assets left over from
# earlier tests. Delete is now always a soft delete (content is preserved),
# so the suite can no longer rely on hard-deleting content for isolation.
# earlier tests. Delete hard-deletes the record but preserves content
# (content rows and files are untouched), so the suite cannot rely on delete
# removing content for isolation.
# Deterministic within a test: the same (name, size) yields the same bytes.
salt = uuid.uuid4().bytes
@@ -262,8 +263,9 @@ def seeded_asset(request: pytest.FixtureRequest, http: requests.Session, api_bas
tags = ["models", "model_type:checkpoints", "unit-tests", "alpha"]
meta = {"purpose": "test", "epoch": 1, "flags": ["x", "y"], "nullable": None}
# Unique content per test so the seed always creates a fresh asset (201).
# Delete is now always a soft delete, so content from a prior test survives
# and would otherwise dedup this upload into an existing asset (200).
# Delete preserves content (only the record is hard-deleted), so content
# from a prior test survives and would otherwise dedup this upload into an
# existing asset (200).
content = uuid.uuid4().bytes + b"A" * (4096 - 16)
files = {"file": (name, content, "application/octet-stream")}
form_data = {

View File

@@ -7,6 +7,7 @@ import pytest
from sqlalchemy import create_engine, event
from sqlalchemy.orm import Session, Session as SASession
from app.assets import mode
from app.assets.database.models import Base
@@ -16,6 +17,20 @@ def autoclean_unit_test_assets():
yield
@pytest.fixture(autouse=True)
def initialised_hash_mode():
# mode._args is process-global and leaks across tests; hashing_enabled() now
# raises when it was never initialised. Give every service test a determinate
# hashing-off baseline. Tests needing hashing on override via their own
# fixture or by patching mode.hashing_enabled after this runs.
class _HashingOff:
enable_asset_hashing = False
mode.init(_HashingOff())
yield
mode.init(None)
@pytest.fixture
def db_engine():
"""In-memory SQLite engine for fast unit tests."""

View File

@@ -1,3 +1,4 @@
import json
from types import SimpleNamespace
import pytest
@@ -66,7 +67,7 @@ async def test_other_tags_unaffected(monkeypatch):
def remove_tags(**kwargs):
calls.append(("remove", kwargs["tags"]))
return SimpleNamespace(
removed=kwargs["tags"], not_present=[], total_tags=[]
removed=kwargs["tags"], not_present=[], total_tags=[], protected=[]
)
monkeypatch.setattr(routes, "apply_tags", apply_tags)
@@ -80,3 +81,24 @@ async def test_other_tags_unaffected(monkeypatch):
assert add_response.status == 200
assert remove_response.status == 200
assert calls == [("add", ["manual"]), ("remove", ["manual"])]
@pytest.mark.asyncio
async def test_remove_tags_response_exposes_protected_bucket(monkeypatch):
"""The DELETE /tags route must serialise the ``protected`` bucket rather than
drop it: a present-but-automatic tag the service reports as protected has to
reach the HTTP body so the contract matches RemoveTagsResult (review2-18)."""
monkeypatch.setattr(routes, "USER_MANAGER", _UserManager())
monkeypatch.setattr(
routes,
"remove_tags",
lambda **_kwargs: SimpleNamespace(
removed=[], not_present=[], total_tags=["auto"], protected=["auto"]
),
)
response = await routes.delete_asset_tags.__wrapped__(_JsonRequest(["auto"]))
assert response.status == 200
body = json.loads(response.body)
assert body["protected"] == ["auto"]