GGThed
506a061405
QUA-003. La suite couvrait securite, autorisation, i18n, CSP et forme des requetes. Elle ne couvrait pas les regles pour lesquelles l application existe : qui entre dans une selection, combien, et ce qui cesse d etre modifiable une fois qu elle est finie. Aucun defaut corrige ici. Ce sont les garanties que les routes tiennent deja, consignees pour qu un remaniement ne puisse pas en laisser tomber une en silence. Deux meritent la lecture, parce qu elles ont l air d etre appliquees et ne le sont qu a moitie : - le plafond de joueurs est un count() suivi d un add(). Deux inscriptions simultanees passent toutes les deux le compte et s inserent toutes les deux. Il n y a aucune contrainte derriere dans le schema (DB-005/006, bloques sur Alembic). Le test enonce la regle pour une requete a la fois, et dit que le cas concurrent n est pas couvert ; - is_ended ferme sur end_date, avec repli sur date, et la comparaison est un < : une selection est encore ouverte le jour meme. C est un choix, et il est facile de l inverser par accident. Le test du plafond a failli passer a vide : une connexion ratee laisse elle aussi exactement une inscription. Il verifie donc que le second joueur est bien entre, et un test temoin montre que sans plafond il s inscrit. C est le piege consigne en vague E — un test qui remplace ou suppose quelque chose doit verifier que ce quelque chose a eu lieu. 501 tests.
299 lines
11 KiB
Python
299 lines
11 KiB
Python
"""Tryout business rules: who may register, and when things close (QUA-003).
|
|
|
|
The suite covered security, authorisation, i18n, CSP and query shape. What it
|
|
did not cover was the rules the application exists to enforce — who gets on a
|
|
tryout, how many, and what stops being editable once a tryout is over.
|
|
|
|
None of these is a defect being fixed. They are the guarantees the routes
|
|
already make, written down so that a refactor cannot quietly drop one. Two
|
|
are worth reading anyway, because they are the kind of rule that looks
|
|
enforced and is not:
|
|
|
|
- the player cap is a `count() >= max_players` check followed by an
|
|
`add()`. Two simultaneous registrations both pass the count and both
|
|
insert. There is no unique or check constraint behind it (DB-005, DB-006,
|
|
blocked on Alembic). The test states the single-request rule, and says
|
|
plainly that the concurrent one is not covered;
|
|
- `is_ended` closes on `end_date`, falling back to `date`. Its boundary is
|
|
a `<`, so a tryout is still open on its own last day. That is a choice,
|
|
and an easy one to invert by accident.
|
|
"""
|
|
|
|
from datetime import date, timedelta
|
|
|
|
import pytest
|
|
|
|
from app.extensions import db
|
|
from app.models import OrgTeam, Tryout, TryoutRegistration
|
|
|
|
|
|
@pytest.fixture
|
|
def make_tryout(app, make_user):
|
|
"""Build a tryout, owned by an admin, with the given attributes."""
|
|
|
|
def _make(created_by=None, **kwargs):
|
|
owner = created_by or make_user('admin')
|
|
with app.app_context():
|
|
org_team = OrgTeam(name=f'Team {owner}', created_by=owner)
|
|
db.session.add(org_team)
|
|
db.session.flush()
|
|
|
|
tryout = Tryout(
|
|
title=kwargs.pop('title', 'Spring'),
|
|
game=kwargs.pop('game', 'Valorant'),
|
|
date=kwargs.pop('date', date.today() + timedelta(days=30)),
|
|
created_by=owner,
|
|
target_org_team_id=org_team.id,
|
|
**kwargs,
|
|
)
|
|
db.session.add(tryout)
|
|
db.session.commit()
|
|
return tryout.id
|
|
|
|
return _make
|
|
|
|
|
|
def _registrations(app, tryout_id):
|
|
with app.app_context():
|
|
return TryoutRegistration.query.filter_by(tryout_id=tryout_id).count()
|
|
|
|
|
|
class TestIsEnded:
|
|
"""Pure date arithmetic on the model."""
|
|
|
|
def test_a_tryout_is_still_open_on_its_own_day(self):
|
|
tryout = Tryout(date=date.today())
|
|
|
|
assert tryout.is_ended is False, 'the comparison is <, not <=, on purpose'
|
|
|
|
def test_yesterday_is_ended(self):
|
|
tryout = Tryout(date=date.today() - timedelta(days=1))
|
|
|
|
assert tryout.is_ended is True
|
|
|
|
def test_the_end_date_wins_over_the_start_date(self):
|
|
"""A tryout that started last week and runs until next week is open."""
|
|
tryout = Tryout(
|
|
date=date.today() - timedelta(days=7),
|
|
end_date=date.today() + timedelta(days=7),
|
|
)
|
|
|
|
assert tryout.is_ended is False
|
|
|
|
def test_a_past_end_date_closes_it_even_if_the_start_is_future(self):
|
|
"""Nonsensical dates, but the property must not disagree with itself:
|
|
end_date is consulted first and answers alone."""
|
|
tryout = Tryout(
|
|
date=date.today() + timedelta(days=7),
|
|
end_date=date.today() - timedelta(days=1),
|
|
)
|
|
|
|
assert tryout.is_ended is True
|
|
|
|
|
|
class TestSelfRegistration:
|
|
def test_a_player_can_register(self, app, client, as_role, make_tryout):
|
|
as_role('player')
|
|
tryout_id = make_tryout()
|
|
|
|
client.post(f'/tryouts/{tryout_id}/register', follow_redirects=True)
|
|
|
|
assert _registrations(app, tryout_id) == 1
|
|
|
|
@pytest.mark.parametrize('role', ['coach', 'manager', 'admin', 'scout'])
|
|
def test_only_players_register_themselves(self, app, client, as_role, make_tryout, role):
|
|
as_role(role)
|
|
tryout_id = make_tryout()
|
|
|
|
client.post(f'/tryouts/{tryout_id}/register', follow_redirects=True)
|
|
|
|
assert _registrations(app, tryout_id) == 0
|
|
|
|
def test_registering_twice_adds_nothing(self, app, client, as_role, make_tryout):
|
|
as_role('player')
|
|
tryout_id = make_tryout()
|
|
|
|
client.post(f'/tryouts/{tryout_id}/register', follow_redirects=True)
|
|
client.post(f'/tryouts/{tryout_id}/register', follow_redirects=True)
|
|
|
|
assert _registrations(app, tryout_id) == 1
|
|
|
|
def test_a_completed_tryout_takes_no_more_registrations(
|
|
self, app, client, as_role, make_tryout
|
|
):
|
|
as_role('player')
|
|
tryout_id = make_tryout(status='completed')
|
|
|
|
client.post(f'/tryouts/{tryout_id}/register', follow_redirects=True)
|
|
|
|
assert _registrations(app, tryout_id) == 0
|
|
|
|
def test_a_tryout_in_progress_still_takes_registrations(
|
|
self, app, client, as_role, make_tryout
|
|
):
|
|
"""Deliberate: someone can join on the day."""
|
|
as_role('player')
|
|
tryout_id = make_tryout(status='in_progress')
|
|
|
|
client.post(f'/tryouts/{tryout_id}/register', follow_redirects=True)
|
|
|
|
assert _registrations(app, tryout_id) == 1
|
|
|
|
def test_the_player_cap_is_enforced(self, app, client, as_role, make_user, make_tryout):
|
|
"""One request at a time.
|
|
|
|
Two concurrent registrations both pass the count and both insert:
|
|
the cap is a read-then-write with nothing behind it in the schema.
|
|
Closing that needs a constraint, which needs Alembic (DB-005/006).
|
|
"""
|
|
tryout_id = make_tryout(max_players=1)
|
|
as_role('player')
|
|
client.post(f'/tryouts/{tryout_id}/register', follow_redirects=True)
|
|
|
|
second = app.test_client()
|
|
make_user('player', username='second')
|
|
signed_in = second.post(
|
|
'/auth/login', data={'username': 'second', 'password': 'Password123'}
|
|
)
|
|
# Without this the test passes for the wrong reason: a failed login
|
|
# also leaves exactly one registration behind.
|
|
assert signed_in.status_code in (301, 302), 'the second player never got in'
|
|
|
|
second.post(f'/tryouts/{tryout_id}/register', follow_redirects=True)
|
|
|
|
assert _registrations(app, tryout_id) == 1
|
|
|
|
def test_without_a_cap_the_second_player_does_get_in(
|
|
self, app, client, as_role, make_user, make_tryout
|
|
):
|
|
"""The control for the test above. If this one failed too, the cap
|
|
test would be proving nothing about the cap."""
|
|
tryout_id = make_tryout()
|
|
as_role('player')
|
|
client.post(f'/tryouts/{tryout_id}/register', follow_redirects=True)
|
|
|
|
second = app.test_client()
|
|
make_user('player', username='second')
|
|
second.post('/auth/login', data={'username': 'second', 'password': 'Password123'})
|
|
second.post(f'/tryouts/{tryout_id}/register', follow_redirects=True)
|
|
|
|
assert _registrations(app, tryout_id) == 2
|
|
|
|
|
|
class TestStaffRegistration:
|
|
def test_a_manager_can_register_a_player(self, app, client, as_role, make_user, make_tryout):
|
|
admin_id = as_role('admin')
|
|
player_id = make_user('player')
|
|
tryout_id = make_tryout(created_by=admin_id)
|
|
|
|
client.post(
|
|
f'/tryouts/{tryout_id}/register_player',
|
|
data={'player_id': str(player_id)},
|
|
follow_redirects=True,
|
|
)
|
|
|
|
assert _registrations(app, tryout_id) == 1
|
|
|
|
def test_only_a_player_can_be_registered(self, app, client, as_role, make_user, make_tryout):
|
|
"""A coach on the roster of a tryout would be evaluated as a
|
|
candidate for it."""
|
|
admin_id = as_role('admin')
|
|
coach_id = make_user('coach')
|
|
tryout_id = make_tryout(created_by=admin_id)
|
|
|
|
client.post(
|
|
f'/tryouts/{tryout_id}/register_player',
|
|
data={'player_id': str(coach_id)},
|
|
follow_redirects=True,
|
|
)
|
|
|
|
assert _registrations(app, tryout_id) == 0
|
|
|
|
def test_the_cap_applies_to_staff_registrations_too(
|
|
self, app, client, as_role, make_user, make_tryout
|
|
):
|
|
admin_id = as_role('admin')
|
|
tryout_id = make_tryout(created_by=admin_id, max_players=1)
|
|
first, second = make_user('player'), make_user('player')
|
|
|
|
for player_id in (first, second):
|
|
client.post(
|
|
f'/tryouts/{tryout_id}/register_player',
|
|
data={'player_id': str(player_id)},
|
|
follow_redirects=True,
|
|
)
|
|
|
|
assert _registrations(app, tryout_id) == 1
|
|
|
|
|
|
class TestStatusTransitions:
|
|
def _set_status(self, client, tryout_id, status):
|
|
return client.post(
|
|
f'/tryouts/{tryout_id}/status', data={'status': status}, follow_redirects=True
|
|
)
|
|
|
|
@pytest.mark.parametrize('status', ['upcoming', 'in_progress', 'completed'])
|
|
def test_the_three_states_are_reachable(self, app, client, as_role, make_tryout, status):
|
|
admin_id = as_role('admin')
|
|
tryout_id = make_tryout(created_by=admin_id)
|
|
|
|
self._set_status(client, tryout_id, status)
|
|
|
|
with app.app_context():
|
|
assert db.session.get(Tryout, tryout_id).status == status
|
|
|
|
def test_an_unknown_state_changes_nothing(self, app, client, as_role, make_tryout):
|
|
admin_id = as_role('admin')
|
|
tryout_id = make_tryout(created_by=admin_id)
|
|
|
|
self._set_status(client, tryout_id, 'cancelled')
|
|
|
|
with app.app_context():
|
|
assert db.session.get(Tryout, tryout_id).status == 'upcoming'
|
|
|
|
def test_a_player_cannot_move_a_tryout_along(self, app, client, as_role, make_tryout):
|
|
tryout_id = make_tryout()
|
|
as_role('player')
|
|
|
|
self._set_status(client, tryout_id, 'completed')
|
|
|
|
with app.app_context():
|
|
assert db.session.get(Tryout, tryout_id).status == 'upcoming'
|
|
|
|
|
|
class TestEndedTryoutsAreClosed:
|
|
"""`is_ended` is what stops a finished tryout being reopened by editing."""
|
|
|
|
def test_an_ended_tryout_cannot_be_edited(self, app, client, as_role, make_tryout):
|
|
admin_id = as_role('admin')
|
|
tryout_id = make_tryout(created_by=admin_id, date=date.today() - timedelta(days=2))
|
|
|
|
client.post(
|
|
f'/tryouts/{tryout_id}/edit',
|
|
data={'title': 'Reopened', 'game': 'Valorant', 'date': '2030-04-01'},
|
|
follow_redirects=True,
|
|
)
|
|
|
|
with app.app_context():
|
|
assert db.session.get(Tryout, tryout_id).title == 'Spring'
|
|
|
|
def test_no_match_can_be_scheduled_in_an_ended_tryout(self, app, client, as_role, make_tryout):
|
|
from app.models import Match
|
|
|
|
admin_id = as_role('admin')
|
|
tryout_id = make_tryout(created_by=admin_id, date=date.today() - timedelta(days=2))
|
|
|
|
client.post(
|
|
f'/matches/create/{tryout_id}',
|
|
data={
|
|
'title': 'Late scrim',
|
|
'date': '2030-04-01',
|
|
'start_time': '18:00',
|
|
'match_type': 'player_scrim',
|
|
},
|
|
follow_redirects=True,
|
|
)
|
|
|
|
with app.app_context():
|
|
assert Match.query.count() == 0
|