mirror of
https://github.com/rommapp/romm.git
synced 2026-06-27 22:35:57 +00:00
test(roms): add fixture-driven multi-file ROM download test
Add a shared `multi_file_rom` fixture (a game folder with multiple
RomFile rows) and an endpoint-level test that downloads it via
`GET /api/roms/{id}/content/{file_name}`. This exercises the multi-file
download path end-to-end, which builds each mod_zip manifest entry from
`file.rom.full_path` after the handler session has closed — the exact
path that 500'd with `DetachedInstanceError` before the backref fix.
The download endpoint had no test coverage for multi-file ROMs (the
`rom` fixture has no RomFile rows), which is why the regression slipped
through. Reuse the new fixture in the handler-level regression test too.
https://claude.ai/code/session_01PSXKmejPRzdxLFMN6P2QQ4
This commit is contained in:
@@ -23,7 +23,7 @@ from models.device import Device
|
||||
from models.device_save_sync import DeviceSaveSync
|
||||
from models.platform import Platform
|
||||
from models.play_session import PlaySession
|
||||
from models.rom import Rom
|
||||
from models.rom import Rom, RomFile
|
||||
from models.sync_session import SyncSession
|
||||
from models.user import Role, User
|
||||
|
||||
@@ -47,6 +47,7 @@ def clear_database():
|
||||
s.query(Save).delete(synchronize_session="evaluate")
|
||||
s.query(State).delete(synchronize_session="evaluate")
|
||||
s.query(Screenshot).delete(synchronize_session="evaluate")
|
||||
s.query(RomFile).delete(synchronize_session="evaluate")
|
||||
s.query(Rom).delete(synchronize_session="evaluate")
|
||||
s.query(Platform).delete(synchronize_session="evaluate")
|
||||
s.query(User).delete(synchronize_session="evaluate")
|
||||
@@ -88,6 +89,41 @@ def rom(admin_user: User, platform: Platform):
|
||||
return rom
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def multi_file_rom(admin_user: User, platform: Platform):
|
||||
"""A ROM stored as a game folder with multiple files (e.g. multi-disc).
|
||||
|
||||
Exercises the multi-file download path, where each entry's download name is
|
||||
derived from `file.rom.full_path` — the back-reference that must remain
|
||||
usable after the handler session closes.
|
||||
"""
|
||||
rom = Rom(
|
||||
platform_id=platform.id,
|
||||
name="test_multi_file_rom",
|
||||
slug="test_multi_file_rom_slug",
|
||||
fs_name="test_multi_file_rom",
|
||||
fs_name_no_tags="test_multi_file_rom",
|
||||
fs_name_no_ext="test_multi_file_rom",
|
||||
fs_extension="",
|
||||
fs_path=f"{platform.slug}/roms",
|
||||
)
|
||||
rom = db_rom_handler.add_rom(rom)
|
||||
db_rom_handler.add_rom_user(rom_id=rom.id, user_id=admin_user.id)
|
||||
|
||||
folder_path = f"{rom.fs_path}/{rom.fs_name}"
|
||||
for file_name in ("disc1.bin", "disc2.bin"):
|
||||
db_rom_handler.add_rom_file(
|
||||
RomFile(
|
||||
rom_id=rom.id,
|
||||
file_name=file_name,
|
||||
file_path=folder_path,
|
||||
file_size_bytes=1,
|
||||
)
|
||||
)
|
||||
|
||||
return db_rom_handler.get_rom(rom.id)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def save(rom: Rom, platform: Platform, admin_user: User):
|
||||
"""Slot-bound save (the canonical device-uploaded shape).
|
||||
|
||||
@@ -38,6 +38,29 @@ def test_get_rom(client: TestClient, access_token: str, rom: Rom):
|
||||
assert body["id"] == rom.id
|
||||
|
||||
|
||||
def test_download_multi_file_rom_content(
|
||||
client: TestClient, access_token: str, multi_file_rom: Rom
|
||||
):
|
||||
"""Downloading a multi-file (game folder) ROM must not 500.
|
||||
|
||||
The download endpoint builds each manifest entry's name from
|
||||
`file.rom.full_path` after the handler session has closed; a missing
|
||||
`RomFile.rom` back-reference previously raised `DetachedInstanceError`.
|
||||
"""
|
||||
response = client.get(
|
||||
f"/api/roms/{multi_file_rom.id}/content/{multi_file_rom.fs_name}",
|
||||
headers={"Authorization": f"Bearer {access_token}"},
|
||||
)
|
||||
|
||||
assert response.status_code == status.HTTP_200_OK
|
||||
# mod_zip manifest: one line per file, plus a generated .m3u playlist.
|
||||
assert response.headers["X-Archive-Files"] == "zip"
|
||||
body = response.text
|
||||
assert "disc1.bin" in body
|
||||
assert "disc2.bin" in body
|
||||
assert f"{multi_file_rom.fs_name}.m3u" in body
|
||||
|
||||
|
||||
def test_get_all_roms(
|
||||
client: TestClient, access_token: str, rom: Rom, platform: Platform
|
||||
):
|
||||
|
||||
@@ -14,7 +14,7 @@ from handler.database import (
|
||||
)
|
||||
from models.assets import Save, Screenshot, State
|
||||
from models.platform import Platform
|
||||
from models.rom import Rom, RomFile
|
||||
from models.rom import Rom
|
||||
from models.user import Role, User
|
||||
|
||||
|
||||
@@ -73,35 +73,24 @@ def test_roms(rom: Rom, platform: Platform):
|
||||
assert len(roms) == 1
|
||||
|
||||
|
||||
def test_multi_file_rom_backref_survives_session_close(rom: Rom, platform: Platform):
|
||||
def test_multi_file_rom_backref_survives_session_close(multi_file_rom: Rom):
|
||||
"""Multi-file ROM downloads read `file.rom.full_path` after the handler
|
||||
session closes. The detail loaders eager-load `Rom.files` but not the
|
||||
reverse `RomFile.rom` relationship, so without the backref being populated
|
||||
this raises `DetachedInstanceError` (a 500 on the download endpoint).
|
||||
"""
|
||||
folder_path = f"{rom.fs_path}/{rom.fs_name}"
|
||||
for file_name in ("disc1.bin", "disc2.bin"):
|
||||
db_rom_handler.add_rom_file(
|
||||
RomFile(
|
||||
rom_id=rom.id,
|
||||
file_name=file_name,
|
||||
file_path=folder_path,
|
||||
file_size_bytes=1,
|
||||
)
|
||||
)
|
||||
folder_path = f"{multi_file_rom.fs_path}/{multi_file_rom.fs_name}"
|
||||
|
||||
fetched = db_rom_handler.get_rom(rom.id)
|
||||
assert fetched is not None
|
||||
assert len(fetched.files) == 2
|
||||
|
||||
# These all dereference `file.rom` on a now-detached instance.
|
||||
for file in fetched.files:
|
||||
# `multi_file_rom` is returned by `get_rom`, with the session already closed.
|
||||
assert len(multi_file_rom.files) == 2
|
||||
for file in multi_file_rom.files:
|
||||
# All of these dereference `file.rom` on a now-detached instance.
|
||||
assert file.rom.full_path == folder_path
|
||||
assert file.is_top_level
|
||||
assert file.file_name_for_download() == file.file_name
|
||||
|
||||
# `get_roms_by_ids` (used by the bulk zip download) must behave the same.
|
||||
by_ids = db_rom_handler.get_roms_by_ids([rom.id])
|
||||
by_ids = db_rom_handler.get_roms_by_ids([multi_file_rom.id])
|
||||
assert len(by_ids) == 1
|
||||
for file in by_ids[0].files:
|
||||
assert file.rom.full_path == folder_path
|
||||
|
||||
Reference in New Issue
Block a user