Simon/romm
1
0
Fork 0
mirror of https://github.com/rommapp/romm.git synced 2026-06-28 06:46:00 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: wrap empty parent dir cleanup in try-except and add test coverage for it

Browse Source 
Agent-Logs-Url: https://github.com/rommapp/romm/sessions/303f2c27-6b65-41a9-b201-c055142b1edb

Co-authored-by: gantoine <3247106+gantoine@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot]
2026-04-03 17:28:50 +00:00
committed by GitHub
parent e540d7c1a2
commit 394799d7c3
2 changed files with 64 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
backend/endpoints/roms/__init__.py
View File

@@ -1483,13 +1483,18 @@ async def delete_roms(
await fs_rom_handler.remove_file(file_path=rom_path)
# Clean up empty parent directory if it becomes empty
parent = full_path.parent
if (
parent != fs_rom_handler.base_path
and parent.is_dir()
and not any(parent.iterdir())
):
await fs_rom_handler.remove_directory(
str(parent.relative_to(fs_rom_handler.base_path))
try:
if (
parent != fs_rom_handler.base_path
and parent.is_dir()
and not any(parent.iterdir())
):
await fs_rom_handler.remove_directory(
str(parent.relative_to(fs_rom_handler.base_path))
)
except OSError as dir_err:
log.warning(
f"Couldn't clean up empty parent directory for {hl(rom.fs_name)}: {dir_err}"
)
except FileNotFoundError:
error = f"Rom file {hl(rom.fs_name)} not found for platform {hl(rom.platform_display_name, color=BLUE)}[{hl(rom.platform_slug)}]"

52
backend/tests/endpoints/roms/test_rom.py
View File

@@ -124,6 +124,10 @@ def test_delete_roms(client: TestClient, access_token: str, rom: Rom):
assert body["successful_items"] == 1
@patch(
"endpoints.roms.fs_rom_handler.remove_directory",
new_callable=AsyncMock,
)
@patch(
"endpoints.roms.fs_rom_handler.remove_file",
new_callable=AsyncMock,
@@ -134,6 +138,7 @@ def test_delete_roms(client: TestClient, access_token: str, rom: Rom):
def test_delete_roms_from_fs_flat(
mock_validate_path,
mock_remove_file,
mock_remove_directory,
client: TestClient,
access_token: str,
rom: Rom,
@@ -158,6 +163,53 @@ def test_delete_roms_from_fs_flat(
assert body["successful_items"] == 1
assert body["failed_items"] == 0
mock_remove_file.assert_called_once()
mock_remove_directory.assert_not_called()
@patch(
"endpoints.roms.fs_rom_handler.remove_directory",
new_callable=AsyncMock,
)
@patch(
"endpoints.roms.fs_rom_handler.remove_file",
new_callable=AsyncMock,
)
@patch(
"endpoints.roms.fs_rom_handler.validate_path",
)
def test_delete_roms_from_fs_flat_cleans_empty_parent(
mock_validate_path,
mock_remove_file,
mock_remove_directory,
client: TestClient,
access_token: str,
rom: Rom,
):
"""Test that empty parent directories are cleaned up after flat ROM file deletion."""
from pathlib import Path
from unittest.mock import MagicMock
mock_path = MagicMock(spec=Path)
mock_path.is_dir.return_value = False
# Parent is not the base_path, is a dir, and is empty
mock_path.parent.__ne__ = lambda self, other: True
mock_path.parent.is_dir.return_value = True
mock_path.parent.__iter__ = lambda self: iter([]) # empty directory
mock_validate_path.return_value = mock_path
response = client.post(
"/api/roms/delete",
headers={"Authorization": f"Bearer {access_token}"},
json={"roms": [rom.id], "delete_from_fs": [rom.id]},
)
assert response.status_code == status.HTTP_200_OK
body = response.json()
assert body["successful_items"] == 1
assert body["failed_items"] == 0
mock_remove_file.assert_called_once()
# remove_directory should be called to clean up the empty parent dir
mock_remove_directory.assert_called_once()
@patch(