Tighten hash field tests and DRY response builder
Some checks are pending
Python Linting / Run Ruff (push) Waiting to run
Python Linting / Run Pylint (push) Waiting to run
Build package / Build Test (3.10) (push) Waiting to run
Build package / Build Test (3.11) (push) Waiting to run
Build package / Build Test (3.12) (push) Waiting to run
Build package / Build Test (3.13) (push) Waiting to run
Build package / Build Test (3.14) (push) Waiting to run

- Extract assert_hash_fields_consistent() helper that verifies presence
  parity and value equality, replacing body.get()-based assertions that
  treated missing keys and explicit nulls identically.
- Conftest seeded_asset fixture and seed-asset list assertions now check
  key absence directly, so a regression that surfaces null fields would
  be caught (validates exclude_none behavior).
- DRY duplicate hash expression in _build_asset_response.
- Add list-endpoint coverage asserting hash is present and consistent on
  populated assets.
- Add schema-level test asserting AssetCreated inherits the hash field
  from Asset, guarding against future inheritance drift.
This commit is contained in:
Matt Miller 2026-05-06 14:56:46 -07:00
parent b7f8a7a86c
commit 5130628ed5
7 changed files with 53 additions and 7 deletions

View File

@ -160,11 +160,12 @@ def _build_asset_response(result: schemas.AssetDetailResult | schemas.UploadResu
preview_url = None
else:
preview_url = _build_preview_url_from_view(result.tags, result.ref.user_metadata)
asset_content_hash = result.asset.hash if result.asset else None
return schemas_out.Asset(
id=result.ref.id,
name=result.ref.name,
hash=result.asset.hash if result.asset else None,
asset_hash=result.asset.hash if result.asset else None,
hash=asset_content_hash,
asset_hash=asset_content_hash,
size=int(result.asset.size_bytes) if result.asset else None,
mime_type=result.asset.mime_type if result.asset else None,
tags=result.tags,

View File

@ -236,7 +236,8 @@ def seeded_asset(request: pytest.FixtureRequest, http: requests.Session, api_bas
r = http.post(api_base + "/api/assets", files=files, data=form_data, timeout=120)
body = r.json()
assert r.status_code == 201, body
assert body.get("hash") == body.get("asset_hash")
from helpers import assert_hash_fields_consistent
assert_hash_fields_consistent(body)
return body

View File

@ -26,3 +26,26 @@ def trigger_sync_seed_assets(session: requests.Session, base_url: str) -> None:
def get_asset_filename(asset_hash: str, extension: str) -> str:
return asset_hash.removeprefix("blake3:") + extension
def assert_hash_fields_consistent(body: dict, expected_hash: str | None = None) -> None:
"""Assert hash and asset_hash invariants on an Asset response.
Both must be present or both absent (so a regression that drops only one
is caught). When present, they must equal each other and, if expected_hash
is provided, must equal that value.
"""
hash_present = "hash" in body
asset_hash_present = "asset_hash" in body
assert hash_present == asset_hash_present, (
f"hash and asset_hash must both be present or both absent: "
f"hash present={hash_present}, asset_hash present={asset_hash_present}"
)
if hash_present:
h = body["hash"]
ah = body["asset_hash"]
assert h == ah, f"hash and asset_hash must match: hash={h!r}, asset_hash={ah!r}"
if expected_hash is not None:
assert h == expected_hash, (
f"hash must equal expected: got {h!r}, expected {expected_hash!r}"
)

View File

@ -40,8 +40,9 @@ def test_seed_asset_removed_when_file_is_deleted(
# there should be exactly one with that name
matches = [a for a in body1.get("assets", []) if a.get("name") == name]
assert matches
assert matches[0].get("asset_hash") is None
assert matches[0].get("hash") is None
# Seed assets have no hash; exclude_none drops both keys from the response
assert "asset_hash" not in matches[0]
assert "hash" not in matches[0]
asset_info_id = matches[0]["id"]
# Remove the underlying file and sync again

View File

@ -294,8 +294,9 @@ def test_metadata_filename_is_set_for_seed_asset_without_hash(
assert r1.status_code == 200, body
matches = [a for a in body.get("assets", []) if a.get("name") == name]
assert matches, "Seed asset should be visible after sync"
assert matches[0].get("asset_hash") is None # still a seed
assert matches[0].get("hash") is None # still a seed
# Seed assets have no hash; exclude_none drops both keys from the response
assert "asset_hash" not in matches[0]
assert "hash" not in matches[0]
aid = matches[0]["id"]
r2 = http.get(f"{api_base}/api/assets/{aid}", timeout=120)

View File

@ -3,6 +3,7 @@ import uuid
import pytest
import requests
from helpers import assert_hash_fields_consistent
def test_list_assets_paging_and_sort(http: requests.Session, api_base: str, asset_factory, make_asset_bytes):
@ -26,6 +27,10 @@ def test_list_assets_paging_and_sort(http: requests.Session, api_base: str, asse
got1 = [a["name"] for a in b1["assets"]]
assert got1 == sorted(names)[:2]
assert b1["has_more"] is True
# Populated assets in list responses must carry both `hash` and `asset_hash` consistently
for asset in b1["assets"]:
assert_hash_fields_consistent(asset)
assert "hash" in asset, "populated asset must emit hash on list endpoint"
r2 = http.get(
api_base + "/api/assets",

View File

@ -5,6 +5,20 @@ from concurrent.futures import ThreadPoolExecutor
import requests
import pytest
from app.assets.api.schemas_out import Asset, AssetCreated
def test_asset_created_inherits_hash_field():
"""AssetCreated must inherit `hash` from Asset so POST /api/assets responses emit it.
Schema-level guard: integration tests cover the wire shape, but inheritance
drift (e.g. AssetCreated ever being redefined to no longer extend Asset)
would silently drop `hash` from a major endpoint without this check.
"""
assert "hash" in Asset.model_fields
assert "hash" in AssetCreated.model_fields
assert AssetCreated.model_fields["hash"].annotation == Asset.model_fields["hash"].annotation
def test_upload_ok_duplicate_reference(http: requests.Session, api_base: str, make_asset_bytes):
name = "dup_a.safetensors"