From efea2ace07565c7d1838531c70498695034306ad Mon Sep 17 00:00:00 2001
From: Ismael Leon <ismaleonsaenz@gmail.com>
Date: Thu, 20 Aug 2026 18:53:34 -0600
Subject: [PATCH] Add automated test suite and CI

The project had no tests, so every change had to be verified by hand in a live
Discord server. This adds a pytest suite that runs offline in under a second.

Covered:
- Queue mutation: add/add_many caps, remove/move range validation, shuffle,
  clear, history limits, go_previous ordering, idempotent destroy.
- The _advance state machine: every combination of loop mode, skip and replay,
  plus autoplay, the idle timeout, and being woken by a late enqueue.
- services.media helpers: search-prefix routing, sparse info dicts, entry
  unwrapping, error classification, kill_stream idempotency.
- Voice-state guards, asserting they explain every refusal.
- Command edge cases: empty queue, nothing playing, out-of-range volume,
  over-length queries, effect throttling, and the bot being left alone in a
  voice channel.

The suite mocks the gateway and never calls yt-dlp, so it needs no credentials,
no network and no .env. CI runs it on Python 3.11 and 3.12 (3.12 is what
Ubuntu 24.04 ships, which is what the bot is deployed on).

Closes #10
---
 .github/workflows/tests.yml  |  32 +++++
 README.md                    |  12 ++
 pytest.ini                   |   8 ++
 requirements-dev.txt         |   5 +
 tests/conftest.py            |  77 +++++++++++
 tests/test_checks.py         |  98 ++++++++++++++
 tests/test_commands.py       | 242 +++++++++++++++++++++++++++++++++++
 tests/test_media_helpers.py  | 177 +++++++++++++++++++++++++
 tests/test_player_advance.py | 165 ++++++++++++++++++++++++
 tests/test_player_queue.py   | 145 +++++++++++++++++++++
 10 files changed, 961 insertions(+)
 create mode 100644 .github/workflows/tests.yml
 create mode 100644 pytest.ini
 create mode 100644 requirements-dev.txt
 create mode 100644 tests/conftest.py
 create mode 100644 tests/test_checks.py
 create mode 100644 tests/test_commands.py
 create mode 100644 tests/test_media_helpers.py
 create mode 100644 tests/test_player_advance.py
 create mode 100644 tests/test_player_queue.py

diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
new file mode 100644
index 0000000..b786069
--- /dev/null
+++ b/.github/workflows/tests.yml
@@ -0,0 +1,32 @@
+name: Tests
+
+on:
+  push:
+    branches: [main]
+  pull_request:
+
+jobs:
+  pytest:
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        # 3.12 is what Ubuntu 24.04 ships, which is what the bot is deployed on.
+        python-version: ["3.11", "3.12"]
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: actions/setup-python@v5
+        with:
+          python-version: ${{ matrix.python-version }}
+          cache: pip
+
+      - name: Install dependencies
+        run: |
+          python -m pip install --upgrade pip
+          pip install -r requirements-dev.txt
+
+      - name: Run tests
+        # The suite needs no credentials, no network and no .env.
+        run: pytest
diff --git a/README.md b/README.md
index bd2a488..b4b1501 100644
--- a/README.md
+++ b/README.md
@@ -96,6 +96,18 @@ python main.py
 
 `!lyrics` is optional — set `GENIUS_TOKEN` in `.env` to enable it.
 
+### Running the tests
+```bash
+.venv/bin/pip install -r requirements-dev.txt   # Windows: .venv\Scripts\pip
+pytest
+```
+
+The suite needs **no credentials, no network and no `.env`** — it mocks the
+Discord gateway and never calls yt-dlp. It covers queue and player state, the
+`_advance` state machine (loop modes, skip, replay, autoplay, idle timeout),
+the `services.media` helpers, the voice-state guards, and the command edge
+cases. CI runs it on every push and pull request against Python 3.11 and 3.12.
+
 ### Deploy to AWS (t4g.micro, ~$6/mo or free tier)
 See **[deploy/README.md](deploy/README.md)** for a full walkthrough: launch script,
 provisioning (`deploy/setup.sh`) and a `systemd` service that auto-restarts and
