refactor(validation): un schema aux frontieres tryout et evaluation
ARCH-005, seconde moitie. Meme forme que pour les matchs : des champs lus a la main sur request.form, deux verifies et le reste cru sur parole. Cote tryout : - game pilote la liste des postes et les champs de gamertag montres au joueur qui s inscrit. Il etait accepte tel quel : une faute de frappe produisait une selection pour laquelle personne ne pouvait etre evalue ; - max_players etait int(x) if x else None — un 500 sur « twelve », et un -3 accepte sans broncher ; - coach_ids etait charge par User.id.in_(...) sans filtre de role. Une soumission fabriquee a la main pouvait donc nommer un joueur coach d une selection, ce qui est une attribution de droits : gerer la selection et evaluer ses joueurs. Ce n est pas un formulaire que l interface propose, et ca marchait. Cote evaluation, validate_score transformait tout ce qui sortait de 1..10 — 11, 0, « bien » — en None. Le critere disparaissait de la moyenne et la page annoncait l evaluation enregistree. Rien ne distinguait « non evalue » de « evalue, refuse, et oublie ». Le calcul de la moyenne remonte sur le modele, en Evaluation.overall_from et apply_scores. Il vivait dans la route, additionnant neuf variables locales, et ne pouvait pas etre exerce sans requete HTTP, session authentifiee et base — c est TEST-002, et c est pourquoi le calcul des scores n avait aucun test. Il en a maintenant six, sans rien monter. Une precision qui compte : aucun critere rempli donne None, pas 0. La grille commence a 1, donc un zero serait une note qu aucun joueur ne peut recevoir, et qui le classerait sous tout le monde dans la liste. Les neuf criteres sont ecrits en toutes lettres dans le schema plutot que generes depuis le modele — un schema se lit — et un test verifie que les deux listes coincident. C est la garde qui empeche la derive, pas l astuce. Douze chaines traduites, dont trois que pybabel avait devinees en fuzzy : une entree fuzzy est ignoree a l execution, le piege est consigne dans docs/translations.md. 30 tests neufs. 477 au total.
This commit is contained in:
@@ -0,0 +1,181 @@
|
||||
"""Evaluation scoring — the arithmetic and the boundary (ARCH-005, QUA-003).
|
||||
|
||||
The audit's note was short: "le calcul des scores d'évaluation n'a aucun
|
||||
test". It had two defects worth the trouble of writing some.
|
||||
|
||||
`validate_score` mapped anything outside 1..10 to None, so a coach typing 11
|
||||
or 0 had that criterion quietly dropped from the mean and was told the
|
||||
evaluation had been submitted. Nothing distinguished "not assessed" from
|
||||
"assessed, rejected, and forgotten".
|
||||
|
||||
And the mean itself lived inline in the route, summing nine named local
|
||||
variables. It could not be exercised without an HTTP request, an
|
||||
authenticated session and a database — which is TEST-002's point, and the
|
||||
reason it had no tests at all.
|
||||
"""
|
||||
|
||||
from datetime import date
|
||||
|
||||
import pytest
|
||||
from marshmallow import ValidationError
|
||||
|
||||
from app.extensions import db
|
||||
from app.models import Evaluation, OrgTeam, Tryout, TryoutRegistration
|
||||
from app.validators import EvaluationSchema
|
||||
|
||||
|
||||
class TestOverallScore:
|
||||
"""Pure arithmetic. No request, no session, no database."""
|
||||
|
||||
def test_the_overall_is_the_mean_of_what_was_scored(self):
|
||||
scores = dict.fromkeys(Evaluation.CRITERIA, 6)
|
||||
|
||||
assert Evaluation.overall_from(scores) == 6
|
||||
|
||||
def test_a_blank_criterion_is_left_out_rather_than_counted_as_zero(self):
|
||||
scores = dict.fromkeys(Evaluation.CRITERIA, None)
|
||||
scores['mecanics_score'] = 8
|
||||
scores['mental_score'] = 6
|
||||
|
||||
assert Evaluation.overall_from(scores) == 7
|
||||
|
||||
def test_nothing_scored_is_no_score_at_all(self):
|
||||
"""None, not 0. The scale starts at one, so a zero would be a score
|
||||
no player can be given, sorted below everyone in the listing."""
|
||||
assert Evaluation.overall_from(dict.fromkeys(Evaluation.CRITERIA, None)) is None
|
||||
assert Evaluation.overall_from({}) is None
|
||||
|
||||
def test_the_mean_is_not_rounded(self):
|
||||
scores = dict.fromkeys(Evaluation.CRITERIA, None)
|
||||
scores['mecanics_score'] = 7
|
||||
scores['mental_score'] = 8
|
||||
|
||||
assert Evaluation.overall_from(scores) == 7.5
|
||||
|
||||
def test_a_key_outside_the_criteria_is_ignored(self):
|
||||
"""apply_scores is handed the whole validated payload, comments and
|
||||
all. Only the nine criteria may reach the mean."""
|
||||
scores = dict.fromkeys(Evaluation.CRITERIA, 5)
|
||||
scores['comments'] = 'excellent'
|
||||
scores['position_recommendation'] = 'Support'
|
||||
|
||||
assert Evaluation.overall_from(scores) == 5
|
||||
|
||||
|
||||
class TestSchemaMatchesModel:
|
||||
def test_every_criterion_has_a_field(self):
|
||||
"""The nine are spelled out in the schema for readability. This is
|
||||
what stops the two lists drifting apart: a tenth criterion added to
|
||||
the model and forgotten in validators.py would otherwise be accepted
|
||||
unvalidated and dropped from the mean."""
|
||||
declared = set(EvaluationSchema().fields)
|
||||
|
||||
assert set(Evaluation.CRITERIA) <= declared
|
||||
|
||||
def test_no_field_claims_to_be_a_criterion_and_is_not(self):
|
||||
extra = set(EvaluationSchema().fields) - set(Evaluation.CRITERIA)
|
||||
|
||||
assert extra == {'comments', 'position_recommendation'}
|
||||
|
||||
|
||||
class TestScoreValidation:
|
||||
@pytest.mark.parametrize('bad', ['11', '0', '-3', 'good', '7.5'])
|
||||
def test_a_score_outside_the_scale_is_refused(self, bad):
|
||||
"""It used to become None: silently dropped, evaluation reported as
|
||||
submitted."""
|
||||
with pytest.raises(ValidationError):
|
||||
EvaluationSchema().load({'mecanics_score': bad})
|
||||
|
||||
@pytest.mark.parametrize('good', ['1', '10', '6'])
|
||||
def test_the_ends_of_the_scale_are_accepted(self, good):
|
||||
assert EvaluationSchema().load({'mecanics_score': good})['mecanics_score'] == int(good)
|
||||
|
||||
def test_a_blank_score_means_not_assessed(self):
|
||||
"""The form submits every criterion it renders, so an untouched one
|
||||
arrives as an empty string rather than not arriving."""
|
||||
data = EvaluationSchema().load({'mecanics_score': '', 'cohesion_score': '4'})
|
||||
|
||||
assert data['mecanics_score'] is None
|
||||
assert data['cohesion_score'] == 4
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def evaluation_setup(app, as_role, make_user):
|
||||
"""A coach who runs a tryout, and a player registered for it."""
|
||||
coach_id = as_role('coach')
|
||||
player_id = make_user('player')
|
||||
|
||||
with app.app_context():
|
||||
org_team = OrgTeam(name='Varsity', created_by=coach_id)
|
||||
db.session.add(org_team)
|
||||
db.session.flush()
|
||||
|
||||
tryout = Tryout(
|
||||
title='Spring',
|
||||
game='Valorant',
|
||||
date=date(2030, 4, 1),
|
||||
created_by=coach_id,
|
||||
target_org_team_id=org_team.id,
|
||||
)
|
||||
db.session.add(tryout)
|
||||
db.session.flush()
|
||||
|
||||
from app.models import User
|
||||
|
||||
# Attached through the m2m relation, not the inherited coach_id
|
||||
# column: ARCH-002 made the permission read the former.
|
||||
tryout.coaches = [db.session.get(User, coach_id)]
|
||||
db.session.add(TryoutRegistration(tryout_id=tryout.id, player_id=player_id))
|
||||
db.session.commit()
|
||||
|
||||
return {'tryout_id': tryout.id, 'player_id': player_id, 'coach_id': coach_id}
|
||||
|
||||
|
||||
class TestThroughTheForm:
|
||||
def _submit(self, client, setup, **fields):
|
||||
return client.post(
|
||||
f'/evaluations/{setup["tryout_id"]}/{setup["player_id"]}',
|
||||
data=fields,
|
||||
follow_redirects=True,
|
||||
)
|
||||
|
||||
def test_a_valid_evaluation_is_stored_with_its_mean(self, app, client, evaluation_setup):
|
||||
self._submit(
|
||||
client,
|
||||
evaluation_setup,
|
||||
mecanics_score='8',
|
||||
mental_score='6',
|
||||
comments='Solid.',
|
||||
)
|
||||
|
||||
with app.app_context():
|
||||
evaluation = Evaluation.query.one()
|
||||
assert evaluation.mecanics_score == 8
|
||||
assert evaluation.overall_score == 7
|
||||
assert evaluation.comments == 'Solid.'
|
||||
|
||||
def test_an_out_of_range_score_stores_nothing(self, app, client, evaluation_setup):
|
||||
self._submit(client, evaluation_setup, mecanics_score='11', mental_score='6')
|
||||
|
||||
with app.app_context():
|
||||
assert Evaluation.query.count() == 0
|
||||
|
||||
def test_a_second_submission_replaces_the_first(self, app, client, evaluation_setup):
|
||||
self._submit(client, evaluation_setup, mecanics_score='8', mental_score='8')
|
||||
self._submit(client, evaluation_setup, mecanics_score='4', mental_score='4')
|
||||
|
||||
with app.app_context():
|
||||
evaluation = Evaluation.query.one()
|
||||
assert evaluation.overall_score == 4
|
||||
|
||||
def test_clearing_a_score_clears_it_and_moves_the_mean(self, app, client, evaluation_setup):
|
||||
"""Every criterion is reassigned on edit, blanks included. Assigning
|
||||
only the ones that came back filled would leave the old value on the
|
||||
record and disagree with the mean beside it."""
|
||||
self._submit(client, evaluation_setup, mecanics_score='10', mental_score='4')
|
||||
self._submit(client, evaluation_setup, mecanics_score='', mental_score='4')
|
||||
|
||||
with app.app_context():
|
||||
evaluation = Evaluation.query.one()
|
||||
assert evaluation.mecanics_score is None
|
||||
assert evaluation.overall_score == 4
|
||||
Reference in New Issue
Block a user