From 56e3ddba8a9199cb941173480dee780e8f1b2fb3 Mon Sep 17 00:00:00 2001 From: Marcin Macierzanka Date: Thu, 1 Oct 2026 13:59:19 +0200 Subject: [PATCH] fix: tests and linters --- tests/test_api_surface.py | 7 ++- tests/test_guild_config_repository.py | 45 ++++++++-------- tests/test_history_repository.py | 73 +++++++++++++------------- tests/test_music_player.py | 75 ++++++++++++--------------- tests/test_queue_repository.py | 49 ++++++++--------- tests/test_shutdown_voice_cleanup.py | 2 + tests/test_uploads_repository.py | 2 +- 7 files changed, 122 insertions(+), 131 deletions(-) diff --git a/tests/test_api_surface.py b/tests/test_api_surface.py index ce612a1..268c9cf 100644 --- a/tests/test_api_surface.py +++ b/tests/test_api_surface.py @@ -4,6 +4,7 @@ from __future__ import annotations import tempfile import unittest +from unittest.mock import MagicMock from zyrarbot.application import Application from zyrarbot.settings import Settings @@ -49,7 +50,11 @@ class ApiSurfaceTest(unittest.TestCase): uploads_dir=f"{cls._workdir.name}/uploads", static_dir=f"{cls._workdir.name}/static", ) - cls.spec = Application(settings).web.openapi() + persistence = MagicMock() + persistence.config = MagicMock() + persistence.history = MagicMock() + persistence.queues = MagicMock() + cls.spec = Application(settings, persistence=persistence).web.openapi() @classmethod def tearDownClass(cls) -> None: diff --git a/tests/test_guild_config_repository.py b/tests/test_guild_config_repository.py index c01ec99..de9fcd6 100644 --- a/tests/test_guild_config_repository.py +++ b/tests/test_guild_config_repository.py @@ -1,32 +1,33 @@ -import json -import tempfile -import unittest -from pathlib import Path +from __future__ import annotations -from zyrarbot.storage.guild_config_repository import GuildConfigRepository -from zyrarbot.storage.json_file_store import JsonFileStore +import unittest +from unittest.mock import MagicMock + +from zyrarbot.storage.adapters.postgres.guild_config import PostgresGuildConfig class GuildConfigRepositoryTest(unittest.TestCase): - def test_settings_are_isolated_and_legacy_default_moves_to_first_guild(self): - with tempfile.TemporaryDirectory() as directory: - path = Path(directory) / "config.json" - path.write_text(json.dumps({"default_plays": 7, "10": 123}), encoding="utf-8") - repository = GuildConfigRepository(JsonFileStore(str(path)), fallback_plays=5) + def setUp(self): + self.database = MagicMock() + self.connection = self.database.connect.return_value.__enter__.return_value + self.repository = PostgresGuildConfig(self.database, fallback_plays=5) - repository.migrate_legacy(10) - self.assertEqual(repository.get_default_plays(10), 7) - self.assertEqual(repository.get_notification_channel_id(10), 123) - self.assertEqual(repository.get_default_plays(20), 5) + def test_default_plays_uses_fallback_when_guild_has_no_override(self): + self.connection.execute.return_value.fetchone.return_value = (None,) - repository.set_default_plays(20, 9) - self.assertEqual(repository.get_default_plays(10), 7) - self.assertEqual(repository.get_default_plays(20), 9) + self.assertEqual(self.repository.get_default_plays(10), 5) - saved = json.loads(path.read_text(encoding="utf-8")) - self.assertNotIn("default_plays", saved) - self.assertEqual(saved["10"]["default_plays"], 7) - self.assertEqual(saved["20"]["default_plays"], 9) + def test_default_plays_returns_guild_override(self): + self.connection.execute.return_value.fetchone.return_value = (7,) + + self.assertEqual(self.repository.get_default_plays(10), 7) + + def test_set_default_plays_creates_guild_then_updates_setting(self): + self.repository.set_default_plays(10, 9) + + statements = [call.args[0] for call in self.connection.execute.call_args_list] + self.assertIn("INSERT INTO guilds", statements[0]) + self.assertIn("UPDATE guilds SET default_plays", statements[1]) if __name__ == "__main__": diff --git a/tests/test_history_repository.py b/tests/test_history_repository.py index e9cc156..5353c42 100644 --- a/tests/test_history_repository.py +++ b/tests/test_history_repository.py @@ -1,52 +1,49 @@ -import json -import tempfile +from __future__ import annotations + import unittest -from pathlib import Path +from unittest.mock import MagicMock from zyrarbot.music.history_entry import HistoryEntry -from zyrarbot.storage.history_repository import HistoryRepository -from zyrarbot.storage.json_file_store import JsonFileStore +from zyrarbot.storage.adapters.postgres.history import PostgresHistory class HistoryRepositoryTest(unittest.TestCase): - def test_history_is_stored_in_separate_files_and_legacy_data_is_migrated(self): - with tempfile.TemporaryDirectory() as directory: - legacy_path = Path(directory) / "history.json" - entry = HistoryEntry( - id="track-1", - title="Track", - url="https://example.test/track", - duration=10, - is_local=False, - filepath=None, - ) - legacy_path.write_text(json.dumps({"10": [entry.to_dict()]}), encoding="utf-8") - repository = HistoryRepository(JsonFileStore(str(legacy_path))) + def setUp(self): + self.database = MagicMock() + self.connection = self.database.connect.return_value.__enter__.return_value + self.repository = PostgresHistory(self.database) - repository.migrate_legacy(10) + def test_list_maps_database_rows_to_history_entries(self): + self.connection.execute.return_value.fetchall.return_value = [ + ("track-1", "Track", "https://example.test/track", 10, False) + ] - self.assertEqual(repository.list(10)[0].id, "track-1") - self.assertEqual(repository.list(20), []) - self.assertTrue((Path(directory) / "history" / "10.json").exists()) - self.assertFalse((Path(directory) / "history" / "20.json").exists()) + self.assertEqual( + self.repository.list(10), + [ + HistoryEntry( + id="track-1", + title="Track", + url="https://example.test/track", + duration=10, + is_local=False, + ) + ], + ) - def test_new_entries_are_available_after_repository_reload(self): - with tempfile.TemporaryDirectory() as directory: - store = JsonFileStore(str(Path(directory) / "history.json")) - entry = HistoryEntry( - id="track-2", - title="Another track", - url="https://example.test/another-track", - ) + def test_add_ignores_entries_without_a_title(self): + self.repository.add(10, HistoryEntry(id="track-1", title="")) - HistoryRepository(store).add(42, entry) + self.database.connect.assert_not_called() - reloaded = HistoryRepository(store) - self.assertEqual(reloaded.list(42), [entry]) - persisted = json.loads( - (Path(directory) / "history" / "42.json").read_text(encoding="utf-8") - ) - self.assertEqual(persisted["entries"][0]["id"], "track-2") + def test_add_persists_entry_for_guild(self): + self.connection.execute.return_value.fetchall.return_value = [] + entry = HistoryEntry(id="track-1", title="Track", url="https://example.test/track") + + self.repository.add(10, entry) + + statements = [call.args[0] for call in self.connection.execute.call_args_list] + self.assertTrue(any("INSERT INTO history_entries" in statement for statement in statements)) if __name__ == "__main__": diff --git a/tests/test_music_player.py b/tests/test_music_player.py index ac9d746..08f77e9 100644 --- a/tests/test_music_player.py +++ b/tests/test_music_player.py @@ -3,12 +3,11 @@ from __future__ import annotations import asyncio -import tempfile import unittest +from unittest.mock import MagicMock from tests.fakes import FakeTrack, build_player from zyrarbot.music.loop_mode import LoopMode -from zyrarbot.storage.queue_repository import QueueRepository GUILD = 1 @@ -293,28 +292,26 @@ class MusicPlayerQueueTest(unittest.IsolatedAsyncioTestCase): async def test_restore_replaces_active_queue_without_old_callback_interference(self): player, voice_client = build_player() - with tempfile.TemporaryDirectory() as directory: - player._queues = QueueRepository(directory) - player._queues.save( - GUILD, - [{"id": "restored", "title": "Restored", "plays_left": 3}], - ) + player._queues = MagicMock() + player._queues.load.return_value = [ + {"id": "restored", "title": "Restored", "plays_left": 3} + ] - async def from_url(url, loop=None): - return FakeTrack("restored") + async def from_url(url, loop=None): + return FakeTrack("restored") - player._tracks.from_url = from_url - await player.enqueue(GUILD, FakeTrack("old")) - old_callback = voice_client.after + player._tracks.from_url = from_url + await player.enqueue(GUILD, FakeTrack("old")) + old_callback = voice_client.after - count = await player.restore_queue(GUILD) + count = await player.restore_queue(GUILD) - self.assertEqual(count, 1) - self.assertEqual([track.id for track in player.state(GUILD).queue], ["restored"]) - self.assertIs(player.state(GUILD).current, player.state(GUILD).queue[0]) - old_callback(None) - await asyncio.sleep(0) - self.assertEqual([track.id for track in player.state(GUILD).queue], ["restored"]) + self.assertEqual(count, 1) + self.assertEqual([track.id for track in player.state(GUILD).queue], ["restored"]) + self.assertIs(player.state(GUILD).current, player.state(GUILD).queue[0]) + old_callback(None) + await asyncio.sleep(0) + self.assertEqual([track.id for track in player.state(GUILD).queue], ["restored"]) async def test_restore_requires_voice_connection(self): player, _ = build_player() @@ -324,34 +321,28 @@ class MusicPlayerQueueTest(unittest.IsolatedAsyncioTestCase): await player.restore_queue(GUILD) def test_save_queue_does_not_overwrite_with_empty_queue(self): - with tempfile.TemporaryDirectory() as directory: - player, _ = build_player() - player._queues = QueueRepository(directory) - saved = [{"id": "saved", "title": "Saved", "plays_left": 2}] - player._queues.save(GUILD, saved) + player, _ = build_player() + player._queues = MagicMock() - # Kolejka w stanie odtwarzacza jest pusta - player.state(GUILD).queue = [] - player.save_queue(GUILD) + # Kolejka w stanie odtwarzacza jest pusta + player.state(GUILD).queue = [] + player.save_queue(GUILD) - # Zapisana wcześniej kolejka powinna pozostać nienaruszona - self.assertEqual(player._queues.load(GUILD), saved) + # Zapisana wcześniej kolejka powinna pozostać nienaruszona + player._queues.save.assert_not_called() def test_save_queue_does_not_save_when_all_plays_consumed(self): - with tempfile.TemporaryDirectory() as directory: - player, _ = build_player() - player._queues = QueueRepository(directory) - saved = [{"id": "saved", "title": "Saved", "plays_left": 2}] - player._queues.save(GUILD, saved) + player, _ = build_player() + player._queues = MagicMock() - # Wszystkie utwory w kolejce mają plays_left = 0 - track = FakeTrack("consumed") - track.plays_left = 0 - player.state(GUILD).queue = [track] - player.save_queue(GUILD) + # Wszystkie utwory w kolejce mają plays_left = 0 + track = FakeTrack("consumed") + track.plays_left = 0 + player.state(GUILD).queue = [track] + player.save_queue(GUILD) - # Zapisana wcześniej kolejka powinna pozostać nienaruszona - self.assertEqual(player._queues.load(GUILD), saved) + # Zapisana wcześniej kolejka powinna pozostać nienaruszona + player._queues.save.assert_not_called() class LoopModeTest(unittest.TestCase): diff --git a/tests/test_queue_repository.py b/tests/test_queue_repository.py index f1a36a1..2c569f9 100644 --- a/tests/test_queue_repository.py +++ b/tests/test_queue_repository.py @@ -1,40 +1,35 @@ -import tempfile -import unittest -from pathlib import Path +from __future__ import annotations -from zyrarbot.storage.queue_repository import QueueRepository +import unittest +from unittest.mock import MagicMock + +from zyrarbot.storage.adapters.postgres.queue import PostgresQueue class QueueRepositoryTest(unittest.TestCase): - def test_snapshots_are_isolated_per_guild(self): - with tempfile.TemporaryDirectory() as directory: - repository = QueueRepository(directory) - first = [{"id": "first", "title": "First", "plays_left": 2}] - second = [{"id": "second", "title": "Second", "plays_left": 4}] + def setUp(self): + self.database = MagicMock() + self.connection = self.database.connect.return_value.__enter__.return_value + self.repository = PostgresQueue(self.database) - repository.save(10, first) - repository.save(20, second) + def test_load_returns_guild_snapshot(self): + snapshot = [{"id": "track-1", "title": "Track", "plays_left": 2}] + self.connection.execute.return_value.fetchone.return_value = (snapshot,) - self.assertEqual(repository.load(10), first) - self.assertEqual(repository.load(20), second) - self.assertTrue((Path(directory) / "10.json").exists()) - self.assertTrue((Path(directory) / "20.json").exists()) + self.assertEqual(self.repository.load(10), snapshot) - def test_save_empty_tracks_does_nothing(self): - with tempfile.TemporaryDirectory() as directory: - repository = QueueRepository(directory) - tracks = [{"id": "first", "title": "First", "plays_left": 2}] + def test_save_empty_snapshot_does_not_overwrite_saved_queue(self): + self.repository.save(10, []) - repository.save(10, tracks) - self.assertEqual(repository.load(10), tracks) + self.database.connect.assert_not_called() - # Próba zapisania pustej listy nie powinna nadpisać poprzedniej - repository.save(10, []) - self.assertEqual(repository.load(10), tracks) + def test_save_upserts_snapshot_for_guild(self): + tracks = [{"id": "track-1", "title": "Track", "plays_left": 2}] - # Zapis pustej kolejki dla nowego serwera nie powinien tworzyć pliku - repository.save(20, []) - self.assertFalse(repository.exists(20)) + self.repository.save(10, tracks) + + statements = [call.args[0] for call in self.connection.execute.call_args_list] + self.assertTrue(any("INSERT INTO queue_snapshots" in statement for statement in statements)) if __name__ == "__main__": diff --git a/tests/test_shutdown_voice_cleanup.py b/tests/test_shutdown_voice_cleanup.py index a2f5976..f6756d7 100644 --- a/tests/test_shutdown_voice_cleanup.py +++ b/tests/test_shutdown_voice_cleanup.py @@ -16,6 +16,7 @@ class ShutdownVoiceCleanupTests(unittest.IsolatedAsyncioTestCase): self.uploads = MagicMock() self.config = MagicMock() self.history = MagicMock() + self.migration = MagicMock() self.bot = MusicBot( self.settings, @@ -23,6 +24,7 @@ class ShutdownVoiceCleanupTests(unittest.IsolatedAsyncioTestCase): self.uploads, self.config, self.history, + self.migration, ) self.player = MagicMock() self.bot.player = self.player diff --git a/tests/test_uploads_repository.py b/tests/test_uploads_repository.py index d5a1f93..abf38b1 100644 --- a/tests/test_uploads_repository.py +++ b/tests/test_uploads_repository.py @@ -2,7 +2,7 @@ import tempfile import unittest from pathlib import Path -from zyrarbot.storage.uploads_repository import UploadsRepository +from zyrarbot.storage.adapters.filesystem.uploads import UploadsRepository from zyrarbot.ui.bot.cogs.playback_cog import upload_choices