432 lines
15 KiB
Python
432 lines
15 KiB
Python
"""Three families of failure, and the three different answers they get.
|
|
|
|
ARCH-008 / QUA-004. Every handler in `discord_bot.py` used to wrap its
|
|
whole body in one `except Exception` that logged a line and returned, so
|
|
these three all looked the same from the outside and two of them looked the
|
|
same from the inside:
|
|
|
|
- the database refused the write — nothing happened, and the person who
|
|
clicked has to be told, because a silent reaction is indistinguishable
|
|
from a bot that is not running;
|
|
- Discord could not be reached — after a commit, the thing is recorded
|
|
whether or not the message got through, and must stay recorded;
|
|
- something else went wrong — a defect, which has to reach a traceback
|
|
rather than be flattened into "Error handling attendance confirmation".
|
|
|
|
The coroutines are driven with `asyncio.run` rather than pytest-asyncio,
|
|
which the project does not depend on — same convention as
|
|
`test_discord_delivery.py`.
|
|
"""
|
|
|
|
import asyncio
|
|
import logging
|
|
from datetime import date, time
|
|
|
|
import pytest
|
|
from discord import HTTPException
|
|
from sqlalchemy.exc import OperationalError
|
|
|
|
from app import discord_bot
|
|
from app.discord_bot import TeamTryoutsBot
|
|
|
|
|
|
class FakeChannel:
|
|
"""The DM channel a reaction came from. Records what it was told to say."""
|
|
|
|
def __init__(self, raises=None):
|
|
self.raises = raises
|
|
self.sent = []
|
|
|
|
async def send(self, message):
|
|
if self.raises is not None:
|
|
raise self.raises
|
|
self.sent.append(message)
|
|
return object()
|
|
|
|
|
|
class FakeReactor:
|
|
"""The Discord account that clicked. Only its snowflake is ever read."""
|
|
|
|
def __init__(self, uid):
|
|
self.id = uid
|
|
|
|
|
|
def http_error():
|
|
class _Response:
|
|
status = 500
|
|
reason = 'test'
|
|
|
|
return HTTPException(_Response(), 'Internal Server Error')
|
|
|
|
|
|
def database_error():
|
|
"""A refusal that is not an IntegrityError.
|
|
|
|
Deliberately: the handlers catch `SQLAlchemyError`, and picking the
|
|
subclass a constraint violation produces would let a narrower catch
|
|
pass this suite while still dropping a lost connection on the floor.
|
|
"""
|
|
return OperationalError('SELECT 1', {}, Exception('server closed the connection'))
|
|
|
|
|
|
@pytest.fixture
|
|
def logs(caplog):
|
|
"""Let the bot's records reach caplog.
|
|
|
|
`configure_logging` sets propagate = False on the 'app' logger, and
|
|
caplog's handler sits on the root. Without this the assertions below
|
|
read an empty log and pass for the wrong reason — the trap recorded in
|
|
the wave G notes.
|
|
"""
|
|
package_logger = logging.getLogger('app')
|
|
previous = package_logger.propagate
|
|
package_logger.propagate = True
|
|
caplog.set_level(logging.INFO, logger='app.discord_bot')
|
|
yield caplog
|
|
package_logger.propagate = previous
|
|
|
|
|
|
@pytest.fixture
|
|
def bot():
|
|
"""A bot with the state the reaction handlers touch, and nothing else.
|
|
|
|
`__new__` because the constructor builds a real discord.py client.
|
|
`_save_pending` is replaced: the real one writes `discord_pending.json`
|
|
at the repository root.
|
|
"""
|
|
instance = TeamTryoutsBot.__new__(TeamTryoutsBot)
|
|
instance.flask_app = None
|
|
instance.pending_requests = {}
|
|
instance._user_cache = {}
|
|
instance.reminders_scheduled = False
|
|
instance.saves = 0
|
|
|
|
def _save():
|
|
instance.saves += 1
|
|
|
|
instance._save_pending = _save
|
|
return instance
|
|
|
|
|
|
@pytest.fixture
|
|
def participation(db, make_user):
|
|
"""A player on a match, plus the pending message asking them to confirm.
|
|
|
|
Returns (participation row id, the player's Discord snowflake).
|
|
"""
|
|
from app.models import Match, MatchParticipant, Tryout
|
|
|
|
snowflake = '4242'
|
|
player_id = make_user('player', discord_user_id=snowflake)
|
|
admin_id = make_user('admin')
|
|
|
|
tryout = Tryout(title='Sélection', game='valorant', date=date(2026, 9, 1), created_by=admin_id)
|
|
db.session.add(tryout)
|
|
db.session.flush()
|
|
|
|
match = Match(
|
|
title='Finale',
|
|
date=date(2026, 9, 2),
|
|
start_time=time(18, 0),
|
|
created_by=admin_id,
|
|
tryout_id=tryout.id,
|
|
match_type='intra',
|
|
)
|
|
db.session.add(match)
|
|
db.session.flush()
|
|
|
|
row = MatchParticipant(match_id=match.id, player_id=player_id)
|
|
db.session.add(row)
|
|
db.session.commit()
|
|
return row.id, snowflake
|
|
|
|
|
|
def _pending(bot, message_id, row_id):
|
|
bot.pending_requests[message_id] = {
|
|
'type': 'schedule_addition',
|
|
'id': row_id,
|
|
'event_type': 'match',
|
|
}
|
|
|
|
|
|
def _confirmed(row_id):
|
|
from app.extensions import db
|
|
from app.models import MatchParticipant
|
|
|
|
return db.session.get(MatchParticipant, row_id).attendance_confirmed
|
|
|
|
|
|
class TestTheDatabaseRefusedTheWrite:
|
|
def test_the_happy_path_records_the_confirmation(self, bot, db, participation, logs):
|
|
"""The premise of every test below. Without it they could all pass
|
|
against a handler that never records anything at all."""
|
|
row_id, snowflake = participation
|
|
_pending(bot, 7, row_id)
|
|
channel = FakeChannel()
|
|
|
|
asyncio.run(bot.handle_attendance_confirm(FakeReactor(snowflake), 7, row_id, channel))
|
|
|
|
assert _confirmed(row_id) is True
|
|
assert channel.sent == ["✅ Your attendance has been confirmed!"]
|
|
assert 7 not in bot.pending_requests
|
|
|
|
def test_a_refused_commit_tells_the_player_instead_of_going_quiet(
|
|
self, bot, db, participation, monkeypatch, logs
|
|
):
|
|
"""The defect ARCH-008 was written about.
|
|
|
|
The success message sat inside the same blanket handler as the
|
|
commit, so a refused write produced no message at all — the reaction
|
|
looked ignored, which is what a stopped bot looks like too.
|
|
"""
|
|
row_id, snowflake = participation
|
|
_pending(bot, 7, row_id)
|
|
channel = FakeChannel()
|
|
|
|
def refuse():
|
|
raise database_error()
|
|
|
|
monkeypatch.setattr(db.session, 'commit', refuse)
|
|
|
|
asyncio.run(bot.handle_attendance_confirm(FakeReactor(snowflake), 7, row_id, channel))
|
|
|
|
assert channel.sent, 'silence is the one answer this must never give'
|
|
assert 'Nothing was recorded' in channel.sent[0]
|
|
# Every message, not just the first: an earlier version of this test
|
|
# looked at sent[0] alone, and a handler that reported the refusal
|
|
# and then sent "✅ Your attendance has been confirmed!" anyway
|
|
# passed it.
|
|
assert not any("✅" in message for message in channel.sent), (
|
|
'a failed write must not be followed by a success message'
|
|
)
|
|
|
|
def test_a_refused_commit_leaves_the_row_untouched(
|
|
self, bot, db, participation, monkeypatch, logs
|
|
):
|
|
row_id, snowflake = participation
|
|
_pending(bot, 7, row_id)
|
|
|
|
def refuse():
|
|
raise database_error()
|
|
|
|
monkeypatch.setattr(db.session, 'commit', refuse)
|
|
asyncio.run(bot.handle_attendance_confirm(FakeReactor(snowflake), 7, row_id, FakeChannel()))
|
|
|
|
monkeypatch.undo()
|
|
assert _confirmed(row_id) is False, (
|
|
'the handler must roll back; leaving the change pending in the '
|
|
'session hands it to whatever commits next'
|
|
)
|
|
|
|
def test_a_refused_commit_keeps_the_reaction_answerable(
|
|
self, bot, db, participation, monkeypatch, logs
|
|
):
|
|
"""Dropping the pending entry on a failed write would make the
|
|
second attempt do nothing, silently, forever."""
|
|
row_id, snowflake = participation
|
|
_pending(bot, 7, row_id)
|
|
|
|
def refuse():
|
|
raise database_error()
|
|
|
|
monkeypatch.setattr(db.session, 'commit', refuse)
|
|
asyncio.run(bot.handle_attendance_confirm(FakeReactor(snowflake), 7, row_id, FakeChannel()))
|
|
|
|
assert 7 in bot.pending_requests
|
|
|
|
def test_the_refusal_reaches_the_log_with_its_traceback(
|
|
self, bot, db, participation, monkeypatch, logs
|
|
):
|
|
row_id, snowflake = participation
|
|
_pending(bot, 7, row_id)
|
|
|
|
def refuse():
|
|
raise database_error()
|
|
|
|
monkeypatch.setattr(db.session, 'commit', refuse)
|
|
with logs.at_level(logging.ERROR, logger='app.discord_bot'):
|
|
asyncio.run(
|
|
bot.handle_attendance_confirm(FakeReactor(snowflake), 7, row_id, FakeChannel())
|
|
)
|
|
|
|
assert any(record.exc_info for record in logs.records), (
|
|
'a database refusal is diagnosed from the driver error, which only '
|
|
'the traceback carries'
|
|
)
|
|
|
|
def test_a_declined_match_that_cannot_be_deleted_says_so(
|
|
self, bot, db, participation, monkeypatch, logs
|
|
):
|
|
"""The decline path deletes a row, which is exactly what a foreign
|
|
key refuses. Same answer as the confirm path."""
|
|
row_id, snowflake = participation
|
|
_pending(bot, 7, row_id)
|
|
channel = FakeChannel()
|
|
|
|
def refuse():
|
|
raise database_error()
|
|
|
|
monkeypatch.setattr(db.session, 'commit', refuse)
|
|
asyncio.run(bot.handle_attendance_decline(FakeReactor(snowflake), 7, row_id, channel))
|
|
|
|
assert channel.sent and 'Nothing was recorded' in channel.sent[0]
|
|
assert 7 in bot.pending_requests
|
|
|
|
|
|
class TestDiscordCouldNotBeReached:
|
|
def test_a_bounced_message_does_not_undo_the_record(self, bot, db, participation, logs):
|
|
"""The confirmation is committed before the message goes out. A DM
|
|
that bounces costs the player a message, not their attendance."""
|
|
row_id, snowflake = participation
|
|
_pending(bot, 7, row_id)
|
|
|
|
asyncio.run(
|
|
bot.handle_attendance_confirm(
|
|
FakeReactor(snowflake), 7, row_id, FakeChannel(raises=http_error())
|
|
)
|
|
)
|
|
|
|
assert _confirmed(row_id) is True
|
|
assert 7 not in bot.pending_requests, (
|
|
'the answer is durable, so the message must not stay answerable'
|
|
)
|
|
|
|
def test_a_bounced_message_is_reported(self, bot, db, participation, logs):
|
|
row_id, snowflake = participation
|
|
_pending(bot, 7, row_id)
|
|
|
|
with logs.at_level(logging.ERROR, logger='app.discord_bot'):
|
|
asyncio.run(
|
|
bot.handle_attendance_confirm(
|
|
FakeReactor(snowflake), 7, row_id, FakeChannel(raises=http_error())
|
|
)
|
|
)
|
|
|
|
assert 'Could not answer a reaction' in logs.text
|
|
|
|
|
|
class TestAnythingElseIsADefect:
|
|
def test_an_unexpected_failure_propagates(self, bot, db, participation, monkeypatch, logs):
|
|
"""It used to be caught here and logged as though it were a delivery
|
|
problem. Letting it out is what puts it in front of on_error."""
|
|
row_id, snowflake = participation
|
|
_pending(bot, 7, row_id)
|
|
|
|
def boom(event_type, reference_id):
|
|
raise ZeroDivisionError('a defect, not an outage')
|
|
|
|
monkeypatch.setattr(TeamTryoutsBot, '_attendance_record', staticmethod(boom))
|
|
|
|
with pytest.raises(ZeroDivisionError):
|
|
asyncio.run(
|
|
bot.handle_attendance_confirm(FakeReactor(snowflake), 7, row_id, FakeChannel())
|
|
)
|
|
|
|
def test_on_error_records_it_under_this_application_s_logger(self, bot, logs):
|
|
"""discord.py logs escaped event exceptions on the `discord` logger,
|
|
which logging_config does not wire to any of this application's
|
|
files. That is where these used to go."""
|
|
with logs.at_level(logging.ERROR, logger='app.discord_bot'):
|
|
try:
|
|
raise ZeroDivisionError('boom')
|
|
except ZeroDivisionError:
|
|
asyncio.run(bot.on_error('on_raw_reaction_add'))
|
|
|
|
assert 'on_raw_reaction_add' in logs.text
|
|
assert 'defect' in logs.text
|
|
assert any(record.exc_info for record in logs.records)
|
|
|
|
|
|
class TestTheQueueBoundary:
|
|
def test_queueing_never_raises_into_the_request_that_caused_it(self, monkeypatch, logs):
|
|
"""This runs in a Flask request thread, after the work it announces
|
|
is committed. A notification must not turn a finished page into a
|
|
500 — but it must not swallow the reason either."""
|
|
|
|
def no_bot():
|
|
raise RuntimeError('the bot could not be built')
|
|
|
|
monkeypatch.setattr(discord_bot, 'get_bot', no_bot)
|
|
|
|
with logs.at_level(logging.ERROR, logger='app.discord_bot'):
|
|
queued = discord_bot.send_schedule_notification(1, 'match', 'Finale', 'd', 't', 2)
|
|
|
|
assert queued is False
|
|
assert any(record.exc_info for record in logs.records)
|
|
|
|
def test_a_queued_notification_reaches_the_queue(self, bot, monkeypatch):
|
|
"""The premise of the test above: the same call succeeds normally."""
|
|
from queue import Queue
|
|
|
|
bot.message_queue = Queue()
|
|
monkeypatch.setattr(discord_bot, 'get_bot', lambda: bot)
|
|
|
|
assert discord_bot.send_schedule_notification(1, 'match', 'Finale', 'd', 't', 2) is True
|
|
assert bot.message_queue.get_nowait()['type'] == 'schedule_addition'
|
|
|
|
def test_an_unknown_item_type_is_named_and_dropped(self, bot, logs):
|
|
with logs.at_level(logging.ERROR, logger='app.discord_bot'):
|
|
asyncio.run(bot._dispatch({'type': 'invented', 'data': {}}))
|
|
|
|
assert 'invented' in logs.text
|
|
|
|
|
|
class TestTheSchedulerSaysWhetherItStarted:
|
|
class FailingScheduler:
|
|
@staticmethod
|
|
def add_job(*args, **kwargs):
|
|
raise RuntimeError('no event loop')
|
|
|
|
class WorkingScheduler:
|
|
def __init__(self):
|
|
self.started = False
|
|
|
|
def add_job(self, *args, **kwargs):
|
|
pass
|
|
|
|
def start(self):
|
|
self.started = True
|
|
|
|
def test_a_scheduler_that_will_not_start_is_visible_from_outside(self, bot, logs):
|
|
"""The other four /health keys all read green in this state: the
|
|
thread is alive, the gateway is up, and no reminder is ever sent."""
|
|
from zoneinfo import ZoneInfo
|
|
|
|
bot.scheduler = self.FailingScheduler()
|
|
bot.timezone = ZoneInfo('America/Toronto')
|
|
|
|
with logs.at_level(logging.ERROR, logger='app.discord_bot'):
|
|
asyncio.run(bot.start_scheduler())
|
|
|
|
assert bot.reminders_scheduled is False
|
|
assert 'No reminder will be sent' in logs.text
|
|
|
|
def test_a_scheduler_that_starts_says_so(self, bot, logs):
|
|
from zoneinfo import ZoneInfo
|
|
|
|
bot.scheduler = self.WorkingScheduler()
|
|
bot.timezone = ZoneInfo('America/Toronto')
|
|
|
|
asyncio.run(bot.start_scheduler())
|
|
|
|
assert bot.scheduler.started is True
|
|
assert bot.reminders_scheduled is True
|
|
|
|
|
|
class TestTwoReactionsOnOneMessage:
|
|
def test_the_second_one_finds_nothing_to_do(self, bot, db, participation, logs):
|
|
"""A confirm and a decline arrive together; both got past the
|
|
membership check in on_raw_reaction_add before either finished. The
|
|
loser used to raise KeyError inside the blanket handler, where it
|
|
read as an error with no consequence."""
|
|
row_id, snowflake = participation
|
|
_pending(bot, 7, row_id)
|
|
reactor = FakeReactor(snowflake)
|
|
|
|
asyncio.run(bot.handle_attendance_confirm(reactor, 7, row_id, FakeChannel()))
|
|
second = FakeChannel()
|
|
asyncio.run(bot.handle_attendance_decline(reactor, 7, row_id, second))
|
|
|
|
assert second.sent == [], 'the second reaction must not answer twice'
|
|
assert _confirmed(row_id) is True
|