diff --git a/pytest.ini b/pytest.ini
new file mode 100644
index 0000000..1cf5cb1
--- /dev/null
+++ b/pytest.ini
@@ -0,0 +1,8 @@
+[pytest]
+testpaths = tests
+asyncio_mode = auto
+# Every async fixture lives for one test only — players hold event-loop state.
+asyncio_default_fixture_loop_scope = function
+filterwarnings =
+    error::RuntimeWarning
+addopts = -q --strict-markers
diff --git a/requirements-dev.txt b/requirements-dev.txt
new file mode 100644
index 0000000..f708f29
--- /dev/null
+++ b/requirements-dev.txt
@@ -0,0 +1,5 @@
+# Test-only dependencies. Install with:
+#   pip install -r requirements.txt -r requirements-dev.txt
+-r requirements.txt
+pytest>=8.0
+pytest-asyncio>=0.24
diff --git a/tests/conftest.py b/tests/conftest.py
new file mode 100644
index 0000000..6837274
--- /dev/null
+++ b/tests/conftest.py
@@ -0,0 +1,77 @@
+"""
+Shared fixtures.
+
+The suite never opens a Discord gateway connection and never touches the
+network. Everything here builds just enough of a fake bot/guild for the real
+:class:`~utils.player.MusicPlayer` logic to run in-process.
+"""
+
+import sys
+from pathlib import Path
+from unittest.mock import MagicMock
+
+import pytest
+
+# Make the project importable without installing it.
+sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
+
+from utils.player import MusicPlayer, players  # noqa: E402
+
+
+def make_track(title: str = "Track", **overrides) -> dict:
+    """A track dict shaped like the one ``services.media._build_track`` returns."""
+    track = {
+        "title": title,
+        "url": f"https://example.invalid/{title.replace(' ', '_')}",
+        "stream": None,
+        "duration": 180,
+        "thumbnail": None,
+        "uploader": "Uploader",
+        "source": "test",
+        "query": "",
+    }
+    track.update(overrides)
+    return track
+
+
+@pytest.fixture
+def track_factory():
+    return make_track
+
+
+@pytest.fixture
+def fake_bot():
+    """
+    A stand-in for the bot.
+
+    ``create_task`` deliberately closes the coroutine instead of scheduling it:
+    unit tests drive ``_advance`` and the queue directly, and letting the real
+    ``_player_loop`` run would try to touch voice state.
+    """
+    bot = MagicMock()
+
+    def _create_task(coro):
+        coro.close()
+        task = MagicMock()
+        task.done.return_value = True
+        return task
+
+    bot.loop.create_task.side_effect = _create_task
+    return bot
+
+
+@pytest.fixture
+def fake_guild():
+    guild = MagicMock()
+    guild.id = 1234567890
+    guild.voice_client = None      # not connected unless a test says otherwise
+    return guild
+
+
+@pytest.fixture
+def player(fake_bot, fake_guild):
+    """A MusicPlayer whose background loop is never started."""
+    p = MusicPlayer(fake_bot, fake_guild, MagicMock())
+    yield p
+    # Keep the module-level singleton clean between tests.
+    players.discard(fake_guild.id)
diff --git a/tests/test_checks.py b/tests/test_checks.py
new file mode 100644
index 0000000..418577b
--- /dev/null
+++ b/tests/test_checks.py
@@ -0,0 +1,98 @@
+"""
+Voice-state guards in ``utils.checks``.
+
+``commands.check`` exposes the wrapped predicate as ``.predicate``, so each
+guard can be exercised directly. Every guard must both return the right verdict
+*and* tell the user why it refused - a silent False is a bug.
+"""
+
+from unittest.mock import AsyncMock, MagicMock
+
+import pytest
+
+from utils.checks import user_in_voice, bot_in_voice, same_voice_channel
+
+
+@pytest.fixture
+def ctx():
+    c = MagicMock()
+    c.send = AsyncMock()
+    c.author.voice = None
+    c.voice_client = None
+    return c
+
+
+def in_channel(channel):
+    """A voice state for someone sitting in ``channel``."""
+    state = MagicMock()
+    state.channel = channel
+    return state
+
+
+def sent_text(ctx) -> str:
+    embed = ctx.send.await_args.kwargs.get("embed") or ctx.send.await_args.args[0]
+    return embed.description or ""
+
+
+# -- user_in_voice -----------------------------------------------------
+
+async def test_user_in_voice_passes_when_connected(ctx):
+    ctx.author.voice = in_channel(MagicMock())
+    assert await user_in_voice().predicate(ctx) is True
+    ctx.send.assert_not_awaited()
+
+
+async def test_user_in_voice_refuses_and_explains_when_not_connected(ctx):
+    assert await user_in_voice().predicate(ctx) is False
+    assert "must be in a voice channel" in sent_text(ctx)
+
+
+async def test_user_in_voice_refuses_a_stale_voice_state(ctx):
+    """A voice state with no channel means the user just left."""
+    ctx.author.voice = in_channel(None)
+    assert await user_in_voice().predicate(ctx) is False
+
+
+# -- bot_in_voice ------------------------------------------------------
+
+async def test_bot_in_voice_passes_when_the_bot_is_connected(ctx):
+    ctx.voice_client = MagicMock()
+    assert await bot_in_voice().predicate(ctx) is True
+
+
+async def test_bot_in_voice_refuses_and_explains_when_the_bot_is_not(ctx):
+    assert await bot_in_voice().predicate(ctx) is False
+    assert "not connected" in sent_text(ctx)
+
+
+# -- same_voice_channel ------------------------------------------------
+
+async def test_same_channel_passes(ctx):
+    channel = MagicMock()
+    ctx.author.voice = in_channel(channel)
+    ctx.voice_client = MagicMock(channel=channel)
+    assert await same_voice_channel().predicate(ctx) is True
+    ctx.send.assert_not_awaited()
+
+
+async def test_different_channel_is_refused(ctx):
+    ctx.author.voice = in_channel(MagicMock())
+    ctx.voice_client = MagicMock(channel=MagicMock())
+    assert await same_voice_channel().predicate(ctx) is False
+    assert "same voice channel" in sent_text(ctx)
+
+
+async def test_same_channel_requires_the_user_to_be_in_voice(ctx):
+    ctx.voice_client = MagicMock(channel=MagicMock())
+    assert await same_voice_channel().predicate(ctx) is False
+    assert "must be in a voice channel" in sent_text(ctx)
+
+
+async def test_same_channel_passes_when_the_bot_is_not_connected_yet(ctx):
+    """
+    With no voice client there is no channel to mismatch, so the guard defers to
+    the command, which reports "nothing is playing" itself.
+    """
+    ctx.author.voice = in_channel(MagicMock())
+    ctx.voice_client = None
+    assert await same_voice_channel().predicate(ctx) is True
diff --git a/tests/test_commands.py b/tests/test_commands.py
new file mode 100644
index 0000000..dd136c4
--- /dev/null
+++ b/tests/test_commands.py
@@ -0,0 +1,242 @@
+"""
+Command bodies, driven through a mocked context.
+
+Commands are invoked via ``.callback(cog, ctx, ...)``, which runs the real
+command body without a gateway connection. Decorator checks (cooldowns,
+``@same_voice_channel``) are covered separately in ``test_checks.py``.
+
+The focus is the edge cases called out in the project brief: acting on an empty
+queue, acting while nothing is playing, out-of-range input, and the bot being
+disconnected from voice by someone else.
+"""
+
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+
+from cogs.music import Music, MAX_QUERY_LEN, _is_playlist_url
+from cogs.effects import Effects
+from utils.player import players
+
+
+@pytest.fixture
+def ctx(fake_guild):
+    c = MagicMock()
+    c.guild = fake_guild
+    c.send = AsyncMock()
+    c.voice_client = None
+    c.author = MagicMock()
+    c.typing = MagicMock(return_value=AsyncMock())
+    return c
+
+
+@pytest.fixture
+def music_cog():
+    return Music(MagicMock())
+
+
+@pytest.fixture
+def effects_cog():
+    return Effects(MagicMock())
+
+
+def sent_text(ctx) -> str:
+    """The text of the last embed the command sent."""
+    assert ctx.send.await_count >= 1, "the command sent nothing at all"
+    embed = ctx.send.await_args.kwargs.get("embed") or ctx.send.await_args.args[0]
+    return (embed.description or "") + (embed.title or "")
+
+
+# -- playlist URL detection -------------------------------------------
+
+@pytest.mark.parametrize("url", [
+    "https://www.youtube.com/playlist?list=PL123",
+    "https://soundcloud.com/artist/sets/my-set",
+    "https://artist.bandcamp.com/album/thing",
+])
+def test_playlist_urls_are_detected(url):
+    assert _is_playlist_url(url) is True
+
+
+@pytest.mark.parametrize("query", [
+    "https://www.youtube.com/watch?v=abc&list=PL123",   # a track inside a playlist
+    "https://www.youtube.com/watch?v=abc",
+    "bohemian rhapsody",
+    "sc: lofi",
+    "playlist",                                          # bare word, not a URL
+])
+def test_non_playlist_queries_are_not_treated_as_playlists(query):
+    assert _is_playlist_url(query) is False
+
+
+# -- acting when nothing is playing ------------------------------------
+
+async def test_skip_with_nothing_playing_reports_it(music_cog, ctx):
+    await Music.skip.callback(music_cog, ctx)
+    assert "Nothing is playing" in sent_text(ctx)
+
+
+async def test_skip_with_an_empty_queue_does_not_raise(music_cog, ctx, fake_bot, fake_guild):
+    """An empty queue must not crash the command."""
+    player = players.get_or_create(fake_bot, ctx)
+    try:
+        player.queue.clear()
+        fake_guild.voice_client = None
+        await Music.skip.callback(music_cog, ctx)
+        assert "Nothing is playing" in sent_text(ctx)
+    finally:
+        players.discard(fake_guild.id)
+
+
+async def test_pause_with_nothing_playing(music_cog, ctx):
+    await Music.pause.callback(music_cog, ctx)
+    assert "Nothing is playing" in sent_text(ctx)
+
+
+async def test_resume_with_nothing_paused(music_cog, ctx):
+    await Music.resume.callback(music_cog, ctx)
+    assert "Nothing is paused" in sent_text(ctx)
+
+
+async def test_previous_with_no_history(music_cog, ctx):
+    await Music.previous.callback(music_cog, ctx)
+    assert "No previous track" in sent_text(ctx)
+
+
+async def test_nowplaying_with_no_player(music_cog, ctx):
+    await Music.nowplaying.callback(music_cog, ctx)
+    assert "Nothing is playing" in sent_text(ctx)
+
+
+async def test_queue_with_no_player(music_cog, ctx):
+    await Music.queue.callback(music_cog, ctx)
+    assert "Nothing is playing" in sent_text(ctx)
+
+
+async def test_stop_with_no_player_and_no_voice_still_confirms(music_cog, ctx):
+    await Music.stop.callback(music_cog, ctx)
+    assert "Disconnected" in sent_text(ctx)
+
+
+# -- out-of-range and oversized input ----------------------------------
+
+@pytest.mark.parametrize("vol", [-1, 101, 1000])
+async def test_volume_out_of_range_is_rejected(music_cog, ctx, vol):
+    await Music.volume.callback(music_cog, ctx, vol)
+    assert "between 0 and 100" in sent_text(ctx)
+
+
+@pytest.mark.parametrize("vol", [0, 50, 100])
+async def test_volume_boundaries_are_accepted(music_cog, ctx, vol):
+    """0 and 100 are valid - an off-by-one here would reject mute and max."""
+    await Music.volume.callback(music_cog, ctx, vol)
+    assert "between 0 and 100" not in sent_text(ctx)
+
+
+async def test_loop_rejects_an_unknown_mode(music_cog, ctx):
+    await Music.loop.callback(music_cog, ctx, "banana")
+    assert "must be" in sent_text(ctx)
+
+
+async def test_play_rejects_an_over_length_query(music_cog, ctx):
+    await Music.play.callback(music_cog, ctx, query="x" * (MAX_QUERY_LEN + 1))
+    assert "too long" in sent_text(ctx)
+    ctx.typing.assert_not_called()      # rejected before any yt-dlp work
+
+
+async def test_shuffle_on_an_empty_queue(music_cog, ctx):
+    await Music.shuffle.callback(music_cog, ctx)
+    assert "empty" in sent_text(ctx).lower()
+
+
+async def test_remove_on_an_empty_queue(music_cog, ctx):
+    await Music.remove.callback(music_cog, ctx, 1)
+    assert "No track at position" in sent_text(ctx)
+
+
+async def test_move_with_no_player(music_cog, ctx):
+    await Music.move.callback(music_cog, ctx, 1, 2)
+    assert "Invalid positions" in sent_text(ctx)
+
+
+# -- effects with no active player -------------------------------------
+
+async def test_effect_command_with_no_player(effects_cog, ctx):
+    await Effects.bassboost.callback(effects_cog, ctx)
+    assert "Nothing is playing" in sent_text(ctx)
+
+
+async def test_effect_changes_are_throttled_per_guild(effects_cog, ctx):
+    await Effects.bass.callback(effects_cog, ctx)
+    ctx.send.reset_mock()
+    await Effects.nightcore.callback(effects_cog, ctx)
+    assert "wait" in sent_text(ctx).lower(), "a second effect inside the cooldown must be refused"
+
+
+async def test_current_effect_reports_none_when_idle(effects_cog, ctx):
+    await Effects.current_effect.callback(effects_cog, ctx)
+    assert "none" in sent_text(ctx).lower()
+
+
+# -- the bot being disconnected by someone else ------------------------
+
+def _guild_with_voice(guild_id: int, members: list):
+    guild = MagicMock()
+    guild.id = guild_id
+    vc = MagicMock()
+    vc.is_connected.return_value = True
+    vc.disconnect = AsyncMock()
+    channel = MagicMock()
+    channel.members = members
+    vc.channel = channel
+    guild.voice_client = vc
+    return guild, vc, channel
+
+
+async def test_bot_left_alone_in_voice_disconnects(music_cog):
+    """Everyone leaving must not strand the bot in an empty channel."""
+    guild, vc, channel = _guild_with_voice(999, [MagicMock(bot=True)])
+    member = MagicMock(bot=False)
+    member.guild = guild
+
+    with patch("asyncio.sleep", new=AsyncMock()):
+        await Music.on_voice_state_update(
+            music_cog, member, MagicMock(channel=channel), MagicMock(channel=None)
+        )
+
+    vc.disconnect.assert_awaited_once()
+
+
+async def test_bot_stays_when_a_human_is_still_in_the_channel(music_cog):
+    guild, vc, channel = _guild_with_voice(
+        998, [MagicMock(bot=True), MagicMock(bot=False)]
+    )
+    member = MagicMock(bot=False)
+    member.guild = guild
+
+    with patch("asyncio.sleep", new=AsyncMock()):
+        await Music.on_voice_state_update(
+            music_cog, member, MagicMock(channel=channel), MagicMock(channel=None)
+        )
+
+    vc.disconnect.assert_not_awaited()
+
+
+async def test_another_bot_leaving_is_ignored(music_cog):
+    member = MagicMock(bot=True)
+    member.guild = MagicMock()
+    # Must return before touching voice state at all.
+    await Music.on_voice_state_update(music_cog, member, MagicMock(), MagicMock())
+
+
+async def test_voice_update_in_a_different_channel_is_ignored(music_cog):
+    guild, vc, _channel = _guild_with_voice(997, [MagicMock(bot=True)])
+    member = MagicMock(bot=False)
+    member.guild = guild
+
+    other_channel = MagicMock()
+    with patch("asyncio.sleep", new=AsyncMock()):
+        await Music.on_voice_state_update(
+            music_cog, member, MagicMock(channel=other_channel), MagicMock(channel=None)
+        )
+    vc.disconnect.assert_not_awaited()
diff --git a/tests/test_media_helpers.py b/tests/test_media_helpers.py
new file mode 100644
index 0000000..6aec248
--- /dev/null
+++ b/tests/test_media_helpers.py
@@ -0,0 +1,177 @@
+"""
+Pure helpers in ``services.media``.
+
+Nothing here touches the network or spawns a real yt-dlp: these cover the
+parsing and process-handling code that decides what gets searched and how a
+failure is reported back to the user.
+"""
+
+import io
+import subprocess
+from unittest.mock import MagicMock
+
+import pytest
+
+from services import media
+
+
+# ── query → yt-dlp target ─────────────────────────────────────────────
+
+@pytest.mark.parametrize("query,expected", [
+    ("sc: lofi",            "scsearch1:lofi"),
+    ("SC: Lofi",            "scsearch1:Lofi"),
+    ("soundcloud: lofi",    "scsearch1:lofi"),
+    ("yt: never gonna",     "ytsearch1:never gonna"),
+    ("youtube: never",      "ytsearch1:never"),
+])
+def test_search_prefixes_pick_the_right_engine(query, expected):
+    target, flat_ok = media._search_target(query)
+    assert target == expected
+    assert flat_ok is True
+
+
+def test_bare_terms_default_to_a_youtube_search():
+    target, flat_ok = media._search_target("bohemian rhapsody")
+    assert target == "ytsearch1:bohemian rhapsody"
+    assert flat_ok is True
+
+
+@pytest.mark.parametrize("url", [
+    "https://www.youtube.com/watch?v=abc123",
+    "https://soundcloud.com/artist/track",
+    "http://example.invalid/audio.mp3",
+])
+def test_urls_are_passed_through_untouched(url):
+    target, flat_ok = media._search_target(url)
+    assert target == url
+    assert flat_ok is False, "a URL must be extracted directly, not flat-listed"
+
+
+def test_a_prefix_wins_over_url_detection():
+    """`sc: https://...` should search SoundCloud, not extract the URL."""
+    target, _ = media._search_target("sc: https://example.invalid/x")
+    assert target == "scsearch1:https://example.invalid/x"
+
+
+# ── info dict → track dict ────────────────────────────────────────────
+
+def test_build_track_maps_the_common_fields():
+    track = media._build_track({
+        "title": "Song",
+        "webpage_url": "https://example.invalid/s",
+        "duration": 210,
+        "thumbnail": "https://example.invalid/t.jpg",
+        "uploader": "Artist",
+        "extractor_key": "Youtube",
+    }, query="song")
+    assert track["title"] == "Song"
+    assert track["url"] == "https://example.invalid/s"
+    assert track["duration"] == 210
+    assert track["source"] == "youtube"
+    assert track["query"] == "song"
+
+
+def test_build_track_survives_an_almost_empty_info_dict():
+    """Flat playlist entries are sparse — this must never raise."""
+    track = media._build_track({})
+    assert track["title"] == "Unknown Title"
+    assert track["url"] is None
+    assert track["duration"] is None
+    assert track["thumbnail"] is None
+    assert track["uploader"] is None
+
+
+def test_build_track_falls_back_to_url_and_channel():
+    track = media._build_track({"url": "https://example.invalid/x", "channel": "Chan"})
+    assert track["url"] == "https://example.invalid/x"
+    assert track["uploader"] == "Chan"
+
+
+def test_build_track_picks_the_largest_thumbnail_when_none_is_flagged():
+    track = media._build_track({"thumbnails": [
+        {"url": "small.jpg"}, {"url": "large.jpg"},
+    ]})
+    assert track["thumbnail"] == "large.jpg"
+
+
+# ── entry unwrapping ──────────────────────────────────────────────────
+
+def test_first_entry_unwraps_a_search_result():
+    assert media._first_entry({"entries": [{"title": "A"}, {"title": "B"}]})["title"] == "A"
+
+
+def test_first_entry_skips_leading_nulls():
+    """yt-dlp puts None in `entries` for unavailable videos."""
+    assert media._first_entry({"entries": [None, None, {"title": "C"}]})["title"] == "C"
+
+
+@pytest.mark.parametrize("info", [None, {"entries": []}, {"entries": [None]}])
+def test_first_entry_returns_none_when_there_is_nothing_playable(info):
+    assert media._first_entry(info) is None
+
+
+def test_first_entry_passes_through_a_single_result():
+    assert media._first_entry({"title": "Solo"})["title"] == "Solo"
+
+
+# ── failure classification ────────────────────────────────────────────
+
+def _finished_proc(stderr: bytes) -> MagicMock:
+    proc = MagicMock(spec=subprocess.Popen)
+    proc.stderr = io.BytesIO(stderr)
+    return proc
+
+
+@pytest.mark.parametrize("stderr", [
+    b"ERROR: Sign in to confirm you're not a bot",
+    b"ERROR: SIGN IN TO CONFIRM you are not a bot",
+    b"please confirm you're not a bot to continue",
+])
+def test_bot_checks_are_classified_as_blocked(stderr):
+    assert media.classify_stream_error(_finished_proc(stderr)) == "blocked"
+
+
+@pytest.mark.parametrize("stderr", [
+    b"ERROR: Video unavailable",
+    b"ERROR: Private video",
+    b"",
+])
+def test_other_failures_are_classified_as_unavailable(stderr):
+    assert media.classify_stream_error(_finished_proc(stderr)) == "unavailable"
+
+
+def test_classify_handles_a_missing_process():
+    assert media.classify_stream_error(None) == "unavailable"
+
+
+def test_classify_survives_undecodable_stderr():
+    """yt-dlp can emit non-UTF-8 bytes; classification must not raise."""
+    assert media.classify_stream_error(_finished_proc(b"\xff\xfe invalid")) == "unavailable"
+
+
+# ── process teardown ──────────────────────────────────────────────────
+
+def test_kill_stream_accepts_none():
+    media.kill_stream(None)        # must not raise
+
+
+def test_kill_stream_leaves_an_already_finished_process_alone():
+    proc = MagicMock(spec=subprocess.Popen)
+    proc.poll.return_value = 0
+    media.kill_stream(proc)
+    proc.kill.assert_not_called()
+
+
+def test_kill_stream_kills_a_running_process():
+    proc = MagicMock(spec=subprocess.Popen)
+    proc.poll.return_value = None
+    media.kill_stream(proc)
+    proc.kill.assert_called_once()
+
+
+def test_kill_stream_is_idempotent():
+    proc = MagicMock(spec=subprocess.Popen)
+    proc.poll.side_effect = [None, 0]     # running, then dead
+    media.kill_stream(proc)
+    media.kill_stream(proc)
+    assert proc.kill.call_count == 1
diff --git a/tests/test_player_advance.py b/tests/test_player_advance.py
new file mode 100644
index 0000000..640b188
--- /dev/null
+++ b/tests/test_player_advance.py
@@ -0,0 +1,165 @@
+"""
+``MusicPlayer._advance`` — the state machine that decides what plays next.
+
+Every combination of loop mode, skip and replay is covered here, because this
+is the one place where a wrong branch produces the classic music-bot bugs:
+double-skips, tracks repeating forever, or history silently losing entries.
+
+``_advance`` returns ``(track, silent)``; ``silent`` suppresses the "Now
+Playing" announcement, and ``(None, _)`` means "disconnect".
+"""
+
+import asyncio
+from unittest.mock import AsyncMock, patch
+
+import pytest
+
+from tests.conftest import make_track
+
+
+async def test_first_track_comes_off_the_queue_and_is_announced(player):
+    player.add(make_track("A"))
+    track, silent = await player._advance()
+    assert track["title"] == "A"
+    assert silent is False
+    assert player.current["title"] == "A"
+
+
+async def test_normal_advance_archives_the_previous_track(player):
+    player.current = make_track("A")
+    player.add(make_track("B"))
+    track, silent = await player._advance()
+    assert track["title"] == "B"
+    assert silent is False
+    assert [t["title"] for t in player.history] == ["A"]
+
+
+# ── loop modes ────────────────────────────────────────────────────────
+
+async def test_loop_track_repeats_silently(player):
+    player.current = make_track("A")
+    player.loop_mode = "track"
+    player.add(make_track("B"))          # must NOT be consumed
+    track, silent = await player._advance()
+    assert track["title"] == "A"
+    assert silent is True, "a loop repeat should not re-announce the track"
+    assert [t["title"] for t in player.to_list()] == ["B"]
+
+
+async def test_loop_queue_sends_the_finished_track_to_the_back(player):
+    player.current = make_track("A")
+    player.loop_mode = "queue"
+    player.add(make_track("B"))
+    track, _ = await player._advance()
+    assert track["title"] == "B"
+    assert [t["title"] for t in player.to_list()] == ["A"]
+
+
+async def test_loop_off_drops_the_finished_track(player):
+    player.current = make_track("A")
+    player.loop_mode = "off"
+    player.add(make_track("B"))
+    track, _ = await player._advance()
+    assert track["title"] == "B"
+    assert player.is_empty
+
+
+# ── skip beats loop mode ──────────────────────────────────────────────
+
+async def test_skip_overrides_loop_track(player):
+    """A user pressing skip must escape a single-track loop."""
+    player.current = make_track("A")
+    player.loop_mode = "track"
+    player._skip = True
+    player.add(make_track("B"))
+    track, silent = await player._advance()
+    assert track["title"] == "B"
+    assert silent is False
+    assert player._skip is False, "the skip flag must be consumed exactly once"
+
+
+async def test_skip_overrides_loop_queue(player):
+    player.current = make_track("A")
+    player.loop_mode = "queue"
+    player._skip = True
+    player.add(make_track("B"))
+    track, _ = await player._advance()
+    assert track["title"] == "B"
+    assert player.is_empty, "a skipped track must not be requeued"
+
+
+async def test_skip_does_not_archive_the_skipped_track(player):
+    player.current = make_track("A")
+    player._skip = True
+    player.add(make_track("B"))
+    await player._advance()
+    assert player.history == []
+
+
+# ── replay (effect change) ────────────────────────────────────────────
+
+async def test_replay_returns_the_same_track_silently(player):
+    player.current = make_track("A")
+    player._replay = True
+    player.add(make_track("B"))
+    track, silent = await player._advance()
+    assert track["title"] == "A"
+    assert silent is True
+    assert [t["title"] for t in player.to_list()] == ["B"], "replay must not consume the queue"
+    assert player.history == [], "replay must not archive anything"
+
+
+# ── autoplay ──────────────────────────────────────────────────────────
+
+async def test_autoplay_fills_an_empty_queue(player):
+    player.current = make_track("A")
+    player.autoplay = True
+    suggestion = make_track("Related")
+    with patch("services.media.related", new=AsyncMock(return_value=suggestion)):
+        track, silent = await player._advance()
+    assert track["title"] == "Related"
+    assert silent is False
+
+
+async def test_autoplay_that_finds_nothing_falls_through_to_waiting(player):
+    player.current = make_track("A")
+    player.autoplay = True
+    with patch("services.media.related", new=AsyncMock(return_value=None)), \
+         patch("utils.player.INACTIVITY_TIMEOUT", 0.01):
+        track, _ = await player._advance()
+    assert track is None, "no suggestion and no queue means disconnect"
+
+
+async def test_autoplay_is_not_consulted_when_the_queue_has_tracks(player):
+    player.current = make_track("A")
+    player.autoplay = True
+    player.add(make_track("B"))
+    related = AsyncMock(return_value=make_track("Related"))
+    with patch("services.media.related", new=related):
+        track, _ = await player._advance()
+    assert track["title"] == "B"
+    related.assert_not_awaited()
+
+
+# ── idle timeout ──────────────────────────────────────────────────────
+
+async def test_empty_queue_times_out_and_signals_disconnect(player):
+    with patch("utils.player.INACTIVITY_TIMEOUT", 0.01):
+        track, _ = await player._advance()
+    assert track is None
+    assert player.current is None
+
+
+async def test_a_track_added_while_idle_wakes_the_player(player):
+    """The 5-minute idle wait must be interrupted the moment something is queued."""
+    async def enqueue_shortly():
+        await asyncio.sleep(0.01)
+        player.add(make_track("Late"))
+
+    with patch("utils.player.INACTIVITY_TIMEOUT", 5):
+        advance = asyncio.create_task(player._advance())
+        await enqueue_shortly()
+        track, silent = await asyncio.wait_for(advance, timeout=2)
+
+    assert track["title"] == "Late"
+    assert silent is False
diff --git a/tests/test_player_queue.py b/tests/test_player_queue.py
new file mode 100644
index 0000000..e0af431
--- /dev/null
+++ b/tests/test_player_queue.py
@@ -0,0 +1,145 @@
+"""Queue mutation logic — pure state, no event loop required."""
+
+import pytest
+
+from utils.player import MAX_QUEUE, HISTORY_LIMIT
+from tests.conftest import make_track
+
+
+# ── add / add_many ────────────────────────────────────────────────────
+
+def test_add_appends_and_signals(player):
+    assert player.add(make_track("A")) is True
+    assert [t["title"] for t in player.to_list()] == ["A"]
+    assert player._added.is_set()
+
+
+def test_add_refuses_past_the_hard_cap(player):
+    player.queue.extend(make_track(f"T{i}") for i in range(MAX_QUEUE))
+    assert player.add(make_track("overflow")) is False
+    assert len(player.queue) == MAX_QUEUE
+
+
+def test_add_many_reports_how_many_fit(player):
+    player.queue.extend(make_track(f"T{i}") for i in range(MAX_QUEUE - 3))
+    accepted = player.add_many([make_track(f"N{i}") for i in range(10)])
+    assert accepted == 3
+    assert len(player.queue) == MAX_QUEUE
+
+
+def test_add_many_on_full_queue_accepts_nothing_and_stays_quiet(player):
+    player.queue.extend(make_track(f"T{i}") for i in range(MAX_QUEUE))
+    player._added.clear()
+    assert player.add_many([make_track("N")]) == 0
+    assert player._added.is_set() is False
+
+
+def test_add_many_empty_list_does_not_signal(player):
+    player._added.clear()
+    assert player.add_many([]) == 0
+    assert player._added.is_set() is False
+
+
+# ── remove ────────────────────────────────────────────────────────────
+
+def test_remove_uses_one_based_positions(player):
+    player.add_many([make_track("A"), make_track("B"), make_track("C")])
+    removed = player.remove(2)
+    assert removed["title"] == "B"
+    assert [t["title"] for t in player.to_list()] == ["A", "C"]
+
+
+@pytest.mark.parametrize("bad_index", [0, -1, 4, 999])
+def test_remove_rejects_out_of_range_and_leaves_queue_intact(player, bad_index):
+    player.add_many([make_track("A"), make_track("B"), make_track("C")])
+    assert player.remove(bad_index) is None
+    assert [t["title"] for t in player.to_list()] == ["A", "B", "C"]
+
+
+def test_remove_from_empty_queue(player):
+    assert player.remove(1) is None
+
+
+# ── move ──────────────────────────────────────────────────────────────
+
+def test_move_reorders(player):
+    player.add_many([make_track("A"), make_track("B"), make_track("C")])
+    assert player.move(3, 1) is True
+    assert [t["title"] for t in player.to_list()] == ["C", "A", "B"]
+
+
+def test_move_onto_itself_is_a_no_op(player):
+    player.add_many([make_track("A"), make_track("B")])
+    assert player.move(1, 1) is True
+    assert [t["title"] for t in player.to_list()] == ["A", "B"]
+
+
+@pytest.mark.parametrize("frm,to", [(0, 1), (1, 0), (5, 1), (1, 5), (-1, 1)])
+def test_move_rejects_out_of_range(player, frm, to):
+    player.add_many([make_track("A"), make_track("B"), make_track("C")])
+    assert player.move(frm, to) is False
+    assert [t["title"] for t in player.to_list()] == ["A", "B", "C"]
+
+
+# ── shuffle / clear ───────────────────────────────────────────────────
+
+def test_shuffle_preserves_every_track(player):
+    titles = [f"T{i}" for i in range(50)]
+    player.add_many([make_track(t) for t in titles])
+    player.shuffle()
+    assert sorted(t["title"] for t in player.to_list()) == sorted(titles)
+
+
+def test_shuffle_on_empty_queue_is_safe(player):
+    player.shuffle()
+    assert player.is_empty
+
+
+def test_clear_empties_the_queue_but_keeps_current(player, track_factory):
+    player.current = track_factory("Playing")
+    player.add_many([make_track("A"), make_track("B")])
+    player.clear()
+    assert player.is_empty
+    assert player.current["title"] == "Playing"
+
+
+# ── history ───────────────────────────────────────────────────────────
+
+def test_history_is_capped(player):
+    player.history = [make_track(f"H{i}") for i in range(HISTORY_LIMIT)]
+    player.current = make_track("current")
+    player.queue.append(make_track("next"))
+    # Trip the archiving path once via the same logic _advance uses.
+    player.history.append(player.current)
+    if len(player.history) > HISTORY_LIMIT:
+        player.history.pop(0)
+    assert len(player.history) == HISTORY_LIMIT
+
+
+def test_go_previous_with_empty_history_returns_false(player):
+    assert player.history == []
+    assert player.go_previous() is False
+
+
+def test_go_previous_requeues_previous_then_current(player):
+    player.history = [make_track("Older"), make_track("Prev")]
+    player.current = make_track("Current")
+    assert player.go_previous() is True
+    # Previous plays first, then the track that was interrupted.
+    assert [t["title"] for t in player.to_list()] == ["Prev", "Current"]
+    assert player.current is None      # so _advance won't re-archive it
+    assert player._skip is True
+    assert [t["title"] for t in player.history] == ["Older"]
+
+
+# ── teardown ──────────────────────────────────────────────────────────
+
+def test_destroy_is_idempotent_and_clears_state(player):
+    player.add_many([make_track("A"), make_track("B")])
+    player.current = make_track("Playing")
+    player.destroy()
+    assert player._destroyed is True
+    assert player.is_empty
+    assert player.current is None
+    player.destroy()               # must not raise
+    assert player._destroyed is True