mirror of
https://github.com/rommapp/romm.git
synced 2026-10-12 02:21:58 +00:00
fix(scan): make scan tracker totals reflect platforms actually scanned
The scan tracker computed total_platforms and total_roms over every filesystem platform, ignoring both the selected platforms and the scan type. For a "new platforms" scan, existing platforms are skipped inside _identify_platform, so their ROMs never count toward scanned_roms, yet they were all included in total_roms. This made the tracker wildly overcount (the whole library instead of just the new platform). Resolve the platform list before computing totals and, for NEW_PLATFORMS scans, exclude platforms that already exist in the database so the totals match what is actually processed. Fixes #3599 Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_0158RJnc7MmAbwRz6Qiyrj5a
This commit is contained in:
@@ -699,8 +699,27 @@ async def scan_platforms(
|
||||
if MetadataSource.HLTB in metadata_sources:
|
||||
meta_hltb_handler.initialize()
|
||||
|
||||
# Resolve the platforms that will actually be scanned. When no platform ids
|
||||
# are provided, every filesystem platform is scanned.
|
||||
platform_list = [
|
||||
platform.fs_slug
|
||||
for s in platform_ids
|
||||
if (platform := db_platform_handler.get_platform(s)) is not None
|
||||
] or fs_platforms
|
||||
platform_list = sorted(platform_list)
|
||||
|
||||
# A "new platforms" scan skips platforms that already exist in the database,
|
||||
# so they must be excluded from the totals to keep the tracker accurate.
|
||||
platforms_to_scan = platform_list
|
||||
if scan_type == ScanType.NEW_PLATFORMS:
|
||||
platforms_to_scan = [
|
||||
platform_slug
|
||||
for platform_slug in platform_list
|
||||
if db_platform_handler.get_platform_by_fs_slug(platform_slug) is None
|
||||
]
|
||||
|
||||
total_roms = 0
|
||||
for platform_slug in fs_platforms:
|
||||
for platform_slug in platforms_to_scan:
|
||||
try:
|
||||
total_roms += await fs_rom_handler.count_roms(
|
||||
Platform(fs_slug=platform_slug)
|
||||
@@ -710,7 +729,7 @@ async def scan_platforms(
|
||||
|
||||
await scan_stats.update(
|
||||
socket_manager=socket_manager,
|
||||
total_platforms=len(fs_platforms),
|
||||
total_platforms=len(platforms_to_scan),
|
||||
total_roms=total_roms,
|
||||
)
|
||||
|
||||
@@ -720,13 +739,6 @@ async def scan_platforms(
|
||||
redis_client.delete(STOP_SCAN_FLAG)
|
||||
|
||||
try:
|
||||
platform_list = [
|
||||
platform.fs_slug
|
||||
for s in platform_ids
|
||||
if (platform := db_platform_handler.get_platform(s)) is not None
|
||||
] or fs_platforms
|
||||
platform_list = sorted(platform_list)
|
||||
|
||||
if len(platform_list) == 0:
|
||||
log.warning(
|
||||
f"{hl(emoji.EMOJI_WARNING, color=LIGHTYELLOW)} No platforms found, verify that the folder structure is right and the volume is mounted correctly."
|
||||
|
||||
@@ -1,9 +1,10 @@
|
||||
from unittest.mock import Mock
|
||||
from unittest.mock import AsyncMock, MagicMock, Mock
|
||||
|
||||
import pytest
|
||||
import socketio
|
||||
|
||||
from endpoints.sockets.scan import ScanStats, _should_scan_rom
|
||||
from endpoints.sockets import scan as scan_module
|
||||
from endpoints.sockets.scan import ScanStats, _should_scan_rom, scan_platforms
|
||||
from handler.filesystem.roms_handler import FSRomsHandler
|
||||
from handler.metadata.base_handler import UniversalPlatformSlug as UPS
|
||||
from handler.scan_handler import ScanType
|
||||
@@ -74,6 +75,89 @@ async def test_merging_scan_stats():
|
||||
assert stats.new_firmware == 25
|
||||
|
||||
|
||||
class TestScanTotals:
|
||||
"""The scan tracker totals must reflect the platforms/roms actually scanned."""
|
||||
|
||||
@pytest.fixture
|
||||
def patched(self, mocker):
|
||||
"""Patch the collaborators of scan_platforms so totals can be inspected."""
|
||||
socket_manager = AsyncMock()
|
||||
mocker.patch.object(
|
||||
scan_module, "_get_socket_manager", return_value=socket_manager
|
||||
)
|
||||
mocker.patch.object(
|
||||
scan_module.fs_platform_handler,
|
||||
"get_platforms",
|
||||
AsyncMock(return_value=["existing", "new1", "new2"]),
|
||||
)
|
||||
# Each platform reports 100 roms on disk.
|
||||
mocker.patch.object(
|
||||
scan_module.fs_rom_handler, "count_roms", AsyncMock(return_value=100)
|
||||
)
|
||||
mocker.patch.object(scan_module.meta_gamelist_handler, "clear_cache")
|
||||
mocker.patch.object(
|
||||
scan_module.db_platform_handler, "mark_missing_platforms", return_value=[]
|
||||
)
|
||||
mocker.patch.object(
|
||||
scan_module.db_platform_handler, "get_platforms", return_value=[]
|
||||
)
|
||||
mocker.patch.object(
|
||||
scan_module.db_rom_handler, "invalidate_filter_values_cache"
|
||||
)
|
||||
config = MagicMock()
|
||||
config.GAMELIST_AUTO_EXPORT_ON_SCAN = False
|
||||
config.PEGASUS_AUTO_EXPORT_ON_SCAN = False
|
||||
mocker.patch.object(scan_module.cm, "get_config", return_value=config)
|
||||
|
||||
# Skip the actual per-platform scanning, returning the stats unchanged.
|
||||
async def fake_identify(**kwargs):
|
||||
return kwargs["scan_stats"]
|
||||
|
||||
mocker.patch.object(
|
||||
scan_module, "_identify_platform", side_effect=fake_identify
|
||||
)
|
||||
return socket_manager
|
||||
|
||||
async def test_new_platforms_total_excludes_existing(self, patched, mocker):
|
||||
"""NEW_PLATFORMS totals must skip platforms already in the database."""
|
||||
|
||||
def get_by_fs_slug(fs_slug):
|
||||
return MagicMock() if fs_slug == "existing" else None
|
||||
|
||||
mocker.patch.object(
|
||||
scan_module.db_platform_handler,
|
||||
"get_platform_by_fs_slug",
|
||||
side_effect=get_by_fs_slug,
|
||||
)
|
||||
|
||||
result = await scan_platforms(
|
||||
platform_ids=[],
|
||||
metadata_sources=[],
|
||||
scan_type=ScanType.NEW_PLATFORMS,
|
||||
)
|
||||
|
||||
# Only the two new platforms (and their roms) should be counted.
|
||||
assert result.total_platforms == 2
|
||||
assert result.total_roms == 200
|
||||
|
||||
async def test_complete_scan_counts_all_selected(self, patched, mocker):
|
||||
"""COMPLETE totals include every filesystem platform being scanned."""
|
||||
mocker.patch.object(
|
||||
scan_module.db_platform_handler,
|
||||
"get_platform_by_fs_slug",
|
||||
return_value=MagicMock(),
|
||||
)
|
||||
|
||||
result = await scan_platforms(
|
||||
platform_ids=[],
|
||||
metadata_sources=[],
|
||||
scan_type=ScanType.COMPLETE,
|
||||
)
|
||||
|
||||
assert result.total_platforms == 3
|
||||
assert result.total_roms == 300
|
||||
|
||||
|
||||
class TestShouldScanRom:
|
||||
def test_new_platforms_scan_with_no_rom(self):
|
||||
"""NEW_PLATFORMS should scan when rom is None"""
|
||||
|
||||
Reference in New Issue
Block a user